[#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 23bc63a573e536e514ac5d79f05f34c526173a15..4fc17db9bfd8a342df6c2094aba4ff1faba80072 100644 (file)
--- 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.
 
     // Save list of subnets checked so we can skip rechecking
     // client classes for global HRs.

-    std::list<ConstSubnet4Ptr> subnets;
+    std::list<ConstSubnet4Ptr> eligible_subnets;

 
 

+    // Start with currently selected subnet.

     ConstSubnet4Ptr subnet = ctx.subnet_;
     while (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
         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);
     }
 
         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 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()) &&
             (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);
         }
     }
             return (true);
         }
     }

diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index 1939f5f7096f5fac4406d9b74d3ff65733774b92..9fc0df92581f36b0deafd3fec551e6ad9d7c2e83 100644 (file)
--- 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(),
         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);
         }
 
             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"),
 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"),
 
     // 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"),
 
     // 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"),
 
     // 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"),
 }
 
 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"),
 
     // 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"),
 
     // 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"),
 
     // 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
 }
 
 }  // namespace test

diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index dfd5251dac46eded75fe3aa555b6c6a4a61822d7..ba6db4923804bf8d6393e53fea3a461a2fe86ff0 100644 (file)
--- 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.
     /// 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,
     /// 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"),
 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"),
 
     // 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"),
 
     // 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"),
 
     // 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"),
 }
 
 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"),
 
     // 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"),
 
     // 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"),
 
     // 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
 }
 
 }  // namespace test