Fwd: Re: Thoughts on DHCP Option Definition

[Tomek Mrugalski]

On 01.10.2012 11:26, Stephen Morris wrote:
> On 28/09/12 13:35, Tomek Mrugalski wrote:
>> On 28.09.2012 13:45, Stephen Morris wrote:
>>> All option definitions - both pre-defined ones and 
>>> user/vendor-defined ones - would be in the configuration 
>>> database.  The pre-defined options definition area would be 
>>> populated from a .spec file, the other two areas would be empty. 
>>> Although pre-defined options could be defined in the code, it 
>>> seems to me that defining everything in the .spec file would
>>> mean just one code path for all options.  It would also make it
>>> easier to add new pre-defined options - just update the .spec
>>> file.
>> That seems like a wrong place for it. Pre-defined options is not 
>> part of the configuration. It is a configuration template. I
>> really think it should be in a separate place. If you put it in the
>> wrong place, then you get the sort of problems you are about to get
>> into now (how to make flexible configuration inflexible, see your
>> next paragraph).
> 
> The idea was to reduce the amount of code being written - all option 
> definitions are in the same place, so the same piece of code loads
> the definitions of pre-defined options and user/vendor-supplied
> ones.
> 
> However, I'm prepared to accept that the ability of the user to
> modify the definitions of pre-defined option makes that unworkable.
I don't know. I don't want to force anything here, but just have a
strong feeling that letting this ability open will hit us badly sooner
or later. Sure, we will discourage redefining options, will print out
warnings, but do think it will prevent users from doing that? Probably
most of them, but not all.

>>> When reading the options, user option definitions would override 
>>> vendor-option definitions, which would in turn override 
>>> pre-defined option definitions (any overriding being marked by a 
>>> warning message written to the log file).  At the end of the 
>>> configuration process, the option definitions would be stored in 
>>> some form of option interpreter object.
>> An approach that prints out warnings when used properly is flawed. 
>> We talked about this with Marcin before. If the format user 
>> specifies is covered by one of our option classes (integers list, 
>> addresses list, fqdn etc), then we use it, because of their 
>> simplicity and effectiveness. For stranger formats, there should
>> be one custom class that supports every possible combination,
>> albeit at the cost of increased complexity and lowered
>> performance.
> 
> I think we're at cross purposes here.  The idea is that if a user 
> defines a new option with the same code as an existing definition,
> the system would issue a warning.
I think that it should be an error and the configuration should be
rejected. I have serious doubts that there exist any valid case, when
user would need to redefine our definitions. If there are such cases, we
should not provide our definitions in the first place.

> Of course, there could be a legitimate reason for the user doing
> such a thing, but the situation is sufficiently unusual for the
> system to issue a warning message.
If there really is one, I'd like to hear it. This essantially means that
users know better than devs how an option should look like. If there are
mistakes in option definitions, a bug report should be submitted.
That is true for: vendor-spec options, for user-defined options and new
options that were standardized, but we haven't written definitions for.
User should be able to define them. I have serious doubts that letting
such a way will have any benefits, but I'm pretty sure of serious issues
that will come out of it. Let me give you an example:

The frequent request in DHCPv6 world is to do host reservations based on
MAC. There are valid reasons, why there is no such thing, because it
will not work in many cases. User thinks that he is smarter than devs,
so he redefines client-id as DUID that is truncated
to 6 bytes, so in most cases he will get an option with MAC address.
Although we would print out warnings about redefinition, logging
interface is flexible in BIND10, so he manages to disable those
warnings. He thinks that he solved the problem and happily moves on.
Everyone is happy till the moment, when someone with entreprise-id DUID
visits his network and then suddently BIND10 will start handling
duplicate addresses. This is one example of how user could hurt himself,
without any easy way to help him or even diagnose, what went wrong.

>>> The option definitions are the first part of the configuration
>>> to be processed.  During the rest of the configuration
>>> processing, when the setting of an option is encountered, the
>>> option name and its value is passed to the interpreter: the
>>> return value is a vector of bytes representing the wire format of
>>> the option.
>> I would very much prefer to avoid this, because it would throw
>> away the whole stack we wrote so far. Option definitions should
>> return a pointer to the Option factory instead. That will be used
>> to parse incoming options. Once value for known options are
>> specified, those factories should be used to instantiate objects,
>> using aforementioned factories. The returned Option object will
>> then be used around, stuffed into server responses, passed to hooks
>> etc. Immediately producing on-wire format would be much less useful
>> and more difficult to work with.
> 
> Fair enough.
> 
> 
>>> * Do we want to allow user and vendor-defined options to override
>>> pre-defined ones?
>> Why would you want to predefine vendor options if you can't 
>> possibly know its format?
> 
> If a site bought a new piece of equipment, the vendor could supply a 
> bindctl script file to provide the definition of any new options.
Yes. That explains the rationale why we should provide a way to define
vendor options. I do not question that. I just object the fact that *we*
should define it beforehand, using some kind of boilerplate.

If you still think that allowing users to redefine existing options,
here's another rationale against it: degraded performance. Right now in
the code I can assume that option with code 1 or 2 (server-id or
client-id) is really an option with DUID. If we allow user to redefine
that, then every time I use it, I would need to use dynamic cast and
check if the option really contains a duid and not something else. This
will slow the code down for everyone, regardless if they override
standard definitions or not.

In my opinion the solution here is very simple:
- whatever we define, remains unchangeable
- we do not define vendor-spec options
- users (or vendors) may provide their own definitions for vendor options

Tomek