From: Marcin Siodelski Date: Mon, 11 Sep 2017 16:47:30 +0000 (+0200) Subject: [5306] Client contexts now hold multiple hosts. X-Git-Tag: trac5363_base~21^2~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6f994e6f6ecc35be4e68785643fc0b1e17105e86;p=thirdparty%2Fkea.git [5306] Client contexts now hold multiple hosts. --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index b3ddc5b2c2..6c75e2fa0f 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -384,9 +384,9 @@ Dhcpv4Exchange::setHostIdentifiers() { void Dhcpv4Exchange::setReservedClientClasses() { - if (context_->host_ && query_) { + if (context_->currentHost() && query_) { BOOST_FOREACH(const std::string& client_class, - context_->host_->getClientClasses4()) { + context_->currentHost()->getClientClasses4()) { query_->addClass(client_class); } } @@ -394,7 +394,7 @@ Dhcpv4Exchange::setReservedClientClasses() { void Dhcpv4Exchange::setReservedMessageFields() { - ConstHostPtr host = context_->host_; + ConstHostPtr host = context_->currentHost(); // Nothing to do if host reservations not specified for this client. if (host) { if (!host->getNextServer().isV4Zero()) { @@ -1167,7 +1167,7 @@ Dhcpv4Srv::buildCfgOptionList(Dhcpv4Exchange& ex) { } // Firstly, host specific options. - const ConstHostPtr& host = ex.getContext()->host_; + const ConstHostPtr& host = ex.getContext()->currentHost(); if (host && !host->getCfgOption4()->empty()) { co_list.push_back(host->getCfgOption4()); } @@ -1471,9 +1471,10 @@ Dhcpv4Srv::processClientFqdnOption(Dhcpv4Exchange& ex) { fqdn_resp->setFlag(Option4ClientFqdn::FLAG_E, fqdn->getFlag(Option4ClientFqdn::FLAG_E)); - if (ex.getContext()->host_ && !ex.getContext()->host_->getHostname().empty()) { + if (ex.getContext()->currentHost() && + !ex.getContext()->currentHost()->getHostname().empty()) { D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr(); - fqdn_resp->setDomainName(d2_mgr.qualifyName(ex.getContext()->host_->getHostname(), + fqdn_resp->setDomainName(d2_mgr.qualifyName(ex.getContext()->currentHost()->getHostname(), true), Option4ClientFqdn::FULL); } else { @@ -1519,7 +1520,7 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) { // Hostname reservations take precedence over any other configuration, // i.e. DDNS configuration. - if (ctx->host_ && !ctx->host_->getHostname().empty()) { + if (ctx->currentHost() && !ctx->currentHost()->getHostname().empty()) { // In order to send a reserved hostname value we expect that at least // one of the following is the case: the client has sent us a hostname // option, or the client has sent Parameter Request List option with @@ -1551,7 +1552,8 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) { // send back a hostname option, send this option with a reserved // name for this client. if (should_send_hostname) { - const std::string& hostname = d2_mgr.qualifyName(ctx->host_->getHostname(), + const std::string& hostname = + d2_mgr.qualifyName(ctx->currentHost()->getHostname(), false); LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_RESERVED_HOSTNAME_ASSIGNED) diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index d76696be8c..0b9e705343 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -861,8 +861,8 @@ Dhcpv6Srv::buildCfgOptionList(const Pkt6Ptr& question, AllocEngine::ClientContext6& ctx, CfgOptionList& co_list) { // Firstly, host specific options. - if (ctx.host_ && !ctx.host_->getCfgOption6()->empty()) { - co_list.push_back(ctx.host_->getCfgOption6()); + if (ctx.currentHost() && !ctx.currentHost()->getCfgOption6()->empty()) { + co_list.push_back(ctx.currentHost()->getCfgOption6()); } // Secondly, pool specific options. Pools are defined within a subnet, so @@ -1250,8 +1250,8 @@ Dhcpv6Srv::processClientFqdn(const Pkt6Ptr& question, const Pkt6Ptr& answer, } else { // No FQDN so get the lease hostname from the host reservation if // there is one. - if (ctx.host_) { - ctx.hostname_ = ctx.host_->getHostname(); + if (ctx.currentHost()) { + ctx.hostname_ = ctx.currentHost()->getHostname(); } return; @@ -1271,10 +1271,10 @@ Dhcpv6Srv::processClientFqdn(const Pkt6Ptr& question, const Pkt6Ptr& answer, d2_mgr.adjustFqdnFlags(*fqdn, *fqdn_resp); // If there's a reservation and it has a hostname specified, use it! - if (ctx.host_ && !ctx.host_->getHostname().empty()) { + if (ctx.currentHost() && !ctx.currentHost()->getHostname().empty()) { // Add the qualifying suffix. // After #3765, this will only occur if the suffix is not empty. - fqdn_resp->setDomainName(d2_mgr.qualifyName(ctx.host_->getHostname(), + fqdn_resp->setDomainName(d2_mgr.qualifyName(ctx.currentHost()->getHostname(), true), Option6ClientFqdn::FULL); } else { @@ -3082,9 +3082,9 @@ void Dhcpv6Srv::classifyPacket(const Pkt6Ptr& pkt) { void Dhcpv6Srv::setReservedClientClasses(const Pkt6Ptr& pkt, const AllocEngine::ClientContext6& ctx) { - if (ctx.host_ && pkt) { + if (ctx.currentHost() && pkt) { BOOST_FOREACH(const std::string& client_class, - ctx.host_->getClientClasses6()) { + ctx.currentHost()->getClientClasses6()) { pkt->addClass(client_class); } } diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 7b6f48fb8d..2e2eb5100d 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -309,21 +309,27 @@ template void AllocEngine::findReservationInternal(ContextType& ctx, const AllocEngine::HostGetFunc& host_get) { - ctx.host_.reset(); + ctx.hosts_.clear(); + + auto subnet = ctx.subnet_; // We can only search for the reservation if a subnet has been selected. - if (ctx.subnet_) { + while (subnet) { + // Iterate over configured identifiers in the order of preference // and try to use each of them to search for the reservations. BOOST_FOREACH(const IdentifierPair& id_pair, ctx.host_identifiers_) { // Attempt to find a host using a specified identifier. - ctx.host_ = host_get(ctx.subnet_->getID(), id_pair.first, - &id_pair.second[0], id_pair.second.size()); - // If we found matching host, return. - if (ctx.host_) { - return; + ConstHostPtr host = host_get(subnet->getID(), id_pair.first, + &id_pair.second[0], id_pair.second.size()); + // If we found matching host for this subnet. + if (host) { + ctx.hosts_[subnet->getID()] = host; + break; } } + + subnet = subnet->getNextSubnet(ctx.subnet_); } } @@ -334,7 +340,7 @@ AllocEngine::findReservationInternal(ContextType& ctx, AllocEngine::ClientContext6::ClientContext6() : query_(), fake_allocation_(false), subnet_(), duid_(), - hwaddr_(), host_identifiers_(), host_(), fwd_dns_update_(false), + hwaddr_(), host_identifiers_(), hosts_(), fwd_dns_update_(false), rev_dns_update_(false), hostname_(), callout_handle_(), ias_() { } @@ -348,7 +354,7 @@ AllocEngine::ClientContext6::ClientContext6(const Subnet6Ptr& subnet, const Pkt6Ptr& query, const CalloutHandlePtr& callout_handle) : query_(query), fake_allocation_(fake_allocation), subnet_(subnet), - duid_(duid), hwaddr_(), host_identifiers_(), host_(), + duid_(duid), hwaddr_(), host_identifiers_(), hosts_(), fwd_dns_update_(fwd_dns), rev_dns_update_(rev_dns), hostname_(hostname), callout_handle_(callout_handle), allocated_resources_(), ias_() { @@ -386,6 +392,16 @@ isAllocated(const asiolink::IOAddress& prefix, const uint8_t prefix_len) const { (allocated_resources_.count(std::make_pair(prefix, prefix_len)))); } +ConstHostPtr +AllocEngine::ClientContext6::currentHost() const { + if (subnet_) { + auto host = hosts_.find(subnet_->getID()); + if (host != hosts_.cend()) { + return (host->second); + } + } + return (ConstHostPtr()); +} void AllocEngine::findReservation(ClientContext6& ctx) { findReservationInternal(ctx, boost::bind(&HostMgr::get6, @@ -432,7 +448,7 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { // Hence independent checks. // Case 1: There are no leases and there's a reservation for this host. - if (leases.empty() && ctx.host_) { + if (leases.empty() && ctx.currentHost()) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V6_ALLOC_NO_LEASES_HR) @@ -456,7 +472,7 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { // There is at least one lease for this client and there are no reservations. // We will return these leases for the client, but we may need to update // FQDN information. - } else if (!leases.empty() && !ctx.host_) { + } else if (!leases.empty() && !ctx.currentHost()) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V6_ALLOC_LEASES_NO_HR) @@ -475,7 +491,7 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) { // assign something new. // Case 3: There are leases and there are reservations. - } else if (!leases.empty() && ctx.host_) { + } else if (!leases.empty() && ctx.currentHost()) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V6_ALLOC_LEASES_HR) @@ -754,7 +770,7 @@ AllocEngine::allocateReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases) { // If there are no reservations or the reservation is v4, there's nothing to do. - if (!ctx.host_ || !ctx.host_->hasIPv6Reservation()) { + if (!ctx.currentHost() || !ctx.currentHost()->hasIPv6Reservation()) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V6_ALLOC_NO_V6_HR) .arg(ctx.query_->getLabel()); @@ -770,8 +786,8 @@ AllocEngine::allocateReservedLeases6(ClientContext6& ctx, // we already have a lease for a reserved address or prefix. BOOST_FOREACH(const Lease6Ptr& lease, existing_leases) { if ((lease->valid_lft_ != 0)) { - if (ctx.host_->hasReservation(IPv6Resrv(type, lease->addr_, - lease->prefixlen_))) { + if (ctx.currentHost()->hasReservation(IPv6Resrv(type, lease->addr_, + lease->prefixlen_))) { // We found existing lease for a reserved address or prefix. // We'll simply extend the lifetime of the lease. LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, @@ -794,7 +810,7 @@ AllocEngine::allocateReservedLeases6(ClientContext6& ctx, // over reservations specified and try to allocate one of them for the IA. // Get the IPv6 reservations of specified type. - const IPv6ResrvRange& reservs = ctx.host_->getIPv6Reservations(type); + const IPv6ResrvRange& reservs = ctx.currentHost()->getIPv6Reservations(type); BOOST_FOREACH(IPv6ResrvTuple type_lease_tuple, reservs) { // We do have a reservation for address or prefix. const IOAddress& addr = type_lease_tuple.second.getPrefix(); @@ -860,16 +876,16 @@ AllocEngine::removeNonmatchingReservedLeases6(ClientContext6& ctx, BOOST_FOREACH(const Lease6Ptr& candidate, copy) { // If we have reservation we should check if the reservation is for // the candidate lease. If so, we simply accept the lease. - if (ctx.host_) { + if (ctx.currentHost()) { if (candidate->type_ == Lease6::TYPE_NA) { - if (ctx.host_->hasReservation(IPv6Resrv(IPv6Resrv::TYPE_NA, - candidate->addr_))) { + if (ctx.currentHost()->hasReservation(IPv6Resrv(IPv6Resrv::TYPE_NA, + candidate->addr_))) { continue; } } else { - if (ctx.host_->hasReservation(IPv6Resrv(IPv6Resrv::TYPE_PD, - candidate->addr_, - candidate->prefixlen_))) { + if (ctx.currentHost()->hasReservation(IPv6Resrv(IPv6Resrv::TYPE_PD, + candidate->addr_, + candidate->prefixlen_))) { continue; } } @@ -956,7 +972,8 @@ AllocEngine::removeNonreservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases) { // This method removes leases that are not reserved for this host. // It will keep at least one lease, though. - if (existing_leases.empty() || !ctx.host_ || !ctx.host_->hasIPv6Reservation()) { + if (existing_leases.empty() || !ctx.currentHost() || + !ctx.currentHost()->hasIPv6Reservation()) { return; } @@ -970,7 +987,7 @@ AllocEngine::removeNonreservedLeases6(ClientContext6& ctx, IPv6Resrv resv(ctx.currentIA().type_ == Lease::TYPE_NA ? IPv6Resrv::TYPE_NA : IPv6Resrv::TYPE_PD, (*lease)->addr_, (*lease)->prefixlen_); - if (!ctx.host_->hasReservation(resv)) { + if (!ctx.currentHost()->hasReservation(resv)) { // We have reservations, but not for this lease. Release it. // Remove this lease from LeaseMgr @@ -1234,7 +1251,7 @@ AllocEngine::renewLeases6(ClientContext6& ctx) { removeNonmatchingReservedLeases6(ctx, leases); } - if (ctx.host_) { + if (ctx.currentHost()) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V6_RENEW_HR) @@ -2154,13 +2171,31 @@ addressReserved(const IOAddress& address, const AllocEngine::ClientContext4& ctx /// dynamic pool. The allocation engine uses this function to check if /// the reservation is made for the IPv4 address. /// -/// @param ctx Client context holding the data extracted from the +/// @param [out] ctx Client context holding the data extracted from the /// client's message. /// /// @return true if the context contains the reservation for the IPv4 address. bool -hasAddressReservation(const AllocEngine::ClientContext4& ctx) { - return (ctx.host_ && !ctx.host_->getIPv4Reservation().isV4Zero()); +hasAddressReservation(AllocEngine::ClientContext4& ctx) { + if (ctx.hosts_.empty()) { + return (false); + } + + Subnet4Ptr subnet = ctx.subnet_; + while (subnet) { + auto host = ctx.hosts_.find(subnet->getID()); + if ((host != ctx.hosts_.end()) && + !(host->second->getIPv4Reservation().isV4Zero())) { + ctx.subnet_ = subnet; + return (true); + } + + // No address reservation found here, so let's try another subnet + // within the same shared network. + subnet = subnet->getNextSubnet(ctx.subnet_); + } + + return (false); } /// @brief Finds existing lease in the database. @@ -2285,7 +2320,7 @@ AllocEngine::ClientContext4::ClientContext4() requested_address_(IOAddress::IPV4_ZERO_ADDRESS()), fwd_dns_update_(false), rev_dns_update_(false), hostname_(""), callout_handle_(), fake_allocation_(false), - old_lease_(), host_(), conflicting_lease_(), query_(), + old_lease_(), hosts_(), conflicting_lease_(), query_(), host_identifiers_() { } @@ -2301,7 +2336,7 @@ AllocEngine::ClientContext4::ClientContext4(const Subnet4Ptr& subnet, requested_address_(requested_addr), fwd_dns_update_(fwd_dns_update), rev_dns_update_(rev_dns_update), hostname_(hostname), callout_handle_(), - fake_allocation_(fake_allocation), old_lease_(), host_(), + fake_allocation_(fake_allocation), old_lease_(), hosts_(), host_identifiers_() { // Initialize host identifiers. @@ -2310,6 +2345,17 @@ AllocEngine::ClientContext4::ClientContext4(const Subnet4Ptr& subnet, } } +ConstHostPtr +AllocEngine::ClientContext4::currentHost() const { + if (subnet_) { + auto host = hosts_.find(subnet_->getID()); + if (host != hosts_.cend()) { + return (host->second); + } + } + return (ConstHostPtr()); +} + Lease4Ptr AllocEngine::allocateLease4(ClientContext4& ctx) { // The NULL pointer indicates that the old lease didn't exist. It may @@ -2366,24 +2412,24 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V4_DISCOVER_HR) .arg(ctx.query_->getLabel()) - .arg(ctx.host_->getIPv4Reservation().toText()); + .arg(ctx.currentHost()->getIPv4Reservation().toText()); // If the client doesn't have a lease or the leased address is different // than the reserved one then let's try to allocate the reserved address. // Otherwise the address that the client has is the one for which it // has a reservation, so just renew it. - if (!client_lease || (client_lease->addr_ != ctx.host_->getIPv4Reservation())) { + if (!client_lease || (client_lease->addr_ != ctx.currentHost()->getIPv4Reservation())) { // The call below will return a pointer to the lease for the address // reserved to this client, if the lease is available, i.e. is not // currently assigned to any other client. // Note that we don't remove the existing client's lease at this point // because this is not a real allocation, we just offer what we can // allocate in the DHCPREQUEST time. - new_lease = allocateOrReuseLease4(ctx.host_->getIPv4Reservation(), ctx); + new_lease = allocateOrReuseLease4(ctx.currentHost()->getIPv4Reservation(), ctx); if (!new_lease) { LOG_WARN(alloc_engine_logger, ALLOC_ENGINE_V4_DISCOVER_ADDRESS_CONFLICT) .arg(ctx.query_->getLabel()) - .arg(ctx.host_->getIPv4Reservation().toText()) + .arg(ctx.currentHost()->getIPv4Reservation().toText()) .arg(ctx.conflicting_lease_ ? ctx.conflicting_lease_->toText() : "(no lease info)"); } @@ -2489,7 +2535,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { // allocation engine needs to find an appropriate address. // If there is a reservation for the client, let's try to // allocate the reserved address. - ctx.requested_address_ = ctx.host_->getIPv4Reservation(); + ctx.requested_address_ = ctx.currentHost()->getIPv4Reservation(); LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V4_REQUEST_USE_HR) @@ -2521,8 +2567,9 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { // address because the reserved address is in use. We will have to // check if the address is in use. if (hasAddressReservation(ctx) && - (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) { - existing = LeaseMgrFactory::instance().getLease4(ctx.host_->getIPv4Reservation()); + (ctx.currentHost()->getIPv4Reservation() != ctx.requested_address_)) { + existing = + LeaseMgrFactory::instance().getLease4(ctx.currentHost()->getIPv4Reservation()); // If the reserved address is not in use, i.e. the lease doesn't // exist or is expired, and the client is requesting a different // address, return NULL. The client should go back to the @@ -2532,7 +2579,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, ALLOC_ENGINE_V4_REQUEST_INVALID) .arg(ctx.query_->getLabel()) - .arg(ctx.host_->getIPv4Reservation().toText()) + .arg(ctx.currentHost()->getIPv4Reservation().toText()) .arg(ctx.requested_address_.toText()); return (Lease4Ptr()); @@ -2543,7 +2590,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { // address is reserved for the client. If the address is not reserved one // and it doesn't belong to the dynamic pool, do not allocate it. if ((!hasAddressReservation(ctx) || - (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) && + (ctx.currentHost()->getIPv4Reservation() != ctx.requested_address_)) && !inAllowedPool(ctx, ctx.requested_address_)) { LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE, diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index e8e1936c16..b0c7699e3b 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -315,10 +315,12 @@ public: /// received by the server. IdentifierList host_identifiers_; - /// @brief A pointer to the object identifying host reservations. + /// @brief Holds a map of hosts belonging to the client within different + /// subnets. /// - /// May be NULL if there are no reservations. - ConstHostPtr host_; + /// Multiple hosts may appear when the client belongs to a shared + /// network. + std::map hosts_; /// @brief A boolean value which indicates that server takes /// responsibility for the forward DNS Update for this lease @@ -441,6 +443,11 @@ public: ias_.push_back(IAContext()); }; + /// @brief Returns host for currently selected subnet. + /// + /// @return Pointer to the host object. + ConstHostPtr currentHost() const; + /// @brief Default constructor. ClientContext6(); @@ -1072,8 +1079,12 @@ public: /// @brief A pointer to an old lease that the client had before update. Lease4Ptr old_lease_; - /// @brief A pointer to the object identifying host reservations. - ConstHostPtr host_; + /// @brief Holds a map of hosts belonging to the client within different + /// subnets. + /// + /// Multiple hosts may appear when the client belongs to a shared + /// network. + std::map hosts_; /// @brief A pointer to the object representing a lease in conflict. /// @@ -1102,6 +1113,11 @@ public: host_identifiers_.push_back(IdentifierPair(id_type, identifier)); } + /// @brief Returns host for currently selected subnet. + /// + /// @return Pointer to the host object. + ConstHostPtr currentHost() const; + /// @brief Default constructor. ClientContext4(); diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index b586fd3fc7..6f9facd063 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -601,6 +601,60 @@ TEST_F(AllocEngine4Test, discoverSharedNetworkClassification) { EXPECT_TRUE(subnet1->inPool(Lease::TYPE_V4, lease->addr_)); } +// Test that reservations within shared network take precedence over the +// existing leases regardless in which subnet belonging to a shared network +// reservations belong. +TEST_F(AllocEngine4Test, discoverSharedNetworkReservations) { + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false); + + // Create two subnets, each with a single address pool. The first subnet + // has only one address in its address pool to make it easier to simulate + // address exhaustion. + Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.2.0"), 24, 1, 2, 3, SubnetID(1))); + Subnet4Ptr subnet2(new Subnet4(IOAddress("10.1.2.0"), 24, 1, 2, 3, SubnetID(2))); + Pool4Ptr pool1(new Pool4(IOAddress("192.0.2.17"), IOAddress("192.0.2.17"))); + Pool4Ptr pool2(new Pool4(IOAddress("10.1.2.5"), IOAddress("10.1.2.100"))); + subnet1->addPool(pool1); + subnet2->addPool(pool2); + + // Both subnets belong to the same network so they can be used + // interchangeably. + SharedNetwork4Ptr network(new SharedNetwork4("test_network")); + network->add(subnet1); + network->add(subnet2); + + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, subnet2->getID(), + SubnetID(0), 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); + AllocEngine::findReservation(ctx); + ctx.query_.reset(new Pkt4(DHCPDISCOVER, 1234)); + 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 allocate an address from a // different subnet than orginally selected, when the address pool in // the first subnet is exhausted. @@ -745,6 +799,63 @@ TEST_F(AllocEngine4Test, requestSharedNetworkClassification) { EXPECT_TRUE(subnet2->inPool(Lease::TYPE_V4, lease->addr_)); } +// 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). +TEST_F(AllocEngine4Test, requestSharedNetworkReservations) { + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false); + + // Create two subnets, each with a single address pool. The first subnet + // has only one address in its address pool to make it easier to simulate + // address exhaustion. + Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.2.0"), 24, 1, 2, 3, SubnetID(1))); + Subnet4Ptr subnet2(new Subnet4(IOAddress("10.1.2.0"), 24, 1, 2, 3, SubnetID(2))); + Pool4Ptr pool1(new Pool4(IOAddress("192.0.2.17"), IOAddress("192.0.2.17"))); + Pool4Ptr pool2(new Pool4(IOAddress("10.1.2.5"), IOAddress("10.1.2.100"))); + subnet1->addPool(pool1); + subnet2->addPool(pool2); + + // Both subnets belong to the same network so they can be used + // interchangeably. + SharedNetwork4Ptr network(new SharedNetwork4("test_network")); + network->add(subnet1); + network->add(subnet2); + + // Create reservation for the client. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, subnet2->getID(), + SubnetID(0), 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); + AllocEngine::findReservation(ctx); + ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234)); + 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) { @@ -1964,7 +2075,7 @@ TEST_F(AllocEngine4Test, findReservation) { // There is no reservation in the database so no host should be returned. ASSERT_NO_THROW(engine.findReservation(ctx)); - EXPECT_FALSE(ctx.host_); + EXPECT_FALSE(ctx.currentHost()); // Create a reservation for the client. HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), @@ -1975,21 +2086,21 @@ TEST_F(AllocEngine4Test, findReservation) { // This time the reservation should be returned. ASSERT_NO_THROW(engine.findReservation(ctx)); - EXPECT_TRUE(ctx.host_); - EXPECT_EQ(ctx.host_->getIPv4Reservation(), host->getIPv4Reservation()); + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); // Regardless of the host reservation mode, the host should be // always returned when findReservation() is called. subnet_->setHostReservationMode(Network::HR_DISABLED); ASSERT_NO_THROW(engine.findReservation(ctx)); - EXPECT_TRUE(ctx.host_); - EXPECT_EQ(ctx.host_->getIPv4Reservation(), host->getIPv4Reservation()); + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); // Check the third possible reservation mode. subnet_->setHostReservationMode(Network::HR_OUT_OF_POOL); ASSERT_NO_THROW(engine.findReservation(ctx)); - EXPECT_TRUE(ctx.host_); - EXPECT_EQ(ctx.host_->getIPv4Reservation(), host->getIPv4Reservation()); + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); // This time use the client identifier to search for the host. host.reset(new Host(&clientid_->getClientId()[0], @@ -2000,14 +2111,14 @@ TEST_F(AllocEngine4Test, findReservation) { CfgMgr::instance().commit(); ASSERT_NO_THROW(engine.findReservation(ctx)); - EXPECT_TRUE(ctx.host_); - EXPECT_EQ(ctx.host_->getIPv4Reservation(), host->getIPv4Reservation()); + EXPECT_TRUE(ctx.currentHost()); + EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); // Remove the subnet. Subnet id is required to find host reservations, so // if it is set to NULL, no reservation should be returned ctx.subnet_.reset(); ASSERT_NO_THROW(engine.findReservation(ctx)); - EXPECT_FALSE(ctx.host_); + EXPECT_FALSE(ctx.currentHost()); // The same if there is a mismatch of the subnet id between the reservation // and the context. @@ -2019,7 +2130,7 @@ TEST_F(AllocEngine4Test, findReservation) { CfgMgr::instance().commit(); ASSERT_NO_THROW(engine.findReservation(ctx)); - EXPECT_FALSE(ctx.host_); + EXPECT_FALSE(ctx.currentHost()); } // This test checks if the simple IPv4 allocation can succeed and that diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index dc495a2e3e..81eab8f28b 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -178,8 +178,8 @@ AllocEngine6Test::findReservation(AllocEngine& engine, AllocEngine::ClientContext6& ctx) { engine.findReservation(ctx); // Let's check whether there's a hostname specified in the reservation - if (ctx.host_) { - std::string hostname = ctx.host_->getHostname(); + if (ctx.currentHost()) { + std::string hostname = ctx.currentHost()->getHostname(); // If there is, let's use it if (!hostname.empty()) { ctx.hostname_ = hostname;