BIND 10 #3173: Add support to perfdhcp to acquire IPv6 prefixes (Prefix Delegation)

[BIND 10 Development]

#3173: Add support to perfdhcp to acquire IPv6 prefixes (Prefix Delegation)
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  enhancement   |  marcin
            Priority:  high          |                       Status:
           Component:  perfdhcp      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130918
         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 tmark):

 * owner:  tmark => marcin


Comment:

 command_options.cc

 Question: Why 'e' for the new parameter letter? Why not 'l' for lease type
 or 't' for type?  There doesn't seem to anything particularly mnemonic
 about 'e'.
 (At least not in English ;) )

 ----------------------------------------------------------------------------

 Question: Why are template files restricted to address-only?

 +    check(!getTemplateFiles().empty() && (getLeaseType() !=
 ADDRESS_ONLY),
 +          "template files may be only used with '-e address-only'");


 ----------------------------------------------------------------------------
 command_options.cc

 Should this method:

 {{{
 std::string CommandOptions::LeaseType::toText()
 }}}

 throw rather than return a string that says "unknown value" or something?
 Usually things like this are used for log messages and such.  It can be
 inconvenient if they throw.

 ----------------------------------------------------------------------------
 command_options_test.cc

 TEST(LeaseTypeTest, toText)

 Does not test the case of an invalid value.

 ----------------------------------------------------------------------------

 test_control.h

 I think this comment:
  {{{
     /// \todo In the future it is planned to add support for the perfdhcp
 to
     /// request address and prefix in the same DHCP message (request both
     /// IA_NA and IA_PD options). In this case, this function will have to
     /// be extended to copy both IA_NA and IA_PD options.
 }}}

 is obsolete. The method already copies address, prefix, or both.

 ----------------------------------------------------------------------------

 test_control_unittest.cc

 The two new tests, Packet6ExchangePrefixDelegation and
 Packet6ExchangeAddressAndPrefix do not really explain what exactly is
 being tested.  I think a little additional commentary for each would be
 helpful.

 ----------------------------------------------------------------------------

 test_control_unittest.cc

 I do not see a packet exchange test for command line  "-e address_only".

 --------------------------------------------------------------------------

 I still get one cppcheck error:

 {{{
 tests/tools/perfdhcp/tests/stats_mgr_unittest.cc:91: check_fail: Function
 parameter 'xchg_type' should be passed by reference.
 (performance,passedByValue)
 }}}

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