[#1090] pkt4_send drop status now actually drops packet

diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox
index 53209b4dea163ac36568600363d78f43e234358b..3608e75f8b1a4b9ed68a24e58eeadc0fddffd15e 100644 (file)
--- a/src/bin/dhcp4/dhcp4_hooks.dox
+++ b/src/bin/dhcp4/dhcp4_hooks.dox
@@ -298,10 +298,11 @@ to the end of this list.
    cannot be modified.
 
  - <b>Next step action</b>: if any callout installed on the "pkt4_send" hook
-   sets the next step action to SKIP or DROP, the server will not construct the raw
+   sets the next step action to SKIP, the server will not construct the raw
    buffer. The expectation is that if the callout set skip flag, it is
    responsible for constructing raw form on its own. Otherwise the output
-   packet will be sent with zero length.
+   packet will be sent with zero length. If any callout set the next step action
+   to DROP, the server will drop the packet.
 
 @subsection dhcpv4HooksBuffer4Send buffer4_send
 

diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc
index 536f88f3aa7c451c46948861d80b7ecc93e5559a..c9997d138ede1863641152ed32dc7b6f17644d6d 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.cc
+++ b/src/bin/dhcp4/dhcp4_messages.cc
@@ -1,4 +1,4 @@
-// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Thu Jan 23 2020 15:28
+// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Fri Jan 31 2020 15:04
 
 #include <cstddef>
 #include <log/message_types.h>
@@ -72,6 +72,7 @@ extern const isc::log::MessageID DHCP4_HOOK_LEASE4_RELEASE_SKIP = "DHCP4_HOOK_LE
 extern const isc::log::MessageID DHCP4_HOOK_LEASES4_COMMITTED_DROP = "DHCP4_HOOK_LEASES4_COMMITTED_DROP";
 extern const isc::log::MessageID DHCP4_HOOK_LEASES4_COMMITTED_PARK = "DHCP4_HOOK_LEASES4_COMMITTED_PARK";
 extern const isc::log::MessageID DHCP4_HOOK_PACKET_RCVD_SKIP = "DHCP4_HOOK_PACKET_RCVD_SKIP";
+extern const isc::log::MessageID DHCP4_HOOK_PACKET_SEND_DROP = "DHCP4_HOOK_PACKET_SEND_DROP";
 extern const isc::log::MessageID DHCP4_HOOK_PACKET_SEND_SKIP = "DHCP4_HOOK_PACKET_SEND_SKIP";
 extern const isc::log::MessageID DHCP4_HOOK_SUBNET4_SELECT_DROP = "DHCP4_HOOK_SUBNET4_SELECT_DROP";
 extern const isc::log::MessageID DHCP4_HOOK_SUBNET4_SELECT_SKIP = "DHCP4_HOOK_SUBNET4_SELECT_SKIP";
@@ -213,6 +214,7 @@ const char* values[] = {
     "DHCP4_HOOK_LEASES4_COMMITTED_DROP", "%1: packet is dropped, because a callout set the next step to DROP",
     "DHCP4_HOOK_LEASES4_COMMITTED_PARK", "%1: packet is parked, because a callout set the next step to PARK",
     "DHCP4_HOOK_PACKET_RCVD_SKIP", "%1: packet is dropped, because a callout set the next step to SKIP",
+    "DHCP4_HOOK_PACKET_SEND_DROP", "%1: prepared DHCPv4 response was not sent because a callout set the next ste to DROP",
     "DHCP4_HOOK_PACKET_SEND_SKIP", "%1: prepared response is not sent, because a callout set the next stp to SKIP",
     "DHCP4_HOOK_SUBNET4_SELECT_DROP", "%1: packet was dropped, because a callout set the next step to 'drop'",
     "DHCP4_HOOK_SUBNET4_SELECT_SKIP", "%1: no subnet was selected, because a callout set the next skip flag",

diff --git a/src/bin/dhcp4/dhcp4_messages.h b/src/bin/dhcp4/dhcp4_messages.h
index 95d24a13f67db813cac9a5925d00e869e1c27561..e5ff545167aadb9eaa9292bef5483c41b585a8bf 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.h
+++ b/src/bin/dhcp4/dhcp4_messages.h
@@ -1,4 +1,4 @@
-// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Thu Jan 23 2020 15:28
+// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Fri Jan 31 2020 15:04
 
 #ifndef DHCP4_MESSAGES_H
 #define DHCP4_MESSAGES_H
@@ -73,6 +73,7 @@ extern const isc::log::MessageID DHCP4_HOOK_LEASE4_RELEASE_SKIP;
 extern const isc::log::MessageID DHCP4_HOOK_LEASES4_COMMITTED_DROP;
 extern const isc::log::MessageID DHCP4_HOOK_LEASES4_COMMITTED_PARK;
 extern const isc::log::MessageID DHCP4_HOOK_PACKET_RCVD_SKIP;
+extern const isc::log::MessageID DHCP4_HOOK_PACKET_SEND_DROP;
 extern const isc::log::MessageID DHCP4_HOOK_PACKET_SEND_SKIP;
 extern const isc::log::MessageID DHCP4_HOOK_SUBNET4_SELECT_DROP;
 extern const isc::log::MessageID DHCP4_HOOK_SUBNET4_SELECT_SKIP;

diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index 94848e707918e6fbdd71a380031ddd943c5ea901..c7c2ca6c57a83962d7a127061294a2fc9b2fe3dc 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -372,6 +372,15 @@ This debug message is printed when a callout installed on the pkt4_receive
 hook point sets the next step to SKIP. For this particular hook point, the
 value setting of the flag instructs the server to drop the packet.
 
+% DHCP4_HOOK_PACKET_SEND_DROP %1: prepared DHCPv4 response was not sent because a callout set the next ste to DROP
+This debug message is printed when a callout installed on the pkt4_send
+hook point set the next step to DROP. For this particular hook point, the setting
+of the value by a callout instructs the server to drop the packet. This
+effectively means that the client will not get any response, even though
+the server processed client's request and acted on it (e.g. possibly
+allocated a lease). The argument specifies the client and transaction
+identification information.
+
 % DHCP4_HOOK_PACKET_SEND_SKIP %1: prepared response is not sent, because a callout set the next stp to SKIP
 This debug message is printed when a callout installed on the pkt4_send
 hook point sets the next step to SKIP. For this particular hook point, this

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 4bb265ae56e52c6d491c6a4a77cafc4eb9a1e9f2..678d0c736104c6715c3df00baf29dbca0ab4af64 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1226,15 +1226,24 @@ Dhcpv4Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle,
                                    *callout_handle);
 
         // Callouts decided to skip the next processing step. The next
-        // processing step would to pack the packet, so skip at this
-        // stage means "skip packing, the hook already did that".
-        if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) ||
-            (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP)) {
-            LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
-                      DHCP4_HOOK_PACKET_SEND_SKIP)
+        // processing step would to pack the packet (create wire data).
+        // That step will be skipped if any callout sets skip flag.
+        // It essentially means that the callout already did packing,
+        // so the server does not have to do it again.
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
+            LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_SKIP)
                 .arg(query->getLabel());
             skip_pack = true;
         }
+
+        /// Callouts decided to drop the packet.
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
+            LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_DROP)
+                .arg(rsp->getLabel());
+            rsp.reset();
+            return;
+        }
+
     }
 
     if (!skip_pack) {

diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc
index 6b1b1095bde4552c4cf4ff41dba913a24c7e31d1..179d6d32d5932e2dfdc821c61ebdb6e2243b7b4a 100644 (file)
--- a/src/bin/dhcp4/tests/hooks_unittest.cc
+++ b/src/bin/dhcp4/tests/hooks_unittest.cc
@@ -1391,13 +1391,8 @@ TEST_F(HooksDhcpv4SrvTest, drop_pkt4_send) {
     // In particular, it should call registered pkt4_send callback.
     srv_->run();
 
-    // Check that the server sent the message
-    ASSERT_EQ(1, srv_->fake_sent_.size());
-
-    // Get the first packet and check that it has zero length (i.e. the server
-    // did not do packing on its own)
-    Pkt4Ptr sent = srv_->fake_sent_.front();
-    EXPECT_EQ(0, sent->getBuffer().getLength());
+    // Check that the server did not the message
+    ASSERT_EQ(0, srv_->fake_sent_.size());
 
     // Check if the callout handle state was reset after the callout.
     checkCalloutHandleReset(sol);