From: Marcin Siodelski Date: Wed, 13 Sep 2017 10:43:28 +0000 (+0200) Subject: [5306] Implemented getNextSubnet() including classes. X-Git-Tag: trac5363_base~21^2~11 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=8c0a4b5a5cceac5e17055b162ca56e203611348a;p=thirdparty%2Fkea.git [5306] Implemented getNextSubnet() including classes. --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 2e2eb5100d..d0395ecaf5 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/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; diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc index 955085f51d..e3808b4b5c 100644 --- a/src/lib/dhcpsrv/subnet.cc +++ b/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; diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index ba230db589..eff71ca9d2 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/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. /// diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index 6f9facd063..991bfdc3b5 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/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);