BIND 10 #2313: Custom namespaces (including vendor spaces)

[BIND 10 Development]

#2313: Custom namespaces (including vendor spaces)
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  task          |  stephen
            Priority:  medium        |                       Status:
           Component:  dhcp          |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130122
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  marcin => stephen


Comment:

 Replying to [comment:7 stephen]:
 > reviewed commit 3e100854e0a85d215d614729f6d429340be1ead2
 >
 > '''src/lib/dhcpsrv/option_space.cc'''[[BR]]
 > Line 33: s/Allowed digits/Allowed characters/

 Corrected.

 >
 > validateName(): Is a name beginning with a hyphen allowed? (I can see
 the code allows its, but semantically, is it allowed?)

 I don't have strong opinion here. Initially, I thought it is up to the
 user to have hyphen at the beginning (like -isc or _isc). Eventually, I
 decided NOT to allow hyphens at the beginning and at the end of the option
 space.

 >
 > '''src/lib/dhcpsrv/option_space.h'''[[BR]]
 > s/disencouraged/discouraged/

 Corrected.

 >
 > In the header for !OptionSpace, suggest when talking about OptionSpace6,
 you mention that it extends the !OptionSpace class.  (My first thought was
 "why not call the V4 class OptionSpace4 for symmetry.)

 I added a note in the class header that explains why I named it
 OptionSpace rather than OptionSpace6.

 >
 > Would be logical to group clearVendorSpace() with isVendorSpace() and
 setVendorSpace() (instead of having getName() between them.)

 Reordered.

 >
 > The documentation seems to imply that the "vendor space" attribute is
 only relevant in V6, but that it is in !OptionSpace (and not OptionSpace6)
 because of the convenience of being able to point to OptionSpace6 objects
 with a pointer to !OptionSpace.  If thid is correct, can the documentation
 for the xxxVendorSpace() methods make that clear (perhaps group the
 methods using the Doxygen "!///@{" construct and attach documentation to
 the group).  Also clarify that point in the header for the OptionSpace6
 class.

 No, that's not quite right. The "vendor space" attribute applies to both
 !OptionSpace and !OptionSpace6 as we have to deal with vendor-specific
 options in both DHCPv4 and DHCPv6. The OptionSpace6 only adds the
 enterprise number which is used when the "vendor space" flag is set. For
 DHCPv4 the enterprise number is NOT needed/used so it is not defined in
 the base class. However, for DHCPv4 we still want to differentiate vendor
 options space and non-vendor option space and the "vendor space" flag is
 used for this differentiation.

 I added some more (hopefully useful) comments that are supposed to make it
 a bit more clear.

 >
 > '''src/lib/dhcpsrv/tests/option_space_unittest.cc'''[[BR]]
 > The optionSpace6 test creates only !OptionSpace objects, and no
 OptionSpace6 objects.  A mixture is required (as the !OptionSpace header
 suggests that both can represent a V6 Option Space).

 The !OptionSpace6Test creates only !OptionSpace6 objects and
 !OptionSpaceTest creates only !OptionSpace objects. Is this what you
 meant?

 I have two classes being tested and thus I created two separate tests for
 them !OptionSpaceTest and !OptionSpace6Test. The first of them fully tests
 the features of !OptionSpace class and the latter fully tests the features
 of OptionSpace6 class (except static function that is inherited from the
 base class). Why would I have to mix them together?

 ---

 Proposed !ChangeLog entry:

 {{{
 XYZ.    [func]          marcin
         Created OptionSpace and OptionSpace6 classes to represent DHCP
         option spaces. The option spaces are used to group instances
         and definitions of options having uniqe codes. The special type
         of option space is so called "vendor specific option space"
         which groups sub-options sent within Vendor Encapsilated Options.
         The new classes are not used yet but they will be used once
         option spaces creation with configuration manager is implemented.
         (Trac #2313, git abc)

 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/2313#comment:8>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development