Browse Source

[master] Merge branch 'trac3252'

Marcin Siodelski 11 years ago
parent
commit
af5eada1bb

+ 1 - 0
doc/devel/mainpage.dox

@@ -74,6 +74,7 @@
  *   - @subpage libdhcpIfaceMgr
  *   - @subpage libdhcpIfaceMgr
  *   - @subpage libdhcpPktFilter
  *   - @subpage libdhcpPktFilter
  *   - @subpage libdhcpPktFilter6
  *   - @subpage libdhcpPktFilter6
+ *   - @subpage libdhcpErrorLogging
  * - @subpage libdhcpsrv
  * - @subpage libdhcpsrv
  *   - @subpage leasemgr
  *   - @subpage leasemgr
  *   - @subpage cfgmgr
  *   - @subpage cfgmgr

+ 5 - 1
src/bin/dhcp6/dhcp6_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2013  Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
 #
 #
 # Permission to use, copy, modify, and/or distribute this software for any
 # Permission to use, copy, modify, and/or distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
 # purpose with or without fee is hereby granted, provided that the above
@@ -250,6 +250,10 @@ 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
+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.
+
 % DHCP6_PACKET_PARSE_FAIL failed to parse incoming packet
 % DHCP6_PACKET_PARSE_FAIL failed to parse incoming packet
 The IPv6 DHCP server has received a packet that it is unable to interpret.
 The IPv6 DHCP server has received a packet that it is unable to interpret.
 
 

+ 16 - 3
src/bin/dhcp6/dhcp6_srv.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -45,6 +45,7 @@
 #include <util/io_utilities.h>
 #include <util/io_utilities.h>
 #include <util/range_utilities.h>
 #include <util/range_utilities.h>
 
 
+#include <boost/bind.hpp>
 #include <boost/foreach.hpp>
 #include <boost/foreach.hpp>
 #include <boost/tokenizer.hpp>
 #include <boost/tokenizer.hpp>
 #include <boost/algorithm/string/erase.hpp>
 #include <boost/algorithm/string/erase.hpp>
@@ -152,7 +153,12 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
                 LOG_ERROR(dhcp6_logger, DHCP6_NO_INTERFACES);
                 LOG_ERROR(dhcp6_logger, DHCP6_NO_INTERFACES);
                 return;
                 return;
             }
             }
-            IfaceMgr::instance().openSockets6(port_);
+            // Create error handler. This handler will be called every time
+            // the socket opening operation fails. We use this handler to
+            // log a warning.
+            isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
+                boost::bind(&Dhcpv6Srv::ifaceMgrSocket6ErrorHandler, _1);
+            IfaceMgr::instance().openSockets6(port_, error_handler);
         }
         }
 
 
         string duid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_DUID_FILE);
         string duid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_DUID_FILE);
@@ -2311,7 +2317,9 @@ Dhcpv6Srv::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().openSockets6(port)) {
+    isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
+        boost::bind(&Dhcpv6Srv::ifaceMgrSocket6ErrorHandler, _1);
+    if (!IfaceMgr::instance().openSockets6(port, error_handler)) {
         LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN);
         LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN);
     }
     }
 }
 }
@@ -2409,6 +2417,11 @@ Dhcpv6Srv::unpackOptions(const OptionBuffer& buf,
     return (offset);
     return (offset);
 }
 }
 
 
+void
+Dhcpv6Srv::ifaceMgrSocket6ErrorHandler(const std::string& errmsg) {
+    // Log the reason for socket opening failure and return.
+    LOG_WARN(dhcp6_logger, DHCP6_OPEN_SOCKET_FAIL).arg(errmsg);
+}
 
 
 };
 };
 };
 };

+ 11 - 1
src/bin/dhcp6/dhcp6_srv.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -534,6 +534,16 @@ protected:
                          size_t* relay_msg_len);
                          size_t* relay_msg_len);
 
 
 private:
 private:
+
+    /// @brief Implements the error handler for socket open failure.
+    ///
+    /// This callback function is installed on the @c isc::dhcp::IfaceMgr
+    /// when IPv6 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 ifaceMgrSocket6ErrorHandler(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

+ 95 - 58
src/lib/dhcp/iface_mgr.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014  Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -37,6 +37,40 @@
 #include <string.h>
 #include <string.h>
 #include <sys/select.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 std;
 using namespace isc::asiolink;
 using namespace isc::asiolink;
 using namespace isc::util::io::internal;
 using namespace isc::util::io::internal;
@@ -366,7 +400,6 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
                 continue;
                 continue;
             }
             }
 
 
-            int sock = -1;
             // If selected interface is broadcast capable set appropriate
             // If selected interface is broadcast capable set appropriate
             // options on the socket so as it can receive and send broadcast
             // options on the socket so as it can receive and send broadcast
             // messages.
             // messages.
@@ -376,22 +409,24 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
                 // bind to INADDR_ANY address but we can do it only once. Thus,
                 // bind to INADDR_ANY address but we can do it only once. Thus,
                 // if one socket has been bound we can't do it any further.
                 // if one socket has been bound we can't do it any further.
                 if (!bind_to_device && bcast_num > 0) {
                 if (!bind_to_device && bcast_num > 0) {
-                    handleSocketConfigError("SO_BINDTODEVICE socket option is"
-                                            " not supported on this OS;"
-                                            " therefore, DHCP server can only"
-                                            " listen broadcast traffic on a"
-                                            " single interface",
-                                            error_handler);
+                    IFACEMGR_ERROR(SocketConfigError, error_handler,
+                                   "SO_BINDTODEVICE socket option is"
+                                   " not supported on this OS;"
+                                   " therefore, DHCP server can only"
+                                   " listen broadcast traffic on a"
+                                   " single interface");
                     continue;
                     continue;
 
 
                 } else {
                 } else {
                     try {
                     try {
                         // We haven't open any broadcast sockets yet, so we can
                         // We haven't open any broadcast sockets yet, so we can
                         // open at least one more.
                         // open at least one more.
-                        sock = openSocket(iface->getName(), *addr, port,
-                                          true, true);
+                        openSocket(iface->getName(), *addr, port, true, true);
                     } catch (const Exception& ex) {
                     } catch (const Exception& ex) {
-                        handleSocketConfigError(ex.what(), error_handler);
+                        IFACEMGR_ERROR(SocketConfigError, error_handler,
+                                       "failed to open socket on interface "
+                                       << iface->getName() << ", reason: "
+                                       << ex.what());
                         continue;
                         continue;
 
 
                     }
                     }
@@ -406,30 +441,26 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
             } else {
             } else {
                 try {
                 try {
                     // Not broadcast capable, do not set broadcast flags.
                     // Not broadcast capable, do not set broadcast flags.
-                    sock = openSocket(iface->getName(), *addr, port,
-                                      false, false);
+                    openSocket(iface->getName(), *addr, port, false, false);
                 } catch (const Exception& ex) {
                 } catch (const Exception& ex) {
-                    handleSocketConfigError(ex.what(), error_handler);
+                    IFACEMGR_ERROR(SocketConfigError, error_handler,
+                                   "failed to open socket on interface "
+                                   << iface->getName() << ", reason: "
+                                   << ex.what());
                     continue;
                     continue;
                 }
                 }
 
 
             }
             }
-            if (sock < 0) {
-                const char* errstr = strerror(errno);
-                handleSocketConfigError(std::string("failed to open IPv4 socket,"
-                                                    " reason:") + errstr,
-                                        error_handler);
-            } else {
-                ++count;
-            }
+            ++count;
 
 
         }
         }
     }
     }
     return (count > 0);
     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;
     int count = 0;
 
 
     for (IfaceCollection::iterator iface = ifaces_.begin();
     for (IfaceCollection::iterator iface = ifaces_.begin();
@@ -448,12 +479,15 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
         for (Iface::AddressCollection::iterator addr = unicasts.begin();
         for (Iface::AddressCollection::iterator addr = unicasts.begin();
              addr != unicasts.end(); ++addr) {
              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++;
             count++;
@@ -487,13 +521,28 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
             // it for some odd use cases which may utilize non-multicast
             // it for some odd use cases which may utilize non-multicast
             // interfaces. Perhaps a warning should be emitted if the
             // interfaces. Perhaps a warning should be emitted if the
             // interface is not a multicast one.
             // 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);
+
+            // The sock variable will hold a socket descriptor. It may be
+            // used to close a socket if the function fails to bind to
+            // multicast address on Linux systems. Because we only bind
+            // a socket to multicast address on Linux, on other systems
+            // the sock variable will be initialized but unused. We have
+            // to suppress the cppcheck warning which shows up on non-Linux
+            // systems.
+            // cppcheck-suppress variableScope
+            int sock;
+            try {
+                // cppcheck-suppress unreadVariable
+                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++;
             count++;
@@ -503,16 +552,18 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
             // To receive multicast traffic, Linux requires binding socket to
             // To receive multicast traffic, Linux requires binding socket to
             // a multicast group. That in turn doesn't work on NetBSD.
             // a multicast group. That in turn doesn't work on NetBSD.
             if (iface->flag_multicast_) {
             if (iface->flag_multicast_) {
-                int sock2 =
+                try {
                     openSocket(iface->getName(),
                     openSocket(iface->getName(),
                                IOAddress(ALL_DHCP_RELAY_AGENTS_AND_SERVERS),
                                IOAddress(ALL_DHCP_RELAY_AGENTS_AND_SERVERS),
                                port);
                                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->getName()
+                                   << ", reason: " << ex.what());
+                    continue;
                 }
                 }
             }
             }
 #endif
 #endif
@@ -522,20 +573,6 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
 }
 }
 
 
 void
 void
-IfaceMgr::handleSocketConfigError(const std::string& errmsg,
-                                  IfaceMgrErrorMsgCallback handler) {
-    // If error handler is installed, we don't want to throw an exception, but
-    // rather call this handler.
-    if (handler != NULL) {
-        handler(errmsg);
-
-    } else {
-        isc_throw(SocketConfigError, errmsg);
-
-    }
-}
-
-void
 IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) {
 IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) {
     for (IfaceCollection::const_iterator iface=ifaces_.begin();
     for (IfaceCollection::const_iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          iface!=ifaces_.end();

+ 40 - 31
src/lib/dhcp/iface_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014  Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -643,24 +643,43 @@ public:
 
 
     /// @brief Opens IPv6 sockets on detected interfaces.
     /// @brief Opens IPv6 sockets on detected interfaces.
     ///
     ///
-    /// @todo This function will throw an exception immediately when a socket
-    /// fails to open. This is undersired behavior because it will preclude
-    /// other sockets from opening. We should strive to provide similar mechanism
-    /// that has been introduced for V4 sockets. If socket creation fails the
-    /// appropriate error handler is called and once the handler returns the
-    /// function contnues to open other sockets. The change in the IfaceMgr
-    /// is quite straight forward and it is proven to work for V4. However,
-    /// unit testing it is a bit involved, because for unit testing we need
-    /// a replacement of the openSocket6 function which will mimic the
-    /// behavior of the real socket opening. For the V4 we have the means to
-    /// to achieve that with the replaceable PktFilter class. For V6, the
-    /// implementation is hardcoded in the openSocket6.
+    /// 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.
+    /// However, the warning about the failure for the particular socket should
+    /// be communicated to the caller. The libdhcp++ is a common library with
+    /// no logger associated with it. Most of the functions in this library
+    /// communicate errors via exceptions. In case of openSockets6 function
+    /// exception must not be thrown if the function is supposed to continue
+    /// opening sockets, despite an error. Therefore, if such a behavior is
+    /// desired, the error handler function can be passed as a parameter.
+    /// This error handler is called (if present) with an error string.
+    /// Typically, error handler will simply log an error using an application
+    /// logger, but it can do more sophisticated error handling too.
+    ///
+    /// @todo It is possible that additional parameters will have to be added
+    /// to the error handler, e.g. Iface if it was really supposed to do
+    /// some more sophisticated error handling.
+    ///
+    /// If the error handler is not installed (is NULL), the exception is thrown
+    /// for each failure (default behavior).
+    ///
+    /// @warning This function does not check if there has been any sockets
+    /// already open by the @c IfaceMgr. Therefore a caller should call
+    /// @c IfaceMgr::closeSockets(AF_INET6) before calling this function.
+    /// If there are any sockets open, the function may either throw an
+    /// exception or invoke an error handler on attempt to bind the new socket
+    /// to the same address and port.
     ///
     ///
     /// @param port specifies port number (usually DHCP6_SERVER_PORT)
     /// @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.
     /// @throw SocketOpenFailure if tried and failed to open socket.
     /// @return true if any sockets were open
     /// @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.
     /// @brief Opens IPv4 sockets on detected interfaces.
     ///
     ///
@@ -715,6 +734,13 @@ public:
     /// If the error handler is not installed (is NULL), the exception is thrown
     /// If the error handler is not installed (is NULL), the exception is thrown
     /// for each failure (default behavior).
     /// for each failure (default behavior).
     ///
     ///
+    /// @warning This function does not check if there has been any sockets
+    /// already open by the @c IfaceMgr. Therefore a caller should call
+    /// @c IfaceMgr::closeSockets(AF_INET) before calling this function.
+    /// If there are any sockets open, the function may either throw an
+    /// exception or invoke an error handler on attempt to bind the new socket
+    /// to the same address and port.
+    ///
     /// @param port specifies port number (usually DHCP4_SERVER_PORT)
     /// @param port specifies port number (usually DHCP4_SERVER_PORT)
     /// @param use_bcast configure sockets to support broadcast messages.
     /// @param use_bcast configure sockets to support broadcast messages.
     /// @param error_handler A pointer to an error handler function which is
     /// @param error_handler A pointer to an error handler function which is
@@ -957,23 +983,6 @@ private:
     getLocalAddress(const isc::asiolink::IOAddress& remote_addr,
     getLocalAddress(const isc::asiolink::IOAddress& remote_addr,
                     const uint16_t port);
                     const uint16_t port);
 
 
-    /// @brief Handles an error which occurs during configuration of a socket.
-    ///
-    /// If the handler callback is specified (non-NULL), this handler is
-    /// called and the specified error message is passed to it. If the
-    /// handler is not specified, the @c isc::dhcpSocketConfigError exception
-    /// is thrown with the specified message.
-    ///
-    /// This function should be called to handle errors which occur during
-    /// socket opening, binding or configuration (e.g. setting socket options
-    /// etc).
-    ///
-    /// @param errmsg An error message to be passed to a handlder function or
-    /// to the @c isc::dhcp::SocketConfigError exception.
-    /// @param handler An error handler function or NULL.
-    void handleSocketConfigError(const std::string& errmsg,
-                                 IfaceMgrErrorMsgCallback handler);
-
     /// @brief Checks if there is at least one socket of the specified family
     /// @brief Checks if there is at least one socket of the specified family
     /// open.
     /// open.
     ///
     ///

+ 30 - 1
src/lib/dhcp/libdhcp++.dox

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2014  Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -231,4 +231,33 @@ functions.
 Use \ref isc::dhcp::IfaceMgr::setPacketFilter function to set the custom packet
 Use \ref isc::dhcp::IfaceMgr::setPacketFilter function to set the custom packet
 filter object to be used by Interface Manager.
 filter object to be used by Interface Manager.
 
 
+@section libdhcpErrorLogging Logging non-fatal errors in IfaceMgr
+
+The libdhcp++ is a common library, meant to be used by various components,
+such as DHCP servers, relays and clients. It is also used by a perfdhcp
+benchmarking application. It provides a basic capabilities for these
+applications to perform operations on DHCP messages such as encoding
+or decoding them. It also provides capabilities to perform low level
+operations on sockets. Since libdhcp++ is a common library, its dependency
+on other BINDX modules should be minimal. In particular, errors occurring
+in the libdhcp++ are reported using exceptions, not a BINDX logger. This
+works well in most cases, but there are some cases in which it is
+undesired for a function to throw an exception in case of non-fatal error.
+
+The typical case, when exception should not be thrown, is when the \ref
+isc::dhcp::IfaceMgr::openSockets4 or \ref isc::dhcp::IfaceMgr::openSockets6
+fails to open a socket on one of the interfaces. This should not preclude
+the function from attempting to open sockets on other interfaces, which
+would be the case if exception was thrown.
+
+In such cases the IfaceMgr makes use of error handler callback function
+which may be installed by a caller. This function must implement the
+isc::dhcp::IfaceMgrErrorMsgCallback. Note that it is allowed to pass a NULL
+value instead, which would result falling back to a default behavior and
+exception will be thrown. If non-NULL value is provided, the
+\ref isc::dhcp::IfaceMgr will call error handler function and pass an
+error string as an argument. The handler function may use its logging
+mechanism to log this error message. In particular, the DHCP server
+will use BINDX logger to log the error message.
+
 */
 */

+ 73 - 5
src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -1465,7 +1465,7 @@ TEST_F(IfaceMgrTest, openSockets4NoErrorHandler) {
 
 
 // Test that the external error handler is called when trying to bind a new
 // 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
 // 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) {
 TEST_F(IfaceMgrTest, openSocket4ErrorHandler) {
     NakedIfaceMgr ifacemgr;
     NakedIfaceMgr ifacemgr;
 
 
@@ -1476,9 +1476,7 @@ TEST_F(IfaceMgrTest, openSocket4ErrorHandler) {
     ASSERT_TRUE(custom_packet_filter);
     ASSERT_TRUE(custom_packet_filter);
     ASSERT_NO_THROW(ifacemgr.setPacketFilter(custom_packet_filter));
     ASSERT_NO_THROW(ifacemgr.setPacketFilter(custom_packet_filter));
 
 
-    // Open socket on eth0. The openSockets4 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.
+    // Open socket on eth0.
     ASSERT_NO_THROW(ifacemgr.openSocket("eth0", IOAddress("10.0.0.1"),
     ASSERT_NO_THROW(ifacemgr.openSocket("eth0", IOAddress("10.0.0.1"),
                                         DHCP4_SERVER_PORT));
                                         DHCP4_SERVER_PORT));
 
 
@@ -1486,6 +1484,9 @@ TEST_F(IfaceMgrTest, openSocket4ErrorHandler) {
     // should be called when the IfaceMgr fails to open socket on eth0.
     // should be called when the IfaceMgr fails to open socket on eth0.
     isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
     isc::dhcp::IfaceMgrErrorMsgCallback error_handler =
         boost::bind(&IfaceMgrTest::ifaceMgrErrorHandler, this, _1);
         boost::bind(&IfaceMgrTest::ifaceMgrErrorHandler, this, _1);
+    // The openSockets4 should detect that there is another socket already
+    // open and bound to the same address and port. An attempt to open
+    // another socket and bind to this address and port should fail.
     ASSERT_NO_THROW(ifacemgr.openSockets4(DHCP4_SERVER_PORT, true, error_handler));
     ASSERT_NO_THROW(ifacemgr.openSockets4(DHCP4_SERVER_PORT, true, error_handler));
     // We expect that an error occured when we tried to open a socket on
     // We expect that an error occured when we tried to open a socket on
     // eth0, but the socket on eth1 should open just fine.
     // eth0, but the socket on eth1 should open just fine.
@@ -1829,6 +1830,73 @@ TEST_F(IfaceMgrTest, openSockets6NoIfaces) {
     EXPECT_FALSE(socket_open);
     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.
+    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);
+    // The openSockets6 should detect that a socket has been already
+    // opened on eth0 and an attempt to open another socket and bind to
+    // the same address and port should fail.
+    ASSERT_NO_THROW(ifacemgr.openSockets6(DHCP6_SERVER_PORT, error_handler));
+    // 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 the Iface structure itself
 TEST_F(IfaceMgrTest, iface) {
 TEST_F(IfaceMgrTest, iface) {
     boost::scoped_ptr<Iface> iface;
     boost::scoped_ptr<Iface> iface;

+ 11 - 2
src/lib/dhcp/tests/pkt_filter6_test_utils.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -181,8 +181,17 @@ PktFilter6Stub::PktFilter6Stub()
 }
 }
 
 
 SocketInfo
 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) {
                            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_;
     ++open_socket_count_;
     return (SocketInfo(addr, port, 0));
     return (SocketInfo(addr, port, 0));
 }
 }

+ 6 - 1
src/lib/dhcp/tests/pkt_filter6_test_utils.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -109,6 +109,11 @@ public:
     /// always set to 0. On each call to this function, the counter of
     /// 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
     /// invocations is increased by one. This is useful to check if packet
     /// filter object has been correctly installed and is used by @c IfaceMgr.
     /// 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 iface Interface descriptor.
     /// @param addr Address on the interface to be used to send packets.
     /// @param addr Address on the interface to be used to send packets.