From: Francis Dupont <fdupont@isc.org>
Date: Tue, 14 Oct 2025 10:01:04 +0000 (+0200)
Subject: [#3337] Made stats responsibility clearer
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6326f13840e25e594a98af6ac5dc1d9a04c8c71f;p=thirdparty%2Fkea.git

[#3337] Made stats responsibility clearer
---

diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox
index 04f57c10cc..b81b361af1 100644
--- a/src/bin/dhcp4/dhcp4_hooks.dox
+++ b/src/bin/dhcp4/dhcp4_hooks.dox
@@ -37,7 +37,9 @@
  - Next step status: the action taken by the server when a callout chooses to set
    status to specified value. Actions not listed explicitly are not supported.
    If a callout sets status to unsupported value, this specific value will be
-   ignored and treated as if the status was CONTINUE.
+   ignored and treated as if the status was CONTINUE. If the action is to
+   drop the incoming query it is the responsibilty of the hook to increase
+   statistics.
 
 @section dhcpv4HooksHookPoints Hooks in the DHCPv4 Server
 
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 93fb8d3b25..cbec84c2f6 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1371,6 +1371,9 @@ Dhcpv4Srv::processPacket(Pkt4Ptr query, bool allow_answer_park) {
                 .arg(query->getRemoteAddr().toText())
                 .arg(query->getLocalAddr().toText())
                 .arg(query->getIface());
+
+            // Not increasing the statistics of the dropped packets because it
+            // is the callouts' responsibility to increase it.
             return (Pkt4Ptr());;
         }
 
@@ -1388,6 +1391,10 @@ Dhcpv4Srv::processPacket(Pkt4Ptr query, bool allow_answer_park) {
         }
 
         callout_handle->getArgument("query4", query);
+        if (!query) {
+            // Please use the status instead of resetting query!
+            return (Pkt4Ptr());
+        }
     }
 
     // Unpack the packet information unless the buffer4_receive callouts
@@ -1498,10 +1505,16 @@ Dhcpv4Srv::processPacket(Pkt4Ptr query, bool allow_answer_park) {
             LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                       DHCP4_HOOK_PACKET_RCVD_SKIP)
                 .arg(query->getLabel());
+            // Not increasing the statistics of the dropped packets because it
+            // is the callouts' responsibility to increase it.
             return (Pkt4Ptr());
         }
 
         callout_handle->getArgument("query4", query);
+        if (!query) {
+            // Please use the status instead of resetting query!
+            return (Pkt4Ptr());
+        }
     }
 
     // Check the DROP special class.
diff --git a/src/bin/dhcp6/dhcp6_hooks.dox b/src/bin/dhcp6/dhcp6_hooks.dox
index 3dca3eb908..f11967a6db 100644
--- a/src/bin/dhcp6/dhcp6_hooks.dox
+++ b/src/bin/dhcp6/dhcp6_hooks.dox
@@ -37,7 +37,9 @@
  - Next step status: the action taken by the server when a callout chooses to set
     status to specified value. Actions not listed explicitly are not supported.
    If a callout sets status to unsupported value, this specific value will be
-   ignored and treated as if the status was CONTINUE.
+   ignored and treated as if the status was CONTINUE. If the action is to
+   drop	the incoming query it is the responsibilty of the hook to increase
+   statistics.
 
 @section dhcpv6HooksHookPoints Hooks in the DHCPv6 Server
 
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index ce67a47c45..8083a4e17a 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -862,10 +862,7 @@ Dhcpv6Srv::processPacket(Pkt6Ptr query) {
                 .arg(query->getIface());
 
             // Not increasing the statistics of the dropped packets because it
-            // is the callouts' responsibility to increase it. There are some
-            // cases when the callouts may elect to not increase the statistics.
-            // For example, packets dropped by the load-balancing algorithm must
-            // not increase the statistics.
+            // is the callouts' responsibility to increase it.
             return (Pkt6Ptr());
         }
 
@@ -980,10 +977,7 @@ Dhcpv6Srv::processPacket(Pkt6Ptr query) {
             LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_PACKET_RCVD_SKIP)
                 .arg(query->getLabel());
             // Not increasing the statistics of the dropped packets because it
-            // is the callouts' responsibility to increase it. There are some
-            // cases when the callouts may elect to not increase the statistics.
-            // For example, packets dropped by the load-balancing algorithm must
-            // not increase the statistics.
+            // is the callouts' responsibility to increase it.
             return (Pkt6Ptr());
         }