Browse Source

[2765] Invoke error handler if the interface opening failed.

Marcin Siodelski 11 years ago
parent
commit
3ca6b9a20b

+ 4 - 0
src/bin/dhcp4/dhcp4_messages.mes

@@ -147,6 +147,10 @@ IPv4 DHCP server but it is not running.
 A debug message issued during startup, this indicates that the IPv4 DHCP
 A debug message issued during startup, this indicates that the IPv4 DHCP
 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
+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.
+
 % DHCP4_PACKET_PARSE_FAIL failed to parse incoming packet: %1
 % DHCP4_PACKET_PARSE_FAIL failed to parse incoming packet: %1
 The IPv4 DHCP server has received a packet that it is unable to
 The IPv4 DHCP server has received a packet that it is unable to
 interpret. The reason why the packet is invalid is included in the message.
 interpret. The reason why the packet is invalid is included in the message.

+ 8 - 1
src/bin/dhcp4/dhcp4_srv.cc

@@ -1241,7 +1241,9 @@ Dhcpv4Srv::openActiveSockets(const uint16_t port,
     // sockets are marked active or inactive.
     // sockets are marked active or inactive.
     // @todo Optimization: we should not reopen all sockets but rather select
     // @todo Optimization: we should not reopen all sockets but rather select
     // those that have been affected by the new configuration.
     // those that have been affected by the new configuration.
-    if (!IfaceMgr::instance().openSockets4(port, use_bcast)) {
+    isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
+        boost::bind(&Dhcpv4Srv::ifaceMgrSocket4ErrorHandler, _1);
+    if (!IfaceMgr::instance().openSockets4(port, use_bcast, error_handler)) {
         LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN);
         LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN);
     }
     }
 }
 }
@@ -1335,6 +1337,11 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf,
     return (offset);
     return (offset);
 }
 }
 
 
+void
+Dhcpv4Srv::ifaceMgrSocket4ErrorHandler(const std::string& errmsg) {
+    // Log the reason for socket opening failure and return.
+    LOG_WARN(dhcp4_logger, DHCP4_OPEN_SOCKET_FAIL).arg(errmsg);
+}
 
 
 }   // namespace dhcp
 }   // namespace dhcp
 }   // namespace isc
 }   // namespace isc

+ 9 - 0
src/bin/dhcp4/dhcp4_srv.h

@@ -381,6 +381,15 @@ private:
     /// @return Option that contains netmask information
     /// @return Option that contains netmask information
     static OptionPtr getNetmaskOption(const Subnet4Ptr& subnet);
     static OptionPtr getNetmaskOption(const Subnet4Ptr& subnet);
 
 
+    /// @brief Implements the error handler for socket open failure.
+    ///
+    /// This callback function is installed on the @c isc::dhcp::IfaceMgr
+    /// when IPv4 sockets are being open. When socket fails to open for
+    /// any reason, this function is called. It simply logs the error message.
+    ///
+    /// @param errmsg An error message containing a cause of the failure.
+    static void ifaceMgrSocket4ErrorHandler(const std::string& errmsg);
+
     /// @brief Allocation Engine.
     /// @brief Allocation Engine.
     /// Pointer to the allocation engine that we are currently using
     /// Pointer to the allocation engine that we are currently using
     /// It must be a pointer, because we will support changing engines
     /// It must be a pointer, because we will support changing engines

+ 20 - 0
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -191,6 +191,26 @@ TEST_F(Dhcpv4SrvTest, basic) {
     EXPECT_TRUE(naked_srv->getServerID());
     EXPECT_TRUE(naked_srv->getServerID());
 }
 }
 
 
+// This test verifies that exception is not thrown when an error occurs during
+// opening sockets. This test forces an error by adding a fictious interface
+// to the IfaceMgr. An attempt to open socket on this interface must always
+// fail. The DHCPv4 installs the error handler function to prevent exceptions
+// being thrown from the openSockets4 function.
+// @todo The server tests for socket should be extended but currently the
+// ability to unit test the sockets code is somewhat limited.
+TEST_F(Dhcpv4SrvTest, openActiveSockets) {
+    ASSERT_NO_THROW(CfgMgr::instance().activateAllIfaces());
+
+    Iface iface("bogusiface", 255);
+    iface.flag_loopback_ = false;
+    iface.flag_up_ = true;
+    iface.flag_running_ = true;
+    iface.inactive4_ = false;
+    iface.addAddress(IOAddress("192.0.0.0"));
+    IfaceMgr::instance().addInterface(iface);
+    ASSERT_NO_THROW(Dhcpv4Srv::openActiveSockets(DHCP4_SERVER_PORT, false));
+}
+
 // This test verifies that the destination address of the response
 // This test verifies that the destination address of the response
 // message is set to giaddr, when giaddr is set to non-zero address
 // message is set to giaddr, when giaddr is set to non-zero address
 // in the received message.
 // in the received message.

+ 1 - 0
src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -44,6 +44,7 @@ Dhcpv4SrvTest::Dhcpv4SrvTest()
     pool_ = Pool4Ptr(new Pool4(IOAddress("192.0.2.100"), IOAddress("192.0.2.110")));
     pool_ = Pool4Ptr(new Pool4(IOAddress("192.0.2.100"), IOAddress("192.0.2.110")));
     subnet_->addPool(pool_);
     subnet_->addPool(pool_);
 
 
+    CfgMgr::instance().deleteActiveIfaces();
     CfgMgr::instance().deleteSubnets4();
     CfgMgr::instance().deleteSubnets4();
     CfgMgr::instance().addSubnet4(subnet_);
     CfgMgr::instance().addSubnet4(subnet_);
 
 

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

@@ -424,7 +424,7 @@ public:
     /// @return true if direct response is supported.
     /// @return true if direct response is supported.
     bool isDirectResponseSupported() const;
     bool isDirectResponseSupported() const;
 
 
-    /// @brief Returns interface with specified interface index
+    /// @brief Returns interfac specified interface index
     ///
     ///
     /// @param ifindex index of searched interface
     /// @param ifindex index of searched interface
     ///
     ///
@@ -732,6 +732,15 @@ public:
     /// not having address assigned.
     /// not having address assigned.
     void setMatchingPacketFilter(const bool direct_response_desired = false);
     void setMatchingPacketFilter(const bool direct_response_desired = false);
 
 
+    /// @brief Adds an interface to list of known interfaces.
+    ///
+    /// @param iface reference to Iface object.
+    /// @note This function must be public because it has to be callable
+    /// from unit tests.
+    void addInterface(const Iface& iface) {
+        ifaces_.push_back(iface);
+    }
+
     /// A value of socket descriptor representing "not specified" state.
     /// A value of socket descriptor representing "not specified" state.
     static const int INVALID_SOCKET = -1;
     static const int INVALID_SOCKET = -1;
 
 
@@ -776,13 +785,6 @@ protected:
     /// @return socket descriptor
     /// @return socket descriptor
     int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port);
     int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port);
 
 
-    /// @brief Adds an interface to list of known interfaces.
-    ///
-    /// @param iface reference to Iface object.
-    void addInterface(const Iface& iface) {
-        ifaces_.push_back(iface);
-    }
-
     /// @brief Detects network interfaces.
     /// @brief Detects network interfaces.
     ///
     ///
     /// This method will eventually detect available interfaces. For now
     /// This method will eventually detect available interfaces. For now