BIND 10 #1708: Integrate DHCP6 process startup into BIND 10

[BIND 10 Development]

#1708: Integrate DHCP6 process startup into BIND 10
-------------------------------------+-------------------------------------
                   Reporter:  tomek  |                 Owner:  tomek
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:  Sprint-
                   Priority:         |  DHCP-20120730
  medium                             |            Resolution:
                  Component:  dhcp6  |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DHCP
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => tomek


Comment:

 '''src/bin/dhcp4/dhcp4_srv.cc'''[[BR]]
 Would suggest that "shutdown_" be initialized to "false" on entry to the
 Dhcp4Srv constructor (or set "false" in the "try" block).  That way, the
 "return" in the "catch" clause becomes unnecessary.

 '''src/bin/dhcp4/main.cc'''[[BR]]
 Trivial point: suggest use "port_number < 1" rather than "port_number <=
 0" in the test for the "-p" switch.  The limits against which port_number
 is checked are then 1 and 65535, which are the values mentioned in the
 error message.

 Another trivial point: suggest making both the order of the options in
 "getopt()" and the order that they are processed in the "case" statement
 alphabetical order - it is more logical and marginally easier to read.

 '''src/bin/dhcp4/tests/dhcp4_test.py'''[[BR]]
 Comments in test_skip_msgq refer to invalid port number, not disabling of
 the message queue.

 '''src/bin/dhcp6/dhcp6_srv.cc'''[[BR]]
 Would suggest that "shutdown_" be initialized to "false" on entry to the
 Dhcp4Srv constructor (or set "false" in the "try" block).  That way, the
 "return" in the "catch" clause becomes unnecessary.

 '''src/bin/dhcp6/main.cc'''[[BR]]
 > Where do you see Dhcpv6Srv started after ControlledDhcpv6Srv is created
 and run()?
 In the bit that reads:
 {{{
         ControlledDhcpv6Srv* server = new
 ControlledDhcpv6Srv(port_number);
         server->run();
         delete server;
         server = NULL;

         cout << "[b10-dhcp6] Initiating DHCPv6 operation." << endl;

         /// @todo: pass verbose to the actual server once logging is
 implemented
         Dhcpv6Srv* srv = new Dhcpv6Srv(port_number);

         srv->run();
 }}}
 (in commit f788c425a50d504d7d23d3945b67f2ad25513495).  However that code
 has now been replaced.

 Same comments apply here that were made in src/bin/dhcp4/main.cc.


 '''src/bin/dhcp6/tests/dhcp6_test.py'''[[BR]]
 Comments in test_skip_msgq refer to invalid port number, not the disabling
 of the message queue.

 '''src/lib/dhcp/iface_mgr.cc'''[[BR]]
 "if (maxfd < session_socket_)" needs braces around the "if" body.

 Suggest adding a "TODO" for refactoring IfaceMgr::receive6() - it's fairly
 long.  At the very least it could be separated into a method that
 identifies the socket on which the information is received and a method
 that reads the data.

 I don't need to see it again after those changes have been made, and so
 you can commit.

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