Browse Source

[3252] Install error handler when IPv6 sockets are being open.

Marcin Siodelski 11 years ago
parent
commit
58da9f45c7

+ 65 - 22
src/lib/dhcp/iface_mgr.cc

@@ -37,6 +37,38 @@
 #include <string.h>
 #include <sys/select.h>
 
+/// @brief A macro which handles an error in IfaceMgr.
+///
+/// There are certain cases when IfaceMgr may hit an error which shouldn't
+/// result in interruption of the function processing. A typical case is
+/// the function which opens sockets on available interfaces for a DHCP
+/// server. If this function fails to open a socket on a specific interface
+/// (for example, there is another socket already open on this interface
+/// and bound to the same address and port), it is desired that the server
+/// logs a warning but will try to open sockets on other interfaces. In order
+/// to log an error, the IfaceMgr will use the error handler function provided
+/// by the server and pass an error string to it. When the handler function
+/// returns, the IfaceMgr will proceed to open other sockets. It is allowed
+/// that the error handler function is not installed (is NULL). In these
+/// cases it is expected that the exception is thrown instead. A possible
+/// solution would be to enclose this conditional behavior in a function.
+/// However, despite the hate for macros, the macro seems to be a bit
+/// better solution in this case as it allows to convenietly pass an
+/// error string in a stream (not as a string).
+///
+/// @param ex_type Exception to be thrown if error_handler is NULL.
+/// @param handler Error handler function to be called or NULL to indicate
+/// that exception should be thrown instead.
+/// @param stream stream object holding an error string.
+#define ifacemgr_error(ex_type, handler, stream) \
+    std::ostringstream oss__; \
+    oss__ << stream; \
+    if (handler) { \
+        handler(oss__.str()); \
+    } else { \
+        isc_throw(ex_type, oss__); \
+    }
+
 using namespace std;
 using namespace isc::asiolink;
 using namespace isc::util::io::internal;
@@ -426,8 +458,9 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
     return (count > 0);
 }
 
-bool IfaceMgr::openSockets6(const uint16_t port) {
-    int sock;
+bool
+IfaceMgr::openSockets6(const uint16_t port,
+                       IfaceMgrErrorMsgCallback error_handler) {
     int count = 0;
 
     for (IfaceCollection::iterator iface = ifaces_.begin();
@@ -446,12 +479,15 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
         for (Iface::AddressCollection::iterator addr = unicasts.begin();
              addr != unicasts.end(); ++addr) {
 
-            sock = openSocket(iface->getName(), *addr, port);
-            if (sock < 0) {
-                const char* errstr = strerror(errno);
-                isc_throw(SocketConfigError, "failed to open unicast socket on "
-                          << addr->toText() << " on interface " << iface->getName()
-                          << ", reason: " << errstr);
+            try {
+                openSocket(iface->getName(), *addr, port);
+
+            } catch (const Exception& ex) {
+                ifacemgr_error(SocketConfigError, error_handler,
+                               "Failed to open unicast socket on  interface "
+                               << iface->getName() << ", reason: "
+                               << ex.what());
+                continue;
             }
 
             count++;
@@ -485,13 +521,18 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
             // it for some odd use cases which may utilize non-multicast
             // interfaces. Perhaps a warning should be emitted if the
             // interface is not a multicast one.
-            sock = openSocket(iface->getName(), *addr, port,
-                              iface->flag_multicast_);
-            if (sock < 0) {
-                const char* errstr = strerror(errno);
-                isc_throw(SocketConfigError, "failed to open link-local"
-                          " socket on " << addr->toText() << " on interface "
-                          << iface->getName() << ", reason: " << errstr);
+            int sock;
+            try {
+                sock = openSocket(iface->getName(), *addr, port,
+                                  iface->flag_multicast_);
+
+            } catch (const Exception& ex) {
+                ifacemgr_error(SocketConfigError, error_handler,
+                               "Failed to open link-local socket on "
+                               " interface " << iface->getName() << ": "
+                               << ex.what());
+                continue;
+
             }
 
             count++;
@@ -501,16 +542,18 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
             // To receive multicast traffic, Linux requires binding socket to
             // a multicast group. That in turn doesn't work on NetBSD.
             if (iface->flag_multicast_) {
-                int sock2 =
+                try {
                     openSocket(iface->getName(),
                                IOAddress(ALL_DHCP_RELAY_AGENTS_AND_SERVERS),
                                port);
-                if (sock2 < 0) {
-                    const char* errstr = strerror(errno);
-                    isc_throw(SocketConfigError, "Failed to open multicast"
-                              " socket on interface " << iface->getFullName()
-                              << ", reason:" << errstr);
-                    iface->delSocket(sock); // delete previously opened socket
+                } catch (const Exception& ex) {
+                    // Delete previously opened socket.
+                    iface->delSocket(sock);
+                    ifacemgr_error(SocketConfigError, error_handler,
+                                   "Failed to open multicast socket on"
+                                   " interface " << iface->getFullName()
+                                   << ", reason: " << ex.what());
+                    continue;
                 }
             }
 #endif

+ 5 - 1
src/lib/dhcp/iface_mgr.h

@@ -643,10 +643,14 @@ public:
     /// implementation is hardcoded in the openSocket6.
     ///
     /// @param port specifies port number (usually DHCP6_SERVER_PORT)
+    /// @param error_handler A pointer to an error handler function which is
+    /// called by the openSockets6 when it fails to open a socket. This
+    /// parameter can be NULL to indicate that the callback should not be used.
     ///
     /// @throw SocketOpenFailure if tried and failed to open socket.
     /// @return true if any sockets were open
-    bool openSockets6(const uint16_t port = DHCP6_SERVER_PORT);
+    bool openSockets6(const uint16_t port = DHCP6_SERVER_PORT,
+                      IfaceMgrErrorMsgCallback error_handler = NULL);
 
     /// @brief Opens IPv4 sockets on detected interfaces.
     ///

+ 72 - 1
src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -1446,7 +1446,7 @@ TEST_F(IfaceMgrTest, openSockets4NoErrorHandler) {
 
 // Test that the external error handler is called when trying to bind a new
 // socket to the address and port being in use. The sockets on the other
-// interfaces should open just fine..
+// interfaces should open just fine.
 TEST_F(IfaceMgrTest, openSocket4ErrorHandler) {
     NakedIfaceMgr ifacemgr;
 
@@ -1810,6 +1810,77 @@ TEST_F(IfaceMgrTest, openSockets6NoIfaces) {
     EXPECT_FALSE(socket_open);
 }
 
+// Test that exception is thrown when trying to bind a new socket to the port
+// and address which is already in use by another socket.
+TEST_F(IfaceMgrTest, openSockets6NoErrorHandler) {
+    NakedIfaceMgr ifacemgr;
+
+    // Remove all real interfaces and create a set of dummy interfaces.
+    ifacemgr.createIfaces();
+
+    boost::shared_ptr<PktFilter6Stub> filter(new PktFilter6Stub());
+    ASSERT_TRUE(filter);
+    ASSERT_NO_THROW(ifacemgr.setPacketFilter(filter));
+
+    // Open socket on eth0. The openSockets6 should detect that this
+    // socket has been already open and an attempt to open another socket
+    // and bind to this address and port should fail.
+    ASSERT_NO_THROW(ifacemgr.openSocket("eth0",
+                                        IOAddress("fe80::3a60:77ff:fed5:cdef"),
+                                        DHCP6_SERVER_PORT));
+
+    // The function throws an exception when it tries to open a socket
+    // and bind it to the address in use.
+    EXPECT_THROW(ifacemgr.openSockets6(DHCP6_SERVER_PORT),
+                 isc::dhcp::SocketConfigError);
+
+}
+
+// Test that the external error handler is called when trying to bind a new
+// socket to the address and port being in use. The sockets on the other
+// interfaces should open just fine.
+TEST_F(IfaceMgrTest, openSocket6ErrorHandler) {
+    NakedIfaceMgr ifacemgr;
+
+    // Remove all real interfaces and create a set of dummy interfaces.
+    ifacemgr.createIfaces();
+
+    boost::shared_ptr<PktFilter6Stub> filter(new PktFilter6Stub());
+    ASSERT_TRUE(filter);
+    ASSERT_NO_THROW(ifacemgr.setPacketFilter(filter));
+
+    // Open socket on eth0. The openSockets6 should detect that this
+    // socket has been already open and an attempt to open another socket
+    // and bind to this address and port should fail.
+    ASSERT_NO_THROW(ifacemgr.openSocket("eth0",
+                                        IOAddress("fe80::3a60:77ff:fed5:cdef"),
+                                        DHCP6_SERVER_PORT));
+
+    // 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);
+    //    ASSERT_NO_THROW(ifacemgr.openSockets6(DHCP6_SERVER_PORT, error_handler));
+    try {
+        ifacemgr.openSockets6(DHCP6_SERVER_PORT, error_handler);
+    } catch (const Exception& ex) {
+        std::cout << ex.what() << std::endl;
+    }
+    // We expect that an error occured when we tried to open a socket on
+    // eth0, but the socket on eth1 should open just fine.
+    EXPECT_EQ(1, errors_count_);
+
+    // Reset errors count.
+    errors_count_ = 0;
+
+    // Now that we have two sockets open, we can try this again but this time
+    // we should get two errors: one when opening a socket on eth0, another one
+    // when opening a socket on eth1.
+    ASSERT_NO_THROW(ifacemgr.openSockets6(DHCP6_SERVER_PORT, error_handler));
+    EXPECT_EQ(2, errors_count_);
+
+}
+
 // Test the Iface structure itself
 TEST_F(IfaceMgrTest, iface) {
     boost::scoped_ptr<Iface> iface;

+ 10 - 1
src/lib/dhcp/tests/pkt_filter6_test_utils.cc

@@ -181,8 +181,17 @@ PktFilter6Stub::PktFilter6Stub()
 }
 
 SocketInfo
-PktFilter6Stub::openSocket(const Iface&, const isc::asiolink::IOAddress& addr,
+PktFilter6Stub::openSocket(const Iface& iface, const isc::asiolink::IOAddress& addr,
                            const uint16_t port, const bool) {
+    // Check if there is any other socket bound to the specified address
+    // and port on this interface.
+    const Iface::SocketCollection& sockets = iface.getSockets();
+    for (Iface::SocketCollection::const_iterator socket = sockets.begin();
+         socket != sockets.end(); ++socket) {
+        if ((socket->addr_ == addr) && (socket->port_ == port)) {
+            isc_throw(SocketConfigError, "test socket bind error");
+        }
+    }
     ++open_socket_count_;
     return (SocketInfo(addr, port, 0));
 }

+ 5 - 0
src/lib/dhcp/tests/pkt_filter6_test_utils.h

@@ -109,6 +109,11 @@ public:
     /// always set to 0. On each call to this function, the counter of
     /// invocations is increased by one. This is useful to check if packet
     /// filter object has been correctly installed and is used by @c IfaceMgr.
+    /// As in the case of opening a real socket, this function will check
+    /// if there is another fake socket "bound" to the same address and port.
+    /// If there is, it will throw an exception. This allows to simulate the
+    /// conditions when one of the sockets can't be open because there is
+    /// a socket already open and test how IfaceMgr will handle it.
     ///
     /// @param iface Interface descriptor.
     /// @param addr Address on the interface to be used to send packets.