From: Razvan Becheriu Date: Tue, 14 Oct 2025 18:56:11 +0000 (+0300) Subject: [#4129] ping check should ignore null lease X-Git-Tag: Kea-3.1.3~45 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2cd4c373386cf5220d4b5d3c0e92c82910bd2da9;p=thirdparty%2Fkea.git [#4129] ping check should ignore null lease --- diff --git a/src/bin/agent/tests/meson.build b/src/bin/agent/tests/meson.build index 679e4379fd..af57c1fc58 100644 --- a/src/bin/agent/tests/meson.build +++ b/src/bin/agent/tests/meson.build @@ -88,5 +88,3 @@ test( is_parallel: false, priority: -1, ) - - diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox index 8f97bd8580..04f57c10cc 100644 --- a/src/bin/dhcp4/dhcp4_hooks.dox +++ b/src/bin/dhcp4/dhcp4_hooks.dox @@ -361,7 +361,7 @@ called before "subnet4_select". "offer_address_in_use" to false if the address is available and the offer should be sent to the client. If the address should be declined, callouts should set the argument to true. This will cause the server to discard the DHCPOFFER, mark - the lease DECLINED in the lease store, update the relevant statitics, and then + the lease DECLINED in the lease store, update the relevant statistics, and then invoke callouts installed for lease4_server_decline. @subsection dhcpv4Lease4ServerDecline lease4_server_decline diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc index 66c2d4902e..8a84d648d8 100644 --- a/src/bin/dhcp4/dhcp4_messages.cc +++ b/src/bin/dhcp4/dhcp4_messages.cc @@ -295,8 +295,8 @@ const char* values[] = { "DHCP4_PACKET_DROP_0008", "%1: DHCP service is globally disabled", "DHCP4_PACKET_DROP_0009", "%1: Option 53 missing (no DHCP message type), is this a BOOTP packet?", "DHCP4_PACKET_DROP_0010", "dropped as member of the special class 'DROP': %1, %2", - "DHCP4_PACKET_DROP_0011", "dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by %6", - "DHCP4_PACKET_DROP_0012", "dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by %6", + "DHCP4_PACKET_DROP_0011", "dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by thread %6", + "DHCP4_PACKET_DROP_0012", "dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by thread %6", "DHCP4_PACKET_DROP_0013", "dropped as member of the special class 'DROP' after host reservation lookup: %1, %2", "DHCP4_PACKET_DROP_0014", "dropped as member of the special class 'DROP' after early global host reservations lookup: %1, %2", "DHCP4_PACKET_NAK_0001", "%1: failed to select a subnet for incoming packet, src %2, type %3", diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 9fd9cf45f9..d6279d4be8 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -721,7 +721,7 @@ Logged at debug log level 15. This debug message is emitted when an incoming packet was classified into the special class 'DROP' and dropped. The packet details are displayed. -% DHCP4_PACKET_DROP_0011 dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by %6 +% DHCP4_PACKET_DROP_0011 dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by thread %6 Logged at debug log level 15. Currently multi-threading processing avoids races between packets sent by a client using the same client id option by dropping new packets until @@ -729,7 +729,7 @@ processing is finished. Packet details and thread identifiers are included for both packets in this warning message. -% DHCP4_PACKET_DROP_0012 dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by %6 +% DHCP4_PACKET_DROP_0012 dropped as sent by the same client than a packet being processed by another thread: dropped %1, %2 by thread %3 as duplicate of %4, %5 processed by thread %6 Logged at debug log level 15. Currently multi-threading processing avoids races between packets sent by a client using the same hardware address by dropping new packets until diff --git a/src/bin/dhcp6/dhcp6_messages.cc b/src/bin/dhcp6/dhcp6_messages.cc index b1dc80f7d1..9a85ec58dd 100644 --- a/src/bin/dhcp6/dhcp6_messages.cc +++ b/src/bin/dhcp6/dhcp6_messages.cc @@ -282,7 +282,7 @@ const char* values[] = { "DHCP6_PACKET_DROP_DROP_CLASS", "dropped as member of the special class 'DROP': %1 %2", "DHCP6_PACKET_DROP_DROP_CLASS2", "dropped as member of the special class 'DROP' after host reservation lookup: %1 %2", "DHCP6_PACKET_DROP_DROP_CLASS_EARLY", "dropped as member of the special class 'DROP' after early global host reservations lookup: %1 %2", - "DHCP6_PACKET_DROP_DUPLICATE", "dropped as sent by the same client than a packet being processed by another thread: dropped %1 %2 by thread %3 as duplicate of %4 %5 processed by %6", + "DHCP6_PACKET_DROP_DUPLICATE", "dropped as sent by the same client than a packet being processed by another thread: dropped %1 %2 by thread %3 as duplicate of %4 %5 processed by thread %6", "DHCP6_PACKET_DROP_PARSE_FAIL", "%1: failed to parse packet from %2 to %3, received over interface %4, reason: %5, %6", "DHCP6_PACKET_DROP_SERVERID_MISMATCH", "%1: dropping packet with server identifier: %2, server is using: %3", "DHCP6_PACKET_DROP_UNICAST", "%1: dropping unicast %2 packet as this packet should be sent to multicast", diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 760f35e1f7..e411ecc6fe 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -673,7 +673,7 @@ This debug message is emitted when an incoming packet was classified after early global host reservations lookup into the special class 'DROP' and dropped. The packet details are displayed. -% DHCP6_PACKET_DROP_DUPLICATE dropped as sent by the same client than a packet being processed by another thread: dropped %1 %2 by thread %3 as duplicate of %4 %5 processed by %6 +% DHCP6_PACKET_DROP_DUPLICATE dropped as sent by the same client than a packet being processed by another thread: dropped %1 %2 by thread %3 as duplicate of %4 %5 processed by thread %6 Logged at debug log level 15. Currently multi-threading processing avoids races between packets sent by the same client by dropping new packets until processing is finished. diff --git a/src/hooks/dhcp/host_cache/tests/meson.build b/src/hooks/dhcp/host_cache/tests/meson.build index 89936eb199..7d70de7f4b 100644 --- a/src/hooks/dhcp/host_cache/tests/meson.build +++ b/src/hooks/dhcp/host_cache/tests/meson.build @@ -23,7 +23,7 @@ dhcp_host_cache_tests = executable( test( 'dhcp-host-cache-tests', dhcp_host_cache_tests, + protocol: 'gtest', is_parallel: false, priority: -1, - protocol: 'gtest', ) diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/meson.build b/src/hooks/dhcp/lease_cmds/libloadtests/meson.build index b738883bf7..241fc83767 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/meson.build +++ b/src/hooks/dhcp/lease_cmds/libloadtests/meson.build @@ -25,4 +25,6 @@ test( dhcp_lease_cmds_libload_tests, depends: [dhcp_lease_cmds_lib], protocol: 'gtest', + is_parallel: false, + priority: -1, ) diff --git a/src/hooks/dhcp/ping_check/libloadtests/callout_unittests.cc b/src/hooks/dhcp/ping_check/libloadtests/callout_unittests.cc new file mode 100644 index 0000000000..15f198329d --- /dev/null +++ b/src/hooks/dhcp/ping_check/libloadtests/callout_unittests.cc @@ -0,0 +1,130 @@ +// Copyright (C) 2019-2025 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +/// @file This file contains tests which exercise the pkt[46]_send callouis +/// called by the flexible option hook library. In order to test the callouts +/// one must be able to pass to the load function it hook library parameters +/// because the only way to populate these parameters is by actually loading +/// the library via HooksManager::loadLibraries(). + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +using namespace std; +using namespace isc; +using namespace isc::asiolink; +using namespace isc::hooks; +using namespace isc::data; +using namespace isc::dhcp; +using namespace isc::dhcp::test; +using namespace isc::process; + +namespace { + +/// @brief Structure that holds registered hook indexes. +struct TestHooks { + /// @brief Index of lease4_offer callout. + int hook_index_lease4_offer_; + + /// @brief Constructor + /// + /// The constructor registers hook points for callout tests. + TestHooks() { + hook_index_lease4_offer_ = HooksManager::registerHook("lease4_offer"); + } +}; + +TestHooks testHooks; + +/// @brief Test fixture for testing callouts called by the flex-option library +class CalloutTest : public LogContentTest { +public: + /// @brief Constructor + CalloutTest() { + reset(); + } + + /// @brief Destructor + /// Removes files that may be left over from previous tests + virtual ~CalloutTest() { + reset(); + } + + /// @brief Removes files that may be left over from previous tests + virtual void reset() { + HooksManager::unloadLibraries(); + } + + void addLib(const std::string& lib, ConstElementPtr params) { + libraries_.push_back(HookLibInfo(lib, params)); + } + + void loadLibs() { + EXPECT_TRUE(HooksManager::loadLibraries(libraries_)); + } + + void unloadLibs() { + EXPECT_NO_THROW(HooksManager::unloadLibraries()); + } + + HookLibsCollection libraries_; +}; + +// Simple test which exercises the lease4_offer callout when lease is reused. +TEST_F(CalloutTest, lease4Offer) { + // Set family and proc name. + CfgMgr::instance().setFamily(AF_INET); + Daemon::setProcName("kea-dhcp4"); + + ElementPtr params = Element::createMap(); + params->set("min-ping-requests", Element::create(3)); + params->set("reply-timeout", Element::create(100)); + params->set("enable-ping-check", Element::create(true)); + params->set("ping-cltt-secs", Element::create(60)); + params->set("ping-channel-threads", Element::create(1)); + + // Load the library. + addLib(PING_CHECK_LIB_SO, params); + loadLibs(); + + // Prepare packets. + Pkt4Ptr query(new Pkt4(DHCPDISCOVER, 12345)); + Lease4CollectionPtr leases4(new Lease4Collection()); + uint32_t offer_lifetime = 3600; + Lease4Ptr old_lease; + ConstHostPtr host; + + // Get and setup the callout handle. + EXPECT_TRUE(HooksManager::calloutsPresent(testHooks.hook_index_lease4_offer_)); + CalloutHandlePtr handle = HooksManager::createCalloutHandle(); + + handle->setArgument("query4", query); + handle->setArgument("leases4", leases4); + handle->setArgument("offer_lifetime", offer_lifetime); + handle->setArgument("old_lease", old_lease); + handle->setArgument("host", host); + + // Execute the callout. + EXPECT_NO_THROW(HooksManager::callCallouts(testHooks.hook_index_lease4_offer_, + *handle)); + EXPECT_EQ(CalloutHandle::CalloutNextStep::NEXT_STEP_CONTINUE, handle->getStatus()); + + EXPECT_EQ(1, countFile("PING_CHECK_CHANNEL_NO_LEASE_OR_LEASE_REUSED Ping check skipped: no lease")); +} + +} // end of anonymous namespace diff --git a/src/hooks/dhcp/ping_check/libloadtests/meson.build b/src/hooks/dhcp/ping_check/libloadtests/meson.build index da8bf439c0..05d5fde2e1 100644 --- a/src/hooks/dhcp/ping_check/libloadtests/meson.build +++ b/src/hooks/dhcp/ping_check/libloadtests/meson.build @@ -4,6 +4,7 @@ endif dhcp_ping_check_libloadtests = executable( 'dhcp-ping-check-libload-tests', + 'callout_unittests.cc', 'load_unload_unittests.cc', 'run_unittests.cc', cpp_args: [ @@ -11,7 +12,7 @@ dhcp_ping_check_libloadtests = executable( ], dependencies: [GTEST_DEP, CRYPTO_DEP], include_directories: [include_directories('.')] + INCLUDES, - link_with: LIBS_BUILT_SO_FAR, + link_with: [kea_testutils_lib] + LIBS_BUILT_SO_FAR, ) test( 'dhcp-ping-check-libloadtests', diff --git a/src/hooks/dhcp/ping_check/ping_check_callouts.cc b/src/hooks/dhcp/ping_check/ping_check_callouts.cc index 721617e4bb..c5f5adf689 100644 --- a/src/hooks/dhcp/ping_check/ping_check_callouts.cc +++ b/src/hooks/dhcp/ping_check/ping_check_callouts.cc @@ -146,7 +146,10 @@ int lease4_offer(CalloutHandle& handle) { } if (!lease4) { - isc_throw(InvalidOperation, "leases4 is empty, no lease to check"); + // lease has been reused or there is no address available to check. + LOG_DEBUG(ping_check_logger, isc::log::DBGLVL_TRACE_DETAIL, + PING_CHECK_CHANNEL_NO_LEASE_OR_LEASE_REUSED); + return (0); } // Fetch the parking lot. If it's empty the server is not employing diff --git a/src/hooks/dhcp/ping_check/ping_check_messages.cc b/src/hooks/dhcp/ping_check/ping_check_messages.cc index 7dea2c2397..68525d3bf7 100644 --- a/src/hooks/dhcp/ping_check/ping_check_messages.cc +++ b/src/hooks/dhcp/ping_check/ping_check_messages.cc @@ -9,6 +9,7 @@ extern const isc::log::MessageID PING_CHECK_CHANNEL_ECHO_REPLY_RECEIVED = "PING_ extern const isc::log::MessageID PING_CHECK_CHANNEL_ECHO_REQUEST_SENT = "PING_CHECK_CHANNEL_ECHO_REQUEST_SENT"; extern const isc::log::MessageID PING_CHECK_CHANNEL_MALFORMED_PACKET_RECEIVED = "PING_CHECK_CHANNEL_MALFORMED_PACKET_RECEIVED"; extern const isc::log::MessageID PING_CHECK_CHANNEL_NETWORK_WRITE_ERROR = "PING_CHECK_CHANNEL_NETWORK_WRITE_ERROR"; +extern const isc::log::MessageID PING_CHECK_CHANNEL_NO_LEASE_OR_LEASE_REUSED = "PING_CHECK_CHANNEL_NO_LEASE_OR_LEASE_REUSED"; extern const isc::log::MessageID PING_CHECK_CHANNEL_SOCKET_CLOSED = "PING_CHECK_CHANNEL_SOCKET_CLOSED"; extern const isc::log::MessageID PING_CHECK_CHANNEL_SOCKET_CLOSE_ERROR = "PING_CHECK_CHANNEL_SOCKET_CLOSE_ERROR"; extern const isc::log::MessageID PING_CHECK_CHANNEL_SOCKET_OPENED = "PING_CHECK_CHANNEL_SOCKET_OPENED"; @@ -54,6 +55,7 @@ const char* values[] = { "PING_CHECK_CHANNEL_ECHO_REQUEST_SENT", "to address %1, id %2, sequence %3", "PING_CHECK_CHANNEL_MALFORMED_PACKET_RECEIVED", "error occurred unpacking message %1, discarding it", "PING_CHECK_CHANNEL_NETWORK_WRITE_ERROR", "occurred trying to ping %1, error %2", + "PING_CHECK_CHANNEL_NO_LEASE_OR_LEASE_REUSED", "Ping check skipped: no lease", "PING_CHECK_CHANNEL_SOCKET_CLOSED", "ICMP socket has been closed.", "PING_CHECK_CHANNEL_SOCKET_CLOSE_ERROR", "an attempt to close the ICMP socket failed %1", "PING_CHECK_CHANNEL_SOCKET_OPENED", "ICMP socket been opened successfully.", diff --git a/src/hooks/dhcp/ping_check/ping_check_messages.h b/src/hooks/dhcp/ping_check/ping_check_messages.h index 9326c699e8..53a998a3b6 100644 --- a/src/hooks/dhcp/ping_check/ping_check_messages.h +++ b/src/hooks/dhcp/ping_check/ping_check_messages.h @@ -10,6 +10,7 @@ extern const isc::log::MessageID PING_CHECK_CHANNEL_ECHO_REPLY_RECEIVED; extern const isc::log::MessageID PING_CHECK_CHANNEL_ECHO_REQUEST_SENT; extern const isc::log::MessageID PING_CHECK_CHANNEL_MALFORMED_PACKET_RECEIVED; extern const isc::log::MessageID PING_CHECK_CHANNEL_NETWORK_WRITE_ERROR; +extern const isc::log::MessageID PING_CHECK_CHANNEL_NO_LEASE_OR_LEASE_REUSED; extern const isc::log::MessageID PING_CHECK_CHANNEL_SOCKET_CLOSED; extern const isc::log::MessageID PING_CHECK_CHANNEL_SOCKET_CLOSE_ERROR; extern const isc::log::MessageID PING_CHECK_CHANNEL_SOCKET_OPENED; diff --git a/src/hooks/dhcp/ping_check/ping_check_messages.mes b/src/hooks/dhcp/ping_check/ping_check_messages.mes index 21d407bedf..f3da414cb4 100644 --- a/src/hooks/dhcp/ping_check/ping_check_messages.mes +++ b/src/hooks/dhcp/ping_check/ping_check_messages.mes @@ -119,6 +119,11 @@ This debug message is emitted when the minimum number of ECHO REQUESTs is greater than 1 and the next ECHO REQUEST for a given lease address has been scheduled. +% PING_CHECK_CHANNEL_NO_LEASE_OR_LEASE_REUSED Ping check skipped: no lease +Logged at debug log level 50. +This debug message is emitted when the ping check is omitted because there is no lease. +This could be caused by the fact that the lease has been reused. + % PING_CHECK_MGR_RECEIVED_ECHO_REPLY from %1, id %2, sequence %3 Logged at debug log level 40. This debug message is emitted when an ECHO REPLY message has been received. diff --git a/src/lib/eval/tests/meson.build b/src/lib/eval/tests/meson.build index f020ac0b30..f89d6e7e2e 100644 --- a/src/lib/eval/tests/meson.build +++ b/src/lib/eval/tests/meson.build @@ -14,4 +14,10 @@ kea_eval_tests = executable( include_directories: [include_directories('.')] + INCLUDES, link_with: [kea_testutils_lib, kea_util_unittests_lib] + LIBS_BUILT_SO_FAR, ) -test('kea-eval-tests', kea_eval_tests, protocol: 'gtest') +test( + 'kea-eval-tests', + kea_eval_tests, + protocol: 'gtest', + is_parallel: false, + priority: -1, +) diff --git a/src/lib/hooks/tests/meson.build b/src/lib/hooks/tests/meson.build index 5b8226d484..c8857126ba 100644 --- a/src/lib/hooks/tests/meson.build +++ b/src/lib/hooks/tests/meson.build @@ -122,8 +122,8 @@ test( 'kea-hooks-tests', kea_hooks_tests, depends: [nvl, ivl, fxl, bcl, lcl, lecl, ucl, fcl, pcl, acl], + protocol: 'gtest', is_parallel: false, priority: -1, - protocol: 'gtest', ) diff --git a/src/lib/process/tests/meson.build b/src/lib/process/tests/meson.build index 9fd2d4c0ec..59961f484d 100644 --- a/src/lib/process/tests/meson.build +++ b/src/lib/process/tests/meson.build @@ -33,7 +33,7 @@ kea_process_tests = executable( test( 'kea-process-tests', kea_process_tests, + protocol: 'gtest', is_parallel: false, priority: -1, - protocol: 'gtest' )