]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#4129] ping check should ignore null lease
authorRazvan Becheriu <razvan@isc.org>
Tue, 14 Oct 2025 18:56:11 +0000 (21:56 +0300)
committerRazvan Becheriu <razvan@isc.org>
Fri, 17 Oct 2025 06:20:45 +0000 (06:20 +0000)
17 files changed:
src/bin/agent/tests/meson.build
src/bin/dhcp4/dhcp4_hooks.dox
src/bin/dhcp4/dhcp4_messages.cc
src/bin/dhcp4/dhcp4_messages.mes
src/bin/dhcp6/dhcp6_messages.cc
src/bin/dhcp6/dhcp6_messages.mes
src/hooks/dhcp/host_cache/tests/meson.build
src/hooks/dhcp/lease_cmds/libloadtests/meson.build
src/hooks/dhcp/ping_check/libloadtests/callout_unittests.cc [new file with mode: 0644]
src/hooks/dhcp/ping_check/libloadtests/meson.build
src/hooks/dhcp/ping_check/ping_check_callouts.cc
src/hooks/dhcp/ping_check/ping_check_messages.cc
src/hooks/dhcp/ping_check/ping_check_messages.h
src/hooks/dhcp/ping_check/ping_check_messages.mes
src/lib/eval/tests/meson.build
src/lib/hooks/tests/meson.build
src/lib/process/tests/meson.build

index 679e4379fdc61f82b14ca0e80361d2fe96f8a1cd..af57c1fc58a2784648c065ef114847013a63732c 100644 (file)
@@ -88,5 +88,3 @@ test(
     is_parallel: false,
     priority: -1,
 )
-
-
index 8f97bd8580b0f9dae477759824098f2262913bf6..04f57c10cc568fd9248e61a1ed18c1e5f4b6e2d9 100644 (file)
@@ -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
index 66c2d4902eee40a81975e5e5b272c2cc7e80aef8..8a84d648d882b5c77aa746419b84532546c91964 100644 (file)
@@ -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",
index 9fd9cf45f9ee87bf2e96c479a2055c7802738130..d6279d4be89606f089bd417816d78c46effa78c2 100644 (file)
@@ -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
index b1dc80f7d17010ba8a738924e84b1c40bffeabd5..9a85ec58dd914494d988991fde2403057dba7065 100644 (file)
@@ -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",
index 760f35e1f7cbe5212307eea3861b4e0f529b5d7b..e411ecc6fe470a3dd2fdd0dd0a6e85d19bf50a00 100644 (file)
@@ -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.
index 89936eb199e89e70c143c6c5a22ff24bf88bab80..7d70de7f4b3a5e688e9e67fa7d56a7a212b40ffb 100644 (file)
@@ -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',
 )
index b738883bf7e0cc809cacc2648af082b3ee72d778..241fc83767ac8c2660a74a1439a279e1d305ba8b 100644 (file)
@@ -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 (file)
index 0000000..15f1983
--- /dev/null
@@ -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 <config.h>
+
+#include <asiolink/io_service_mgr.h>
+#include <dhcp/pkt4.h>
+#include <hooks/hooks.h>
+#include <hooks/hooks_manager.h>
+#include <hooks/callout_manager.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/network_state.h>
+#include <process/daemon.h>
+#include <testutils/log_utils.h>
+
+#include <gtest/gtest.h>
+#include <errno.h>
+
+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
index da8bf439c0570f6a582cda318b958566e50c793d..05d5fde2e13b97c3a92c2b0c5b6ac683fd258a42 100644 (file)
@@ -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',
index 721617e4bb69efd675b63e49d6f62babbe089078..c5f5adf689be983206cab1c86736c45772d13de7 100644 (file)
@@ -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
index 7dea2c2397c3d2cedb7e3e94f97ffb952c5eb127..68525d3bf77de5fae93493a97192657034bbb48a 100644 (file)
@@ -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.",
index 9326c699e86842e43a90851724e613e11b833441..53a998a3b662e0b6c19fe1621aa972736f02925b 100644 (file)
@@ -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;
index 21d407bedf7a4f10a3623697bd7e78a33b56a269..f3da414cb467630d20014b5ca21130fcfa01291c 100644 (file)
@@ -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.
index f020ac0b305d1cf60e94b8bdf5b868e9be867ba1..f89d6e7e2e246d0214494421d376e12ccd6f838c 100644 (file)
@@ -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,
+)
index 5b8226d4842c35ca104a5d011d40065671cea75b..c8857126ba5da85582d26e070c5631d076a3d2c4 100644 (file)
@@ -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',
 )
 
index 9fd2d4c0ec637b9f2c68bb9c6760f68adeef3426..59961f484d7616586672cccdb537f85e3b48e190 100644 (file)
@@ -33,7 +33,7 @@ kea_process_tests = executable(
 test(
     'kea-process-tests',
     kea_process_tests,
+    protocol: 'gtest',
     is_parallel: false,
     priority: -1,
-    protocol: 'gtest'
 )