[3487] Log error from the IfaceMgr if the interface is misconfigured/down.

diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index 0644237e9b9cf40e45aca90135e535c099bc47e3..10d2dabf6032b16c43cec1cae65c1c3b31152778 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/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.
 

diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes
index 43a24e5eb9a9848fc8e6cecc55bf3ff3d70fbba7..5ad2010700ebb633f136b4ecd6903f9b1b30049e 100644 (file)
--- a/src/bin/dhcp6/dhcp6_messages.mes
+++ b/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.
 

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index b618f05d178da185db313952f32aa3eb8d1de2e5..d9f5f5b98db5efe2b24b4110bfcee5dbc183a6ee 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/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

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index d3218f36bd6125fcc8b6bbff045e53ba1a710707..6713c32d0f5197e6d61a9decbf2a7fd6606f530e 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/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 (is 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 (is 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

diff --git a/src/lib/dhcp/tests/iface_mgr_test_config.cc b/src/lib/dhcp/tests/iface_mgr_test_config.cc
index 14beee867872e14970ea90409f10a297f14f56a7..7b285b147cf0675d2406e3caf708bfd28e003a61 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_test_config.cc
+++ b/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);
 }
 

diff --git a/src/lib/dhcp/tests/iface_mgr_test_config.h b/src/lib/dhcp/tests/iface_mgr_test_config.h
index d7d08df1b721df2ebe853b981af91f0f021e6a21..c1676e9fcfc6018041fbf10b45765b3692fc9a50 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_test_config.h
+++ b/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
     ///

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 2b0cd4c0ed011b35a10f5a14b2d2b211251a3f4d..ce50efe82b3dbe4a948071adb3c43669e80c338d 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/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.