From: Marcin Siodelski Date: Mon, 6 Jun 2016 11:19:37 +0000 (+0200) Subject: [4320] Allocation engine updates context with allocated resources. X-Git-Tag: fdxhook_base~3^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b2b8f69dfd5253e3e18edf903d99b285e2c1e89c;p=thirdparty%2Fkea.git [4320] Allocation engine updates context with allocated resources. The new context field allocated_resources_ is now updated during lease allocation or renewal with addresses/prefixes assigned. --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 01a33be064..2c508f3a65 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -434,11 +434,6 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { // someone else. allocateReservedLeases6(ctx, leases); - // If we got at least one lease, we're good to go. - if (!leases.empty()) { - return (leases); - } - // If not, we'll need to continue and will eventually fall into case 4: // getting a regular lease. That could happen when we're processing // request from client X, there's a reserved address A for X, but @@ -462,10 +457,7 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { // If they're not, we're ok to keep using them. removeNonmatchingReservedLeases6(ctx, leases); - if (!leases.empty()) { - // Return old leases so the server can see what has changed. - return (updateLeaseData(ctx, leases)); - } + leases = updateLeaseData(ctx, leases); // If leases are empty at this stage, it means that we used to have // leases for this client, but we checked and those leases are reserved @@ -507,11 +499,6 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { // All checks are done. Let's hope we have some leases left. - // If we have any leases left, let's return them and we're done. - if (!leases.empty()) { - return (leases); - } - // If we don't have any leases at this stage, it means that we hit // one of the following cases: // - we have a reservation, but it's not for this IAID/ia-type and @@ -523,24 +510,33 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { // someone else, so we released it. } - // Case 4/catch-all: One of the following is true: - // - we don't have leases and there are no reservations - // - we used to have leases, but we lost them, because they are now - // reserved for someone else - // - we have a reservation, but it is not usable yet, because the address - // is still used by someone else - // - // In any case, we need to go through normal lease assignment process - // for now. This is also a catch-all or last resort approach, when we - // couldn't find any reservations (or couldn't use them). + if (leases.empty()) { + // Case 4/catch-all: One of the following is true: + // - we don't have leases and there are no reservations + // - we used to have leases, but we lost them, because they are now + // reserved for someone else + // - we have a reservation, but it is not usable yet, because the address + // is still used by someone else + // + // In any case, we need to go through normal lease assignment process + // for now. This is also a catch-all or last resort approach, when we + // couldn't find any reservations (or couldn't use them). - LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, - ALLOC_ENGINE_V6_ALLOC_UNRESERVED) - .arg(ctx.query_->getLabel()); + LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, + ALLOC_ENGINE_V6_ALLOC_UNRESERVED) + .arg(ctx.query_->getLabel()); - leases = allocateUnreservedLeases6(ctx); + leases = allocateUnreservedLeases6(ctx); + } if (!leases.empty()) { + // If there are any leases allocated, let's store in them in the + // IA context so as they are available when we process subsequent + // IAs. + BOOST_FOREACH(Lease6Ptr lease, leases) { + ctx.currentIA().addAllocatedResource(lease->addr_, + lease->prefixlen_); + } return (leases); } @@ -1211,6 +1207,16 @@ AllocEngine::renewLeases6(ClientContext6& ctx) { extendLease6(ctx, *l); } + if (!leases.empty()) { + // If there are any leases allocated, let's store in them in the + // IA context so as they are available when we process subsequent + // IAs. + BOOST_FOREACH(Lease6Ptr lease, leases) { + ctx.currentIA().addAllocatedResource(lease->addr_, + lease->prefixlen_); + } + } + return (leases); } catch (const isc::Exception& e) { diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index 5b41bb9d3b..8d93f8c784 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -226,6 +227,8 @@ AllocEngine6Test::allocateTest(AllocEngine& engine, const Pool6Ptr& pool, // Do all checks on the lease checkLease6(*it, type, expected_len, in_pool, in_pool); + checkAllocatedResources(*it, ctx, std::distance(leases.begin(), it)); + // Check that the lease is indeed in LeaseMgr Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(type, (*it)->addr_); @@ -337,6 +340,10 @@ AllocEngine6Test::renewTest(AllocEngine& engine, const Pool6Ptr& pool, // Do all checks on the lease checkLease6(*it, type, expected_len, in_pool, in_pool); + // Check that context has been updated with allocated addresses or + // prefixes. + checkAllocatedResources(*it, ctx, std::distance(leases.begin(), it)); + // Check that the lease is indeed in LeaseMgr Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(type, (*it)->addr_); diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h index f5dbe2c2d1..2d2f58574e 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h @@ -190,6 +190,22 @@ public: /// @todo: check cltt } + /// @brief Checks if context has been updated with allocated addresses + /// or prefixes. + /// + /// @param lease Allocated lease. + /// @param ctx Context structure in which this function should check if + /// leased address is stored as allocated resource. + /// @param lease_index Index of the lease within IA. + void checkAllocatedResources(const Lease6Ptr& lease, + AllocEngine::ClientContext6& ctx, + const size_t lease_index) { + ASSERT_GE(ctx.currentIA().allocated_resources_.size(), lease_index + 1); + EXPECT_EQ(lease->addr_, ctx.currentIA().allocated_resources_[lease_index].first); + EXPECT_EQ(static_cast(lease->prefixlen_), + static_cast(ctx.currentIA().allocated_resources_[lease_index].second)); + } + /// @brief Checks if specified address is increased properly /// /// Method uses gtest macros to mark check failure. This is a proxy