[bind10-dev] Comments on proposed hooks framework

[Michal 'vorner' Vaner]

Hello

On Fri, May 10, 2013 at 07:57:08PM +0200, Tomek Mrugalski wrote:
> Only the first 3 are mandatory, rest are dependent on what the specific
> library needs.

I don't think libload() and libunload() should be mandatory. If the function is
not present, it is just not called.

> - think about a situation in a year or two: there will be many user
> libraries, some developed by us, but many more developed by others.
> There would be many similar, but slightly different code bases in them
> each doing very similar thing. Each registration code base could be
> potentially managed by a different organization.

You seem to be optimistic. I don't see much adoption of Bind10. I see nearly no
community. Who will write them?

> 2. The position parameter. When the user library calls add(), there is
> the last parameter called position. It is intended to specify calling
> order, 0 implies first, 1 implies second etc. It will not work. What if
> there are 2 libraries and both want to be called first? What if there is
> a single library that wants to be called 5th? I think we should get rid
> of that parameter altogether. If there are objections and we want to
> keep it (Michal mentioned an example of a wrapper library that wants to
> call decrypt before and encrypt after), then it should be renamed to
> requested_order and have values: FIRST, LAST, DEFAULT (which implies the
> order in which the libraries are loaded).

I proposed to include priority, not position. So you would say to want to be
called very early (0) or very late (1000) or somewhere in the middle. The
advantage is that you can position relatively against the hooks you already
know.

> 3. return status. Two of return codes will not work: SUCCESS_SKIP and
> COMPLETE_SKIP. Server can't skip certain operations and then continue as
> if nothing happened. Let me give specific examples. DHCPv6 packet is
> received. The first callout is dhcp6_buffer_rcvd(). The next server
> operation is parsing that buffer and create option objects. If we skip
> it there is no further processing possible. Another example. Once the
> packet is parsed, the server calls selectSubnet callout before selecting
> subnet on its own. Later address and other configuration parameters are
> assigned from that subnet. If we can't select subnet, it is not possible
> to further process the packet.

They will and they are needed. Of course, they need to be used with caution and
we should warn about them. But if someone really wants to break the code from
the hook, he has always a chance to do so. If he skips in place where it is not
possible, he will know right away.

My example is, let's say I want to test some new format of DNS packet. It's
modern to use XML everywhere, so I want to encode the data into XML (let alone
the fact that this looks like a stupid, even stupid ideas should be
implementable by the hooks). So what I need to do is:
 • Do my own parsing.
 • Prevent the usual parsing from happening. Otherwise, it'll fail and throw
   exception, going directly to the FORMERR handler.

I could want to replace whatever part of the processing, not only parsing.

> 5. Calling a callout in the current form does not allow to set const
> parameters. They are essential. Let me give you a specific example: when
> DHCP server parses a packet, it selects a subnet from which it will
> allocate leases and assign other options. Hook will allow the user
> library to pick a different subnet. We need to provide a list of
> alternatives, so the callout will know what it can choose from. That
> information is part of the configuration. It must be read-only at this
> stage. If the user library want to update the configuration, it must use
> bindctl interface. It must not modify the configuration in the middle of
> packet processing.

I'm not against const types, or something.


Also, another reason for the more flexible approach: I'd like a hook library to
define a callout position too (so there could be hooks into other hooks).

With regards

-- 
Something is obviously wrong. The thing works.

Michal 'vorner' Vaner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <https://lists.isc.org/pipermail/bind10-dev/attachments/20130513/6f507604/attachment.bin>