From a63885a00db21df8411fe31aca2ec49d51945d4b Mon Sep 17 00:00:00 2001
From: Thomas Markwalder <tmark@isc.org>
Date: Mon, 24 Apr 2017 11:07:57 -0400
Subject: [PATCH] [5252] Corrected lease state and assigned stat when reusing
 expired-reclaimed via V6 request

src/lib/dhcpsrv/alloc_engine.cc
    AllocEngine::updateLeaseData() - added logic to set lease state
    to default and increment assigned stat if the lease is expired-reclaimed

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
    TEST_F(AllocEngine6Test, requestReuseDeclinedLease6Stats) - new test
    to verify the behavior
---
 src/lib/dhcpsrv/alloc_engine.cc               | 16 +++++
 .../dhcpsrv/tests/alloc_engine6_unittest.cc   | 62 +++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 95e75012f7..121d7964cb 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -1443,6 +1443,22 @@ AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases
         lease->fqdn_rev_ = ctx.rev_dns_update_;
         lease->hostname_ = ctx.hostname_;
         if (!ctx.fake_allocation_) {
+
+            if (lease->state_ == Lease::STATE_EXPIRED_RECLAIMED) {
+                // Transition lease state to default (aka assigned)
+                lease->state_ = Lease::STATE_DEFAULT;
+
+                // If the lease is in the current subnet we need to account
+                // for the re-assignment of The lease.
+                if (ctx.subnet_->inPool(ctx.currentIA().type_, lease->addr_)) {
+                    StatsMgr::instance().addValue(
+                        StatsMgr::generateName("subnet", ctx.subnet_->getID(),
+                                               ctx.currentIA().type_ == Lease::TYPE_NA ?
+                                               "assigned-nas" : "assigned-pds"),
+                        static_cast<int64_t>(1));
+                }
+            }
+
             bool fqdn_changed = ((lease->type_ != Lease::TYPE_PD) &&
                                  !(lease->hasIdenticalFqdn(**lease_it)));
 
diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index dc88f05305..08663bd408 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -1909,6 +1909,68 @@ TEST_F(AllocEngine6Test, requestReuseDeclinedLease6Stats) {
 
 }
 
+// This test checks if an expired-reclaimed lease can be reused by
+// a returning client via REQUEST, rather than renew/rebind.  This
+// would be typical of cable modem clients which do not retain lease
+// data across reboots.
+TEST_F(AllocEngine6Test, reuseReclaimedExpiredViaRequest) {
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
+    ASSERT_TRUE(engine);
+
+    IOAddress addr("2001:db8:1::ad");
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+    cfg_mgr.clear(); // Get rid of the default test configuration
+
+    // Create configuration similar to other tests, but with a single address pool
+    subnet_ = Subnet6Ptr(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+    pool_ = Pool6Ptr(new Pool6(Lease::TYPE_NA, addr, addr)); // just a single address
+    subnet_->addPool(pool_);
+    cfg_mgr.getStagingCfg()->getCfgSubnets6()->add(subnet_);
+    cfg_mgr.commit();
+
+    // Verify relevant stats are zero. 
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID()));
+
+    // Let's create an expired lease
+    Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid_,
+                               501, 502, 503, 504, subnet_->getID(), HWAddrPtr(),
+                               0));
+    lease->cltt_ = time(NULL) - 500; // Allocated 500 seconds ago
+    lease->valid_lft_ = 495; // Lease was valid for 495 seconds
+    lease->fqdn_fwd_ = true;
+    lease->fqdn_rev_ = true;
+    lease->hostname_ = "myhost.example.com.";
+    lease->state_ = Lease::STATE_EXPIRED_RECLAIMED;
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    // Same client comes along and issues a request
+    AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", false,
+                                    Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
+
+    EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx)));
+
+    // Check that he got the orginal lease back.
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(addr, lease->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(lease, from_mgr);
+
+    // Verify assigned-nas got bumped.  Reclaimed stats should still
+    // be zero as we artifically marked it reclaimed.
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID()));
+}
+
 }; // namespace test
 }; // namespace dhcp
 }; // namespace isc
-- 
2.47.2