]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[3958] Fixed invalid number of allocation attampts in the log message.
authorMarcin Siodelski <marcin@isc.org>
Sun, 19 Jul 2015 09:37:39 +0000 (11:37 +0200)
committerMarcin Siodelski <marcin@isc.org>
Sun, 19 Jul 2015 10:03:07 +0000 (12:03 +0200)
src/lib/dhcpsrv/alloc_engine.cc
src/lib/dhcpsrv/alloc_engine.h
src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc

index 96e5e5af3f4dc87f86055a031f620f47f4724122..0afcc5f9eb9781919508f6ad7e86fa1dd8000210 100644 (file)
@@ -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);
 }
 
index d6f23ae53b2814233f0385e1252cb62febe132e4..1ad2b3b081c6f465ad04e0d57e2ce608ffbb5c42 100644 (file)
@@ -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<Lease::Type, AllocatorPtr> 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
index b146638278c6e1804e0fba142487c8ad5a0d3d4a..9106c388f92758fb8134222578635d7c9c237d09 100644 (file)
@@ -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));