Browse Source

[3499] bool skip flag replaced by enum status.

Tomek Mrugalski 9 years ago
parent
commit
0849048d17

+ 29 - 26
src/bin/dhcp4/dhcp4_hooks.dox

@@ -40,8 +40,10 @@
  - Description of the hook. This explains where in the processing the hook
    is located, the possible actions a callout attached to this hook could take,
    and a description of the data passed to the callouts.
- - Skip flag action: the action taken by the server if a callout chooses to set
-    the "skip" flag.
+ - 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.
 
 @section dhcpv4HooksHookPoints Hooks in the DHCPv4 Server
 
@@ -64,7 +66,7 @@ packet processing. Hook points that are not specific to packet processing
    are set yet. Callouts installed on this hook point can modify the data
     in the received buffer. The server will parse the buffer afterwards.
 
- - <b>Skip flag action</b>: If any callout sets the skip flag, the server will
+ - <b>Next step status</b>: If any callout sets the status to SKIP, the server will
    skip the buffer parsing. In this case there is an expectation that
    the callout will parse the options carried in the buffer, create
    @c isc::dhcp::Option objects (or derived) and add them to the "query4"
@@ -90,7 +92,7 @@ packet processing. Hook points that are not specific to packet processing
    field has been already parsed and stored in other fields. Therefore,
    the modification in the data_ field has no effect.
 
- - <b>Skip flag action</b>: If any callout sets the skip flag, the server will
+ - <b>Nest step status</b>: If any callout sets the status to SKIP, the server will
    drop the packet and start processing the next one.  The reason for the drop
    will be logged if logging is set to the appropriate debug level.
 
@@ -109,9 +111,9 @@ packet processing. Hook points that are not specific to packet processing
    configured are provided as "subnet4collection". The list itself must
    not be modified.
 
- - <b>Skip flag action</b>: If any callout, installed on "subnet4_select",
-   sets the skip flag, the server will not select any subnet. Packet processing
-   will continue, but will be severely limited.
+ - <b>Next step status</b>: If any callout installed on the "subnet4_select" hook
+   sets the next step status to SKIP, the server will not select any subnet.
+   Packet processing will continue, but will be severely limited.
 
 @subsection dhcpv4HooksLeaseSelect lease4_select
 
@@ -134,11 +136,11 @@ packet processing. Hook points that are not specific to packet processing
    lease won't be inserted into the database (DHCPDISCOVER case), a value of
    false indicates that it will (DHCPREQUEST case).
 
- - <b>Skip flag action</b>: If any callout installed on "lease4_select"
-   sets the skip flag, the server will not assign any lease and the callouts
-   become responsible for the lease assignment. If the callouts fail to provide
-   a lease, the packet processing will continue, but client will not get
-   an address.
+ - <b>Next step status</b>: If any callout installed on the "lease4_select" hook
+   sets the next step action to SKIP, the server will not assign any lease and
+   the callouts become responsible for the lease assignment. If the callouts
+   fail to provide a lease, the packet processing will continue, but client
+   will not get an address.
 
 @subsection dhcpv4HooksLeaseRenew lease4_renew
 
@@ -155,8 +157,8 @@ packet processing. Hook points that are not specific to packet processing
    be taken as the server will attempt to update the lease in the database
    without any additional checks.
 
- - <b>Skip flag action</b>: If any callout installed on "lease4_renew"
-   sets the skip flag, the server will not update the lease in the
+ - <b>Next step status</b>: If any callout installed on the "lease4_renew" hook
+   sets the next step action to SKIP, the server will not update the lease in the
    database and will continue using the old values instead.
 
 @subsection dhcpv4HooksLeaseRelease lease4_release
@@ -170,9 +172,9 @@ packet processing. Hook points that are not specific to packet processing
    The "lease4" argument points to @c Lease4 object that contains the lease to
    be released. It doesn't make sense to modify it at this time.
 
- - <b>Skip flag action</b>: If any callout installed on "lease4_release"
-   sets the skip flag, the server will not delete the lease. It will be
-   kept in the database and will go through the regular expiration/reuse
+ - <b>Nest step status</b>: If any callout installed on the "lease4_release" hook
+   sets the next step action to SKIP, the server will not delete the lease.
+   It will be kept in the database and will go through the regular expiration/reuse
    process.
 
 @subsection dhcpv4HooksPkt4Send pkt4_send
@@ -190,10 +192,11 @@ packet processing. Hook points that are not specific to packet processing
    pkt4_send callouts are complete, so any changes to that field will
    be overwritten.)
 
- - <b>Skip flag action</b>: if any callout sets the skip flag, 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.
+ - <b>Nest step action</b>: if any callout installed on the "pkt4_send" hook
+   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.
 
 @subsection dhcpv4HooksBuffer4Send buffer4_send
 
@@ -211,9 +214,9 @@ packet processing. Hook points that are not specific to packet processing
    time, because they were already used to construct on-wire buffer. Their
    modification would have no effect.
 
- - <b>Skip flag action</b>: if any callout sets the skip flag, the server
-   will drop this response packet. However, the original request packet
-   from a client was processed, so server's state most likely has changed
+ - <b>Next step status</b>: if any callout sets the next step action to SKIP,
+   the server will drop this response packet. However, the original request
+   packet from a client was processed, so server's state most likely has changed
    (e.g. lease was allocated). Setting this flag merely stops the change
    being communicated to the client.
 
@@ -237,8 +240,8 @@ packet processing. Hook points that are not specific to packet processing
   argument is set to "true" if the "flush-reclaimed-timer-wait-time" is
   set to 0 in the server configuration file.
 
-- <b>Skip flag action</b>: if the callout sets the skip flag, the server
-  will assume that the callout has fully reclaimed the lease, i.e.
+- <b>Next step status</b>: if the callout sets the next step action to SKIP,
+  the server will assume that the callout has fully reclaimed the lease, i.e.
   performed the DNS update and updated the lease in the database. The
   server will not perform any further actions on the lease for which the
   skip flag has been set. It is important to note that if the callout

+ 19 - 7
src/bin/dhcp4/dhcp4_srv.cc

@@ -316,13 +316,15 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query) const {
         // Callouts decided to skip this step. This means that no subnet
         // will be selected. Packet processing will continue, but it will
         // be severely limited (i.e. only global options will be assigned)
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                       DHCP4_HOOK_SUBNET4_SELECT_SKIP)
                 .arg(query->getLabel());
             return (Subnet4Ptr());
         }
 
+        /// @todo: Add support for DROP status
+
         // Use whatever subnet was specified by the callout
         callout_handle->getArgument("subnet4", subnet);
     }
@@ -464,7 +466,7 @@ Dhcpv4Srv::run() {
             // processing step would to parse the packet, so skip at this
             // stage means that callouts did the parsing already, so server
             // should skip parsing.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP4_DETAIL, DHCP4_HOOK_BUFFER_RCVD_SKIP)
                     .arg(query->getRemoteAddr().toText())
                     .arg(query->getLocalAddr().toText())
@@ -473,6 +475,8 @@ Dhcpv4Srv::run() {
             }
 
             callout_handle->getArgument("query4", query);
+
+            /// @todo: add support for DROP status
         }
 
         // Unpack the packet information unless the buffer4_receive callouts
@@ -550,12 +554,14 @@ Dhcpv4Srv::run() {
             // Callouts decided to skip the next processing step. The next
             // processing step would to process the packet, so skip at this
             // stage means drop.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_RCVD_SKIP)
                     .arg(query->getLabel());
                 continue;
             }
 
+            /// @todo: Add support for DROP status
+
             callout_handle->getArgument("query4", query);
         }
 
@@ -625,7 +631,7 @@ Dhcpv4Srv::run() {
             callout_handle->deleteAllArguments();
 
             // Clear skip flag if it was set in previous callouts
-            callout_handle->setSkip(false);
+            callout_handle->setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
 
             // Set our response
             callout_handle->setArgument("response4", rsp);
@@ -637,11 +643,13 @@ Dhcpv4Srv::run() {
             // Callouts decided to skip the next processing step. The next
             // processing step would to send the packet, so skip at this
             // stage means "drop response".
-            if (callout_handle->getSkip()) {
+            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;
             }
+
+            /// @todo: Add support for DROP status
         }
 
         if (!skip_pack) {
@@ -677,13 +685,15 @@ Dhcpv4Srv::run() {
                 // Callouts decided to skip the next processing step. The next
                 // processing step would to parse the packet, so skip at this
                 // stage means drop.
-                if (callout_handle->getSkip()) {
+                if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                     LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                               DHCP4_HOOK_BUFFER_SEND_SKIP)
                         .arg(rsp->getLabel());
                     continue;
                 }
 
+                /// @todo: Add support for DROP status.
+
                 callout_handle->getArgument("response4", rsp);
             }
 
@@ -1769,12 +1779,14 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
             // Callouts decided to skip the next processing step. The next
             // processing step would to send the packet, so skip at this
             // stage means "drop response".
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 skip = true;
                 LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                           DHCP4_HOOK_LEASE4_RELEASE_SKIP)
                     .arg(release->getLabel());
             }
+
+            /// @todo add support for DROP status
         }
 
         // Callout didn't indicate to skip the release process. Let's release

+ 4 - 4
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1599,7 +1599,7 @@ public:
     static int
     buffer4_receive_skip(CalloutHandle& callout_handle) {
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // Carry on as usual
         return buffer4_receive_callout(callout_handle);
@@ -1664,7 +1664,7 @@ public:
         Pkt4Ptr pkt;
         callout_handle.getArgument("query4", pkt);
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // carry on as usual
         return pkt4_receive_callout(callout_handle);
@@ -1727,7 +1727,7 @@ public:
         Pkt4Ptr pkt;
         callout_handle.getArgument("response4", pkt);
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // carry on as usual
         return pkt4_send_callout(callout_handle);
@@ -1768,7 +1768,7 @@ public:
     static int
     skip_callout(CalloutHandle& callout_handle) {
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         return (0);
     }

+ 17 - 15
src/bin/dhcp6/dhcp6_hooks.dox

@@ -40,8 +40,10 @@
  - Description of the hook. This explains where in the processing the hook
    is located, the possible actions a callout attached to this hook could take,
    and a description of the data passed to the callouts.
- - Skip flag action: the action taken by the server if a callout chooses to set
-    the "skip" flag.
+ - 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.
 
 @section dhcpv6HooksHookPoints Hooks in the DHCPv6 Server
 
@@ -67,7 +69,7 @@ packet processing. Hook points that are not specific to packet processing
    in their raw form. Unless you need to access to the raw data, it is
    usually better to install your callout on the "pkt6_receive" hook point.
 
- - <b>Skip flag action</b>: If any callout sets the skip flag, the
+ - <b>Next step status</b>: If any callout sets the status to SKIP, the
    server will assume that the callout parsed the buffer and added the
    necessary option objects to the @c options_ field; the server will not
    do any parsing. If the callout sets the skip flag but does not parse
@@ -92,7 +94,7 @@ packet processing. Hook points that are not specific to packet processing
    other fields in the Pkt6 object.  For this reason, modification of the
    @c data_ field would have no effect.)
 
- - <b>Skip flag action</b>: If any callout sets the skip flag, the server will
+ - <b>Next step status</b>: If any callout sets the status to SKIP, the server will
    drop the packet and start processing the next one.  The reason for the drop
    will be logged if logging is set to the appropriate debug level.
 
@@ -110,8 +112,8 @@ packet processing. Hook points that are not specific to packet processing
    configured being provided as "subnet6collection". The list itself must
    not be modified.
 
- - <b>Skip flag action</b>: If any callout installed on "subnet6_select"
-   sets the skip flag, the server will not select any subnet. Packet processing
+ - <b>Next step status</b>: If any callout installed on "subnet6_select"
+   sets the status to SKIP, the server will not select any subnet. Packet processing
    will continue, but will be severely limited.
 
 @subsection dhcpv6HooksLease6Select lease6_select
@@ -135,8 +137,8 @@ packet processing. Hook points that are not specific to packet processing
    of true means that the lease won't be inserted into the database
    (SOLICIT case), a value of false means that it will (REQUEST).
 
- - <b>Skip flag action</b>: If any callout installed on "lease6_select"
-   sets the skip flag, the server will not assign that particular lease.
+ - <b>Next step status</b>: If any callout installed on "lease6_select"
+   sets the status to SKIP, the server will not assign that particular lease.
    Packet processing will continue and the client may get other addresses
    or prefixes if it requested more than one address and/or prefix.
 
@@ -166,8 +168,8 @@ packet processing. Hook points that are not specific to packet processing
    when the update is valid, i.e. conditions such as an invalid addresses
    or invalid iaid renewal attempts will not trigger this hook point.
 
- - <b>Skip flag action</b>: If any callout installed on "lease6_renew"
-   sets the skip flag, the server will not renew the lease. Under these
+ - <b>Next step status</b>: If any callout installed on "lease6_renew"
+   sets the status to SKIP, the server will not renew the lease. Under these
    circumstances, the callout should modify the "ia_na" argument to reflect
    this fact; otherwise the client will think the lease was renewed and continue
    to operate under this assumption.
@@ -185,8 +187,8 @@ packet processing. Hook points that are not specific to packet processing
    make sense to do so as it will be destroyed immediately the callouts
    finish execution.
 
- - <b>Skip flag action</b>: If any callout installed on "lease6_release"
-   sets the skip flag, the server will not delete the lease, which will
+ - <b>Next step status</b>: If any callout installed on "lease6_release"
+   sets the status to SKIP, the server will not delete the lease, which will
    remain in the database until it expires. However, the server will send out
    the response back to the client as if it did.
 
@@ -205,7 +207,7 @@ packet processing. Hook points that are not specific to packet processing
    placed in the @c buffer_out_ field will be overwritten when the callout
    returns. (buffer_out_ is scratch space used for constructing the packet.)
 
- - <b>Skip flag action</b>: If any callout sets the skip flag, the server
+ - <b>Next step status</b>: If any callout sets the status to SKIP, the server
    will assume that the callout did pack the "transaction-id", "message type"
    and option objects into the @c buffer_out_ field and will skip packing part.
    Note that if the callout sets skip flag, but did not prepare the
@@ -229,7 +231,7 @@ packet processing. Hook points that are not specific to packet processing
    to modify that data, it will probably be found easier to modify the
    option objects in a callout attached to the "pkt6_send" hook).
 
- - <b>Skip flag action</b>: If any callout sets the skip flag, the server
+ - <b>Next step status</b>: If any callout sets the status to SKIP, the server
    will drop this response packet. However, the original request packet
    from a client has been processed, so server's state has most likely changed
    (e.g. lease was allocated). Setting this flag merely stops the change
@@ -255,7 +257,7 @@ packet processing. Hook points that are not specific to packet processing
   argument is set to "true" if the "flush-reclaimed-timer-wait-time" is
   set to 0 in the server configuration file.
 
-- <b>Skip flag action</b>: if the callout sets the skip flag, the server
+- <b>Next step status</b>: if the callout sets the status to SKIP, the server
   will assume that the callout has fully reclaimed the lease, i.e.
   performed the DNS update and updated the lease in the database. The
   server will not perform any further actions on the lease for which the

+ 19 - 7
src/bin/dhcp6/dhcp6_srv.cc

@@ -411,7 +411,7 @@ bool Dhcpv6Srv::run() {
             // processing step would to parse the packet, so skip at this
             // stage means that callouts did the parsing already, so server
             // should skip parsing.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP6_DETAIL, DHCP6_HOOK_BUFFER_RCVD_SKIP)
                     .arg(query->getRemoteAddr().toText())
                     .arg(query->getLocalAddr().toText())
@@ -419,6 +419,8 @@ bool Dhcpv6Srv::run() {
                 skip_unpack = true;
             }
 
+            /// @todo: Add support for DROP status.
+
             callout_handle->getArgument("query6", query);
         }
 
@@ -500,12 +502,14 @@ bool Dhcpv6Srv::run() {
             // Callouts decided to skip the next processing step. The next
             // processing step would to process the packet, so skip at this
             // stage means drop.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_PACKET_RCVD_SKIP)
                     .arg(query->getLabel());
                 continue;
             }
 
+            /// @todo: Add support for DROP status.
+
             callout_handle->getArgument("query6", query);
         }
 
@@ -639,11 +643,13 @@ bool Dhcpv6Srv::run() {
                 // 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->getSkip()) {
+                if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                     LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_PACKET_SEND_SKIP)
                         .arg(rsp->getLabel());
                     skip_pack = true;
                 }
+
+                /// @todo: Add support for DROP status
             }
 
             if (!skip_pack) {
@@ -678,12 +684,14 @@ bool Dhcpv6Srv::run() {
                     // Callouts decided to skip the next processing step. The next
                     // processing step would to parse the packet, so skip at this
                     // stage means drop.
-                    if (callout_handle->getSkip()) {
+                    if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                         LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_BUFFER_SEND_SKIP)
                             .arg(rsp->getLabel());
                         continue;
                     }
 
+                    /// @todo: Add support for DROP status
+
                     callout_handle->getArgument("response6", rsp);
                 }
 
@@ -1068,12 +1076,14 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) {
         // subnet will be selected. Packet processing will continue,
         // but it will be severely limited (i.e. only global options
         // will be assigned)
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_SUBNET6_SELECT_SKIP)
                 .arg(question->getLabel());
             return (Subnet6Ptr());
         }
 
+        /// @todo: Add support for DROP status.
+
         // Use whatever subnet was specified by the callout
         callout_handle->getArgument("subnet6", subnet);
     }
@@ -2142,11 +2152,13 @@ Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, const Pkt6Ptr& query,
         // Callouts decided to skip the next processing step. The next
         // processing step would to send the packet, so skip at this
         // stage means "drop response".
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             skip = true;
             LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_LEASE6_RELEASE_NA_SKIP)
                 .arg(query->getLabel());
         }
+
+        /// @todo: Add support for DROP status
     }
 
     // Ok, we've passed all checks. Let's release this address.
@@ -2295,7 +2307,7 @@ Dhcpv6Srv::releaseIA_PD(const DuidPtr& duid, const Pkt6Ptr& query,
         // Call all installed callouts
         HooksManager::callCallouts(Hooks.hook_index_lease6_release_, *callout_handle);
 
-        skip = callout_handle->getSkip();
+        skip = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
     }
 
     // Ok, we've passed all checks. Let's release this prefix.

+ 5 - 5
src/bin/dhcp6/tests/hooks_unittest.cc

@@ -189,7 +189,7 @@ public:
         Pkt6Ptr pkt;
         callout_handle.getArgument("query6", pkt);
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // Carry on as usual
         return pkt6_receive_callout(callout_handle);
@@ -261,7 +261,7 @@ public:
     /// @return always 0
     static int
     buffer6_receive_skip(CalloutHandle& callout_handle) {
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // Carry on as usual
         return buffer6_receive_callout(callout_handle);
@@ -324,7 +324,7 @@ public:
         Pkt6Ptr pkt;
         callout_handle.getArgument("response6", pkt);
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // carry on as usual
         return pkt6_send_callout(callout_handle);
@@ -426,7 +426,7 @@ public:
     lease6_renew_skip_callout(CalloutHandle& callout_handle) {
         callback_name_ = string("lease6_renew");
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         return (0);
     }
@@ -452,7 +452,7 @@ public:
     lease6_release_skip_callout(CalloutHandle& callout_handle) {
         callback_name_ = string("lease6_release");
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         return (0);
     }

+ 22 - 8
src/lib/dhcpsrv/alloc_engine.cc

@@ -978,11 +978,13 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_SELECT_SKIP);
             return (Lease6Ptr());
         }
 
+        /// @todo: Add support for DROP status
+
         // Let's use whatever callout returned. Hopefully it is the same lease
         // we handed to it.
         ctx.callout_handle_->getArgument("lease6", expired);
@@ -1040,7 +1042,7 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_SELECT_SKIP);
             return (Lease6Ptr());
         }
@@ -1245,12 +1247,14 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
         // Callouts decided to skip the next processing step. The next
         // processing step would actually renew the lease, so skip at this
         // stage means "keep the old lease as it is".
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             skip = true;
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
                       DHCPSRV_HOOK_LEASE6_EXTEND_SKIP)
                 .arg(ctx.query_->getName());
         }
+
+        /// @todo: Add support for DROP status
     }
 
     if (!skip) {
@@ -1329,9 +1333,12 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
                 HooksManager::callCallouts(Hooks.hook_index_lease6_expire_,
                                            *callout_handle);
 
-                skipped = callout_handle->getSkip();
+                skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
             }
 
+            /// @todo: Maybe add support for DROP stauts?
+            /// Not sure if we need to support every possible status everywhere.
+
             if (!skipped) {
                 // Generate removal name change request for D2, if required.
                 // This will return immediatelly if the DNS wasn't updated
@@ -1448,9 +1455,12 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
                 HooksManager::callCallouts(Hooks.hook_index_lease4_expire_,
                                            *callout_handle);
 
-                skipped = callout_handle->getSkip();
+                skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
             }
 
+            /// @todo: Maybe add support for DROP stauts?
+            /// Not sure if we need to support every possible status everywhere.
+
             if (!skipped) {
                 // Generate removal name change request for D2, if required.
                 // This will return immediatelly if the DNS wasn't updated
@@ -2146,7 +2156,7 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr) {
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
             return (Lease4Ptr());
         }
@@ -2234,11 +2244,13 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
         // Callouts decided to skip the next processing step. The next
         // processing step would actually renew the lease, so skip at this
         // stage means "keep the old lease as it is".
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             skip = true;
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
                       DHCPSRV_HOOK_LEASE4_RENEW_SKIP);
         }
+
+        /// @todo: Add support for DROP status
     }
 
     if (!ctx.fake_allocation_ && !skip) {
@@ -2312,12 +2324,14 @@ AllocEngine::reuseExpiredLease4(Lease4Ptr& expired,
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
                       DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
             return (Lease4Ptr());
         }
 
+        /// @todo: add support for DROP
+
         // Let's use whatever callout returned. Hopefully it is the same lease
         // we handed to it.
         ctx.callout_handle_->getArgument("lease4", expired);

+ 1 - 1
src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc

@@ -497,7 +497,7 @@ public:
     /// @return Zero.
     static int leaseExpireWithSkipCallout(CalloutHandle& callout_handle) {
         leaseExpireCallout(callout_handle);
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         return (0);
     }

+ 2 - 2
src/lib/hooks/callout_handle.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013,2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -31,7 +31,7 @@ CalloutHandle::CalloutHandle(const boost::shared_ptr<CalloutManager>& manager,
                     const boost::shared_ptr<LibraryManagerCollection>& lmcoll)
     : lm_collection_(lmcoll), arguments_(), context_collection_(),
       manager_(manager), server_hooks_(ServerHooks::getServerHooks()),
-      skip_(false) {
+      next_step_(NEXT_STEP_CONTINUE) {
 
     // Call the "context_create" hook.  We should be OK doing this - although
     // the constructor has not finished running, all the member variables

+ 40 - 13
src/lib/hooks/callout_handle.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013,2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -91,6 +91,18 @@ class LibraryManagerCollection;
 class CalloutHandle {
 public:
 
+    /// @brief Specifies allowed next steps
+    ///
+    /// Those values are used to designate the next step in packet processing.
+    /// They are set by hook callouts and read by the Kea server. See
+    /// @ref setStatus for detailed description of each value.
+    enum CalloutNextStep {
+        NEXT_STEP_CONTINUE = 0, ///< continue normally
+        NEXT_STEP_SKIP = 1,     ///< skip the next processing step
+        NEXT_STEP_DROP = 2      ///< drop the packet
+    };
+
+
     /// Typedef to allow abbreviation of iterator specification in methods.
     /// The std::string is the argument name and the "boost::any" is the
     /// corresponding value associated with it.
@@ -202,24 +214,39 @@ public:
         arguments_.clear();
     }
 
-    /// @brief Set skip flag
+    /// @brief Sets the next processing step.
+    ///
+    /// This method is used by the callouts to determine the next step
+    /// in processing. This method replaces former setSkip() method
+    /// that allowed only two values.
+    ///
+    /// Currently there are three possible value allowed:
+    /// NEXT_STEP_CONTINUE - tells the server to continue processing as usual
+    ///                      (equivalent of previous setSkip(false) )
+    ///
+    /// NEXT_STEP_SKIP - tells the server to skip the processing. Exact meaning
+    ///                  is hook specific. See hook documentation for details.
+    ///                  (equivalent of previous setSkip(true))
+    ///
+    /// NEXT_STEP_DROP - tells the server to unconditionally drop the packet
+    ///                  and do not process it further.
     ///
-    /// Sets the "skip" variable in the callout handle.  This variable is
-    /// interrogated by the server to see if the remaining callouts associated
-    /// with the current hook should be bypassed.
+    /// This variable is interrogated by the server to see if the remaining
+    /// callouts associated with the current hook should be bypassed.
     ///
     /// @param skip New value of the "skip" flag.
-    void setSkip(bool skip) {
-        skip_ = skip;
+    void setStatus(const CalloutNextStep next) {
+        next_step_ = next;
     }
 
-    /// @brief Get skip flag
+    /// @brief Returns the next processing step.
     ///
-    /// Gets the current value of the "skip" flag.
+    /// Gets the current value of the next step. See @ref setStatus for detailed
+    /// definition.
     ///
     /// @return Current value of the skip flag.
-    bool getSkip() const {
-        return (skip_);
+    CalloutNextStep getStatus() const {
+        return (next_step_);
     }
 
     /// @brief Access current library handle
@@ -376,8 +403,8 @@ private:
     /// a reference instead of accessing the singleton within the code.
     ServerHooks& server_hooks_;
 
-    /// "Skip" flag, indicating if the caller should bypass remaining callouts.
-    bool skip_;
+    /// Next processing step, indicating what the server should do next.
+    CalloutNextStep next_step_;
 };
 
 /// A shared pointer to a CalloutHandle object.

+ 1 - 1
src/lib/hooks/callout_manager.cc

@@ -120,7 +120,7 @@ CalloutManager::callCallouts(int hook_index, CalloutHandle& callout_handle) {
     // Clear the "skip" flag so we don't carry state from a previous call.
     // This is done regardless of whether callouts are present to avoid passing
     // any state from the previous call of callCallouts().
-    callout_handle.setSkip(false);
+    callout_handle.setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
 
     // Only initialize and iterate if there are callouts present.  This check
     // also catches the case of an invalid index.

+ 32 - 12
src/lib/hooks/hooks_user.dox

@@ -374,48 +374,68 @@ the server.
 - If you alter an argument, call @c CalloutHandle::setArgument to update the
 value in the @c CalloutHandle object.
 
-@subsubsection hooksdgSkipFlag The "Skip" Flag
+@subsubsection hooksdgSkipFlag Next step status (formerly The "Skip" Flag)
+
+Note: In releases 0.9.2 and earlier, this functionality was provided by
+a boolean flag called "Skip". However, since it only allowed to either continue
+or skip the next processing step and was not extensible to other decisions,
+setSkip(bool) call was replaced with a setStatus(enum) in Kea 1.0. This
+new approach is extensible. If we decide to add new results (e.g. WAIT
+or RATELIMIT), we will be able to do so without changing the API again.
 
 When a to callouts attached to a hook returns, the server will usually continue
 its processing.  However, a callout might have done something that means that
 the server should follow another path.  Possible actions a server could take
 include:
 
+- Continue as usual. This is the default value. Unless callouts explicitly
+change the status, the server will continue processing. There is no need
+to set the status, unless one callout wants to override the status set
+by another callout. This action is represented by CalloutHandle::NEXT_STEP_CONTINUE.
+
 - Skip the next stage of processing because the callout has already
 done it.  For example, a hook is located just before the DHCP server
 allocates an address to the client.  A callout may decide to allocate
 special addresses for certain clients, in which case it needs to tell
-the server not to allocate an address in this case.
+the server not to allocate an address in this case. This action is
+hook specific and is represented by CalloutHandle::NEXT_STEP_SKIP.
+
 - Drop the packet and continue with the next request. A possible scenario
 is a server where a callout inspects the hardware address of the client
 sending the packet and compares it against a black list; if the address
-is on it, the callout notifies the server to drop the packet.
+is on it, the callout notifies the server to drop the packet. This
+action is represented by CalloutHandle::NEXT_STEP_DROP.
 
-To handle these common cases, the @c CalloutHandle has a "skip" flag.
-This is set by a callout when it wishes the server to skip normal
-processing.  It is set false by the hooks framework before callouts on a
-hook are called.  If the flag is set on return, the server will take the
-"skip" action relevant for the hook.
+To handle these common cases, the @c CalloutHandle has a setStatus method.
+This is set by a callout when it wishes the server to change the normal
+processing. Exact meaning is hook specific. Please consult hook API
+documentation for details. For historic reasons (Kea 0.9.2 used a single
+boolean flag called skip that also doubled in some cases as an indicator
+to drop the packet) several hooks use SKIP status to drop the packet.
 
 The methods to get and set the "skip" flag are getSkip and setSkip. Their
 usage is intuitive:
 
 @code
-    // Get the current setting of the skip flag.
-    bool skip = handle.getSkip();
+    // Get the current setting of the next step status.
+    CalloutHandle::NextStepStatus status = handle.getStatus();
+
+    if (status == CalloutHandle::NEXT_STEP_DROP)
+       // Do something...
+       :
 
     // Do some processing...
         :
     if (lease_allocated) {
         // Flag the server to skip the next step of the processing as we
         // already have an address.
-        handle.setSkip(true);
+        handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
     }
     return;
 
 @endcode
 
-Like arguments, the "skip" flag is passed to all callouts on a hook.  Callouts
+Like arguments, the next step status is passed to all callouts on a hook.  Callouts
 later in the list are able to examine (and modify) the settings of earlier ones.
 
 @subsubsection hooksdgCalloutContext Per-Request Context

+ 11 - 9
src/lib/hooks/tests/callout_handle_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013,2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -307,19 +307,21 @@ TEST_F(CalloutHandleTest, DeleteAllArguments) {
     EXPECT_THROW(handle.getArgument("four", value), NoSuchArgument);
 }
 
-// Test the "skip" flag.
-
-TEST_F(CalloutHandleTest, SkipFlag) {
+// Test the "status" field.
+TEST_F(CalloutHandleTest, StatusField) {
     CalloutHandle handle(getCalloutManager());
 
     // Should be false on construction.
-    EXPECT_FALSE(handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, handle.getStatus());
+
+    handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_SKIP, handle.getStatus());
 
-    handle.setSkip(true);
-    EXPECT_TRUE(handle.getSkip());
+    handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_DROP, handle.getStatus());
 
-    handle.setSkip(false);
-    EXPECT_FALSE(handle.getSkip());
+    handle.setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, handle.getStatus());
 }
 
 // Further tests of the "skip" flag and tests of getting the name of the

+ 22 - 13
src/lib/hooks/tests/handles_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013,2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -751,28 +751,37 @@ TEST_F(HandlesTest, DynamicDeregistrationSameHook) {
 
 // Testing the operation of the "skip" flag.  Callouts print the value
 // they see in the flag and either leave it unchanged, set it or clear it.
-
 int
 calloutPrintSkip(CalloutHandle& handle) {
     static const std::string YES("Y");
     static const std::string NO("N");
-
-    HandlesTest::common_string_ = HandlesTest::common_string_ +
-        (handle.getSkip() ? YES : NO);
+    static const std::string DROP("D");
+
+    switch (handle.getStatus()) {
+    case CalloutHandle::NEXT_STEP_CONTINUE:
+        HandlesTest::common_string_ += NO; // skip = no
+        break;
+    case CalloutHandle::NEXT_STEP_SKIP:
+        HandlesTest::common_string_ += YES; // skip = yes
+        break;
+    case CalloutHandle::NEXT_STEP_DROP:
+        HandlesTest::common_string_ += DROP; // drop
+        break;
+    }
     return (0);
 }
 
 int
 calloutSetSkip(CalloutHandle& handle) {
     static_cast<void>(calloutPrintSkip(handle));
-    handle.setSkip(true);
+    handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
     return (0);
 }
 
 int
 calloutClearSkip(CalloutHandle& handle) {
     static_cast<void>(calloutPrintSkip(handle));
-    handle.setSkip(false);
+    handle.setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
     return (0);
 }
 
@@ -806,7 +815,7 @@ TEST_F(HandlesTest, ReturnSkipSet) {
     EXPECT_EQ(std::string("NNYY" "NNYYN" "NNYN"), common_string_);
 
     // ... and check that the skip flag on exit from callCallouts is set.
-    EXPECT_TRUE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_SKIP, callout_handle.getStatus());
 }
 
 // Repeat the test, returning with the skip flag clear.
@@ -838,7 +847,7 @@ TEST_F(HandlesTest, ReturnSkipClear) {
     EXPECT_EQ(std::string("NYY" "NNYNYN" "NNNY"), common_string_);
 
     // ... and check that the skip flag on exit from callCallouts is set.
-    EXPECT_FALSE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle.getStatus());
 }
 
 // Check that the skip flag is cleared when callouts are called - even if
@@ -849,14 +858,14 @@ TEST_F(HandlesTest, NoCalloutsSkipTest) {
     CalloutHandle callout_handle(getCalloutManager());
 
     // Clear the skip flag and call a hook with no callouts.
-    callout_handle.setSkip(false);
+    callout_handle.setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
     getCalloutManager()->callCallouts(alpha_index_, callout_handle);
-    EXPECT_FALSE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle.getStatus());
 
     // Set the skip flag and call a hook with no callouts.
-    callout_handle.setSkip(true);
+    callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
     getCalloutManager()->callCallouts(alpha_index_, callout_handle);
-    EXPECT_FALSE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle.getStatus());
 }
 
 // The next set of callouts do a similar thing to the above "skip" tests,