From: Thomas Markwalder Date: Thu, 30 Jul 2026 10:42:26 +0000 (-0400) Subject: [#4678] Addressed review comments X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;ds=sidebyside;p=thirdparty%2Fkea.git [#4678] Addressed review comments Minor nits: modified: src/lib/dhcpsrv/alloc_engine.cc modified: src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc modified: src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 23bc63a573..4fc17db9bf 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -3683,11 +3683,12 @@ hasAddressReservation(AllocEngine::ClientContext4& ctx) { // Save list of subnets checked so we can skip rechecking // client classes for global HRs. - std::list subnets; + std::list eligible_subnets; + // Start with currently selected subnet. ConstSubnet4Ptr subnet = ctx.subnet_; while (subnet) { - subnets.push_back(subnet); + eligible_subnets.push_back(subnet); if (subnet->getReservationsInSubnet()) { auto host = ctx.hosts_.find(subnet->getID()); // The out-of-pool flag indicates that no client should be assigned @@ -3726,15 +3727,17 @@ hasAddressReservation(AllocEngine::ClientContext4& ctx) { return (false); } - // Start with currently selected subnet. - for (auto subnet : subnets) { + // Iterate over the list of eligible subnets (starting with + // with the currently selected subnet) looking for a subnet + // in which the global host address is valid. + for (auto eligible_subnet : eligible_subnets) { // If global reservations are enabled for this subnet and there is // globally reserved address and that address is feasible for this // subnet, update the selected subnet and return true. - if (subnet->getReservationsGlobal() && + if (eligible_subnet->getReservationsGlobal() && (global_host_address != IOAddress::IPV4_ZERO_ADDRESS()) && - (subnet->inRange(global_host_address))) { - ctx.subnet_ = subnet; + (eligible_subnet->inRange(global_host_address))) { + ctx.subnet_ = eligible_subnet; return (true); } } diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index 1939f5f709..9fc0df9258 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -1022,8 +1022,8 @@ public: HostPtr subnet_host; if (include_subnet_host) { subnet_host.reset(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), - Host::IDENT_HWADDR, subnet_host_subnet_id, - SUBNET_ID_UNUSED, subnet_host_address)); + Host::IDENT_HWADDR, subnet_host_subnet_id, + SUBNET_ID_UNUSED, subnet_host_address)); CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(subnet_host); } @@ -6203,45 +6203,45 @@ TEST_F(AllocEngine4Test, getOfferLft4) { TEST_F(SharedNetworkAlloc4Test, discoverGlobalResVsSubnetRes) { // global_host with address in subnet1, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("192.0.2.100"), - true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), - SubnetID(20), IOAddress("10.1.2.5")); + true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), + SubnetID(20), IOAddress("10.1.2.5")); // global_host with address in subnet1, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("192.0.2.100"), - true, SubnetID(20), IOAddress("10.1.2.200"), - SubnetID(20), IOAddress("10.1.2.200")); + true, SubnetID(20), IOAddress("10.1.2.200"), + SubnetID(20), IOAddress("10.1.2.200")); // global_host with address in subnet2, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("10.1.2.1"), - true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), - SubnetID(20), IOAddress("10.1.2.6")); + true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), + SubnetID(20), IOAddress("10.1.2.6")); // global_host with address in subnet2, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("10.1.2.1"), - true, SubnetID(20), IOAddress("10.1.2.200"), - SubnetID(20), IOAddress("10.1.2.200")); + true, SubnetID(20), IOAddress("10.1.2.200"), + SubnetID(20), IOAddress("10.1.2.200")); } TEST_F(SharedNetworkAlloc4Test, requestGlobalResVsSubnetRes) { // global_host with address in subnet1, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("192.0.2.100"), - true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), - SubnetID(20), IOAddress("10.1.2.5"), false); + true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), + SubnetID(20), IOAddress("10.1.2.5"), false); // global_host with address in subnet1, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("192.0.2.100"), - true, SubnetID(20), IOAddress("10.1.2.200"), - SubnetID(20), IOAddress("10.1.2.200"), false); + true, SubnetID(20), IOAddress("10.1.2.200"), + SubnetID(20), IOAddress("10.1.2.200"), false); // global_host with address in subnet2, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("10.1.2.1"), - true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), - SubnetID(20), IOAddress("10.1.2.6"), false); + true, SubnetID(20), IOAddress::IPV4_ZERO_ADDRESS(), + SubnetID(20), IOAddress("10.1.2.6"), false); // global_host with address in subnet2, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("10.1.2.1"), - true, SubnetID(20), IOAddress("10.1.2.200"), - SubnetID(20), IOAddress("10.1.2.200"), false); + true, SubnetID(20), IOAddress("10.1.2.200"), + SubnetID(20), IOAddress("10.1.2.200"), false); } } // namespace test diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index dfd5251dac..ba6db49238 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc @@ -3239,7 +3239,7 @@ public: /// none if it is IPV6_ZERO_ADDRESS. /// @param exp_subnet expected subnet of the resultant lease. /// @param exp_address expected address of the resultant lease. - /// @param fake_allocation tests the DISCOVER code path when true, REQUEST when + /// @param fake_allocation tests the SOLICIT code path when true, REQUEST when /// false. void globalResVsSubnetRes(bool include_global_host, IOAddress global_host_address, bool include_subnet_host, SubnetID subnet_host_subnet_id, @@ -6940,45 +6940,45 @@ TEST_F(AllocEngine6Test, useReclaimedReservedLease) { TEST_F(SharedNetworkAlloc6Test, solicitGlobalResVsSubnetRes) { // global_host with address in subnet1, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:1::100"), - true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), - SubnetID(20), IOAddress("2001:db8:2::")); + true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), + SubnetID(20), IOAddress("2001:db8:2::")); // global_host with address in subnet1, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:1::100"), - true, SubnetID(20), IOAddress("2001:db8:2::200"), - SubnetID(20), IOAddress("2001:db8:2::200")); + true, SubnetID(20), IOAddress("2001:db8:2::200"), + SubnetID(20), IOAddress("2001:db8:2::200")); // global_host with address in subnet2, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:2::100"), - true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), - SubnetID(20), IOAddress("2001:db8:2::1")); + true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), + SubnetID(20), IOAddress("2001:db8:2::1")); // global_host with address in subnet2, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:2::100"), - true, SubnetID(20), IOAddress("2001:db8:2::200"), - SubnetID(20), IOAddress("2001:db8:2::200")); + true, SubnetID(20), IOAddress("2001:db8:2::200"), + SubnetID(20), IOAddress("2001:db8:2::200")); } TEST_F(SharedNetworkAlloc6Test, requestGlobalResVsSubnetRes) { // global_host with address in subnet1, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:1::100"), - true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), - SubnetID(20), IOAddress("2001:db8:2::"), false); + true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), + SubnetID(20), IOAddress("2001:db8:2::"), false); // global_host with address in subnet1, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:1::100"), - true, SubnetID(20), IOAddress("2001:db8:2::200"), - SubnetID(20), IOAddress("2001:db8:2::200"), false); + true, SubnetID(20), IOAddress("2001:db8:2::200"), + SubnetID(20), IOAddress("2001:db8:2::200"), false); // global_host with address in subnet2, subnet_host with no address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:2::100"), - true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), - SubnetID(20), IOAddress("2001:db8:2::1"), false); + true, SubnetID(20), IOAddress::IPV6_ZERO_ADDRESS(), + SubnetID(20), IOAddress("2001:db8:2::1"), false); // global_host with address in subnet2, subnet_host with address in subnet2 globalResVsSubnetRes(true, IOAddress("2001:db8:2::100"), - true, SubnetID(20), IOAddress("2001:db8:2::200"), - SubnetID(20), IOAddress("2001:db8:2::200"), false); + true, SubnetID(20), IOAddress("2001:db8:2::200"), + SubnetID(20), IOAddress("2001:db8:2::200"), false); } } // namespace test