From: Francis Dupont Date: Wed, 27 Feb 2019 10:31:56 +0000 (+0100) Subject: [474-optionally-disable-collection-host-lookups] Added a host manager flag to disable... X-Git-Tag: Kea-1.6.0-beta~355 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0208373f19d2c1a7d4a3702653c614aeb28c5ea2;p=thirdparty%2Fkea.git [474-optionally-disable-collection-host-lookups] Added a host manager flag to disable getAll() in alloc engine --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 95f2e0d025..a39f2ee592 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2019 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -544,7 +544,10 @@ void AllocEngine::findReservation(ClientContext6& ctx) { // client rather than a query for each subnet within this shared network. // The only case when it is going to be less efficient is when there are // more host identifier types in use than subnets within a shared network. + // As it breaks RADIUS and host caching this can be disabled by the + // host manager. const bool use_single_query = network && + !HostMgr::instance().getPreventCollection() && (network->getAllSubnets()->size() > ctx.host_identifiers_.size()); if (use_single_query) { @@ -3070,7 +3073,10 @@ AllocEngine::findReservation(ClientContext4& ctx) { // client rather than a query for each subnet within this shared network. // The only case when it is going to be less efficient is when there are // more host identifier types in use than subnets within a shared network. + // As it breaks RADIUS and host caching this can be disabled by the + // host manager. const bool use_single_query = network && + !HostMgr::instance().getPreventCollection() && (network->getAllSubnets()->size() > ctx.host_identifiers_.size()); if (use_single_query) { diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h index 27f73b6ccf..501a1f5c07 100644 --- a/src/lib/dhcpsrv/host_mgr.h +++ b/src/lib/dhcpsrv/host_mgr.h @@ -427,6 +427,19 @@ public: negative_caching_ = negative_caching; } + /// @brief Returns the prevent collection flag. + /// + /// @return the prevent collection flag. + bool getPreventCollection() const { + return (prevent_collection_); + } + + /// @brief Sets the prevent collection flag. + /// + void setPreventCollection(bool prevent_collection) { + prevent_collection_ = prevent_collection; + } + protected: /// @brief The negative caching flag. /// @@ -435,6 +448,12 @@ protected: /// This works for get[46] for a subnet and an identifier. bool negative_caching_; + /// @brief The prevent collection flag. + /// + /// When true prevent the use of lookup methods returning a + /// collection when methods returning a single object are usable instead. + bool prevent_collection_; + /// @brief Cache an answer. /// /// @param host Pointer to the missied host. @@ -456,7 +475,7 @@ protected: private: /// @brief Private default constructor. - HostMgr() : negative_caching_(false) { } + HostMgr() : negative_caching_(false), prevent_collection_(false) { } /// @brief List of alternate host data sources. HostDataSourceList alternate_sources_; diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index a898717244..4cd5212655 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2019 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -839,6 +840,47 @@ TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkReservations) { EXPECT_EQ("10.2.3.23", lease->addr_.toText()); } +// Test that reservations within shared network take precedence over the +// existing leases regardless in which subnet belonging to a shared network +// reservations belong. Host lookups returning a collection are disabled. +TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkReservationsNoColl) { + + // Disable host lookups returning a collection. + ASSERT_FALSE(HostMgr::instance().getPreventCollection()); + HostMgr::instance().setPreventCollection(true); + + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, subnet2_->getID(), + SUBNET_ID_UNUSED, IOAddress("10.2.3.23"))); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + // Start allocation from subnet1. The engine should determine that the + // client has reservations in subnet2 and should rather assign reserved + // addresses. + AllocEngine::ClientContext4 + ctx(subnet1_, ClientIdPtr(), hwaddr_, IOAddress::IPV4_ZERO_ADDRESS(), + false, false, "host.example.com.", true); + 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()); + + // Let's create a lease for the client to make sure the lease is not + // renewed but a reserved lease is offerred. + Lease4Ptr lease2(new Lease4(IOAddress("192.0.2.17"), hwaddr_, ClientIdPtr(), + 501, 502, 503, time(NULL), subnet1_->getID())); + lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago + ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease2)); + ctx.subnet_ = subnet1_; + AllocEngine::findReservation(ctx); + lease = engine_.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ("10.2.3.23", lease->addr_.toText()); +} + // This test verifies that the server can offer an address from a shared // subnet if there's at least 1 address left there, but will not offer // anything if both subnets are completely full. @@ -1098,6 +1140,51 @@ TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkReservations) { EXPECT_EQ("10.2.3.23", lease->addr_.toText()); } +// Test that reservations within shared network take precedence over the +// existing leases regardless in which subnet belonging to a shared network +// reservations belong (DHCPREQUEST case). Host lookups returning a collection +// are disabled. +TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkReservationsNoColl) { + + // Disable host lookups returning a collection. + ASSERT_FALSE(HostMgr::instance().getPreventCollection()); + HostMgr::instance().setPreventCollection(true); + + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, subnet2_->getID(), + SUBNET_ID_UNUSED, IOAddress("10.2.3.23"))); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + // Start allocation from subnet1. The engine should determine that the + // client has reservations in subnet2 and should rather assign reserved + // addresses. + AllocEngine::ClientContext4 + ctx(subnet1_, ClientIdPtr(), hwaddr_, IOAddress::IPV4_ZERO_ADDRESS(), + false, false, "host.example.com.", false); + 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()); + + // Remove the lease for another test below. + ASSERT_TRUE(LeaseMgrFactory::instance().deleteLease(lease->addr_)); + + // Let's create a lease for the client to make sure the lease is not + // renewed but a reserved lease is allocated again. + Lease4Ptr lease2(new Lease4(IOAddress("192.0.2.17"), hwaddr_, ClientIdPtr(), + 501, 502, 503, time(NULL), subnet1_->getID())); + lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago + ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease2)); + ctx.subnet_ = subnet1_; + AllocEngine::findReservation(ctx); + lease = engine_.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ("10.2.3.23", lease->addr_.toText()); +} + // This test checks if an expired lease can be reused in DHCPDISCOVER (fake // allocation) TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) { diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index da90675c33..011fb3da89 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2019 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -2601,6 +2602,35 @@ TEST_F(SharedNetworkAlloc6Test, solicitSharedNetworkReservations) { ASSERT_EQ("2001:db8:2::15", lease->addr_.toText()); } +// This test verifies that the client is offerred a reserved address +// even if this address belongs to another subnet within the same +// shared network. Host lookups returning a collection are disabled. +TEST_F(SharedNetworkAlloc6Test, solicitSharedNetworkReservationsNoColl) { + // Disable host lookups returning a collection. + ASSERT_FALSE(HostMgr::instance().getPreventCollection()); + HostMgr::instance().setPreventCollection(true); + + // Create reservation for the client in the second subnet. + subnet_ = subnet2_; + createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:2::15"), 128); + + // Start allocation from subnet1_. The engine should determine that the + // client has reservations in subnet2_ and should rather assign reserved + // addresses. + Pkt6Ptr query(new Pkt6(DHCPV6_SOLICIT, 1234)); + AllocEngine::ClientContext6 ctx(subnet1_, duid_, false, false, "", true, + query); + ctx.currentIA().iaid_ = iaid_; + + // Find reservations for this subnet/shared network. + AllocEngine::findReservation(ctx); + + Lease6Ptr lease; + ASSERT_NO_THROW(lease = expectOneLease(engine_.allocateLeases6(ctx))); + ASSERT_TRUE(lease); + ASSERT_EQ("2001:db8:2::15", lease->addr_.toText()); +} + // This test verifies that the client is allocated a reserved address // even if this address belongs to another subnet within the same // shared network. @@ -2626,6 +2656,35 @@ TEST_F(SharedNetworkAlloc6Test, requestSharedNetworkReservations) { ASSERT_EQ("2001:db8:2::15", lease->addr_.toText()); } +// This test verifies that the client is allocated a reserved address +// even if this address belongs to another subnet within the same +// shared network. Host lookups returning a collection are disabled. +TEST_F(SharedNetworkAlloc6Test, requestSharedNetworkReservationsNoColl) { + // Disable host lookups returning a collection. + ASSERT_FALSE(HostMgr::instance().getPreventCollection()); + HostMgr::instance().setPreventCollection(true); + + // Create reservation for the client in the second subnet. + subnet_ = subnet2_; + createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:2::15"), 128); + + // Start allocation from subnet1_. The engine should determine that the + // client has reservations in subnet2_ and should rather assign reserved + // addresses. + Pkt6Ptr query(new Pkt6(DHCPV6_REQUEST, 1234)); + AllocEngine::ClientContext6 ctx(subnet1_, duid_, false, false, "", false, + query); + ctx.currentIA().iaid_ = iaid_; + + // Find reservations for this subnet/shared network. + AllocEngine::findReservation(ctx); + + Lease6Ptr lease; + ASSERT_NO_THROW(lease = expectOneLease(engine_.allocateLeases6(ctx))); + ASSERT_TRUE(lease); + ASSERT_EQ("2001:db8:2::15", lease->addr_.toText()); +} + // This test verifies that client is assigned an existing lease from a // shared network, regardless of the default subnet. It also verifies that // the client is assigned a reserved address from a shared network which