[2983] Remaining v4 hooks implemented.

src/bin/dhcp4/dhcp4_messages.mes

@@ -70,6 +70,26 @@ This message is printed when DHCPv4 server disables an interface from being
 used to receive DHCPv4 traffic. Sockets on this interface will not be opened
 by the Interface Manager until interface is enabled.
 
+% DHCP4_HOOK_BUFFER_RCVD_SKIP received DHCPv4 buffer was dropped because a callout set the skip flag.
+This debug message is printed when a callout installed on buffer4_receive
+hook point set the skip flag. For this particular hook point, the
+setting of the flag by a callout instructs the server to drop the packet.
+
+% DHCP4_HOOK_BUFFER_SEND_SKIP prepared DHCPv4 response was dropped because a callout set the skip flag.
+This debug message is printed when a callout installed on buffer4_send
+hook point set the skip flag. For this particular hook point, the
+setting of the flag by a callout instructs the server to drop the packet.
+Server completed all the processing (e.g. may have assigned, updated
+or released leases), but the response will not be send to the client.
+
+% DHCP4_HOOK_LEASE4_RELEASE_SKIP DHCPv6 lease was not released because a callout set the skip flag.
+This debug message is printed when a callout installed on lease4_release
+hook point set the skip flag. For this particular hook point, the
+setting of the flag by a callout instructs the server to not release
+a lease. If client requested release of multiples leases (by sending
+multiple IA options), the server will retains this particular lease and
+will proceed with other renewals as usual.
+
 % DHCP4_HOOK_PACKET_RCVD_SKIP received DHCPv4 packet was dropped, because a callout set the skip flag.
 This debug message is printed when a callout installed on the pkt4_receive
 hook point sets the skip flag. For this particular hook point, the

src/bin/dhcp4/dhcp4_srv.cc

@@ -46,16 +46,22 @@ using namespace isc::log;
 using namespace std;
 
 /// Structure that holds registered hook indexes
-struct Dhcp6Hooks {
+struct Dhcp4Hooks {
+    int hook_index_buffer4_receive_;///< index for "buffer4_receive" hook point
     int hook_index_pkt4_receive_;   ///< index for "pkt4_receive" hook point
     int hook_index_subnet4_select_; ///< index for "subnet4_select" hook point
+    int hook_index_lease4_release_; ///< index for "lease4_release" hook point
     int hook_index_pkt4_send_;      ///< index for "pkt4_send" hook point
+    int hook_index_buffer4_send_;   ///< index for "buffer4_send" hook point
 
-    /// Constructor that registers hook points for DHCPv6 engine
-    Dhcp6Hooks() {
+    /// Constructor that registers hook points for DHCPv4 engine
+    Dhcp4Hooks() {
+        hook_index_buffer4_receive_= HooksManager::registerHook("buffer4_receive");
         hook_index_pkt4_receive_   = HooksManager::registerHook("pkt4_receive");
         hook_index_subnet4_select_ = HooksManager::registerHook("subnet4_select");
         hook_index_pkt4_send_      = HooksManager::registerHook("pkt4_send");
+        hook_index_lease4_release_ = HooksManager::registerHook("lease4_release");
+        hook_index_buffer4_send_   = HooksManager::registerHook("buffer4_send");
     }
 };
 
@@ -63,7 +69,7 @@ struct Dhcp6Hooks {
 // will be instantiated (and the constructor run) when the module is loaded.
 // As a result, the hook indexes will be defined before any method in this
 // module is called.
-Dhcp6Hooks Hooks;
+Dhcp4Hooks Hooks;
 
 namespace isc {
 namespace dhcp {
@@ -82,8 +88,8 @@ static const char* SERVER_ID_FILE = "b10-dhcp4-serverid";
 
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
                      const bool direct_response_desired)
-: serverid_(), shutdown_(true), alloc_engine_(), port_(port), 
-    use_bcast_(use_bcast), hook_index_pkt4_receive_(-1), 
+: serverid_(), shutdown_(true), alloc_engine_(), port_(port),
+    use_bcast_(use_bcast), hook_index_pkt4_receive_(-1),
     hook_index_subnet4_select_(-1), hook_index_pkt4_send_(-1) {
 
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
@@ -183,151 +189,229 @@ Dhcpv4Srv::run() {
             LOG_ERROR(dhcp4_logger, DHCP4_PACKET_RECEIVE_FAIL).arg(e.what());
         }
 
-        if (query) {
+        // Timeout may be reached or signal received, which breaks select()
+        // with no reception ocurred
+        if (!query) {
+            continue;
+        }
+
+        bool skip_unpack = false;
+
+        // The packet has just been received so contains the uninterpreted wire
+        // data; execute callouts registered for buffer6_receive.
+        if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_buffer4_receive_)) {
+            CalloutHandlePtr callout_handle = getCalloutHandle(query);
+
+            // Delete previously set arguments
+            callout_handle->deleteAllArguments();
+
+            // Pass incoming packet as argument
+            callout_handle->setArgument("query4", query);
+
+            // Call callouts
+            HooksManager::callCallouts(Hooks.hook_index_buffer4_receive_, *callout_handle);
+
+            // Callouts decided to skip the next processing step. The next
+            // 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()) {
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_BUFFER_RCVD_SKIP);
+                skip_unpack = true;
+            }
+
+            callout_handle->getArgument("query4", query);
+        }
+
+        // Unpack the packet information unless the buffer4_receive callouts
+        // indicated they did it
+        if (!skip_unpack) {
             try {
                 query->unpack();
-
             } catch (const std::exception& e) {
                 // Failed to parse the packet.
                 LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL,
                           DHCP4_PACKET_PARSE_FAIL).arg(e.what());
                 continue;
             }
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_RECEIVED)
-                      .arg(serverReceivedPacketName(query->getType()))
-                      .arg(query->getType())
-                      .arg(query->getIface());
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUERY_DATA)
-                      .arg(static_cast<int>(query->getType()))
-                      .arg(query->toText());
-
-            // Let's execute all callouts registered for packet_received
-            if (HooksManager::calloutsPresent(hook_index_pkt4_receive_)) {
-                CalloutHandlePtr callout_handle = getCalloutHandle(query);
-
-                // Delete previously set arguments
-                callout_handle->deleteAllArguments();
-
-                // Pass incoming packet as argument
-                callout_handle->setArgument("query4", query);
+        }
 
-                // Call callouts
-                HooksManager::callCallouts(hook_index_pkt4_receive_,
-                                           *callout_handle);
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_RECEIVED)
+            .arg(serverReceivedPacketName(query->getType()))
+            .arg(query->getType())
+            .arg(query->getIface());
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUERY_DATA)
+            .arg(static_cast<int>(query->getType()))
+            .arg(query->toText());
+
+        // Let's execute all callouts registered for packet_received
+        if (HooksManager::calloutsPresent(hook_index_pkt4_receive_)) {
+            CalloutHandlePtr callout_handle = getCalloutHandle(query);
+
+            // Delete previously set arguments
+            callout_handle->deleteAllArguments();
+
+            // Pass incoming packet as argument
+            callout_handle->setArgument("query4", query);
+
+            // Call callouts
+            HooksManager::callCallouts(hook_index_pkt4_receive_,
+                                       *callout_handle);
+
+            // 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()) {
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_RCVD_SKIP);
+                continue;
+            }
 
-                // 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()) {
-                    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_RCVD_SKIP);
-                    continue;
-                }
+            callout_handle->getArgument("query4", query);
+        }
 
-                callout_handle->getArgument("query4", query);
+        try {
+            switch (query->getType()) {
+            case DHCPDISCOVER:
+                rsp = processDiscover(query);
+                break;
+
+            case DHCPREQUEST:
+                // Note that REQUEST is used for many things in DHCPv4: for
+                // requesting new leases, renewing existing ones and even
+                // for rebinding.
+                rsp = processRequest(query);
+                break;
+
+            case DHCPRELEASE:
+                processRelease(query);
+                break;
+
+            case DHCPDECLINE:
+                processDecline(query);
+                break;
+
+            case DHCPINFORM:
+                processInform(query);
+                break;
+
+            default:
+                // Only action is to output a message if debug is enabled,
+                // and that is covered by the debug statement before the
+                // "switch" statement.
+                ;
             }
-
-            try {
-                switch (query->getType()) {
-                case DHCPDISCOVER:
-                    rsp = processDiscover(query);
-                    break;
-
-                case DHCPREQUEST:
-                    rsp = processRequest(query);
-                    break;
-
-                case DHCPRELEASE:
-                    processRelease(query);
-                    break;
-
-                case DHCPDECLINE:
-                    processDecline(query);
-                    break;
-
-                case DHCPINFORM:
-                    processInform(query);
-                    break;
-
-                default:
-                    // Only action is to output a message if debug is enabled,
-                    // and that is covered by the debug statement before the
-                    // "switch" statement.
-                    ;
-                }
-            } catch (const isc::Exception& e) {
-
-                // Catch-all exception (at least for ones based on the isc
-                // Exception class, which covers more or less all that
-                // are explicitly raised in the BIND 10 code).  Just log
-                // the problem and ignore the packet. (The problem is logged
-                // as a debug message because debug is disabled by default -
-                // it prevents a DDOS attack based on the sending of problem
-                // packets.)
-                if (dhcp4_logger.isDebugEnabled(DBG_DHCP4_BASIC)) {
-                    std::string source = "unknown";
-                    HWAddrPtr hwptr = query->getHWAddr();
-                    if (hwptr) {
-                        source = hwptr->toText();
-                    }
-                    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
-                              DHCP4_PACKET_PROCESS_FAIL)
-                              .arg(source).arg(e.what());
+        } catch (const isc::Exception& e) {
+
+            // Catch-all exception (at least for ones based on the isc
+            // Exception class, which covers more or less all that
+            // are explicitly raised in the BIND 10 code).  Just log
+            // the problem and ignore the packet. (The problem is logged
+            // as a debug message because debug is disabled by default -
+            // it prevents a DDOS attack based on the sending of problem
+            // packets.)
+            if (dhcp4_logger.isDebugEnabled(DBG_DHCP4_BASIC)) {
+                std::string source = "unknown";
+                HWAddrPtr hwptr = query->getHWAddr();
+                if (hwptr) {
+                    source = hwptr->toText();
                 }
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
+                          DHCP4_PACKET_PROCESS_FAIL)
+                    .arg(source).arg(e.what());
             }
+        }
 
-            if (rsp) {
+        if (!rsp) {
+            continue;
+        }
 
-                adjustRemoteAddr(query, rsp);
+        adjustRemoteAddr(query, rsp);
 
-                if (!rsp->getHops()) {
-                    rsp->setRemotePort(DHCP4_CLIENT_PORT);
-                } else {
-                    rsp->setRemotePort(DHCP4_SERVER_PORT);
-                }
+        if (!rsp->getHops()) {
+            rsp->setRemotePort(DHCP4_CLIENT_PORT);
+        } else {
+            rsp->setRemotePort(DHCP4_SERVER_PORT);
+        }
 
-                rsp->setLocalAddr(query->getLocalAddr());
-                rsp->setLocalPort(DHCP4_SERVER_PORT);
-                rsp->setIface(query->getIface());
-                rsp->setIndex(query->getIndex());
+        rsp->setLocalAddr(query->getLocalAddr());
+        rsp->setLocalPort(DHCP4_SERVER_PORT);
+        rsp->setIface(query->getIface());
+        rsp->setIndex(query->getIndex());
 
-                // Execute all callouts registered for packet6_send
-                if (HooksManager::calloutsPresent(hook_index_pkt4_send_)) {
-                    CalloutHandlePtr callout_handle = getCalloutHandle(query);
+        // Specifies if server should do the packing
+        bool skip_pack = false;
 
-                    // Delete all previous arguments
-                    callout_handle->deleteAllArguments();
+        // Execute all callouts registered for packet6_send
+        if (HooksManager::calloutsPresent(hook_index_pkt4_send_)) {
+            CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-                    // Clear skip flag if it was set in previous callouts
-                    callout_handle->setSkip(false);
+            // Delete all previous arguments
+            callout_handle->deleteAllArguments();
 
-                    // Set our response
-                    callout_handle->setArgument("response4", rsp);
+            // Clear skip flag if it was set in previous callouts
+            callout_handle->setSkip(false);
 
-                    // Call all installed callouts
-                    HooksManager::callCallouts(hook_index_pkt4_send_,
-                                               *callout_handle);
+            // Set our response
+            callout_handle->setArgument("response4", rsp);
 
-                    // 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()) {
-                        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_SKIP);
-                        continue;
-                    }
-                }
+            // Call all installed callouts
+            HooksManager::callCallouts(hook_index_pkt4_send_,
+                                       *callout_handle);
 
-                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
-                          DHCP4_RESPONSE_DATA)
-                          .arg(rsp->getType()).arg(rsp->toText());
+            // 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()) {
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_SKIP);
+                skip_pack = true;
+            }
+        }
+
+        if (!skip_pack) {
+            try {
+                rsp->pack();
+            } catch (const std::exception& e) {
+                LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL)
+                    .arg(e.what());
+            }
+        }
+
+        try {
+            // Now all fields and options are constructed into output wire buffer.
+            // Option objects modification does not make sense anymore. Hooks
+            // can only manipulate wire buffer at this stage.
+            // Let's execute all callouts registered for buffer6_send
+            if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_buffer4_send_)) {
+                CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-                try {
-                    rsp->pack();
-                    sendPacket(rsp);
-                } catch (const std::exception& e) {
-                    LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL)
-                              .arg(e.what());
+                // Delete previously set arguments
+                callout_handle->deleteAllArguments();
+
+                // Pass incoming packet as argument
+                callout_handle->setArgument("response4", rsp);
+
+                // Call callouts
+                HooksManager::callCallouts(Hooks.hook_index_buffer4_send_, *callout_handle);
+
+                // 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()) {
+                    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_BUFFER_SEND_SKIP);
+                    continue;
                 }
+
+                callout_handle->getArgument("response4", rsp);
             }
+
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
+                      DHCP4_RESPONSE_DATA)
+                .arg(static_cast<int>(rsp->getType())).arg(rsp->toText());
+
+            sendPacket(rsp);
+        } catch (const std::exception& e) {
+            LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL)
+                .arg(e.what());
         }
     }
 
@@ -758,6 +842,9 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
     appendDefaultOptions(ack, DHCPACK);
     appendRequestedOptions(request, ack);
 
+    // Note that we treat REQUEST message uniformly, regardless if this is a
+    // first request (requesting for new address), renewing existing address
+    // or even rebinding.
     assignLease(request, ack);
 
     // There are a few basic options that we always want to
@@ -812,21 +899,53 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
             return;
         }
 
-        // Ok, hw and client-id match - let's release the lease.
-        if (LeaseMgrFactory::instance().deleteLease(lease->addr_)) {
+        bool skip = false;
 
-            // Release successful - we're done here
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE)
-                .arg(lease->addr_.toText())
-                .arg(client_id ? client_id->toText() : "(no client-id)")
-                .arg(release->getHWAddr()->toText());
-        } else {
+        // Execute all callouts registered for packet6_send
+        if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_lease4_release_)) {
+            CalloutHandlePtr callout_handle = getCalloutHandle(release);
+
+            // Delete all previous arguments
+            callout_handle->deleteAllArguments();
 
-            // Release failed -
-            LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_FAIL)
-                .arg(lease->addr_.toText())
+            // Pass the original packet
+            callout_handle->setArgument("query4", release);
+
+            // Pass the lease to be updated
+            callout_handle->setArgument("lease4", lease);
+
+            // Call all installed callouts
+            HooksManager::callCallouts(Hooks.hook_index_lease4_release_, *callout_handle);
+
+            // 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()) {
+                skip = true;
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_LEASE4_RELEASE_SKIP);
+            }
+        }
+
+        // Ok, we've passed all checks. Let's release this address.
+        bool success = false; // was the removal operation succeessful?
+
+        // Ok, hw and client-id match - let's release the lease.
+        if (!skip) {
+            success = LeaseMgrFactory::instance().deleteLease(lease->addr_);
+
+            if (success) {
+                // Release successful - we're done here
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE)
+                    .arg(lease->addr_.toText())
+                    .arg(client_id ? client_id->toText() : "(no client-id)")
+                    .arg(release->getHWAddr()->toText());
+            } else {
+                // Release failed -
+                LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_FAIL)
+                    .arg(lease->addr_.toText())
                 .arg(client_id ? client_id->toText() : "(no client-id)")
-                .arg(release->getHWAddr()->toText());
+                    .arg(release->getHWAddr()->toText());
+            }
         }
     } catch (const isc::Exception& ex) {
         // Rethrow the exception with a bit more data.

src/bin/dhcp6/dhcp6_messages.mes

@@ -81,7 +81,7 @@ hook point set the skip flag. For this particular hook point, the
 setting of the flag by a callout instructs the server to drop the packet.
 
 % DHCP6_HOOK_BUFFER_SEND_SKIP prepared DHCPv6 response was dropped because a callout set the skip flag.
-This debug message is printed when a callout installed on buffer6_receive
+This debug message is printed when a callout installed on buffer6_send
 hook point set the skip flag. For this particular hook point, the
 setting of the flag by a callout instructs the server to drop the packet.
 Server completed all the processing (e.g. may have assigned, updated

src/lib/dhcpsrv/alloc_engine.cc

@@ -31,11 +31,13 @@ namespace {
 /// Structure that holds registered hook indexes
 struct AllocEngineHooks {
     int hook_index_lease4_select_; ///< index for "lease4_receive" hook point
+    int hook_index_lease4_renew_;  ///< index for "lease4_renew" hook point
     int hook_index_lease6_select_; ///< index for "lease6_receive" hook point
 
     /// Constructor that registers hook points for AllocationEngine
     AllocEngineHooks() {
         hook_index_lease4_select_ = HooksManager::registerHook("lease4_select");
+        hook_index_lease4_renew_  = HooksManager::registerHook("lease4_renew");
         hook_index_lease6_select_ = HooksManager::registerHook("lease6_select");
     }
 };
@@ -338,7 +340,8 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
         if (existing) {
             // We have a lease already. This is a returning client, probably after
             // its reboot.
-            existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
+            existing = renewLease4(subnet, clientid, hwaddr, existing, callout_handle,
+                                   fake_allocation);
             if (existing) {
                 return (existing);
             }
@@ -352,7 +355,8 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
             if (existing) {
                 // we have a lease already. This is a returning client, probably after
                 // its reboot.
-                existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
+                existing = renewLease4(subnet, clientid, hwaddr, existing, callout_handle,
+                                       fake_allocation);
                 // @todo: produce a warning. We haven't found him using MAC address, but
                 // we found him using client-id
                 if (existing) {
@@ -450,8 +454,18 @@ Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
                                    const ClientIdPtr& clientid,
                                    const HWAddrPtr& hwaddr,
                                    const Lease4Ptr& lease,
+                                   const isc::hooks::CalloutHandlePtr& callout_handle,
                                    bool fake_allocation /* = false */) {
 
+    if (!lease) {
+        isc_throw(InvalidOperation, "Lease4 must be specified");
+    }
+
+    // Let's keep the old data. This is essential if we are using memfile
+    // (the lease returned points directly to the lease4 object in the database)
+    // We'll need it if we want to skip update (i.e. roll back renewal)
+    Lease4 old_values = *lease;
+
     lease->subnet_id_ = subnet->getID();
     lease->hwaddr_ = hwaddr->hwaddr_;
     lease->client_id_ = clientid;
@@ -460,10 +474,41 @@ Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
     lease->t2_ = subnet->getT2();
     lease->valid_lft_ = subnet->getValid();
 
-    if (!fake_allocation) {
+    bool skip = false;
+    // Execute all callouts registered for packet6_send
+    if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_lease4_renew_)) {
+
+        // Delete all previous arguments
+        callout_handle->deleteAllArguments();
+
+        // Pass the parameters
+        callout_handle->setArgument("subnet4", subnet);
+        callout_handle->setArgument("clientid", clientid);
+        callout_handle->setArgument("hwaddr", hwaddr);
+
+        // Pass the lease to be updated
+        callout_handle->setArgument("lease4", lease);
+
+        // Call all installed callouts
+        HooksManager::callCallouts(Hooks.hook_index_lease4_renew_, *callout_handle);
+
+        // Callouts decided to skip the next processing step. The next
+        // processing step would to actually renew the lease, so skip at this
+        // stage means "keep the old lease as it is".
+        if (callout_handle->getSkip()) {
+            skip = true;
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_RENEW_SKIP);
+        }
+    }
+
+    if (!fake_allocation && !skip) {
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease4(lease);
     }
+    if (skip) {
+        // Rollback changes (really useful only for memfile)
+        *lease = old_values;
+    }
 
     return (lease);
 }

src/lib/dhcpsrv/alloc_engine.h

@@ -218,6 +218,8 @@ protected:
     /// @param clientid client identifier
     /// @param hwaddr client's hardware address
     /// @param lease lease to be renewed
+    /// @param callout_handle a callout handle (used in hooks). A lease callouts
+    ///        will be executed if this parameter is passed.
     /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
     ///        an address for DISCOVER that is not really allocated (true)
     Lease4Ptr
@@ -225,6 +227,7 @@ protected:
                 const ClientIdPtr& clientid,
                 const HWAddrPtr& hwaddr,
                 const Lease4Ptr& lease,
+                const isc::hooks::CalloutHandlePtr& callout_handle,
                 bool fake_allocation /* = false */);
 
     /// @brief Allocates an IPv6 lease

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -141,6 +141,12 @@ hook point sets the skip flag. It means that the server was told that
 no lease4 should be assigned. The server will not put that lease in its
 database and the client will get a NAK packet.
 
+% DHCPSRV_HOOK_LEASE4_RENEW_SKIP DHCPv4 lease was not renewed because a callout set the skip flag.
+This debug message is printed when a callout installed on lease4_renew
+hook point set the skip flag. For this particular hook point, the setting
+of the flag by a callout instructs the server to not renew a lease. The
+server will use existing lease as it is, without extending its lifetime.
+
 % DHCPSRV_HOOK_LEASE6_SELECT_SKIP Lease6 (non-temporary) creation was skipped, because of callout skip flag.
 This debug message is printed when a callout installed on lease6_select
 hook point sets the skip flag. It means that the server was told that

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -998,6 +998,8 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
 // called
 TEST_F(AllocEngine4Test, renewLease4) {
     boost::scoped_ptr<AllocEngine> engine;
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
+
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
     ASSERT_TRUE(engine);
 
@@ -1018,7 +1020,8 @@ TEST_F(AllocEngine4Test, renewLease4) {
     // Lease was assigned 45 seconds ago and is valid for 100 seconds. Let's
     // renew it.
     ASSERT_FALSE(lease->expired());
-    lease = engine->renewLease4(subnet_, clientid_, hwaddr_, lease, false);
+    lease = engine->renewLease4(subnet_, clientid_, hwaddr_, lease,
+                                callout_handle, false);
     // Check that he got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr.toText(), lease->addr_.toText());