Commit e29eee4f authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[3564] Resolve conflicts with the reserved addresses in allocation engine.

parent bf3ea66a
......@@ -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");
......
......@@ -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.
///
......
......@@ -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
......
......@@ -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<ConstHostCollection>(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<HostCollection>(address, collection);
return (collection);
}
template<typename Storage>
......@@ -76,6 +80,25 @@ CfgHosts::getAllInternal(const HWAddrPtr& hwaddr, const DuidPtr& duid,
}
}
template<typename Storage>
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 {
......
......@@ -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<typename Storage>
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
......
......@@ -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<Host, const asiolink::IOAddress&,
&Host::getIPv4Reservation>
>
>
> HostContainer;
......@@ -78,6 +86,16 @@ typedef HostContainer::nth_index<0>::type HostContainerIndex0;
typedef std::pair<HostContainerIndex0::iterator,
HostContainerIndex0::iterator> 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<HostContainerIndex1::iterator,
HostContainerIndex1::iterator> HostContainerIndex1Range;
}
}
......
......@@ -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 {
......
......@@ -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
......
......@@ -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_,