[#3144] addressed review comments

diff --git a/ChangeLog b/ChangeLog
index 81a71157f5a18f7fd7fb5b57fb13fd0d56693e4e..4874b54b3b316c5790ef7d8c04f891a05d33487e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,8 +3,8 @@
        which can be used to add interfaces, list currently detected
        interfaces and issue a re-detect procedure which updates the
        interface configuration respectively. The re-detect procedure
-       only adds newly discovered interfaces, without removing any
-       previously detected interfaces.
+       only adds newly discovered interfaces and addresses, without
+       removing any previously detected interfaces or addresses.
        (Gitlab #3144)
 
 Kea 3.1.8 (development) released on April 29, 2026

diff --git a/doc/sphinx/arm/ctrl-channel.rst b/doc/sphinx/arm/ctrl-channel.rst
index cd3d56933eebfd7a6b684ddaf9613e01a841b125..26c86bc243b7a155e90c83fbaf41e0db28fb61fe 100644 (file)
--- a/doc/sphinx/arm/ctrl-channel.rst
+++ b/doc/sphinx/arm/ctrl-channel.rst
@@ -534,8 +534,8 @@ The ``interface-redetect`` Command
 ----------------------------------
 
 The :isccmd:`interface-redetect` command retrieves the list of detected interfaces
-after performing a re-detect procedure which only adds newly discovered interfaces,
-without removing any previously detected interfaces.
+after performing a re-detect procedure which only adds newly discovered interfaces
+and addresses, without removing any previously detected interfaces or addresses.
 This command does not take any parameters."
 
 .. isccmd:: list-commands

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index a7dcaaff54ee284772c8200435e9708ebc115779..cc2568c995392c5e646f21565cc9ba9a8e4ebebf 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -719,6 +719,7 @@ ControlledDhcpv4Srv::commandInterfaceRedetectHandler(const std::string&,
     try {
         // stop thread pool (if running)
         MultiThreadingCriticalSection cs;
+        ReceiverCriticalSection rcs;
         IfaceMgr::instance().detectIfaces(true);
     } catch (const std::exception& ex) {
         error = true;

diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc
index 41341bfc9017fc4d881e0515f45e43e821029cc2..3399670d3f8a25065f036350447efcb8bbf4fe05 100644 (file)
--- a/src/bin/dhcp4/json_config_parser.cc
+++ b/src/bin/dhcp4/json_config_parser.cc
@@ -361,6 +361,8 @@ processDhcp4Config(isc::data::ConstElementPtr config_set) {
     // for option definitions. This is equivalent to committing empty container.
     LibDHCP::setRuntimeOptionDefs(OptionDefSpaceContainer());
 
+    ReceiverCriticalSection rcs;
+
     IfaceMgr::instance().detectIfaces(true);
 
     // Answer will hold the result.

diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index 635904f4251af70779b86c43304f4c2db35801ac..9afb02db662e2818315d54594e267246d11209f3 100644 (file)
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -722,6 +722,7 @@ ControlledDhcpv6Srv::commandInterfaceRedetectHandler(const std::string&,
     try {
         // stop thread pool (if running)
         MultiThreadingCriticalSection cs;
+        ReceiverCriticalSection rcs;
         IfaceMgr::instance().detectIfaces(true);
     } catch (const std::exception& ex) {
         error = true;

diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc
index 41ae05e877bcac689e5e9e0db33cbd6263cc806d..35691341e70f7a62ef2fa96fb8f4af368f5f129d 100644 (file)
--- a/src/bin/dhcp6/json_config_parser.cc
+++ b/src/bin/dhcp6/json_config_parser.cc
@@ -455,6 +455,8 @@ processDhcp6Config(isc::data::ConstElementPtr config_set) {
     // for option definitions. This is equivalent to committing empty container.
     LibDHCP::setRuntimeOptionDefs(OptionDefSpaceContainer());
 
+    ReceiverCriticalSection rcs;
+
     IfaceMgr::instance().detectIfaces(true);
 
     // Answer will hold the result.

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 0f640f0d3ee786b464f4ecd2186256e043c889c8..9a0b2e05dcd2ea6849f0d5fd0223deaca0dba591 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -1859,6 +1859,11 @@ private:
 };
 
 /// @brief RAII class creating a critical section for the receiver thread.
+///
+/// Node: Should be used outside IfaceMgr member functions as this might
+/// cause a deadlock then calling the members in the IfaceMgr constructor
+/// because this class uses IfaceMgr::instance() to access the singleton
+/// instance.
 class ReceiverCriticalSection : public boost::noncopyable {
 public:
 
@@ -1866,10 +1871,10 @@ public:
     ///
     /// Entering the critical section: if running, the receiver
     /// is stopped not clearing the packet queue.
-    ReceiverCriticalSection(IfaceMgr& iface_mgr)
-        : iface_mgr_(iface_mgr), is_running_(iface_mgr.isDHCPReceiverRunning()) {
+    ReceiverCriticalSection()
+        : is_running_(IfaceMgr::instance().isDHCPReceiverRunning()) {
         if (is_running_) {
-            iface_mgr_.stopDHCPReceiver(false);
+            IfaceMgr::instance().stopDHCPReceiver(false);
         }
     }
 
@@ -1878,16 +1883,13 @@ public:
     /// Leaving the critical section: if it was running, the receiver
     /// is started.
     ~ReceiverCriticalSection() {
-        if (is_running_) {
-            auto family = iface_mgr_.getFamily();
-            iface_mgr_.startDHCPReceiver(family);
+        if (is_running_ && !IfaceMgr::instance().isDHCPReceiverRunning()) {
+            auto family = IfaceMgr::instance().getFamily();
+            IfaceMgr::instance().startDHCPReceiver(family);
         }
     }
 
 private:
-    /// @brief The IfaceMgr instance.
-    IfaceMgr& iface_mgr_;
-
     /// @brief Is running flag.
     bool is_running_;
 };

diff --git a/src/lib/dhcp/iface_mgr_bsd.cc b/src/lib/dhcp/iface_mgr_bsd.cc
index a29ab6a6087a6d07c07e9ef81628af114800d5b2..bf4e2f32145c272bd845179b5357edd02b8461be 100644 (file)
--- a/src/lib/dhcp/iface_mgr_bsd.cc
+++ b/src/lib/dhcp/iface_mgr_bsd.cc
@@ -31,7 +31,6 @@ namespace dhcp {
 /// This is a BSD specific interface detection method.
 void
 IfaceMgr::detectIfaces(bool update_only) {
-    ReceiverCriticalSection rcs(*this);
     if (detect_callback_) {
         if (!detect_callback_(update_only)) {
             return;

diff --git a/src/lib/dhcp/iface_mgr_linux.cc b/src/lib/dhcp/iface_mgr_linux.cc
index 8be49e15ecd005b573ec9b720e397b8bf818bcec..c03bad7250b9852d9f1e6f9f2a192a7c96b995de 100644 (file)
--- a/src/lib/dhcp/iface_mgr_linux.cc
+++ b/src/lib/dhcp/iface_mgr_linux.cc
@@ -413,7 +413,6 @@ namespace dhcp {
 /// Uses the socket-based netlink protocol to retrieve the list of interfaces
 /// from the Linux kernel.
 void IfaceMgr::detectIfaces(bool update_only) {
-    ReceiverCriticalSection rcs(*this);
     if (detect_callback_) {
         if (!detect_callback_(update_only)) {
             return;

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 0939882c8961d3e22b0d9bcb810d2d9e69a2af36..5722c0a105bb9e7c0ae1299cd92b5e024286d713 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -4847,7 +4847,14 @@ TEST_F(IfaceMgrTest, receiverCS4) {
     ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4());
     ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
     {
-        ReceiverCriticalSection rcs(IfaceMgr::instance());
+        ReceiverCriticalSection rcs;
+        ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+        ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4());
+        {
+            ReceiverCriticalSection rcs_deep;
+            ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+            ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4());
+        }
         ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
         ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4());
     }
@@ -4864,7 +4871,14 @@ TEST_F(IfaceMgrTest, receiverCS4) {
     ASSERT_TRUE(IfaceMgr::instance().isDHCPReceiverRunning());
     ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4()->empty());
     {
-        ReceiverCriticalSection rcs(IfaceMgr::instance());
+        ReceiverCriticalSection rcs;
+        ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+        ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4()->empty());
+        {
+            ReceiverCriticalSection rcs_deep;
+            ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+            ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4()->empty());
+        }
         ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
         ASSERT_FALSE(IfaceMgr::instance().getPacketQueue4()->empty());
     }
@@ -4881,7 +4895,14 @@ TEST_F(IfaceMgrTest, receiverCS6) {
     ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6());
     ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
     {
-        ReceiverCriticalSection rcs(IfaceMgr::instance());
+        ReceiverCriticalSection rcs;
+        ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+        ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6());
+        {
+            ReceiverCriticalSection rcs_deep;
+            ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+            ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6());
+        }
         ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
         ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6());
     }
@@ -4898,7 +4919,14 @@ TEST_F(IfaceMgrTest, receiverCS6) {
     ASSERT_TRUE(IfaceMgr::instance().isDHCPReceiverRunning());
     ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6()->empty());
     {
-        ReceiverCriticalSection rcs(IfaceMgr::instance());
+        ReceiverCriticalSection rcs;
+        ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+        ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6()->empty());
+        {
+            ReceiverCriticalSection rcs_deep;
+            ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
+            ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6()->empty());
+        }
         ASSERT_FALSE(IfaceMgr::instance().isDHCPReceiverRunning());
         ASSERT_FALSE(IfaceMgr::instance().getPacketQueue6()->empty());
     }

diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc
index 35258cfdce826959a4638c909a254ee8ef088da1..a94c042d189d760817855c5b84e55934552c6cee 100644 (file)
--- a/src/lib/dhcpsrv/cfg_iface.cc
+++ b/src/lib/dhcpsrv/cfg_iface.cc
@@ -156,6 +156,8 @@ CfgIface::openSocketsWithRetry(ReconnectCtlPtr reconnect_ctl,
                                const bool can_use_bcast, bool skip_opened) const {
     MultiThreadingCriticalSection cs;
 
+    ReceiverCriticalSection rcs;
+
     // The detection must be done before resetting and setting the
     // inactive4_ and inactive6_ flags.
     IfaceMgr::instance().detectIfaces(true);