[libvirt] RFC: dhcp <option> element revisited

[Laine Stump]

On 03/05/2013 03:48 PM, Laine Stump wrote:
> Before 1.0.3, Pieter Hollants kindly implemented the XML I had earlier
> suggested to support configure-by-number dhcp options in libvirt networks.
>
>   https://www.redhat.com/archives/libvir-list/2013-February/msg01251.html
>
> I also posted a followup patch for a "force" attribute to options:
>
>    https://www.redhat.com/archives/libvir-list/2013-February/msg01349.html
>
> I actually pushed a slightly fixed version of Pieter's patch, and was
> looking for an ACK to mine when Eric questioned whether it was all
> really ready to push since we hadn't implemented *named* options
> (although there is no official RFC standard for the names of options,
> dnsmasq uses a pretty good set of lowercase-dash-separated names that
> make sense, and would certainly make it easier to decode the config). At
> the time I had just started trying to write such a patch, and was
> realizing that the XML we'd come up with didn't have any provision to
> easily deal with an option that took a list of values (multiple IP
> addresses, multiple domain names, etc); for numbered options we could
> just punt on this and say "put the multiple values in the single 'value'
> attribute, separated by commas", but we all know that's a cop-out, so I
> decided to temporarily pull Pieter's patch to avoid regrets over getting
> something suboptimal in an official release.
>
> That's all the history. Now on to discussing how we should modify these
> patches to be exactly what we want.
>
> I tried digging down into the data of the options to try and make an
> exhaustive XML representation, with the results below (these would all
> go directly inside the <dhcp> element of a libvirt network). But it's
> starting to seem like the deeper I go, the deeper it gets (the
> classless-static-route option is the current "deepest", and I haven't
> even attempted to do anything about vendor-specific options), so I'm
> starting to wonder where to draw the line - see the "STEPPING BACK"
> section at the bottom.
>
>
> 1) numeric option, single value:
>
>     <option code='119'>
>        <value type='domain' data='a.example.com'/>
>     </option>
>
>
> (Note that I changed "number" to "code" after seeing that suggestion in
> [1] because that's what it's called in the RFC).
>
> (Also note that the actual encoding of option 119's data is more
> complicated than described here [2], but it's complicated enough that I
> think whatever dhcp server implementation is underneath libvirt will
> need to know the details anyway and decided what we should go for with
> the "type" attribute is enough for libvirt to validate the input, not
> necessarily to encode it into a DHCP response packet; any translation
> from the validated input will be handled internally to the backend
> driver (currently bridge_driver.c which uses dnsmasq) (I'm actually
> wondering if type is needed in the XML at all, since the type of any
> piece of data is always either implicit, or unknown (in which case we
> just accept any text and pass it through to the backend driver).)
>
> 2) numeric option, multiple values:
>
>     <option code='119' force='yes'>
>       <value type='domain' data='a.example.com'/>
>       <value type='domain' data='b.example.com'/>
>       <value type='domain' data='c.example.com'/>
>     </option>
>
> 3) named option, single simple value:
>
>     <option name='ip-forward-enable'>
>       <value type='boolean' data='yes'/>
>     </option>
>
> 4) named option, multiple simple values:
>
>     <option name='dns-server'>
>       <value type='ipv4Addr' data='10.1.1.1'/>
>       <value type='ipv4Addr' data='10.1.1.2'/>
>     </option>
>     <option
>
> (for all named options and known numeric options, the "type" field would
> be optional, as libvirt will already have an internal table associating
> option names/code with the type of data, as well as whether or not
> multiples are allowed)
>
>
> 5) named option, multiple compound values:
>
>     <option name='classless-static-route'>
>        <value type='compound'>
>          <value type='ipv4Addr'   data='1.2.3.0'/>
>          <value type='ipv4Prefix' data='24'/>
>          <value type='ipv4Addr'   data='192.168.122.5'/>
>        </value>
>        <value type='compound'>
>          <value type='ipv4Addr'   data='1.2.4.0'/>
>          <value type='ipv4Prefix' data='24'/>
>          <value type='ipv4Addr'   data='192,.168.122.6'/>
>        </value>
>     </option>

After 9 days and no replies (except a short conversation with DV on
IRC), I guess it's time to take some good ol' 'Murican unilateral action :-)

a) I do think we need to allow for multiple values in a single option,
and to be safe we should make that a separate case from multiple options
with the same code and a single value each. This means that:

      <option code='10' value='xyzzy'/>
      <option code='10' value='plugh'/>

won't work. We'll need this:

      <option code='10'>
        <value data='xyzzy'/>
        <value data='plugh'/>
      </option>

b) The more I look at the "type='ipv4Addr'" attribute, the more I hate
it; I think the type of value needs to be implicit in the element, and
I'm thinking that should be done with differently-named attributes. So
instead of:

       <value type='compound'>
         <value type='ipv4Addr'   data='1.2.4.0'/>
         <value type='ipv4Prefix' data='24'/>
         <value type='ipv4Addr'   data='192.168.122.6'/>
       </value>


I think this would be better:

       <value address='1.2.4.0' prefix='24' gateway='192.168.122.6'/>

Basically the attribute *name* would imply the type of data. The list of
attributes in <value> is:

    text
    path
    domain
    boolean
    number
    address
    prefix
    gateway

The parser would have no idea about which of those attributes are
appropriate for which option code - errors about that would come out
when the network driver was translating the network object into a
dnsmasq (or whatever) configuration file.

c) As DV pointed out to me, the option code is really our connection to
the RFC, so we must implement that in order to have something
unambiguous that will be recognized by any possible dhcp implementation.

d) I think we do need a whitelist, and we should minimally populate it
to begin with, using a table as I described in the earlier mail:


>   uint8 code;
>   const char *name;   /* could be "", in which case we don't recognize a
> name for it */
>   unsigned int flags; /* whether or not multiple values are allowed,
> name recognized by dnsmasq */
>   /* some representation of type here */
>