Browse Source

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

Marcin Siodelski 10 years ago
parent
commit
fdc4cee68e

+ 1 - 1
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
 A debug message issued during startup, this indicates that the DHCPv4
 server is about to open sockets on the specified port.
 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
 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.
 for the failure is appended as an argument of the log message.
 
 

+ 1 - 1
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
 A debug message issued during startup, this indicates that the IPv6 DHCP
 server is about to open sockets on the specified port.
 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
 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.
 for the failure is appended as an argument of the log message.
 
 

+ 45 - 10
src/lib/dhcp/iface_mgr.cc

@@ -430,8 +430,6 @@ void IfaceMgr::stubDetectIfaces() {
     addInterface(iface);
     addInterface(iface);
 }
 }
 
 
-
-
 bool
 bool
 IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
 IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
                        IfaceMgrErrorMsgCallback error_handler) {
                        IfaceMgrErrorMsgCallback error_handler) {
@@ -442,11 +440,30 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
          iface != ifaces_.end();
          iface != ifaces_.end();
          ++iface) {
          ++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;
             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();
         Iface::AddressCollection addrs = iface->getAddresses();
@@ -533,11 +550,29 @@ IfaceMgr::openSockets6(const uint16_t port,
          iface != ifaces_.end();
          iface != ifaces_.end();
          ++iface) {
          ++iface) {
 
 
-        if (iface->flag_loopback_ ||
-            !iface->flag_up_ ||
-            !iface->flag_running_ ||
-            iface->inactive6_) {
+        if (iface->inactive6_) {
             continue;
             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
         // Open unicast sockets if there are any unicast addresses defined

+ 9 - 8
src/lib/dhcp/iface_mgr.h

@@ -714,6 +714,11 @@ public:
 
 
     /// @brief Opens IPv6 sockets on detected interfaces.
     /// @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
     /// 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
     /// failure to open a socket on a particular interface doesn't cause a
     /// fatal error and sockets should be opened on remaining interfaces.
     /// fatal error and sockets should be opened on remaining interfaces.
@@ -754,14 +759,10 @@ public:
 
 
     /// @brief Opens IPv4 sockets on detected interfaces.
     /// @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
     /// The type of the socket being open depends on the selected Packet Filter
     /// represented by a class derived from @c isc::dhcp::PktFilter abstract
     /// represented by a class derived from @c isc::dhcp::PktFilter abstract

+ 6 - 2
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);
     Iface iface(name, ifindex);
     if (name == "lo") {
     if (name == "lo") {
         iface.flag_loopback_ = true;
         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;
     iface.flag_multicast_ = true;
     // On BSD systems, the SO_BINDTODEVICE option is not supported.
     // 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_broadcast_ = false;
     iface.flag_up_ = true;
     iface.flag_up_ = true;
     iface.flag_running_ = true;
     iface.flag_running_ = true;
-    iface.inactive4_ = false;
-    iface.inactive6_ = false;
     return (iface);
     return (iface);
 }
 }
 
 

+ 2 - 1
src/lib/dhcp/tests/iface_mgr_test_config.h

@@ -176,7 +176,8 @@ public:
     /// - loopback flag if interface name is "lo"
     /// - loopback flag if interface name is "lo"
     /// - up always true
     /// - up always true
     /// - running 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
     /// - multicast always to true
     /// - broadcast always to false
     /// - broadcast always to false
     ///
     ///

+ 28 - 4
src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -246,6 +246,12 @@ public:
         Iface iface(name, ifindex);
         Iface iface(name, ifindex);
         if (name == "lo") {
         if (name == "lo") {
             iface.flag_loopback_ = true;
             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;
         iface.flag_multicast_ = true;
         // On BSD systems, the SO_BINDTODEVICE option is not supported.
         // On BSD systems, the SO_BINDTODEVICE option is not supported.
@@ -256,8 +262,6 @@ public:
         iface.flag_broadcast_ = false;
         iface.flag_broadcast_ = false;
         iface.flag_up_ = true;
         iface.flag_up_ = true;
         iface.flag_running_ = true;
         iface.flag_running_ = true;
-        iface.inactive4_ = false;
-        iface.inactive6_ = false;
         return (iface);
         return (iface);
     }
     }
 
 
@@ -1465,8 +1469,18 @@ TEST_F(IfaceMgrTest, openSockets4IfaceDown) {
                          FlagRunning(false), FlagInactive4(false),
                          FlagRunning(false), FlagInactive4(false),
                          FlagInactive6(false));
                          FlagInactive6(false));
     ASSERT_FALSE(IfaceMgr::instance().getIface("eth0")->flag_up_);
     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,
     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.
     // There should be no socket on eth0 open, because interface was down.
     EXPECT_TRUE(IfaceMgr::instance().getIface("eth0")->getSockets().empty());
     EXPECT_TRUE(IfaceMgr::instance().getIface("eth0")->getSockets().empty());
@@ -1844,11 +1858,21 @@ TEST_F(IfaceMgrTest, openSockets6IfaceDown) {
     // - is active for both v4 and v6
     // - is active for both v4 and v6
     ifacemgr.setIfaceFlags("eth0", false, false, false, false, false);
     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.
     // Simulate opening sockets using the dummy packet filter.
     bool success = false;
     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);
     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.
     // Check that we have correct number of sockets on each interface.
     checkSocketsCount6(*ifacemgr.getIface("lo"), 0);
     checkSocketsCount6(*ifacemgr.getIface("lo"), 0);
     // There should be no sockets on eth0 because interface is down.
     // There should be no sockets on eth0 because interface is down.