From: Marcin Siodelski Date: Thu, 11 Dec 2014 15:59:22 +0000 (+0100) Subject: [3564] Resolve conflicts with the reserved addresses in allocation engine. X-Git-Tag: trac3504_base~25^2~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e29eee4fa7a0bcff002aa1d2d47c8cdd6eafd414;p=thirdparty%2Fkea.git [3564] Resolve conflicts with the reserved addresses in allocation engine. --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index a40b45d4e9..c3f97e92b9 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -524,50 +524,90 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid ctx.old_lease_ = old_lease; ctx.host_ = HostMgr::instance().get4(subnet->getID(), hwaddr); + // If there is a reservation for this client we want to allocate the + // reserved address to the client, rather than any other address. if (ctx.host_) { + // In some cases the client doesn't supply any address, e.g. when + // it sends a DHCPDISCOVER. In such cases, we use the reserved + // address as a hint. if (ctx.hint_ == IOAddress("0.0.0.0")) { ctx.hint_ = ctx.host_->getIPv4Reservation(); + // If the client supplied an address we have to check if this + // address is reserved for this client. If not, we will send + // DHCPNAK to inform the client that we were not able to + // assign a requested address. The fake allocation (DHCPDISCOVER + // case) is an exception. In such case we treat the address + // supplied by the client as a hint, but we may offer address + // other than desired by the client. So, we don't return an + // empty lease. } else if (!ctx.fake_allocation_ && (ctx.hint_ != ctx.host_->getIPv4Reservation())) { return (Lease4Ptr()); } } + // Check if the client has any leases in the lease database, using HW + // address or client identifier. LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); Lease4Ptr existing = lease_mgr.getLease4(*hwaddr, ctx.subnet_->getID()); if (!existing && clientid) { existing = lease_mgr.getLease4(*clientid, ctx.subnet_->getID()); } + // If client has a lease there are two choices. The server may need + // to renew (no address change) the lease. Or, the server may need + // to replace the lease with a different one. This is the case when + // the server has a dynamically assigned lease but an administrator + // has made a new reservation for the client for a different address. if (existing) { existing = reallocateClientLease(existing, ctx); - if (ctx.host_ || existing) { + // The interrupt_processing_ flag indicates that the lease + // reallocation was not successful and that the allocation + // engine should cease allocation process for this client. + // This may occur when the client is trying to renew the + // lease which is reserved for someone else. The server should + // send DHCPNAK to indicate that the client should try to + // start over the allocation process. + if (ctx.interrupt_processing_) { + old_lease = ctx.old_lease_; + return (Lease4Ptr()); + + // If we tried to reallocate the reserved lease we return + // at this point regardless if reallocation failed or passed. + // We also return when allocation passed, no matter if this + // was a reserved address or not. + } else if (ctx.host_ || existing) { old_lease = ctx.old_lease_; return (existing); } } - // Check if we have a specific address we would like to allocate for the - // client. It may either be an address which the client is currently - // using, or it may be a administratively reserved address. - if (ctx.host_ || subnet->inPool(Lease::TYPE_V4, hint)) { + // There is no lease in the database for this client, so we will + // proceed with a new allocation. We will try to allocate a + // reserved address or an address from a dynamic pool if there is + // no reservation. + if (ctx.host_ || subnet->inPool(Lease::TYPE_V4, ctx.hint_)) { // If a client is requesting specific IP address, but the // reservation was made for a different address the server returns // NAK to the client. By returning NULL lease here we indicate to // the server that we're not able to fulfil client's request for the - // particular IP address. + // particular IP address. We don't return NULL lease in case of the + // fake allocation (DHCPDISCOVER) because in this case the address + // supplied by the client is only a hint. if (!ctx.fake_allocation_ && ctx.host_ && - (hint != IOAddress("0.0.0.0")) && - (ctx.host_->getIPv4Reservation() != hint)) { + (ctx.hint_ != IOAddress("0.0.0.0")) && + (ctx.host_->getIPv4Reservation() != ctx.hint_)) { return (Lease4Ptr()); } - // The reserved address always takes precedence over hints. But, if - // there is no reservation, try to respect the client's hint. + // The reserved address always takes precedence over an address + // supplied by the client (renewed address or requested). const IOAddress& candidate = ctx.host_ ? - ctx.host_->getIPv4Reservation() : hint; + ctx.host_->getIPv4Reservation() : ctx.hint_; + // Once we picked an address we want to allocate, we have to check + // if this address is available. existing = LeaseMgrFactory::instance().getLease4(candidate); if (!existing) { // The candidate address is currently unused. Let's create a @@ -588,14 +628,21 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid return (lease); } + // There is a lease for this address in the lease database but + // it is possible that the lease has expired, in which case + // we will be able to reuse it. } else { if (existing->expired()) { // Save the old lease, before reusing it. old_lease.reset(new Lease4(*existing)); return (reuseExpiredLease(existing, ctx)); - } else if (ctx.host_ && (ctx.host_->getIPv4Reservation() != - IOAddress("0.0.0.0"))) { + // The existing lease is not expired (is in use by some + // other client). If we are trying to get this lease because + // the address has been reserved for the client we have no + // choice but to return a NULL lease to indicate that the + // allocation has failed. + } else if (ctx.host_) { return (Lease4Ptr()); } @@ -621,10 +668,15 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid unsigned int i = attempts_; do { - IOAddress candidate = allocator->pickAddress(subnet, clientid, hint); - - /// @todo: check if the address is reserved once we have host support - /// implemented + IOAddress candidate = allocator->pickAddress(subnet, clientid, ctx.hint_); + + // Check if this address is reserved. There is no need to check for + // whom it is reserved, because if it has been reserved for us we would + // have already allocated a lease. + if (HostMgr::instance().get4(subnet->getID(), candidate)) { + // Don't allocate a reserved address. + continue; + } Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(candidate); if (!existing) { @@ -667,11 +719,25 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid Lease4Ptr AllocEngine::renewLease4(const Lease4Ptr& lease, - const AllocEngine::Context4& ctx) { + AllocEngine::Context4& ctx) { if (!lease) { isc_throw(BadValue, "null lease specified for renewLease4"); } + // The ctx.host_ possibly contains a reservation for the client for which + // we are renewing a lease. If this reservation exists, we assume that + // there is no conflict in assigning the address to this client. Note + // that the reallocateClientLease checks if the address reserved for + // the client matches the address in the lease we're renewing here. + if (!ctx.host_) { + ConstHostPtr host = HostMgr::instance().get4(ctx.subnet_->getID(), + lease->addr_); + if (host && ctx.hwaddr_ && (*host->getHWAddress() != *ctx.hwaddr_)) { + ctx.interrupt_processing_ = !ctx.fake_allocation_; + return (Lease4Ptr()); + } + } + // Let's keep the old data. This is essential if we are using memfile // (the lease returned points directly to the lease4 object in the database) // We'll need it if we want to skip update (i.e. roll back renewal) @@ -815,7 +881,7 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired, Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired, - const AllocEngine::Context4& ctx) { + AllocEngine::Context4& ctx) { if (!expired) { isc_throw(BadValue, "null lease specified for reuseExpiredLease"); } @@ -884,7 +950,8 @@ AllocEngine::reuseExpiredLease(Lease4Ptr& expired, } Lease4Ptr -AllocEngine::replaceClientLease(Lease4Ptr& lease, const Context4& ctx) { +AllocEngine::replaceClientLease(Lease4Ptr& lease, Context4& ctx) { + if (!lease) { isc_throw(BadValue, "null lease specified for replaceClientLease"); } @@ -893,18 +960,28 @@ AllocEngine::replaceClientLease(Lease4Ptr& lease, const Context4& ctx) { isc_throw(BadValue, "null subnet specified for replaceClientLease"); } - if (!ctx.host_) { - isc_throw(BadValue, "null host specified for replaceClientLease"); - } - if (ctx.hint_ == IOAddress("0.0.0.0")) { isc_throw(BadValue, "zero address specified for the" " replaceClientLease"); } IOAddress prev_address = lease->addr_; + if (!ctx.host_) { + ConstHostPtr host = HostMgr::instance().get4(ctx.subnet_->getID(), + ctx.hint_); + if (host && ctx.hwaddr_ && (*host->getHWAddress() != *ctx.hwaddr_)) { + ctx.interrupt_processing_ = true; + return (Lease4Ptr()); + } + lease->addr_ = ctx.hint_; + + } else { + lease->addr_ = ctx.host_->getIPv4Reservation(); + + } + + updateLease4Information(lease, ctx); - lease->addr_ = ctx.host_->getIPv4Reservation(); // Execute callouts registered for lease4_select. if (ctx.callout_handle_ && HooksManager::getHooksManager() @@ -949,7 +1026,8 @@ AllocEngine::reallocateClientLease(Lease4Ptr& lease, // Save the old lease, before renewal. ctx.old_lease_.reset(new Lease4(*lease)); - if (ctx.host_ && ctx.host_->getIPv4Reservation() != lease->addr_) { + if ((ctx.host_ && (ctx.host_->getIPv4Reservation() != lease->addr_)) || + (lease->addr_ != ctx.hint_)) { lease = replaceClientLease(lease, ctx); return (lease); @@ -1146,7 +1224,7 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet, void AllocEngine::updateLease4Information(const Lease4Ptr& lease, - const AllocEngine::Context4& ctx) const { + AllocEngine::Context4& ctx) const { // This should not happen in theory. if (!lease) { isc_throw(BadValue, "null lease specified for updateLease4Information"); diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index 220219ce1a..f617911c67 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -273,12 +273,20 @@ protected: /// @brief A pointer to the object identifying host reservations. ConstHostPtr host_; + /// @brief Signals that the allocation should be interrupted. + /// + /// This flag is set by the downstream methods called by the + /// @c AllocEngine::allocateLease4. This flag is set to true to + /// indicate that an attempt to allocate a lease should be + /// interrupted. + bool interrupt_processing_; + /// @brief Default constructor. Context4() : subnet_(), clientid_(), hwaddr_(), hint_("0.0.0.0"), fwd_dns_update_(false), rev_dns_update_(false), hostname_(""), callout_handle_(), fake_allocation_(false), - old_lease_(), host_() { + old_lease_(), host_(), interrupt_processing_(false) { } }; @@ -363,7 +371,7 @@ protected: /// @return Returns renewed lease. Note that the lease is only updated when /// it is an actual allocation (not processing DHCPDISCOVER message). Lease4Ptr - renewLease4(const Lease4Ptr& lease, const Context4& ctx); + renewLease4(const Lease4Ptr& lease, Context4& ctx); /// @brief Allocates an IPv6 lease /// @@ -462,7 +470,7 @@ private: /// @param ctx A context containing information from the server about the /// client and its message. void updateLease4Information(const Lease4Ptr& lease, - const Context4& ctx) const; + Context4& ctx) const; /// @brief creates a lease and inserts it in LeaseMgr if necessary /// @@ -515,7 +523,7 @@ private: /// @return Updated lease instance. /// @throw BadValue if trying to reuse a lease which is still valid or /// when the provided parameters are invalid. - Lease4Ptr reuseExpiredLease(Lease4Ptr& expired, const Context4& ctx); + Lease4Ptr reuseExpiredLease(Lease4Ptr& expired, Context4& ctx); /// @brief Updates the existing, non expired lease with a information from /// the context. @@ -533,7 +541,7 @@ private: /// /// @return Pointer to the updated lease. /// @throw BadValue if the provided parameters are invalid. - Lease4Ptr replaceClientLease(Lease4Ptr& lease, const Context4& ctx); + Lease4Ptr replaceClientLease(Lease4Ptr& lease, Context4& ctx); /// @brief Replace or renew client's lease. /// diff --git a/src/lib/dhcpsrv/base_host_data_source.h b/src/lib/dhcpsrv/base_host_data_source.h index 4cbd588afc..162520e2e6 100644 --- a/src/lib/dhcpsrv/base_host_data_source.h +++ b/src/lib/dhcpsrv/base_host_data_source.h @@ -31,6 +31,14 @@ public: isc::Exception(file, line, what) { }; }; +/// @brief Exception thrown when invalid IP address has been specified for +/// @c Host. +class BadHostAddress : public isc::BadValue { +public: + BadHostAddress(const char* file, size_t line, const char* what) : + isc::BadValue(file, line, what) { }; +}; + /// @brief Base interface for the classes implementing simple data source /// for host reservations. /// @@ -105,6 +113,26 @@ public: get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr, const DuidPtr& duid = DuidPtr()) const = 0; + /// @brief Returns a host connected to the IPv4 subnet and having + /// a reservation for a specified IPv4 address. + /// + /// One of the use cases for this method is to detect collisions between + /// dynamically allocated addresses and reserved addresses. When the new + /// address is assigned to a client, the allocation mechanism should check + /// if this address is not reserved for some other host and do not allocate + /// this address if reservation is present. + /// + /// Implementations of this method should guard against invalid addresses, + /// such as IPv6 address. + /// + /// @param subnet_id Subnet identifier. + /// @param address reserved IPv4 address. + /// + /// @return Const @c Host object using a specified IPv4 address. + virtual ConstHostPtr + get4(const SubnetID& subnet_id, + const asiolink::IOAddress& address) const = 0; + /// @brief Returns a host connected to the IPv6 subnet. /// /// Implementations of this method should guard against the case when diff --git a/src/lib/dhcpsrv/cfg_hosts.cc b/src/lib/dhcpsrv/cfg_hosts.cc index 8bb17f3cc3..4c2b8b3d85 100644 --- a/src/lib/dhcpsrv/cfg_hosts.cc +++ b/src/lib/dhcpsrv/cfg_hosts.cc @@ -36,13 +36,17 @@ CfgHosts::getAll(const HWAddrPtr& hwaddr, const DuidPtr& duid) { } ConstHostCollection -CfgHosts::getAll4(const IOAddress&) const { - isc_throw(isc::NotImplemented, "getAll4(address) const is not implemented"); +CfgHosts::getAll4(const IOAddress& address) const { + ConstHostCollection collection; + getAllInternal4(address, collection); + return (collection); } HostCollection -CfgHosts::getAll4(const IOAddress&) { - isc_throw(isc::NotImplemented, "getAll4(address) is not implemented"); +CfgHosts::getAll4(const IOAddress& address) { + HostCollection collection; + getAllInternal4(address, collection); + return (collection); } template @@ -76,6 +80,25 @@ CfgHosts::getAllInternal(const HWAddrPtr& hwaddr, const DuidPtr& duid, } } +template +void +CfgHosts::getAllInternal4(const IOAddress& address, Storage& storage) const { + // Must not specify address other than IPv4. + if (!address.isV4()) { + isc_throw(BadHostAddress, "must specify an IPv4 address when searching" + " for a host, specified address was " << address); + } + // Search for the Host using the reserved IPv4 address as a key. + const HostContainerIndex1& idx = hosts_.get<1>(); + HostContainerIndex1Range r = idx.equal_range(address); + // Append each Host object to the storage. + for (HostContainerIndex1::iterator host = r.first; host != r.second; + ++host) { + storage.push_back(*host); + } +} + + ConstHostPtr CfgHosts::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr, const DuidPtr& duid) const { @@ -90,6 +113,19 @@ CfgHosts::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr, return (getHostInternal(subnet_id, false, hwaddr, duid)); } +ConstHostPtr +CfgHosts::get4(const SubnetID& subnet_id, const IOAddress& address) const { + ConstHostCollection hosts = getAll4(address); + for (ConstHostCollection::const_iterator host = hosts.begin(); + host != hosts.end(); ++host) { + if ((*host)->getIPv4SubnetID() == subnet_id) { + return (*host); + } + } + return (ConstHostPtr()); +} + + ConstHostPtr CfgHosts::get6(const SubnetID& subnet_id, const DuidPtr& duid, const HWAddrPtr& hwaddr) const { diff --git a/src/lib/dhcpsrv/cfg_hosts.h b/src/lib/dhcpsrv/cfg_hosts.h index a94790934c..389a3de7c7 100644 --- a/src/lib/dhcpsrv/cfg_hosts.h +++ b/src/lib/dhcpsrv/cfg_hosts.h @@ -80,7 +80,7 @@ public: /// /// @param address IPv4 address for which the @c Host object is searched. /// - /// @throw isc::NotImplemented. + /// @return Collection of const @c Host objects. virtual ConstHostCollection getAll4(const asiolink::IOAddress& address) const; @@ -91,7 +91,7 @@ public: /// /// @param address IPv4 address for which the @c Host object is searched. /// - /// @throw isc::NotImplemented + /// @return Collection of const @c Host objects. virtual HostCollection getAll4(const asiolink::IOAddress& address); @@ -125,6 +125,16 @@ public: get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr, const DuidPtr& duid = DuidPtr()); + /// @brief Returns a host connected to the IPv4 subnet and having + /// a reservation for a specified IPv4 address. + /// + /// @param subnet_id Subnet identifier. + /// @param address reserved IPv4 address. + /// + /// @return Const @c Host object using a specified IPv4 address. + virtual ConstHostPtr + get4(const SubnetID& subnet_id, const asiolink::IOAddress& address) const; + /// @brief Returns a host connected to the IPv6 subnet and matching /// the specified identifiers. /// @@ -216,6 +226,21 @@ private: void getAllInternal(const HWAddrPtr& hwaddr, const DuidPtr& duid, Storage& storage) const; + /// @brief Returns @c Host objects for the specified IPv4 address. + /// + /// This private method is called by the @c CfgHosts::getAll4 methods + /// to retrieve the @c Host for which the specified IPv4 address is + /// reserved. The retrieved objects are appended to the @c storage + /// container. + /// + /// @param address IPv4 address. + /// @param [out] storage Container to which the retrieved objects are + /// appended. + /// @tparam One of the @c ConstHostCollection or @c HostCollection. + template + void getAllInternal4(const asiolink::IOAddress& address, + Storage& storage) const; + /// @brief Returns @c Host object connected to a subnet. /// /// This private method returns a pointer to the @c Host object identified diff --git a/src/lib/dhcpsrv/host_container.h b/src/lib/dhcpsrv/host_container.h index 6f2401cf04..7835240a11 100644 --- a/src/lib/dhcpsrv/host_container.h +++ b/src/lib/dhcpsrv/host_container.h @@ -64,6 +64,14 @@ typedef boost::multi_index_container< &Host::getIdentifierType > > + >, + + // Second index is used to search for the host using reserved IPv4 + // address. + boost::multi_index::ordered_non_unique< + // Index using values returned by the @c Host::getIPv4Resrvation. + boost::multi_index::const_mem_fun > > > HostContainer; @@ -78,6 +86,16 @@ typedef HostContainer::nth_index<0>::type HostContainerIndex0; typedef std::pair HostContainerIndex0Range; +/// @brief Second index type in the @c HostContainer. +/// +/// This index allows for searching for @c Host objects using a +/// reserved IPv4 address. +typedef HostContainer::nth_index<1>::type HostContainerIndex1; + +/// @brief Results range returned using the @c HostContainerIndex1. +typedef std::pair HostContainerIndex1Range; + } } diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc index 8296cadd85..6bb0488812 100644 --- a/src/lib/dhcpsrv/host_mgr.cc +++ b/src/lib/dhcpsrv/host_mgr.cc @@ -88,6 +88,17 @@ HostMgr::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr, return (host); } +ConstHostPtr +HostMgr::get4(const SubnetID& subnet_id, + const asiolink::IOAddress& address) const { + ConstHostPtr host = getCfgHosts()->get4(subnet_id, address); + if (!host && alternate_source) { + host = alternate_source->get4(subnet_id, address); + } + return (host); +} + + ConstHostPtr HostMgr::get6(const SubnetID& subnet_id, const DuidPtr& duid, const HWAddrPtr& hwaddr) const { diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h index 09847f4979..ee04ecb1e6 100644 --- a/src/lib/dhcpsrv/host_mgr.h +++ b/src/lib/dhcpsrv/host_mgr.h @@ -139,6 +139,20 @@ public: get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr, const DuidPtr& duid = DuidPtr()) const; + /// @brief Returns a host connected to the IPv4 subnet and having + /// a reservation for a specified IPv4 address. + /// + /// This method returns a single reservation for the particular host + /// (identified by the HW address or DUID) as documented in the + /// @c BaseHostDataSource::get4. + /// + /// @param subnet_id Subnet identifier. + /// @param address reserved IPv4 address. + /// + /// @return Const @c Host object using a specified IPv4 address. + virtual ConstHostPtr + get4(const SubnetID& subnet_id, const asiolink::IOAddress& address) const; + /// @brief Returns a host connected to the IPv6 subnet. /// /// This method returns a host connected to the IPv6 subnet and identified diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc index ae5ecdd0ba..8c095e2407 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc @@ -2131,6 +2131,133 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseNoHintFakeAllocation) { detailCompareLease(lease, from_mgr); } +// This test checks that the behavior of the allocation engine in the following +// scenario: +// - Client A has a lease for the address. +// - Client B has a reservation for the same address that the Client A is using. +// - Client B requests allocation of the reserved address. +// - Server returns DHCPNAK to the client to indicate that the requested address +// can't be allocated. +// - Client A renews the lease. +// - Server determines that the lease that the Client A is trying to renews +// is for the address reserved for Client B. Therefore, the server returns +// DHCPNAK to force the client to return to the server discovery. +// - The Client A sends DHCPDISCOVER. +// - The server offers an address to the Client A, which is different than +// the address reserved for Client B. +// - The Client A requests allocation of the offered address. +// - The server allocates the new address to Client A. +// - The Client B sends DHCPDISCOVER to the server. +// - The server offers a reserved address to the Client B. +// - The Client B requests the offered address. +// - The server allocates the reserved address to the Client B. +TEST_F(AllocEngine4Test, reservedAddressConflictResolution) { + // Create a reservation for client B. + HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, subnet_->getID(), + SubnetID(0), IOAddress("192.0.2.101"))); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + // Create a lease for Client A. + Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0, + 100, 30, 60, time(NULL), subnet_->getID(), + false, false, "")); + LeaseMgrFactory::instance().addLease(lease); + + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false); + + + // Client B sends a DHCPREQUEST to allocate a reserved lease. The + // allocation engine declines allocation of the address for the + // client because Client A has a lease for it. + ASSERT_FALSE(engine.allocateLease4(subnet_, ClientIdPtr(), hwaddr2_, + IOAddress("192.0.2.101"), false, + false, "", false, CalloutHandlePtr(), + old_lease_)); + + // Client A tries to renew the lease. The renewal should fail because + // server detects that Client A doesn't have reservation for this + // address. + ASSERT_FALSE(engine.allocateLease4(subnet_, clientid_, hwaddr_, + IOAddress("192.0.2.101"), false, false, + "", false, CalloutHandlePtr(), + old_lease_)); + ASSERT_TRUE(old_lease_); + EXPECT_EQ("192.0.2.101", old_lease_->addr_.toText()); + + // Client A returns to DHCPDISCOVER and should be offered a lease. + // The offered lease address must be different than the one the + // Client B has reservation for. + Lease4Ptr offered_lease = engine.allocateLease4(subnet_, clientid_, + hwaddr_, + IOAddress("192.0.2.101"), + false, false, "", true, + CalloutHandlePtr(), + old_lease_); + ASSERT_TRUE(offered_lease); + EXPECT_NE(offered_lease->addr_.toText(), "192.0.2.101"); + + // Client A tried to acquire the lease. It should succeed. At this point + // the previous lease should be released and become available for the + // Client B. + Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_, + hwaddr_, + offered_lease->addr_, + false, false, "", false, + CalloutHandlePtr(), + old_lease_); + ASSERT_TRUE(allocated_lease); + EXPECT_NE(allocated_lease->addr_.toText(), "192.0.2.101"); + + // Client B tries to get the lease again. It should be offered + // a reserved lease. + offered_lease = engine.allocateLease4(subnet_, ClientIdPtr(), + hwaddr2_, + IOAddress("0.0.0.0"), + false, false, "", true, + CalloutHandlePtr(), + old_lease_); + ASSERT_TRUE(offered_lease); + EXPECT_EQ("192.0.2.101", offered_lease->addr_.toText()); + + // Client B requests allocation of the lease and it should succeed. + allocated_lease = engine.allocateLease4(subnet_, ClientIdPtr(), + hwaddr2_, + offered_lease->addr_, + false, false, "", false, + CalloutHandlePtr(), + old_lease_); + ASSERT_TRUE(allocated_lease); + EXPECT_EQ("192.0.2.101", allocated_lease->addr_.toText()); +} + +// This test checks that the address is not assigned from the dynamic +// pool if it has been reserved for another client. +TEST_F(AllocEngine4Test, reservedAddressVsDynamicPool) { + // Create a reservation for the client. + HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, subnet_->getID(), + SubnetID(0), IOAddress("192.0.2.100"))); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false); + + // Different client tries to allocate a lease. Note, that we're using + // an iterative allocator which would pick the first address from the + // dynamic pool, i.e. 192.0.2.100. This address is reserved so we expect + // that a different address will be allocated. + Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, ClientIdPtr(), + hwaddr_, + IOAddress("0.0.0.0"), + false, false, "", false, + CalloutHandlePtr(), + old_lease_); + ASSERT_TRUE(allocated_lease); + EXPECT_NE(allocated_lease->addr_.toText(), "192.0.2.100"); +} + /// @brief helper class used in Hooks testing in AllocEngine6 /// /// It features a couple of callout functions and buffers to store diff --git a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc index 04ed40d923..3b22d0a41b 100644 --- a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc @@ -19,6 +19,7 @@ #include #include #include +#include using namespace isc; using namespace isc::dhcp; @@ -188,6 +189,35 @@ TEST_F(CfgHostsTest, getAllRepeatingHosts) { } } +// This test checks that all reservations for the specified IPv4 address can +// be retrieved. +TEST_F(CfgHostsTest, getAll4ByAddress) { + CfgHosts cfg; + // Add hosts. + for (int i = 0; i < 25; ++i) { + // Add host identified by the HW address. + cfg.add(HostPtr(new Host(hwaddrs_[i]->toText(false), + "hw-address", + SubnetID(1 + i), SubnetID(0), + IOAddress("192.0.2.5")))); + // Add host identified by the DUID. + cfg.add(HostPtr(new Host(duids_[i]->toText(), + "duid", + SubnetID(1 + i), SubnetID(0), + IOAddress("192.0.2.10")))); + } + + HostCollection hosts = cfg.getAll4(IOAddress("192.0.2.10")); + std::set subnet_ids; + for (HostCollection::const_iterator host = hosts.begin(); host != hosts.end(); + ++host) { + subnet_ids.insert((*host)->getIPv4SubnetID()); + } + ASSERT_EQ(25, subnet_ids.size()); + EXPECT_EQ(1, *subnet_ids.begin()); + EXPECT_EQ(25, *subnet_ids.rbegin()); +} + // This test checks that the reservations can be retrieved for the particular // host connected to the specific IPv4 subnet (by subnet id). TEST_F(CfgHostsTest, get4) {