[#2739] Consistently calculate v6 lease lifetimes

Added a ChangeLog entry

src/lib/dhcpsrv/alloc_engine.cc
    AllocEngine::reuseExpiredLease(Lease6Ptr...)
    AllocEngine::extendLease6()
    - replaced explicit lifetime logic with call to getLifetimes6()

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
    TEST_F(AllocEngine6Test, classReuseExpiredLease6)
    TEST_F(AllocEngine6Test, renewClassLeaseLifetime) - new tests

src/lib/dhcpsrv/tests/alloc_engine_utils.*
    AllocEngine6Test::simpleAlloc6Test() - now accepts option class defintion

diff --git a/ChangeLog b/ChangeLog
index ee836b7f19960c07c6c32bc67489a1213a4eaa19..49a5d12dd93201eabd1ed67c22df62ad0c35f821 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2105.  [bug]           tmark
+       kea-dhcp6 now consistently uses valid and preferred lifetime
+       values provided via classes.  Prior to this it was not
+       utilizing class-supplied lifetimes when renewing existing
+       leases or when reusing expired leases.
+       (Gitlab #2739)
+
 Kea 2.3.5 (development) released on February 22, 2023
 
 2104.  [build]         andrei

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index cad8417d8a9e7f05af019a8942a8a0d36b292a42..7adae505adfdf0075417d54b291974348a2ff3b4 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -1680,25 +1680,11 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
     // address, lease type and prefixlen (0) stay the same
     expired->iaid_ = ctx.currentIA().iaid_;
     expired->duid_ = ctx.duid_;
-    // Use subnet's preferred triplet to conditionally determine
-    // preferred lifetime based on hint
-    if (!ctx.currentIA().hints_.empty() &&
-        ctx.currentIA().hints_[0].getPreferred()) {
-        uint32_t preferred = ctx.currentIA().hints_[0].getPreferred();
-        expired->preferred_lft_ = ctx.subnet_->getPreferred().get(preferred);
-    } else {
-        expired->preferred_lft_ = ctx.subnet_->getPreferred();
-    }
-    // Use subnet's valid triplet to conditionally determine
-    // valid lifetime based on hint
+
+    // Calculate life times.
+    getLifetimes6(ctx, expired->preferred_lft_, expired->valid_lft_);
     expired->reuseable_valid_lft_ = 0;
-    if (!ctx.currentIA().hints_.empty() &&
-        ctx.currentIA().hints_[0].getValid()) {
-        uint32_t valid = ctx.currentIA().hints_[0].getValid();
-        expired->valid_lft_ = ctx.subnet_->getValid().get(valid);
-    } else {
-        expired->valid_lft_ = ctx.subnet_->getValid();
-    }
+
     expired->cltt_ = time(NULL);
     expired->subnet_id_ = ctx.subnet_->getID();
     expired->hostname_ = ctx.hostname_;
@@ -2132,26 +2118,14 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
     Lease6Ptr old_data(new Lease6(*lease));
 
     bool changed = false;
+
+    // Calculate life times.
     uint32_t current_preferred_lft = lease->preferred_lft_;
-    if (!ctx.currentIA().hints_.empty() &&
-        ctx.currentIA().hints_[0].getPreferred()) {
-        uint32_t preferred = ctx.currentIA().hints_[0].getPreferred();
-        lease->preferred_lft_ = ctx.subnet_->getPreferred().get(preferred);
-    } else {
-        lease->preferred_lft_ = ctx.subnet_->getPreferred();
-    }
-    if (lease->preferred_lft_ < current_preferred_lft) {
-        changed = true;
-    }
-    lease->reuseable_valid_lft_ = 0;
-    if (!ctx.currentIA().hints_.empty() &&
-        ctx.currentIA().hints_[0].getValid()) {
-        uint32_t valid = ctx.currentIA().hints_[0].getValid();
-        lease->valid_lft_ = ctx.subnet_->getValid().get(valid);
-    } else {
-        lease->valid_lft_ = ctx.subnet_->getValid();
-    }
-    if (lease->valid_lft_ < lease->current_valid_lft_) {
+    getLifetimes6(ctx, lease->preferred_lft_, lease->valid_lft_);
+
+    // If either has changed set the changed flag.
+    if ((lease->preferred_lft_ != current_preferred_lft) ||
+        (lease->valid_lft_ != lease->current_valid_lft_)) {
         changed = true;
     }
 

diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index f2e12cfda55a18ae674b5016fc11b5cd88f4c2f6..4c614fdb51a1e51d507469c3be29573d074f170a 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -622,7 +622,7 @@ TEST_F(AllocEngine6Test, maxReuseExpiredLease6) {
 
     // Asking specifically for this address with zero lifetimes
     AllocEngine::ClientContext6 ctx2(subnet_, duid_, false, false, "", true,
-                                     Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
+                    Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
     ctx2.currentIA().iaid_ = iaid_;
     ctx2.currentIA().addHint(addr, 128, 500, 600);
 
@@ -637,6 +637,65 @@ TEST_F(AllocEngine6Test, maxReuseExpiredLease6) {
     EXPECT_EQ(500, lease->valid_lft_);
 }
 
+// This test checks if an expired lease can be reused using class lifetimes.
+// Verifies getLifetimes() is invoked by v6 resuseExpiredLease().
+TEST_F(AllocEngine6Test, classReuseExpiredLease6) {
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(100)));
+    ASSERT_TRUE(engine);
+
+    IOAddress addr("2001:db8:1::ad");
+
+    // Let's make a classes with valid-lifetime and add it to the dictionary.
+    ClientClassDictionaryPtr dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary();
+
+    ClientClassDefPtr class_def(new ClientClassDef("valid_one", ExpressionPtr()));
+    Triplet<uint32_t> valid_one(600, 700, 800);
+    class_def->setValid(valid_one);
+    dictionary->addClass(class_def);
+
+    // Create one subnet with a pool holding one address.
+    initSubnet(IOAddress("2001:db8:1::"), addr, addr);
+    subnet_->setPreferred(Triplet<uint32_t>(100, 200, 300));
+    subnet_->setValid(Triplet<uint32_t>(300, 400, 500));
+
+    // Initialize FQDN data for the lease.
+    initFqdn("myhost.example.com", true, true);
+
+    // Just a different duid
+    DuidPtr other_duid = DuidPtr(new DUID(vector<uint8_t>(12, 0xff)));
+    const uint32_t other_iaid = 3568;
+    Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, other_duid, other_iaid,
+                               501, 502, subnet_->getID(),
+                               HWAddrPtr(), 0));
+    lease->cltt_ = time(NULL) - 500; // Allocated 500 seconds ago
+    lease->valid_lft_ = 495; // Lease was valid for 495 seconds
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    // Make sure that we really created expired lease
+    ASSERT_TRUE(lease->expired());
+
+    // Asking specifically for this address with zero lifetimes
+    AllocEngine::ClientContext6 ctx2(subnet_, duid_, false, false, "", true,
+                                     Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
+    ctx2.currentIA().iaid_ = iaid_;
+    ctx2.currentIA().addHint(addr, 128, 0, 0);
+
+    // Add the class name to the context.
+    ctx2.query_->addClass("valid_one");
+
+    EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx2)));
+
+    // Check that we got that single lease
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(addr, lease->addr_);
+
+    // Check lifetimes: specified values are expected.
+    EXPECT_EQ(200, lease->preferred_lft_);
+    EXPECT_EQ(700, lease->valid_lft_);
+}
+
+
 // This test checks if an expired lease can be reused in REQUEST (actual allocation)
 TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     boost::scoped_ptr<AllocEngine> engine;
@@ -944,7 +1003,57 @@ TEST_F(AllocEngine6Test, maxRenewLeaseLifetime) {
     EXPECT_EQ(500, renewed[0]->valid_lft_);
 }
 
-// Checks if the lease lifetime is extended when the client sends the
+// Verifies renewal uses class life times via getLifetimes() in invoked by
+// extendLease6().
+TEST_F(AllocEngine6Test, renewClassLeaseLifetime) {
+    // Create a lease for the client.
+    Lease6Ptr lease(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::15"),
+                               duid_, iaid_, 300, 400,
+                               subnet_->getID(), HWAddrPtr(), 128));
+    // Allocated 200 seconds ago - half of the lifetime.
+    time_t lease_cltt = time(NULL) - 200;
+    lease->cltt_ = lease_cltt;
+
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    AllocEngine engine(100);
+
+    // This is what the client will send in his renew message.
+    AllocEngine::HintContainer hints;
+    hints.push_back(AllocEngine::Resource(IOAddress("2001:db8:1::15"), 128));
+
+    Lease::Type type = Lease::TYPE_NA;
+
+    Pkt6Ptr query(new Pkt6(DHCPV6_RENEW, 1234));
+    AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "",
+                                    false, query);
+    ctx.currentIA().hints_ = hints;
+    ctx.currentIA().iaid_ = iaid_;
+    ctx.currentIA().type_ = type;
+
+    // Add a client class with both valid and preferred lifetime values.
+    ClientClassDefPtr class_def(new ClientClassDef("lft_class", ExpressionPtr()));
+    Triplet<uint32_t> valid_lft(600, 700, 800);
+    class_def->setValid(valid_lft);
+    Triplet<uint32_t> preferred_lft(650, 750, 850);
+    class_def->setPreferred(preferred_lft);
+    CfgMgr::instance().getCurrentCfg()->getClientClassDictionary()->addClass(class_def);
+    ctx.query_->addClass(class_def->getName());
+
+
+    findReservation(engine, ctx);
+    Lease6Collection renewed = engine.renewLeases6(ctx);
+    ASSERT_EQ(1, renewed.size());
+
+    // And the lease lifetime should be extended.
+    EXPECT_GT(renewed[0]->cltt_, lease_cltt)
+        << "Lease lifetime was not extended, but it should";
+
+    // Verify life times came from the class.
+    EXPECT_EQ(renewed[0]->preferred_lft_, 750);
+    EXPECT_EQ(renewed[0]->valid_lft_, 700);
+}
+
 // Renew and the client has a reservation for the lease.
 TEST_F(AllocEngine6Test, renewExtendLeaseLifetimeForReservation) {
     // Create reservation for the client. This is in-pool reservation,

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc
index 86fc416b103a17d35f8f75b9017b391ec9f9b0fa..e334d1e242f7a1af167c51c2c95ed1bdc03107a1 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc
@@ -289,7 +289,8 @@ AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint,
 Lease6Ptr
 AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint,
                                    uint32_t preferred, uint32_t valid,
-                                   uint32_t exp_preferred, uint32_t exp_valid) {
+                                   uint32_t exp_preferred, uint32_t exp_valid,
+                                   ClientClassDefPtr class_def) {
     Lease::Type type = pool->getType();
     uint8_t expected_len = pool->getLength();
 
@@ -312,6 +313,12 @@ AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint,
     subnet_->setPreferred(Triplet<uint32_t>(200, 300, 400));
     subnet_->setValid(Triplet<uint32_t>(300, 400, 500));
 
+    if (class_def) {
+        std::cout << "adding class defintion" << std::endl;
+        CfgMgr::instance().getStagingCfg()->getClientClassDictionary()->addClass(class_def);
+        ctx.query_->addClass(class_def->getName());
+    }
+
     // Set some non-standard callout status to make sure it doesn't affect the
     // allocation.
     ctx.callout_handle_ = HooksManager::createCalloutHandle();
@@ -404,7 +411,6 @@ Lease6Collection
 AllocEngine6Test::renewTest(AllocEngine& engine, const Pool6Ptr& pool,
                             AllocEngine::HintContainer& hints,
                             bool in_subnet, bool in_pool) {
-
     Lease::Type type = pool->getType();
     uint8_t expected_len = pool->getLength();
 

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
index 22efed4c09754177484c427d7444a940a7c96378..2d327299b1d32aa952fc0d97370e39a7e054242f 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h
+++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
@@ -306,13 +306,15 @@ public:
     /// @param valid valid lifetime to be used as a hint
     /// @param exp_preferred expected lease preferred lifetime
     /// @param exp_valid expected lease valid lifetime
+    /// @param class_def class definition to add to the context
     /// @return allocated lease (or NULL)
     Lease6Ptr simpleAlloc6Test(const Pool6Ptr& pool,
                                const asiolink::IOAddress& hint,
                                uint32_t preferred,
                                uint32_t valid,
                                uint32_t exp_preferred,
-                               uint32_t exp_valid);
+                               uint32_t exp_valid,
+                               ClientClassDefPtr class_def = ClientClassDefPtr());
 
     /// @brief Checks if the simple allocation can succeed for custom DUID.
     ///