[master] Merge branch 'trac3985' (automated declined v6 lease recovery)

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>

src/bin/dhcp6/dhcp6_srv.cc

@@ -2758,6 +2758,14 @@ Dhcpv6Srv::setStatusCode(boost::shared_ptr<isc::dhcp::Option6IA>& container,
 void
 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.
 
     // Check if a lease has flags indicating that the FQDN update has
     // been performed. If so, create NameChangeRequest which removes

src/lib/dhcpsrv/alloc_engine.cc

@@ -936,6 +936,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_;
@@ -1347,9 +1358,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));
             }
@@ -1575,6 +1602,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,
@@ -2281,7 +2339,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).

src/lib/dhcpsrv/alloc_engine.h

@@ -505,6 +505,29 @@ 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.
@@ -529,7 +552,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 +570,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).
     ///
@@ -772,7 +795,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 +805,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.

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

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

@@ -908,13 +908,14 @@ public:
     /// @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 +941,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 +975,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 +988,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 +1015,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.
@@ -1283,6 +1298,27 @@ TEST_F(ExpirationAllocEngine6Test, reclaimExpiredLeasesShortTimeout) {
     testReclaimExpiredLeasesTimeout(1);
 }
 
+/// 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!
@@ -1651,7 +1687,7 @@ TEST_F(ExpirationAllocEngine4Test, reclaimExpiredLeasesShortTimeout) {
 /// 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 +1695,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/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) {