[1555] Address review comments.

doc/guide/bind10-guide.xml

@@ -3599,7 +3599,7 @@ $</screen>
         will be available. It will look similar to this:
 <screen>
 &gt; <userinput>config show Dhcp4</userinput>
-Dhcp4/interface/	list	(default)
+Dhcp4/interfaces/	list	(default)
 Dhcp4/renew-timer	1000	integer	(default)
 Dhcp4/rebind-timer	2000	integer	(default)
 Dhcp4/valid-lifetime	4000	integer	(default)
@@ -4420,7 +4420,7 @@ Dhcp4/renew-timer	1000	integer	(default)
         will be available. It will look similar to this:
 <screen>
 &gt; <userinput>config show Dhcp6</userinput>
-Dhcp6/interface/	list	(default)
+Dhcp6/interfaces/	list	(default)
 Dhcp6/renew-timer	1000	integer	(default)
 Dhcp6/rebind-timer	2000	integer	(default)
 Dhcp6/preferred-lifetime	3000	integer	(default)

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -253,42 +253,5 @@ ControlledDhcpv4Srv::execDhcpv4ServerCommand(const std::string& command_id,
     }
 }
 
-void
-ControlledDhcpv4Srv::openActiveSockets(const uint16_t port,
-                                       const bool use_bcast) {
-    IfaceMgr::instance().closeSockets();
-
-    // Get the reference to the collection of interfaces. This reference should
-    // be valid as long as the program is run because IfaceMgr is a singleton.
-    // Therefore we can safely iterate over instances of all interfaces and
-    // modify their flags. Here we modify flags which indicate whether socket
-    // should be open for a particular interface or not.
-    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
-    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
-         iface != ifaces.end(); ++iface) {
-        Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName());
-        if (CfgMgr::instance().isActiveIface(iface->getName())) {
-            iface_ptr->inactive4_ = false;
-            LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE)
-                .arg(iface->getFullName());
-
-        } else {
-            // For deactivating interface, it should be sufficient to log it
-            // on the debug level because it is more useful to know what
-            // interface is activated which is logged on the info level.
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
-                      DHCP4_DEACTIVATE_INTERFACE).arg(iface->getName());
-            iface_ptr->inactive4_ = true;
-
-        }
-    }
-    // Let's reopen active sockets. openSockets4 will check internally whether
-    // sockets are marked active or inactive.
-    if (!IfaceMgr::instance().openSockets4(port, use_bcast)) {
-        LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN);
-    }
-}
-
-
 };
 };

src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -130,16 +130,6 @@ protected:
     /// when there is a new command or configuration sent over msgq.
     static void sessionReader(void);
 
-    /// @brief Open sockets which are marked as active in @c CfgMgr.
-    ///
-    /// This function reopens sockets according to the current settings in the
-    /// Configuration Manager. It holds the list of the interfaces which server
-    /// should listen on. This function will open sockets on these interfaces
-    /// only. This function is not exception safe.
-    ///
-    /// @param port UDP port on which server should listen.
-    /// @param use_bcast should broadcast flags be set on the sockets.
-    static void openActiveSockets(const uint16_t port, const bool use_bcast);
 
     /// @brief IOService object, used for all ASIO operations.
     isc::asiolink::IOService io_service_;

src/bin/dhcp4/dhcp4_srv.cc

@@ -821,5 +821,42 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid) {
     }
 }
 
+void
+Dhcpv4Srv::openActiveSockets(const uint16_t port,
+                             const bool use_bcast) {
+    IfaceMgr::instance().closeSockets();
+
+    // Get the reference to the collection of interfaces. This reference should
+    // be valid as long as the program is run because IfaceMgr is a singleton.
+    // Therefore we can safely iterate over instances of all interfaces and
+    // modify their flags. Here we modify flags which indicate whether socket
+    // should be open for a particular interface or not.
+    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
+    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
+         iface != ifaces.end(); ++iface) {
+        Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName());
+        if (CfgMgr::instance().isActiveIface(iface->getName())) {
+            iface_ptr->inactive4_ = false;
+            LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE)
+                .arg(iface->getFullName());
+
+        } else {
+            // For deactivating interface, it should be sufficient to log it
+            // on the debug level because it is more useful to know what
+            // interface is activated which is logged on the info level.
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
+                      DHCP4_DEACTIVATE_INTERFACE).arg(iface->getName());
+            iface_ptr->inactive4_ = true;
+
+        }
+    }
+    // Let's reopen active sockets. openSockets4 will check internally whether
+    // sockets are marked active or inactive.
+    if (!IfaceMgr::instance().openSockets4(port, use_bcast)) {
+        LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN);
+    }
+}
+
+
 }   // namespace dhcp
 }   // namespace isc

src/bin/dhcp4/dhcp4_srv.h

@@ -125,6 +125,9 @@ public:
     ///
     /// @brief Get UDP port on which server should listen.
     ///
+    /// Typically, server listens on UDP port number 67. Other ports are used
+    /// for testing purposes only.
+    ///
     /// @return UDP port on which server should listen.
     uint16_t getPort() const {
         return (port_);
@@ -139,6 +142,17 @@ public:
     }
     //@}
 
+    /// @brief Open sockets which are marked as active in @c CfgMgr.
+    ///
+    /// This function reopens sockets according to the current settings in the
+    /// Configuration Manager. It holds the list of the interfaces which server
+    /// should listen on. This function will open sockets on these interfaces
+    /// only. This function is not exception safe.
+    ///
+    /// @param port UDP port on which server should listen.
+    /// @param use_bcast should broadcast flags be set on the sockets.
+    static void openActiveSockets(const uint16_t port, const bool use_bcast);
+
 protected:
 
     /// @brief verifies if specified packet meets RFC requirements

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -1785,10 +1785,10 @@ TEST_F(Dhcp4ParserTest, selectedInterfaces) {
 TEST_F(Dhcp4ParserTest, allInterfaces) {
     ConstElementPtr x;
     // This configuration specifies two interfaces on which server should listen
-    // but it also includes keyword 'all'. This keyword switches server into the
+    // but it also includes asterisk. The asterisk switches server into the
     // mode when it listens on all interfaces regardless of what interface names
     // were specified in the "interfaces" parameter.
-    string config = "{ \"interfaces\": [ \"eth0\",\"*\",\"eth1\" ],"
+    string config = "{ \"interfaces\": [ \"eth0\", \"*\", \"eth1\" ],"
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"valid-lifetime\": 4000 }";

src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -254,40 +254,5 @@ ControlledDhcpv6Srv::execDhcpv6ServerCommand(const std::string& command_id,
     }
 }
 
-void
-ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) {
-    IfaceMgr::instance().closeSockets();
-
-    // Get the reference to the collection of interfaces. This reference should be
-    // valid as long as the program is run because IfaceMgr is a singleton.
-    // Therefore we can safely iterate over instances of all interfaces and modify
-    // their flags. Here we modify flags which indicate wheter socket should be
-    // open for a particular interface or not.
-    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
-    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
-         iface != ifaces.end(); ++iface) {
-        Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName());
-        if (CfgMgr::instance().isActiveIface(iface->getName())) {
-            iface_ptr->inactive4_ = false;
-            LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE)
-                .arg(iface->getFullName());
-
-        } else {
-            // For deactivating interface, it should be sufficient to log it
-            // on the debug level because it is more useful to know what
-            // interface is activated which is logged on the info level.
-            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC,
-                      DHCP6_DEACTIVATE_INTERFACE).arg(iface->getName());
-            iface_ptr->inactive6_ = true;
-
-        }
-    }
-    // Let's reopen active sockets. openSockets6 will check internally whether
-    // sockets are marked active or inactive.
-    if (!IfaceMgr::instance().openSockets6(port)) {
-        LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN);
-    }
-}
-
 };
 };

src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -128,16 +128,6 @@ protected:
     /// when there is a new command or configuration sent over msgq.
     static void sessionReader(void);
 
-    /// @brief Open sockets which are marked as active in @c CfgMgr.
-    ///
-    /// This function reopens sockets according to the current settings in the
-    /// Configuration Manager. It holds the list of the interfaces which server
-    /// should listen on. This function will open sockets on these interfaces
-    /// only. This function is not exception safe.
-    ///
-    /// @param port UDP port on which server should listen.
-    static void openActiveSockets(const uint16_t port);
-
     /// @brief IOService object, used for all ASIO operations.
     isc::asiolink::IOService io_service_;
 

src/bin/dhcp6/dhcp6_srv.cc

@@ -1103,5 +1103,40 @@ Dhcpv6Srv::processInfRequest(const Pkt6Ptr& infRequest) {
     return reply;
 }
 
+void
+Dhcpv6Srv::openActiveSockets(const uint16_t port) {
+    IfaceMgr::instance().closeSockets();
+
+    // Get the reference to the collection of interfaces. This reference should be
+    // valid as long as the program is run because IfaceMgr is a singleton.
+    // Therefore we can safely iterate over instances of all interfaces and modify
+    // their flags. Here we modify flags which indicate wheter socket should be
+    // open for a particular interface or not.
+    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
+    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
+         iface != ifaces.end(); ++iface) {
+        Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName());
+        if (CfgMgr::instance().isActiveIface(iface->getName())) {
+            iface_ptr->inactive4_ = false;
+            LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE)
+                .arg(iface->getFullName());
+
+        } else {
+            // For deactivating interface, it should be sufficient to log it
+            // on the debug level because it is more useful to know what
+            // interface is activated which is logged on the info level.
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC,
+                      DHCP6_DEACTIVATE_INTERFACE).arg(iface->getName());
+            iface_ptr->inactive6_ = true;
+
+        }
+    }
+    // Let's reopen active sockets. openSockets6 will check internally whether
+    // sockets are marked active or inactive.
+    if (!IfaceMgr::instance().openSockets6(port)) {
+        LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN);
+    }
+}
+
 };
 };

src/bin/dhcp6/dhcp6_srv.h

@@ -89,6 +89,9 @@ public:
 
     /// @brief Get UDP port on which server should listen.
     ///
+    /// Typically, server listens on UDP port 547. Other ports are only
+    /// used for testing purposes.
+    ///
     /// This accessor must be public because sockets are reopened from the
     /// static configuration callback handler. This callback handler invokes
     /// @c ControlledDhcpv4Srv::openActiveSockets which requires port parameter
@@ -100,6 +103,16 @@ public:
         return (port_);
     }
 
+    /// @brief Open sockets which are marked as active in @c CfgMgr.
+    ///
+    /// This function reopens sockets according to the current settings in the
+    /// Configuration Manager. It holds the list of the interfaces which server
+    /// should listen on. This function will open sockets on these interfaces
+    /// only. This function is not exception safe.
+    ///
+    /// @param port UDP port on which server should listen.
+    static void openActiveSockets(const uint16_t port);
+
 protected:
 
     /// @brief verifies if specified packet meets RFC requirements

src/lib/dhcp/iface_mgr.cc

@@ -296,7 +296,7 @@ bool IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast) {
 
         if (iface->flag_loopback_ ||
             !iface->flag_up_ ||
-            !iface->flag_running_,
+            !iface->flag_running_ ||
             iface->inactive4_) {
             continue;
         }
@@ -363,7 +363,7 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
 
         if (iface->flag_loopback_ ||
             !iface->flag_up_ ||
-            !iface->flag_running_,
+            !iface->flag_running_ ||
             iface->inactive6_) {
             continue;
         }

src/lib/dhcpsrv/cfgmgr.h

@@ -254,23 +254,30 @@ public:
     /// @param iface A name of the interface being added to the listening set.
     void addActiveIface(const std::string& iface);
 
-    /// @brief Configures the server to listen on all interfaces.
+    /// @brief Sets the flag which indicates that server is supposed to listen
+    /// on all available interfaces.
     ///
-    /// This function configrues the server to listen on all available
-    /// interfaces regardless of the interfaces specified with
-    /// @c CfgMgr::addListeningInterface.
+    /// This function does not close or open sockets. It simply marks that
+    /// server should start to listen on all interfaces the next time sockets
+    /// are reopened. Server should examine this flag when it gets reconfigured
+    /// and configuration changes the interfaces that server should listen on.
     void activateAllIfaces();
 
-    /// @brief Clear the collection of the interfaces that server is configured
-    /// to use to listen for incoming requests.
+    /// @brief Clear the collection of the interfaces that server should listen
+    /// on.
     ///
     /// Apart from clearing the list of interfaces specified with
     /// @c CfgMgr::addListeningInterface, it also disables listening on all
-    /// interfaces if it has been enabled using @c CfgMgr::listenAllInterfaces.
+    /// interfaces if it has been enabled using
+    /// @c CfgMgr::activateAllInterfaces.
+    /// Likewise @c CfgMgr::activateAllIfaces, this function does not close or
+    /// open sockets. It marks all interfaces inactive for DHCP traffic.
+    /// Server should examine this new setting when it attempts to
+    /// reopen sockets (as a result of reconfiguration).
     void deleteActiveIfaces();
 
-    /// @brief Check if server is configured to listen on the interface which
-    /// name is specified as an argument to this function.
+    /// @brief Check if specified interface should be used to listen to DHCP
+    /// traffic.
     ///
     /// @param iface A name of the interface to be checked.
     ///