diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e30811767d..6ade319bd7 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/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 diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index aaf49ddd57..9f29e9d03f 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/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 diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 1bbf0ca699..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,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); } 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 b4251a8deb..0cf9df98f1 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/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 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(12, + static_cast(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