Browse Source

[5306] Client contexts now hold multiple hosts.

Marcin Siodelski 7 years ago
parent
commit
6f994e6f6e

+ 10 - 8
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)

+ 8 - 8
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<Option6ClientFqdn>(*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);
         }
     }

+ 86 - 39
src/lib/dhcpsrv/alloc_engine.cc

@@ -309,21 +309,27 @@ template<typename ContextType>
 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,

+ 21 - 5
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<SubnetID, ConstHostPtr> 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<SubnetID, ConstHostPtr> 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();
 

+ 122 - 11
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

+ 2 - 2
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;