[master] Merge branch 'trac3958'

src/bin/dhcp4/dhcp4_srv.cc

@@ -240,8 +240,10 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const bool use_bcast,
             IfaceMgr::instance().setMatchingPacketFilter(direct_response_desired);
         }
 
-        // Instantiate allocation engine
-        alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100,
+        // Instantiate allocation engine. The number of allocation attempts equal
+        // to zero indicates that the allocation engine will use the number of
+        // attempts depending on the pool size.
+        alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 0,
                                             false /* false = IPv4 */));
 
         // Register hook points

src/bin/dhcp6/dhcp6_srv.cc

@@ -212,8 +212,10 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
             }
         }
 
-        // Instantiate allocation engine
-        alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
+        // Instantiate allocation engine. The number of allocation attempts equal
+        // to zero indicates that the allocation engine will use the number of
+        // attempts depending on the pool size.
+        alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 0));
 
         /// @todo call loadLibraries() when handling configuration changes
 

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,14 +621,9 @@ 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).
-    uint32_t max_attempts = ctx.subnet_->getPoolCapacity(ctx.type_);
-    for (uint32_t i = 0; i < max_attempts; ++i)
+    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);
 }
 

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<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

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc

@@ -476,6 +476,7 @@ TEST_F(AllocEngine6Test, outOfAddresses6) {
 
 }
 
+
 // This test checks if an expired lease can be reused in SOLICIT (fake allocation)
 TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) {
     boost::scoped_ptr<AllocEngine> engine;
@@ -1563,6 +1564,96 @@ TEST_F(AllocEngine6Test, reservedAddressByMacInPoolRequestValidHint) {
     EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
 }
 
+// This test checks that the allocation engine can delegate the long prefix.
+// The pool with prefix of 64 and with long delegated prefix has a very
+// high capacity. The number of attempts that the allocation engine makes
+// to allocate the prefix for high capacity pools is equal to the capacity
+// value. This test verifies that the prefix can be allocated in that
+// case.
+TEST_F(AllocEngine6Test, largePDPool) {
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0);
+
+    // Remove the default PD pool.
+    subnet_->delPools(Lease::TYPE_PD);
+
+    // Configure the PD pool with the prefix length of /64 and the delegated
+    // length /96.
+    Pool6Ptr pool(new Pool6(Lease::TYPE_PD, IOAddress("2001:db8:1::"), 64, 96));
+    subnet_->addPool(pool);
+
+    // We should have got exactly one lease.
+    Lease6Collection leases = allocateTest(engine, pool, IOAddress("::"),
+                                           false, true);
+    ASSERT_EQ(1, leases.size());
+}
+
+// This test checks that the allocation engine can delegate addresses
+// from ridiculously large pool. The configuration provides 2^80 or
+// 1208925819614629174706176 addresses. We used to have a bug that would
+// confuse the allocation engine if the number of available addresses
+// was larger than 2^32.
+TEST_F(AllocEngine6Test, largePoolOver32bits) {
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0);
+
+    // Configure 2001:db8::/32 subnet
+    subnet_.reset(new Subnet6(IOAddress("2001:db8::"), 32, 1, 2, 3, 4));
+
+    // Configure the NA pool of /48. So there are 2^80 addresses there. Make
+    // sure that we still can handle cases where number of available addresses
+    // is over max_uint64
+    Pool6Ptr pool(new Pool6(Lease::TYPE_NA, IOAddress("2001:db8:1::"), 48));
+    subnet_->addPool(pool);
+
+    // We should have got exactly one lease.
+    Lease6Collection leases = allocateTest(engine, pool, IOAddress("::"),
+                                           false, true);
+    ASSERT_EQ(1, leases.size());
+}
+
+// This test verifies that it is possible to override the number of allocation
+// attempts made by the allocation engine for a single lease.
+TEST_F(AllocEngine6Test, largeAllocationAttemptsOverride) {
+    // Remove the default NA pools.
+    subnet_->delPools(Lease::TYPE_NA);
+
+    // Add exactly one pool with many addresses.
+    Pool6Ptr pool(new Pool6(Lease::TYPE_NA, IOAddress("2001:db8:1::"), 56));
+    subnet_->addPool(pool);
+
+    // Allocate 5 addresses from the pool configured.
+    for (int i = 0; i < 5; ++i) {
+        DuidPtr duid = DuidPtr(new DUID(vector<uint8_t>(12,
+                                                        static_cast<uint8_t>(i))));
+        // Get the unique IAID.
+        const uint32_t iaid = 3568 + i;
+
+        // Construct the unique address from the pool.
+        std::ostringstream address;
+        address << "2001:db8:1::";
+        address << i;
+
+        // Allocate the leease.
+        Lease6Ptr lease(new Lease6(Lease::TYPE_NA, IOAddress(address.str()),
+                                   duid, iaid, 501, 502, 503, 504, subnet_->getID(),
+                                   HWAddrPtr(), 0));
+        ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+    }
+
+    // Try to use the allocation engine to allocate the lease. The iterative
+    // allocator will pick the addresses already allocated until it finds the
+    // available address. Since, we have restricted the number of attempts the
+    // allocation should fail.
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 3);
+    Lease6Collection leases = allocateTest(engine, pool_, IOAddress("::"),
+                                           false, true);
+    ASSERT_EQ(0, leases.size());
+
+    // This time, lets allow more attempts, and expect that the allocation will
+    // be successful.
+    AllocEngine engine2(AllocEngine::ALLOC_ITERATIVE, 6);
+    leases = allocateTest(engine2, pool_, IOAddress("::"), false, true);
+    ASSERT_EQ(1, leases.size());
+}
 
 }; // namespace test
 }; // namespace dhcp