[5306] Implemented getNextSubnet() including classes.

src/lib/dhcpsrv/alloc_engine.cc

@@ -329,7 +329,7 @@ AllocEngine::findReservationInternal(ContextType& ctx,
             }
         }
 
-        subnet = subnet->getNextSubnet(ctx.subnet_);
+        subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses());
     }
 }
 
@@ -2192,7 +2192,7 @@ hasAddressReservation(AllocEngine::ClientContext4& ctx) {
 
         // No address reservation found here, so let's try another subnet
         // within the same shared network.
-        subnet = subnet->getNextSubnet(ctx.subnet_);
+        subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses());
     }
 
     return (false);
@@ -2224,13 +2224,6 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease)
 
     while (subnet) {
 
-        // Some of the subnets within a shared network may not be allowed
-        // for the client if classification restrictions have been applied.
-        if (!subnet->clientSupported(ctx.query_->getClasses())) {
-            subnet = subnet->getNextSubnet(original_subnet);
-            continue;
-        }
-
         // If client identifier has been supplied, use it to lookup the lease. This
         // search will return no lease if the client doesn't have any lease in the
         // database or if the client didn't use client identifier to allocate the
@@ -2256,20 +2249,17 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease)
                     existing_lease->belongsToClient(ctx.hwaddr_, ctx.clientid_)) {
                     // Found the lease of this client, so return it.
                     client_lease = existing_lease;
-                    break;
+                    // We got a lease but the subnet it belongs to may differ from
+                    // the original subnet. Let's now stick to this subnet.
+                    ctx.subnet_ = subnet;
+                    return;
                 }
             }
         }
 
-        if (client_lease || !network) {
-            // We got the leases but the subnet they belong to may differ from
-            // the original subnet. Let's now stick to this subnet.
-            ctx.subnet_ = subnet;
-            subnet.reset();
-
-        } else {
-            subnet = subnet->getNextSubnet(original_subnet);
-        }
+        // Haven't found any lease in this subnet, so let's try another subnet
+        // within the shared network.
+        subnet = subnet->getNextSubnet(original_subnet, ctx.query_->getClasses());
     }
 }
 
@@ -2294,17 +2284,16 @@ inAllowedPool(AllocEngine::ClientContext4& ctx, const IOAddress& address) {
     Subnet4Ptr current_subnet = ctx.subnet_;
     while (current_subnet) {
 
-        if (current_subnet->clientSupported(ctx.query_->getClasses())) {
-            if (current_subnet->inPool(Lease::TYPE_V4, address)) {
-                // We found a subnet that this address belongs to, so it
-                // seems that this subnet is the good candidate for allocation.
-                // Let's update the selected subnet.
-                ctx.subnet_ = current_subnet;
-                return (true);
-            }
+        if (current_subnet->inPool(Lease::TYPE_V4, address)) {
+            // We found a subnet that this address belongs to, so it
+            // seems that this subnet is the good candidate for allocation.
+            // Let's update the selected subnet.
+            ctx.subnet_ = current_subnet;
+            return (true);
         }
 
-        current_subnet = current_subnet->getNextSubnet(ctx.subnet_);
+        current_subnet = current_subnet->getNextSubnet(ctx.subnet_,
+                                                       ctx.query_->getClasses());
     }
 
     return (false);
@@ -2365,6 +2354,16 @@ AllocEngine::allocateLease4(ClientContext4& ctx) {
 
     Lease4Ptr new_lease;
 
+    // Before we start allocation process, we need to make sure that the
+    // selected subnet is allowed for this client. If not, we'll try to
+    // use some other subnet within the shared network. If there are no
+    // subnets allowed for this client within the shared network, we
+    // can't allocate a lease.
+    Subnet4Ptr subnet = ctx.subnet_;
+    if (subnet && !subnet->clientSupported(ctx.query_->getClasses())) {
+        ctx.subnet_ = subnet->getNextSubnet(subnet, ctx.query_->getClasses());
+    }
+
     try {
         if (!ctx.subnet_) {
             isc_throw(BadValue, "Can't allocate IPv4 address without subnet");
@@ -3000,13 +2999,6 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) {
     uint64_t total_attempts = 0;
     while (subnet) {
 
-        // Some of the subnets within a shared network may not be allowed
-        // for the client if classification restrictions have been applied.
-        if (!subnet->clientSupported(ctx.query_->getClasses())) {
-            subnet = subnet->getNextSubnet(original_subnet);
-            continue;
-        }
-
         const uint64_t max_attempts = (attempts_ > 0 ? attempts_ :
                                        subnet->getPoolCapacity(Lease::TYPE_V4));
         for (uint64_t i = 0; i < max_attempts; ++i) {
@@ -3033,7 +3025,7 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) {
 
         // This pointer may be set to NULL if hooks set SKIP status.
         if (subnet) {
-            subnet = subnet->getNextSubnet(original_subnet);
+            subnet = subnet->getNextSubnet(original_subnet, ctx.query_->getClasses());
 
             if (subnet) {
                 ctx.subnet_ = subnet;

src/lib/dhcpsrv/subnet.cc

@@ -188,6 +188,34 @@ Subnet4::getNextSubnet(const Subnet4Ptr& first_subnet) const {
     return (Subnet4Ptr());
 }
 
+Subnet4Ptr
+Subnet4::getNextSubnet(const Subnet4Ptr& first_subnet,
+                       const ClientClasses& client_classes) const {
+    SharedNetwork4Ptr network;
+    getSharedNetwork(network);
+    // We can only get next subnet if shared network has been defined for
+    // the current subnet.
+    if (network) {
+        Subnet4Ptr subnet;
+        do {
+            // Use subnet identifier of this subnet if this is the first
+            // time we're calling getNextSubnet. Otherwise, use the
+            // subnet id of the previously returned subnet.
+            SubnetID subnet_id = subnet ? subnet->getID() : getID();
+            subnet = network->getNextSubnet(first_subnet, subnet_id);
+            // If client classes match the subnet, return it. Otherwise,
+            // try another subnet.
+            if (subnet && subnet->clientSupported(client_classes)) {
+                return (subnet);
+            }
+        } while (subnet);
+    }
+
+    // No subnet found.
+    return (Subnet4Ptr());
+}
+
+
 bool
 Subnet4::clientSupported(const isc::dhcp::ClientClasses& client_classes) const {
     NetworkPtr network;
@@ -473,6 +501,33 @@ Subnet6::getNextSubnet(const Subnet6Ptr& first_subnet) const {
     return (Subnet6Ptr());
 }
 
+Subnet6Ptr
+Subnet6::getNextSubnet(const Subnet6Ptr& first_subnet,
+                       const ClientClasses& client_classes) const {
+    SharedNetwork6Ptr network;
+    getSharedNetwork(network);
+    // We can only get next subnet if shared network has been defined for
+    // the current subnet.
+    if (network) {
+        Subnet6Ptr subnet;
+        do {
+            // Use subnet identifier of this subnet if this is the first
+            // time we're calling getNextSubnet. Otherwise, use the
+            // subnet id of the previously returned subnet.
+            SubnetID subnet_id = subnet ? subnet->getID() : getID();
+            subnet = network->getNextSubnet(first_subnet, subnet_id);
+            // If client classes match the subnet, return it. Otherwise,
+            // try another subnet.
+            if (subnet && subnet->clientSupported(client_classes)) {
+                return (subnet);
+            }
+        } while (subnet);
+    }
+
+    // No subnet found.
+    return (Subnet6Ptr());
+}
+
 bool
 Subnet6::clientSupported(const isc::dhcp::ClientClasses& client_classes) const {
     NetworkPtr network;

src/lib/dhcpsrv/subnet.h

@@ -416,6 +416,21 @@ public:
     /// shared network.
     Subnet4Ptr getNextSubnet(const Subnet4Ptr& first_subnet) const;
 
+    /// @brief Returns next subnet within shared network that matches
+    /// client classes.
+    ///
+    /// @param first_subnet Pointer to the subnet from which iterations have
+    /// started.
+    /// @param client_classes List of classes that the client belongs to.
+    /// The subnets not matching the classes aren't returned by this
+    /// method.
+    ///
+    /// @return Pointer to the next subnet or NULL pointer if the next subnet
+    /// is the first subnet or if the current subnet doesn't belong to a
+    /// shared network.
+    Subnet4Ptr getNextSubnet(const Subnet4Ptr& first_subnet,
+                             const ClientClasses& client_classes) const;
+
     /// @brief Checks whether this subnet and parent shared network supports
     /// the client that belongs to specified classes.
     ///
@@ -533,6 +548,21 @@ public:
     /// shared network.
     Subnet6Ptr getNextSubnet(const Subnet6Ptr& first_subnet) const;
 
+    /// @brief Returns next subnet within shared network that matches
+    /// client classes.
+    ///
+    /// @param first_subnet Pointer to the subnet from which iterations have
+    /// started.
+    /// @param client_classes List of classes that the client belongs to.
+    /// The subnets not matching the classes aren't returned by this
+    /// method.
+    ///
+    /// @return Pointer to the next subnet or NULL pointer if the next subnet
+    /// is the first subnet or if the current subnet doesn't belong to a
+    /// shared network.
+    Subnet6Ptr getNextSubnet(const Subnet6Ptr& first_subnet,
+                             const ClientClasses& client_classes) const;
+
     /// @brief Checks whether this subnet and parent shared network supports
     /// the client that belongs to specified classes.
     ///

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc

@@ -591,6 +591,21 @@ TEST_F(AllocEngine4Test, discoverSharedNetworkClassification) {
     ASSERT_TRUE(lease);
     EXPECT_TRUE(subnet2->inPool(Lease::TYPE_V4, lease->addr_));
 
+    // Create reservation for the client in subnet1. Because this subnet is
+    // not allowed for the client the client should still be offerred a
+    // lease from subnet2.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet1->getID(),
+                          SubnetID(0), IOAddress("192.0.2.17")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+    AllocEngine::findReservation(ctx);
+
+    ctx.subnet_ = subnet1;
+    lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(lease);
+    EXPECT_TRUE(subnet2->inPool(Lease::TYPE_V4, lease->addr_));
+
     // Assign cable-modem class and try again. This time, we should
     // offer an address from the subnet1.
     ctx.query_->addClass(ClientClass("cable-modem"));
@@ -598,7 +613,7 @@ TEST_F(AllocEngine4Test, discoverSharedNetworkClassification) {
     ctx.subnet_ = subnet1;
     lease = engine.allocateLease4(ctx);
     ASSERT_TRUE(lease);
-    EXPECT_TRUE(subnet1->inPool(Lease::TYPE_V4, lease->addr_));
+    EXPECT_EQ("192.0.2.17", lease->addr_.toText());
 }
 
 // Test that reservations within shared network take precedence over the
@@ -636,8 +651,8 @@ TEST_F(AllocEngine4Test, discoverSharedNetworkReservations) {
     AllocEngine::ClientContext4
         ctx(subnet1, ClientIdPtr(), hwaddr_, IOAddress::IPV4_ZERO_ADDRESS(),
             false, false, "host.example.com.", true);
-    AllocEngine::findReservation(ctx);
     ctx.query_.reset(new Pkt4(DHCPDISCOVER, 1234));
+    AllocEngine::findReservation(ctx);
     Lease4Ptr lease = engine.allocateLease4(ctx);
     ASSERT_TRUE(lease);
     EXPECT_EQ("10.2.3.23", lease->addr_.toText());
@@ -834,8 +849,8 @@ TEST_F(AllocEngine4Test, requestSharedNetworkReservations) {
     AllocEngine::ClientContext4
         ctx(subnet1, ClientIdPtr(), hwaddr_, IOAddress::IPV4_ZERO_ADDRESS(),
             false, false, "host.example.com.", false);
-    AllocEngine::findReservation(ctx);
     ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234));
+    AllocEngine::findReservation(ctx);
     Lease4Ptr lease = engine.allocateLease4(ctx);
     ASSERT_TRUE(lease);
     EXPECT_EQ("10.2.3.23", lease->addr_.toText());
@@ -2070,6 +2085,7 @@ TEST_F(AllocEngine4Test, findReservation) {
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
                                     IOAddress("0.0.0.0"), false, false,
                                     "", false);
+    ctx.query_.reset(new Pkt4(DHCPDISCOVER, 1234));
     ctx.addHostIdentifier(Host::IDENT_HWADDR, hwaddr_->hwaddr_);
     ctx.addHostIdentifier(Host::IDENT_DUID, clientid_->getDuid());
 
@@ -2227,8 +2243,8 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseStat) {
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
                                     IOAddress("192.0.2.123"), false, false,
                                     "", false);
-    AllocEngine::findReservation(ctx);
     ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234));
+    AllocEngine::findReservation(ctx);
 
     Lease4Ptr allocated_lease = engine.allocateLease4(ctx);