[3968] Merge branch 'master' into trac3968

ChangeLog

@@ -1,7 +1,65 @@
+1030.	[bug]		marcin
+	Fixed failing 'reclaimExpiredLeasesTimeout' unit tests on
+	some virtual machines.
+	(trac #4075, git c3a2487f53ecf69edc0a38f574fce17c4332162c)
+
+1029.	[func]		tomek
+	A new hook point lease6_decline has been added. It is called when
+	the DHCPv6 server is about to decline a lease as a result of
+	processing incoming DECLINE message.
+	(Trac #3986, git b6e3f1bbe3595aeba769d627d571e2eeee38a397)
+
+1028.	[func]		marcin
+	Expired leases are processed periodically according to the
+	server configuration.
+	(Trac #3975, git 3bd8891c0b9cb7dc504fa69251610996775cefbf)
+
+1027.	[func]		tomek
+	Expired declined IPv6 leases can now be reclaimed (returned to the
+	available pool) after probation	period elapses.
+	(Trac #3985, git 9aadfa902d898ce1f52b773152a5b34519a9a9fe)
+
+1026.	[doc]		stephen
+	Added documentation for the kea-dhcp4.commands and
+	kea-dhcp6.commands loggers.
+	(Trac #3952, git 3eb5d3185683e05494c1d84ed7195627fce4b6c1)
+
+1025.	[func]		tomek
+	A new hook point lease4_decline has been added. It is called when
+	the DHCPv4 server is about to decline a lease as a result of
+	processing incoming DHCPDECLINE message.
+	(Trac #3986, git 39bde93fe25e4aff52623d4df7fd55c64e0a9c21)
+
+1024.	[func]*		tomek
+	Boolean Skip flag in Hooks API has been replaced by enum status.
+	This is backward incompatible change if you developed hook
+	library that takes advantage of the skip flag. See Hooks
+	Developer Guide for easy steps necessary for migration.
+	(Trac #3499, git 99ca398d4d042a098b5c491368733220db8cdd08)
+
+1023.	[func]		tmark
+	kea-admin now supports upgrading from MySQL schema version 3.0
+	to 4.0.  In addition, the lease6 data dump now contains the
+	text label for lease_hwaddr_source column rather than its
+	numeric value.
+	(Trac #3967, git 2e13ac3b0b278faabe338b00ffee8259c13f5342)
+
+1022.	[func]		fdupont
+	Added support for the V4 link selection sub-option (RFC 3527).  If
+	present in an incoming packet, the server will allocate an address
+	in the subnet identified in the option. If this is impossible, no
+	address will be allocated and the request refused.
+	(Trac #4057, git 8c02cec5ec8e311a9d23fd582d8e9e8647667abb)
+
+1021.	[bug]		stephen
+	Added missing address parameter to ALLOC_ENGINE_V4_REQUEST_OUT_OF_POOL
+	message.
+	(Trac #3996, git 680233550747209a1707e8f920179479b980aa2a)
+
 1020.	[func]		kalmus
 	A general purpose base class for MySQL connection has been
 	implemented.
-	(Trac #3681, git tbd)
+	(Trac #3681, git 884d8bb4a55d3d7b1b8f3f01efb312bd8dec399b)
 
 1019.	[func]		marcin
 	Added new configuration parameters controlling processing of the

configure.ac

@@ -1412,6 +1412,7 @@ AC_CONFIG_FILES([compatcheck/Makefile
                  src/bin/admin/scripts/mysql/Makefile
                  src/bin/admin/scripts/mysql/upgrade_1.0_to_2.0.sh
                  src/bin/admin/scripts/mysql/upgrade_2.0_to_3.0.sh
+                 src/bin/admin/scripts/mysql/upgrade_3.0_to_4.0.sh
                  src/bin/admin/scripts/pgsql/Makefile
                  src/hooks/Makefile
                  src/hooks/dhcp/Makefile

doc/guide/dhcp6-srv.xml

@@ -2680,8 +2680,30 @@ should include options from the isc option space:
       </listitem>
     </itemizedlist>
     </para>
+  </section>
+
+    <section id="dhcp6-decline">
+      <title>Duplicate Addresses (DECLINE support)</title>
+      <note>
+        <para>@todo: Full text will be added as part of #3990.</para>
+      </note>
+
+      <para>
+        The server does not decrease assigned-addresses statistics
+        when Decline message is received and processed successfully. While
+        technically a declined address is no longer assigned, the primary usage
+        of the assigned-addresses statistic is to monitor pool utilization. Most
+        people would forget to include declined-addresses in the calculation,
+        and simply do assigned-addresses/total-addresses. This would have a bias
+        towards under-representing pool utilization. As this has a
+        potential for major issues, we decided not to decrease assigned
+        addresses immediately after receiving Decline, but to do
+        it later when we recover the address back to the available pool.
+      </para>
+
     </section>
 
+
     <section id="dhcp6-stats">
       <title>Statistics in DHCPv6 server</title>
       <note>

doc/guide/logging.xml

@@ -200,6 +200,12 @@
             </simpara>
           </listitem>
           <listitem>
+            <simpara><command>kea-dhcp4.commands</command> - this logger is used
+            to log messages relating to the handling of commands received by the
+            the DHCPv4 server over the command channel.
+            </simpara>
+          </listitem>
+          <listitem>
             <simpara><command>kea-dhcp4.ddns</command> - this logger is used by
             the DHCPv4 server to log messages related to the Client FQDN and
             Hostname option processing. It also includes log messages
@@ -259,6 +265,13 @@
             provided.</simpara>
           </listitem>
           <listitem>
+            <simpara><command>kea-dhcp6.alloc-engine</command> - this is the
+            logger used by the lease allocation engine, which is responsible
+            for managing leases in the lease database, i.e. create, modify
+            and remove DHCPv6 leases as a result of processing messages from
+            the clients.</simpara>
+          </listitem>
+          <listitem>
             <simpara><command>kea-dhcp6.bad-packets</command> - this is the
             logger used by the DHCPv6 server deamon for logging inbound client
             packets that were dropped.</simpara>
@@ -270,6 +283,12 @@
             </simpara>
           </listitem>
           <listitem>
+            <simpara><command>kea-dhcp6.commands</command> - this logger is used
+            to log messages relating to the handling of commands received by the
+            the DHCPv6 server over the command channel.
+            </simpara>
+          </listitem>
+          <listitem>
             <simpara><command>kea-dhcp6.ddns</command> - this logger is used by
             the DHCPv6 server to log messages related to the Client FQDN option
             processing. It also includes log messages related to the relevant
@@ -280,13 +299,6 @@
             used DHCPv6 server deamon to log basic operations.</simpara>
           </listitem>
           <listitem>
-            <simpara><command>kea-dhcp6.alloc-engine</command> - this is the
-            logger used by the lease allocation engine, which is responsible
-            for managing leases in the lease database, i.e. create, modify
-            and remove DHCPv6 leases as a result of processing messages from
-            the clients.</simpara>
-          </listitem>
-          <listitem>
             <simpara><command>kea-dhcp6.dhcpsrv</command> - this is a base
             logger for the libdhcpsrv library.</simpara>
           </listitem>

src/bin/admin/scripts/mysql/.gitignore

@@ -1,2 +1,3 @@
 /upgrade_1.0_to_2.0.sh
 /upgrade_2.0_to_3.0.sh
+/upgrade_3.0_to_4.0.sh

src/bin/admin/scripts/mysql/Makefile.am

@@ -1,6 +1,9 @@
 SUBDIRS = .
 
 sqlscriptsdir = ${datarootdir}/${PACKAGE_NAME}/scripts/mysql
-sqlscripts_DATA = dhcpdb_create.mysql upgrade_1.0_to_2.0.sh upgrade_2.0_to_3.0.sh
+sqlscripts_DATA = dhcpdb_create.mysql
+sqlscripts_DATA += upgrade_1.0_to_2.0.sh
+sqlscripts_DATA += upgrade_2.0_to_3.0.sh
+sqlscripts_DATA += upgrade_3.0_to_4.0.sh
 
-EXTRA_DIST = dhcpdb_create.mysql upgrade_1.0_to_2.0.sh upgrade_2.0_to_3.0.sh
+EXTRA_DIST = ${sqlscripts_DATA}

src/bin/admin/scripts/mysql/dhcpdb_create.mysql

@@ -369,11 +369,12 @@ SELECT
     l.hostname,
     IFNULL(HEX(l.hwaddr), ''),
     IFNULL(l.hwtype, ''),
-    IFNULL(l.hwaddr_source, ''),
+    IFNULL(h.name, ''),
     IFNULL(s.name, '')
 FROM lease6 l
     left outer join lease6_types t on (l.lease_type = t.lease_type)
-    left outer join lease_state s on (l.state = s.state);
+    left outer join lease_state s on (l.state = s.state)
+    left outer join lease_hwaddr_source h on (l.hwaddr_source = h.hwaddr_source);
 END $$
 DELIMITER ;
 

src/bin/admin/scripts/mysql/upgrade_3.0_to_4.0.sh.in

@@ -0,0 +1,123 @@
+#!/bin/sh
+
+# Include utilities. Use installed version if available and
+# use build version if it isn't.
+if [ -e @datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh ]; then
+    . @datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh
+else
+    . @abs_top_builddir@/src/bin/admin/admin-utils.sh
+fi
+
+VERSION=`mysql_version "$@"`
+
+if [ "$VERSION" != "3.0" ]; then
+    printf "This script upgrades 3.0 to 4.0. Reported version is $VERSION. Skipping upgrade.\n"
+    exit 0
+fi
+
+mysql "$@" <<EOF
+# Add state column to the lease4 table.
+ALTER TABLE lease4
+    ADD COLUMN state INT UNSIGNED DEFAULT 0;
+
+# Add state column to the lease6 table.
+ALTER TABLE lease6
+    ADD COLUMN state INT UNSIGNED DEFAULT 0;
+
+# Create indexes for querying leases in a given state and segregated
+# by the expiration time. One of the applications is to retrieve all
+# expired leases. However, these indexes can be also used to retrieve
+# leases in a given state regardless of the expiration time.
+CREATE INDEX lease4_by_state_expire ON lease4 (state, expire);
+CREATE INDEX lease6_by_state_expire ON lease6 (state, expire);
+
+# Create table holding mapping of the lease states to their names.
+# This is not used in queries from the DHCP server but rather in
+# direct queries from the lease database management tools.
+CREATE TABLE IF NOT EXISTS lease_state (
+  state INT UNSIGNED PRIMARY KEY NOT NULL,
+  name VARCHAR(64) NOT NULL);
+
+# Insert currently defined state names.
+INSERT INTO lease_state VALUES (0, "default");
+INSERT INTO lease_state VALUES (1, "declined");
+INSERT INTO lease_state VALUES (2, "expired-reclaimed");
+
+# FUNCTION that returns a result set containing the column names for lease4 dumps
+DROP PROCEDURE IF EXISTS lease4DumpHeader;
+DELIMITER $$
+CREATE PROCEDURE lease4DumpHeader()
+BEGIN
+SELECT 'address,hwaddr,client_id,valid_lifetime,expire,subnet_id,fqdn_fwd,fqdn_rev,hostname,state';
+END  $$
+DELIMITER ;
+
+# FUNCTION that returns a result set containing the data for lease4 dumps
+DROP PROCEDURE IF EXISTS lease4DumpData;
+DELIMITER $$
+CREATE PROCEDURE lease4DumpData()
+BEGIN
+SELECT
+    INET_NTOA(l.address),
+    IFNULL(HEX(l.hwaddr), ''),
+    IFNULL(HEX(l.client_id), ''),
+    l.valid_lifetime,
+    l.expire,
+    l.subnet_id,
+    l.fqdn_fwd,
+    l.fqdn_rev,
+    l.hostname,
+    s.name
+from
+    lease4 l
+    LEFT OUTER JOIN lease_state s on (l.state = s.state);
+END $$
+DELIMITER ;
+
+# FUNCTION that returns a result set containing the column names for lease6 dumps
+DROP PROCEDURE IF EXISTS lease6DumpHeader;
+DELIMITER $$
+CREATE PROCEDURE lease6DumpHeader()
+BEGIN
+SELECT 'address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr,hwtype,hwaddr_source,state';
+END  $$
+DELIMITER ;
+
+# FUNCTION that returns a result set containing the data for lease6 dumps
+DROP PROCEDURE IF EXISTS lease6DumpData;
+DELIMITER $$
+CREATE PROCEDURE lease6DumpData()
+BEGIN
+SELECT
+    l.address,
+    IFNULL(HEX(l.duid), ''),
+    l.valid_lifetime,
+    l.expire,
+    l.subnet_id,
+    l.pref_lifetime,
+    IFNULL(t.name, ''),
+    l.iaid,
+    l.prefix_len,
+    l.fqdn_fwd,
+    l.fqdn_rev,
+    l.hostname,
+    IFNULL(HEX(l.hwaddr), ''),
+    IFNULL(l.hwtype, ''),
+    IFNULL(h.name, ''),
+    IFNULL(s.name, '')
+FROM lease6 l
+    left outer join lease6_types t on (l.lease_type = t.lease_type)
+    left outer join lease_state s on (l.state = s.state)
+    left outer join lease_hwaddr_source h on (l.hwaddr_source = h.hwaddr_source);
+END $$
+DELIMITER ;
+
+# Update the schema version number
+UPDATE schema_version
+SET version = '4', minor = '0';
+# This line concludes database upgrade to version 4.0.
+EOF
+
+RESULT=$?
+
+exit $?

src/bin/admin/tests/data/mysql.lease6_dump_test.reference.csv

@@ -1,4 +1,4 @@
 address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr,hwtype,hwaddr_source,state
-10,3230,30,2015-04-04 01:15:30,40,50,IA_TA,60,70,1,1,one.example.com,3830,90,100,default
-11,,30,2015-05-05 02:30:45,40,50,IA_TA,60,70,1,1,,3830,90,100,declined
-12,3231,30,2015-06-06 11:01:07,40,50,IA_TA,60,70,1,1,three.example.com,3830,90,100,expired-reclaimed
+10,3230,30,2015-04-04 01:15:30,40,50,IA_TA,60,70,1,1,one.example.com,3830,90,,default
+11,,30,2015-05-05 02:30:45,40,50,IA_TA,60,70,1,1,,3830,90,HWADDR_SOURCE_RAW,declined
+12,3231,30,2015-06-06 11:01:07,40,50,IA_TA,60,70,1,1,three.example.com,3830,90,HWADDR_SOURCE_DUID,expired-reclaimed

src/bin/admin/tests/mysql_tests.sh.in

@@ -212,7 +212,7 @@ mysql_upgrade_test() {
 
     assert_str_eq "1.0" ${version} "Expected kea-admin to return %s, returned value was %s"
 
-    # Ok, we have a 1.0 database. Let's upgrade it to 3.0
+    # Ok, we have a 1.0 database. Let's upgrade it to 4.0
     ${keaadmin} lease-upgrade mysql -u $db_user -p $db_password -n $db_name -d @abs_top_srcdir@/src/bin/admin/scripts
     ERRCODE=$?
 
@@ -263,9 +263,38 @@ EOF
     assert_eq 0 $ERRCODE "dhcp6_options table is missing or broken. (returned status code %d, expected %d)"
 
     # Verify that it reports version 3.0.
+    #table: lease_state table added (upgrade 3.0 -> 4.0)
+    mysql -u$db_user -p$db_password $db_name >/dev/null 2>&1 <<EOF
+    SELECT state,name from lease_state;
+EOF
+    ERRCODE=$?
+    assert_eq 0 $ERRCODE "dhcp6_options table is missing or broken. (returned status code %d, expected %d)"
+
+    #table: state column added to lease4 (upgrade 3.0 -> 4.0)
+    mysql -u$db_user -p$db_password $db_name >/dev/null 2>&1 <<EOF
+    SELECT state from lease4;
+EOF
+    ERRCODE=$?
+    assert_eq 0 $ERRCODE "lease4 is missing state column. (returned status code %d, expected %d)"
+
+    #table: state column added to lease6 (upgrade 3.0 -> 4.0)
+    mysql -u$db_user -p$db_password $db_name >/dev/null 2>&1 <<EOF
+    SELECT state from lease6;
+EOF
+    ERRCODE=$?
+    assert_eq 0 $ERRCODE "lease6 is missing state column. (returned status code %d, expected %d)"
+
+    #table: stored procedures for lease dumps added (upgrade 3.0 -> 4.0)
+    mysql -u$db_user -p$db_password $db_name >/dev/null 2>&1 <<EOF
+    call lease4DumpHeader(); call lease4DumpData(); call lease6DumpHeader(); call lease6DumpHeader();
+EOF
+    ERRCODE=$?
+    assert_eq 0 $ERRCODE "lease dump stored procedures are missing or broken. (returned status code %d, expected %d)"
+
+    # Verify that it reports version 4.0.
     version=$(${keaadmin} lease-version mysql -u $db_user -p $db_password -n $db_name)
 
-    assert_str_eq "3.0" ${version} "Expected kea-admin to return %s, returned value was %s"
+    assert_str_eq "4.0" ${version} "Expected kea-admin to return %s, returned value was %s"
 
     # Let's wipe the whole database
     mysql_wipe
@@ -366,9 +395,9 @@ mysql_lease6_dump_test() {
 
     # Insert the reference record
     insert_sql="\
-insert into lease6 values(10,20,30,\"2015-04-04 01:15:30\",40,50,1,60,70,1,1,\"one.example.com\",80,90,100, 0);\
-insert into lease6 values(11,NULL,30,\"2015-05-05 02:30:45\",40,50,1,60,70,1,1,\"\",80,90,100, 1);\
-insert into lease6 values(12,21,30,\"2015-06-06 11:01:07\",40,50,1,60,70,1,1,\"three.example.com\",80,90,100, 2);"
+insert into lease6 values(10,20,30,\"2015-04-04 01:15:30\",40,50,1,60,70,1,1,\"one.example.com\",80,90,0,0);\
+insert into lease6 values(11,NULL,30,\"2015-05-05 02:30:45\",40,50,1,60,70,1,1,\"\",80,90,1,1);\
+insert into lease6 values(12,21,30,\"2015-06-06 11:01:07\",40,50,1,60,70,1,1,\"three.example.com\",80,90,4,2);"
 
     mysql_execute "$insert_sql"
     ERRCODE=$?

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -135,20 +135,6 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
 
     ConstElementPtr answer = configureDhcp4Server(*srv, config);
 
-    // Start worker thread if there are any timers installed. Note that
-    // we also start worker thread when the reconfiguration failed, because
-    // in that case we continue using an old configuration and the server
-    // should still run installed timers.
-    if (TimerMgr::instance()->timersCount() > 0) {
-        try {
-            TimerMgr::instance()->startThread();
-        } catch (const std::exception& ex) {
-            err << "Unable to start worker thread running timers: "
-                << ex.what() << ".";
-            return (isc::config::createAnswer(1, err.str()));
-        }
-    }
-
     // Check that configuration was successful. If not, do not reopen sockets
     // and don't bother with DDNS stuff.
     try {
@@ -181,6 +167,32 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
     CfgMgr::instance().getStagingCfg()->getCfgIface()->
         openSockets(AF_INET, srv->getPort(), getInstance()->useBroadcast());
 
+    // Install the timers for handling leases reclamation.
+    try {
+        CfgMgr::instance().getStagingCfg()->getCfgExpiration()->
+            setupTimers(&ControlledDhcpv4Srv::reclaimExpiredLeases,
+                        &ControlledDhcpv4Srv::deleteExpiredReclaimedLeases,
+                        server_);
+
+    } catch (const std::exception& ex) {
+        err << "unable to setup timers for periodically running the"
+            " reclamation of the expired leases: "
+            << ex.what() << ".";
+        return (isc::config::createAnswer(1, err.str()));
+    }
+
+    // Start worker thread if there are any timers installed.
+    if (TimerMgr::instance()->timersCount() > 0) {
+        try {
+            TimerMgr::instance()->startThread();
+        } catch (const std::exception& ex) {
+            err << "Unable to start worker thread running timers: "
+                << ex.what() << ".";
+            return (isc::config::createAnswer(1, err.str()));
+        }
+    }
+
+
 
     return (answer);
 }
@@ -229,8 +241,10 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() {
     try {
         cleanup();
 
-        // Stop worker thread running timers, if it is running.
+        // Stop worker thread running timers, if it is running. Then
+        // unregister any timers.
         timer_mgr_->stopThread();
+        timer_mgr_->unregisterTimers();
 
         // Close the command socket (if it exists).
         CommandMgr::instance().closeCommandSocket();
@@ -262,5 +276,24 @@ void ControlledDhcpv4Srv::sessionReader(void) {
     }
 }
 
+void
+ControlledDhcpv4Srv::reclaimExpiredLeases(const size_t max_leases,
+                                          const uint16_t timeout,
+                                          const bool remove_lease,
+                                          const uint16_t max_unwarned_cycles) {
+    server_->alloc_engine_->reclaimExpiredLeases4(max_leases, timeout,
+                                                  remove_lease,
+                                                  max_unwarned_cycles);
+    // We're using the ONE_SHOT timer so there is a need to re-schedule it.
+    TimerMgr::instance()->setup(CfgExpiration::RECLAIM_EXPIRED_TIMER_NAME);
+}
+
+void
+ControlledDhcpv4Srv::deleteExpiredReclaimedLeases(const uint32_t secs) {
+    server_->alloc_engine_->deleteExpiredReclaimedLeases4(secs);
+    // We're using the ONE_SHOT timer so there is a need to re-schedule it.
+    TimerMgr::instance()->setup(CfgExpiration::FLUSH_RECLAIMED_TIMER_NAME);
+}
+
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -150,6 +150,37 @@ private:
     commandConfigReloadHandler(const std::string& command,
                                isc::data::ConstElementPtr args);
 
+
+    /// @brief Reclaims expired IPv4 leases and reschedules timer.
+    ///
+    /// This is a wrapper method for @c AllocEngine::reclaimExpiredLeases4.
+    /// It reschedules the timer for leases reclamation upon completion of
+    /// this method.
+    ///
+    /// @param max_leases Maximum number of leases to be reclaimed.
+    /// @param timeout Maximum amount of time that the reclaimation routine
+    /// may be processing expired leases, expressed in milliseconds.
+    /// @param remove_lease A boolean value indicating if the lease should
+    /// be removed when it is reclaimed (if true) or it should be left in the
+    /// database in the "expired-reclaimed" state (if false).
+    /// @param max_unwarned_cycles A number of consecutive processing cycles
+    /// of expired leases, after which the system issues a warning if there
+    /// are still expired leases in the database. If this value is 0, the
+    /// warning is never issued.
+    void reclaimExpiredLeases(const size_t max_leases, const uint16_t timeout,
+                              const bool remove_lease,
+                              const uint16_t max_unwarned_cycles);
+
+    /// @brief Deletes reclaimed leases and reschedules the timer.
+    ///
+    /// This is a wrapper method for @c AllocEngine::deleteExpiredReclaimed4.
+    /// It reschedules the timer for leases reclamation upon completion of
+    /// this method.
+    ///
+    /// @param secs Minimum number of seconds after which a lease can be
+    /// deleted.
+    void deleteExpiredReclaimedLeases(const uint32_t secs);
+
     /// @brief Static pointer to the sole instance of the DHCP server.
     ///
     /// This is required for config and command handlers to gain access to

src/bin/dhcp4/dhcp4_hooks.dox

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

src/bin/dhcp4/dhcp4_messages.mes

@@ -247,6 +247,13 @@ 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_DECLINE_SKIP Decline4 hook callouts set status to DROP, ignoring packet.
+This message indicates that the server received DHCPDECLINE message, it was verified
+to be correct and matching server's lease information. The server called hooks
+for decline4 hook point and one of the callouts set next step status to DROP.
+The server will now abort processing of the packet as if it was never
+received. The lease will continue to be assigned to this client.
+
 % DHCP4_HOOK_LEASE4_RELEASE_SKIP %1: 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

src/bin/dhcp4/dhcp4_srv.cc

@@ -80,6 +80,7 @@ struct Dhcp4Hooks {
     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
+    int hook_index_lease4_decline_; ///< index for "lease4_decline" hook point
 
     /// Constructor that registers hook points for DHCPv4 engine
     Dhcp4Hooks() {
@@ -89,6 +90,7 @@ struct Dhcp4Hooks {
         hook_index_pkt4_send_      = HooksManager::registerHook("pkt4_send");
         hook_index_lease4_release_ = HooksManager::registerHook("lease4_release");
         hook_index_buffer4_send_   = HooksManager::registerHook("buffer4_send");
+        hook_index_lease4_decline_ = HooksManager::registerHook("lease4_decline");
     }
 };
 
@@ -271,6 +273,10 @@ Dhcpv4Srv::~Dhcpv4Srv() {
     }
 
     IfaceMgr::instance().closeSockets();
+
+    // The lease manager was instantiated during DHCPv4Srv configuration,
+    // so we should clean up after ourselves.
+    LeaseMgrFactory::destroy();
 }
 
 void
@@ -292,6 +298,29 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query) const {
     selector.client_classes_ = query->classes_;
     selector.iface_name_ = query->getIface();
 
+    // If the link-selection sub-option is present, extract its value.
+    // "The link-selection sub-option is used by any DHCP relay agent
+    // that desires to specify a subnet/link for a DHCP client request
+    // that it is relaying but needs the subnet/link specification to
+    // be different from the IP address the DHCP server should use
+    // when communicating with the relay agent." (RFC 3257)
+    OptionPtr rai = query->getOption(DHO_DHCP_AGENT_OPTIONS);
+    if (rai) {
+        OptionCustomPtr rai_custom =
+            boost::dynamic_pointer_cast<OptionCustom>(rai);
+        if (rai_custom) {
+            OptionPtr link_select =
+                rai_custom->getOption(RAI_OPTION_LINK_SELECTION);
+            if (link_select) {
+                OptionBuffer link_select_buf = link_select->getData();
+                if (link_select_buf.size() == sizeof(uint32_t)) {
+                    selector.option_select_ =
+                        IOAddress::fromBytes(AF_INET, &link_select_buf[0]);
+                }
+            }
+        }
+    }
+
     CfgMgr& cfgmgr = CfgMgr::instance();
     subnet = cfgmgr.getCurrentCfg()->getCfgSubnets4()->selectSubnet(selector);
 
@@ -316,13 +345,15 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query) const {
         // Callouts decided to skip this step. This means that no subnet
         // will be selected. Packet processing will continue, but it will
         // be severely limited (i.e. only global options will be assigned)
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                       DHCP4_HOOK_SUBNET4_SELECT_SKIP)
                 .arg(query->getLabel());
             return (Subnet4Ptr());
         }
 
+        /// @todo: Add support for DROP status
+
         // Use whatever subnet was specified by the callout
         callout_handle->getArgument("subnet4", subnet);
     }
@@ -464,7 +495,7 @@ Dhcpv4Srv::run() {
             // processing step would to parse the packet, so skip at this
             // stage means that callouts did the parsing already, so server
             // should skip parsing.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP4_DETAIL, DHCP4_HOOK_BUFFER_RCVD_SKIP)
                     .arg(query->getRemoteAddr().toText())
                     .arg(query->getLocalAddr().toText())
@@ -473,6 +504,8 @@ Dhcpv4Srv::run() {
             }
 
             callout_handle->getArgument("query4", query);
+
+            /// @todo: add support for DROP status
         }
 
         // Unpack the packet information unless the buffer4_receive callouts
@@ -550,12 +583,14 @@ Dhcpv4Srv::run() {
             // Callouts decided to skip the next processing step. The next
             // processing step would to process the packet, so skip at this
             // stage means drop.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_RCVD_SKIP)
                     .arg(query->getLabel());
                 continue;
             }
 
+            /// @todo: Add support for DROP status
+
             callout_handle->getArgument("query4", query);
         }
 
@@ -625,7 +660,7 @@ Dhcpv4Srv::run() {
             callout_handle->deleteAllArguments();
 
             // Clear skip flag if it was set in previous callouts
-            callout_handle->setSkip(false);
+            callout_handle->setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
 
             // Set our response
             callout_handle->setArgument("response4", rsp);
@@ -637,11 +672,13 @@ Dhcpv4Srv::run() {
             // Callouts decided to skip the next processing step. The next
             // processing step would to send the packet, so skip at this
             // stage means "drop response".
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_SKIP)
                     .arg(query->getLabel());
                 skip_pack = true;
             }
+
+            /// @todo: Add support for DROP status
         }
 
         if (!skip_pack) {
@@ -677,13 +714,15 @@ Dhcpv4Srv::run() {
                 // Callouts decided to skip the next processing step. The next
                 // processing step would to parse the packet, so skip at this
                 // stage means drop.
-                if (callout_handle->getSkip()) {
+                if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                     LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                               DHCP4_HOOK_BUFFER_SEND_SKIP)
                         .arg(rsp->getLabel());
                     continue;
                 }
 
+                /// @todo: Add support for DROP status.
+
                 callout_handle->getArgument("response4", rsp);
             }
 
@@ -1769,12 +1808,14 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
             // Callouts decided to skip the next processing step. The next
             // processing step would to send the packet, so skip at this
             // stage means "drop response".
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 skip = true;
                 LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                           DHCP4_HOOK_LEASE4_RELEASE_SKIP)
                     .arg(release->getLabel());
             }
+
+            /// @todo add support for DROP status
         }
 
         // Callout didn't indicate to skip the release process. Let's release
@@ -1883,11 +1924,37 @@ Dhcpv4Srv::processDecline(Pkt4Ptr& decline) {
 
     // Ok, all is good. The client is reporting its own address. Let's
     // process it.
-    declineLease(lease, decline->getLabel());
+    declineLease(lease, decline);
 }
 
 void
-Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const std::string& descr) {
+Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline) {
+
+    // Let's check if there are hooks installed for decline4 hook point.
+    // If they are, let's pass the lease and client's packet. If the hook
+    // sets status to drop, we reject this Decline.
+    if (HooksManager::calloutsPresent(Hooks.hook_index_lease4_decline_)) {
+        CalloutHandlePtr callout_handle = getCalloutHandle(decline);
+
+        // Delete previously set arguments
+        callout_handle->deleteAllArguments();
+
+        // Pass incoming Decline and the lease to be declined.
+        callout_handle->setArgument("lease4", lease);
+        callout_handle->setArgument("query4", decline);
+
+        // Call callouts
+        HooksManager::callCallouts(Hooks.hook_index_lease4_decline_,
+                                   *callout_handle);
+
+        // Check if callouts decided to drop the packet. If any of them did,
+        // we will drop the packet.
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
+            LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_DECLINE_SKIP)
+                .arg(decline->getLabel()).arg(lease->addr_.toText());
+            return;
+        }
+    }
 
     // Clean up DDNS, if needed.
     if (CfgMgr::instance().ddnsEnabled()) {
@@ -1926,7 +1993,7 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const std::string& descr) {
     LeaseMgrFactory::instance().updateLease4(lease);
 
     LOG_INFO(dhcp4_logger, DHCP4_DECLINE_LEASE).arg(lease->addr_.toText())
-        .arg(descr).arg(lease->valid_lft_);
+        .arg(decline->getLabel()).arg(lease->valid_lft_);
 }
 
 Pkt4Ptr

src/bin/dhcp4/dhcp4_srv.h

@@ -546,12 +546,12 @@ private:
     /// - disassociate the client information
     /// - update lease in the database (switch to DECLINED state)
     /// - increase necessary statistics
-    /// - call appropriate hook (@todo)
+    /// - call lease4_decline hook
     ///
     /// @param lease lease to be declined
-    /// @param descr textual description of the client (will be used for logging)
+    /// @param decline client's message
     void
-    declineLease(const Lease4Ptr& lease, const std::string& descr);
+    declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline);
 
 protected:
 

src/bin/dhcp4/tests/Makefile.am

@@ -85,6 +85,7 @@ dhcp4_unittests_SOURCES += config_parser_unittest.cc
 dhcp4_unittests_SOURCES += fqdn_unittest.cc
 dhcp4_unittests_SOURCES += marker_file.cc
 dhcp4_unittests_SOURCES += dhcp4_client.cc dhcp4_client.h
+dhcp4_unittests_SOURCES += hooks_unittest.cc
 dhcp4_unittests_SOURCES += inform_unittest.cc
 dhcp4_unittests_SOURCES += dora_unittest.cc
 dhcp4_unittests_SOURCES += release_unittest.cc

src/bin/dhcp4/tests/decline_unittest.cc

@@ -34,7 +34,6 @@ using namespace isc::dhcp::test;
 using namespace isc::stats;
 
 namespace {
-
 /// @brief Set of JSON configurations used throughout the Decline tests.
 ///
 /// - Configuration 0:
@@ -63,56 +62,14 @@ const char* DECLINE_CONFIGS[] = {
     "}"
 };
 
-/// @brief Test fixture class for testing DHCPDECLINE message handling.
-///
-/// @todo This class is very similar to ReleaseTest. Maybe we could
-/// merge those two classes one day and use derived classes?
-class DeclineTest : public Dhcpv4SrvTest {
-public:
-
-    enum ExpectedResult {
-        SHOULD_PASS, // pass = accept decline, move lease to declined state.
-        SHOULD_FAIL  // fail = reject the decline
-    };
-
-    /// @brief Constructor.
-    ///
-    /// Sets up fake interfaces.
-    DeclineTest()
-        : Dhcpv4SrvTest(),
-          iface_mgr_test_config_(true) {
-        IfaceMgr::instance().openSockets4();
-    }
-
-    /// @brief Performs 4-way exchange to obtain new lease.
-    ///
-    /// This is used as a preparatory step for Decline operation.
-    ///
-    /// @param client Client to be used to obtain a lease.
-    void acquireLease(Dhcp4Client& client);
-
-    /// @brief Tests if the acquired lease is or is not declined.
-    ///
-    /// @param hw_address_1 HW Address to be used to acquire the lease.
-    /// @param client_id_1 Client id to be used to acquire the lease.
-    /// @param hw_address_2 HW Address to be used to decline the lease.
-    /// @param client_id_2 Client id to be used to decline the lease.
-    /// @param expected_result SHOULD_PASS if the lease is expected to
-    /// be successfully declined, or SHOULD_FAIL if the lease is expected
-    /// to not be declined.
-    void acquireAndDecline(const std::string& hw_address_1,
-                           const std::string& client_id_1,
-                           const std::string& hw_address_2,
-                           const std::string& client_id_2,
-                           ExpectedResult expected_result);
-
-    /// @brief Interface Manager's fake configuration control.
-    IfaceMgrTestConfig iface_mgr_test_config_;
-
 };
 
+namespace isc {
+namespace dhcp {
+namespace test {
+
 void
-DeclineTest::acquireLease(Dhcp4Client& client) {
+Dhcpv4SrvTest::acquireLease(Dhcp4Client& client) {
     // Perform 4-way exchange with the server but to not request any
     // specific address in the DHCPDISCOVER message.
     ASSERT_NO_THROW(client.doDORA());
@@ -132,11 +89,12 @@ DeclineTest::acquireLease(Dhcp4Client& client) {
 }
 
 void
-DeclineTest::acquireAndDecline(const std::string& hw_address_1,
-                               const std::string& client_id_1,
-                               const std::string& hw_address_2,
-                               const std::string& client_id_2,
-                               ExpectedResult expected_result) {
+Dhcpv4SrvTest::acquireAndDecline(Dhcp4Client& client,
+                                 const std::string& hw_address_1,
+                                 const std::string& client_id_1,
+                                 const std::string& hw_address_2,
+                                 const std::string& client_id_2,
+                                 ExpectedResult expected_result) {
 
     // Set this global statistic explicitly to zero.
     isc::stats::StatsMgr::instance().setValue("declined-addresses",
@@ -144,7 +102,7 @@ DeclineTest::acquireAndDecline(const std::string& hw_address_1,
 
     // Ok, do the normal lease aquisition.
     CfgMgr::instance().clear();
-    Dhcp4Client client(Dhcp4Client::SELECTING);
+
     // Configure DHCP server.
     configure(DECLINE_CONFIGS[0], *client.getServer());
     // Explicitly set the client id.
@@ -218,9 +176,37 @@ DeclineTest::acquireAndDecline(const std::string& hw_address_1,
     }
 }
 
+}; // end of isc::dhcp::test namespace
+}; // end of isc::dhcp namespace
+}; // end of isc namespace
+
+namespace {
+
+/// @brief Test fixture class for testing DHCPDECLINE message handling.
+///
+/// @todo This class is very similar to ReleaseTest. Maybe we could
+/// merge those two classes one day and use derived classes?
+class DeclineTest : public Dhcpv4SrvTest {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Sets up fake interfaces.
+    DeclineTest()
+        : Dhcpv4SrvTest(),
+          iface_mgr_test_config_(true) {
+        IfaceMgr::instance().openSockets4();
+    }
+
+    /// @brief Interface Manager's fake configuration control.
+    IfaceMgrTestConfig iface_mgr_test_config_;
+
+};
+
 // This test checks that the client can acquire and decline the lease.
 TEST_F(DeclineTest, declineNoIdentifierChange) {
-    acquireAndDecline("01:02:03:04:05:06", "12:14",
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    acquireAndDecline(client, "01:02:03:04:05:06", "12:14",
                       "01:02:03:04:05:06", "12:14",
                       SHOULD_PASS);
 }
@@ -231,7 +217,8 @@ TEST_F(DeclineTest, declineNoIdentifierChange) {
 //   client identifier.
 // - The server successfully declines the lease.
 TEST_F(DeclineTest, declineHWAddressOnly) {
-    acquireAndDecline("01:02:03:04:05:06", "",
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    acquireAndDecline(client, "01:02:03:04:05:06", "",
                       "01:02:03:04:05:06", "",
                       SHOULD_PASS);
 }
@@ -242,7 +229,8 @@ TEST_F(DeclineTest, declineHWAddressOnly) {
 //   client identifier.
 // - The server successfully declines the lease.
 TEST_F(DeclineTest, declineNoClientId) {
-    acquireAndDecline("01:02:03:04:05:06", "12:14",
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    acquireAndDecline(client, "01:02:03:04:05:06", "12:14",
                       "01:02:03:04:05:06", "",
                       SHOULD_PASS);
 }
@@ -254,7 +242,8 @@ TEST_F(DeclineTest, declineNoClientId) {
 // - The server identifies the lease using HW address and declines
 //   this lease.
 TEST_F(DeclineTest, declineNoClientId2) {
-    acquireAndDecline("01:02:03:04:05:06", "",
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    acquireAndDecline(client, "01:02:03:04:05:06", "",
                       "01:02:03:04:05:06", "12:14",
                       SHOULD_PASS);
 }
@@ -265,7 +254,8 @@ TEST_F(DeclineTest, declineNoClientId2) {
 //   client identifier.
 // - The server should not remove the lease.
 TEST_F(DeclineTest, declineNonMatchingClientId) {
-    acquireAndDecline("01:02:03:04:05:06", "12:14",
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    acquireAndDecline(client, "01:02:03:04:05:06", "12:14",
                       "01:02:03:04:05:06", "12:15:16",
                       SHOULD_FAIL);
 }
@@ -277,7 +267,8 @@ TEST_F(DeclineTest, declineNonMatchingClientId) {
 // - The server uses client identifier to find the client's lease and
 //   declines it.
 TEST_F(DeclineTest, declineNonMatchingHWAddress) {
-    acquireAndDecline("01:02:03:04:05:06", "12:14",
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    acquireAndDecline(client, "01:02:03:04:05:06", "12:14",
                       "06:06:06:06:06:06", "12:14",
                       SHOULD_PASS);
 }
@@ -310,4 +301,4 @@ TEST_F(DeclineTest, declineNonMatchingIPAddress) {
     EXPECT_EQ(Lease::STATE_DEFAULT, lease->state_);
 }
 
-} // end of anonymous namespace
+}; // end of anonymous namespace

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -617,7 +617,7 @@ Dhcpv4SrvTest::pretendReceivingPkt(NakedDhcpv4Srv& srv, const std::string& confi
     configure(config);
 
     // Let's just use one of the actual captured packets that we have.
-    Pkt4Ptr pkt = isc::test::PktCaptures::captureRelayedDiscover();
+    Pkt4Ptr pkt = PktCaptures::captureRelayedDiscover();
 
     // We just need to tweak it a it, to pretend that it's type is as desired.
     // Note that when receiving a packet, its on-wire form is stored in data_

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -88,6 +88,11 @@ public:
 
 typedef boost::shared_ptr<PktFilterTest> PktFilterTestPtr;
 
+/// Forward definition for Dhcp4Client defined in dhcp4_client.h
+/// dhcp4_client.h includes dhcp_test_utils.h (this file), so to avoid
+/// circular dependencies, we need a forward class declaration.
+class Dhcp4Client;
+
 /// @brief "Naked" DHCPv4 server, exposes internal fields
 class NakedDhcpv4Srv: public Dhcpv4Srv {
 public:
@@ -208,9 +213,19 @@ public:
     using Dhcpv4Srv::alloc_engine_;
 };
 
+// We need to pass one reference to the Dhcp4Client, which is defined in
+// dhcp4_client.h. That header includes this file. To avoid circular
+// dependencies, we use forward declaration here.
+class Dhcp4Client;
+
 class Dhcpv4SrvTest : public ::testing::Test {
 public:
 
+    enum ExpectedResult {
+        SHOULD_PASS, // pass = accept decline, move lease to declined state.
+        SHOULD_FAIL  // fail = reject the decline
+    };
+
     /// @brief Constructor
     ///
     /// Initializes common objects used in many tests.
@@ -408,6 +423,30 @@ public:
     /// @brief Create @c Dhcpv4Exchange from client's query.
     Dhcpv4Exchange createExchange(const Pkt4Ptr& query);
 
+    /// @brief Performs 4-way exchange to obtain new lease.
+    ///
+    /// This is used as a preparatory step for Decline operation.
+    ///
+    /// @param client Client to be used to obtain a lease.
+    void acquireLease(Dhcp4Client& client);
+
+    /// @brief Tests if the acquired lease is or is not declined.
+    ///
+    /// @param client Dhcp4Client instance
+    /// @param hw_address_1 HW Address to be used to acquire the lease.
+    /// @param client_id_1 Client id to be used to acquire the lease.
+    /// @param hw_address_2 HW Address to be used to decline the lease.
+    /// @param client_id_2 Client id to be used to decline the lease.
+    /// @param expected_result SHOULD_PASS if the lease is expected to
+    /// be successfully declined, or SHOULD_FAIL if the lease is expected
+    /// to not be declined.
+    void acquireAndDecline(Dhcp4Client& client,
+                           const std::string& hw_address_1,
+                           const std::string& client_id_1,
+                           const std::string& hw_address_2,
+                           const std::string& client_id_2,
+                           ExpectedResult expected_result);
+
     /// @brief This function cleans up after the test.
     virtual void TearDown();
 

src/bin/dhcp4/tests/kea_controller_unittest.cc

@@ -14,11 +14,17 @@
 
 #include <config.h>
 
+#include <asiolink/io_address.h>
 #include <cc/command_interpreter.h>
 #include <dhcp/dhcp4.h>
+#include <dhcp/hwaddr.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/lease.h>
+#include <dhcpsrv/lease_mgr_factory.h>
 #include <log/logger_support.h>
+#include <util/stopwatch.h>
 
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
@@ -58,6 +64,7 @@ public:
     }
 
     ~JSONFileBackendTest() {
+        LeaseMgrFactory::destroy();
         isc::log::setDefaultLoggingOutput();
         static_cast<void>(remove(TEST_FILE));
     };
@@ -77,6 +84,21 @@ public:
         out.close();
     }
 
+    /// @brief Runs timers for specified time.
+    ///
+    /// Internally, this method calls @c IfaceMgr::receive4 to run the
+    /// callbacks for the installed timers.
+    ///
+    /// @param timeout_ms Amount of time after which the method returns.
+    void runTimersWithTimeout(const long timeout_ms) {
+        isc::util::Stopwatch stopwatch;
+        while (stopwatch.getTotalMilliseconds() < timeout_ms) {
+            // Block for up to one millisecond waiting for the timers'
+            // callbacks to be executed.
+            IfaceMgr::instancePtr()->receive4(0, 1000);
+        }
+    }
+
     /// Name of a config file used during tests
     static const char* TEST_FILE;
 };
@@ -303,4 +325,82 @@ TEST_F(JSONFileBackendTest, DISABLED_loadAllConfigs) {
     }
 }
 
+// This test verifies that the DHCP server installs the timers for reclaiming
+// and flushing expired leases.
+TEST_F(JSONFileBackendTest, timers) {
+    // This is a basic configuration which enables timers for reclaiming
+    // expired leases and flushing them after 500 seconds since they expire.
+    // Both timers run at 1 second intervals.
+    string config =
+        "{ \"Dhcp4\": {"
+        "\"interfaces-config\": {"
+        "    \"interfaces\": [ ]"
+        "},"
+        "\"lease-database\": {"
+        "     \"type\": \"memfile\","
+        "     \"persist\": false"
+        "},"
+        "\"expired-leases-processing\": {"
+        "     \"reclaim-timer-wait-time\": 1,"
+        "     \"hold-reclaimed-time\": 500,"
+        "     \"flush-reclaimed-timer-wait-time\": 1"
+        "},"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, \n"
+        "\"subnet4\": [ ],"
+        "\"valid-lifetime\": 4000 }"
+        "}";
+    writeFile(config);
+
+    // Create an instance of the server and intialize it.
+    boost::scoped_ptr<ControlledDhcpv4Srv> srv;
+    ASSERT_NO_THROW(srv.reset(new ControlledDhcpv4Srv(0)));
+    ASSERT_NO_THROW(srv->init(TEST_FILE));
+
+    // Create an expired lease. The lease is expired by 40 seconds ago
+    // (valid lifetime = 60, cltt = now - 100). The lease will be reclaimed
+    // but shouldn't be flushed in the database because the reclaimed are
+    // held in the database 500 seconds after reclamation, according to the
+    // current configuration.
+    HWAddrPtr hwaddr_expired(new HWAddr(HWAddr::fromText("00:01:02:03:04:05")));
+    Lease4Ptr lease_expired(new Lease4(IOAddress("10.0.0.1"), hwaddr_expired,
+                                       ClientIdPtr(), 60, 10, 20,
+                                       time(NULL) - 100, SubnetID(1)));
+
+    // Create expired-reclaimed lease. The lease has expired 1000 - 60 seconds
+    // ago. It should be removed from the lease database when the "flush" timer
+    // goes off.
+    HWAddrPtr hwaddr_reclaimed(new HWAddr(HWAddr::fromText("01:02:03:04:05:06")));
+    Lease4Ptr lease_reclaimed(new Lease4(IOAddress("10.0.0.2"), hwaddr_reclaimed,
+                                         ClientIdPtr(), 60, 10, 20,
+                                         time(NULL) - 1000, SubnetID(1)));
+    lease_reclaimed->state_ = Lease4::STATE_EXPIRED_RECLAIMED;
+
+    // Add leases to the database.
+    LeaseMgr& lease_mgr = LeaseMgrFactory().instance();
+    ASSERT_NO_THROW(lease_mgr.addLease(lease_expired));
+    ASSERT_NO_THROW(lease_mgr.addLease(lease_reclaimed));
+
+    // Make sure they have been added.
+    ASSERT_TRUE(lease_mgr.getLease4(IOAddress("10.0.0.1")));
+    ASSERT_TRUE(lease_mgr.getLease4(IOAddress("10.0.0.2")));
+
+    // Poll the timers for a while to make sure that each of them is executed
+    // at least once.
+    ASSERT_NO_THROW(runTimersWithTimeout(5000));
+
+    // Verify that the leases in the database have been processed as expected.
+
+    // First lease should be reclaimed, but not removed.
+    ASSERT_NO_THROW(lease_expired = lease_mgr.getLease4(IOAddress("10.0.0.1")));
+    ASSERT_TRUE(lease_expired);
+    EXPECT_TRUE(lease_expired->stateExpiredReclaimed());
+
+    // Second lease should have been removed.
+    ASSERT_NO_THROW(
+        lease_reclaimed = lease_mgr.getLease4(IOAddress("10.0.0.2"));
+    );
+    EXPECT_FALSE(lease_reclaimed);
+}
+
 } // End of anonymous namespace

src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -131,21 +131,6 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
 
     ConstElementPtr answer = configureDhcp6Server(*srv, config);
 
-    // Start worker thread if there are any timers installed. Note that
-    // we also start worker thread when the reconfiguration failed, because
-    // in that case we continue using an old configuration and the server
-    // should still run installed timers.
-    if (TimerMgr::instance()->timersCount() > 0) {
-        try {
-            TimerMgr::instance()->startThread();
-        } catch (const std::exception& ex) {
-            std::ostringstream err;
-            err << "Unable to start worker thread running timers: "
-                << ex.what() << ".";
-            return (isc::config::createAnswer(1, err.str()));
-        }
-    }
-
     // Check that configuration was successful. If not, do not reopen sockets
     // and don't bother with DDNS stuff.
     try {
@@ -178,6 +163,33 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
     // of the interfaces.
     CfgMgr::instance().getStagingCfg()->getCfgIface()->openSockets(AF_INET6, srv->getPort());
 
+    // Install the timers for handling leases reclamation.
+    try {
+        CfgMgr::instance().getStagingCfg()->getCfgExpiration()->
+            setupTimers(&ControlledDhcpv6Srv::reclaimExpiredLeases,
+                        &ControlledDhcpv6Srv::deleteExpiredReclaimedLeases,
+                        server_);
+
+    } catch (const std::exception& ex) {
+        std::ostringstream err;
+        err << "unable to setup timers for periodically running the"
+            " reclamation of the expired leases: "
+            << ex.what() << ".";
+        return (isc::config::createAnswer(1, err.str()));
+    }
+
+    // Start worker thread if there are any timers installed.
+    if (TimerMgr::instance()->timersCount() > 0) {
+        try {
+            TimerMgr::instance()->startThread();
+        } catch (const std::exception& ex) {
+            std::ostringstream err;
+            err << "Unable to start worker thread running timers: "
+                << ex.what() << ".";
+            return (isc::config::createAnswer(1, err.str()));
+        }
+    }
+
     return (answer);
 }
 
@@ -225,8 +237,10 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() {
     try {
         cleanup();
 
-        // Stop worker thread running timers, if it is running.
+        // Stop worker thread running timers, if it is running. Then
+        // unregister any timers.
         timer_mgr_->stopThread();
+        timer_mgr_->unregisterTimers();
 
         // Close the command socket (if it exists).
         CommandMgr::instance().closeCommandSocket();
@@ -258,5 +272,25 @@ void ControlledDhcpv6Srv::sessionReader(void) {
     }
 }
 
+void
+ControlledDhcpv6Srv::reclaimExpiredLeases(const size_t max_leases,
+                                          const uint16_t timeout,
+                                          const bool remove_lease,
+                                          const uint16_t max_unwarned_cycles) {
+    server_->alloc_engine_->reclaimExpiredLeases6(max_leases, timeout,
+                                                  remove_lease,
+                                                  max_unwarned_cycles);
+    // We're using the ONE_SHOT timer so there is a need to re-schedule it.
+    TimerMgr::instance()->setup(CfgExpiration::RECLAIM_EXPIRED_TIMER_NAME);
+}
+
+void
+ControlledDhcpv6Srv::deleteExpiredReclaimedLeases(const uint32_t secs) {
+    server_->alloc_engine_->deleteExpiredReclaimedLeases6(secs);
+    // We're using the ONE_SHOT timer so there is a need to re-schedule it.
+    TimerMgr::instance()->setup(CfgExpiration::FLUSH_RECLAIMED_TIMER_NAME);
+}
+
+
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -150,6 +150,37 @@ private:
     commandConfigReloadHandler(const std::string& command,
                                isc::data::ConstElementPtr args);
 
+    /// @brief Reclaims expired IPv6 leases and reschedules timer.
+    ///
+    /// This is a wrapper method for @c AllocEngine::reclaimExpiredLeases6.
+    /// It reschedules the timer for leases reclamation upon completion of
+    /// this method.
+    ///
+    /// @param max_leases Maximum number of leases to be reclaimed.
+    /// @param timeout Maximum amount of time that the reclaimation routine
+    /// may be processing expired leases, expressed in milliseconds.
+    /// @param remove_lease A boolean value indicating if the lease should
+    /// be removed when it is reclaimed (if true) or it should be left in the
+    /// database in the "expired-reclaimed" state (if false).
+    /// @param max_unwarned_cycles A number of consecutive processing cycles
+    /// of expired leases, after which the system issues a warning if there
+    /// are still expired leases in the database. If this value is 0, the
+    /// warning is never issued.
+    void reclaimExpiredLeases(const size_t max_leases, const uint16_t timeout,
+                              const bool remove_lease,
+                              const uint16_t max_unwarned_cycles);
+
+
+    /// @brief Deletes reclaimed leases and reschedules the timer.
+    ///
+    /// This is a wrapper method for @c AllocEngine::deleteExpiredReclaimed6.
+    /// It reschedules the timer for leases reclamation upon completion of
+    /// this method.
+    ///
+    /// @param secs Minimum number of seconds after which a lease can be
+    /// deleted.
+    void deleteExpiredReclaimedLeases(const uint32_t secs);
+
     /// @brief Static pointer to the sole instance of the DHCP server.
     ///
     /// This is required for config and command handlers to gain access to

src/bin/dhcp6/dhcp6_hooks.dox

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

src/bin/dhcp6/dhcp6_messages.mes

@@ -292,6 +292,21 @@ or released leases), but the response will not be send to the client.
 The argument includes the client and transaction identification
 information.
 
+% DHCP6_HOOK_DECLINE_SKIP During Decline processing (client=%1, interface=%2, addr=%3) hook callout set status to SKIP, skipping decline.
+This message indicates that the server received DECLINE message, it was verified
+to be correct and matching server's lease information. The server called hooks
+for the lease6_decline hook point and one of the callouts set next step status to SKIP.
+The server will skip the operation of moving the lease to the declined state and
+will continue processing the packet. In particular, it will send a REPLY message
+as if the decline actually took place.
+
+% DHCP6_HOOK_DECLINE_DROP During Decline processing (client=%1, interface=%2, addr=%3) hook callout set status to DROP, dropping packet.
+This message indicates that the server received DECLINE message, it was verified
+to be correct and matching server's lease information. The server called hooks
+for the lease6_decline hook point and one of the callouts set next step status to DROP.
+The server will now abort processing of the packet as if it was never
+received. The lease will continue to be assigned to this client.
+
 % DHCP6_HOOK_LEASE6_RELEASE_NA_SKIP %1: DHCPv6 address lease was not released because a callout set the skip flag
 This debug message is printed when a callout installed on the
 lease6_release hook point set the skip flag. For this particular hook

src/bin/dhcp6/dhcp6_srv.cc

@@ -96,6 +96,7 @@ struct Dhcp6Hooks {
     int hook_index_lease6_release_; ///< index for "lease6_release" hook point
     int hook_index_pkt6_send_;      ///< index for "pkt6_send" hook point
     int hook_index_buffer6_send_;   ///< index for "buffer6_send" hook point
+    int hook_index_lease6_decline_; ///< index for "lease6_decline" hook point
 
     /// Constructor that registers hook points for DHCPv6 engine
     Dhcp6Hooks() {
@@ -105,6 +106,7 @@ struct Dhcp6Hooks {
         hook_index_lease6_release_ = HooksManager::registerHook("lease6_release");
         hook_index_pkt6_send_      = HooksManager::registerHook("pkt6_send");
         hook_index_buffer6_send_   = HooksManager::registerHook("buffer6_send");
+        hook_index_lease6_decline_ = HooksManager::registerHook("lease6_decline");
     }
 };
 
@@ -180,7 +182,7 @@ const std::string Dhcpv6Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
 static const char* SERVER_DUID_FILE = "kea-dhcp6-serverid";
 
 Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
-:alloc_engine_(), serverid_(), port_(port), shutdown_(true)
+    : serverid_(), port_(port), shutdown_(true), alloc_engine_()
 {
 
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_OPEN_SOCKET).arg(port);
@@ -411,7 +413,7 @@ bool Dhcpv6Srv::run() {
             // processing step would to parse the packet, so skip at this
             // stage means that callouts did the parsing already, so server
             // should skip parsing.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP6_DETAIL, DHCP6_HOOK_BUFFER_RCVD_SKIP)
                     .arg(query->getRemoteAddr().toText())
                     .arg(query->getLocalAddr().toText())
@@ -419,6 +421,8 @@ bool Dhcpv6Srv::run() {
                 skip_unpack = true;
             }
 
+            /// @todo: Add support for DROP status.
+
             callout_handle->getArgument("query6", query);
         }
 
@@ -500,12 +504,14 @@ bool Dhcpv6Srv::run() {
             // Callouts decided to skip the next processing step. The next
             // processing step would to process the packet, so skip at this
             // stage means drop.
-            if (callout_handle->getSkip()) {
+            if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                 LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_PACKET_RCVD_SKIP)
                     .arg(query->getLabel());
                 continue;
             }
 
+            /// @todo: Add support for DROP status.
+
             callout_handle->getArgument("query6", query);
         }
 
@@ -639,11 +645,13 @@ bool Dhcpv6Srv::run() {
                 // That step will be skipped if any callout sets skip flag.
                 // It essentially means that the callout already did packing,
                 // so the server does not have to do it again.
-                if (callout_handle->getSkip()) {
+                if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                     LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_PACKET_SEND_SKIP)
                         .arg(rsp->getLabel());
                     skip_pack = true;
                 }
+
+                /// @todo: Add support for DROP status
             }
 
             if (!skip_pack) {
@@ -678,12 +686,14 @@ bool Dhcpv6Srv::run() {
                     // Callouts decided to skip the next processing step. The next
                     // processing step would to parse the packet, so skip at this
                     // stage means drop.
-                    if (callout_handle->getSkip()) {
+                    if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
                         LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_BUFFER_SEND_SKIP)
                             .arg(rsp->getLabel());
                         continue;
                     }
 
+                    /// @todo: Add support for DROP status
+
                     callout_handle->getArgument("response6", rsp);
                 }
 
@@ -1068,12 +1078,14 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) {
         // subnet will be selected. Packet processing will continue,
         // but it will be severely limited (i.e. only global options
         // will be assigned)
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_SUBNET6_SELECT_SKIP)
                 .arg(question->getLabel());
             return (Subnet6Ptr());
         }
 
+        /// @todo: Add support for DROP status.
+
         // Use whatever subnet was specified by the callout
         callout_handle->getArgument("subnet6", subnet);
     }
@@ -2142,11 +2154,13 @@ Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, const Pkt6Ptr& query,
         // Callouts decided to skip the next processing step. The next
         // processing step would to send the packet, so skip at this
         // stage means "drop response".
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             skip = true;
             LOG_DEBUG(hooks_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_LEASE6_RELEASE_NA_SKIP)
                 .arg(query->getLabel());
         }
+
+        /// @todo: Add support for DROP status
     }
 
     // Ok, we've passed all checks. Let's release this address.
@@ -2295,7 +2309,7 @@ Dhcpv6Srv::releaseIA_PD(const DuidPtr& duid, const Pkt6Ptr& query,
         // Call all installed callouts
         HooksManager::callCallouts(Hooks.hook_index_lease6_release_, *callout_handle);
 
-        skip = callout_handle->getSkip();
+        skip = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
     }
 
     // Ok, we've passed all checks. Let's release this prefix.
@@ -2581,12 +2595,17 @@ Dhcpv6Srv::processDecline(const Pkt6Ptr& decline) {
     // Include server-id
     appendDefaultOptions(decline, reply);
 
-    declineLeases(decline, reply, ctx);
+    if (declineLeases(decline, reply, ctx)) {
+        return (reply);
+    } else {
 
-    return (reply);
+        // declineLeases returns false only if the hooks set the next step
+        // status to DROP. We'll just doing as requested.
+        return (Pkt6Ptr());
+    }
 }
 
-void
+bool
 Dhcpv6Srv::declineLeases(const Pkt6Ptr& decline, Pkt6Ptr& reply,
                          AllocEngine::ClientContext6& ctx) {
 
@@ -2607,7 +2626,15 @@ Dhcpv6Srv::declineLeases(const Pkt6Ptr& decline, Pkt6Ptr& reply,
             OptionPtr answer_opt = declineIA(decline, ctx.duid_, general_status,
                                    boost::dynamic_pointer_cast<Option6IA>(opt->second));
             if (answer_opt) {
+
+                // We have an answer, let's use it.
                 reply->addOption(answer_opt);
+            } else {
+
+                // The only case when declineIA could return NULL is if one of the
+                // hook callouts set next step status to DROP. We just need to drop
+                // this packet.
+                return (false);
             }
             break;
         }
@@ -2616,6 +2643,8 @@ Dhcpv6Srv::declineLeases(const Pkt6Ptr& decline, Pkt6Ptr& reply,
             ;
         }
     }
+
+    return (true);
 }
 
 OptionPtr
@@ -2722,7 +2751,11 @@ Dhcpv6Srv::declineIA(const Pkt6Ptr& decline, const DuidPtr& duid,
         }
 
         // Ok, all is good. Decline this lease.
-        declineLease(decline, lease, ia_rsp);
+        if (!declineLease(decline, lease, ia_rsp)) {
+            // declineLease returns false only when hook callouts set the next
+            // step status to drop. We just propagate the bad news here.
+            return (OptionPtr());
+        }
     }
 
     if (total_addrs == 0) {
@@ -2743,9 +2776,54 @@ Dhcpv6Srv::setStatusCode(boost::shared_ptr<isc::dhcp::Option6IA>& container,
     container->addOption(status);
 }
 
-void
+bool
 Dhcpv6Srv::declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease,
                         boost::shared_ptr<Option6IA> ia_rsp) {
+    // We do not want to decrease the assigned-nas at this time. While
+    // technically a declined address is no longer allocated, the primary usage
+    // of the assigned-addresses statistic is to monitor pool utilization. Most
+    // people would forget to include declined-addresses in the calculation,
+    // and simply do assigned-addresses/total-addresses. This would have a bias
+    // towards under-representing pool utilization, if we decreased allocated
+    // immediately after receiving DHCPDECLINE, rather than later when we recover
+    // the address.
+
+    // Let's call lease6_decline hooks if necessary.
+    if (HooksManager::calloutsPresent(Hooks.hook_index_lease6_decline_)) {
+        CalloutHandlePtr callout_handle = getCalloutHandle(decline);
+
+        // Delete previously set arguments
+        callout_handle->deleteAllArguments();
+
+        // Pass incoming packet as argument
+        callout_handle->setArgument("query6", decline);
+        callout_handle->setArgument("lease6", lease);
+
+        // Call callouts
+        HooksManager::callCallouts(Hooks.hook_index_lease6_decline_,
+                                   *callout_handle);
+
+        // Callouts decided to SKIP the next processing step. The next
+        // processing step would to actually decline the lease, so we'll
+        // keep the lease as is.
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
+            LOG_DEBUG(hooks_logger, DBG_DHCP6_DETAIL, DHCP6_HOOK_DECLINE_SKIP)
+                .arg(decline->getLabel())
+                .arg(decline->getIface())
+                .arg(lease->addr_.toText());
+            return (true);
+        }
+
+        // Callouts decided to DROP the packet. Let's simply log it and
+        // return false, so callers will act accordingly.
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
+            LOG_DEBUG(hooks_logger, DBG_DHCP6_DETAIL, DHCP6_HOOK_DECLINE_DROP)
+                .arg(decline->getLabel())
+                .arg(decline->getIface())
+                .arg(lease->addr_.toText());
+            return (false);
+        }
+    }
 
     // Check if a lease has flags indicating that the FQDN update has
     // been performed. If so, create NameChangeRequest which removes
@@ -2760,8 +2838,6 @@ Dhcpv6Srv::declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease,
     // Global declined addresses counter.
     StatsMgr::instance().addValue("declined-addresses", static_cast<int64_t>(1));
 
-    // @todo: Call hooks.
-
     // We need to disassociate the lease from the client. Once we move a lease
     // to declined state, it is no longer associated with the client in any
     // way.
@@ -2773,6 +2849,8 @@ Dhcpv6Srv::declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease,
 
     ia_rsp->addOption(createStatusCode(*decline, *ia_rsp, STATUS_Success,
                       "Lease declined. Hopefully the next one will be better."));
+
+    return (true);
 }
 
 Pkt6Ptr

src/bin/dhcp6/dhcp6_srv.h

@@ -709,9 +709,10 @@ protected:
     /// @param decline Decline messege sent by a client
     /// @param reply Server's response (IA_NA with status will be added here)
     /// @param client context
-    void
-    declineLeases(const Pkt6Ptr& decline, Pkt6Ptr& reply,
-                  AllocEngine::ClientContext6& ctx);
+    /// @return true when expected to continue, false when hooks told us to drop
+    ///         the packet
+    bool declineLeases(const Pkt6Ptr& decline, Pkt6Ptr& reply,
+                       AllocEngine::ClientContext6& ctx);
 
     /// @brief Declines leases in a single IA_NA option
     ///
@@ -743,9 +744,10 @@ protected:
     /// @param decline used for generating removal Name Change Request.
     /// @param lease lease to be declined
     /// @param ia_rsp response IA_NA.
-    void
-    declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease,
-                 boost::shared_ptr<Option6IA> ia_rsp);
+    /// @return true when expected to continue, false when hooks told us to drop
+    ///         the packet
+    bool declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease,
+                      boost::shared_ptr<Option6IA> ia_rsp);
 
     /// @brief A simple utility method that sets the status code
     ///
@@ -823,12 +825,6 @@ private:
     /// @param query packet transmitted
     static void processStatsSent(const Pkt6Ptr& response);
 
-    /// @brief Allocation Engine.
-    /// Pointer to the allocation engine that we are currently using
-    /// It must be a pointer, because we will support changing engines
-    /// during normal operation (e.g. to use different allocators)
-    boost::shared_ptr<AllocEngine> alloc_engine_;
-
     /// Server DUID (to be sent in server-identifier option)
     OptionPtr serverid_;
 
@@ -841,6 +837,12 @@ protected:
     /// initiate server shutdown procedure.
     volatile bool shutdown_;
 
+    /// @brief Allocation Engine.
+    /// Pointer to the allocation engine that we are currently using
+    /// It must be a pointer, because we will support changing engines
+    /// during normal operation (e.g. to use different allocators)
+    boost::shared_ptr<AllocEngine> alloc_engine_;
+
     /// Holds a list of @c isc::dhcp_ddns::NameChangeRequest objects, which
     /// are waiting for sending to kea-dhcp-ddns module.
     std::queue<isc::dhcp_ddns::NameChangeRequest> name_change_reqs_;

src/bin/dhcp6/json_config_parser.cc

@@ -26,6 +26,7 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/pool.h>
 #include <dhcpsrv/subnet.h>
+#include <dhcpsrv/timer_mgr.h>
 #include <dhcpsrv/triplet.h>
 #include <dhcpsrv/parsers/dbaccess_parser.h>
 #include <dhcpsrv/parsers/dhcp_config_parser.h>
@@ -745,6 +746,9 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
     // so newly recreated configuration starts with first subnet-id equal 1.
     Subnet::resetSubnetID();
 
+    // Remove any existing timers.
+    TimerMgr::instance()->unregisterTimers();
+
     // Some of the values specified in the configuration depend on
     // other values. Typically, the values in the subnet6 structure
     // depend on the global values. Also, option values configuration

src/bin/dhcp6/tests/confirm_unittest.cc

@@ -25,7 +25,6 @@ using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 
 namespace {
 

src/bin/dhcp6/tests/decline_unittest.cc

@@ -18,6 +18,7 @@
 #include <dhcp/tests/iface_mgr_test_config.h>
 #include <dhcp6/json_config_parser.h>
 #include <dhcp6/tests/dhcp6_message_test.h>
+#include <dhcpsrv/lease.h>
 #include <stats/stats_mgr.h>
 
 using namespace isc;
@@ -25,7 +26,6 @@ using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 using namespace isc::stats;
 
 namespace {
@@ -56,38 +56,6 @@ const char* DECLINE_CONFIGS[] = {
 class DeclineTest : public Dhcpv6MessageTest {
 public:
 
-    /// @brief Specifies expected outcome
-    enum ExpectedResult {
-        SHOULD_PASS, // pass = accept decline, move lease to declined state.
-        SHOULD_FAIL  // fail = reject the decline
-    };
-
-    /// @brief Specifies what address should the client include in its Decline
-    enum AddressInclusion {
-        VALID_ADDR, // Client will include its own, valid address
-        BOGUS_ADDR, // Client will include an address it doesn't own
-        NO_ADDR,    // Client will send empty IA_NA (without address)
-        NO_IA       // Client will not send IA_NA at all
-    };
-
-    /// @brief Tests if the acquired lease is or is not declined.
-    ///
-    /// @param duid1 DUID used during lease acquisition
-    /// @param iaid1 IAID used during lease acquisition
-    /// @param duid2 DUID used during Decline exchange
-    /// @param iaid2 IAID used during Decline exchange
-    /// @param addr_type specify what sort of address the client should
-    ///        include (its own, a bogus one or no address at all)
-    /// @param expected_result SHOULD_PASS if the lease is expected to
-    /// be successfully declined, or SHOULD_FAIL if the lease is expected
-    /// to not be declined.
-    void acquireAndDecline(const std::string& duid1,
-                           const uint32_t iaid1,
-                           const std::string& duid2,
-                           const uint32_t iaid2,
-                           AddressInclusion addr_type,
-                           ExpectedResult expected_result);
-
     /// @brief Constructor.
     ///
     /// Sets up fake interfaces.
@@ -100,17 +68,23 @@ public:
 
 };
 
+};
+
+namespace isc {
+namespace dhcp {
+namespace test {
+
 void
-DeclineTest::acquireAndDecline(const std::string& duid1,
-                               const uint32_t iaid1,
-                               const std::string& duid2,
-                               const uint32_t iaid2,
-                               AddressInclusion addr_type,
-                               ExpectedResult expected_result) {
+Dhcpv6SrvTest::acquireAndDecline(Dhcp6Client& client,
+                                 const std::string& duid1,
+                                 const uint32_t iaid1,
+                                 const std::string& duid2,
+                                 const uint32_t iaid2,
+                                 AddressInclusion addr_type,
+                                 ExpectedResult expected_result) {
     // Set this global statistic explicitly to zero.
     StatsMgr::instance().setValue("declined-addresses", static_cast<int64_t>(0));
 
-    Dhcp6Client client;
     client.setDUID(duid1);
     client.useNA(iaid1);
 
@@ -135,7 +109,7 @@ DeclineTest::acquireAndDecline(const std::string& duid1,
     // Make sure that the client has acquired NA lease.
     std::vector<Lease6> leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(1, leases_client_na.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(iaid1));
 
     // Remember the acquired address.
     IOAddress acquired_address = leases_client_na[0].addr_;
@@ -214,19 +188,25 @@ DeclineTest::acquireAndDecline(const std::string& duid1,
 
 // This test checks that the client can acquire and decline the lease.
 TEST_F(DeclineTest, basic) {
-    acquireAndDecline("01:02:03:04:05:06", 1234,
-                      "01:02:03:04:05:06", 1234,
-                      VALID_ADDR, SHOULD_PASS);
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234, "01:02:03:04:05:06",
+                      1234, VALID_ADDR, SHOULD_PASS);
 }
 
+};
+};
+};
+
+namespace {
+
 // This test verifies the decline is rejected in the following case:
 // - Client acquires new lease using duid, iaid
 // - Client sends the DECLINE with duid, iaid, but uses wrong address.
 // - The server rejects Decline due to address mismatch
 TEST_F(DeclineTest, addressMismatch) {
-    acquireAndDecline("01:02:03:04:05:06", 1234,
-                      "01:02:03:04:05:06", 1234,
-                      BOGUS_ADDR, SHOULD_FAIL);
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234, "01:02:03:04:05:06",
+                      1234, BOGUS_ADDR, SHOULD_FAIL);
 }
 
 // This test verifies the decline is rejected in the following case:
@@ -234,9 +214,9 @@ TEST_F(DeclineTest, addressMismatch) {
 // - Client sends the DECLINE with duid, iaid2
 // - The server rejects Decline due to IAID mismatch
 TEST_F(DeclineTest, iaidMismatch) {
-    acquireAndDecline("01:02:03:04:05:06", 1234,
-                      "01:02:03:04:05:06", 1235,
-                      VALID_ADDR, SHOULD_FAIL);
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234, "01:02:03:04:05:06",
+                      1235, VALID_ADDR, SHOULD_FAIL);
 }
 
 // This test verifies the decline correctness in the following case:
@@ -244,7 +224,8 @@ TEST_F(DeclineTest, iaidMismatch) {
 // - Client sends the DECLINE using duid2, iaid
 // - The server rejects the Decline due to DUID mismatch
 TEST_F(DeclineTest, duidMismatch) {
-    acquireAndDecline("01:02:03:04:05:06", 1234,
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234,
                       "01:02:03:04:05:07", 1234,
                       VALID_ADDR, SHOULD_FAIL);
 }
@@ -255,7 +236,8 @@ TEST_F(DeclineTest, duidMismatch) {
 //   include the address in it
 // - The server rejects the Decline due to missing address
 TEST_F(DeclineTest, noAddrsSent) {
-    acquireAndDecline("01:02:03:04:05:06", 1234,
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234,
                       "01:02:03:04:05:06", 1234,
                       NO_ADDR, SHOULD_FAIL);
 }
@@ -266,7 +248,8 @@ TEST_F(DeclineTest, noAddrsSent) {
 //   include IA_NA at all
 // - The server rejects the Decline due to missing IA_NA
 TEST_F(DeclineTest, noIAs) {
-    acquireAndDecline("01:02:03:04:05:06", 1234,
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234,
                       "01:02:03:04:05:06", 1234,
                       NO_IA, SHOULD_FAIL);
 }

src/bin/dhcp6/tests/dhcp6_client.cc

@@ -30,7 +30,6 @@
 
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 
 namespace {
 

src/bin/dhcp6/tests/dhcp6_client.h

@@ -178,7 +178,7 @@ public:
     /// - not relayed
     ///
     /// @param srv Object representing server under test.
-    Dhcp6Client(boost::shared_ptr<isc::test::NakedDhcpv6Srv>& srv);
+    Dhcp6Client(boost::shared_ptr<isc::dhcp::test::NakedDhcpv6Srv>& srv);
 
     /// @brief Create lease for the client.
     ///
@@ -365,7 +365,7 @@ public:
     }
 
     /// @brief Returns the server that the client is communicating with.
-    boost::shared_ptr<isc::test::NakedDhcpv6Srv> getServer() const {
+    boost::shared_ptr<isc::dhcp::test::NakedDhcpv6Srv> getServer() const {
         return (srv_);
     }
 
@@ -727,7 +727,7 @@ private:
     std::string iface_name_;
 
     /// @brief Pointer to the server that the client is communicating with.
-    boost::shared_ptr<isc::test::NakedDhcpv6Srv> srv_;
+    boost::shared_ptr<isc::dhcp::test::NakedDhcpv6Srv> srv_;
 
     bool use_na_;    ///< Enable address assignment.
     bool use_pd_;    ///< Enable prefix delegation.

src/bin/dhcp6/tests/dhcp6_message_test.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2014, 2015  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -26,7 +26,7 @@ namespace test {
 
 /// @brief Base class for test fixure classes used to validate the DHCPv6
 /// message processing by the server.
-class Dhcpv6MessageTest : public isc::test::Dhcpv6SrvTest {
+class Dhcpv6MessageTest : public isc::dhcp::test::Dhcpv6SrvTest {
 public:
     /// @brief Constructor.
     ///

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -53,7 +53,6 @@
 
 using namespace isc;
 using namespace isc::data;
-using namespace isc::test;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;

src/bin/dhcp6/tests/dhcp6_test_utils.cc

@@ -29,6 +29,7 @@ using namespace isc::asiolink;
 using namespace isc::stats;
 
 namespace isc {
+namespace dhcp {
 namespace test {
 
 const char* NakedDhcpv6SrvTest::DUID_FILE = "server-id-test.txt";
@@ -827,6 +828,6 @@ NakedDhcpv6SrvTest::checkIA_NAStatusCode(
     }
 }
 
-
-}; // end of isc::test namespace
+}; // end of isc::dhcp::test namespace
+}; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp6/tests/dhcp6_test_utils.h

@@ -39,6 +39,7 @@
 #include <list>
 
 namespace isc {
+namespace dhcp {
 namespace test {
 
 /// @brief "naked" Dhcpv6Srv class that exposes internal members
@@ -290,11 +291,28 @@ public:
     std::string valid_iface_;
 };
 
+// We need to pass one reference to the Dhcp6Client, which is defined in
+// dhcp6_client.h. That header includes this file. To avoid circular
+// dependencies, we use forward declaration here.
+class Dhcp6Client;
+
 // Provides suport for tests against a preconfigured subnet6
 // extends upon NakedDhcp6SrvTest
 class Dhcpv6SrvTest : public NakedDhcpv6SrvTest {
 public:
-    /// Name of the server-id file (used in server-id tests)
+    /// @brief Specifies expected outcome
+    enum ExpectedResult {
+        SHOULD_PASS, // pass = accept decline, move lease to declined state.
+        SHOULD_FAIL  // fail = reject the decline
+    };
+
+    /// @brief Specifies what address should the client include in its Decline
+    enum AddressInclusion {
+        VALID_ADDR, // Client will include its own, valid address
+        BOGUS_ADDR, // Client will include an address it doesn't own
+        NO_ADDR,    // Client will send empty IA_NA (without address)
+        NO_IA       // Client will not send IA_NA at all
+    };
 
     /// @brief Constructor that initializes a simple default configuration
     ///
@@ -437,6 +455,26 @@ public:
     bool compareOptions(const isc::dhcp::OptionPtr& option1,
                         const isc::dhcp::OptionPtr& option2);
 
+    /// @brief Tests if the acquired lease is or is not declined.
+    ///
+    /// @param client Dhcp6Client instance
+    /// @param duid1 DUID used during lease acquisition
+    /// @param iaid1 IAID used during lease acquisition
+    /// @param duid2 DUID used during Decline exchange
+    /// @param iaid2 IAID used during Decline exchange
+    /// @param addr_type specify what sort of address the client should
+    ///        include (its own, a bogus one or no address at all)
+    /// @param expected_result SHOULD_PASS if the lease is expected to
+    /// be successfully declined, or SHOULD_FAIL if the lease is expected
+    /// to not be declined.
+    void acquireAndDecline(Dhcp6Client& client,
+                           const std::string& duid1,
+                           const uint32_t iaid1,
+                           const std::string& duid2,
+                           const uint32_t iaid2,
+                           AddressInclusion addr_type,
+                           ExpectedResult expected_result);
+
     /// @brief Performs basic (positive) RENEW test
     ///
     /// See renewBasic and pdRenewBasic tests for detailed explanation.
@@ -520,7 +558,8 @@ public:
     NakedDhcpv6Srv srv_;
 };
 
-}; // end of isc::test namespace
+}; // end of isc::dhcp::test namespace
+}; // end of isc::dhcp namespace
 }; // end of isc namespace
 
 #endif // DHCP6_TEST_UTILS_H

src/bin/dhcp6/tests/fqdn_unittest.cc

@@ -32,7 +32,7 @@
 #include <gtest/gtest.h>
 
 using namespace isc;
-using namespace isc::test;
+using namespace isc::dhcp::test;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::dhcp_ddns;

src/bin/dhcp6/tests/hooks_unittest.cc

@@ -28,6 +28,8 @@
 #include <hooks/server_hooks.h>
 
 #include <dhcp6/tests/dhcp6_test_utils.h>
+#include <dhcp6/tests/dhcp6_client.h>
+#include <dhcp/tests/iface_mgr_test_config.h>
 #include <dhcp/tests/pkt_captures.h>
 #include <cc/command_interpreter.h>
 #include <boost/scoped_ptr.hpp>
@@ -39,7 +41,7 @@
 
 using namespace isc;
 using namespace isc::data;
-using namespace isc::test;
+using namespace isc::dhcp::test;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::util;
@@ -189,7 +191,7 @@ public:
         Pkt6Ptr pkt;
         callout_handle.getArgument("query6", pkt);
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // Carry on as usual
         return pkt6_receive_callout(callout_handle);
@@ -261,7 +263,7 @@ public:
     /// @return always 0
     static int
     buffer6_receive_skip(CalloutHandle& callout_handle) {
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // Carry on as usual
         return buffer6_receive_callout(callout_handle);
@@ -324,7 +326,7 @@ public:
         Pkt6Ptr pkt;
         callout_handle.getArgument("response6", pkt);
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         // carry on as usual
         return pkt6_send_callout(callout_handle);
@@ -426,7 +428,7 @@ public:
     lease6_renew_skip_callout(CalloutHandle& callout_handle) {
         callback_name_ = string("lease6_renew");
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         return (0);
     }
@@ -452,11 +454,48 @@ public:
     lease6_release_skip_callout(CalloutHandle& callout_handle) {
         callback_name_ = string("lease6_release");
 
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         return (0);
     }
 
+    /// Lease6_decline test callback
+    ///
+    /// Stores all parameters in callback_* fields.
+    ///
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    lease6_decline_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("lease6_decline");
+        callout_handle.getArgument("query6", callback_pkt6_);
+        callout_handle.getArgument("lease6", callback_lease6_);
+
+        return (0);
+    }
+
+    /// Lease6_decline callout that sets status to SKIP
+    ///
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    lease6_decline_skip_callout(CalloutHandle& callout_handle) {
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
+
+        return (lease6_decline_callout(callout_handle));
+    }
+
+    /// Lease6_decline callout that sets status to DROP
+    ///
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    lease6_decline_drop_callout(CalloutHandle& callout_handle) {
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
+
+        return (lease6_decline_callout(callout_handle));
+    }
+
     /// Resets buffers used to store data received by callouts
     void resetCalloutBuffers() {
         callback_name_ = string("");
@@ -518,7 +557,7 @@ boost::shared_ptr<Option6IA> HooksDhcpv6SrvTest::callback_ia_na_;
 //
 // Note that the test name does not follow test naming convention,
 // but the proper hook name is "buffer6_receive".
-TEST_F(HooksDhcpv6SrvTest, simple_buffer6_receive) {
+TEST_F(HooksDhcpv6SrvTest, simpleBuffer6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -551,7 +590,7 @@ TEST_F(HooksDhcpv6SrvTest, simple_buffer6_receive) {
 
 // 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) {
+TEST_F(HooksDhcpv6SrvTest, valueChangeBuffer6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -588,7 +627,7 @@ TEST_F(HooksDhcpv6SrvTest, valueChange_buffer6_receive) {
 // Checks if callouts installed on buffer6_receive 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(HooksDhcpv6SrvTest, deleteClientId_buffer6_receive) {
+TEST_F(HooksDhcpv6SrvTest, deleteClientIdBuffer6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -612,7 +651,7 @@ TEST_F(HooksDhcpv6SrvTest, deleteClientId_buffer6_receive) {
 
 // Checks if callouts installed on buffer6_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(HooksDhcpv6SrvTest, skip_buffer6_receive) {
+TEST_F(HooksDhcpv6SrvTest, skipBuffer6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -639,7 +678,7 @@ TEST_F(HooksDhcpv6SrvTest, skip_buffer6_receive) {
 //
 // Note that the test name does not follow test naming convention,
 // but the proper hook name is "pkt6_receive".
-TEST_F(HooksDhcpv6SrvTest, simple_pkt6_receive) {
+TEST_F(HooksDhcpv6SrvTest, simplePkt6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -672,7 +711,7 @@ TEST_F(HooksDhcpv6SrvTest, simple_pkt6_receive) {
 
 // Checks if callouts installed on pkt6_received is able to change
 // the values and the parameters are indeed used by the server.
-TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_receive) {
+TEST_F(HooksDhcpv6SrvTest, valueChangePkt6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -708,7 +747,7 @@ TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_receive) {
 // Checks if callouts installed on pkt6_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(HooksDhcpv6SrvTest, deleteClientId_pkt6_receive) {
+TEST_F(HooksDhcpv6SrvTest, deleteClientIdPkt6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -732,7 +771,7 @@ TEST_F(HooksDhcpv6SrvTest, deleteClientId_pkt6_receive) {
 
 // Checks if callouts installed on pkt6_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(HooksDhcpv6SrvTest, skip_pkt6_receive) {
+TEST_F(HooksDhcpv6SrvTest, skipPkt6Receive) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -757,7 +796,7 @@ TEST_F(HooksDhcpv6SrvTest, skip_pkt6_receive) {
 
 // Checks if callouts installed on pkt6_send are indeed called and the
 // all necessary parameters are passed.
-TEST_F(HooksDhcpv6SrvTest, simple_pkt6_send) {
+TEST_F(HooksDhcpv6SrvTest, simplePkt6Send) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -793,7 +832,7 @@ TEST_F(HooksDhcpv6SrvTest, simple_pkt6_send) {
 
 // Checks if callouts installed on pkt6_send is able to change
 // the values and the packet sent contains those changes
-TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_send) {
+TEST_F(HooksDhcpv6SrvTest, valueChangePkt6Send) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -830,7 +869,7 @@ TEST_F(HooksDhcpv6SrvTest, valueChange_pkt6_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(HooksDhcpv6SrvTest, deleteServerId_pkt6_send) {
+TEST_F(HooksDhcpv6SrvTest, deleteServerIdPkt6Send) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -861,7 +900,7 @@ TEST_F(HooksDhcpv6SrvTest, deleteServerId_pkt6_send) {
 
 // Checks if callouts installed on pkt6_skip is able to set skip flag that
 // will cause the server to not process the packet (drop), even though it is valid.
-TEST_F(HooksDhcpv6SrvTest, skip_pkt6_send) {
+TEST_F(HooksDhcpv6SrvTest, skipPkt6Send) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -892,7 +931,7 @@ TEST_F(HooksDhcpv6SrvTest, skip_pkt6_send) {
 
 // This test checks if subnet6_select callout is triggered and reports
 // valid parameters
-TEST_F(HooksDhcpv6SrvTest, subnet6_select) {
+TEST_F(HooksDhcpv6SrvTest, subnet6Select) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -965,7 +1004,7 @@ TEST_F(HooksDhcpv6SrvTest, subnet6_select) {
 
 // This test checks if callout installed on subnet6_select hook point can pick
 // a different subnet.
-TEST_F(HooksDhcpv6SrvTest, subnet_select_change) {
+TEST_F(HooksDhcpv6SrvTest, subnet6SselectChange) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -1038,7 +1077,7 @@ TEST_F(HooksDhcpv6SrvTest, subnet_select_change) {
 
 // This test verifies that incoming (positive) RENEW can be handled properly,
 // and the lease6_renew callouts are triggered.
-TEST_F(HooksDhcpv6SrvTest, basic_lease6_renew) {
+TEST_F(HooksDhcpv6SrvTest, basicLease6Renew) {
     NakedDhcpv6Srv srv(0);
 
     // Install pkt6_receive_callout
@@ -1137,7 +1176,7 @@ TEST_F(HooksDhcpv6SrvTest, basic_lease6_renew) {
 
 // This test verifies that incoming (positive) RENEW can be handled properly,
 // and the lease6_renew callouts are able to change the lease being updated.
-TEST_F(HooksDhcpv6SrvTest, leaseUpdate_lease6_renew) {
+TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Renew) {
     NakedDhcpv6Srv srv(0);
 
     // Install pkt6_receive_callout
@@ -1230,7 +1269,7 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdate_lease6_renew) {
 // This test verifies that incoming (positive) RENEW can be handled properly,
 // and the lease6_renew callouts are able to set the skip flag that will
 // reject the renewal
-TEST_F(HooksDhcpv6SrvTest, skip_lease6_renew) {
+TEST_F(HooksDhcpv6SrvTest, skipLease6Renew) {
     NakedDhcpv6Srv srv(0);
 
     // Install pkt6_receive_callout
@@ -1308,7 +1347,7 @@ TEST_F(HooksDhcpv6SrvTest, skip_lease6_renew) {
 // - returned REPLY message has server-id
 // - returned REPLY message has IA that does not include an IAADDR
 // - lease is actually removed from LeaseMgr
-TEST_F(HooksDhcpv6SrvTest, basic_lease6_release) {
+TEST_F(HooksDhcpv6SrvTest, basicLease6Release) {
     NakedDhcpv6Srv srv(0);
 
     // Install pkt6_receive_callout
@@ -1390,7 +1429,7 @@ TEST_F(HooksDhcpv6SrvTest, basic_lease6_release) {
 // - returned REPLY message has server-id
 // - returned REPLY message has IA that does not include an IAADDR
 // - lease is actually removed from LeaseMgr
-TEST_F(HooksDhcpv6SrvTest, skip_lease6_release) {
+TEST_F(HooksDhcpv6SrvTest, skipLease6Release) {
     NakedDhcpv6Srv srv(0);
 
     // Install pkt6_receive_callout
@@ -1452,4 +1491,135 @@ TEST_F(HooksDhcpv6SrvTest, skip_lease6_release) {
     ASSERT_TRUE(l);
 }
 
+// This test checks that the basic decline hook (lease6_decline) is
+// triggered properly.
+TEST_F(HooksDhcpv6SrvTest, basicLease6Decline) {
+    IfaceMgrTestConfig test_config(true);
+
+    // Install lease6_decline callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease6_decline", lease6_decline_callout));
+
+    // Get an address and decline it. DUIDs, IAID match and we send valid
+    // address, so the decline procedure should be successful.
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234, "01:02:03:04:05:06",
+                      1234, VALID_ADDR, SHOULD_PASS);
+
+    // Check that the proper callback was called.
+    EXPECT_EQ("lease6_decline", callback_name_);
+
+    // And valid parameters were passed.
+    ASSERT_TRUE(callback_pkt6_);
+    ASSERT_TRUE(callback_lease6_);
+
+    // Test sanity check - it was a decline, right?
+    EXPECT_EQ(DHCPV6_DECLINE, callback_pkt6_->getType());
+
+    // Get the address from this decline.
+    OptionPtr ia = callback_pkt6_->getOption(D6O_IA_NA);
+    ASSERT_TRUE(ia);
+    boost::shared_ptr<Option6IAAddr> addr_opt =
+        boost::dynamic_pointer_cast<Option6IAAddr>(ia->getOption(D6O_IAADDR));
+    ASSERT_TRUE(addr_opt);
+    IOAddress addr(addr_opt->getAddress());
+
+    // Now get a lease from the database.
+    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
+                                                               addr);
+    ASSERT_TRUE(from_mgr);
+    // Now check that it's indeed declined.
+    EXPECT_EQ(Lease::STATE_DECLINED, from_mgr->state_);
+
+    // And that the parameters passed to callout are consistent with the database
+    EXPECT_EQ(addr, from_mgr->addr_);
+    EXPECT_EQ(addr, callback_lease6_->addr_);
+}
+
+// Test that the lease6_decline hook point can handle SKIP status.
+TEST_F(HooksDhcpv6SrvTest, lease6DeclineSkip) {
+    IfaceMgrTestConfig test_config(true);
+
+    // Install lease6_decline callout. It will set the status to skip
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease6_decline", lease6_decline_skip_callout));
+
+    // Get an address and decline it. DUIDs, IAID match and we send valid
+    // address, so the decline procedure should be successful.
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234, "01:02:03:04:05:06",
+                      1234, VALID_ADDR, SHOULD_FAIL);
+
+    // Check that the proper callback was called.
+    EXPECT_EQ("lease6_decline", callback_name_);
+
+    // And valid parameters were passed.
+    ASSERT_TRUE(callback_pkt6_);
+    ASSERT_TRUE(callback_lease6_);
+
+    // Test sanity check - it was a decline, right?
+    EXPECT_EQ(DHCPV6_DECLINE, callback_pkt6_->getType());
+
+    // Get the address from this decline.
+    OptionPtr ia = callback_pkt6_->getOption(D6O_IA_NA);
+    ASSERT_TRUE(ia);
+    boost::shared_ptr<Option6IAAddr> addr_opt =
+        boost::dynamic_pointer_cast<Option6IAAddr>(ia->getOption(D6O_IAADDR));
+    ASSERT_TRUE(addr_opt);
+    IOAddress addr(addr_opt->getAddress());
+
+    // Now get a lease from the database.
+    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
+                                                               addr);
+    ASSERT_TRUE(from_mgr);
+    // Now check that it's NOT declined.
+    EXPECT_EQ(Lease::STATE_DEFAULT, from_mgr->state_);
+
+    // And that the parameters passed to callout are consistent with the database
+    EXPECT_EQ(addr, from_mgr->addr_);
+    EXPECT_EQ(addr, callback_lease6_->addr_);
+}
+
+// Test that the lease6_decline hook point can handle DROP status.
+TEST_F(HooksDhcpv6SrvTest, lease6DeclineDrop) {
+    IfaceMgrTestConfig test_config(true);
+
+    // Install lease6_decline callout. It will set the status to skip
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease6_decline", lease6_decline_drop_callout));
+
+    // Get an address and decline it. DUIDs, IAID match and we send valid
+    // address, so it would work, but the callout sets status to DROP, so
+    // the server should not update the lease and should not send back any
+    // packets.
+    Dhcp6Client client;
+    acquireAndDecline(client, "01:02:03:04:05:06", 1234, "01:02:03:04:05:06",
+                      1234, VALID_ADDR, SHOULD_FAIL);
+
+    // Check that the proper callback was called.
+    EXPECT_EQ("lease6_decline", callback_name_);
+
+    // And valid parameters were passed.
+    ASSERT_TRUE(callback_pkt6_);
+    ASSERT_TRUE(callback_lease6_);
+
+    // Test sanity check - it was a decline, right?
+    EXPECT_EQ(DHCPV6_DECLINE, callback_pkt6_->getType());
+
+    // Get the address from this decline.
+    OptionPtr ia = callback_pkt6_->getOption(D6O_IA_NA);
+    ASSERT_TRUE(ia);
+    boost::shared_ptr<Option6IAAddr> addr_opt =
+        boost::dynamic_pointer_cast<Option6IAAddr>(ia->getOption(D6O_IAADDR));
+    ASSERT_TRUE(addr_opt);
+    IOAddress addr(addr_opt->getAddress());
+
+    // Now get a lease from the database.
+    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
+                                                               addr);
+    ASSERT_TRUE(from_mgr);
+    // Now check that it's NOT declined.
+    EXPECT_EQ(Lease::STATE_DEFAULT, from_mgr->state_);
+}
+
 }   // end of anonymous namespace

src/bin/dhcp6/tests/host_unittest.cc

@@ -20,7 +20,6 @@
 using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 
 namespace {
 

src/bin/dhcp6/tests/infrequest_unittest.cc

@@ -23,7 +23,6 @@
 using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 
 namespace {
 

src/bin/dhcp6/tests/kea_controller_unittest.cc

@@ -14,11 +14,17 @@
 
 #include <config.h>
 
+#include <asiolink/io_address.h>
 #include <cc/command_interpreter.h>
 #include <dhcp/dhcp6.h>
+#include <dhcp/duid.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcp6/ctrl_dhcp6_srv.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/lease.h>
+#include <dhcpsrv/lease_mgr_factory.h>
 #include <log/logger_support.h>
+#include <util/stopwatch.h>
 
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
@@ -53,6 +59,7 @@ public:
     }
 
     ~JSONFileBackendTest() {
+        LeaseMgrFactory::destroy();
         isc::log::setDefaultLoggingOutput();
         static_cast<void>(remove(TEST_FILE));
     };
@@ -66,6 +73,21 @@ public:
         out.close();
     }
 
+    /// @brief Runs timers for specified time.
+    ///
+    /// Internally, this method calls @c IfaceMgr::receive6 to run the
+    /// callbacks for the installed timers.
+    ///
+    /// @param timeout_ms Amount of time after which the method returns.
+    void runTimersWithTimeout(const long timeout_ms) {
+        isc::util::Stopwatch stopwatch;
+        while (stopwatch.getTotalMilliseconds() < timeout_ms) {
+            // Block for up to one millisecond waiting for the timers'
+            // callbacks to be executed.
+            IfaceMgr::instancePtr()->receive6(0, 1000);
+        }
+    }
+
     static const char* TEST_FILE;
 };
 
@@ -245,4 +267,86 @@ TEST_F(JSONFileBackendTest, configBroken) {
     EXPECT_THROW(srv->init(TEST_FILE), BadValue);
 }
 
+// This test verifies that the DHCP server installs the timers for reclaiming
+// and flushing expired leases.
+TEST_F(JSONFileBackendTest, timers) {
+    // This is a basic configuration which enables timers for reclaiming
+    // expired leases and flushing them after 500 seconds since they expire.
+    // Both timers run at 1 second intervals.
+    string config =
+        "{ \"Dhcp6\": {"
+        "\"interfaces-config\": {"
+        "    \"interfaces\": [ ]"
+        "},"
+        "\"lease-database\": {"
+        "     \"type\": \"memfile\","
+        "     \"persist\": false"
+        "},"
+        "\"expired-leases-processing\": {"
+        "     \"reclaim-timer-wait-time\": 1,"
+        "     \"hold-reclaimed-time\": 500,"
+        "     \"flush-reclaimed-timer-wait-time\": 1"
+        "},"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ ],"
+        "\"preferred-lifetime\": 3000, "
+        "\"valid-lifetime\": 4000 }"
+        "}";
+    writeFile(TEST_FILE, config);
+
+    // Create an instance of the server and intialize it.
+    boost::scoped_ptr<ControlledDhcpv6Srv> srv;
+    ASSERT_NO_THROW(srv.reset(new ControlledDhcpv6Srv(0)));
+    ASSERT_NO_THROW(srv->init(TEST_FILE));
+
+    // Create an expired lease. The lease is expired by 40 seconds ago
+    // (valid lifetime = 60, cltt = now - 100). The lease will be reclaimed
+    // but shouldn't be flushed in the database because the reclaimed are
+    // held in the database 500 seconds after reclamation, according to the
+    // current configuration.
+    DuidPtr duid_expired(new DUID(DUID::fromText("00:01:02:03:04:05:06").getDuid()));
+    Lease6Ptr lease_expired(new Lease6(Lease::TYPE_NA, IOAddress("3000::1"),
+                                       duid_expired, 1, 50, 60, 10, 20, SubnetID(1)));
+    lease_expired->cltt_ = time(NULL) - 100;
+
+
+    // Create expired-reclaimed lease. The lease has expired 1000 - 60 seconds
+    // ago. It should be removed from the lease database when the "flush" timer
+    // goes off.
+    DuidPtr duid_reclaimed(new DUID(DUID::fromText("01:02:03:04:05:06:07").getDuid()));
+    Lease6Ptr lease_reclaimed(new Lease6(Lease::TYPE_NA, IOAddress("3000::2"),
+                                         duid_reclaimed, 1, 50, 60, 10, 20, SubnetID(1)));
+    lease_reclaimed->cltt_ = time(NULL) - 1000;
+    lease_reclaimed->state_ = Lease6::STATE_EXPIRED_RECLAIMED;
+
+    // Add leases to the database.
+    LeaseMgr& lease_mgr = LeaseMgrFactory().instance();
+    ASSERT_NO_THROW(lease_mgr.addLease(lease_expired));
+    ASSERT_NO_THROW(lease_mgr.addLease(lease_reclaimed));
+
+    // Make sure they have been added.
+    ASSERT_TRUE(lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::1")));
+    ASSERT_TRUE(lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::2")));
+
+    // Poll the timers for a while to make sure that each of them is executed
+    // at least once.
+    ASSERT_NO_THROW(runTimersWithTimeout(5000));
+
+    // Verify that the leases in the database have been processed as expected.
+
+    // First lease should be reclaimed, but not removed.
+    ASSERT_NO_THROW(
+        lease_expired = lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::1"))
+    );
+    ASSERT_TRUE(lease_expired);
+    EXPECT_TRUE(lease_expired->stateExpiredReclaimed());
+
+    // Second lease should have been removed.
+    ASSERT_NO_THROW(
+        lease_reclaimed = lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::2"))
+    );
+    EXPECT_FALSE(lease_reclaimed);
+}
+
 } // End of anonymous namespace

src/bin/dhcp6/tests/rebind_unittest.cc

@@ -25,7 +25,6 @@ using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 
 namespace {
 

src/bin/dhcp6/tests/renew_unittest.cc

@@ -24,7 +24,6 @@ using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 
 namespace {
 

src/bin/dhcp6/tests/sarr_unittest.cc

@@ -25,7 +25,6 @@
 using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
-using namespace isc::test;
 
 namespace {
 

src/bin/keactrl/kea.conf.pre

@@ -19,6 +19,22 @@
     "type": "memfile"
   },
 
+# Setup reclamation of the expired leases and leases affinity.
+# Expired leases will be reclaimed every 10 seconds. Every 25
+# seconds reclaimed leases, which have expired more than 3600
+# seconds ago, will be removed. The limits for leases reclamation
+# are 100 leases or 250 ms for a single cycle. A warning message
+# will be logged if there are still expired leases in the
+# database after 5 consecutive reclamation cycles.
+  "expired-leases-processing": {
+    "reclaim-timer-wait-time": 10,
+    "flush-reclaimed-timer-wait-time": 25,
+    "hold-reclaimed-time": 3600,
+    "max-reclaim-leases": 100,
+    "max-reclaim-time": 250,
+    "unwarned-reclaim-cycles": 5
+  },
+
 # Global (inherited by all subnets) lease lifetime is mandatory parameter.
   "valid-lifetime": 4000,
 
@@ -46,6 +62,22 @@
     "type": "memfile"
   },
 
+# Setup reclamation of the expired leases and leases affinity.
+# Expired leases will be reclaimed every 10 seconds. Every 25
+# seconds reclaimed leases, which have expired more than 3600
+# seconds ago, will be removed. The limits for leases reclamation
+# are 100 leases or 250 ms for a single cycle. A warning message
+# will be logged if there are still expired leases in the
+# database after 5 consecutive reclamation cycles.
+  "expired-leases-processing": {
+    "reclaim-timer-wait-time": 10,
+    "flush-reclaimed-timer-wait-time": 25,
+    "hold-reclaimed-time": 3600,
+    "max-reclaim-leases": 100,
+    "max-reclaim-time": 250,
+    "unwarned-reclaim-cycles": 5
+  },
+
 # Addresses will be assigned with preferred and valid lifetimes
 # being 3000 and 4000, respectively. Client is told to start
 # renewing after 1000 seconds. If the server does not respond

src/lib/config/command_socket_factory.cc

@@ -85,8 +85,17 @@ private:
         // shut down properly.
         static_cast<void>(remove(file_name.c_str()));
 
+        // Set this socket to be closed-on-exec.
+        if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) {
+            const char* errmsg = strerror(errno);
+            ::close(fd);
+            isc_throw(SocketError, "Failed to set close-on-exec on unix socket\
+ "
+                      << fd << ": " << errmsg);
+        }
+
         // Set this socket to be non-blocking one.
-        if (fcntl(fd, F_SETFL, O_NONBLOCK) !=0 ) {
+        if (fcntl(fd, F_SETFL, O_NONBLOCK) != 0) {
             const char* errmsg = strerror(errno);
             ::close(fd);
             isc_throw(SocketError, "Failed to set non-block mode on unix socket "

src/lib/dhcp/iface_mgr_linux.cc

@@ -42,6 +42,7 @@
 #include <boost/array.hpp>
 #include <boost/static_assert.hpp>
 
+#include <fcntl.h>
 #include <stdint.h>
 #include <net/if.h>
 #include <linux/rtnetlink.h>
@@ -135,6 +136,10 @@ void Netlink::rtnl_open_socket() {
         isc_throw(Unexpected, "Failed to create NETLINK socket.");
     }
 
+    if (fcntl(fd_, F_SETFD, FD_CLOEXEC) < 0) {
+        isc_throw(Unexpected, "Failed to set close-on-exec in NETLINK socket.");
+    }
+
     if (setsockopt(fd_, SOL_SOCKET, SO_SNDBUF, &SNDBUF_SIZE, sizeof(SNDBUF_SIZE)) < 0) {
         isc_throw(Unexpected, "Failed to set send buffer in NETLINK socket.");
     }

src/lib/dhcp/pkt_filter.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -32,6 +32,14 @@ PktFilter::openFallbackSocket(const isc::asiolink::IOAddress& addr,
                   " address " << addr << ", port " << port
                   << ", reason: " << strerror(errno));
     }
+    // Set the close-on-exec flag.
+    if (fcntl(sock, F_SETFD, FD_CLOEXEC) < 0) {
+        close(sock);
+        isc_throw(SocketConfigError, "Failed to set close-on-exec flag"
+                  << " on fallback socket for address " << addr
+                  << ", port " << port
+                  << ", reason: " << strerror(errno));
+    }
     // Bind the socket to a specified address and port.
     struct sockaddr_in addr4;
     memset(&addr4, 0, sizeof(addr4));

src/lib/dhcp/pkt_filter_bpf.cc

@@ -266,6 +266,14 @@ PktFilterBPF::openSocket(Iface& iface,
         }
     }
 
+    // Set the close-on-exec flag.
+    if (fcntl(sock, F_SETFD, FD_CLOEXEC) < 0) {
+        close(fallback);
+        close(sock);
+        isc_throw(SocketConfigError, "Failed to set close-on-exec flag"
+                  << " on BPF device with interface " << iface.getName());
+    }
+
     // The BPF device is now open. Now it needs to be configured.
 
     // Associate the device with the interface name.

src/lib/dhcp/pkt_filter_inet.cc

@@ -18,6 +18,7 @@
 #include <dhcp/pkt_filter_inet.h>
 #include <errno.h>
 #include <cstring>
+#include <fcntl.h>
 
 using namespace isc::asiolink;
 
@@ -55,6 +56,13 @@ PktFilterInet::openSocket(Iface& iface,
         isc_throw(SocketConfigError, "Failed to create UDP6 socket.");
     }
 
+    // Set the close-on-exec flag.
+    if (fcntl(sock, F_SETFD, FD_CLOEXEC) < 0) {
+        close(sock);
+        isc_throw(SocketConfigError, "Failed to set close-on-exec flag"
+                  << " on socket " << sock);
+    }
+
 #ifdef SO_BINDTODEVICE
     if (receive_bcast && iface.flag_broadcast_) {
         // Bind to device so as we receive traffic on a specific interface.

src/lib/dhcp/pkt_filter_inet6.cc

@@ -19,6 +19,7 @@
 #include <dhcp/pkt_filter_inet6.h>
 #include <util/io/pktinfo_utilities.h>
 
+#include <fcntl.h>
 #include <netinet/in.h>
 
 using namespace isc::asiolink;
@@ -66,6 +67,13 @@ PktFilterInet6::openSocket(const Iface& iface,
         isc_throw(SocketConfigError, "Failed to create UDP6 socket.");
     }
 
+    // Set the close-on-exec flag.
+    if (fcntl(sock, F_SETFD, FD_CLOEXEC) < 0) {
+        close(sock);
+        isc_throw(SocketConfigError, "Failed to set close-on-exec flag"
+                  << " on IPv6 socket.");
+    }
+
     // Set SO_REUSEADDR option.
     int flag = 1;
     if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,

src/lib/dhcp/pkt_filter_lpf.cc

@@ -19,6 +19,7 @@
 #include <dhcp/pkt_filter_lpf.h>
 #include <dhcp/protocol_util.h>
 #include <exceptions/exceptions.h>
+#include <fcntl.h>
 #include <linux/filter.h>
 #include <linux/if_ether.h>
 #include <linux/if_packet.h>
@@ -154,6 +155,14 @@ PktFilterLPF::openSocket(Iface& iface,
         isc_throw(SocketConfigError, "Failed to create raw LPF socket");
     }
 
+    // Set the close-on-exec flag.
+    if (fcntl(sock, F_SETFD, FD_CLOEXEC) < 0) {
+        close(sock);
+        close(fallback);
+        isc_throw(SocketConfigError, "Failed to set close-on-exec flag"
+                  << " on the socket " << sock);
+    }
+
     // Create socket filter program. This program will only allow incoming UDP
     // traffic which arrives on the specific (DHCP) port). It will also filter
     // out all fragmented packets.

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -44,6 +44,7 @@ using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
+using namespace isc::dhcp::test;
 using boost::scoped_ptr;
 
 namespace {
@@ -1284,7 +1285,7 @@ TEST_F(Pkt6Test, getMAC_DOCSIS_Modem) {
 
     // Let's use a captured traffic. The one we have comes from a
     // modem with MAC address 10:0d:7f:00:07:88.
-    Pkt6Ptr pkt = isc::test::PktCaptures::captureDocsisRelayedSolicit();
+    Pkt6Ptr pkt = PktCaptures::captureDocsisRelayedSolicit();
     ASSERT_NO_THROW(pkt->unpack());
 
     // The method should return MAC based on the vendor-specific info,
@@ -1312,7 +1313,7 @@ TEST_F(Pkt6Test, getMAC_DOCSIS_CMTS) {
 
     // Let's use a captured traffic. The one we have comes from a
     // modem with MAC address 20:e5:2a:b8:15:14.
-    Pkt6Ptr pkt = isc::test::PktCaptures::captureeRouterRelayedSolicit();
+    Pkt6Ptr pkt = PktCaptures::captureeRouterRelayedSolicit();
     ASSERT_NO_THROW(pkt->unpack());
 
     // The method should return MAC based on the vendor-specific info,

src/lib/dhcp/tests/pkt_captures.h

@@ -12,10 +12,14 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#ifndef PKT_CAPTURES_H
+#define PKT_CAPTURES_H
+
 #include <dhcp/pkt4.h>
 #include <dhcp/pkt6.h>
 
 namespace isc {
+namespace dhcp {
 namespace test {
 
 class PktCaptures {
@@ -69,5 +73,8 @@ protected:
     static void captureSetDefaultFields(const isc::dhcp::Pkt4Ptr& pkt);
 };
 
-};
-};
+}; // end of namespace isc::dhcp::test
+}; // end of namespace isc::dhcp
+}; // end of namespace isc
+
+#endif

src/lib/dhcp/tests/pkt_captures4.cc

@@ -45,6 +45,7 @@ using namespace isc::dhcp;
 using namespace isc::asiolink;
 
 namespace isc {
+namespace dhcp {
 namespace test {
 
 Pkt4Ptr PktCaptures::packetFromCapture(const std::string& hex_string) {
@@ -185,5 +186,6 @@ Bootstrap Protocol
     return (packetFromCapture(hex_string));
 }
 
-}; // end of isc::test namespace
+}; // end of isc::dhcp::test namespace
+}; // end of isc::dhcp namespace
 }; // end of isc namespace

src/lib/dhcp/tests/pkt_captures6.cc

@@ -45,6 +45,7 @@ using namespace isc::asiolink;
 using namespace std;
 
 namespace isc {
+namespace dhcp {
 namespace test {
 
 void PktCaptures::captureSetDefaultFields(const Pkt6Ptr& pkt) {
@@ -107,7 +108,7 @@ Pkt6Ptr PktCaptures::captureRelayedSolicit() {
 }
 
 /// returns a buffer with relayed SOLICIT (from DOCSIS3.0 cable modem)
-Pkt6Ptr isc::test::PktCaptures::captureDocsisRelayedSolicit() {
+Pkt6Ptr PktCaptures::captureDocsisRelayedSolicit() {
 
     // This is an actual DOCSIS packet
     // RELAY-FORW (12)
@@ -169,7 +170,7 @@ Pkt6Ptr isc::test::PktCaptures::captureDocsisRelayedSolicit() {
 }
 
 /// returns a buffer with relayed SOLICIT (from DOCSIS3.0 eRouter)
-Pkt6Ptr isc::test::PktCaptures::captureeRouterRelayedSolicit() {
+Pkt6Ptr PktCaptures::captureeRouterRelayedSolicit() {
 
 /* Packet description exported from wireshark:
 DHCPv6
@@ -304,7 +305,7 @@ DHCPv6
     return (pkt);
 }
 
-Pkt6Ptr isc::test::PktCaptures::captureCableLabsShortVendorClass() {
+Pkt6Ptr PktCaptures::captureCableLabsShortVendorClass() {
     // This is a simple non-relayed Solicit:
     // - client-identifier
     // - IA_NA
@@ -364,7 +365,7 @@ Pkt6Ptr isc::test::PktCaptures::captureCableLabsShortVendorClass() {
 /// The original capture was posted to dibbler users mailing list.
 ///
 /// @return created double relayed SOLICIT message
-isc::dhcp::Pkt6Ptr isc::test::PktCaptures::captureRelayed2xRSOO() {
+isc::dhcp::Pkt6Ptr PktCaptures::captureRelayed2xRSOO() {
 
     // string exported from Wireshark
     string hex_string =
@@ -399,6 +400,6 @@ isc::dhcp::Pkt6Ptr isc::test::PktCaptures::captureRelayed2xRSOO() {
     return (pkt);
 }
 
-
-}; // end of isc::test namespace
+}; // end of isc::dhcp::test namespace
+}; // end of isc::dhcp namespace
 }; // end of isc namespace

src/lib/dhcpsrv/alloc_engine.cc

@@ -247,7 +247,8 @@ AllocEngine::RandomAllocator::pickAddress(const SubnetPtr&,
 
 AllocEngine::AllocEngine(AllocType engine_type, uint64_t attempts,
                          bool ipv6)
-    : attempts_(attempts) {
+    : attempts_(attempts), incomplete_v4_reclamations_(0),
+      incomplete_v6_reclamations_(0) {
 
     // Choose the basic (normal address) lease type
     Lease::Type basic_type = ipv6 ? Lease::TYPE_NA : Lease::TYPE_V4;
@@ -936,6 +937,17 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
         prefix_len = 128; // non-PD lease types must be always /128
     }
 
+    if ( (expired->state_ == Lease::STATE_DECLINED) &&
+         (ctx.fake_allocation_ == false)) {
+        // If this is a declined lease that expired, we need to conduct
+        // extra steps for it. However, we do want to conduct those steps
+        // only once. In particular, if we have an expired declined lease
+        // and client sent DHCPDISCOVER and will later send DHCPREQUEST,
+        // we only want to call this method once when responding to
+        // DHCPREQUEST (when the actual reclaimation takes place).
+        reclaimDeclined(expired);
+    }
+
     // address, lease type and prefixlen (0) stay the same
     expired->iaid_ = ctx.iaid_;
     expired->duid_ = ctx.duid_;
@@ -978,11 +990,13 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_SELECT_SKIP);
             return (Lease6Ptr());
         }
 
+        /// @todo: Add support for DROP status
+
         // Let's use whatever callout returned. Hopefully it is the same lease
         // we handed to it.
         ctx.callout_handle_->getArgument("lease6", expired);
@@ -1040,7 +1054,7 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_SELECT_SKIP);
             return (Lease6Ptr());
         }
@@ -1245,12 +1259,14 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
         // Callouts decided to skip the next processing step. The next
         // processing step would actually renew the lease, so skip at this
         // stage means "keep the old lease as it is".
-        if (callout_handle->getSkip()) {
+        if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             skip = true;
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
                       DHCPSRV_HOOK_LEASE6_EXTEND_SKIP)
                 .arg(ctx.query_->getName());
         }
+
+        /// @todo: Add support for DROP status
     }
 
     if (!skip) {
@@ -1288,7 +1304,8 @@ AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases
 
 void
 AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeout,
-                                   const bool remove_lease) {
+                                   const bool remove_lease,
+                                   const uint16_t max_unwarned_cycles) {
 
     LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
               ALLOC_ENGINE_V6_LEASES_RECLAMATION_START)
@@ -1301,8 +1318,31 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
 
     LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
 
+    // This value indicates if we have been able to deal with all expired
+    // leases in this pass.
+    bool incomplete_reclamation = false;
     Lease6Collection leases;
-    lease_mgr.getExpiredLeases6(leases, max_leases);
+    // The value of 0 has a special meaning - reclaim all.
+    if (max_leases > 0) {
+        // If the value is non-zero, the caller has limited the number of
+        // leases to reclaim. We obtain one lease more to see if there will
+        // be still leases left after this pass.
+        lease_mgr.getExpiredLeases6(leases, max_leases + 1);
+        // There are more leases expired leases than we will process in this
+        // pass, so we should mark it as an incomplete reclamation. We also
+        // remove this extra lease (which we don't want to process anyway)
+        // from the collection.
+        if (leases.size() > max_leases) {
+            leases.pop_back();
+            incomplete_reclamation = true;
+        }
+
+    } else {
+        // If there is no limitation on the number of leases to reclaim,
+        // we will try to process all. Hence, we don't mark it as incomplete
+        // reclamation just yet.
+        lease_mgr.getExpiredLeases6(leases, max_leases);
+    }
 
     // Do not initialize the callout handle until we know if there are any
     // lease6_expire callouts installed.
@@ -1329,9 +1369,12 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
                 HooksManager::callCallouts(Hooks.hook_index_lease6_expire_,
                                            *callout_handle);
 
-                skipped = callout_handle->getSkip();
+                skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
             }
 
+            /// @todo: Maybe add support for DROP stauts?
+            /// Not sure if we need to support every possible status everywhere.
+
             if (!skipped) {
                 // Generate removal name change request for D2, if required.
                 // This will return immediatelly if the DNS wasn't updated
@@ -1340,9 +1383,25 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
                     queueRemovalNameChangeRequest(lease, *(lease->duid_));
                 }
 
+                // Let's check if the lease that just expired is in DECLINED state.
+                // If it is, we need to conduct couple extra steps and also force
+                // its removal.
+                bool remove_tmp = remove_lease;
+                if (lease->state_ == Lease::STATE_DECLINED) {
+                    // There's no point in keeping declined lease after its
+                    // reclaimation. Declined lease doesn't have any client
+                    // identifying information anymore.
+                    remove_tmp = true;
+
+                    // Do extra steps required for declined lease reclaimation:
+                    // - bump decline-related stats
+                    // - log separate message
+                    reclaimDeclined(lease);
+                }
+
                 // Reclaim the lease - depending on the configuration, set the
                 // expired-reclaimed state or simply remove it.
-                reclaimLeaseInDatabase<Lease6Ptr>(lease, remove_lease,
+                reclaimLeaseInDatabase<Lease6Ptr>(lease, remove_tmp,
                                                   boost::bind(&LeaseMgr::updateLease6,
                                                               &lease_mgr, _1));
             }
@@ -1388,6 +1447,15 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
         // return if we have. We're checking it here, because we always want to
         // allow reclaiming at least one lease.
         if ((timeout > 0) && (stopwatch.getTotalMilliseconds() >= timeout)) {
+            // Timeout. This will likely mean that we haven't been able to process
+            // all leases we wanted to process. The reclamation pass will be
+            // probably marked as incomplete.
+            if (!incomplete_reclamation) {
+                if (leases_processed < leases.size()) {
+                    incomplete_reclamation = true;
+                }
+            }
+
             LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
                       ALLOC_ENGINE_V6_LEASES_RECLAMATION_TIMEOUT)
                 .arg(timeout);
@@ -1403,11 +1471,57 @@ AllocEngine::reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeo
               ALLOC_ENGINE_V6_LEASES_RECLAMATION_COMPLETE)
         .arg(leases_processed)
         .arg(stopwatch.logFormatTotalDuration());
+
+    // Check if this was an incomplete reclamation and increase the number of
+    // consecutive incomplete reclamations.
+    if (incomplete_reclamation) {
+        ++incomplete_v6_reclamations_;
+        // If the number of incomplete reclamations is beyond the threshold, we
+        // need to issue a warning.
+        if ((max_unwarned_cycles > 0) &&
+            (incomplete_v6_reclamations_ > max_unwarned_cycles)) {
+            LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V6_LEASES_RECLAMATION_SLOW)
+                .arg(max_unwarned_cycles);
+            // We issued a warning, so let's now reset the counter.
+            incomplete_v6_reclamations_ = 0;
+        }
+
+    } else {
+        // This was a complete reclamation, so let's reset the counter.
+        incomplete_v6_reclamations_ = 0;
+
+        LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+                  ALLOC_ENGINE_V6_NO_MORE_EXPIRED_LEASES);
+    }
 }
 
 void
+AllocEngine::deleteExpiredReclaimedLeases6(const uint32_t secs) {
+    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+              ALLOC_ENGINE_V6_RECLAIMED_LEASES_DELETE)
+        .arg(secs);
+
+    uint64_t deleted_leases = 0;
+    try {
+        // Try to delete leases from the lease database.
+        LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
+        deleted_leases = lease_mgr.deleteExpiredReclaimedLeases6(secs);
+
+    } catch (const std::exception& ex) {
+        LOG_ERROR(alloc_engine_logger, ALLOC_ENGINE_V6_RECLAIMED_LEASES_DELETE_FAILED)
+            .arg(ex.what());
+    }
+
+    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+              ALLOC_ENGINE_V6_RECLAIMED_LEASES_DELETE_COMPLETE)
+        .arg(deleted_leases);
+}
+
+
+void
 AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeout,
-                                   const bool remove_lease) {
+                                   const bool remove_lease,
+                                   const uint16_t max_unwarned_cycles) {
 
     LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
               ALLOC_ENGINE_V4_LEASES_RECLAMATION_START)
@@ -1420,8 +1534,32 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
 
     LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
 
+    // This value indicates if we have been able to deal with all expired
+    // leases in this pass.
+    bool incomplete_reclamation = false;
     Lease4Collection leases;
-    lease_mgr.getExpiredLeases4(leases, max_leases);
+    // The value of 0 has a special meaning - reclaim all.
+    if (max_leases > 0) {
+        // If the value is non-zero, the caller has limited the number of
+        // leases to reclaim. We obtain one lease more to see if there will
+        // be still leases left after this pass.
+        lease_mgr.getExpiredLeases4(leases, max_leases + 1);
+        // There are more leases expired leases than we will process in this
+        // pass, so we should mark it as an incomplete reclamation. We also
+        // remove this extra lease (which we don't want to process anyway)
+        // from the collection.
+        if (leases.size() > max_leases) {
+            leases.pop_back();
+            incomplete_reclamation = true;
+        }
+
+    } else {
+        // If there is no limitation on the number of leases to reclaim,
+        // we will try to process all. Hence, we don't mark it as incomplete
+        // reclamation just yet.
+        lease_mgr.getExpiredLeases4(leases, max_leases);
+    }
+
 
     // Do not initialize the callout handle until we know if there are any
     // lease4_expire callouts installed.
@@ -1448,9 +1586,12 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
                 HooksManager::callCallouts(Hooks.hook_index_lease4_expire_,
                                            *callout_handle);
 
-                skipped = callout_handle->getSkip();
+                skipped = callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP;
             }
 
+            /// @todo: Maybe add support for DROP stauts?
+            /// Not sure if we need to support every possible status everywhere.
+
             if (!skipped) {
                 // Generate removal name change request for D2, if required.
                 // This will return immediatelly if the DNS wasn't updated
@@ -1517,8 +1658,17 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
         // return if we have. We're checking it here, because we always want to
         // allow reclaiming at least one lease.
         if ((timeout > 0) && (stopwatch.getTotalMilliseconds() >= timeout)) {
+            // Timeout. This will likely mean that we haven't been able to process
+            // all leases we wanted to process. The reclamation pass will be
+            // probably marked as incomplete.
+            if (!incomplete_reclamation) {
+                if (leases_processed < leases.size()) {
+                    incomplete_reclamation = true;
+                }
+            }
+
             LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
-                      ALLOC_ENGINE_V6_LEASES_RECLAMATION_TIMEOUT)
+                      ALLOC_ENGINE_V4_LEASES_RECLAMATION_TIMEOUT)
                 .arg(timeout);
             break;
         }
@@ -1532,6 +1682,50 @@ AllocEngine::reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeo
               ALLOC_ENGINE_V4_LEASES_RECLAMATION_COMPLETE)
         .arg(leases_processed)
         .arg(stopwatch.logFormatTotalDuration());
+
+    // Check if this was an incomplete reclamation and increase the number of
+    // consecutive incomplete reclamations.
+    if (incomplete_reclamation) {
+        ++incomplete_v4_reclamations_;
+        // If the number of incomplete reclamations is beyond the threshold, we
+        // need to issue a warning.
+        if ((max_unwarned_cycles > 0) &&
+            (incomplete_v4_reclamations_ > max_unwarned_cycles)) {
+            LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V4_LEASES_RECLAMATION_SLOW)
+                .arg(max_unwarned_cycles);
+            // We issued a warning, so let's now reset the counter.
+            incomplete_v4_reclamations_ = 0;
+        }
+
+    } else {
+        // This was a complete reclamation, so let's reset the counter.
+        incomplete_v4_reclamations_ = 0;
+
+        LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+                  ALLOC_ENGINE_V4_NO_MORE_EXPIRED_LEASES);
+    }
+}
+
+void
+AllocEngine::deleteExpiredReclaimedLeases4(const uint32_t secs) {
+    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+              ALLOC_ENGINE_V4_RECLAIMED_LEASES_DELETE)
+        .arg(secs);
+
+    uint64_t deleted_leases = 0;
+    try {
+        // Try to delete leases from the lease database.
+        LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
+        deleted_leases = lease_mgr.deleteExpiredReclaimedLeases4(secs);
+
+    } catch (const std::exception& ex) {
+        LOG_ERROR(alloc_engine_logger, ALLOC_ENGINE_V4_RECLAIMED_LEASES_DELETE_FAILED)
+            .arg(ex.what());
+    }
+
+    LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
+              ALLOC_ENGINE_V4_RECLAIMED_LEASES_DELETE_COMPLETE)
+        .arg(deleted_leases);
 }
 
 void
@@ -1565,6 +1759,37 @@ AllocEngine::reclaimDeclined(const Lease4Ptr& lease) {
     /// @todo: call lease4_decline_recycle hook here.
 }
 
+void
+AllocEngine::reclaimDeclined(const Lease6Ptr& lease) {
+
+    if (!lease || (lease->state_ != Lease::STATE_DECLINED) ) {
+        return;
+    }
+
+    LOG_INFO(alloc_engine_logger, ALLOC_ENGINE_V6_DECLINED_RECOVERED)
+        .arg(lease->addr_.toText())
+        .arg(lease->valid_lft_);
+
+    StatsMgr& stats_mgr = StatsMgr::instance();
+
+    // Decrease subnet specific counter for currently declined addresses
+    stats_mgr.addValue(StatsMgr::generateName("subnet", lease->subnet_id_,
+        "declined-addresses"), static_cast<int64_t>(-1));
+
+    // Decrease global counter for declined addresses
+    stats_mgr.addValue("declined-addresses", static_cast<int64_t>(-1));
+
+    stats_mgr.addValue("reclaimed-declined-addresses", static_cast<int64_t>(1));
+
+    stats_mgr.addValue(StatsMgr::generateName("subnet", lease->subnet_id_,
+        "reclaimed-declined-addresses"), static_cast<int64_t>(1));
+
+    // Note that we do not touch assigned-addresses counters. Those are
+    // modified in whatever code calls this method.
+
+    /// @todo: call lease6_decline_recycle hook here.
+}
+
 template<typename LeasePtrType, typename IdentifierType>
 void
 AllocEngine::queueRemovalNameChangeRequest(const LeasePtrType& lease,
@@ -2146,7 +2371,7 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr) {
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
             return (Lease4Ptr());
         }
@@ -2234,11 +2459,13 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
         // Callouts decided to skip the next processing step. The next
         // processing step would actually renew the lease, so skip at this
         // stage means "keep the old lease as it is".
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             skip = true;
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
                       DHCPSRV_HOOK_LEASE4_RENEW_SKIP);
         }
+
+        /// @todo: Add support for DROP status
     }
 
     if (!ctx.fake_allocation_ && !skip) {
@@ -2269,7 +2496,7 @@ AllocEngine::reuseExpiredLease4(Lease4Ptr& expired,
          (ctx.fake_allocation_ == false)) {
         // If this is a declined lease that expired, we need to conduct
         // extra steps for it. However, we do want to conduct those steps
-        // only once. In paricular, if we have an expired declined lease
+        // only once. In particular, if we have an expired declined lease
         // and client sent DHCPDISCOVER and will later send DHCPREQUEST,
         // we only want to call this method once when responding to
         // DHCPREQUEST (when the actual reclaimation takes place).
@@ -2312,12 +2539,14 @@ AllocEngine::reuseExpiredLease4(Lease4Ptr& expired,
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (ctx.callout_handle_->getSkip()) {
+        if (ctx.callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
             LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
                       DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
             return (Lease4Ptr());
         }
 
+        /// @todo: add support for DROP
+
         // Let's use whatever callout returned. Hopefully it is the same lease
         // we handed to it.
         ctx.callout_handle_->getArgument("lease4", expired);

src/lib/dhcpsrv/alloc_engine.h

@@ -505,14 +505,49 @@ public:
     ///   "expired-reclaimed" or removing it from the lease databse,
     /// - updating statistics of assigned and reclaimed leases
     ///
+    /// Note: declined leases fall under the same expiration/reclaimation
+    /// processing as normal leases. In principle, it would be more elegant
+    /// to have a separate processing for declined leases reclaimation. However,
+    /// due to performance reasons we decided to use them together. Several
+    /// aspects were taken into consideration. First, normal leases are expected
+    /// to expire frequently, so in a typical deployment this method will have
+    /// some leases to process. Second, declined leases are expected to be very
+    /// rare event, so in most cases there won't be any declined expired leases.
+    /// Third, the calls to LeaseMgr to obtain all leases of specific expiration
+    /// criteria are expensive, so it is better to have one call rather than
+    /// two, especially if one of those calls is expected to usually return no
+    /// leases.
+    ///
+    /// It doesn't make sense to retain declined leases that are reclaimed,
+    /// because those leases don't contain any useful information (all client
+    /// identifying information was stripped when the leave was moved to the
+    /// declined state). Therefore remove_leases parameter is ignored for
+    /// declined leases. They are always removed.
+    ///
+    /// Also, for declined leases @ref reclaimDeclined is called. It conducts
+    /// several declined specific operation (extra log entry, stats dump,
+    /// hooks).
+    ///
     /// @param max_leases Maximum number of leases to be reclaimed.
     /// @param timeout Maximum amount of time that the reclaimation routine
     /// may be processing expired leases, expressed in milliseconds.
     /// @param remove_lease A boolean value indicating if the lease should
     /// be removed when it is reclaimed (if true) or it should be left in the
     /// database in the "expired-reclaimed" state (if false).
+    /// @param max_unwarned_cycles A number of consecutive processing cycles
+    /// of expired leases, after which the system issues a warning if there
+    /// are still expired leases in the database. If this value is 0, the
+    /// warning is never issued.
     void reclaimExpiredLeases6(const size_t max_leases, const uint16_t timeout,
-                               const bool remove_lease);
+                               const bool remove_lease,
+                               const uint16_t max_unwarned_cycles = 0);
+
+    /// @brief Deletes reclaimed leases expired more than specified amount
+    /// of time ago.
+    ///
+    /// @param secs Minimum number of seconds after which the lease can be
+    /// deleted.
+    void deleteExpiredReclaimedLeases6(const uint32_t secs);
 
     /// @brief Reclaims expired IPv4 leases.
     ///
@@ -529,7 +564,7 @@ public:
     /// - updating statistics of assigned and reclaimed leases
     ///
     /// Note: declined leases fall under the same expiration/reclaimation
-    /// processing as normal leases. In principle, it would more elegant
+    /// processing as normal leases. In principle, it would be more elegant
     /// to have a separate processing for declined leases reclaimation. However,
     /// due to performance reasons we decided to use them together. Several
     /// aspects were taken into consideration. First, normal leases are expected
@@ -547,7 +582,7 @@ public:
     /// declined state). Therefore remove_leases parameter is ignored for
     /// declined leases. They are always removed.
     ///
-    /// Also, for delined leases @ref reclaimDeclined is called. It conducts
+    /// Also, for declined leases @ref reclaimDeclined is called. It conducts
     /// several declined specific operation (extra log entry, stats dump,
     /// hooks).
     ///
@@ -557,8 +592,21 @@ public:
     /// @param remove_lease A boolean value indicating if the lease should
     /// be removed when it is reclaimed (if true) or it should be left in the
     /// database in the "expired-reclaimed" state (if false).
+    /// @param max_unwarned_cycles A number of consecutive processing cycles
+    /// of expired leases, after which the system issues a warning if there
+    /// are still expired leases in the database. If this value is 0, the
+    /// warning is never issued.
     void reclaimExpiredLeases4(const size_t max_leases, const uint16_t timeout,
-                               const bool remove_lease);
+                               const bool remove_lease,
+                               const uint16_t max_unwarned_cycles = 0);
+
+    /// @brief Deletes reclaimed leases expired more than specified amount
+    /// of time ago.
+    ///
+    /// @param secs Minimum number of seconds after which the lease can be
+    /// deleted.
+    void deleteExpiredReclaimedLeases4(const uint32_t secs);
+
 
     /// @brief Attempts to find appropriate host reservation.
     ///
@@ -772,7 +820,7 @@ private:
                                 const boost::function<void (const LeasePtrType&)>&
                                 lease_update_fun) const;
 
-    /// @brief Conducts steps necessary for reclaiming declined lease.
+    /// @brief Conducts steps necessary for reclaiming declined IPv4 lease.
     ///
     /// These are the additional steps required when recoving a declined lease:
     /// - bump decline recovered stat
@@ -782,6 +830,16 @@ private:
     /// @param lease Lease to be reclaimed from Declined state
     void reclaimDeclined(const Lease4Ptr& lease);
 
+    /// @brief Conducts steps necessary for reclaiming declined IPv6 lease.
+    ///
+    /// These are the additional steps required when recoving a declined lease:
+    /// - bump decline recovered stat
+    /// - log lease recovery
+    /// - call hook (@todo)
+    ///
+    /// @param lease Lease to be reclaimed from Declined state
+    void reclaimDeclined(const Lease6Ptr& lease);
+
 public:
 
     /// @brief Context information for the DHCPv4 lease allocation.
@@ -1220,6 +1278,16 @@ private:
     /// otherwise.
     bool conditionalExtendLifetime(Lease& lease) const;
 
+private:
+
+    /// @brief Number of consecutive DHCPv4 leases' reclamations after
+    /// which there are still expired leases in the database.
+    uint16_t incomplete_v4_reclamations_;
+
+    /// @brief Number of consecutive DHCPv6 leases' reclamations after
+    /// which there are still expired leases in the database.
+    uint16_t incomplete_v6_reclamations_;
+
 };
 
 /// @brief A pointer to the @c AllocEngine object.

src/lib/dhcpsrv/alloc_engine_messages.mes

@@ -48,7 +48,14 @@ consider reducing the lease lifetime.  In this way, addresses allocated
 to clients that are no longer active on the network will become available
 sooner.
 
-% ALLOC_ENGINE_V4_DECLINED_RECOVERED Address %1 was recovered after %2 seconds of probation-period
+% ALLOC_ENGINE_V4_DECLINED_RECOVERED IPv4 address %1 was recovered after %2 seconds of probation-period
+This informational message indicates that the specified address was reported
+as duplicate (client sent DECLINE) and the server marked this address as
+unvailable for a period of time. This time now has elapsed and the address
+has been returned to the available pool. This step concludes decline recovery
+process.
+
+% ALLOC_ENGINE_V6_DECLINED_RECOVERED IPv6 address %1 was recovered after %2 seconds of probation-period
 This informational message indicates that the specified address was reported
 as duplicate (client sent DECLINE) and the server marked this address as
 unvailable for a period of time. This time now has elapsed and the address
@@ -83,7 +90,23 @@ the number of reclaimed leases may also be limited by the timeout
 value, configured with 'max-reclaim-time'. The message includes the
 number of reclaimed leases and the total time.
 
-% ALLOC_ENGINE_V4_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 seconds)
+% ALLOC_ENGINE_V4_LEASES_RECLAMATION_SLOW expired leases still exist after %1 reclamations
+This warning message is issued when the server has been unable to
+reclaim all expired leases in a specified number of consecutive
+attempts. This indicates that the value of "reclaim-timer-wait-time"
+may be too high. However, if this is just a short burst of leases'
+expirations the value does not have to be modified and the server
+should deal with this in subsequent reclamation attempts. If this
+is a result of a permanent increase of the server load, the value
+of "reclaim-timer-wait-time" should be decreased, or the
+values of "max-reclaim-leases" and "max-reclaim-time" should be
+increased to allow processing more leases in a single cycle.
+Alternatively, these values may be set to 0 to remove the
+limitations on the number of leases and duration. However, this
+may result in longer periods of server's unresponsiveness to
+DHCP packets, while it processes the expired leases.
+
+% ALLOC_ENGINE_V4_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 milliseconds)
 This debug message is issued when the allocation engine starts the
 reclamation of the expired leases. The maximum number of leases to
 be reclaimed and the timeout is included in the message. If any of
@@ -97,6 +120,10 @@ been reclaimed, because of the timeout, will be reclaimed when the
 next scheduled reclamation is started. The argument is the timeout
 value expressed in milliseconds.
 
+% ALLOC_ENGINE_V4_NO_MORE_EXPIRED_LEASES all expired leases have been reclaimed
+This debug message is issued when the server reclaims all expired
+DHCPv4 leases in the database.
+
 % ALLOC_ENGINE_V4_OFFER_EXISTING_LEASE allocation engine will try to offer existing lease to the client %1
 This message is issued when the allocation engine determines that
 the client has a lease in the lease database, it doesn't have
@@ -118,6 +145,25 @@ offer the lease specified in the hint. This situation may occur
 when: (a) client doesn't have any reservations, (b) client has
 reservation but the reserved address is leased to another client.
 
+% ALLOC_ENGINE_V4_RECLAIMED_LEASES_DELETE begin deletion of reclaimed leases expired more than %1 seconds ago
+This debug message is issued when the allocation engine begins
+deletion of the reclaimed leases which have expired more than
+a specified number of seconds ago. This operation is triggered
+periodically according to the "flush-reclaimed-timer-wait-time"
+parameter. The "hold-reclaimed-time" parameter defines a number
+of seconds for which the leases are stored before they are
+removed.
+
+% ALLOC_ENGINE_V4_RECLAIMED_LEASES_DELETE_COMPLETE successfully deleted %1 expired-reclaimed leases
+This debug message is issued when the server successfully deletes
+"expired-reclaimed" leases from the lease database. The number of
+deleted leases is included in the log message.
+
+% ALLOC_ENGINE_V4_RECLAIMED_LEASES_DELETE_FAILED deletion of expired-reclaimed leases failed: %1
+This error message is issued when the deletion of "expired-reclaimed"
+leases from the database failed. The error message is appended to
+the log message.
+
 % ALLOC_ENGINE_V4_REQUEST_ADDRESS_RESERVED %1: requested address %2 is reserved
 This message is issued when the allocation engine refused to
 allocate address requested by the client because this
@@ -149,7 +195,7 @@ This message is issued when the client is requesting or has a
 reservation for an address which is in use. The first argument
 includes the client identification information.
 
-% ALLOC_ENGINE_V4_REQUEST_OUT_OF_POOL client %1, which doesn't have reservation, requested address out of the dynamic pool
+% ALLOC_ENGINE_V4_REQUEST_OUT_OF_POOL client %1, which doesn't have a reservation, requested address %2 out of the dynamic pool
 This message is issued when the client has requested allocation
 of the address which doesn't belong to any address pool from
 which addresses are dynamically allocated. The client also
@@ -309,7 +355,23 @@ the number of reclaimed leases may also be limited by the timeout
 value, configured with 'max-reclaim-time'. The message includes the
 number of reclaimed leases and the total time.
 
-% ALLOC_ENGINE_V6_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 seconds)
+% ALLOC_ENGINE_V6_LEASES_RECLAMATION_SLOW expired leases still exist after %1 reclamations
+This warning message is issued when the server has been unable to
+reclaim all expired leases in a specified number of consecutive
+attempts. This indicates that the value of "reclaim-timer-wait-time"
+may be too high. However, if this is just a short burst of leases'
+expirations the value does not have to be modified and the server
+should deal with this in subsequent reclamation attempts. If this
+is a result of a permanent increase of the server load, the value
+of "reclaim-timer-wait-time" should be decreased, or the
+values of "max-reclaim-leases" and "max-reclaim-time" should be
+increased to allow processing more leases in a single cycle.
+Alternatively, these values may be set to 0 to remove the
+limitations on the number of leases and duration. However, this
+may result in longer periods of server's unresponsiveness to
+DHCP packets, while it processes the expired leases.
+
+% ALLOC_ENGINE_V6_LEASES_RECLAMATION_START starting reclamation of expired leases (limit = %1 leases or %2 milliseconds)
 This debug message is issued when the allocation engine starts the
 reclamation of the expired leases. The maximum number of leases to
 be reclaimed and the timeout is included in the message. If any of
@@ -323,6 +385,29 @@ been reclaimed, because of the timeout, will be reclaimed when the
 next scheduled reclamation is started. The argument is the timeout
 value expressed in milliseconds.
 
+% ALLOC_ENGINE_V6_NO_MORE_EXPIRED_LEASES all expired leases have been reclaimed
+This debug message is issued when the server reclaims all expired
+DHCPv6 leases in the database.
+
+% ALLOC_ENGINE_V6_RECLAIMED_LEASES_DELETE begin deletion of reclaimed leases expired more than %1 seconds ago
+This debug message is issued when the allocation engine begins
+deletion of the reclaimed leases which have expired more than
+a specified number of seconds ago. This operation is triggered
+periodically according to the "flush-reclaimed-timer-wait-time"
+parameter. The "hold-reclaimed-time" parameter defines a number
+of seconds for which the leases are stored before they are
+removed.
+
+% ALLOC_ENGINE_V6_RECLAIMED_LEASES_DELETE_COMPLETE successfully deleted %1 expired-reclaimed leases
+This debug message is issued when the server successfully deletes
+"expired-reclaimed" leases from the lease database. The number of
+deleted leases is included in the log message.
+
+% ALLOC_ENGINE_V6_RECLAIMED_LEASES_DELETE_FAILED deletion of expired-reclaimed leases failed: %1
+This error message is issued when the deletion of "expired-reclaimed"
+leases from the database failed. The error message is appended to
+the log message.
+
 % ALLOC_ENGINE_V6_RENEW_HR allocating leases reserved for the client %1 as a result of Renew
 This debug message is issued when the allocation engine tries to
 allocate reserved leases for the client sending a Renew message.

src/lib/dhcpsrv/cfg_expiration.cc

@@ -40,13 +40,22 @@ const uint16_t CfgExpiration::LIMIT_MAX_RECLAIM_TIME = 10000;
 const uint16_t CfgExpiration::LIMIT_UNWARNED_RECLAIM_CYCLES =
     std::numeric_limits<uint16_t>::max();
 
-CfgExpiration::CfgExpiration()
+// Timers' names
+const std::string CfgExpiration::RECLAIM_EXPIRED_TIMER_NAME =
+    "reclaim-expired-leases";
+
+const std::string CfgExpiration::FLUSH_RECLAIMED_TIMER_NAME =
+    "flush-reclaimed-leases";
+
+CfgExpiration::CfgExpiration(const bool test_mode)
     : reclaim_timer_wait_time_(DEFAULT_RECLAIM_TIMER_WAIT_TIME),
-    flush_reclaimed_timer_wait_time_(DEFAULT_FLUSH_RECLAIMED_TIMER_WAIT_TIME),
-    hold_reclaimed_time_(DEFAULT_HOLD_RECLAIMED_TIME),
-    max_reclaim_leases_(DEFAULT_MAX_RECLAIM_LEASES),
-    max_reclaim_time_(DEFAULT_MAX_RECLAIM_TIME),
-    unwarned_reclaim_cycles_(DEFAULT_UNWARNED_RECLAIM_CYCLES) {
+      flush_reclaimed_timer_wait_time_(DEFAULT_FLUSH_RECLAIMED_TIMER_WAIT_TIME),
+      hold_reclaimed_time_(DEFAULT_HOLD_RECLAIMED_TIME),
+      max_reclaim_leases_(DEFAULT_MAX_RECLAIM_LEASES),
+      max_reclaim_time_(DEFAULT_MAX_RECLAIM_TIME),
+      unwarned_reclaim_cycles_(DEFAULT_UNWARNED_RECLAIM_CYCLES),
+      timer_mgr_(TimerMgr::instance()),
+      test_mode_(test_mode) {
 }
 
 void
@@ -102,7 +111,5 @@ CfgExpiration::rangeCheck(const int64_t value, const uint64_t max_value,
     }
 }
 
-
-
 }
 }

src/lib/dhcpsrv/cfg_expiration.h

@@ -15,6 +15,9 @@
 #ifndef CFG_EXPIRATION_H
 #define CFG_EXPIRATION_H
 
+#include <asiolink/interval_timer.h>
+#include <dhcpsrv/timer_mgr.h>
+#include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 #include <stdint.h>
 #include <string>
@@ -111,10 +114,26 @@ public:
 
     //@}
 
+    /// @name Timers' names
+    //@{
+
+    /// @brief Name of the timer for reclaiming expired leases.
+    static const std::string RECLAIM_EXPIRED_TIMER_NAME;
+
+    /// @brief Name of the timer for flushing relclaimed leases.
+    static const std::string FLUSH_RECLAIMED_TIMER_NAME;
+
+    //@}
+
     /// @brief Constructor.
     ///
     /// Sets all parameters to their defaults.
-    CfgExpiration();
+    ///
+    /// @param test_mode Indicates if the instance should be created in the
+    /// test mode. In this mode the intervals for the timers are considered to
+    /// be specified in milliseconds, rather than seconds. This facilitates
+    /// testing execution of timers without the delays.
+    CfgExpiration(const bool test_mode = false);
 
     /// @brief Returns reclaim-timer-wait-time
     uint16_t getReclaimTimerWaitTime() const {
@@ -176,6 +195,42 @@ public:
     /// @param unwarned_reclaim_cycles New value.
     void setUnwarnedReclaimCycles(const int64_t unwarned_reclaim_cycles);
 
+    /// @brief Setup timers for the reclamation of expired leases according
+    /// to the configuration parameters.
+    ///
+    /// This method includes the logic for setting the interval timers
+    /// performing the reclamation of the expired leases and the removal
+    /// of expired-reclaimed leases.
+    ///
+    /// The following is the sample code illustrating how to call this function
+    /// to setup the leases reclamation for the DHCPv4 server.
+    /// @code
+    ///     CfgExpiration cfg;
+    ///
+    ///     (set some cfg values here)
+    ///
+    ///     AllocEnginePtr alloc_engine(new AllocEngine(...));
+    ///     cfg.setupTimers(&AllocEngine::reclaimExpiredLeases4,
+    ///                     &AllocEngine::deleteExpiredReclaimedLeases4,
+    ///                     alloc_engine.get());
+    /// @endcode
+    ///
+    /// @param reclaim_fun Pointer to the leases reclamation routine.
+    /// @param delete_fun Pointer to the function which removes the
+    /// expired-reclaimed leases from the lease database.
+    /// @param instance_ptr Pointer to the instance of the object which
+    /// implements the lease reclamation routine. Typically it will be
+    /// the pointer to the @c AllocEngine. In case of unit tests it
+    /// will be a pointer to some test class which provides stub
+    /// implementation of the leases reclamation routines.
+    /// @tparam Instance Instance of the object in which both functions
+    /// are implemented.
+    template<typename Instance>
+    void setupTimers(void (Instance::*reclaim_fun)(const size_t, const uint16_t,
+                                                   const bool, const uint16_t),
+                     void (Instance::*delete_fun)(const uint32_t),
+                     Instance* instance_ptr) const;
+
 private:
 
     /// @brief Checks if the value being set by one of the modifiers is
@@ -209,6 +264,11 @@ private:
     /// @brief unwarned-reclaim-cycles.
     uint16_t unwarned_reclaim_cycles_;
 
+    /// @brief Pointer to the instance of the Timer Manager.
+    TimerMgrPtr timer_mgr_;
+
+    /// @brief Indicates if the instance is in the test mode.
+    bool test_mode_;
 };
 
 /// @name Pointers to the @c CfgExpiration objects.
@@ -221,6 +281,59 @@ typedef boost::shared_ptr<const CfgExpiration> ConstCfgExpirationPtr;
 
 //@}
 
+template<typename Instance>
+void
+CfgExpiration::setupTimers(void (Instance::*reclaim_fun)(const size_t,
+                                                         const uint16_t,
+                                                         const bool,
+                                                         const uint16_t),
+                           void (Instance::*delete_fun)(const uint32_t),
+                           Instance* instance_ptr) const {
+    // One of the parameters passed to the leases' reclamation routine
+    // is a boolean value which indicates if reclaimed leases should
+    // be removed by the leases' reclamation routine. This is the case
+    // when the timer for flushing reclaimed leases is set to 0
+    // (disabled).
+    const bool flush_timer_disabled = (getFlushReclaimedTimerWaitTime() == 0);
+
+    // If the timer interval for the leases reclamation is non-zero
+    // the timer will be scheduled.
+    if (getReclaimTimerWaitTime() > 0) {
+        // In the test mode the interval is expressed in milliseconds.
+        // If this is not the test mode, the interval is in seconds.
+        const long reclaim_interval = test_mode_ ? getReclaimTimerWaitTime() :
+            1000 * getReclaimTimerWaitTime();
+        // Register timer for leases' reclamation routine.
+        timer_mgr_->registerTimer(RECLAIM_EXPIRED_TIMER_NAME,
+                                  boost::bind(reclaim_fun, instance_ptr,
+                                              getMaxReclaimLeases(),
+                                              getMaxReclaimTime(),
+                                              flush_timer_disabled,
+                                              getUnwarnedReclaimCycles()),
+                                  reclaim_interval,
+                                  asiolink::IntervalTimer::ONE_SHOT);
+        timer_mgr_->setup(RECLAIM_EXPIRED_TIMER_NAME);
+    }
+
+    // If the interval for the timer flusing expired-reclaimed leases
+    // is set we will schedule the timer.
+    if (!flush_timer_disabled) {
+        // The interval is specified in milliseconds if we're in the test mode.
+        // It is specified in seconds otherwise.
+        const long flush_interval = test_mode_ ?
+            getFlushReclaimedTimerWaitTime() :
+            1000 * getFlushReclaimedTimerWaitTime();
+        // Register and setup the timer.
+        timer_mgr_->registerTimer(FLUSH_RECLAIMED_TIMER_NAME,
+                                  boost::bind(delete_fun, instance_ptr,
+                                              getHoldReclaimedTime()),
+                                  flush_interval,
+                                  asiolink::IntervalTimer::ONE_SHOT);
+        timer_mgr_->setup(FLUSH_RECLAIMED_TIMER_NAME);
+    }
+}
+
+
 } // end of isc::dhcp namespace
 } // end of isc namespace
 

src/lib/dhcpsrv/cfg_subnets4.cc

@@ -39,6 +39,12 @@ CfgSubnets4::add(const Subnet4Ptr& subnet) {
 
 Subnet4Ptr
 CfgSubnets4::selectSubnet(const SubnetSelector& selector) const {
+    // First use RAI link select sub-option or subnet select option
+    if (!selector.option_select_.isV4Zero()) {
+        return (selectSubnet(selector.option_select_,
+                             selector.client_classes_));
+    }
+
     // If relayed message has been received, try to match the giaddr with the
     // relay address specified for a subnet. It is also possible that the relay
     // address will not match with any of the relay addresses accross all

src/lib/dhcpsrv/cfg_subnets4.h

@@ -61,6 +61,9 @@ public:
     /// parameters extracted from the client's message using the following
     /// logic.
     ///
+    /// First when link select suboption of relay agent information option
+    /// or subnet select option in this order exists the address is used
+    ///
     /// If the giaddr value is set in the selector it means that the client's
     /// message was relayed. The subnet configuration allows for setting the
     /// relay address for each subnet to indicate that the subnet must be

src/lib/dhcpsrv/subnet_selector.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -35,6 +35,8 @@ struct SubnetSelector {
     asiolink::IOAddress ciaddr_;
     /// @brief giaddr from the client's message.
     asiolink::IOAddress giaddr_;
+    /// @brief RAI link select or subnet select option
+    asiolink::IOAddress option_select_;
     //@}
 
     /// @name DHCPv6 specific parameters.
@@ -60,6 +62,7 @@ struct SubnetSelector {
     SubnetSelector()
         : ciaddr_(asiolink::IOAddress("0.0.0.0")),
           giaddr_(asiolink::IOAddress("0.0.0.0")),
+          option_select_(asiolink::IOAddress("0.0.0.0")),
           interface_id_(),
           first_relay_linkaddr_(asiolink::IOAddress("::")),
           local_address_(asiolink::IOAddress("0.0.0.0")),

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc

@@ -1385,7 +1385,7 @@ TEST_F(AllocEngine6Test, reserved2Addresses) {
     AllocEngine::ClientContext6 ctx3(subnet_, duid_, iaid_ + 1, IOAddress("::"),
                                     pool_->getType(), false, false, "", false);
     ctx3.query_.reset(new Pkt6(DHCPV6_REQUEST, 1234));
-    
+
     Lease6Collection leases3;
     findReservation(engine, ctx3);
     EXPECT_NO_THROW(leases3 = engine.allocateLeases6(ctx3));
@@ -1655,6 +1655,161 @@ TEST_F(AllocEngine6Test, largeAllocationAttemptsOverride) {
     ASSERT_EQ(1, leases.size());
 }
 
+// This test checks if an expired declined lease can be reused in SOLICIT (fake allocation)
+TEST_F(AllocEngine6Test, solicitReuseDeclinedLease6) {
+
+    AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
+    ASSERT_TRUE(engine);
+
+    // Now prepare a configuration with single address pool.
+    // Create one subnet with a pool holding one address.
+    string addr_txt("2001:db8:1::ad");
+    IOAddress addr(addr_txt);
+    initSubnet(IOAddress("2001:db8:1::"), addr, addr);
+
+    // Use information that is different than what we'll request
+    Lease6Ptr declined = generateDeclinedLease(addr_txt, 100, -10);
+    ASSERT_TRUE(declined->expired());
+
+    // CASE 1: Asking for any address
+    Lease6Ptr assigned;
+    testReuseLease6(engine, declined, "::", true, SHOULD_PASS, assigned);
+
+    // Check that we got that single lease
+    ASSERT_TRUE(assigned);
+    EXPECT_EQ(addr, assigned->addr_);
+
+    // Do all checks on the lease (if subnet-id, preferred/valid times are ok etc.)
+    checkLease6(assigned, Lease::TYPE_NA, 128);
+
+    // CASE 2: Asking specifically for this address
+    testReuseLease6(engine, declined, addr_txt, true, SHOULD_PASS, assigned);
+
+    // Check that we got that single lease
+    ASSERT_TRUE(assigned);
+    EXPECT_EQ(addr, assigned->addr_);
+}
+
+// This test checks if an expired declined lease can be reused when responding
+// to REQUEST (actual allocation)
+TEST_F(AllocEngine6Test, requestReuseDeclinedLease6) {
+
+    AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100, true));
+    ASSERT_TRUE(engine);
+
+    // Now prepare a configuration with single address pool.
+    string addr_txt("2001:db8::7");
+    IOAddress addr(addr_txt);
+    initSubnet(IOAddress("2001:db8::"), addr, addr);
+
+    // Now create a declined lease, decline it and rewind its cltt, so it
+    // is expired.
+    Lease6Ptr declined = generateDeclinedLease(addr_txt, 100, -10);
+
+    // Asking specifically for this address
+    Lease6Ptr assigned;
+    testReuseLease6(engine, declined, addr_txt, false, SHOULD_PASS, assigned);
+    // Check that we got it.
+    ASSERT_TRUE(assigned);
+    EXPECT_EQ(addr, assigned->addr_);
+
+    // Check that the lease is indeed updated in LeaseMgr
+    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
+                                                               addr);
+    ASSERT_TRUE(from_mgr);
+
+    // Now check that the lease in LeaseMgr has the same parameters
+    detailCompareLease(assigned, from_mgr);
+}
+
+// This test checks if statistics are not updated when expired declined lease
+// is reused when responding to SOLICIT (fake allocation)
+TEST_F(AllocEngine6Test, solicitReuseDeclinedLease6Stats) {
+
+    // Now prepare for SOLICIT processing
+    AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
+                                          100, true));
+    ASSERT_TRUE(engine);
+
+    // Now prepare a configuration with single address pool.
+    string addr_txt("2001:db8:1::1");
+    IOAddress addr(addr_txt);
+    initSubnet(IOAddress("2001:db8:1::"), addr, addr);
+
+    // Now create a declined lease, decline it and rewind its cltt, so it
+    // is expired.
+    Lease6Ptr declined = generateDeclinedLease(addr_txt, 100, -10);
+
+    // Let's fix some global stats...
+    StatsMgr& stats_mgr = StatsMgr::instance();
+    stats_mgr.setValue("declined-addresses", static_cast<int64_t>(1000));
+    stats_mgr.setValue("reclaimed-declined-addresses", static_cast<int64_t>(1000));
+
+    // ...and subnet specific stats as well.
+    string stat1 = StatsMgr::generateName("subnet", subnet_->getID(),
+                                          "declined-addresses");
+    string stat2 = StatsMgr::generateName("subnet", subnet_->getID(),
+                                          "reclaimed-declined-addresses");
+    stats_mgr.setValue(stat1, static_cast<int64_t>(1000));
+    stats_mgr.setValue(stat2, static_cast<int64_t>(1000));
+
+    // Ask for any address. There's only one address in the pool, so it doesn't
+    // matter much.
+    Lease6Ptr assigned;
+    testReuseLease6(engine, declined, "::", true, SHOULD_PASS, assigned);
+
+    // Check that the stats were not modified
+    testStatistics("declined-addresses", 1000);
+    testStatistics("reclaimed-declined-addresses", 1000);
+
+    testStatistics(stat1, 1000);
+    testStatistics(stat2, 1000);
+}
+
+// This test checks if statistics are not updated when expired declined lease
+// is reused when responding to REQUEST (actual allocation)
+TEST_F(AllocEngine6Test, requestReuseDeclinedLease6Stats) {
+
+    // Prepare for REQUEST processing.
+    AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
+                                          100, true));
+    ASSERT_TRUE(engine);
+
+    // Now prepare a configuration with single address pool.
+    string addr_txt("2001:db8::1");
+    IOAddress addr(addr_txt);
+    initSubnet(IOAddress("2001:db8::"), addr, addr);
+
+    // Now create a declined lease, decline it and rewind its cltt, so it
+    // is expired.
+    Lease6Ptr declined = generateDeclinedLease(addr_txt, 100, -10);
+
+    // Let's fix some global stats...
+    StatsMgr& stats_mgr = StatsMgr::instance();
+    stats_mgr.setValue("declined-addresses", static_cast<int64_t>(1000));
+    stats_mgr.setValue("reclaimed-declined-addresses", static_cast<int64_t>(1000));
+
+    // ...and subnet specific stats as well.
+    string stat1 = StatsMgr::generateName("subnet", subnet_->getID(),
+                                          "declined-addresses");
+    string stat2 = StatsMgr::generateName("subnet", subnet_->getID(),
+                                          "reclaimed-declined-addresses");
+    stats_mgr.setValue(stat1, static_cast<int64_t>(1000));
+    stats_mgr.setValue(stat2, static_cast<int64_t>(1000));
+
+    // Ask for any address. There's only one address in the pool, so it doesn't
+    // matter much.
+    Lease6Ptr assigned;
+    testReuseLease6(engine, declined, "::", false, SHOULD_PASS, assigned);
+
+    // Check that the stats were not modified
+    testStatistics("declined-addresses", 999);
+    testStatistics("reclaimed-declined-addresses", 1001);
+
+    testStatistics(stat1, 999);
+    testStatistics(stat2, 1001);
+}
+
 }; // namespace test
 }; // namespace dhcp
 }; // namespace isc

src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc

@@ -250,6 +250,20 @@ public:
         ASSERT_NO_THROW(updateLease(lease_index));
     }
 
+    /// @brief Marks lease as expired-reclaimed.
+    ///
+    /// @param lease_index Lease index. Must be between 0 and
+    /// @c TEST_LEASES_NUM.
+    /// @param secs Offset of the expiration time since now. For example
+    /// a value of 2 would set the lease expiration time to 2 seconds ago.
+    void reclaim(const uint16_t lease_index, const time_t secs) {
+        ASSERT_GT(leases_.size(), lease_index);
+        leases_[lease_index]->cltt_ = time(NULL) - secs -
+            leases_[lease_index]->valid_lft_;
+        leases_[lease_index]->state_ = Lease::STATE_EXPIRED_RECLAIMED;
+        ASSERT_NO_THROW(updateLease(lease_index));
+    }
+
     /// @brief Declines specified lease
     ///
     /// Sets specified lease to declined state and sets its probation-period.
@@ -291,6 +305,13 @@ public:
                                       const uint16_t timeout,
                                       const bool remove_lease) = 0;
 
+    /// @brief Wrapper method for removing expired-reclaimed leases.
+    ///
+    /// @param secs The minimum amount of time, expressed in seconds,
+    /// for the lease to be left in the "expired-reclaimed" state
+    /// before it can be removed.
+    virtual void deleteExpiredReclaimedLeases(const uint32_t secs) = 0;
+
     /// @brief Test selected leases using the specified algorithms.
     ///
     /// This function picks leases from the range of 0 thru
@@ -497,26 +518,32 @@ public:
     /// @return Zero.
     static int leaseExpireWithSkipCallout(CalloutHandle& callout_handle) {
         leaseExpireCallout(callout_handle);
-        callout_handle.setSkip(true);
+        callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
         return (0);
     }
 
     /// @brief Implements "lease{4,6}_expire callout, which lasts at least
-    /// 2ms.
+    /// 40ms.
     ///
     /// This callout is useful to test scenarios where the reclamation of the
     /// lease needs to take a known amount of time. If the callout is installed
-    /// it will take at least 2ms for each lease. It is then possible to calculate
+    /// it will take at least 40ms for each lease. It is then possible to calculate
     /// the approximate time that the reclamation of all leases would take and
     /// test that the timeouts for the leases' reclamation work as expected.
     ///
+    /// The value of 40ms is relatively high, but it has been selected to
+    /// mitigate the problems with usleep on some virtual machines. On those
+    /// machines the wakeup from usleep may take significant amount of time,
+    /// i.e. usually around 10ms. Thus, the sleep time should be considerably
+    /// higher than this delay.
+    ///
     /// @param callout_handle Callout handle.
     /// @return Zero.
     static int leaseExpireWithDelayCallout(CalloutHandle& callout_handle) {
         leaseExpireCallout(callout_handle);
-        // Delay the return from the callout by 2ms.
-        usleep(2000);
+        // Delay the return from the callout by 40ms.
+        usleep(40000);
 
         return (0);
     }
@@ -575,7 +602,7 @@ public:
 
     /// @brief Test that leases can be reclaimed without being removed.
     void testReclaimExpiredLeasesUpdateState() {
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             // Mark leases with even indexes as expired.
             if (evenLeaseIndex(i)) {
                 // The higher the index, the more expired the lease.
@@ -596,7 +623,7 @@ public:
 
     /// @brief Test that the leases may be reclaimed by being deleted.
     void testReclaimExpiredLeasesDelete() {
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             // Mark leases with even indexes as expired.
             if (evenLeaseIndex(i)) {
                 // The higher the index, the more expired the lease.
@@ -618,7 +645,7 @@ public:
     /// @brief Test that it is possible to specify the limit for the number
     /// of reclaimed leases.
     void testReclaimExpiredLeasesLimit() {
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             // Mark all leaes as expired. The higher the index the less
             // expired the lease.
             expire(i, 1000 - i);
@@ -631,7 +658,7 @@ public:
         BOOST_STATIC_ASSERT(TEST_LEASES_NUM % reclamation_group_size == 0);
 
         // Leases will be reclaimed in groups of 10.
-        for (int i = reclamation_group_size; i < TEST_LEASES_NUM;
+        for (unsigned int i = reclamation_group_size; i < TEST_LEASES_NUM;
              i += reclamation_group_size) {
 
             // Reclaim 10 most expired leases out of TEST_LEASES_NUM. Since
@@ -659,7 +686,7 @@ public:
         // DNS must be started for the D2 client to accept NCRs.
         ASSERT_NO_THROW(enableDDNS());
 
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             // Expire all leases with even indexes.
             if (evenLeaseIndex(i)) {
                 // The higher the index, the more expired the lease.
@@ -688,7 +715,7 @@ public:
         // DNS must be started for the D2 client to accept NCRs.
         ASSERT_NO_THROW(enableDDNS());
 
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             // Expire only leases with even indexes.
             if (evenLeaseIndex(i)) {
                 // The higher the index, the more expired the lease.
@@ -700,7 +727,7 @@ public:
         BOOST_STATIC_ASSERT(TEST_LEASES_NUM % reclamation_group_size == 0);
 
         // Leases will be reclaimed in groups of 10
-        for (int i = 10; i < TEST_LEASES_NUM;  i += reclamation_group_size) {
+        for (unsigned int i = 10; i < TEST_LEASES_NUM;  i += reclamation_group_size) {
             // Reclaim 10 most expired leases. Note that the leases with the
             // higher index are more expired. For example, if the
             // TEST_LEASES_NUM is equal to 100, the most expired lease will
@@ -806,7 +833,7 @@ public:
     /// @brief This test verfies that callouts are executed for each expired
     /// lease when installed.
     void testReclaimExpiredLeasesHooks() {
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             if (evenLeaseIndex(i)) {
                 expire(i, 1000 - i);
             }
@@ -836,7 +863,7 @@ public:
     /// @brief This test verfies that callouts are executed for each expired
     /// lease and that the lease is not reclaimed when skip flag is set.
     void testReclaimExpiredLeasesHooksWithSkip() {
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             if (evenLeaseIndex(i)) {
                 expire(i, 1000 - i);
             }
@@ -866,7 +893,7 @@ public:
     /// the execution of the lease reclamation routine.
     void testReclaimExpiredLeasesTimeout(const uint16_t timeout) {
         // Leases are segregated from the most expired to the least expired.
-        for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
             expire(i, 2000 - i);
         }
 
@@ -874,7 +901,7 @@ public:
         HooksManager::loadLibraries(libraries);
 
         // Install a callout: lease4_expire or lease6_expire. Each callout
-        // takes at least 2ms to run (it uses usleep).
+        // takes at least 40ms to run (it uses usleep).
         std::ostringstream callout_name;
         callout_name << callout_argument_name << "_expire";
         EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -883,8 +910,8 @@ public:
         // Reclaim leases with timeout.
         ASSERT_NO_THROW(reclaimExpiredLeases(0, timeout, false));
 
-        // We reclaimed at most (timeout / 2ms) leases.
-        const uint16_t theoretical_reclaimed = static_cast<uint16_t>(timeout / 2);
+        // We reclaimed at most (timeout / 40ms) leases.
+        const uint16_t theoretical_reclaimed = static_cast<uint16_t>(timeout / 40);
 
         // The actual number of leases reclaimed is likely to be lower than
         // the theoretical number. For low theoretical number the adjusted
@@ -905,16 +932,38 @@ public:
                                UpperBound(TEST_LEASES_NUM)));
     }
 
+    /// @brief This test verifies that expired-reclaimed leases are removed
+    /// from the lease database.
+    void testDeleteExpiredReclaimedLeases() {
+        for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
+            // Mark leases with even indexes as expired.
+            if (evenLeaseIndex(i)) {
+                // The higher the index, the more expired the lease.
+                reclaim(i, 10 + i);
+            }
+        }
+
+        // Run leases reclamation routine on all leases. This should result
+        // in removal of all leases with even indexes.
+        ASSERT_NO_THROW(deleteExpiredReclaimedLeases(10));
+
+        // Leases with odd indexes shouldn't be removed from the database.
+        EXPECT_TRUE(testLeases(&leaseExists, &oddLeaseIndex));
+        // Leases with even indexes should have been removed.
+        EXPECT_TRUE(testLeases(&leaseDoesntExist, &evenLeaseIndex));
+    }
+
     /// @brief Test that declined expired leases can be removed.
     ///
     /// This method allows controlling remove_leases parameter when calling
-    /// @ref AllocEngine::reclaimExpiredLeases4. This should not matter, as
+    /// @ref AllocEngine::reclaimExpiredLeases4 or
+    /// @ref AllocEngine::reclaimExpiredLeases6. This should not matter, as
     /// the address affinity doesn't make sense for declined leases (they don't
     /// have any useful information in them anymore), so AllocEngine should
     /// remove them all the time.
     ///
     /// @param remove see description above
-    void testReclaimDeclined4(bool remove) {
+    void testReclaimDeclined(bool remove) {
         for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
 
             // Mark leases with even indexes as expired.
@@ -940,7 +989,15 @@ public:
 
     /// @brief Test that appropriate statistics are updated when
     ///        declined expired leases are processed by AllocEngine.
-    void testReclaimDeclined4Stats() {
+    ///
+    /// This method works for both v4 and v6. Just make sure the correct
+    /// statistic name is passed. This is the name of the assigned addresses,
+    /// that is expected to be decreased once the reclaimation procedure
+    /// is complete.
+    ///
+    /// @param stat_name name of the statistic for assigned addresses statistic
+    ///        ("assgined-addresses" for both v4 and "assigned-nas" for v6)
+    void testReclaimDeclinedStats(const std::string& stat_name) {
 
         // Leases by default all belong to subnet_id_ = 1. Let's count the
         // number of declined leases.
@@ -966,6 +1023,7 @@ public:
         }
 
         StatsMgr& stats_mgr = StatsMgr::instance();
+
         // Let's set the global statistic. Values are arbitrary and can
         // be used to easily detect whether a given stat was decreased or
         // increased. They are sufficiently high compared to number of leases
@@ -978,19 +1036,19 @@ public:
 
         // And those subnet specific as well
         stats_mgr.setValue(stats_mgr.generateName("subnet", 1,
-                           "assigned-addresses"), int64_t(1000));
+                           stat_name), int64_t(1000));
         stats_mgr.setValue(stats_mgr.generateName("subnet", 2,
-                           "assigned-addresses"), int64_t(2000));
+                           stat_name), int64_t(2000));
 
         stats_mgr.setValue(stats_mgr.generateName("subnet", 1,
-                           "reclaimed-declined-addresses"), int64_t(3000));
+                           "reclaimed-declined-addresses"), int64_t(10000));
         stats_mgr.setValue(stats_mgr.generateName("subnet", 2,
-                           "reclaimed-declined-addresses"), int64_t(4000));
+                           "reclaimed-declined-addresses"), int64_t(20000));
 
         stats_mgr.setValue(stats_mgr.generateName("subnet", 1,
-                           "declined-addresses"), int64_t(10));
+                           "declined-addresses"), int64_t(100));
         stats_mgr.setValue(stats_mgr.generateName("subnet", 2,
-                           "declined-addresses"), int64_t(20));
+                           "declined-addresses"), int64_t(200));
 
         // Run leases reclamation routine on all leases. This should result
         // in removal of all leases with even indexes.
@@ -1005,17 +1063,22 @@ public:
         testStatistics("reclaimed-declined-addresses", 2000 + subnet1_cnt + subnet2_cnt);
 
         // subnet[X].assigned-addresses should go down. Between the time
-        // of DHCPDECLINE reception and declined expired lease reclaimation,
-        // we count this address as assigned-addresses. We decrease assigned-
-        // addresses when we reclaim the lease, not when the packet is received.
-        // For explanation, see Duplicate Addresses (DHCPDECLINE support)
-        // section in the User's Guide or a comment in Dhcpv4Srv::declineLease.
-        testStatistics("subnet[1].assigned-addresses", 1000 - subnet1_cnt);
-        testStatistics("subnet[2].assigned-addresses", 2000 - subnet2_cnt);
+        // of DHCPDECLINE(v4)/DECLINE(v6) reception and declined expired lease
+        // reclaimation, we count this address as assigned-addresses. We decrease
+        // assigned-addresses(v4)/assgined-nas(v6) when we reclaim the lease,
+        // not when the packet is received. For explanation, see Duplicate
+        // Addresses (DHCPDECLINE support) (v4) or Duplicate Addresses (DECLINE
+        // support) sections in the User's Guide or a comment in
+        // Dhcpv4Srv::declineLease or Dhcpv6Srv::declineLease.
+        testStatistics("subnet[1]." + stat_name, 1000 - subnet1_cnt);
+        testStatistics("subnet[2]." + stat_name, 2000 - subnet2_cnt);
+
+        testStatistics("subnet[1].declined-addresses", 100 - subnet1_cnt);
+        testStatistics("subnet[2].declined-addresses", 200 - subnet2_cnt);
 
         // subnet[X].reclaimed-declined-addresses should go up in each subnet
-        testStatistics("subnet[1].reclaimed-declined-addresses", 3000 + subnet1_cnt);
-        testStatistics("subnet[2].reclaimed-declined-addresses", 4000 + subnet1_cnt);
+        testStatistics("subnet[1].reclaimed-declined-addresses", 10000 + subnet1_cnt);
+        testStatistics("subnet[2].reclaimed-declined-addresses", 20000 + subnet1_cnt);
     }
 
     /// @brief Collection of leases created at construction time.
@@ -1086,6 +1149,15 @@ public:
         engine_->reclaimExpiredLeases6(max_leases, timeout, remove_lease);
     }
 
+    /// @brief Wrapper method for removing expired-reclaimed leases.
+    ///
+    /// @param secs The minimum amount of time, expressed in seconds,
+    /// for the lease to be left in the "expired-reclaimed" state
+    /// before it can be removed.
+    virtual void deleteExpiredReclaimedLeases(const uint32_t secs) {
+        engine_->deleteExpiredReclaimedLeases6(secs);
+    }
+
     /// @brief Test that statistics is updated when leases are reclaimed.
     void testReclaimExpiredLeasesStats();
 
@@ -1174,7 +1246,7 @@ ExpirationAllocEngine6Test::testReclaimExpiredLeasesStats() {
     // This test requires that the number of leases is an even number.
     BOOST_STATIC_ASSERT(TEST_LEASES_NUM % 2 == 0);
 
-    for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
         // Mark all leaes as expired. The higher the index the less
         // expired the lease.
         expire(i, 1000 - i);
@@ -1187,7 +1259,7 @@ ExpirationAllocEngine6Test::testReclaimExpiredLeasesStats() {
 
     // Leases will be reclaimed in groups of 8.
     const size_t reclamation_group_size = 8;
-    for (int i = reclamation_group_size; i < TEST_LEASES_NUM;
+    for (unsigned int i = reclamation_group_size; i < TEST_LEASES_NUM;
          i += reclamation_group_size) {
 
         // Reclaim 8 most expired leases out of TEST_LEASES_NUM.
@@ -1267,8 +1339,8 @@ TEST_F(ExpirationAllocEngine6Test, reclaimExpiredLeasesHooksWithSkip) {
 TEST_F(ExpirationAllocEngine6Test, reclaimExpiredLeasesTimeout) {
     // This test needs at least 40 leases to make sense.
     BOOST_STATIC_ASSERT(TEST_LEASES_NUM >= 40);
-    // Run with timeout of 60ms.
-    testReclaimExpiredLeasesTimeout(60);
+    // Run with timeout of 1.2s.
+    testReclaimExpiredLeasesTimeout(1200);
 }
 
 // This test verifies that at least one lease is reclaimed if the timeout
@@ -1283,6 +1355,34 @@ TEST_F(ExpirationAllocEngine6Test, reclaimExpiredLeasesShortTimeout) {
     testReclaimExpiredLeasesTimeout(1);
 }
 
+// This test verifies that expired-reclaimed leases are removed from the
+// lease database.
+TEST_F(ExpirationAllocEngine6Test, deleteExpiredReclaimedLeases) {
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM >= 10);
+    testDeleteExpiredReclaimedLeases();
+}
+
+/// This test verifies that @ref AllocEngine::reclaimExpiredLeases6 properly
+/// handles declined leases that have expired in case when it is told to
+/// remove leases.}
+TEST_F(ExpirationAllocEngine6Test, reclaimDeclined1) {
+    testReclaimDeclined(true);
+}
+
+/// This test verifies that @ref AllocEngine::reclaimExpiredLeases6 properly
+/// handles declined leases that have expired in case when it is told to
+/// not remove leases. This flag should not matter and declined expired
+/// leases should always be removed.
+TEST_F(ExpirationAllocEngine6Test, reclaimDeclined2) {
+    testReclaimDeclined(false);
+}
+
+/// This test verifies that statistics are modified correctly after
+/// reclaim expired leases is called.
+TEST_F(ExpirationAllocEngine6Test, reclaimDeclinedStats) {
+    testReclaimDeclinedStats("assigned-nas");
+}
+
 // *******************************************************
 //
 // DHCPv4 lease reclamation routine tests start here!
@@ -1346,6 +1446,15 @@ public:
         engine_->reclaimExpiredLeases4(max_leases, timeout, remove_lease);
     }
 
+    /// @brief Wrapper method for removing expired-reclaimed leases.
+    ///
+    /// @param secs The minimum amount of time, expressed in seconds,
+    /// for the lease to be left in the "expired-reclaimed" state
+    /// before it can be removed.
+    virtual void deleteExpiredReclaimedLeases(const uint32_t secs) {
+        engine_->deleteExpiredReclaimedLeases4(secs);
+    }
+
     /// @brief Lease algorithm checking if NCR has been generated from client
     /// identifier.
     ///
@@ -1503,7 +1612,7 @@ ExpirationAllocEngine4Test::testReclaimExpiredLeasesWithDDNSAndClientId() {
     // DNS must be started for the D2 client to accept NCRs.
     ASSERT_NO_THROW(enableDDNS());
 
-    for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
         // Set client identifiers for leases with even indexes only.
         if (evenLeaseIndex(i)) {
             setUniqueClientId(i);
@@ -1532,7 +1641,7 @@ ExpirationAllocEngine4Test::testReclaimExpiredLeasesStats() {
     // This test requires that the number of leases is an even number.
     BOOST_STATIC_ASSERT(TEST_LEASES_NUM % 2 == 0);
 
-    for (int i = 0; i < TEST_LEASES_NUM; ++i) {
+    for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
         // Mark all leaes as expired. The higher the index the less
         // expired the lease.
         expire(i, 1000 - i);
@@ -1544,7 +1653,7 @@ ExpirationAllocEngine4Test::testReclaimExpiredLeasesStats() {
 
     // Leases will be reclaimed in groups of 8.
     const size_t reclamation_group_size = 8;
-    for (int i = reclamation_group_size; i < TEST_LEASES_NUM;
+    for (unsigned int i = reclamation_group_size; i < TEST_LEASES_NUM;
          i += reclamation_group_size) {
 
         // Reclaim 8 most expired leases out of TEST_LEASES_NUM.
@@ -1631,8 +1740,8 @@ TEST_F(ExpirationAllocEngine4Test, reclaimExpiredLeasesHooksWithSkip) {
 TEST_F(ExpirationAllocEngine4Test, reclaimExpiredLeasesTimeout) {
     // This test needs at least 40 leases to make sense.
     BOOST_STATIC_ASSERT(TEST_LEASES_NUM >= 40);
-    // Run with timeout of 60ms.
-    testReclaimExpiredLeasesTimeout(60);
+    // Run with timeout of 1.2s.
+    testReclaimExpiredLeasesTimeout(1200);
 }
 
 // This test verifies that at least one lease is reclaimed if the timeout
@@ -1647,11 +1756,18 @@ TEST_F(ExpirationAllocEngine4Test, reclaimExpiredLeasesShortTimeout) {
     testReclaimExpiredLeasesTimeout(1);
 }
 
+// This test verifies that expired-reclaimed leases are removed from the
+// lease database.
+TEST_F(ExpirationAllocEngine4Test, deleteExpiredReclaimedLeases) {
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM >= 10);
+    testDeleteExpiredReclaimedLeases();
+}
+
 /// This test verifies that @ref AllocEngine::reclaimExpiredLeases4 properly
 /// handles declined leases that have expired in case when it is told to
 /// remove leases.
 TEST_F(ExpirationAllocEngine4Test, reclaimDeclined1) {
-    testReclaimDeclined4(true);
+    testReclaimDeclined(true);
 }
 
 /// This test verifies that @ref AllocEngine::reclaimExpiredLeases4 properly
@@ -1659,13 +1775,13 @@ TEST_F(ExpirationAllocEngine4Test, reclaimDeclined1) {
 /// not remove leases. This flag should not matter and declined expired
 /// leases should always be removed.
 TEST_F(ExpirationAllocEngine4Test, reclaimDeclined2) {
-    testReclaimDeclined4(false);
+    testReclaimDeclined(false);
 }
 
 /// This test verifies that statistics are modified correctly after
 /// reclaim expired leases is called.
 TEST_F(ExpirationAllocEngine4Test, reclaimDeclinedStats) {
-    testReclaimDeclined4Stats();
+    testReclaimDeclinedStats("assigned-addresses");
 }
 
 }; // end of anonymous namespace

src/lib/dhcpsrv/tests/alloc_engine_utils.cc

@@ -59,6 +59,9 @@ bool testStatistics(const std::string& stat_name, const int64_t exp_value) {
                     <<  "doesn't match expected value (" << exp_value << ")";
             }
             return (observation->getInteger().first == exp_value);
+        } else {
+            ADD_FAILURE() << "Expected statistic " << stat_name
+                          << " not found.";
         }
 
     } catch (...) {
@@ -430,6 +433,62 @@ AllocEngine6Test::allocBogusHint6(Lease::Type type, asiolink::IOAddress hint,
 }
 
 void
+AllocEngine6Test::testReuseLease6(const AllocEnginePtr& engine,
+                                  Lease6Ptr& existing_lease,
+                                  const std::string& addr,
+                                  const bool fake_allocation,
+                                  ExpectedResult exp_result,
+                                  Lease6Ptr& result) {
+    ASSERT_TRUE(engine);
+
+    if (existing_lease) {
+        // If an existing lease was specified, we need to add it to the
+        // database. Let's wipe any leases for that address (if any). We
+        // ignore any errors (previous lease may not exist)
+        LeaseMgrFactory::instance().deleteLease(existing_lease->addr_);
+
+        // Let's add it.
+        ASSERT_TRUE(LeaseMgrFactory::instance().addLease(existing_lease));
+    }
+
+    // A client comes along, asking specifically for a given address
+    AllocEngine::ClientContext6 ctx(subnet_, duid_, iaid_, IOAddress(addr),
+                                    Lease::TYPE_NA, false, false, "", fake_allocation);
+    ctx.query_.reset(new Pkt6(fake_allocation ? DHCPV6_SOLICIT : DHCPV6_REQUEST, 1234));
+
+    Lease6Collection leases;
+
+    leases = engine->allocateLeases6(ctx);
+
+    switch (exp_result) {
+    case SHOULD_PASS:
+        ASSERT_FALSE(leases.empty());
+        ASSERT_EQ(1, leases.size());
+        result = leases[0];
+
+        checkLease6(result, Lease::TYPE_NA, 128);
+        break;
+
+    case SHOULD_FAIL:
+        ASSERT_TRUE(leases.empty());
+        break;
+    }
+}
+
+Lease6Ptr
+AllocEngine6Test::generateDeclinedLease(const std::string& addr,
+                                        time_t probation_period,
+                                        int32_t expired) {
+    Lease6Ptr declined(new Lease6(Lease::TYPE_NA, IOAddress(addr),
+                       duid_, iaid_, 100, 100, 100, 100, subnet_->getID()));
+
+    time_t now = time(NULL);
+    declined->decline(probation_period);
+    declined->cltt_ = now - probation_period + expired;
+    return (declined);
+}
+
+void
 AllocEngine4Test::initSubnet(const asiolink::IOAddress& pool_start,
                              const asiolink::IOAddress& pool_end) {
     CfgMgr& cfg_mgr = CfgMgr::instance();

src/lib/dhcpsrv/tests/alloc_engine_utils.h

@@ -88,6 +88,12 @@ public:
 class AllocEngine6Test : public ::testing::Test {
 public:
 
+    /// @brief Specified expected result of a given operation
+    enum ExpectedResult {
+        SHOULD_PASS,
+        SHOULD_FAIL
+    };
+
     /// @brief Default constructor
     ///
     /// Sets duid_, iaid_, subnet_, pool_ fields to example values used
@@ -279,6 +285,41 @@ public:
                                asiolink::IOAddress requested,
                                uint8_t expected_pd_len);
 
+    /// @brief Generic test used for IPv6 lease allocation and reuse
+    ///
+    /// This test inserts existing_lease (if specified, may be null) into the
+    /// LeaseMgr, then conducts lease allocation (pretends that client
+    /// sent either Solicit or Request, depending on fake_allocation).
+    /// Allocated lease is then returned (using result) for further inspection.
+    ///
+    /// @param alloc_engine allocation engine
+    /// @param existing_lease optional lease to be inserted in the database
+    /// @param addr address to be requested by client
+    /// @param fake_allocation true = SOLICIT, false = REQUEST
+    /// @param exp_result expected result
+    /// @param result [out] allocated lease
+    void testReuseLease6(const AllocEnginePtr& alloc_engine,
+                         Lease6Ptr& existing_lease,
+                         const std::string& addr,
+                         const bool fake_allocation,
+                         ExpectedResult exp_result,
+                         Lease6Ptr& result);
+
+    /// @brief Creates a declined IPv6 lease with specified expiration time
+    ///
+    /// expired parameter controls probation period. Positive value
+    /// means that the lease will expire in X seconds. Negative means
+    /// that the lease expired X seconds ago. 0 means it expires now.
+    /// Probation period is a parameter that specifies for how long
+    /// a lease will stay unavailable after decline.
+    ///
+    /// @param addr address of the lease
+    /// @param probation_period expressed in seconds
+    /// @param expired number of seconds when the lease will expire
+    Lease6Ptr generateDeclinedLease(const std::string& addr,
+                                    time_t probation_period,
+                                    int32_t expired);
+
     /// @brief checks if bogus hint can be ignored and the allocation succeeds
     ///
     /// This test checks if the allocation with a hing that is out of the blue
@@ -387,7 +428,7 @@ public:
         /// @todo: check cltt
     }
 
-    /// @brief Generic test used for lease allocation and reuse
+    /// @brief Generic test used for IPv4 lease allocation and reuse
     ///
     /// This test inserts existing_lease (if specified, may be null) into the
     /// LeaseMgr, then conducts lease allocation (pretends that client
@@ -407,7 +448,7 @@ public:
                          ExpectedResult exp_result,
                          Lease4Ptr& result);
 
-    /// @brief Creates a declined lease with specified expiration time
+    /// @brief Creates a declined IPv4 lease with specified expiration time
     ///
     /// expired parameter controls probation period. Positive value
     /// means that the lease will expire in X seconds. Negative means
@@ -417,7 +458,7 @@ public:
     ///
     /// @param addr address of the lease
     /// @param probation_period expressed in seconds
-    /// @param expired number of seconds where it will expire
+    /// @param expired number of seconds when the lease will expire
     Lease4Ptr generateDeclinedLease(const std::string& addr,
                                     time_t probation_period,
                                     int32_t expired);

src/lib/dhcpsrv/tests/cfg_expiration_unittest.cc

@@ -13,14 +13,19 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <config.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcpsrv/cfg_expiration.h>
+#include <dhcpsrv/timer_mgr.h>
 #include <exceptions/exceptions.h>
+#include <util/stopwatch.h>
 #include <boost/function.hpp>
+#include <boost/shared_ptr.hpp>
 #include <gtest/gtest.h>
 #include <stdint.h>
 
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 namespace {
 
@@ -158,4 +163,265 @@ TEST(CfgExpirationTest, getUnwarnedReclaimCycles) {
                            &CfgExpiration::getUnwarnedReclaimCycles);
 }
 
+/// @brief Implements test routines for leases reclamation.
+///
+/// This class implements two routines called by the @c CfgExpiration object
+/// instead of the typical routines for leases' reclamation in the
+/// @c AllocEngine. These methods do not perform the actual reclamation,
+/// but instead they record the number of calls to them and the parameters
+/// with which they were executed. This allows for checking if the
+/// @c CfgExpiration object calls the leases reclamation routine with the
+/// appropriate parameteres.
+class LeaseReclamationStub {
+public:
+
+    /// @brief Collection of parameters with which the @c reclaimExpiredLeases
+    /// method is called.
+    ///
+    /// Examination of these values allows for assesment if the @c CfgExpiration
+    /// calls the routine with the appropriate values.
+    struct RecordedParams {
+        /// @brief Maximum number of leases to be processed.
+        size_t max_leases;
+
+        /// @brief Timeout for processing leases in milliseconds.
+        uint16_t timeout;
+
+        /// @brief Boolean flag which indicates if the leases should be removed
+        /// when reclaimed.
+        bool remove_lease;
+
+        /// @brief Maximum number of reclamation attempts after which all leases
+        /// should be reclaimed.
+        uint16_t max_unwarned_cycles;
+
+        /// @brief Constructor
+        ///
+        /// Sets all numeric values to 0xFFFF and the boolean values to false.
+        RecordedParams()
+            : max_leases(0xFFFF), timeout(0xFFFF), remove_lease(false),
+              max_unwarned_cycles(0xFFFF) {
+        }
+    };
+
+    /// @brief Constructor.
+    ///
+    /// Resets recorded parameters and obtains the instance of the @c TimerMgr.
+    LeaseReclamationStub()
+        : reclaim_calls_count_(0), delete_calls_count_(0), reclaim_params_(),
+          secs_param_(0), timer_mgr_(TimerMgr::instance()) {
+    }
+
+    /// @brief Stub implementation of the leases' reclamation routine.
+    ///
+    /// @param max_leases Maximum number of leases to be processed.
+    /// @param timeout Timeout for processing leases in milliseconds.
+    /// @remove_lease Boolean flag which indicates if the leases should be
+    /// removed when it is reclaimed.
+    /// @param Maximum number of reclamation attempts after which all leases
+    /// should be reclaimed.
+    void
+    reclaimExpiredLeases(const size_t max_leases, const uint16_t timeout,
+                         const bool remove_lease,
+                         const uint16_t max_unwarned_cycles) {
+        // Increase calls counter for this method.
+        ++reclaim_calls_count_;
+        // Record all parameters with which this method has been called.
+        reclaim_params_.max_leases = max_leases;
+        reclaim_params_.timeout = timeout;
+        reclaim_params_.remove_lease = remove_lease;
+        reclaim_params_.max_unwarned_cycles = max_unwarned_cycles;
+
+        // Leases' reclamation routine is responsible for re-scheduling
+        // the timer.
+        timer_mgr_->setup(CfgExpiration::RECLAIM_EXPIRED_TIMER_NAME);
+    }
+
+    /// @brief Stub implementation of the routine which flushes
+    /// expired-reclaimed leases.
+    ///
+    /// @param secs Specifies the minimum amount of time, expressed in
+    /// seconds, that must elapse before the expired-reclaime lease is
+    /// deleted from the database.
+    void
+    deleteReclaimedLeases(const uint32_t secs) {
+        // Increase calls counter for this method.
+        ++delete_calls_count_;
+        // Record the value of the parameter.
+        secs_param_ = secs;
+
+        // Routine which flushes the reclaimed leases is responsible for
+        // re-scheduling the timer.
+        timer_mgr_->setup(CfgExpiration::FLUSH_RECLAIMED_TIMER_NAME);
+    }
+
+    /// @brief Counter holding the number of calls to @c reclaimExpiredLeases.
+    long reclaim_calls_count_;
+
+    /// @brief Counter holding the number of calls to @c deleteReclaimedLeases.
+    long delete_calls_count_;
+
+    /// @brief Structure holding values of parameters with which the
+    /// @c reclaimExpiredLeases was called.
+    ///
+    /// These values are overriden on subsequent calls to this method.
+    RecordedParams reclaim_params_;
+
+    /// @brief Value of the parameter with which the @c deleteReclaimedLeases
+    /// was called.
+    uint32_t secs_param_;
+
+private:
+
+    /// @brief Pointer to the @c TimerMgr.
+    TimerMgrPtr timer_mgr_;
+
+};
+
+/// @brief Pointer to the @c LeaseReclamationStub.
+typedef boost::shared_ptr<LeaseReclamationStub> LeaseReclamationStubPtr;
+
+/// @brief Test fixture class for the @c CfgExpiration.
+class CfgExpirationTimersTest : public ::testing::Test {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Creates instance of the test fixture class. Besides initialization
+    /// of the class members, it also stops the @c TimerMgr worker thread
+    /// and removes any registered timers.
+    CfgExpirationTimersTest()
+        : timer_mgr_(TimerMgr::instance()),
+          stub_(new LeaseReclamationStub()),
+          cfg_(true) {
+        cleanupTimerMgr();
+    }
+
+    /// @brief Destructor.
+    ///
+    /// It stops the @c TimerMgr worker thread and removes any registered
+    /// timers.
+    virtual ~CfgExpirationTimersTest() {
+        cleanupTimerMgr();
+    }
+
+    /// @brief Stop @c TimerMgr worker thread and remove the timers.
+    void cleanupTimerMgr() const {
+        timer_mgr_->stopThread();
+        timer_mgr_->unregisterTimers();
+    }
+
+    /// @brief Runs timers for specified time.
+    ///
+    /// Internally, this method calls @c IfaceMgr::receive6 to run the
+    /// callbacks for the installed timers.
+    ///
+    /// @param timeout_ms Amount of time after which the method returns.
+    void runTimersWithTimeout(const long timeout_ms) {
+        Stopwatch stopwatch;
+        while (stopwatch.getTotalMilliseconds() < timeout_ms) {
+            // Block for up to one millisecond waiting for the timers'
+            // callbacks to be executed.
+            IfaceMgr::instancePtr()->receive6(0, 1000);
+        }
+    }
+
+    /// @brief Setup timers according to the configuration and run them
+    /// for the specified amount of time.
+    ///
+    /// @param timeout_ms Timeout in milliseconds.
+    void setupAndRun(const long timeout_ms) {
+        cfg_.setupTimers(&LeaseReclamationStub::reclaimExpiredLeases,
+                         &LeaseReclamationStub::deleteReclaimedLeases,
+                         stub_.get());
+        // Run timers.
+        ASSERT_NO_THROW({
+            timer_mgr_->startThread();
+            runTimersWithTimeout(timeout_ms);
+            timer_mgr_->stopThread();
+        });
+    }
+
+    /// @brief Pointer to the @c TimerMgr.
+    TimerMgrPtr timer_mgr_;
+
+    /// @brief Pointer to the @c LeaseReclamationStub instance.
+    LeaseReclamationStubPtr stub_;
+
+    /// @brief Instance of the @c CfgExpiration class used by the tests.
+    CfgExpiration cfg_;
+};
+
+// Test that the reclamation routines are called with the appropriate parameters.
+TEST_F(CfgExpirationTimersTest, reclamationParameters) {
+    // Set this value to true, to make sure that the timer callback would
+    // modify this value to false.
+    stub_->reclaim_params_.remove_lease = true;
+
+    // Set parameters to some non-default values.
+    cfg_.setMaxReclaimLeases(1000);
+    cfg_.setMaxReclaimTime(1500);
+    cfg_.setHoldReclaimedTime(1800);
+    cfg_.setUnwarnedReclaimCycles(13);
+
+    // Run timers for 500ms.
+    ASSERT_NO_FATAL_FAILURE(setupAndRun(500));
+
+    // Make sure we had more than one call to the reclamation routine.
+    ASSERT_GT(stub_->reclaim_calls_count_, 1);
+    // Make sure it was called with appropriate arguments.
+    EXPECT_EQ(1000, stub_->reclaim_params_.max_leases);
+    EXPECT_EQ(1500, stub_->reclaim_params_.timeout);
+    EXPECT_FALSE(stub_->reclaim_params_.remove_lease);
+    EXPECT_EQ(13, stub_->reclaim_params_.max_unwarned_cycles);
+
+    // Make sure we had more than one call to the routine which flushes
+    // expired reclaimed leases.
+    ASSERT_GT(stub_->delete_calls_count_, 1);
+    // Make sure that the argument was correct.
+    EXPECT_EQ(1800, stub_->secs_param_);
+}
+
+// This test verifies that if the value of "flush-reclaimed-timer-wait-time"
+// configuration parameter is set to 0, the lease reclamation routine would
+// delete reclaimed leases from a lease database.
+TEST_F(CfgExpirationTimersTest, noLeaseAffinity) {
+    // Set the timer for flushing leases to 0. This effectively disables
+    // the timer.
+    cfg_.setFlushReclaimedTimerWaitTime(0);
+
+    // Run the lease reclamation timer for a while.
+    ASSERT_NO_FATAL_FAILURE(setupAndRun(500));
+
+    // Make sure that the lease reclamation routine has been executed a
+    // couple of times.
+    ASSERT_GT(stub_->reclaim_calls_count_, 1);
+    EXPECT_EQ(CfgExpiration::DEFAULT_MAX_RECLAIM_LEASES,
+              stub_->reclaim_params_.max_leases);
+    EXPECT_EQ(CfgExpiration::DEFAULT_MAX_RECLAIM_TIME,
+              stub_->reclaim_params_.timeout);
+    // When the "flush" timer is disabled, the lease reclamation routine is
+    // responsible for removal of reclaimed leases. This is controlled using
+    // the "remove_lease" parameter which should be set to true in this case.
+    EXPECT_TRUE(stub_->reclaim_params_.remove_lease);
+
+    // The routine flushing reclaimed leases should not be run at all.
+    EXPECT_EQ(0, stub_->delete_calls_count_);
+}
+
+// This test verfies that lease reclamation may be disabled.
+TEST_F(CfgExpirationTimersTest, noLeaseReclamation) {
+    // Disable both timers.
+    cfg_.setReclaimTimerWaitTime(0);
+    cfg_.setFlushReclaimedTimerWaitTime(0);
+
+    // Wait for 500ms.
+    ASSERT_NO_FATAL_FAILURE(setupAndRun(500));
+
+    // Make sure that neither leases' reclamation routine nor the routine
+    // flushing expired-reclaimed leases was run.
+    EXPECT_EQ(0, stub_->reclaim_calls_count_);
+    EXPECT_EQ(0, stub_->delete_calls_count_);
+}
+
 } // end of anonymous namespace

src/lib/dhcpsrv/tests/cfg_option_unittest.cc

@@ -132,7 +132,7 @@ TEST(CfgOptionTest, add) {
 }
 
 // This test verifies that two option configurations can be merged.
-TEST(CfgOption, merge) {
+TEST(CfgOptionTest, merge) {
     CfgOption cfg_src;
     CfgOption cfg_dst;
 
@@ -308,7 +308,7 @@ TEST(CfgOptionTest, encapsulate) {
 
 // This test verifies that single option can be retrieved from the configuration
 // using option code and option space.
-TEST(CfgOption, get) {
+TEST(CfgOptionTest, get) {
     CfgOption cfg;
 
     // Add 10 options to a "dhcp6" option space in the subnet.

src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc

@@ -143,9 +143,46 @@ TEST(CfgSubnets4Test, selectSubnetByClasses) {
     EXPECT_FALSE(cfg.selectSubnet(selector));
 }
 
+// This test verifies the option selection can be used and is only
+// used when present.
+TEST(CfgSubnets4Test, selectSubnetByOptionSelect) {
+    CfgSubnets4 cfg;
+
+    // Create 3 subnets.
+    Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.2.0"), 26, 1, 2, 3));
+    Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.64"), 26, 1, 2, 3));
+    Subnet4Ptr subnet3(new Subnet4(IOAddress("192.0.2.128"), 26, 1, 2, 3));
+
+    // Add them to the configuration.
+    cfg.add(subnet1);
+    cfg.add(subnet2);
+    cfg.add(subnet3);
+
+    SubnetSelector selector;
+
+    // Check that without option selection something else is used
+    selector.ciaddr_ = IOAddress("192.0.2.5");
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
+
+    // The option selection has precedence
+    selector.option_select_ = IOAddress("192.0.2.130");
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
+
+    // Over relay-info too
+    selector.giaddr_ = IOAddress("10.0.0.1");
+    subnet2->setRelayInfo(IOAddress("10.0.0.1"));
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
+    selector.option_select_ = IOAddress("0.0.0.0");
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
+
+    // Check that a not matching option selection it shall fail
+    selector.option_select_ = IOAddress("10.0.0.1");
+    EXPECT_FALSE(cfg.selectSubnet(selector));
+}
+
 // This test verifies that the relay information can be used to retrieve the
 // subnet.
-TEST(CfgSubnetsTest, selectSubnetByRelayAddress) {
+TEST(CfgSubnets4Test, selectSubnetByRelayAddress) {
     CfgSubnets4 cfg;
 
     // Create 3 subnets.
@@ -184,7 +221,7 @@ TEST(CfgSubnetsTest, selectSubnetByRelayAddress) {
 
 // This test verifies that the subnet can be selected for the client
 // using a source address if the client hasn't set the ciaddr.
-TEST(CfgSubnetsTest, selectSubnetNoCiaddr) {
+TEST(CfgSubnets4Test, selectSubnetNoCiaddr) {
     CfgSubnets4 cfg;
 
     // Create 3 subnets.
@@ -224,7 +261,7 @@ TEST(CfgSubnetsTest, selectSubnetNoCiaddr) {
 
 // This test verifies that the subnet can be selected using an address
 // set on the local interface.
-TEST(CfgSubnetsTest, selectSubnetInterface) {
+TEST(CfgSubnets4Test, selectSubnetInterface) {
     // The IfaceMgrTestConfig object initializes fake interfaces:
     // eth0, eth1 and lo on the configuration manager. The CfgSubnets4
     // object uses addresses assigned to these fake interfaces to
@@ -276,7 +313,7 @@ TEST(CfgSubnetsTest, selectSubnetInterface) {
 
 // Checks that detection of duplicated subnet IDs works as expected. It should
 // not be possible to add two IPv4 subnets holding the same ID.
-TEST(CfgSubnets4, duplication) {
+TEST(CfgSubnets4Test, duplication) {
     CfgSubnets4 cfg;
 
     Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.2.0"), 26, 1, 2, 3, 123));

src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc

@@ -337,7 +337,7 @@ TEST(CfgSubnets6Test, selectSubnetByInterfaceIdAndClassify) {
 
 // Checks that detection of duplicated subnet IDs works as expected. It should
 // not be possible to add two IPv6 subnets holding the same ID.
-TEST(CfgSubnets6, duplication) {
+TEST(CfgSubnets6Test, duplication) {
     CfgSubnets6 cfg;
 
     Subnet6Ptr subnet1(new Subnet6(IOAddress("2000::"), 48, 1, 2, 3, 4, 123));

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014, 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -2241,6 +2241,159 @@ GenericLeaseMgrTest::testGetDeclinedLeases4() {
     }
 }
 
+void
+GenericLeaseMgrTest::testGetDeclinedLeases6() {
+    // Get the leases to be used for the test.
+    vector<Lease6Ptr> leases = createLeases6();
+
+    // Make sure we have at least 8 leases there.
+    ASSERT_GE(leases.size(), 8);
+
+    // Use the same current time for all leases.
+    time_t current_time = time(NULL);
+
+    // Add them to the database
+    for (size_t i = 0; i < leases.size(); ++i) {
+
+        // Mark the first half of the leases as DECLINED
+        if (i < leases.size()/2) {
+            // Mark as declined with 1000 seconds of probation-period
+            leases[i]->decline(1000);
+        }
+
+        // Mark every second lease as expired.
+        if (i % 2 == 0) {
+            // Set client last transmission time to the value older than the
+            // valid lifetime to make it expired. The expiration time also
+            // depends on the lease index, so as we can later check that the
+            // leases are ordered by the expiration time.
+            leases[i]->cltt_ = current_time - leases[i]->valid_lft_ - 10 - i;
+
+        } else {
+            // Set current time as cltt for remaining leases. These leases are
+            // not expired.
+            leases[i]->cltt_ = current_time;
+        }
+
+        ASSERT_TRUE(lmptr_->addLease(leases[i]));
+    }
+
+    // The leases are:
+    // 0 - declined, expired
+    // 1 - declined, not expired
+    // 2 - declined, expired
+    // 3 - declined, not expired
+    // 4 - default, expired
+    // 5 - default, not expired
+    // 6 - default, expired
+    // 7 - default, not expired
+
+    // Retrieve at most 1000 expired leases.
+    Lease6Collection expired_leases;
+    ASSERT_NO_THROW(lmptr_->getExpiredLeases6(expired_leases, 1000));
+
+    // Leases with even indexes should be returned as expired. It shouldn't
+    // matter if the state is default or expired. The getExpiredLeases4 does
+    // not pay attention to state, just expiration timers.
+    ASSERT_EQ(static_cast<size_t>(leases.size() / 2), expired_leases.size());
+
+    unsigned int declined_state = 0;
+    unsigned int default_state = 0;
+
+    // The expired leases should be returned from the most to least expired.
+    // This matches the reverse order to which they have been added.
+    for (Lease6Collection::reverse_iterator lease = expired_leases.rbegin();
+         lease != expired_leases.rend(); ++lease) {
+        int index = static_cast<int>(std::distance(expired_leases.rbegin(), lease));
+        // Multiple current index by two, because only leases with even indexes
+        // should have been returned.
+        EXPECT_EQ(leases[2 * index]->addr_, (*lease)->addr_);
+
+        // Count leases in default and declined states
+        if ((*lease)->state_ == Lease::STATE_DEFAULT) {
+            default_state++;
+        } else if ((*lease)->state_ == Lease::STATE_DECLINED) {
+            declined_state++;
+        }
+    }
+
+    // LeaseMgr is supposed to return both default and declined leases
+    EXPECT_NE(0, declined_state);
+    EXPECT_NE(0, default_state);
+
+    // Update current time for the next test.
+    current_time = time(NULL);
+    // Also, remove expired leases collected during the previous test.
+    expired_leases.clear();
+
+    // This time let's reverse the expiration time and see if they will be returned
+    // in the correct order.
+    leases = createLeases6();
+    for (int i = 0; i < leases.size(); ++i) {
+
+        // Mark the second half of the leases as DECLINED
+        if (i >= leases.size()/2) {
+            // Mark as declined with 1000 seconds of probation-period
+            leases[i]->decline(1000);
+        }
+
+        // Update the time of expired leases with even indexes.
+        if (i % 2 == 0) {
+            leases[i]->cltt_ = current_time - leases[i]->valid_lft_ - 1000 + i;
+
+        } else {
+            // Make sure remaining leases remain unexpired.
+            leases[i]->cltt_ = current_time + 100;
+        }
+        ASSERT_NO_THROW(lmptr_->updateLease6(leases[i]));
+    }
+
+    // Retrieve expired leases again. The limit of 0 means return all expired
+    // leases.
+    ASSERT_NO_THROW(lmptr_->getExpiredLeases6(expired_leases, 0));
+    // The same leases should be returned.
+    ASSERT_EQ(static_cast<size_t>(leases.size() / 2), expired_leases.size());
+
+    // This time leases should be returned in the non-reverse order.
+    declined_state = 0;
+    default_state = 0;
+    for (Lease6Collection::iterator lease = expired_leases.begin();
+         lease != expired_leases.end(); ++lease) {
+        int index = static_cast<int>(std::distance(expired_leases.begin(), lease));
+        EXPECT_EQ(leases[2 * index]->addr_, (*lease)->addr_);
+
+        // Count leases in default and declined states
+        if ((*lease)->state_ == Lease::STATE_DEFAULT) {
+            default_state++;
+        } else if ((*lease)->state_ == Lease::STATE_DECLINED) {
+            declined_state++;
+        }
+    }
+
+    // Check that both declined and default state leases were returned.
+    EXPECT_NE(0, declined_state);
+    EXPECT_NE(0, default_state);
+
+    // Remember expired leases returned.
+    std::vector<Lease6Ptr> saved_expired_leases = expired_leases;
+
+    // Remove expired leases again.
+    expired_leases.clear();
+
+    // Limit the number of leases to be returned to 2.
+    ASSERT_NO_THROW(lmptr_->getExpiredLeases6(expired_leases, 2));
+
+    // Make sure we have exactly 2 leases returned.
+    ASSERT_EQ(2, expired_leases.size());
+
+    // Test that most expired leases have been returned.
+    for (Lease6Collection::iterator lease = expired_leases.begin();
+         lease != expired_leases.end(); ++lease) {
+        int index = static_cast<int>(std::distance(expired_leases.begin(), lease));
+        EXPECT_EQ(leases[2 * index]->addr_, (*lease)->addr_);
+    }
+}
+
 }; // namespace test
 }; // namespace dhcp
 }; // namespace isc

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h

@@ -250,7 +250,7 @@ public:
     /// Checks that the code is able to update an IPv6 lease in the database.
     void testUpdateLease6();
 
-    /// @brief Check that the DHCPv6 lease can be added, removed and recreated.
+    /// @brief Check that the IPv6 lease can be added, removed and recreated.
     ///
     /// This test creates a lease, removes it and then recreates it with some
     /// of the attributes changed. Next it verifies that the lease in the
@@ -275,7 +275,7 @@ public:
     /// - reclaimed leases are not returned.
     void testGetExpiredLeases4();
 
-    /// @brief Checks that the expired DHCPv6 leases can be retrieved.
+    /// @brief Checks that the expired IPv6 leases can be retrieved.
     ///
     /// This test checks the following:
     /// - all expired and not reclaimed leases are retured
@@ -285,7 +285,7 @@ public:
     /// - reclaimed leases are not returned.
     void testGetExpiredLeases6();
 
-    /// @brief Checks that declined DHCPv4 leases that have expired can be retrieved.
+    /// @brief Checks that declined IPv4 leases that have expired can be retrieved.
     ///
     /// This test checks that the following:
     /// - all expired and not reclaimed leases are returned, regardless if
@@ -295,7 +295,17 @@ public:
     ///   expired
     void testGetDeclinedLeases4();
 
-    /// @brief Checks that selected expired-reclaimed DHCPv6 leases
+    /// @brief Checks that declined IPv6 leases that have expired can be retrieved.
+    ///
+    /// This test checks that the following:
+    /// - all expired and not reclaimed leases are returned, regardless if
+    ///   they're normal or declined
+    /// - the order in which they're updated in LeaseMgr doesn't matter
+    /// - leases are returned in the order from most expired to the least
+    ///   expired
+    void testGetDeclinedLeases6();
+
+    /// @brief Checks that selected expired-reclaimed IPv6 leases
     /// are removed.
     ///
     /// This creates a number of DHCPv6 leases and marks some of them
@@ -303,7 +313,7 @@ public:
     /// leases can be removed.
     void testDeleteExpiredReclaimedLeases6();
 
-    /// @brief Checks that selected expired-reclaimed DHCPv4 leases
+    /// @brief Checks that selected expired-reclaimed IPv4 leases
     /// are removed.
     ///
     /// This creates a number of DHCPv4 leases and marks some of them

src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -992,13 +992,18 @@ TEST_F(MemfileLeaseMgrTest, versionCheck) {
     LeaseMgrFactory::destroy();
 }
 
-// Checks that declined leases can be returned correctly.
+// Checks that declined IPv4 leases can be returned correctly.
 TEST_F(MemfileLeaseMgrTest, getDeclined4) {
-
     startBackend(V4);
     testGetDeclinedLeases4();
 }
 
+// Checks that declined IPv6 leases can be returned correctly.
+TEST_F(MemfileLeaseMgrTest, getDeclined6) {
+    startBackend(V6);
+    testGetDeclinedLeases6();
+}
+
 // This test checks that the backend reads DHCPv4 lease data from multiple
 // files.
 TEST_F(MemfileLeaseMgrTest, load4MultipleLeaseFiles) {

src/lib/dhcpsrv/tests/pool_unittest.cc

@@ -119,7 +119,7 @@ TEST(Pool4Test, unique_id) {
 }
 
 // Simple check if toText returns reasonable values
-TEST(Poo4Test,toText) {
+TEST(Pool4Test,toText) {
     Pool4 pool1(IOAddress("192.0.2.7"), IOAddress("192.0.2.17"));
     EXPECT_EQ("type=V4, 192.0.2.7-192.0.2.17", pool1.toText());
 

src/lib/hooks/callout_handle.cc

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

src/lib/hooks/callout_handle.h

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

src/lib/hooks/callout_manager.cc

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

src/lib/hooks/hooks_component_developer.dox

@@ -99,13 +99,14 @@ information can be passed to a callout and/or the callout can affect
 the processing of the component. The latter is achieved in either or both
 of the following ways:
 
-- Setting the "skip" flag.  This is a boolean flag that the callout can set
+- Setting the nest step status.  This is an enum that the callout can set
   and is a quick way of passing information back to the component.  It is used
-  to indicate that the component should skip the processing step associated with
-  the hook.  The exact action is up to the component, but is likely to be one
-  of skipping the processing step (probably because the callout has
-  done its own processing for the action) or dropping the current packet
-  and starting on a new request.
+  to indicate that the component should perform certain actions. Currently
+  there are three statuses defined: CONTINUE (this is the default, the server
+  is expected to continue as usual), SKIP (the server is expected to skip the
+  next processing step, but otherwise continue as usual) and DROP (the server
+  is expected to drop the packet or request completely. The exact action is up
+  to the component.
 
 - Modifying data passed to it.  The component should be prepared to continue
   processing with the data returned by the callout.  It is up to the component
@@ -282,7 +283,11 @@ underlying object is altered through that pointer, the change will be
 reflected in the component even if the callout makes no call to setArgument.
 This can be avoided by passing a pointer to a "const" object.
 
-@subsection hooksComponentSkipFlag The Skip Flag
+@subsection hooksComponentSkipFlag The Skip Flag (obsolete)
+
+
+
+@subsection hooksComponentNextStep The next step status
 
 Although information is passed back to the component from callouts through
 @c CalloutHandle arguments, a common action for callouts is to inform the component
@@ -290,24 +295,25 @@ that its flow of control should be altered.  For example:
 
 - In the DHCP servers, there is a hook at the point at which a lease is
   about to be assigned.  Callouts attached to this hooks may handle the
-  lease assignment in special cases, in which case they set the skip flag
-  to indicate that the server should not perform lease assignment in this
-  case.
+  lease assignment in special cases, in which case they set the next step
+  status to SKIP to indicate that the server should not perform lease assignment
+  in this case.
 - A server may define a hook just after a packet is received.  A callout
   attached to the hook might inspect the source address and compare it
   against a blacklist.  If the address is on the list, the callout could set
-  the skip flag to indicate to the server that the packet should be dropped.
+  the DROP flag to indicate to the server that the packet should be dropped.
 
 For ease of processing, the @c CalloutHandle contains
-two methods, @c isc::hooks::CalloutHandle::getSkip() and
-@c isc::hooks::CalloutHandle::setSkip().  It is only meaningful for the
-component to use the "get" method.  The skip flag is cleared by the hooks
-framework when the component requests that callouts be executed, so any
-value set by the component is lost.  Callouts can both inspect the flag (it
+two methods, @c isc::hooks::CalloutHandle::getStatus() and
+@c isc::hooks::CalloutHandle::setStatus().  It is only meaningful for the
+component to use the "get" method.  The next step status is cleared (set to
+the default value of CONTINUE) by the hooks framework when the component
+requests that callouts be executed, so any
+value set by the component is lost.  Callouts can both inspect the status (it
 might have been set by callouts earlier in the callout list for the hook)
-and set it.  Note that the setting of the flag by a callout does not
-prevent callouts later in the list from being called: the skip flag is
-just a boolean flag - the only significance comes from its interpretation
+and set it.  Note that the setting of the status by a callout does not
+prevent callouts later in the list from being called: the next step status is
+just an enum value - the only significance comes from its interpretation
 by the component.
 
 An example of use could be:
@@ -316,7 +322,7 @@ An example of use could be:
 handle->setArgument("query", query);
 handle->setArgument("response", response);
 HooksManager::callCallouts(lease_hook_index, *handle_ptr);
-if (! handle_ptr->getSkip()) {
+if (! handle_ptr->getStatus() != CalloutHandle::NEXT_STEP_SKIP) {
     // Skip flag not set, do the address allocation
     :
 }
@@ -359,7 +365,7 @@ the hook can be executed by:
 ... where "*handle_ptr" is a reference (note: not a pointer) to the
 @c isc::hooks::CalloutHandle object holding the arguments.  No status code
 is returned.  If a component needs to get data returned (other than that
-provided by the "skip" flag), it should define an argument through which
+provided by the next step status), it should define an argument through which
 the callout can do so.
 
 @subsubsection hooksComponentConditionalCallout Conditionally Calling Hook Callouts
@@ -379,8 +385,8 @@ if (HooksManager::calloutsPresent(lease_hook_index)) {
     handle->setArgument("query", query);
     handle->setArgument("response", response);
     HooksManager::callCallouts(lease_hook_index, *handle);
-    if (! handle->getSkip()) {
-        // Skip flag not set, do the address allocation
+    if ( handle->getStatus() != CalloutHandle::NEXT_STEP_DROP ) {
+        // Next step allows us to continue, do the address allocation
         :
     }
 }

src/lib/hooks/hooks_user.dox

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

src/lib/hooks/tests/callout_handle_unittest.cc

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

src/lib/hooks/tests/handles_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013,2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -751,28 +751,37 @@ TEST_F(HandlesTest, DynamicDeregistrationSameHook) {
 
 // Testing the operation of the "skip" flag.  Callouts print the value
 // they see in the flag and either leave it unchanged, set it or clear it.
-
 int
 calloutPrintSkip(CalloutHandle& handle) {
     static const std::string YES("Y");
     static const std::string NO("N");
-
-    HandlesTest::common_string_ = HandlesTest::common_string_ +
-        (handle.getSkip() ? YES : NO);
+    static const std::string DROP("D");
+
+    switch (handle.getStatus()) {
+    case CalloutHandle::NEXT_STEP_CONTINUE:
+        HandlesTest::common_string_ += NO; // skip = no
+        break;
+    case CalloutHandle::NEXT_STEP_SKIP:
+        HandlesTest::common_string_ += YES; // skip = yes
+        break;
+    case CalloutHandle::NEXT_STEP_DROP:
+        HandlesTest::common_string_ += DROP; // drop
+        break;
+    }
     return (0);
 }
 
 int
 calloutSetSkip(CalloutHandle& handle) {
     static_cast<void>(calloutPrintSkip(handle));
-    handle.setSkip(true);
+    handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
     return (0);
 }
 
 int
 calloutClearSkip(CalloutHandle& handle) {
     static_cast<void>(calloutPrintSkip(handle));
-    handle.setSkip(false);
+    handle.setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
     return (0);
 }
 
@@ -806,7 +815,7 @@ TEST_F(HandlesTest, ReturnSkipSet) {
     EXPECT_EQ(std::string("NNYY" "NNYYN" "NNYN"), common_string_);
 
     // ... and check that the skip flag on exit from callCallouts is set.
-    EXPECT_TRUE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_SKIP, callout_handle.getStatus());
 }
 
 // Repeat the test, returning with the skip flag clear.
@@ -838,7 +847,7 @@ TEST_F(HandlesTest, ReturnSkipClear) {
     EXPECT_EQ(std::string("NYY" "NNYNYN" "NNNY"), common_string_);
 
     // ... and check that the skip flag on exit from callCallouts is set.
-    EXPECT_FALSE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle.getStatus());
 }
 
 // Check that the skip flag is cleared when callouts are called - even if
@@ -849,14 +858,14 @@ TEST_F(HandlesTest, NoCalloutsSkipTest) {
     CalloutHandle callout_handle(getCalloutManager());
 
     // Clear the skip flag and call a hook with no callouts.
-    callout_handle.setSkip(false);
+    callout_handle.setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
     getCalloutManager()->callCallouts(alpha_index_, callout_handle);
-    EXPECT_FALSE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle.getStatus());
 
     // Set the skip flag and call a hook with no callouts.
-    callout_handle.setSkip(true);
+    callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
     getCalloutManager()->callCallouts(alpha_index_, callout_handle);
-    EXPECT_FALSE(callout_handle.getSkip());
+    EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle.getStatus());
 }
 
 // The next set of callouts do a similar thing to the above "skip" tests,
@@ -876,13 +885,18 @@ calloutSetArgumentCommon(CalloutHandle& handle, const char* what) {
 }
 
 int
-calloutSetArgumentYes(CalloutHandle& handle) {
-    return (calloutSetArgumentCommon(handle, "Y"));
+calloutSetArgumentSkip(CalloutHandle& handle) {
+    return (calloutSetArgumentCommon(handle, "S"));
+}
+
+int
+calloutSetArgumentContinue(CalloutHandle& handle) {
+    return (calloutSetArgumentCommon(handle, "C"));
 }
 
 int
-calloutSetArgumentNo(CalloutHandle& handle) {
-    return (calloutSetArgumentCommon(handle, "N"));
+calloutSetArgumentDrop(CalloutHandle& handle) {
+    return (calloutSetArgumentCommon(handle, "D"));
 }
 
 // ... and a callout to just copy the argument to the "common_string_" variable
@@ -894,23 +908,25 @@ calloutPrintArgument(CalloutHandle& handle) {
     return (0);
 }
 
+// This test verifies that the next step status is processed appropriately.
+// The test checks the following next step statuses: CONTINUE, SKIP, DROP.
 TEST_F(HandlesTest, CheckModifiedArgument) {
     getCalloutManager()->setLibraryIndex(0);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentYes);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentNo);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentNo);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentSkip);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentContinue);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentContinue);
 
     getCalloutManager()->setLibraryIndex(1);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentYes);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentYes);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentSkip);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentDrop);
     getCalloutManager()->registerCallout("alpha", calloutPrintArgument);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentNo);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentNo);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentDrop);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentContinue);
 
     getCalloutManager()->setLibraryIndex(2);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentYes);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentNo);
-    getCalloutManager()->registerCallout("alpha", calloutSetArgumentYes);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentSkip);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentContinue);
+    getCalloutManager()->registerCallout("alpha", calloutSetArgumentSkip);
 
     // Create the argument with an initial empty string value.  Then call the
     // sequence of callouts above.
@@ -922,10 +938,10 @@ TEST_F(HandlesTest, CheckModifiedArgument) {
     // Check the intermediate and results.  For visual checking, the expected
     // string is divided into sections corresponding to the blocks of callouts
     // above.
-    EXPECT_EQ(std::string("YNN" "YY"), common_string_);
+    EXPECT_EQ(std::string("SCC" "SD"), common_string_);
 
     callout_handle.getArgument(MODIFIED_ARG, modified_arg);
-    EXPECT_EQ(std::string("YNN" "YYNN" "YNY"), modified_arg);
+    EXPECT_EQ(std::string("SCC" "SDDC" "SCS"), modified_arg);
 }
 
 // Test that the CalloutHandle provides the name of the hook to which the

src/lib/util/Makefile.am

@@ -10,7 +10,6 @@ lib_LTLIBRARIES = libkea-util.la
 libkea_util_la_SOURCES  = boost_time_utils.h boost_time_utils.cc
 libkea_util_la_SOURCES += csv_file.h csv_file.cc
 libkea_util_la_SOURCES += filename.h filename.cc
-libkea_util_la_SOURCES += fork_detector.h
 libkea_util_la_SOURCES += strutil.h strutil.cc
 libkea_util_la_SOURCES += buffer.h io_utilities.h
 libkea_util_la_SOURCES += time_utilities.h time_utilities.cc

src/lib/util/fork_detector.h

@@ -1,62 +0,0 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
-//
-// Permission to use, copy, modify, and/or distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-#ifndef FORK_DETECTOR_H
-#define FORK_DETECTOR_H
-
-#include <sys/types.h>
-#include <unistd.h>
-
-namespace isc {
-namespace util {
-
-/// @brief Class which detects being child process.
-///
-/// This class detects if it is in the child process, by checking if the
-/// process PID matches the PID of the process which created instance of
-/// the class.
-///
-/// Detecting if we're in the child process is important when the application
-/// spawns new process using fork/exec. If exec step fails for any reason
-/// the child process exits. However, to exit gracefully the process may
-/// need to know if it is a child or parent and take a different path
-/// during destruction.
-class ForkDetector {
-public:
-
-    /// @brief Constructor.
-    ///
-    /// Stores the PID of the process creating this instance.
-    ForkDetector()
-        : creator_pid_(getpid()) {
-    }
-
-    /// @brief Check if the process is a parent process;
-    ///
-    /// @return true if the process is a parent process.
-    bool isParent() const {
-        return (getpid() == creator_pid_);
-    }
-
-private:
-
-    /// @brief PID of the process which created instance of this class.
-    pid_t creator_pid_;
-
-};
-
-} // namespace isc::util
-} // namespace isc
-
-#endif // FORK_DETECTOR_H

src/lib/util/process_spawn.cc

@@ -217,10 +217,10 @@ ProcessSpawnImpl::spawn() {
         if (execvp(executable_.c_str(), args_) != 0) {
             // We may end up here if the execvp failed, e.g. as a result
             // of issue with permissions or invalid executable name.
-            exit(EXIT_FAILURE);
+            _exit(EXIT_FAILURE);
         }
         // Process finished, exit the child process.
-        exit(EXIT_SUCCESS);
+        _exit(EXIT_SUCCESS);
     }
 
     // We're in the parent process.

src/lib/util/threads/tests/thread_unittest.cc

@@ -242,26 +242,13 @@ TEST_F(ThreadTest, sigmask) {
 }
 
 
-/// The @c ProcessSpawn class spawns new processes using the fork/exec
-/// scheme. If the call to exec fails, child process exits with the
-/// EXIT_FAILURE status code. The call to exit triggers destruction of
-/// all static objects, i.e. destructors of statically declared
-/// @c Thread objects are called in the child process. The call to
-/// fork doesn't clone threads existing in the main process. So, the
-/// @c Thread objects in the child process have invalid state, because
-/// they point to non-existing threads. As a result any attempts to
-/// detach or join the thread may result in errors or asserts.
-///
-/// This test verifies that the @c Thread class protects against such
-/// errors. It is supposed to detect that the @c Thread object is in
-/// the child process and not assert when the destruction fails.
+/// This test verifies using threads and spawning child processes
+/// work together.
 TEST_F(ThreadTest, spawnProcessWithThread) {
     // Initialize and run the stoppable thread. Note that the 'thread'
     // is a static variable, which will be 'cloned' into the child
-    // process. Its destructor will be called when the child process
-    // terminates with EXIT_FAILURE status. So in fact the destructor
-    // of the @c StoppableThread will be called twice: in the main
-    // process and in the child process.
+    // process. Its destructor must not be called when the child process
+    // terminates with EXIT_FAILURE status.
     thread.reset(new StoppableThread());
     thread->run();
 
@@ -273,14 +260,11 @@ TEST_F(ThreadTest, spawnProcessWithThread) {
     while (process_spawn.isRunning(pid)) {
         usleep(100);
     }
-    // When the child process terminates it will call destructors of
-    // static objects. This means that it will call the destructor of
-    // the 'thread' object too. The 'thread' object in the child
-    // process points to non-existing thread, but we expect the
-    // graceful destruction, i.e. the destructor should not assert.
-    // If the destructor asserts the exit code returned here will
-    // be 0 - same as if we aborted.
+    // When the child process terminates it will call _exit() so
+    // nothing bad should happen from the child.
     EXPECT_EQ(EXIT_FAILURE, process_spawn.getExitStatus(pid));
+    // The thread is still there.
+    EXPECT_TRUE(thread);
 }
 
 }

src/lib/util/threads/thread.cc

@@ -12,7 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <util/fork_detector.h>
 #include <util/threads/thread.h>
 #include <util/threads/sync.h>
 
@@ -75,8 +74,7 @@ public:
         // and the creating thread needs to release it.
         waiting_(2),
         main_(main),
-        exception_(false),
-        fork_detector_()
+        exception_(false)
     {}
     // Another of the waiting events is done. If there are no more, delete
     // impl.
@@ -127,8 +125,6 @@ public:
     Mutex mutex_;
     // Which thread are we talking about anyway?
     pthread_t tid_;
-    // Class to detect if we're in the child or parent process.
-    ForkDetector fork_detector_;
 };
 
 Thread::Thread(const boost::function<void ()>& main) :
@@ -152,15 +148,8 @@ Thread::Thread(const boost::function<void ()>& main) :
 
 Thread::~Thread() {
     if (impl_ != NULL) {
-
-        int result = pthread_detach(impl_->tid_);
-        // If the error indicates that thread doesn't exist but we're
-        // in child process (after fork) it is expected. We should
-        // not cause an assert.
-        if (result == ESRCH && !impl_->fork_detector_.isParent()) {
-            result = 0;
-        }
-
+        // In case we didn't call wait yet
+        const int result = pthread_detach(impl_->tid_);
         Impl::done(impl_);
         impl_ = NULL;
         // If the detach ever fails, something is screwed rather badly.
@@ -175,20 +164,13 @@ Thread::wait() {
                   "Wait called and no thread to wait for");
     }
 
-    // Was there an exception in the thread?
-    scoped_ptr<UncaughtException> ex;
-
     const int result = pthread_join(impl_->tid_, NULL);
     if (result != 0) {
-        // We will not throw exception if the error indicates that the
-        // thread doesn't exist and we are in the child process (forked).
-        // For the child process it is expected that the thread is not
-        // re-created when we fork.
-        if (result != ESRCH || impl_->fork_detector_.isParent()) {
-            isc_throw(isc::InvalidOperation, std::strerror(result));
-        }
+        isc_throw(isc::InvalidOperation, std::strerror(result));
     }
 
+    // Was there an exception in the thread?
+    scoped_ptr<UncaughtException> ex;
     // Something here could in theory throw. But we already terminated the thread, so
     // we need to make sure we are in consistent state even in such situation (like
     // releasing the mutex and impl_).

src/lib/util/watch_socket.cc

@@ -42,6 +42,18 @@ WatchSocket::WatchSocket()
     source_ = fds[1];
     sink_ = fds[0];
 
+    if (fcntl(source_, F_SETFD, FD_CLOEXEC)) {
+        const char* errstr = strerror(errno);
+        isc_throw(WatchSocketError, "Cannot set source to close-on-exec: "
+                                     << errstr);
+    }
+
+    if (fcntl(sink_, F_SETFD, FD_CLOEXEC)) {
+        const char* errstr = strerror(errno);
+        isc_throw(WatchSocketError, "Cannot set sink to close-on-exec: "
+                                     << errstr);
+    }
+
     if (fcntl(sink_, F_SETFL, O_NONBLOCK)) {
         const char* errstr = strerror(errno);
         isc_throw(WatchSocketError, "Cannot set sink to non-blocking: "