From: Thomas Markwalder Date: Wed, 22 Feb 2023 20:00:13 +0000 (-0500) Subject: [#2739] Consistently calculate v6 lease lifetimes X-Git-Tag: Kea-2.3.6~124 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b2a1e0f77f91fa3e7c763a6c7274de088588a891;p=thirdparty%2Fkea.git [#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 ee836b7f19..49a5d12dd9 100644 --- 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 cad8417d8a..7adae505ad 100644 --- 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 f2e12cfda5..4c614fdb51 100644 --- 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 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 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(100, 200, 300)); + subnet_->setValid(Triplet(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(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 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 valid_lft(600, 700, 800); + class_def->setValid(valid_lft); + Triplet 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 86fc416b10..e334d1e242 100644 --- 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(200, 300, 400)); subnet_->setValid(Triplet(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 22efed4c09..2d327299b1 100644 --- 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. ///