From e46618e63aaf9adaeaa506254534c48e6e2fe77c Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 10 Aug 2018 08:56:21 -0400 Subject: [PATCH] [5705] AllocEngine4 now matches clients to global HRs src/bin/dhcp4/tests/Makefile.am src/bin/dhcp4/tests/host_unittest.cc - new file with global HR tests src/lib/dhcpsrv/alloc_engine.* findGlobalReservation() - new function that searches for global HR findReservation() - calls findGlobalReservation() if mode is global hasAddressReservation() ClientContext4::currentHost() - modified to recognize global HRs src/lib/dhcpsrv/network.h Added new mode, Netork::HR_GLOBAL src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc TEST_F(AllocEngine4Test, globalReservationReservedAddressDiscover) TEST_F(AllocEngine4Test, globalReservationReservedAddressRequest) TEST_F(AllocEngine4Test, globalReservationDynamicDiscover) TEST_F(AllocEngine4Test, globalReservationDynamicRequest) - new tests --- src/bin/dhcp4/tests/Makefile.am | 1 + src/bin/dhcp4/tests/host_unittest.cc | 155 ++++++++++++++ src/lib/dhcpsrv/alloc_engine.cc | 42 +++- src/lib/dhcpsrv/alloc_engine.h | 11 + src/lib/dhcpsrv/network.h | 5 + .../dhcpsrv/tests/alloc_engine4_unittest.cc | 201 ++++++++++++++++++ 6 files changed, 414 insertions(+), 1 deletion(-) create mode 100644 src/bin/dhcp4/tests/host_unittest.cc diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index 0c31643ae6..d9d58779d9 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -102,6 +102,7 @@ dhcp4_unittests_SOURCES += dhcp4to6_ipc_unittest.cc dhcp4_unittests_SOURCES += simple_parser4_unittest.cc dhcp4_unittests_SOURCES += get_config_unittest.cc get_config_unittest.h dhcp4_unittests_SOURCES += shared_network_unittest.cc +dhcp4_unittests_SOURCES += host_unittest.cc nodist_dhcp4_unittests_SOURCES = marker_file.h test_libraries.h diff --git a/src/bin/dhcp4/tests/host_unittest.cc b/src/bin/dhcp4/tests/host_unittest.cc new file mode 100644 index 0000000000..d05f229b2f --- /dev/null +++ b/src/bin/dhcp4/tests/host_unittest.cc @@ -0,0 +1,155 @@ +// Copyright (C) 2018 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 +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace isc; +using namespace isc::asiolink; +using namespace isc::data; +using namespace isc::dhcp; +using namespace isc::dhcp::test; + +namespace { + +/// @brief Set of JSON configuration(s) used throughout the Host tests. +/// +/// - Configuration 0: +/// - Used for testing global host reservations +/// - 5 global reservations +/// - 1 subnet: 10.0.0.0/24 +const char* CONFIGS[] = { +// Configuration 4 + "{ \"interfaces-config\": {\n" + " \"interfaces\": [ \"*\" ]\n" + "},\n" + "\"host-reservation-identifiers\": [ \"circuit-id\", \"hw-address\",\n" + " \"duid\", \"client-id\" ],\n" + "\"reservations\": [ \n" + "{\n" + " \"hw-address\": \"aa:bb:cc:dd:ee:ff\",\n" + " \"hostname\": \"hw-host-dynamic\"\n" + "},\n" + "{\n" + " \"hw-address\": \"01:02:03:04:05:06\",\n" + " \"hostname\": \"hw-host-fixed\",\n" + " \"ip-address\": \"10.0.0.7\"\n" + "},\n" + "{\n" + " \"duid\": \"01:02:03:04:05\",\n" + " \"hostname\": \"duid-host\"\n" + "},\n" + "{\n" + " \"circuit-id\": \"'charter950'\",\n" + " \"hostname\": \"circuit-id-host\"\n" + "},\n" + "{\n" + " \"client-id\": \"01:11:22:33:44:55:66\",\n" + " \"hostname\": \"client-id-host\"\n" + "}\n" + "],\n" + "\"valid-lifetime\": 600,\n" + "\"subnet4\": [ { \n" + " \"subnet\": \"10.0.0.0/24\", \n" + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]\n" + "} ]\n" + "}\n" +}; + +/// @brief Test fixture class for testing v4 exchanges. +class HostTest : public Dhcpv4SrvTest { +public: + + /// @brief Constructor. + /// + /// Sets up fake interfaces. + HostTest() + : Dhcpv4SrvTest(), + iface_mgr_test_config_(true) { + IfaceMgr::instance().openSockets4(); + + // Let's wipe all existing statistics. + isc::stats::StatsMgr::instance().removeAll(); + } + + /// @brief Destructor. + /// + /// Cleans up statistics after the test. + ~HostTest() { + // Let's wipe all existing statistics. + isc::stats::StatsMgr::instance().removeAll(); + } + + /// @brief Interface Manager's fake configuration control. + IfaceMgrTestConfig iface_mgr_test_config_; + + void runDoraTest(const std::string& config, Dhcp4Client& client, + const std::string& expected_host, + const std::string& expected_addr) { + + // Configure DHCP server. + configure(config, *client.getServer()); + client.requestOptions(DHO_HOST_NAME); + + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + ASSERT_NO_THROW(client.doDORA()); + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + + // Fetch the hostname option + OptionStringPtr hostname = boost::dynamic_pointer_cast< + OptionString>(resp->getOption(DHO_HOST_NAME)); + + if (expected_host.empty()) { + ASSERT_FALSE(hostname); + } else { + ASSERT_TRUE(hostname); + EXPECT_EQ(expected_host, hostname->getValue()); + } + + EXPECT_EQ(client.config_.lease_.addr_.toText(), expected_addr); + } + + +}; + +TEST_F(HostTest, globalHardwareDynamicAddress) { + Dhcp4Client client(Dhcp4Client::SELECTING); + + client.setHWAddress("aa:bb:cc:dd:ee:ff"); + runDoraTest(CONFIGS[0], client, "hw-host-dynamic", "10.0.0.10"); +} + + +TEST_F(HostTest, globalHardwareFixedAddress) { + Dhcp4Client client(Dhcp4Client::SELECTING); + + //client.includeClientId(clientid_a); + client.setHWAddress("01:02:03:04:05:06"); + runDoraTest(CONFIGS[0], client, "hw-host-fixed", "10.0.0.7"); +} + +} // end of anonymous namespace diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index c48f3857f5..90b9058de4 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -2628,6 +2628,14 @@ hasAddressReservation(AllocEngine::ClientContext4& ctx) { Subnet4Ptr subnet = ctx.subnet_; while (subnet) { + if (subnet->getHostReservationMode() == Network::HR_GLOBAL) { + auto host = ctx.hosts_.find(SUBNET_ID_GLOBAL); + return (host != ctx.hosts_.end() && + !(host->second->getIPv4Reservation().isV4Zero())); + // if we want global + other modes we would need to + // return only if true, else continue + } + auto host = ctx.hosts_.find(subnet->getID()); if ((host != ctx.hosts_.end()) && !(host->second->getIPv4Reservation().isV4Zero()) && @@ -2811,7 +2819,10 @@ AllocEngine::ClientContext4::ClientContext4(const Subnet4Ptr& subnet, ConstHostPtr AllocEngine::ClientContext4::currentHost() const { if (subnet_) { - auto host = hosts_.find(subnet_->getID()); + SubnetID id = (subnet_->getHostReservationMode() == Network::HR_GLOBAL ? + SUBNET_ID_GLOBAL : subnet_->getID()); + + auto host = hosts_.find(id); if (host != hosts_.cend()) { return (host->second); } @@ -2878,6 +2889,17 @@ AllocEngine::findReservation(ClientContext4& ctx) { SharedNetwork4Ptr network; subnet->getSharedNetwork(network); + if (subnet->getHostReservationMode() == Network::HR_GLOBAL) { + ConstHostPtr ghost = findGlobalReservation(ctx); + if (ghost) { + ctx.hosts_[SUBNET_ID_GLOBAL] = ghost; + + // @todo In theory, to support global as part of HR_ALL, + // we would just keep going, instead of returning. + return; + } + } + // If the subnet belongs to a shared network it is usually going to be // more efficient to make a query for all reservations for a particular // client rather than a query for each subnet within this shared network. @@ -2942,6 +2964,24 @@ AllocEngine::findReservation(ClientContext4& ctx) { } } +ConstHostPtr +AllocEngine::findGlobalReservation(ClientContext4& ctx) { + ConstHostPtr host; + BOOST_FOREACH(const IdentifierPair& id_pair, ctx.host_identifiers_) { + // Attempt to find a host using a specified identifier. + host = HostMgr::instance().get4(SUBNET_ID_GLOBAL, id_pair.first, + &id_pair.second[0], id_pair.second.size()); + + // If we found matching global host we're done. + if (host) { + break; + } + } + + return (host); +} + + Lease4Ptr AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) { // Find an existing lease for this client. This function will return true diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index 3a64eb91a4..198e32c9cd 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -1299,6 +1299,17 @@ public: /// @param ctx Client context holding various information about the client. static void findReservation(ClientContext4& ctx); + /// @brief Attempts to find the host reservation for the client. + /// + /// This method attempts to find a "global" host reservation matching the + /// client identifier. It will return the first global reservation that matches + /// per the configured list of host identifiers, or an empty pointer if no + /// matches are found. + /// + /// @param ctx Client context holding various information about the client. + /// @return Pointer to the reservation found, or an empty pointer. + static ConstHostPtr findGlobalReservation(ClientContext4& ctx); + private: /// @brief Offers the lease. diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h index 5b51732b4b..ce253cb758 100644 --- a/src/lib/dhcpsrv/network.h +++ b/src/lib/dhcpsrv/network.h @@ -96,11 +96,16 @@ public: /// dealing with with addresses that are in pool. HR_OUT_OF_POOL, + /// Only global reservations are allowed. This mode + /// instructs AllocEngine to only look at global reservations. + HR_GLOBAL, + /// Both out-of-pool and in-pool reservations are allowed. This is the /// most flexible mode, where sysadmin have biggest liberty. However, /// there is a non-trivial performance penalty for it, as the /// AllocEngine code has to check whether there are reservations, even /// when dealing with reservations from within the dynamic pools. + /// @todo - should ALL include global? HR_ALL } HRMode; diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index 6d8e2776a6..a898717244 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -2486,6 +2486,207 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseStat) { EXPECT_FALSE(ctx.fake_allocation_); } +// This test checks the behavior of the allocation engine in the following +// scenario: +// - Client has no lease in the database. +// - Client has a global reservation. +// - Client sends DISCOVER +// - Client is allocated the reserved address. +// - Lease is not added to the lease database +TEST_F(AllocEngine4Test, globalReservationReservedAddressDiscover) { + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, SUBNET_ID_GLOBAL, + SUBNET_ID_UNUSED, IOAddress("192.0.77.123"))); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false); + + subnet_->setHostReservationMode(Network::HR_GLOBAL); + + // Query allocation engine for the lease to be assigned to this + // client without specifying the address to be assigned. + AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, + IOAddress("0.0.0.0"), false, false, + "", true); + ctx.query_.reset(new Pkt4(DHCPDISCOVER, 1234)); + + // Look up the host. + AllocEngine::findReservation(ctx); + + // We should have the correct current host + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getHostname(), host->getHostname()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); + + // We should allocate the reserverd address. + Lease4Ptr lease = engine.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ("192.0.77.123", lease->addr_.toText()); + + // This is a "fake" allocation so the returned lease should not be committed + // to the lease database. + EXPECT_FALSE(LeaseMgrFactory::instance().getLease4(lease->addr_)); + + // Client had no lease in the database, so the old lease returned should + // be NULL. + EXPECT_FALSE(ctx.old_lease_); +} + +// This test checks the behavior of the allocation engine in the following +// scenario: +// - Client has no lease in the database. +// - Client has a global reservation. +// - Client sends REQUEST +// - Client is allocated the reserved address. +// - Lease is added to the lease database +TEST_F(AllocEngine4Test, globalReservationReservedAddressRequest) { + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, SUBNET_ID_GLOBAL, + SUBNET_ID_UNUSED, IOAddress("192.0.77.123"))); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false); + + subnet_->setHostReservationMode(Network::HR_GLOBAL); + + // Query allocation engine for the lease to be assigned to this + // client without specifying the address to be assigned. + AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, + IOAddress("0.0.0.0"), false, false, + "", false); + ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234)); + + // Look up the host. + AllocEngine::findReservation(ctx); + + // We should have the correct current host + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getHostname(), host->getHostname()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); + + // We should allocate the reserverd address. + Lease4Ptr lease = engine.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ("192.0.77.123", lease->addr_.toText()); + + // Check that the lease is indeed in LeaseMgr + Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_); + ASSERT_TRUE(from_mgr); + + // Now check that the lease in LeaseMgr has the same parameters + detailCompareLease(lease, from_mgr); + + // Client had no lease in the database, so the old lease returned should + // be NULL. + EXPECT_FALSE(ctx.old_lease_); +} + + +// This test checks the behavior of the allocation engine in the following +// scenario: +// - Client has no lease in the database. +// - Client has a global reservation. +// - Client sends DISCOVER +// - Client is allocated a dynamic address from matched subnet +// - Lease is not added to the lease database +TEST_F(AllocEngine4Test, globalReservationDynamicDiscover) { + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, SUBNET_ID_GLOBAL, + SUBNET_ID_UNUSED, IOAddress::IPV4_ZERO_ADDRESS(), + "foo.example.org")); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false); + + subnet_->setHostReservationMode(Network::HR_GLOBAL); + + // Query allocation engine for the lease to be assigned to this + // client without specifying the address to be assigned. + AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, + IOAddress("0.0.0.0"), false, false, + "", true); + ctx.query_.reset(new Pkt4(DHCPDISCOVER, 1234)); + + // Look up the host. + AllocEngine::findReservation(ctx); + + // We should have the correct current host + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getHostname(), host->getHostname()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); + + // We should allocate a dynamic address. + Lease4Ptr lease = engine.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ("192.0.2.100", lease->addr_.toText()); + + // This is a "fake" allocation so the returned lease should not be committed + // to the lease database. + EXPECT_FALSE(LeaseMgrFactory::instance().getLease4(lease->addr_)); + + // Client had no lease in the database, so the old lease returned should + // be NULL. + EXPECT_FALSE(ctx.old_lease_); +} + +// This test checks the behavior of the allocation engine in the following +// scenario: +// - Client has no lease in the database. +// - Client has a global reservation. +// - Client sends REQUEST +// - Client is allocated a dynamic address from matched subnet +// - Lease is added to the lease database +TEST_F(AllocEngine4Test, globalReservationDynamicRequest) { + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, SUBNET_ID_GLOBAL, + SUBNET_ID_UNUSED, IOAddress::IPV4_ZERO_ADDRESS(), + "foo.example.org")); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false); + + subnet_->setHostReservationMode(Network::HR_GLOBAL); + + // Query allocation engine for the lease to be assigned to this + // client without specifying the address to be assigned. + AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, + IOAddress("0.0.0.0"), false, false, + "", false); + ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234)); + + // Look up the host. + AllocEngine::findReservation(ctx); + + // We should have the correct current host + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getHostname(), host->getHostname()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); + + // We should allocate a dynamic address. + Lease4Ptr lease = engine.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ("192.0.2.100", lease->addr_.toText()); + + // Check that the lease is indeed in LeaseMgr + Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_); + ASSERT_TRUE(from_mgr); + + // Now check that the lease in LeaseMgr has the same parameters + detailCompareLease(lease, from_mgr); + + // Client had no lease in the database, so the old lease returned should + // be NULL. + EXPECT_FALSE(ctx.old_lease_); +} + }; // namespace test }; // namespace dhcp }; // namespace isc -- 2.47.2