BIND 10 trac2765, updated. 5b9c261d8b66d61f1df4a088e9a283855332c392 [2765] Fixed error handling in the IfaceMgr::openSockets4.
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Nov 29 12:57:35 UTC 2013
The branch, trac2765 has been updated
via 5b9c261d8b66d61f1df4a088e9a283855332c392 (commit)
via 3ca6b9a20ba4d9931a686fd5b684166d413d700f (commit)
from b52a45ce15d894e540d0c8011786a913630dbffc (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 5b9c261d8b66d61f1df4a088e9a283855332c392
Author: Marcin Siodelski <marcin at isc.org>
Date: Fri Nov 29 13:57:18 2013 +0100
[2765] Fixed error handling in the IfaceMgr::openSockets4.
commit 3ca6b9a20ba4d9931a686fd5b684166d413d700f
Author: Marcin Siodelski <marcin at isc.org>
Date: Fri Nov 29 13:30:03 2013 +0100
[2765] Invoke error handler if the interface opening failed.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_messages.mes | 4 ++++
src/bin/dhcp4/dhcp4_srv.cc | 9 ++++++++-
src/bin/dhcp4/dhcp4_srv.h | 9 +++++++++
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 20 ++++++++++++++++++++
src/bin/dhcp4/tests/dhcp4_test_utils.cc | 1 +
src/lib/dhcp/iface_mgr.cc | 14 +++++++++-----
src/lib/dhcp/iface_mgr.h | 18 ++++++++++--------
7 files changed, 61 insertions(+), 14 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index 1bfce66..ae6c9bb 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/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
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
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.
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 4ac24dd..8a73b1a 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1241,7 +1241,9 @@ Dhcpv4Srv::openActiveSockets(const uint16_t port,
// sockets are marked active or inactive.
// @todo Optimization: we should not reopen all sockets but rather select
// 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);
}
}
@@ -1335,6 +1337,11 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf,
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 isc
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 66fe3a6..5bc015e 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -381,6 +381,15 @@ private:
/// @return Option that contains netmask information
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.
/// Pointer to the allocation engine that we are currently using
/// It must be a pointer, because we will support changing engines
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 1570ca9..dcce78c 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -191,6 +191,26 @@ TEST_F(Dhcpv4SrvTest, basic) {
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
// message is set to giaddr, when giaddr is set to non-zero address
// in the received message.
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index 56e96f1..e29c31d 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc
+++ b/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")));
subnet_->addPool(pool_);
+ CfgMgr::instance().deleteActiveIfaces();
CfgMgr::instance().deleteSubnets4();
CfgMgr::instance().addSubnet4(subnet_);
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 986aadf..bd13598 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -295,7 +295,6 @@ void IfaceMgr::stubDetectIfaces() {
bool
IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
IfaceMgrErrorMsgCallback error_handler) {
- int sock;
int count = 0;
// This option is used to bind sockets to particular interfaces.
@@ -332,6 +331,7 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
continue;
}
+ int sock = -1;
// If selected interface is broadcast capable set appropriate
// options on the socket so as it can receive and send broadcast
// messages.
@@ -347,6 +347,7 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
" listen broadcast traffic on a"
" single interface",
error_handler);
+ continue;
} else {
try {
@@ -356,6 +357,7 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
true, true);
} catch (const Exception& ex) {
handleSocketConfigError(ex.what(), error_handler);
+ continue;
}
// Binding socket to an interface is not supported so we
@@ -373,17 +375,19 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
false, false);
} catch (const Exception& ex) {
handleSocketConfigError(ex.what(), error_handler);
+ continue;
}
}
if (sock < 0) {
const char* errstr = strerror(errno);
- isc_throw(SocketConfigError, "failed to open IPv4 socket"
- << " supporting broadcast traffic, reason:"
- << errstr);
+ handleSocketConfigError(std::string("failed to open IPv4 socket,"
+ " reason:") + errstr,
+ error_handler);
+ } else {
+ ++count;
}
- count++;
}
}
return (count > 0);
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 731de41..d308d2a 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -424,7 +424,7 @@ public:
/// @return true if direct response is supported.
bool isDirectResponseSupported() const;
- /// @brief Returns interface with specified interface index
+ /// @brief Returns interfac specified interface index
///
/// @param ifindex index of searched interface
///
@@ -732,6 +732,15 @@ public:
/// not having address assigned.
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.
static const int INVALID_SOCKET = -1;
@@ -776,13 +785,6 @@ protected:
/// @return socket descriptor
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.
///
/// This method will eventually detect available interfaces. For now
More information about the bind10-changes
mailing list