From: Marcin Siodelski Date: Mon, 4 Jun 2018 12:45:34 +0000 (+0200) Subject: [5638] Reset callout handle status prior to lease6_select. X-Git-Tag: trac5117_base~1^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28641c3be2fabc4825e53f26aa62a910415c0178;p=thirdparty%2Fkea.git [5638] Reset callout handle status prior to lease6_select. --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 476a893dcd..391915b37d 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -919,6 +919,13 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { uint64_t max_attempts = (attempts_ > 0 ? attempts_ : possible_attempts); Network::HRMode hr_mode = subnet->getHostReservationMode(); + // Set the default status code in case the lease6_select callouts + // do not exist and the callout handle has a status returned by + // any of the callouts already invoked for this packet. + if (ctx.callout_handle_) { + ctx.callout_handle_->setStatus(CalloutHandle::NEXT_STEP_CONTINUE); + } + for (uint64_t i = 0; i < max_attempts; ++i) { ++total_attempts; @@ -967,6 +974,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { leases.push_back(lease); return (leases); + } else if (ctx.callout_handle_ && (ctx.callout_handle_->getStatus() != CalloutHandle::NEXT_STEP_CONTINUE)) { diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index 0a8afebc32..e4baabdfce 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc @@ -76,6 +76,19 @@ TEST_F(AllocEngine6Test, simpleAlloc6) { // We should have bumped the assigned counter by 1 EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + + // Reset last allocated address to check that the other client will + // be refused the already allocated address and will get the one + // available. + pool_->resetLastAllocated(); + + // Simulate another client. This client should be assigned a different + // address. + DuidPtr duid(new DUID(std::vector(8, 0x84))); + simpleAlloc6Test(pool_, duid, IOAddress("::"), false); + + // We should have bumped the assigned counter by 2 + EXPECT_TRUE(testStatistics("assigned-nas", 2, subnet_->getID())); } // This test checks if the simple PD allocation (REQUEST) can succeed @@ -749,7 +762,7 @@ TEST_F(AllocEngine6Test, smallPool6) { EXPECT_EQ("2001:db8:1::ad", lease->addr_.toText()); // Do all checks on the lease - checkLease6(lease, Lease::TYPE_NA, 128); + checkLease6(duid_, lease, Lease::TYPE_NA, 128); // Check that the lease is indeed in LeaseMgr Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_, @@ -847,7 +860,7 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) { EXPECT_EQ(addr, lease->addr_); // Do all checks on the lease (if subnet-id, preferred/valid times are ok etc.) - checkLease6(lease, Lease::TYPE_NA, 128); + checkLease6(duid_, lease, Lease::TYPE_NA, 128); // CASE 2: Asking specifically for this address AllocEngine::ClientContext6 ctx2(subnet_, duid_, false, false, "", true, @@ -2098,7 +2111,7 @@ TEST_F(AllocEngine6Test, solicitReuseDeclinedLease6) { 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); + checkLease6(duid_, assigned, Lease::TYPE_NA, 128); // CASE 2: Asking specifically for this address testReuseLease6(engine, declined, addr_txt, true, SHOULD_PASS, assigned); diff --git a/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc index 1aa2ed2abf..933f31fb27 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -189,7 +189,7 @@ TEST_F(HookAllocEngine6Test, lease6_select) { ASSERT_TRUE(lease); // Do all checks on the lease - checkLease6(lease, Lease::TYPE_NA, 128); + checkLease6(duid_, lease, Lease::TYPE_NA, 128); // Check that the lease is indeed in LeaseMgr Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_, diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index 81eab8f28b..30ac73677b 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -14,6 +14,8 @@ #include #include #include +#include +#include #include #include @@ -227,7 +229,7 @@ AllocEngine6Test::allocateTest(AllocEngine& engine, const Pool6Ptr& pool, for (Lease6Collection::iterator it = leases.begin(); it != leases.end(); ++it) { // Do all checks on the lease - checkLease6(*it, type, expected_len, in_pool, in_pool); + checkLease6(duid_, *it, type, expected_len, in_pool, in_pool); // Check that context has been updated with allocated addresses or // prefixes. @@ -265,6 +267,12 @@ AllocEngine6Test::allocateTest(AllocEngine& engine, const Pool6Ptr& pool, Lease6Ptr AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint, bool fake, bool in_pool) { + return (simpleAlloc6Test(pool, duid_, hint, fake, in_pool)); +} + +Lease6Ptr +AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const DuidPtr& duid, + const IOAddress& hint, bool fake, bool in_pool) { Lease::Type type = pool->getType(); uint8_t expected_len = pool->getLength(); @@ -279,13 +287,18 @@ AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint, Pkt6Ptr query(new Pkt6(fake ? DHCPV6_SOLICIT : DHCPV6_REQUEST, 1234)); - AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", fake, query); + AllocEngine::ClientContext6 ctx(subnet_, duid, false, false, "", fake, query); ctx.hwaddr_ = hwaddr_; ctx.addHostIdentifier(Host::IDENT_HWADDR, hwaddr_->hwaddr_); ctx.currentIA().iaid_ = iaid_; ctx.currentIA().type_ = type; ctx.currentIA().addHint(hint); + // Set some non-standard callout status to make sure it doesn't affect the + // allocation. + ctx.callout_handle_ = HooksManager::createCalloutHandle(); + ctx.callout_handle_->setStatus(CalloutHandle::NEXT_STEP_SKIP); + findReservation(*engine, ctx); Lease6Ptr lease; EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx))); @@ -297,7 +310,7 @@ AllocEngine6Test::simpleAlloc6Test(const Pool6Ptr& pool, const IOAddress& hint, } // Do all checks on the lease - checkLease6(lease, type, expected_len, in_pool, in_pool); + checkLease6(duid, lease, type, expected_len, in_pool, in_pool); // Check that the lease is indeed in LeaseMgr Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(type, lease->addr_); @@ -342,7 +355,7 @@ AllocEngine6Test::renewTest(AllocEngine& engine, const Pool6Ptr& pool, for (Lease6Collection::iterator it = leases.begin(); it != leases.end(); ++it) { // Do all checks on the lease - checkLease6(*it, type, expected_len, in_pool, in_pool); + checkLease6(duid_, *it, type, expected_len, in_pool, in_pool); // Check that context has been updated with allocated addresses or // prefixes. @@ -406,7 +419,7 @@ AllocEngine6Test::allocWithUsedHintTest(Lease::Type type, IOAddress used_addr, EXPECT_NE(requested, lease->addr_); // Do all checks on the lease - checkLease6(lease, type, expected_pd_len); + checkLease6(duid_, lease, type, expected_pd_len); // Check that the lease is indeed in LeaseMgr Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_, @@ -445,7 +458,7 @@ AllocEngine6Test::allocBogusHint6(Lease::Type type, asiolink::IOAddress hint, EXPECT_NE(hint, lease->addr_); // Do all checks on the lease - checkLease6(lease, type, expected_pd_len); + checkLease6(duid_, lease, type, expected_pd_len); // Check that the lease is indeed in LeaseMgr Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_, @@ -493,7 +506,7 @@ AllocEngine6Test::testReuseLease6(const AllocEnginePtr& engine, ASSERT_EQ(1, leases.size()); result = leases[0]; - checkLease6(result, Lease::TYPE_NA, 128); + checkLease6(duid_, result, Lease::TYPE_NA, 128); break; case SHOULD_FAIL: diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h index 5b86b4e6a5..1d83d505cd 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h @@ -163,13 +163,15 @@ public: /// @brief checks if Lease6 matches expected configuration /// + /// @param duid pointer to the client's DUID. /// @param lease lease to be checked /// @param exp_type expected lease type /// @param exp_pd_len expected prefix length /// @param expected_in_subnet whether the lease is expected to be in subnet /// @param expected_in_pool whether the lease is expected to be in dynamic - void checkLease6(const Lease6Ptr& lease, Lease::Type exp_type, - uint8_t exp_pd_len = 128, bool expected_in_subnet = true, + void checkLease6(const DuidPtr& duid, const Lease6Ptr& lease, + Lease::Type exp_type, uint8_t exp_pd_len = 128, + bool expected_in_subnet = true, bool expected_in_pool = true) { // that is belongs to the right subnet @@ -198,7 +200,7 @@ public: EXPECT_EQ(fqdn_fwd_, lease->fqdn_fwd_); EXPECT_EQ(fqdn_rev_, lease->fqdn_rev_); EXPECT_EQ(hostname_, lease->hostname_); - EXPECT_TRUE(*lease->duid_ == *duid_); + EXPECT_TRUE(*lease->duid_ == *duid); /// @todo: check cltt } @@ -246,7 +248,7 @@ public: /// @brief Checks if the simple allocation can succeed /// - /// The type of lease is determined by pool type (pool->getType() + /// The type of lease is determined by pool type (pool->getType()) /// /// @param pool pool from which the lease will be allocated from /// @param hint address to be used as a hint @@ -257,6 +259,21 @@ public: const asiolink::IOAddress& hint, bool fake, bool in_pool = true); + /// @brief Checks if the simple allocation can succeed for custom DUID. + /// + /// The type of lease is determined by pool type (pool->getType()) + /// + /// @param pool pool from which the lease will be allocated from + /// @param duid pointer to the DUID used for allocation. + /// @param hint address to be used as a hint + /// @param fake true - this is fake allocation (SOLICIT) + /// @param in_pool specifies whether the lease is expected to be in pool + /// @return allocated lease (or NULL) + Lease6Ptr simpleAlloc6Test(const Pool6Ptr& pool, const DuidPtr& duid, + const asiolink::IOAddress& hint, + bool fake, bool in_pool = true); + + /// @brief Checks if the allocation can succeed. /// /// The type of lease is determined by pool type (pool->getType()).