From: Razvan Becheriu Date: Fri, 8 Apr 2022 18:05:03 +0000 (+0300) Subject: [#1716] fixed unittests X-Git-Tag: Kea-2.1.5~67 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7b4a0d26714c68dcc190064cc549d8fe12964207;p=thirdparty%2Fkea.git [#1716] fixed unittests --- diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index a4cde25dd8..fc037dd474 100644 --- 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 b005d59240..7fe94b0303 100644 --- 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 86e45dff6f..c9cafe8a5b 100644 --- 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 a126169eb2..d5893d4d0a 100644 --- 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 08b97dcfbb..372be70aa5 100644 --- 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 0fa502bfff..3b285af17c 100644 --- 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 1a06d51460..bc7c2b638b 100644 --- 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 a15d8f1ff7..f1e3cbdcb4 100644 --- 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( @@ -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