[#1716] fixed unittests

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index a4cde25dd8ee550ff449a92489e7c85cf9b3eccf..fc037dd474f983548a09b436f74a61afe13b7838 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -722,7 +722,7 @@ IfaceMgr::openSockets6(const uint16_t port,
                 try {
                     // Pass a null pointer as an error handler to avoid
                     // suppressing an exception in a system-specific function.
-                    IfaceMgr::openMulticastSocket(*iface, addr, port, nullptr);
+                    IfaceMgr::openMulticastSocket(*iface, addr, port);
                 } catch (const Exception& ex) {
                     IFACEMGR_ERROR(SocketConfigError, error_handler,
                         "Failed to open multicast socket on interface "

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index b005d59240157c7d0f4652627ae3c3fcca0da80b..7fe94b0303a105064dbb44c54625535bbc9db4bf 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -1195,6 +1195,11 @@ public:
     /// socket is bound to the port (and address is unspecified), the
     /// method will check if the address passed in the argument is configured
     /// on an interface.
+    /// Note: On BSD and Solaris the socket is opened for "::" address instead
+    /// of the link-local address or the "ff02::1:2" address. If there are
+    /// multiple interfaces joining the multicast group, this function matches
+    /// the "::" address bound by any interface, not necessary the one with the
+    /// specified link-local address and returns true.
     ///
     /// @param addr Address of the socket being searched.
     ///

diff --git a/src/lib/dhcp/iface_mgr_sun.cc b/src/lib/dhcp/iface_mgr_sun.cc
index 86e45dff6f08492c5a75d183247811a87c725505..c9cafe8a5b630cbd8bc938cf9bd884aaca277df6 100644 (file)
--- a/src/lib/dhcp/iface_mgr_sun.cc
+++ b/src/lib/dhcp/iface_mgr_sun.cc
@@ -170,6 +170,10 @@ IfaceMgr::openMulticastSocket(Iface& iface,
 int
 IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, uint16_t port,
                       const bool join_multicast) {
+    // On Solaris, we bind the socket to in6addr_any and join multicast group
+    // to receive multicast traffic. So, if the multicast is requested,
+    // replace the address specified by the caller with the "unspecified"
+    // address.
     IOAddress actual_address = join_multicast ? IOAddress("::") : addr;
     SocketInfo info = packet_filter6_->openSocket(iface, actual_address, port,
                                                   join_multicast);

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index a126169eb2638d57e9157a77759a1769aa725867..d5893d4d0ad2187ebc8b578d7d96df2e0c3e7c44 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -2004,6 +2004,8 @@ TEST_F(IfaceMgrTest, openSockets4SkipOpen) {
     // Open socket on eth1. 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.
+    // This test checks if calling openSockets4 does not throw when sockets are
+    // already opened for the interface "eth1".
     ASSERT_NO_THROW(ifacemgr.openSocket("eth1", IOAddress("192.0.2.3"),
                                         DHCP4_SERVER_PORT));
 
@@ -2497,6 +2499,14 @@ TEST_F(IfaceMgrTest, openSockets6SkipOpen) {
     // 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.
+    // This test checks if calling openSockets6 does not throw when sockets are
+    // already opened for the interface "eth0".
+    // Because the @ref IfaceMgr::hasOpenSocket(addr) does match the "::"
+    // address on BSD and Solaris on any interface, we make sure that the
+    // interface "eth0" does not join the multicast group. Otherwise the
+    // @ref IfaceMgr::openSockets6 will see that at least one socket for address
+    // "::" has been opened for interface "eth1" and will not try to open the
+    // rest of the sockets.
     ASSERT_NO_THROW(ifacemgr.openSocket("eth0",
                                         IOAddress("fe80::3a60:77ff:fed5:cdef"),
                                         DHCP6_SERVER_PORT, false));

diff --git a/src/lib/dhcp/tests/pkt_filter6_test_stub.cc b/src/lib/dhcp/tests/pkt_filter6_test_stub.cc
index 08b97dcfbba15a20844097fbb6ea820c008c618c..372be70aa5ef3a2aa735dcb9034ded52c01d3862 100644 (file)
--- a/src/lib/dhcp/tests/pkt_filter6_test_stub.cc
+++ b/src/lib/dhcp/tests/pkt_filter6_test_stub.cc
@@ -13,14 +13,14 @@ namespace isc {
 namespace dhcp {
 namespace test {
 
-PktFilter6TestStub::PktFilter6TestStub() : open_socket_callback_(nullptr) {
+PktFilter6TestStub::PktFilter6TestStub() : open_socket_callback_() {
 }
 
 SocketInfo
 PktFilter6TestStub::openSocket(const Iface&,
            const isc::asiolink::IOAddress& addr,
            const uint16_t port, const bool) {
-    if (open_socket_callback_ != nullptr) {
+    if (open_socket_callback_) {
         open_socket_callback_(port);
     }
 

diff --git a/src/lib/dhcp/tests/pkt_filter_test_stub.cc b/src/lib/dhcp/tests/pkt_filter_test_stub.cc
index 0fa502bfff43ffdfbef04f623eb3967d909d6b31..3b285af17ca2390b00236f2d6a9d6775ac91c0b4 100644 (file)
--- a/src/lib/dhcp/tests/pkt_filter_test_stub.cc
+++ b/src/lib/dhcp/tests/pkt_filter_test_stub.cc
@@ -16,7 +16,7 @@ namespace dhcp {
 namespace test {
 
 PktFilterTestStub::PktFilterTestStub()
-    : direct_response_supported_(true), open_socket_callback_(nullptr) {
+    : direct_response_supported_(true), open_socket_callback_() {
 }
 
 bool
@@ -35,7 +35,7 @@ PktFilterTestStub::openSocket(Iface&,
                   "PktFilterTestStub: cannot open /dev/null:" << errmsg);
     }
 
-    if (open_socket_callback_ != nullptr) {
+    if (open_socket_callback_) {
         open_socket_callback_(port);
     }
 

diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc
index 1a06d51460c2cd6a1c00be78138003726cae0930..bc7c2b638bc25f84b6d9b0b785472b68582ae04d 100644 (file)
--- a/src/lib/dhcpsrv/cfg_iface.cc
+++ b/src/lib/dhcpsrv/cfg_iface.cc
@@ -264,7 +264,7 @@ CfgIface::openSocketsWithRetry(ReconnectCtlPtr reconnect_ctl,
         }
         // Has errors but retries exceed
         if (has_errors) {
-            if (open_sockets_failed_callback_ != nullptr) {
+            if (open_sockets_failed_callback_) {
                 open_sockets_failed_callback_(reconnect_ctl);
             }
         }

diff --git a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc
index a15d8f1ff7a9faee4f1eea84f101bdc392f7f34f..f1e3cbdcb457e014bbf77b53b0fc1375a5ec5a23 100644 (file)
--- a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc
@@ -95,7 +95,7 @@ CfgIfaceTest::TearDown() {
     IfaceMgr::instance().detectIfaces();
 
     // Reset global handlers
-    CfgIface::open_sockets_failed_callback_ = nullptr;
+    CfgIface::open_sockets_failed_callback_ = 0;
 }
 
 bool
@@ -570,7 +570,7 @@ TEST_F(CfgIfaceTest, requireOpenAllServiceSockets) {
     uint16_t fail_calls = 0;
     CfgIface::OpenSocketsFailedCallback on_fail_callback =
         [&fail_calls](ReconnectCtlPtr reconnect_ctl) {
-            EXPECT_TRUE(reconnect_ctl != nullptr);
+            EXPECT_TRUE(reconnect_ctl);
             EXPECT_TRUE(reconnect_ctl->exitOnFailure());
             fail_calls++;
         };
@@ -638,7 +638,7 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets4) {
     // Set the callback to count calls and check wait time
     size_t total_calls = 0;
     auto last_call_time = std::chrono::system_clock::time_point::min();
-    auto open_callback = [&total_calls, &last_call_time, WAIT_TIME, CALLS_PER_RETRY](uint16_t) {
+    auto open_callback = [&total_calls, &last_call_time, WAIT_TIME](uint16_t) {
         auto now = std::chrono::system_clock::now();
 
         // Check waiting time only for the first call in a retry attempt.
@@ -690,6 +690,9 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets4OmitBound) {
     ASSERT_NO_THROW(cfg4.use(AF_INET, "eth0"));
     ASSERT_NO_THROW(cfg4.use(AF_INET, "eth1/192.0.2.3"));
 
+    // This test checks if calling @ref CfgIface::openSockets does activate the
+    // retry mechanism for the interface "eth1".
+
     // Parameters
     const uint16_t RETRIES = 5;
     const uint16_t WAIT_TIME = 10; // miliseconds
@@ -701,7 +704,7 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets4OmitBound) {
     // Set the callback to count calls and check wait time
     size_t total_calls = 0;
     auto last_call_time = std::chrono::system_clock::time_point::min();
-    auto open_callback = [&total_calls, &last_call_time, RETRIES, WAIT_TIME](uint16_t) {
+    auto open_callback = [&total_calls, &last_call_time, WAIT_TIME](uint16_t) {
         auto now = std::chrono::system_clock::now();
         bool is_eth1 = total_calls == 1;
 
@@ -836,7 +839,7 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets6) {
     // Set the callback to count calls and check wait time
     size_t total_calls = 0;
     auto last_call_time = std::chrono::system_clock::time_point::min();
-    auto open_callback = [&total_calls, &last_call_time, WAIT_TIME, CALLS_PER_RETRY](uint16_t) {
+    auto open_callback = [&total_calls, &last_call_time, WAIT_TIME](uint16_t) {
         auto now = std::chrono::system_clock::now();
 
         // Check waiting time only for the first call in a retry attempt.
@@ -886,6 +889,17 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets6OmitBound) {
     ASSERT_NO_THROW(cfg6.use(AF_INET6, "eth0/2001:db8:1::1"));
     ASSERT_NO_THROW(cfg6.use(AF_INET6, "eth1"));
 
+    // This test checks if calling @ref CfgIface::openSockets does activate the
+    // retry mechanism for the interface "eth1".
+    // Because the @ref IfaceMgr::hasOpenSocket(addr) does match the "::"
+    // address on BSD and Solaris on any interface, we make sure that the
+    // interface "eth0" does not join the multicast group. Otherwise the
+    // @ref IfaceMgr::openSockets6 will see that at least one socket for address
+    // "::" has been opened for interface "eth1" and will not try to open the
+    // rest of the sockets.
+    // Make eth0 multicast-incapable.
+    IfaceMgr::instance().getIface("eth0")->flag_multicast_ = false;
+
     // Parameters
     const uint16_t RETRIES = 5;
     const uint16_t WAIT_TIME = 10; // miliseconds
@@ -897,9 +911,9 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets6OmitBound) {
     // Set the callback to count calls and check wait time
     size_t total_calls = 0;
     auto last_call_time = std::chrono::system_clock::time_point::min();
-    auto open_callback = [&total_calls, &last_call_time, RETRIES, WAIT_TIME](uint16_t) {
+    auto open_callback = [&total_calls, &last_call_time, WAIT_TIME](uint16_t) {
         auto now = std::chrono::system_clock::now();
-        bool is_eth0 = total_calls < 3;
+        bool is_eth0 = total_calls < 2;
 
         // Skip the wait time check for the socket when two sockets are
         // binding in a single attempt.
@@ -907,9 +921,8 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets6OmitBound) {
         // Don't check the waiting time for initial calls.
         // iface: eth0 addr: 2001:db8:1::1 port: 547 multicast: 0
         // iface: eth0 addr: fe80::3a60:77ff:fed5:cdef port: 547 multicast: 1
-        // iface: eth0 addr: ff02::1:2 port: 547 multicast: 0
         // iface: eth1 addr: fe80::3a60:77ff:fed5:abcd port: 547 multicast: 1 - fails
-        if (total_calls > 4) {
+        if (total_calls > 3) {
             auto interval = now - last_call_time;
             auto interval_ms =
                 std::chrono::duration_cast<std::chrono::milliseconds>(
@@ -944,9 +957,9 @@ TEST_F(CfgIfaceTest, retryOpenServiceSockets6OmitBound) {
     // Wait for a finish sockets binding (with a safe margin).
     doWait(RETRIES * WAIT_TIME * 2);
 
-    // For eth0 interface perform only 3 init open,
+    // For eth0 interface perform only 2 init open,
     // for eth1 interface perform 1 init open and a few retries.
-    EXPECT_EQ((RETRIES + 1) + 3, total_calls);
+    EXPECT_EQ((RETRIES + 1) + 2, total_calls);
 }
 
 // Test that only one reopen timer is active simultaneously. If a new opening