[master] Merge branch 'trac3487'

src/bin/dhcp4/dhcp4_messages.mes

@@ -207,7 +207,7 @@ received message is dropped.
 A debug message issued during startup, this indicates that the DHCPv4
 server is about to open sockets on the specified port.
 
-% DHCP4_OPEN_SOCKET_FAIL failed to create socket: %1
+% DHCP4_OPEN_SOCKET_FAIL failed to open socket: %1
 A warning message issued when IfaceMgr fails to open and bind a socket. The reason
 for the failure is appended as an argument of the log message.
 

src/bin/dhcp4/dhcp4_srv.cc

@@ -1806,7 +1806,11 @@ Dhcpv4Srv::openActiveSockets(const uint16_t port,
                       << " instance of the interface when DHCPv4 server was"
                       << " trying to reopen sockets after reconfiguration");
         }
-        if (CfgMgr::instance().isActiveIface(iface->getName())) {
+        // Ignore loopback interfaces.
+        if (iface_ptr->flag_loopback_) {
+            iface_ptr->inactive4_ = true;
+
+        } else if (CfgMgr::instance().isActiveIface(iface->getName())) {
             iface_ptr->inactive4_ = false;
             LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE)
                 .arg(iface->getFullName());

src/bin/dhcp6/dhcp6_messages.mes

@@ -304,7 +304,7 @@ configured to receive the traffic.
 A debug message issued during startup, this indicates that the IPv6 DHCP
 server is about to open sockets on the specified port.
 
-% DHCP6_OPEN_SOCKET_FAIL failed to create socket: %1
+% DHCP6_OPEN_SOCKET_FAIL failed to open socket: %1
 A warning message issued when IfaceMgr fails to open and bind a socket. The reason
 for the failure is appended as an argument of the log message.
 

src/bin/dhcp6/dhcp6_srv.cc

@@ -2440,7 +2440,12 @@ Dhcpv6Srv::openActiveSockets(const uint16_t port) {
                       << " instance of the interface when DHCPv6 server was"
                       << " trying to reopen sockets after reconfiguration");
         }
-        if (CfgMgr::instance().isActiveIface(iface->getName())) {
+
+        // Ignore loopback interfaces.
+        if (iface_ptr->flag_loopback_) {
+            iface_ptr->inactive6_ = true;
+
+        } else  if (CfgMgr::instance().isActiveIface(iface->getName())) {
             iface_ptr->inactive6_ = false;
             LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE)
                 .arg(iface->getFullName());

src/lib/dhcp/iface_mgr.cc

@@ -430,8 +430,6 @@ void IfaceMgr::stubDetectIfaces() {
     addInterface(iface);
 }
 
-
-
 bool
 IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
                        IfaceMgrErrorMsgCallback error_handler) {
@@ -442,11 +440,30 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
          iface != ifaces_.end();
          ++iface) {
 
-        if (iface->flag_loopback_ ||
-            !iface->flag_up_ ||
-            !iface->flag_running_ ||
-            iface->inactive4_) {
+        // If the interface is inactive, there is nothing to do. Simply
+        // proceed to the next detected interface.
+        if (iface->inactive4_) {
             continue;
+
+        } else {
+            // If the interface has been specified in the configuration that
+            // it should be used to listen the DHCP traffic we have to check
+            // that the interface configuration is valid and that the interface
+            // is not a loopback interface. In both cases, we want to report
+            // that the socket will not be opened.
+            if (iface->flag_loopback_) {
+                IFACEMGR_ERROR(SocketConfigError, error_handler,
+                               "must not open socket on the loopback"
+                               " interface " << iface->getName());
+                continue;
+
+            } else if (!iface->flag_up_ || !iface->flag_running_) {
+                IFACEMGR_ERROR(SocketConfigError, error_handler,
+                               "the interface " << iface->getName()
+                               << " is down or has no usable IPv4"
+                               " addresses configured");
+                continue;
+            }
         }
 
         Iface::AddressCollection addrs = iface->getAddresses();
@@ -533,11 +550,29 @@ IfaceMgr::openSockets6(const uint16_t port,
          iface != ifaces_.end();
          ++iface) {
 
-        if (iface->flag_loopback_ ||
-            !iface->flag_up_ ||
-            !iface->flag_running_ ||
-            iface->inactive6_) {
+        if (iface->inactive6_) {
             continue;
+
+        } else {
+            // If the interface has been specified in the configuration that
+            // it should be used to listen the DHCP traffic we have to check
+            // that the interface configuration is valid and that the interface
+            // is not a loopback interface. In both cases, we want to report
+            // that the socket will not be opened.
+            if (iface->flag_loopback_) {
+                IFACEMGR_ERROR(SocketConfigError, error_handler,
+                               "must not open socket on the loopback"
+                               " interface " << iface->getName());
+                continue;
+
+            } else if (!iface->flag_up_ || !iface->flag_running_) {
+                IFACEMGR_ERROR(SocketConfigError, error_handler,
+                               "the interface " << iface->getName()
+                               << " is down or has no usable IPv6"
+                               " addresses configured");
+                continue;
+            }
+
         }
 
         // Open unicast sockets if there are any unicast addresses defined

src/lib/dhcp/iface_mgr.h

@@ -714,6 +714,11 @@ public:
 
     /// @brief Opens IPv6 sockets on detected interfaces.
     ///
+    /// This method opens sockets only on interfaces which have the
+    /// @c inactive6_ field set to false (are active). If the interface is active
+    /// but it is not running, it is down, or is a loopback interface,
+    /// an error is reported.
+    ///
     /// On the systems with multiple interfaces, it is often desired that the
     /// failure to open a socket on a particular interface doesn't cause a
     /// fatal error and sockets should be opened on remaining interfaces.
@@ -754,14 +759,10 @@ public:
 
     /// @brief Opens IPv4 sockets on detected interfaces.
     ///
-    /// This function attempts to open sockets on all interfaces which have been
-    /// detected by @c IfaceMgr and meet the following conditions:
-    /// - interface is not a local loopback,
-    /// - interface is running (connected),
-    /// - interface is up,
-    /// - interface is active, e.g. selected from the configuration to be used
-    /// to listen DHCPv4 messages,
-    /// - interface has an IPv4 address assigned.
+    /// This method opens sockets only on interfaces which have the
+    /// @c inactive4_ field set to false (are active). If the interface is active
+    /// but it is not running, it is down, or is a loopback interface,
+    /// an error is reported.
     ///
     /// The type of the socket being open depends on the selected Packet Filter
     /// represented by a class derived from @c isc::dhcp::PktFilter abstract

src/lib/dhcp/tests/iface_mgr_test_config.cc

@@ -74,6 +74,12 @@ IfaceMgrTestConfig::createIface(const std::string &name, const int ifindex) {
     Iface iface(name, ifindex);
     if (name == "lo") {
         iface.flag_loopback_ = true;
+        // Don't open sockets on the loopback interface.
+        iface.inactive4_ = true;
+        iface.inactive6_ = true;
+    } else {
+        iface.inactive4_ = false;
+        iface.inactive6_ = false;
     }
     iface.flag_multicast_ = true;
     // On BSD systems, the SO_BINDTODEVICE option is not supported.
@@ -84,8 +90,6 @@ IfaceMgrTestConfig::createIface(const std::string &name, const int ifindex) {
     iface.flag_broadcast_ = false;
     iface.flag_up_ = true;
     iface.flag_running_ = true;
-    iface.inactive4_ = false;
-    iface.inactive6_ = false;
     return (iface);
 }
 

src/lib/dhcp/tests/iface_mgr_test_config.h

@@ -176,7 +176,8 @@ public:
     /// - loopback flag if interface name is "lo"
     /// - up always true
     /// - running always true
-    /// - inactive always to false
+    /// - inactive4 set to false for non-loopback interface
+    /// - inactive6 set to false for non-loopback interface
     /// - multicast always to true
     /// - broadcast always to false
     ///

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -246,6 +246,12 @@ public:
         Iface iface(name, ifindex);
         if (name == "lo") {
             iface.flag_loopback_ = true;
+            // Don't open sockets on loopback interface.
+            iface.inactive4_ = true;
+            iface.inactive6_ = true;
+        } else {
+            iface.inactive4_ = false;
+            iface.inactive6_ = false;
         }
         iface.flag_multicast_ = true;
         // On BSD systems, the SO_BINDTODEVICE option is not supported.
@@ -256,8 +262,6 @@ public:
         iface.flag_broadcast_ = false;
         iface.flag_up_ = true;
         iface.flag_running_ = true;
-        iface.inactive4_ = false;
-        iface.inactive6_ = false;
         return (iface);
     }
 
@@ -1465,8 +1469,18 @@ TEST_F(IfaceMgrTest, openSockets4IfaceDown) {
                          FlagRunning(false), FlagInactive4(false),
                          FlagInactive6(false));
     ASSERT_FALSE(IfaceMgr::instance().getIface("eth0")->flag_up_);
+
+    // Install an error handler before trying to open sockets. This handler
+    // should be called when the IfaceMgr fails to open socket on an interface
+    // on which the server is configured to listen.
+    isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
+        boost::bind(&IfaceMgrTest::ifaceMgrErrorHandler, this, _1);
+
     ASSERT_NO_THROW(IfaceMgr::instance().openSockets4(DHCP4_SERVER_PORT, true,
-                                                      NULL));
+                                                      error_handler));
+    // Since the interface is down, an attempt to open a socket should result
+    // in error.
+    EXPECT_EQ(1, errors_count_);
 
     // There should be no socket on eth0 open, because interface was down.
     EXPECT_TRUE(IfaceMgr::instance().getIface("eth0")->getSockets().empty());
@@ -1844,11 +1858,21 @@ TEST_F(IfaceMgrTest, openSockets6IfaceDown) {
     // - is active for both v4 and v6
     ifacemgr.setIfaceFlags("eth0", false, false, false, false, false);
 
+    // Install an error handler before trying to open sockets. This handler
+    // should be called when the IfaceMgr fails to open socket on eth0.
+    isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
+        boost::bind(&IfaceMgrTest::ifaceMgrErrorHandler, this, _1);
+
     // Simulate opening sockets using the dummy packet filter.
     bool success = false;
-    ASSERT_NO_THROW(success = ifacemgr.openSockets6(DHCP6_SERVER_PORT));
+    ASSERT_NO_THROW(success = ifacemgr.openSockets6(DHCP6_SERVER_PORT,
+                                                    error_handler));
     EXPECT_TRUE(success);
 
+    // Opening socket on the interface which is not configured, should
+    // result in error.
+    EXPECT_EQ(1, errors_count_);
+
     // Check that we have correct number of sockets on each interface.
     checkSocketsCount6(*ifacemgr.getIface("lo"), 0);
     // There should be no sockets on eth0 because interface is down.