pass interface information to handlers?

[dulitz]

Most handlers don't care what interface the packet came in on, but a few do. And while it's possible to run a completely different handler configuration for each listener, if you have 48 ports and each one needs one handler configured differently, that's a lot of repetition (and leads to unreadable config files).

@Natolumin what would you think about adding Setup6WithContext and Setup4WithContext to the Plugin struct:

type Plugin struct {
    // keep all existing properties
    Setup6WithContext SetupFunc6WithContext
    Setup4WithContext SetupFunc4WithContext
}
type SetupFunc6WithContext func(args ...string) (handler.ContextHandler6, error)

type ExtendedContext struct {
    interfacename string  // e.g.
}
type ContextHandler6 func(req, resp dhcpv6.DHCPv6, xreq, xresp ExtendedContext) (dhcpv6.DHCPv6, ExtendedContext, bool)

If the new properties are nil (as they would be for all existing plugins) the existing behavior is maintained: call Setup6 and Setup4 and install the handlers they return. If the WithContext functions are non-nil, the "no context" functions must be nil.

In future if we want to widen the interface more we could just add properties to ExtendedContext which would be transparent to everyone.

Other alternatives considered:

• Just widen handler.Handler{6,4} to include ExtendedContext. But few plugins care about extended context, as proven by the fact no one has asked for this yet. Maybe making the 90% use case more complicated isn't worth it.
• Provide a function that takes req and resp as arguments and returns ExtendedContext. Then you have to maintain a map of some kind for every packet processed which also doesn't seem worth it.
• Have Plugin.Setup{6,4} return an interface with a Handler() function, and do a type assertion on the returned interface to see if it has an ExtendedHandler() function also. This seems complicated and also confusing because the interface value will have both Handler() and ExtendedHandler() methods. Do you call them both or not?

[Natolumin]

If / When we eventually get to rework the handlers, we'll definitely pass some extended context. There's some previous discussion in #50 for example

I'm strongly opposed to making multiple options and signatures for handlers, that will lead to fragmentation and be even more difficult to follow. In your options, sending the context to every handler is what I would go for, handlers can always ignore useless parameters.
I actually like your second option most (maybe with passing a handle object, instead of trying to remap from the request itself), as it makes extending this easier, and maybe feasible through plugins themselves in the future

[dulitz]

If you'll merge it, I'll implement your preferred design.

Your suggestion of allowing plugins to update context themselves, via a handle object, is very interesting and reminds me of coredns. It might also satisfy the use case of #111 .