From: Marcin Siodelski Date: Mon, 11 Aug 2014 14:37:15 +0000 (+0200) Subject: [3487] Log error from the IfaceMgr if the interface is misconfigured/down. X-Git-Tag: trac3482_base~48^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=fdc4cee68e6cb7bd500fd8a31ca65c4c47af9995;p=thirdparty%2Fkea.git [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 0644237e9b..10d2dabf60 100644 --- 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 43a24e5eb9..5ad2010700 100644 --- 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 b618f05d17..d9f5f5b98d 100644 --- 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 d3218f36bd..6713c32d0f 100644 --- 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 14beee8678..7b285b147c 100644 --- 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 d7d08df1b7..c1676e9fcf 100644 --- 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 2b0cd4c0ed..ce50efe82b 100644 --- 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.