Browse Source

[master] Merge branch 'trac2983' (4 extra hooks for DHCPv4)

Conflicts:
	ChangeLog
Tomek Mrugalski 11 years ago
parent
commit
8ab644fc53

+ 5 - 0
ChangeLog

@@ -1,3 +1,8 @@
+667.	[func]		tomek
+	Additional hooks (buffer4_receive, lease4_renew,
+	lease4_release, buffer4_send) added to the DHCPv4 server.
+	(Trac #2983, git fd47f18f898695b98623a63a0a1c68d2e4b37568)
+
 666.	[func]		vorner
 	The CmdCtl's command "print_settings" was removed. It served no real
 	purpose and was just experimental leftover from early development.

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

@@ -49,22 +49,45 @@ The following list is ordered by appearance of specific hook points during
 packet processing. Hook points that are not specific to packet processing
 (e.g. lease expiration) will be added to the end of this list.
 
+ @subsection dhcpv4HooksBuffer4Receive buffer4_receive
+
+ - @b Arguments:
+   - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
+
+ - @b Description: this callout is executed when an incoming DHCPv4
+   buffer is received, before its content is parsed. The sole argument
+   - query4 - contains a pointer to an isc::dhcp::Pkt4 object that
+   contains raw information regarding incoming packet, including its
+   source and destination addresses, interface over which it was
+   received, and a raw buffer stored in data_ field. None of the
+   packet fields (op_, hlen_, chaddr_, etc.) are set yet. Callouts
+   installed on this hook point can modify the incoming buffer. The
+   server will parse the buffer afterwards.
+
+ - <b>Skip flag action</b>: If any callout sets the skip flag, the server will
+   skip the buffer parsing. In such case there is an expectation that
+   the callout will parse the buffer and create option objects (see
+   isc::dhcp::Pkt4::addOption()). Otherwise the server will find out
+   that some mandatory options are missing (e.g. DHCP Message Type) and
+   will drop the packet. If you want to have the capability to drop
+   a message, it is better to use skip flag in pkt4_receive callout.
+
  @subsection dhcpv4HooksPkt4Receive pkt4_receive
 
  - @b Arguments:
    - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when an incoming DHCPv4
-   packet is received and its content is parsed. The sole argument -
-   query4 - contains a pointer to an isc::dhcp::Pkt4 object that contains
-   all information regarding incoming packet, including its source and
-   destination addresses, interface over which it was received, a list
-   of all options present within and relay information.  All fields of
-   the Pkt4 object can be modified at this time, except data_. (data_
-   contains the incoming packet as raw buffer. By the time this hook is
-   reached, that information has already parsed and is available though
-   other fields in the Pkt4 object.  For this reason, it doesn't make
-   sense to modify it.)
+   packet is received and its content has been parsed. The sole
+   argument - query4 - contains a pointer to an isc::dhcp::Pkt4 object
+   that contains all information regarding incoming packet, including
+   its source and destination addresses, interface over which it was
+   received, a list of all options present within and relay
+   information.  All fields of the Pkt4 object can be modified at this
+   time, except data_. (By the time this hook is reached, the contents
+   of the data_ 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
    drop the packet and start processing the next one.  The reason for the drop
@@ -97,42 +120,97 @@ packet processing. Hook points that are not specific to packet processing
    - name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed after the server engine
-   has selected a lease for client's request but before the lease
-   has been inserted into the database. Any modifications made to the
-   isc::dhcp::Lease4 object will be stored in the lease's record in the
-   database. The callout should make sure that any modifications are
-   sanity checked as the server will use that data as is with no further
-   checking.\n\n The server processes lease requests for DISCOVER and
-   REQUEST in a very similar way. The only major difference is that
-   for DISCOVER the lease is just selected, but not inserted into
-   the database.  It is possible to distinguish between DISCOVER and
-   REQUEST by checking value of the fake_allocation flag: a value of true
-   means that the lease won't be inserted into the database (DISCOVER),
-   a value of false means that it will (REQUEST).
+   has selected a lease for client's request but before the lease has
+   been inserted into the database. Any modifications made to the
+   isc::dhcp::Lease4 object will be stored in the lease's record in
+   the database. The callout should sanity check all modifications as
+   the server will use that data as is with no further checking.\n\n
+   The server processes lease requests for DISCOVER and REQUEST in a
+   very similar way. The only major difference is that for DISCOVER
+   the lease is just selected, but not inserted into the database.  It
+   is possible to distinguish between DISCOVER and REQUEST by checking
+   value of the fake_allocation flag: a value of true indicates that the
+   lease won't be inserted into the database (DISCOVER), a value of
+   false indicates that it will (REQUEST).
 
  - <b>Skip flag action</b>: If any callout installed on 'lease4_select'
    sets the skip flag, the server will not assign any lease. Packet
    processing will continue, but client will not get an address.
 
+@subsection dhcpv4HooksLeaseRenew lease4_renew
+
+ - @b Arguments:
+   - name: @b subnet4, type: isc::dhcp::Subnet4Ptr, direction: <b>in</b>
+   - name: @b clientid, type: isc::dhcp::ClientId, direction: <b>in</b>
+   - name: @b hwaddr, type: isc::dhcp::HWAddr, direction: <b>in</b>
+   - name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: <b>in/out</b>
+
+ - @b Description: this callout is executed when the server engine
+   is about to renew a lease, as a result of receiving REQUEST/Renewing
+   packet. The lease4 argument points to Lease4 object that contains
+   the updated values. Callout can modify those values. Care should 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 and will
+   use old values instead.
+
+@subsection dhcpv4HooksLeaseRelease lease4_release
+
+ - @b Arguments:
+   - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: <b>in</b>
+   - name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: <b>in</b>
+
+ - @b Description: this callout is executed when the server engine
+   is about to release a lease, as a result of receiving RELEASE packet.
+   The lease4 argument points to 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
+   process.
+
 @subsection dhcpv4HooksPkt4Send pkt4_send
 
  - @b Arguments:
    - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when server's response
-   is about to be send back to the client. The sole argument - response4 -
+   is about to be sent back to the client. The sole argument - response4 -
    contains a pointer to an isc::dhcp::Pkt4 object that contains the
-   packet, with set source and destination addresses, interface over which
-   it will be send, list of all options and relay information.  All fields
-   of the Pkt4 object can be modified at this time, except bufferOut_.
+   packet, with source and destination addresses set, interface over which
+   it will be sent, and a list of all options and relay information.  All fields
+   of the Pkt4 object can be modified at this time, except buffer_out_.
    (This is scratch space used for constructing the packet after all
    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 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
+
+ - @b Arguments:
+   - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
+
+ - @b Description: this callout is executed when server's response
+   is about to be sent back to the client. The sole argument - response4 -
+   contains a pointer to an isc::dhcp::Pkt4 object that contains the
+   packet, with source and destination addresses set, interface over which
+   it will be sent, and a list of all options and relay information. The raw
+   on-wire form is already prepared in buffer_out_ (see isc::dhcp::Pkt4::getBuffer())
+   It doesn't make any sense to modify packet fields or options content
+   at this time, because they were already used to construct on-wire buffer.
+
+ - <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 was most likely changed
    (e.g. lease was allocated). Setting this flag merely stops the change
    being communicated to the client.
 
+
 */

+ 22 - 0
src/bin/dhcp4/dhcp4_messages.mes

@@ -70,6 +70,24 @@ 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 DHCPv4 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.
+
 % 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
@@ -138,6 +156,10 @@ This is a general catch-all message indicating that the processing of a
 received packet failed.  The reason is given in the message.  The server
 will not send a response but will instead ignore the packet.
 
+% DHCP4_PACKET_DROP_NO_TYPE packet received on interface %1 dropped, because of missing msg-type option
+This is a debug message informing that incoming DHCPv4 packet did not
+have mandatory DHCP message type option and thus was dropped.
+
 % DHCP4_PACKET_RECEIVED %1 (type %2) packet received on interface %3
 A debug message noting that the server has received the specified type of
 packet on the specified interface.  Note that a packet marked as UNKNOWN

+ 291 - 142
src/bin/dhcp4/dhcp4_srv.cc

@@ -47,16 +47,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");
     }
 };
 
@@ -64,7 +70,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 {
@@ -83,8 +89,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);
@@ -184,151 +190,245 @@ 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 buffer4_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);
+        // When receiving a packet without message type option, getType() will
+        // throw. Let's set type to -1 as default error indicator.
+        int type = -1;
+        try {
+            type = query->getType();
+        } catch (...) {
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_NO_TYPE)
+                .arg(query->getIface());
+            continue;
+        }
 
-                // Call callouts
-                HooksManager::callCallouts(hook_index_pkt4_receive_,
-                                           *callout_handle);
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_RECEIVED)
+            .arg(serverReceivedPacketName(type))
+            .arg(type)
+            .arg(query->getIface());
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUERY_DATA)
+            .arg(type)
+            .arg(query->toText());
+
+        // Let's execute all callouts registered for pkt4_receive
+        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 pkt4_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);
+
+            // 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 buffer4_send
+            if (HooksManager::getHooksManager()
+                .calloutsPresent(Hooks.hook_index_buffer4_send_)) {
+                CalloutHandlePtr callout_handle = getCalloutHandle(query);
+
+                // Delete previously set arguments
+                callout_handle->deleteAllArguments();
+
+                // Pass incoming packet as argument
+                callout_handle->setArgument("response4", rsp);
 
-                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
-                          DHCP4_RESPONSE_DATA)
-                          .arg(rsp->getType()).arg(rsp->toText());
+                // Call callouts
+                HooksManager::callCallouts(Hooks.hook_index_buffer4_send_,
+                                           *callout_handle);
 
-                try {
-                    rsp->pack();
-                    sendPacket(rsp);
-                } catch (const std::exception& e) {
-                    LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL)
-                              .arg(e.what());
+                // 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());
         }
     }
 
@@ -752,6 +852,10 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 
 Pkt4Ptr
 Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
+
+    /// @todo Uncomment this (see ticket #3116)
+    // sanityCheck(request, MANDATORY);
+
     Pkt4Ptr ack = Pkt4Ptr
         (new Pkt4(DHCPACK, request->getTransid()));
 
@@ -759,6 +863,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
@@ -772,6 +879,9 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 void
 Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
 
+    /// @todo Uncomment this (see ticket #3116)
+    // sanityCheck(release, MANDATORY);
+
     // Try to find client-id
     ClientIdPtr client_id;
     OptionPtr opt = release->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
@@ -813,21 +923,56 @@ 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 lease4_release
+        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.
@@ -840,12 +985,13 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
 
 void
 Dhcpv4Srv::processDecline(Pkt4Ptr& /* decline */) {
-    /// TODO: Implement this.
+    /// @todo Implement this (also see ticket #3116)
 }
 
 Pkt4Ptr
 Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
-    /// TODO: Currently implemented echo mode. Implement this for real
+
+    /// @todo Implement this for real. (also see ticket #3116)
     return (inform);
 }
 
@@ -899,7 +1045,7 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
 
     /// @todo Implement getSubnet4(interface-name)
 
-    // Let's execute all callouts registered for packet_received
+    // Let's execute all callouts registered for subnet4_select
     if (HooksManager::calloutsPresent(hook_index_subnet4_select_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(question);
 
@@ -909,16 +1055,19 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
         // Set new arguments
         callout_handle->setArgument("query4", question);
         callout_handle->setArgument("subnet4", subnet);
-        callout_handle->setArgument("subnet4collection", CfgMgr::instance().getSubnets4());
+        callout_handle->setArgument("subnet4collection",
+                                    CfgMgr::instance().getSubnets4());
 
         // Call user (and server-side) callouts
-        HooksManager::callCallouts(hook_index_subnet4_select_, *callout_handle);
+        HooksManager::callCallouts(hook_index_subnet4_select_,
+                                   *callout_handle);
 
         // Callouts decided to skip this step. This means that no subnet will be
-        // selected. Packet processing will continue, but it will be severly limited
-        // (i.e. only global options will be assigned)
+        // selected. Packet processing will continue, but it will be severly
+        // limited (i.e. only global options will be assigned)
         if (callout_handle->getSkip()) {
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_SUBNET4_SELECT_SKIP);
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS,
+                      DHCP4_HOOK_SUBNET4_SELECT_SKIP);
             return (Subnet4Ptr());
         }
 

+ 678 - 25
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1631,21 +1631,33 @@ TEST_F(Dhcpv4SrvTest, Hooks) {
     NakedDhcpv4Srv srv(0);
 
     // check if appropriate hooks are registered
-    int hook_index_pkt4_received = -1;
-    int hook_index_select_subnet = -1;
-    int hook_index_pkt4_send     = -1;
+    int hook_index_buffer4_receive = -1;
+    int hook_index_pkt4_receive    = -1;
+    int hook_index_select_subnet   = -1;
+    int hook_index_lease4_release  = -1;
+    int hook_index_pkt4_send       = -1;
+    int hook_index_buffer4_send    = -1;
 
     // check if appropriate indexes are set
-    EXPECT_NO_THROW(hook_index_pkt4_received = ServerHooks::getServerHooks()
+    EXPECT_NO_THROW(hook_index_buffer4_receive = ServerHooks::getServerHooks()
+                    .getIndex("buffer4_receive"));
+    EXPECT_NO_THROW(hook_index_pkt4_receive = ServerHooks::getServerHooks()
                     .getIndex("pkt4_receive"));
     EXPECT_NO_THROW(hook_index_select_subnet = ServerHooks::getServerHooks()
                     .getIndex("subnet4_select"));
-    EXPECT_NO_THROW(hook_index_pkt4_send     = ServerHooks::getServerHooks()
+    EXPECT_NO_THROW(hook_index_lease4_release = ServerHooks::getServerHooks()
+                    .getIndex("lease4_release"));
+    EXPECT_NO_THROW(hook_index_pkt4_send = ServerHooks::getServerHooks()
                     .getIndex("pkt4_send"));
+    EXPECT_NO_THROW(hook_index_buffer4_send = ServerHooks::getServerHooks()
+                    .getIndex("buffer4_send"));
 
-    EXPECT_TRUE(hook_index_pkt4_received > 0);
+    EXPECT_TRUE(hook_index_buffer4_receive > 0);
+    EXPECT_TRUE(hook_index_pkt4_receive > 0);
     EXPECT_TRUE(hook_index_select_subnet > 0);
+    EXPECT_TRUE(hook_index_lease4_release > 0);
     EXPECT_TRUE(hook_index_pkt4_send > 0);
+    EXPECT_TRUE(hook_index_buffer4_send > 0);
 }
 
     // a dummy MAC address
@@ -1689,9 +1701,13 @@ public:
     /// @brief destructor (deletes Dhcpv4Srv)
     virtual ~HooksDhcpv4SrvTest() {
 
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("buffer4_receive");
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("buffer4_send");
         HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("pkt4_receive");
         HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("pkt4_send");
         HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("subnet4_select");
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("lease4_renew");
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("lease4_release");
 
         delete srv_;
     }
@@ -1766,6 +1782,67 @@ public:
         return (Pkt4Ptr(new Pkt4(&buf[0], buf.size())));
     }
 
+    /// Test callback that stores received callout name and pkt4 value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    buffer4_receive_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("buffer4_receive");
+
+        callout_handle.getArgument("query4", callback_pkt4_);
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+    /// Test callback that changes hwaddr value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    buffer4_receive_change_hwaddr(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("query4", pkt);
+
+        // If there is at least one option with data
+        if (pkt->data_.size() >= Pkt4::DHCPV4_PKT_HDR_LEN) {
+            // Offset of the first byte of the CHWADDR field. Let's the first
+            // byte to some new value that we could later check
+            pkt->data_[28] = 0xff;
+        }
+
+        // Carry on as usual
+        return buffer4_receive_callout(callout_handle);
+    }
+
+    /// Test callback that deletes MAC address
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    buffer4_receive_delete_hwaddr(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("query4", pkt);
+
+        pkt->data_[2] = 0; // offset 2 is hlen, let's set it to zero
+        memset(&pkt->data_[28], 0, Pkt4::MAX_CHADDR_LEN); // Clear CHADDR content
+
+        // carry on as usual
+        return buffer4_receive_callout(callout_handle);
+    }
+
+    /// Test callback that sets skip flag
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    buffer4_receive_skip(CalloutHandle& callout_handle) {
+
+        callout_handle.setSkip(true);
+
+        // Carry on as usual
+        return buffer4_receive_callout(callout_handle);
+    }
+
     /// test callback that stores received callout name and pkt4 value
     /// @param callout_handle handle passed by the hooks framework
     /// @return always 0
@@ -1894,6 +1971,46 @@ public:
         return pkt4_send_callout(callout_handle);
     }
 
+    /// Test callback that stores received callout name and pkt4 value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    buffer4_send_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("buffer4_send");
+
+        callout_handle.getArgument("response4", callback_pkt4_);
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+    /// Test callback changes the output buffer to a hardcoded value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    buffer4_send_change_callout(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("response4", pkt);
+
+        // modify buffer to set a diffferent payload
+        pkt->getBuffer().clear();
+        pkt->getBuffer().writeData(dummyFile, sizeof(dummyFile));
+
+        return (0);
+    }
+
+    /// Test callback that stores received callout name and pkt4 value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    skip_callout(CalloutHandle& callout_handle) {
+
+        callout_handle.setSkip(true);
+
+        return (0);
+    }
+
     /// Test callback that stores received callout name and subnet4 values
     /// @param callout_handle handle passed by the hooks framework
     /// @return always 0
@@ -1932,10 +2049,44 @@ public:
         return (0);
     }
 
+    /// Test callback that stores received callout name passed parameters
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    lease4_release_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("lease4_release");
+
+        callout_handle.getArgument("query4", callback_pkt4_);
+        callout_handle.getArgument("lease4", callback_lease4_);
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+    /// Test callback that stores received callout name and subnet4 values
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    lease4_renew_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("lease4_renew");
+
+        callout_handle.getArgument("subnet4", callback_subnet4_);
+        callout_handle.getArgument("lease4", callback_lease4_);
+        callout_handle.getArgument("hwaddr", callback_hwaddr_);
+        callout_handle.getArgument("clientid", callback_clientid_);
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+
     /// resets buffers used to store data received by callouts
     void resetCalloutBuffers() {
         callback_name_ = string("");
         callback_pkt4_.reset();
+        callback_lease4_.reset();
+        callback_hwaddr_.reset();
+        callback_clientid_.reset();
         callback_subnet4_.reset();
         callback_subnet4collection_ = NULL;
         callback_argument_names_.clear();
@@ -1952,6 +2103,15 @@ public:
     /// Pkt4 structure returned in the callout
     static Pkt4Ptr callback_pkt4_;
 
+    /// Lease4 structure returned in the callout
+    static Lease4Ptr callback_lease4_;
+
+    /// Hardware address returned in the callout
+    static HWAddrPtr callback_hwaddr_;
+
+    /// Client-id returned in the callout
+    static ClientIdPtr callback_clientid_;
+
     /// Pointer to a subnet received by callout
     static Subnet4Ptr callback_subnet4_;
 
@@ -1967,23 +2127,124 @@ public:
 string HooksDhcpv4SrvTest::callback_name_;
 Pkt4Ptr HooksDhcpv4SrvTest::callback_pkt4_;
 Subnet4Ptr HooksDhcpv4SrvTest::callback_subnet4_;
+HWAddrPtr HooksDhcpv4SrvTest::callback_hwaddr_;
+ClientIdPtr HooksDhcpv4SrvTest::callback_clientid_;
+Lease4Ptr HooksDhcpv4SrvTest::callback_lease4_;
 const Subnet4Collection* HooksDhcpv4SrvTest::callback_subnet4collection_;
 vector<string> HooksDhcpv4SrvTest::callback_argument_names_;
 
+// Checks if callouts installed on pkt4_receive are indeed called and the
+// all necessary parameters are passed.
+//
+// Note that the test name does not follow test naming convention,
+// but the proper hook name is "buffer4_receive".
+TEST_F(HooksDhcpv4SrvTest, Buffer4ReceiveSimple) {
+
+    // Install pkt6_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "buffer4_receive", buffer4_receive_callout));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr dis = generateSimpleDiscover();
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(dis);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered buffer4_receive callback.
+    srv_->run();
+
+    // Check that the callback called is indeed the one we installed
+    EXPECT_EQ("buffer4_receive", callback_name_);
+
+    // Check that pkt4 argument passing was successful and returned proper value
+    EXPECT_TRUE(callback_pkt4_.get() == dis.get());
 
-// Checks if callouts installed on pkt4_received are indeed called and the
+    // Check that all expected parameters are there
+    vector<string> expected_argument_names;
+    expected_argument_names.push_back(string("query4"));
+
+    EXPECT_TRUE(expected_argument_names == callback_argument_names_);
+}
+
+// Checks if callouts installed on buffer4_receive is able to change
+// the values and the parameters are indeed used by the server.
+TEST_F(HooksDhcpv4SrvTest, buffer4RreceiveValueChange) {
+
+    // Install callback that modifies MAC addr of incoming packet
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "buffer4_receive", buffer4_receive_change_hwaddr));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr discover = generateSimpleDiscover();
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(discover);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive6(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered buffer4_receive callback.
+    srv_->run();
+
+    // Check that the server did send a reposonce
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+
+    // Make sure that we received a response
+    Pkt4Ptr offer = srv_->fake_sent_.front();
+    ASSERT_TRUE(offer);
+
+    // Get client-id...
+    HWAddrPtr hwaddr = offer->getHWAddr();
+
+    ASSERT_TRUE(hwaddr); // basic sanity check. HWAddr is always present
+
+    // ... and check if it is the modified value
+    ASSERT_FALSE(hwaddr->hwaddr_.empty()); // there must be a MAC address
+    EXPECT_EQ(0xff, hwaddr->hwaddr_[0]); // check that its first byte was modified
+}
+
+// Checks if callouts installed on buffer4_receive is able to set skip flag that
+// will cause the server to not parse the packet. Even though the packet is valid,
+// the server should eventually drop it, because there won't be mandatory options
+// (or rather option objects) in it.
+TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveSkip) {
+
+    // Install pkt6_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "buffer4_receive", buffer4_receive_skip));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr discover = generateSimpleDiscover();
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(discover);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive6(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt6_receive callback.
+    srv_->run();
+
+    // Check that the server dropped the packet and did not produce any response
+    ASSERT_EQ(0, srv_->fake_sent_.size());
+}
+
+// Checks if callouts installed on pkt4_receive are indeed called and the
 // all necessary parameters are passed.
 //
 // Note that the test name does not follow test naming convention,
 // but the proper hook name is "pkt4_receive".
-TEST_F(HooksDhcpv4SrvTest, simple_pkt4_receive) {
+TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveSimple) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "pkt4_receive", pkt4_receive_callout));
 
     // Let's create a simple DISCOVER
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2016,7 +2277,7 @@ TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_receive) {
                         "pkt4_receive", pkt4_receive_change_clientid));
 
     // Let's create a simple DISCOVER
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2045,14 +2306,14 @@ TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_receive) {
 // Checks if callouts installed on pkt4_received is able to delete
 // existing options and that change impacts server processing (mandatory
 // client-id option is deleted, so the packet is expected to be dropped)
-TEST_F(HooksDhcpv4SrvTest, deleteClientId_pkt4_receive) {
+TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveDeleteClientId) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "pkt4_receive", pkt4_receive_delete_clientid));
 
     // Let's create a simple DISCOVER
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2069,14 +2330,14 @@ TEST_F(HooksDhcpv4SrvTest, deleteClientId_pkt4_receive) {
 
 // Checks if callouts installed on pkt4_received is able to set skip flag that
 // will cause the server to not process the packet (drop), even though it is valid.
-TEST_F(HooksDhcpv4SrvTest, skip_pkt4_receive) {
+TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveSkip) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "pkt4_receive", pkt4_receive_skip));
 
     // Let's create a simple DISCOVER
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2094,14 +2355,14 @@ TEST_F(HooksDhcpv4SrvTest, skip_pkt4_receive) {
 
 // Checks if callouts installed on pkt4_send are indeed called and the
 // all necessary parameters are passed.
-TEST_F(HooksDhcpv4SrvTest, simple_pkt4_send) {
+TEST_F(HooksDhcpv4SrvTest, pkt4SendSimple) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "pkt4_send", pkt4_send_callout));
 
     // Let's create a simple DISCOVER
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2130,14 +2391,14 @@ TEST_F(HooksDhcpv4SrvTest, simple_pkt4_send) {
 
 // Checks if callouts installed on pkt4_send is able to change
 // the values and the packet sent contains those changes
-TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_send) {
+TEST_F(HooksDhcpv4SrvTest, pkt4SendValueChange) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "pkt4_send", pkt4_send_change_serverid));
 
     // Let's create a simple DISCOVER
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2167,14 +2428,14 @@ TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_send) {
 // existing options and that server applies those changes. In particular,
 // we are trying to send a packet without server-id. The packet should
 // be sent
-TEST_F(HooksDhcpv4SrvTest, deleteServerId_pkt4_send) {
+TEST_F(HooksDhcpv4SrvTest, pkt4SendDeleteServerId) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "pkt4_send", pkt4_send_delete_serverid));
 
     // Let's create a simple DISCOVER
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2205,7 +2466,7 @@ TEST_F(HooksDhcpv4SrvTest, skip_pkt4_send) {
                         "pkt4_send", pkt4_send_skip));
 
     // Let's create a simple REQUEST
-    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+    Pkt4Ptr sol = generateSimpleDiscover();
 
     // Simulate that we have received that traffic
     srv_->fakeReceive(sol);
@@ -2213,16 +2474,111 @@ TEST_F(HooksDhcpv4SrvTest, skip_pkt4_send) {
     // Server will now process to run its normal loop, but instead of calling
     // IfaceMgr::receive4(), it will read all packets from the list set by
     // fakeReceive()
+    // 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());
+}
+
+// Checks if callouts installed on buffer4_send are indeed called and the
+// all necessary parameters are passed.
+TEST_F(HooksDhcpv4SrvTest, buffer4SendSimple) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "buffer4_send", buffer4_send_callout));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr discover = generateSimpleDiscover();
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(discover);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
     // In particular, it should call registered pkt4_receive callback.
     srv_->run();
 
-    // check that the server dropped the packet and did not produce any response
+    // Check that the callback called is indeed the one we installed
+    EXPECT_EQ("buffer4_send", callback_name_);
+
+    // Check that there is one packet sent
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+    Pkt4Ptr adv = srv_->fake_sent_.front();
+
+    // Check that pkt4 argument passing was successful and returned proper value
+    EXPECT_TRUE(callback_pkt4_.get() == adv.get());
+
+    // Check that all expected parameters are there
+    vector<string> expected_argument_names;
+    expected_argument_names.push_back(string("response4"));
+    EXPECT_TRUE(expected_argument_names == callback_argument_names_);
+}
+
+// Checks if callouts installed on buffer4_send are indeed called and that
+// the output buffer can be changed.
+TEST_F(HooksDhcpv4SrvTest, buffer4Send) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "buffer4_send", buffer4_send_change_callout));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr discover = generateSimpleDiscover();
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(discover);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // Check that there is one packet sent
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+    Pkt4Ptr adv = srv_->fake_sent_.front();
+
+    // The callout is supposed to fill the output buffer with dummyFile content
+    ASSERT_EQ(sizeof(dummyFile), adv->getBuffer().getLength());
+    EXPECT_EQ(0, memcmp(adv->getBuffer().getData(), dummyFile, sizeof(dummyFile)));
+}
+
+// Checks if callouts installed on buffer4_send can set skip flag and that flag
+// causes the packet to not be sent
+TEST_F(HooksDhcpv4SrvTest, buffer4SendSkip) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "buffer4_send", skip_callout));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr discover = generateSimpleDiscover();
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(discover);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // Check that there is no packet sent.
     ASSERT_EQ(0, srv_->fake_sent_.size());
 }
 
+
 // This test checks if subnet4_select callout is triggered and reports
 // valid parameters
-TEST_F(HooksDhcpv4SrvTest, subnet4_select) {
+TEST_F(HooksDhcpv4SrvTest, subnet4SelectSimple) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2288,9 +2644,9 @@ TEST_F(HooksDhcpv4SrvTest, subnet4_select) {
 
 // This test checks if callout installed on subnet4_select hook point can pick
 // a different subnet.
-TEST_F(HooksDhcpv4SrvTest, subnet_select_change) {
+TEST_F(HooksDhcpv4SrvTest, subnet4SelectChange) {
 
-    // Install pkt4_receive_callout
+    // Install a callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "subnet4_select", subnet4_select_different_subnet_callout));
 
@@ -2345,6 +2701,303 @@ TEST_F(HooksDhcpv4SrvTest, subnet_select_change) {
     EXPECT_TRUE((*subnets)[1]->inPool(addr));
 }
 
+// This test verifies that incoming (positive) REQUEST/Renewing can be handled
+// properly and that callout installed on lease4_renew is triggered with
+// expected parameters.
+TEST_F(HooksDhcpv4SrvTest, lease4RenewSimple) {
 
+    const IOAddress addr("192.0.2.106");
+    const uint32_t temp_t1 = 50;
+    const uint32_t temp_t2 = 75;
+    const uint32_t temp_valid = 100;
+    const time_t temp_timestamp = time(NULL) - 10;
+
+    // Install a callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease4_renew", lease4_renew_callout));
+
+    // Generate client-id also sets client_id_ member
+    OptionPtr clientid = generateClientId();
+
+    // Check that the address we are about to use is indeed in pool
+    ASSERT_TRUE(subnet_->inPool(addr));
+
+    // let's create a lease and put it in the LeaseMgr
+    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
+                              &client_id_->getDuid()[0], client_id_->getDuid().size(),
+                              temp_valid, temp_t1, temp_t2, temp_timestamp,
+                              subnet_->getID()));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
+
+    // Check that the lease is really in the database
+    Lease4Ptr l = LeaseMgrFactory::instance().getLease4(addr);
+    ASSERT_TRUE(l);
+
+    // Let's create a RENEW
+    Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234));
+    req->setRemoteAddr(IOAddress(addr));
+    req->setYiaddr(addr);
+    req->setCiaddr(addr); // client's address
+
+    req->addOption(clientid);
+    req->addOption(srv_->getServerID());
+
+    // Pass it to the server and hope for a REPLY
+    Pkt4Ptr ack = srv_->processRequest(req);
+
+    // Check if we get response at all
+    checkResponse(ack, DHCPACK, 1234);
+
+    // Check that the lease is really in the database
+    l = checkLease(ack, clientid, req->getHWAddr(), addr);
+    ASSERT_TRUE(l);
+
+    // Check that T1, T2, preferred, valid and cltt were really updated
+    EXPECT_EQ(l->t1_, subnet_->getT1());
+    EXPECT_EQ(l->t2_, subnet_->getT2());
+    EXPECT_EQ(l->valid_lft_, subnet_->getValid());
+
+    // Check that the callback called is indeed the one we installed
+    EXPECT_EQ("lease4_renew", callback_name_);
+
+    // Check that hwaddr parameter is passed properly
+    ASSERT_TRUE(callback_hwaddr_);
+    EXPECT_EQ(*callback_hwaddr_, *req->getHWAddr());
+
+    // Check that the subnet is passed properly
+    ASSERT_TRUE(callback_subnet4_);
+    EXPECT_EQ(callback_subnet4_->toText(), subnet_->toText());
+
+    ASSERT_TRUE(callback_clientid_);
+    ASSERT_TRUE(client_id_);
+    EXPECT_TRUE(*client_id_ == *callback_clientid_);
+
+    // Check if all expected parameters were really received
+    vector<string> expected_argument_names;
+    expected_argument_names.push_back("subnet4");
+    expected_argument_names.push_back("clientid");
+    expected_argument_names.push_back("hwaddr");
+    expected_argument_names.push_back("lease4");
+    sort(callback_argument_names_.begin(), callback_argument_names_.end());
+    sort(expected_argument_names.begin(), expected_argument_names.end());
+    EXPECT_TRUE(callback_argument_names_ == expected_argument_names);
+
+    EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr));
+}
+
+// This test verifies that a callout installed on lease4_renew can trigger
+// the server to not renew a lease.
+TEST_F(HooksDhcpv4SrvTest, lease4RenewSkip) {
+
+    const IOAddress addr("192.0.2.106");
+    const uint32_t temp_t1 = 50;
+    const uint32_t temp_t2 = 75;
+    const uint32_t temp_valid = 100;
+    const time_t temp_timestamp = time(NULL) - 10;
+
+    // Install a callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease4_renew", skip_callout));
+
+    // Generate client-id also sets client_id_ member
+    OptionPtr clientid = generateClientId();
+
+    // Check that the address we are about to use is indeed in pool
+    ASSERT_TRUE(subnet_->inPool(addr));
+
+    // let's create a lease and put it in the LeaseMgr
+    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
+                              &client_id_->getDuid()[0], client_id_->getDuid().size(),
+                              temp_valid, temp_t1, temp_t2, temp_timestamp,
+                              subnet_->getID()));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
+
+    // Check that the lease is really in the database
+    Lease4Ptr l = LeaseMgrFactory::instance().getLease4(addr);
+    ASSERT_TRUE(l);
+
+    // Check that T1, T2, preferred, valid and cltt really set.
+    // Constructed lease looks as if it was assigned 10 seconds ago
+    // EXPECT_EQ(l->t1_, temp_t1);
+    // EXPECT_EQ(l->t2_, temp_t2);
+    EXPECT_EQ(l->valid_lft_, temp_valid);
+    EXPECT_EQ(l->cltt_, temp_timestamp);
+
+    // Let's create a RENEW
+    Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234));
+    req->setRemoteAddr(IOAddress(addr));
+    req->setYiaddr(addr);
+    req->setCiaddr(addr); // client's address
+
+    req->addOption(clientid);
+    req->addOption(srv_->getServerID());
+
+    // Pass it to the server and hope for a REPLY
+    Pkt4Ptr ack = srv_->processRequest(req);
+    ASSERT_TRUE(ack);
+
+    // Check that the lease is really in the database
+    l = checkLease(ack, clientid, req->getHWAddr(), addr);
+    ASSERT_TRUE(l);
+
+    // Check that T1, T2, valid and cltt were NOT updated
+    EXPECT_EQ(temp_t1, l->t1_);
+    EXPECT_EQ(temp_t2, l->t2_);
+    EXPECT_EQ(temp_valid, l->valid_lft_);
+    EXPECT_EQ(temp_timestamp, l->cltt_);
+
+    EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr));
+}
+
+// This test verifies that valid RELEASE triggers lease4_release callouts
+TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSimple) {
+
+    const IOAddress addr("192.0.2.106");
+    const uint32_t temp_t1 = 50;
+    const uint32_t temp_t2 = 75;
+    const uint32_t temp_valid = 100;
+    const time_t temp_timestamp = time(NULL) - 10;
+
+    // Install a callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease4_release", lease4_release_callout));
+
+    // Generate client-id also duid_
+    OptionPtr clientid = generateClientId();
+
+    // Check that the address we are about to use is indeed in pool
+    ASSERT_TRUE(subnet_->inPool(addr));
+
+    // Let's create a lease and put it in the LeaseMgr
+    uint8_t mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
+    Lease4Ptr used(new Lease4(addr, mac_addr, sizeof(mac_addr),
+                              &client_id_->getDuid()[0], client_id_->getDuid().size(),
+                              temp_valid, temp_t1, temp_t2, temp_timestamp,
+                              subnet_->getID()));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
+
+    // Check that the lease is really in the database
+    Lease4Ptr l = LeaseMgrFactory::instance().getLease4(addr);
+    ASSERT_TRUE(l);
+
+    // Let's create a RELEASE
+    // Generate client-id also duid_
+    Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
+    rel->setRemoteAddr(addr);
+    rel->setYiaddr(addr);
+    rel->addOption(clientid);
+    rel->addOption(srv_->getServerID());
+    rel->setHWAddr(hw);
+
+    // Pass it to the server and hope for a REPLY
+    // Note: this is no response to RELEASE in DHCPv4
+    EXPECT_NO_THROW(srv_->processRelease(rel));
+
+    // The lease should be gone from LeaseMgr
+    l = LeaseMgrFactory::instance().getLease4(addr);
+    EXPECT_FALSE(l);
+
+    // Try to get the lease by hardware address
+    // @todo: Uncomment this once trac2592 is implemented
+    // Lease4Collection leases = LeaseMgrFactory::instance().getLease4(hw->hwaddr_);
+    // EXPECT_EQ(leases.size(), 0);
+
+    // Try to get it by hw/subnet_id combination
+    l = LeaseMgrFactory::instance().getLease4(hw->hwaddr_, subnet_->getID());
+    EXPECT_FALSE(l);
+
+    // Try by client-id
+    // @todo: Uncomment this once trac2592 is implemented
+    //Lease4Collection leases = LeaseMgrFactory::instance().getLease4(*client_id_);
+    //EXPECT_EQ(leases.size(), 0);
+
+    // Try by client-id/subnet-id
+    l = LeaseMgrFactory::instance().getLease4(*client_id_, subnet_->getID());
+    EXPECT_FALSE(l);
+
+    // Ok, the lease is *really* not there.
+
+    // Check that the callback called is indeed the one we installed
+    EXPECT_EQ("lease4_release", callback_name_);
+
+    // Check that pkt4 argument passing was successful and returned proper value
+    EXPECT_TRUE(callback_pkt4_.get() == rel.get());
+
+    // Check if all expected parameters were really received
+    vector<string> expected_argument_names;
+    expected_argument_names.push_back("query4");
+    expected_argument_names.push_back("lease4");
+    sort(callback_argument_names_.begin(), callback_argument_names_.end());
+    sort(expected_argument_names.begin(), expected_argument_names.end());
+    EXPECT_TRUE(callback_argument_names_ == expected_argument_names);
+}
+
+// This test verifies that skip flag returned by a callout installed on the
+// lease4_release hook point will keep the lease
+TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSkip) {
+
+    const IOAddress addr("192.0.2.106");
+    const uint32_t temp_t1 = 50;
+    const uint32_t temp_t2 = 75;
+    const uint32_t temp_valid = 100;
+    const time_t temp_timestamp = time(NULL) - 10;
+
+    // Install a callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease4_release", skip_callout));
+
+    // Generate client-id also duid_
+    OptionPtr clientid = generateClientId();
+
+    // Check that the address we are about to use is indeed in pool
+    ASSERT_TRUE(subnet_->inPool(addr));
+
+    // Let's create a lease and put it in the LeaseMgr
+    uint8_t mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
+    Lease4Ptr used(new Lease4(addr, mac_addr, sizeof(mac_addr),
+                              &client_id_->getDuid()[0], client_id_->getDuid().size(),
+                              temp_valid, temp_t1, temp_t2, temp_timestamp,
+                              subnet_->getID()));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
+
+    // Check that the lease is really in the database
+    Lease4Ptr l = LeaseMgrFactory::instance().getLease4(addr);
+    ASSERT_TRUE(l);
+
+    // Let's create a RELEASE
+    // Generate client-id also duid_
+    Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
+    rel->setRemoteAddr(addr);
+    rel->setYiaddr(addr);
+    rel->addOption(clientid);
+    rel->addOption(srv_->getServerID());
+    rel->setHWAddr(hw);
+
+    // Pass it to the server and hope for a REPLY
+    // Note: this is no response to RELEASE in DHCPv4
+    EXPECT_NO_THROW(srv_->processRelease(rel));
+
+    // The lease should be still there
+    l = LeaseMgrFactory::instance().getLease4(addr);
+    EXPECT_TRUE(l);
+
+    // Try by client-id/subnet-id
+    l = LeaseMgrFactory::instance().getLease4(*client_id_, subnet_->getID());
+    EXPECT_TRUE(l);
+
+    // Try to get the lease by hardware address
+    // @todo: Uncomment this once trac2592 is implemented
+    // Lease4Collection leases = LeaseMgrFactory::instance().getLease4(hw->hwaddr_);
+    // EXPECT_EQ(leases.size(), 1);
+
+    // Try by client-id
+    // @todo: Uncomment this once trac2592 is implemented
+    //Lease4Collection leases = LeaseMgrFactory::instance().getLease4(*client_id_);
+    //EXPECT_EQ(leases.size(), 1);
+}
 
 } // end of anonymous namespace

+ 1 - 1
src/bin/dhcp6/dhcp6_messages.mes

@@ -118,7 +118,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

+ 2 - 6
src/bin/dhcp6/tests/hooks_unittest.cc

@@ -285,10 +285,6 @@ public:
     /// @return always 0
     static int
     buffer6_receive_skip(CalloutHandle& callout_handle) {
-
-        Pkt6Ptr pkt;
-        callout_handle.getArgument("query6", pkt);
-
         callout_handle.setSkip(true);
 
         // Carry on as usual
@@ -561,7 +557,7 @@ TEST_F(HooksDhcpv6SrvTest, simple_buffer6_receive) {
     // Server will now process to run its normal loop, but instead of calling
     // IfaceMgr::receive6(), it will read all packets from the list set by
     // fakeReceive()
-    // In particular, it should call registered pkt6_receive callback.
+    // In particular, it should call registered buffer6_receive callback.
     srv_->run();
 
     // Check that the callback called is indeed the one we installed
@@ -577,7 +573,7 @@ TEST_F(HooksDhcpv6SrvTest, simple_buffer6_receive) {
     EXPECT_TRUE(expected_argument_names == callback_argument_names_);
 }
 
-// Checks if callouts installed on pkt6_received is able to change
+// Checks if callouts installed on buffer6_receive is able to change
 // the values and the parameters are indeed used by the server.
 TEST_F(HooksDhcpv6SrvTest, valueChange_buffer6_receive) {
 

+ 27 - 27
src/lib/dhcp/pkt4.cc

@@ -33,7 +33,8 @@ namespace dhcp {
 const IOAddress DEFAULT_ADDRESS("0.0.0.0");
 
 Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
-     :local_addr_(DEFAULT_ADDRESS),
+     :buffer_out_(DHCPV4_PKT_HDR_LEN),
+      local_addr_(DEFAULT_ADDRESS),
       remote_addr_(DEFAULT_ADDRESS),
       iface_(""),
       ifindex_(0),
@@ -48,8 +49,7 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
       ciaddr_(DEFAULT_ADDRESS),
       yiaddr_(DEFAULT_ADDRESS),
       siaddr_(DEFAULT_ADDRESS),
-      giaddr_(DEFAULT_ADDRESS),
-      bufferOut_(DHCPV4_PKT_HDR_LEN)
+      giaddr_(DEFAULT_ADDRESS)
 {
     memset(sname_, 0, MAX_SNAME_LEN);
     memset(file_, 0, MAX_FILE_LEN);
@@ -58,7 +58,8 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
 }
 
 Pkt4::Pkt4(const uint8_t* data, size_t len)
-     :local_addr_(DEFAULT_ADDRESS),
+     :buffer_out_(0), // not used, this is RX packet
+      local_addr_(DEFAULT_ADDRESS),
       remote_addr_(DEFAULT_ADDRESS),
       iface_(""),
       ifindex_(0),
@@ -72,8 +73,7 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
       ciaddr_(DEFAULT_ADDRESS),
       yiaddr_(DEFAULT_ADDRESS),
       siaddr_(DEFAULT_ADDRESS),
-      giaddr_(DEFAULT_ADDRESS),
-      bufferOut_(0) // not used, this is RX packet
+      giaddr_(DEFAULT_ADDRESS)
 {
     if (len < DHCPV4_PKT_HDR_LEN) {
         isc_throw(OutOfRange, "Truncated DHCPv4 packet (len=" << len
@@ -111,25 +111,25 @@ Pkt4::pack() {
     try {
         size_t hw_len = hwaddr_->hwaddr_.size();
 
-        bufferOut_.writeUint8(op_);
-        bufferOut_.writeUint8(hwaddr_->htype_);
-        bufferOut_.writeUint8(hw_len < MAX_CHADDR_LEN ? 
+        buffer_out_.writeUint8(op_);
+        buffer_out_.writeUint8(hwaddr_->htype_);
+        buffer_out_.writeUint8(hw_len < MAX_CHADDR_LEN ?
                               hw_len : MAX_CHADDR_LEN);
-        bufferOut_.writeUint8(hops_);
-        bufferOut_.writeUint32(transid_);
-        bufferOut_.writeUint16(secs_);
-        bufferOut_.writeUint16(flags_);
-        bufferOut_.writeUint32(ciaddr_);
-        bufferOut_.writeUint32(yiaddr_);
-        bufferOut_.writeUint32(siaddr_);
-        bufferOut_.writeUint32(giaddr_);
+        buffer_out_.writeUint8(hops_);
+        buffer_out_.writeUint32(transid_);
+        buffer_out_.writeUint16(secs_);
+        buffer_out_.writeUint16(flags_);
+        buffer_out_.writeUint32(ciaddr_);
+        buffer_out_.writeUint32(yiaddr_);
+        buffer_out_.writeUint32(siaddr_);
+        buffer_out_.writeUint32(giaddr_);
 
 
         if (hw_len <= MAX_CHADDR_LEN) {
             // write up to 16 bytes of the hardware address (CHADDR field is 16
             // bytes long in DHCPv4 message).
-            bufferOut_.writeData(&hwaddr_->hwaddr_[0], 
-                                 (hw_len < MAX_CHADDR_LEN ? 
+            buffer_out_.writeData(&hwaddr_->hwaddr_[0],
+                                 (hw_len < MAX_CHADDR_LEN ?
                                   hw_len : MAX_CHADDR_LEN) );
             hw_len = MAX_CHADDR_LEN - hw_len;
         } else {
@@ -138,20 +138,20 @@ Pkt4::pack() {
 
         // write (len) bytes of padding
         vector<uint8_t> zeros(hw_len, 0);
-        bufferOut_.writeData(&zeros[0], hw_len);
-        // bufferOut_.writeData(chaddr_, MAX_CHADDR_LEN);
+        buffer_out_.writeData(&zeros[0], hw_len);
+        // buffer_out_.writeData(chaddr_, MAX_CHADDR_LEN);
 
-        bufferOut_.writeData(sname_, MAX_SNAME_LEN);
-        bufferOut_.writeData(file_, MAX_FILE_LEN);
+        buffer_out_.writeData(sname_, MAX_SNAME_LEN);
+        buffer_out_.writeData(file_, MAX_FILE_LEN);
 
         // write DHCP magic cookie
-        bufferOut_.writeUint32(DHCP_OPTIONS_COOKIE);
+        buffer_out_.writeUint32(DHCP_OPTIONS_COOKIE);
 
-        LibDHCP::packOptions(bufferOut_, options_);
+        LibDHCP::packOptions(buffer_out_, options_);
 
         // add END option that indicates end of options
         // (End option is very simple, just a 255 octet)
-         bufferOut_.writeUint8(DHO_END);
+         buffer_out_.writeUint8(DHO_END);
      } catch(const Exception& e) {
         // An exception is thrown and message will be written to Logger
          isc_throw(InvalidOperation, e.what());
@@ -259,7 +259,7 @@ void Pkt4::setType(uint8_t dhcp_type) {
 }
 
 void Pkt4::repack() {
-    bufferOut_.writeData(&data_[0], data_.size());
+    buffer_out_.writeData(&data_[0], data_.size());
 }
 
 std::string

+ 38 - 28
src/lib/dhcp/pkt4.h

@@ -70,7 +70,7 @@ public:
     ///
     /// Prepares on-wire format of message and all its options.
     /// Options must be stored in options_ field.
-    /// Output buffer will be stored in bufferOut_.
+    /// Output buffer will be stored in buffer_out_.
     ///
     /// @throw InvalidOperation if packing fails
     void
@@ -103,7 +103,7 @@ public:
     ///
     /// This is mostly a diagnostic function. It is being used for sending
     /// received packet. Received packet is stored in bufferIn_, but
-    /// transmitted data is stored in bufferOut_. If we want to send packet
+    /// transmitted data is stored in buffer_out_. If we want to send packet
     /// that we just received, a copy between those two buffers is necessary.
     void repack();
 
@@ -307,11 +307,12 @@ public:
     /// is only valid till Pkt4 object is valid.
     ///
     /// RX packet or TX packet before pack() will return buffer with
-    /// zero length
+    /// zero length. This buffer is returned as non-const, so hooks
+    /// framework (and user's callouts) can modify them if needed
     ///
     /// @return reference to output buffer
-    const isc::util::OutputBuffer&
-    getBuffer() const { return (bufferOut_); };
+    isc::util::OutputBuffer&
+    getBuffer() { return (buffer_out_); };
 
     /// @brief Add an option.
     ///
@@ -489,6 +490,38 @@ public:
     /// @throw isc::Unexpected if timestamp update failed
     void updateTimestamp();
 
+    /// Output buffer (used during message transmission)
+    ///
+    /// @warning This public member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc. This field is also public, because
+    /// it may be modified by callouts (which are written in C++ now,
+    /// but we expect to also have them in Python, so any accesibility
+    /// methods would overly complicate things here and degrade
+    /// performance).
+    isc::util::OutputBuffer buffer_out_;
+
+    /// @brief That's the data of input buffer used in RX packet.
+    ///
+    /// @note Note that InputBuffer does not store the data itself, but just
+    /// expects that data will be valid for the whole life of InputBuffer.
+    /// Therefore we need to keep the data around.
+    ///
+    /// @warning This public member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc. This field is also public, because
+    /// it may be modified by callouts (which are written in C++ now,
+    /// but we expect to also have them in Python, so any accesibility
+    /// methods would overly complicate things here and degrade
+    /// performance).
+    std::vector<uint8_t> data_;
+
 private:
 
     /// @brief Generic method that validates and sets HW address.
@@ -591,29 +624,6 @@ protected:
 
     // end of real DHCPv4 fields
 
-    /// output buffer (used during message transmission)
-    ///
-    /// @warning This protected member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc.
-    isc::util::OutputBuffer bufferOut_;
-
-    /// that's the data of input buffer used in RX packet. Note that
-    /// InputBuffer does not store the data itself, but just expects that
-    /// data will be valid for the whole life of InputBuffer. Therefore we
-    /// need to keep the data around.
-    ///
-    /// @warning This protected member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc.
-    std::vector<uint8_t> data_;
-
     /// collection of options present in this message
     ///
     /// @warning This protected member is accessed by derived

+ 56 - 3
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");
     }
 };
@@ -351,7 +353,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);
             }
@@ -365,7 +368,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) {
@@ -463,8 +467,19 @@ 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)
+    /// @todo: remove this once #3083 is implemented
+    Lease4 old_values = *lease;
+
     lease->subnet_id_ = subnet->getID();
     lease->hwaddr_ = hwaddr->hwaddr_;
     lease->client_id_ = clientid;
@@ -473,10 +488,48 @@ 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();
+
+        // Subnet from which we do the allocation. Convert the general subnet
+        // pointer to a pointer to a Subnet4.  Note that because we are using
+        // boost smart pointers here, we need to do the cast using the boost
+        // version of dynamic_pointer_cast.
+        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(subnet);
+
+        // Pass the parameters
+        callout_handle->setArgument("subnet4", subnet4);
+        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)
+        /// @todo: remove this once #3083 is implemented
+        *lease = old_values;
+    }
 
     return (lease);
 }

+ 3 - 0
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

+ 6 - 0
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

+ 8 - 1
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -1017,6 +1017,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);
 
@@ -1037,7 +1039,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());
@@ -1278,6 +1281,10 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
 ///
 /// It features a couple of callout functions and buffers to store
 /// the data that is accessible via callouts.
+///
+/// Note: lease4_renew callout is tested from DHCPv4 server.
+/// See HooksDhcpv4SrvTest.basic_lease4_renew in
+/// src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
 class HookAllocEngine4Test : public AllocEngine4Test {
 public:
     HookAllocEngine4Test() {

+ 3 - 2
src/lib/hooks/callout_manager.cc

@@ -157,12 +157,13 @@ CalloutManager::callCallouts(int hook_index, CalloutHandle& callout_handle) {
                             .getName(current_hook_))
                         .arg(PointerConverter(i->second).dlsymPtr());
                 }
-            } catch (...) {
+            } catch (const std::exception& e) {
                 // Any exception, not just ones based on isc::Exception
                 LOG_ERROR(hooks_logger, HOOKS_CALLOUT_EXCEPTION)
                     .arg(current_library_)
                     .arg(ServerHooks::getServerHooks().getName(current_hook_))
-                    .arg(PointerConverter(i->second).dlsymPtr());
+                    .arg(PointerConverter(i->second).dlsymPtr())
+                    .arg(e.what());
             }
 
         }

+ 1 - 1
src/lib/hooks/hooks_messages.mes

@@ -44,7 +44,7 @@ is issued.  It identifies the hook to which the callout is attached, the
 index of the library (in the list of loaded libraries) that registered
 it and the address of the callout.  The error is otherwise ignored.
 
-% HOOKS_CALLOUT_EXCEPTION exception thrown by callout on hook %1 registered by library with index %2 (callout address %3)
+% HOOKS_CALLOUT_EXCEPTION exception thrown by callout on hook %1 registered by library with index %2 (callout address %3): %4
 If a callout throws an exception when called, this error message is
 issued.  It identifies the hook to which the callout is attached, the
 index of the library (in the list of loaded libraries) that registered

+ 1 - 1
tests/tools/perfdhcp/perf_pkt4.cc

@@ -40,7 +40,7 @@ PerfPkt4::rawPack() {
                                options_,
                                getTransidOffset(),
                                getTransid(),
-                               bufferOut_));
+                               buffer_out_));
 }
 
 bool