From: Marcin Siodelski Date: Sun, 19 Jul 2015 09:37:39 +0000 (+0200) Subject: [3958] Fixed invalid number of allocation attampts in the log message. X-Git-Tag: trac4006_base~9^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3946e8ad94165971431b49bfb4b196d09bb08a29;p=thirdparty%2Fkea.git [3958] Fixed invalid number of allocation attampts in the log message. --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 96e5e5af3f..0afcc5f9eb 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -231,9 +231,9 @@ AllocEngine::RandomAllocator::pickAddress(const SubnetPtr&, } -AllocEngine::AllocEngine(AllocType engine_type, unsigned int attempts, +AllocEngine::AllocEngine(AllocType engine_type, uint64_t attempts, bool ipv6) - :attempts_(attempts) { + : attempts_(attempts) { // Choose the basic (normal address) lease type Lease::Type basic_type = ipv6 ? Lease::TYPE_NA : Lease::TYPE_V4; @@ -502,10 +502,6 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { return (leases); } - // Unable to allocate an address, return an empty lease. - LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V6_ALLOC_FAIL) - .arg(ctx.query_->getLabel()) - .arg(attempts_); } catch (const isc::Exception& e) { @@ -625,13 +621,8 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { // - we find a free address // - we find an address for which the lease has expired // - we exhaust number of tries - // - /// @todo: We used to use hardcoded number of attempts (100). Now we dynamically - /// calculate the number of possible leases in all pools in this subnet and - /// try that number of times at most. It would be useful to that value if - /// attempts_, specified by the user could override that value (and keep - /// dynamic if they're set to 0). - uint64_t max_attempts = ctx.subnet_->getPoolCapacity(ctx.type_); + uint64_t max_attempts = (attempts_ > 0 ? attempts_ : + ctx.subnet_->getPoolCapacity(ctx.type_)); for (uint64_t i = 0; i < max_attempts; ++i) { IOAddress candidate = allocator->pickAddress(ctx.subnet_, ctx.duid_, hint); @@ -693,6 +684,13 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { } } + // Unable to allocate an address, return an empty lease. + LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V6_ALLOC_FAIL) + .arg(ctx.query_->getLabel()) + .arg(max_attempts); + + + // We failed to allocate anything. Let's return empty collection. return (Lease6Collection()); } @@ -1424,12 +1422,6 @@ AllocEngine::allocateLease4(ClientContext4& ctx) { } new_lease = ctx.fake_allocation_ ? discoverLease4(ctx) : requestLease4(ctx); - if (!new_lease) { - // Unable to allocate an address, return an empty lease. - LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V4_ALLOC_FAIL) - .arg(ctx.query_->getLabel()) - .arg(attempts_); - } } catch (const isc::Exception& e) { // Some other error, return an empty lease. @@ -2008,7 +2000,8 @@ Lease4Ptr AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) { Lease4Ptr new_lease; AllocatorPtr allocator = getAllocator(Lease::TYPE_V4); - const uint64_t max_attempts = ctx.subnet_->getPoolCapacity(Lease::TYPE_V4); + const uint64_t max_attempts = (attempts_ > 0 ? attempts_ : + ctx.subnet_->getPoolCapacity(Lease::TYPE_V4)); for (uint64_t i = 0; i < max_attempts; ++i) { IOAddress candidate = allocator->pickAddress(ctx.subnet_, ctx.clientid_, ctx.requested_address_); @@ -2024,6 +2017,11 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) { } } + // Unable to allocate an address, return an empty lease. + LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V4_ALLOC_FAIL) + .arg(ctx.query_->getLabel()) + .arg(max_attempts); + return (new_lease); } diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index d6f23ae53b..1ad2b3b081 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -219,7 +219,7 @@ public: /// @param attempts number of attempts for each lease allocation before /// we give up (0 means unlimited) /// @param ipv6 specifies if the engine should work for IPv4 or IPv6 - AllocEngine(AllocType engine_type, unsigned int attempts, bool ipv6 = true); + AllocEngine(AllocType engine_type, uint64_t attempts, bool ipv6 = true); /// @brief Destructor. virtual ~AllocEngine() { } @@ -240,7 +240,7 @@ private: std::map allocators_; /// @brief number of attempts before we give up lease allocation (0=unlimited) - unsigned int attempts_; + uint64_t attempts_; // hook name indexes (used in hooks callouts) int hook_index_lease4_select_; ///< index for lease4_select hook diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index b146638278..9106c388f9 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc @@ -1571,7 +1571,7 @@ TEST_F(AllocEngine6Test, reservedAddressByMacInPoolRequestValidHint) { // value. This test verifies that the prefix can be allocated in that // case. TEST_F(AllocEngine6Test, largePDPool) { - AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100); + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0); // Remove the default PD pool. subnet_->delPools(Lease::TYPE_PD); @@ -1593,7 +1593,7 @@ TEST_F(AllocEngine6Test, largePDPool) { // confuse the allocation engine if the number of available addresses // was larger than 2^32. TEST_F(AllocEngine6Test, largePoolOver32bits) { - AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100); + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0); // Configure 2001:db8::/32 subnet subnet_.reset(new Subnet6(IOAddress("2001:db8::"), 32, 1, 2, 3, 4));