[3694] Restructured lease4 allocation routines.

src/lib/dhcpsrv/alloc_engine.cc

@@ -29,6 +29,7 @@
 #include <string.h>
 
 using namespace isc::asiolink;
+using namespace isc::dhcp;
 using namespace isc::hooks;
 
 namespace {
@@ -57,11 +58,63 @@ struct AllocEngineHooks {
 // module is called.
 AllocEngineHooks Hooks;
 
+bool
+addressReserved(const IOAddress& address, const AllocEngine::ClientContext4& ctx) {
+    ConstHostPtr host = HostMgr::instance().get4(ctx.subnet_->getID(), address);
+    HWAddrPtr host_hwaddr;
+    if (host) {
+        host_hwaddr = host->getHWAddress();
+        if (ctx.hwaddr_ && host_hwaddr) {
+            /// @todo Use the equality operators for HWAddr class.
+            /// Currently, this is impossible because the HostMgr uses the
+            /// HTYPE_ETHER type, whereas the unit tests may use other types
+            /// which HostMgr doesn't support yet.
+            return (host_hwaddr->hwaddr_ != ctx.hwaddr_->hwaddr_);
+
+        } else {
+            return (true);
+
+        }
+    }
+    return (false);
+}
+
+
 }; // anonymous namespace
 
 namespace isc {
 namespace dhcp {
 
+AllocEngine::ClientContext4::ClientContext4()
+    : subnet_(), clientid_(), hwaddr_(),
+      requested_address_(IOAddress::IPV4_ZERO_ADDRESS()),
+      fwd_dns_update_(false), rev_dns_update_(false),
+      hostname_(""), callout_handle_(), fake_allocation_(false),
+      old_lease_(), host_(), lease_for_host_(),
+      interrupt_processing_(false) {
+}
+
+bool
+AllocEngine::ClientContext4::myLease(const Lease4& lease) const {
+    if ((!hwaddr_ && lease.hwaddr_) || (hwaddr_ && !lease.hwaddr_)) {
+        return (false);
+    }
+
+    if ((hwaddr_ && lease.hwaddr_) && (hwaddr_->hwaddr_ != lease.hwaddr_->hwaddr_)) {
+        return (false);
+    }
+
+    if ((!clientid_ && lease.client_id_) || (clientid_ && !lease.client_id_)) {
+        return (false);
+    }
+
+    if ((clientid_ && lease.client_id_) && (*clientid_ != *lease.client_id_)) {
+        return (false);
+    }
+
+    return (true);
+}
+
 AllocEngine::IterativeAllocator::IterativeAllocator(Lease::Type lease_type)
     :Allocator(lease_type) {
 }
@@ -813,11 +866,32 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid
     // database.
     old_lease.reset();
 
-    try {
-
-        // Set allocator.
-        AllocatorPtr allocator = getAllocator(Lease::TYPE_V4);
+    Lease4Ptr new_lease;
+
+    /// @todo The context for lease allocation should really be created
+    /// by the DHCPv4 server and passed to this function. The reason for
+    /// this is that the server should retrieve the Host object for the
+    /// client because the Host object contains the data not only useful
+    /// for the address allocation but also hostname and DHCP options
+    /// for the client. The Host object should be passed in the context.
+    /// Making this change would require a change to the allocateLease4
+    /// API which would in turn require lots of changes in unit tests.
+    /// The ticket introducing a context and host reservation in the
+    /// allocation engine is complex enough by itself to warrant that
+    /// the API change is done with a separate ticket (#3709).
+    ClientContext4 ctx;
+    ctx.subnet_ = subnet;
+    ctx.clientid_ = clientid;
+    ctx.hwaddr_ = hwaddr;
+    ctx.requested_address_ = hint;
+    ctx.fwd_dns_update_ = fwd_dns_update;
+    ctx.rev_dns_update_ = rev_dns_update;
+    ctx.hostname_ = hostname;
+    ctx.fake_allocation_ = fake_allocation;
+    ctx.callout_handle_ = callout_handle;
+    ctx.old_lease_ = old_lease;
 
+    try {
         if (!subnet) {
             isc_throw(BadValue, "Can't allocate IPv4 address without subnet");
         }
@@ -826,239 +900,24 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid
             isc_throw(BadValue, "HWAddr must be defined");
         }
 
-        /// @todo The context for lease allocation should really be created
-        /// by the DHCPv4 server and passed to this function. The reason for
-        /// this is that the server should retrieve the Host object for the
-        /// client because the Host object contains the data not only useful
-        /// for the address allocation but also hostname and DHCP options
-        /// for the client. The Host object should be passed in the context.
-        /// Making this change would require a change to the allocateLease4
-        /// API which would in turn require lots of changes in unit tests.
-        /// The ticket introducing a context and host reservation in the
-        /// allocation engine is complex enough by itself to warrant that
-        /// the API change is done with a separate ticket (#3709).
-        ClientContext4 ctx;
-        ctx.subnet_ = subnet;
-        ctx.clientid_ = clientid;
-        ctx.hwaddr_ = hwaddr;
-        ctx.requested_address_ = hint;
-        ctx.fwd_dns_update_ = fwd_dns_update;
-        ctx.rev_dns_update_ = rev_dns_update;
-        ctx.hostname_ = hostname;
-        ctx.fake_allocation_ = fake_allocation;
-        ctx.callout_handle_ = callout_handle;
-        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.requested_address_ == IOAddress("0.0.0.0")) {
-                ctx.requested_address_ = 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.requested_address_ != 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);
-            // 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);
-            }
-        }
-
-        // 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.requested_address_)) {
-            // 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. 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_ &&
-                (ctx.requested_address_ != IOAddress("0.0.0.0")) &&
-                (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) {
-                return (Lease4Ptr());
-            }
-
-            // Now let's pick an address to be allocated to the client. The
-            // candidate address may either be a reserved one or the one that
-            // the client requests.
-            IOAddress candidate = 0;
-            ConstHostPtr other_host;
-            if (ctx.host_) {
-                candidate = ctx.host_->getIPv4Reservation();
-
-            } else {
-                candidate = ctx.requested_address_;
-                // If client is requesting an address we have to check if this address
-                // is not reserved for someone else. Note that for DHCPDISCOVER we
-                // treat the requested address as a hint and we don't return an empty
-                // lease.
-                other_host = HostMgr::instance().get4(ctx.subnet_->getID(), candidate);
-                if (!ctx.fake_allocation_ && other_host) {
-                    return (Lease4Ptr());
-                }
-            }
-
-            // If address is not reserved for another client, let's try allocate it.
-            if (!other_host) {
-                // 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
-                    // lease for it.
-                    Lease4Ptr lease = createLease4(subnet, clientid, hwaddr,
-                                                   candidate, fwd_dns_update,
-                                                   rev_dns_update,
-                                                   hostname, callout_handle,
-                                                   fake_allocation);
-
-                    // If we have allocated the lease let's return it. Also,
-                    // always return when tried to allocate reserved address,
-                    // regardless if allocation was successful or not. If it
-                    // was not successful, we will return a NULL pointer which
-                    // indicates to the server that it should send NAK to the
-                    // client.
-                    if (lease || ctx.host_) {
-                        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));
-
-                        // 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());
-
-                    }
-
-                }
-            }
+        new_lease = ctx.fake_allocation_ ? discoverLease4(ctx) : requestLease4(ctx);
+        if (!new_lease) {
+            // Unable to allocate an address, return an empty lease.
+            LOG_WARN(dhcpsrv_logger, DHCPSRV_ADDRESS4_ALLOC_FAIL).arg(attempts_);
         }
 
-        // No address was requested, requested address was not in pool or the
-        // allocation was not successful so far. Let's try to find a different
-        // address for the client.  Search the pool until first of the following
-        // occurs:
-        // - we find a free address
-        // - we find an address for which the lease has expired
-        // - we exhaust the number of tries
-        //
-        /// @todo: We used to use hardcoded number of attempts (100). Now we dynamically
-        /// calculate the number of possible leases in all pools in this subnet and
-        /// try that number of times at most. It would be useful to that value if
-        /// attempts_, specified by the user could override that value (and keep
-        /// dynamic if they're set to 0).
-        uint64_t i = subnet->getPoolCapacity(Lease::TYPE_V4);
-        do {
-            // Decrease the number of remaining attempts here so as we guarantee
-            // that it is decreased when the code below uses "continue".
-            --i;
-            IOAddress candidate = allocator->pickAddress(subnet, clientid,
-                                                         ctx.requested_address_);
-
-            // 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) {
-                // there's no existing lease for selected candidate, so it is
-                // free. Let's allocate it.
-                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr,
-                                               candidate, fwd_dns_update,
-                                               rev_dns_update, hostname,
-                                               callout_handle, fake_allocation);
-                if (lease) {
-                    return (lease);
-                }
-
-                // Although the address was free just microseconds ago, it may have
-                // been taken just now. If the lease insertion fails, we continue
-                // allocation attempts.
-            } else {
-                if (existing->expired()) {
-                    // Save old lease before reusing it.
-                    old_lease.reset(new Lease4(*existing));
-                    return (reuseExpiredLease(existing, ctx));
-                }
-            }
-
-            // Continue trying allocation until we run out of attempts
-            // (or attempts are set to 0, which means infinite)
-        } while ((i > 0) || !attempts_);
-
-        // Unable to allocate an address, return an empty lease.
-        LOG_WARN(dhcpsrv_logger, DHCPSRV_ADDRESS4_ALLOC_FAIL).arg(attempts_);
-
     } catch (const isc::Exception& e) {
-
         // Some other error, return an empty lease.
         LOG_ERROR(dhcpsrv_logger, DHCPSRV_ADDRESS4_ALLOC_ERROR).arg(e.what());
     }
-    return (Lease4Ptr());
+
+    if (ctx.old_lease_) {
+        old_lease.reset(new Lease4(*ctx.old_lease_));
+    }
+
+    return (new_lease);
 }
 
 Lease4Ptr
@@ -1074,12 +933,10 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
     // 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_);
         // Do not renew the lease if:
         // - If address is reserved for someone else or ...
         // - renewed address doesn't belong to a pool.
-        if ((host && ctx.hwaddr_ && (*host->getHWAddress() != *ctx.hwaddr_)) ||
+        if (addressReserved(lease->addr_, ctx) ||
             (!ctx.subnet_->inPool(Lease::TYPE_V4, lease->addr_))) {
             ctx.interrupt_processing_ = !ctx.fake_allocation_;
             return (Lease4Ptr());
@@ -1092,6 +949,7 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
     // We'll need it if we want to skip update (i.e. roll back renewal)
     /// @todo: remove this once #3083 is implemented
     Lease4 old_values = *lease;
+    ctx.old_lease_.reset(new Lease4(old_values));
 
     // Update the lease with the information from the context.
     updateLease4Information(lease, ctx);
@@ -1222,6 +1080,25 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
 }
 
 Lease4Ptr
+AllocEngine::allocateOrReuseLease(const IOAddress& candidate, ClientContext4& ctx) {
+    Lease4Ptr exist_lease = LeaseMgrFactory::instance().getLease4(candidate);
+    if (exist_lease) {
+        if (exist_lease->expired()) {
+            ctx.old_lease_.reset(new Lease4(*exist_lease));
+            return (reuseExpiredLease(exist_lease, ctx));
+        }
+
+    } else {
+        return (createLease4(ctx.subnet_, ctx.clientid_,
+                             ctx.hwaddr_, candidate, ctx.fwd_dns_update_,
+                             ctx.rev_dns_update_, ctx.hostname_, ctx.callout_handle_,
+                             ctx.fake_allocation_));
+    }
+    return (Lease4Ptr());
+}
+
+
+Lease4Ptr
 AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
                                AllocEngine::ClientContext4& ctx) {
     if (!expired) {
@@ -1292,125 +1169,131 @@ AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
 }
 
 Lease4Ptr
-AllocEngine::replaceClientLease(Lease4Ptr& lease, ClientContext4& ctx) {
+AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) { 
+    LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
 
-    if (!lease) {
-        isc_throw(BadValue, "null lease specified for replaceClientLease");
+    Lease4Ptr client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID());
+    if (!client_lease && ctx.clientid_) {
+        client_lease = lease_mgr.getLease4(*ctx.clientid_, ctx.subnet_->getID());
     }
 
-    if (!ctx.subnet_) {
-        isc_throw(BadValue, "null subnet specified for replaceClientLease");
+    Lease4Ptr new_lease;
+    if (ctx.host_) {
+        new_lease = allocateOrReuseLease(ctx.host_->getIPv4Reservation(), ctx);
+        if (new_lease) {
+            if (client_lease) {
+                ctx.old_lease_.reset(new Lease4(*client_lease));
+            }
+            return (new_lease);
+        }
     }
 
-    if (ctx.requested_address_ == IOAddress("0.0.0.0")) {
-        isc_throw(BadValue, "zero address specified for the"
-                  " replaceClientLease");
-    }
+    if (ctx.requested_address_.isV4Zero() && !addressReserved(ctx.requested_address_, ctx)) {
+        if (ctx.subnet_->inPool(Lease::TYPE_V4, ctx.requested_address_)) {
+            new_lease = allocateOrReuseLease(ctx.requested_address_, ctx);
+            if (new_lease) {
+                if (client_lease) {
+                    ctx.old_lease_.reset(new Lease4(*client_lease));
+                }
+                return (new_lease);
+            }
+        }
 
-    // Remember the previous address for this lease.
-    IOAddress prev_address = lease->addr_;
+        if (client_lease) {
+            if ((client_lease->addr_ == ctx.requested_address_) ||
+                ctx.requested_address_.isV4Zero()) {
+                return (renewLease4(client_lease, ctx));
+            }
+        }
+    }
 
-    if (!ctx.host_) {
-        ConstHostPtr host = HostMgr::instance().get4(ctx.subnet_->getID(),
+    AllocatorPtr allocator = getAllocator(Lease::TYPE_V4);
+    const uint64_t attempts = attempts_ == 0 ? std::numeric_limits<uint64_t>::max() : attempts_;
+    for (uint64_t i = ctx.subnet_->getPoolCapacity(Lease::TYPE_V4); i < attempts; ++i) {
+        IOAddress candidate = allocator->pickAddress(ctx.subnet_, ctx.clientid_,
                                                      ctx.requested_address_);
-        // If there is a reservation for the new address and the reservation
-        // is made for another client, do not use this address.
-        if (host && ctx.hwaddr_ && (*host->getHWAddress() != *ctx.hwaddr_)) {
-            ctx.interrupt_processing_ = true;
-            return (Lease4Ptr());
+        if (!addressReserved(candidate, ctx)) {
+            new_lease = allocateOrReuseLease(candidate, ctx);
+            if (new_lease) {
+                return (new_lease);
+            }
         }
-        lease->addr_ = ctx.requested_address_;
-    } else {
-        lease->addr_ = ctx.host_->getIPv4Reservation();
     }
 
-    updateLease4Information(lease, ctx);
-
-    bool skip = false;
-    // Execute callouts registered for lease4_select.
-    if (ctx.callout_handle_ && HooksManager::getHooksManager()
-        .calloutsPresent(hook_index_lease4_select_)) {
-
-        // Delete all previous arguments.
-        ctx.callout_handle_->deleteAllArguments();
+    return (Lease4Ptr());
+}
 
-        // Pass arguments.
-        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(ctx.subnet_);
-        ctx.callout_handle_->setArgument("subnet4", subnet4);
+Lease4Ptr
+AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
+    LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
+    Lease4Ptr client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID());
+    if (!client_lease && ctx.clientid_) {
+        client_lease = lease_mgr.getLease4(*ctx.clientid_, ctx.subnet_->getID());
+    }
 
-        ctx.callout_handle_->setArgument("fake_allocation",
-                                         ctx.fake_allocation_);
+    if (!ctx.requested_address_.isV4Zero()) {
+        if (addressReserved(ctx.requested_address_, ctx)) {
+            return (Lease4Ptr());
+        }
 
-        ctx.callout_handle_->setArgument("lease4", lease);
+    } else if (ctx.host_) {
+        ctx.requested_address_ = ctx.host_->getIPv4Reservation();
+    }
 
-        HooksManager::callCallouts(hook_index_lease4_select_,
-                                   *ctx.callout_handle_);
+    if (!ctx.requested_address_.isV4Zero()) {
+        Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(ctx.requested_address_);
+        if (existing && !existing->expired()) {
+            if (!ctx.myLease(*existing)) {
+                return (Lease4Ptr());
+            }
 
-        if (ctx.callout_handle_->getSkip()) {
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
-                      DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
+        } else if (ctx.host_ && (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) {
             return (Lease4Ptr());
         }
+    }
 
-        // Let's use whatever callout returned.
-        ctx.callout_handle_->getArgument("lease4", lease);
-
-        // Callouts decided to skip the next processing step. The next
-        // processing step would to actually renew the lease, so skip at this
-        // stage means "keep the old lease as it is".
-        if (ctx.callout_handle_->getSkip()) {
-            skip = true;
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
-                      DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
+    if (client_lease) {
+        if ((client_lease->addr_ == ctx.requested_address_) ||
+            ctx.requested_address_.isV4Zero()) {
+            return (renewLease4(client_lease, ctx));
         }
     }
 
-    /// @todo There should be a callout for a deletion of an old lease.
-    /// The lease4_release callout is in appropriate, because by definition
-    /// it is invoked when DHCPRELEASE packet is received.
-
-    if (!ctx.fake_allocation_ && !skip) {
-        // We can't use LeaseMgr::updateLease because it identifies the
-        // lease by an IP address. Instead, we have to delete an old
-        // lease and add a new one.
-        LeaseMgrFactory::instance().deleteLease(prev_address);
-        LeaseMgrFactory::instance().addLease(lease);
+    if (!ctx.subnet_->inPool(Lease4::TYPE_V4, ctx.requested_address_)) {
+        if ((ctx.host_ && (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) ||
+            (!ctx.host_ && !ctx.requested_address_.isV4Zero())) {
+            return (Lease4Ptr());
+        }
     }
 
-    return (lease);
-}
-
-Lease4Ptr
-AllocEngine::reallocateClientLease(Lease4Ptr& lease,
-                                   AllocEngine::ClientContext4& ctx) {
-    // Save the old lease, before renewal.
-    ctx.old_lease_.reset(new Lease4(*lease));
-
-    /// The client's address will need to be modified in case if:
-    /// - There is a reservation for the client (likely new one) and
-    ///   the currently used address is different.
-    /// - Client requested some IP address and the requested address
-    ///   is different than the currently used one. Note that if this
-    ///   is a DHCPDISCOVER the requested IP address is ignored when
-    ///   it doesn't match the one in use.
-    if ((ctx.host_ && (ctx.host_->getIPv4Reservation() != lease->addr_)) ||
-        (!ctx.fake_allocation_ &&
-         (ctx.requested_address_ != IOAddress("0.0.0.0")) &&
-         (lease->addr_ != ctx.requested_address_))) {
-        lease = replaceClientLease(lease, ctx);
-        return (lease);
+    Lease4Ptr new_lease;
+    if (!ctx.requested_address_.isV4Zero()) {
+        new_lease = allocateOrReuseLease(ctx.requested_address_, ctx);
+        if (new_lease) {
+            if (client_lease && (client_lease->addr_ != new_lease->addr_)) {
+                ctx.old_lease_ = client_lease;
+                lease_mgr.deleteLease(client_lease->addr_);
+            }
+            return (new_lease);
+        }
+    }
 
-    } else {
-        lease = renewLease4(lease, ctx);
-        if (lease) {
-            return (lease);
+    AllocatorPtr allocator = getAllocator(Lease::TYPE_V4);
+    const uint64_t attempts = attempts_ == 0 ? std::numeric_limits<uint64_t>::max() : attempts_;
+    for (uint64_t i = ctx.subnet_->getPoolCapacity(Lease::TYPE_V4); i < attempts; ++i) {
+        IOAddress candidate = allocator->pickAddress(ctx.subnet_, ctx.clientid_,
+                                                     ctx.requested_address_);
+        if (!addressReserved(candidate, ctx)) {
+            new_lease = allocateOrReuseLease(candidate, ctx);
+            if (new_lease) {
+                return (new_lease);
+            }
         }
     }
 
     return (Lease4Ptr());
 }
 
-
 Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
                                     const IOAddress& addr,
                                     uint8_t prefix_len) {

src/lib/dhcpsrv/alloc_engine.h

@@ -268,6 +268,9 @@ protected:
         /// @brief A pointer to the object identifying host reservations.
         ConstHostPtr host_;
 
+        /// @brief A pointer to the existing lease for the host reservation.
+        Lease4Ptr lease_for_host_;
+
         /// @brief Signals that the allocation should be interrupted.
         ///
         /// This flag is set by the downstream methods called by the
@@ -285,12 +288,10 @@ protected:
         bool interrupt_processing_;
 
         /// @brief Default constructor.
-        ClientContext4()
-            : subnet_(), clientid_(), hwaddr_(), requested_address_("0.0.0.0"),
-              fwd_dns_update_(false), rev_dns_update_(false),
-              hostname_(""), callout_handle_(), fake_allocation_(false),
-              old_lease_(), host_(), interrupt_processing_(false) {
-        }
+        ClientContext4();
+
+        bool myLease(const Lease4& lease) const;
+
     };
 
     /// @brief Defines a single hint (an address + prefix-length).
@@ -853,6 +854,9 @@ private:
     removeNonreservedLeases6(ClientContext6& ctx,
                              Lease6Collection& existing_leases);
 
+    Lease4Ptr allocateOrReuseLease(const asiolink::IOAddress& address,
+                                   ClientContext4& ctx);
+
     /// @brief Reuses expired DHCPv4 lease.
     ///
     /// Makes new allocation using an expired lease. The lease is updated with
@@ -905,6 +909,10 @@ private:
     /// @throw BadValue if specified parameters are invalid.
     Lease4Ptr reallocateClientLease(Lease4Ptr& lease, ClientContext4& ctx);
 
+    Lease4Ptr discoverLease4(ClientContext4& ctx);
+
+    Lease4Ptr requestLease4(ClientContext4& ctx);
+
     /// @brief Reuses expired IPv6 lease
     ///
     /// Updates existing expired lease with new information. Lease database

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc

@@ -163,10 +163,10 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
     // unfortunately it is used already. The same address must not be allocated
     // twice.
     Lease4Ptr lease = engine->allocateLease4(subnet_, clientid_, hwaddr_,
-                                               IOAddress("192.0.2.106"),
-                                               false, false, "",
-                                               false, CalloutHandlePtr(),
-                                               old_lease_);
+                                             IOAddress("192.0.2.106"),
+                                             false, false, "",
+                                             true, CalloutHandlePtr(),
+                                             old_lease_);
 
     // New lease has been allocated, so the old lease should not exist.
     EXPECT_FALSE(old_lease_);
@@ -183,12 +183,9 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
     // Do all checks on the lease
     checkLease4(lease);
 
-    // Check that the lease is indeed in LeaseMgr
+    // The lease should not be in the LeaseMgr because it was a failed allocation.
     Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
-    ASSERT_TRUE(from_mgr);
-
-    // Now check that the lease in LeaseMgr has the same parameters
-    detailCompareLease(lease, from_mgr);
+    ASSERT_FALSE(from_mgr);
 }
 
 
@@ -204,10 +201,10 @@ TEST_F(AllocEngine4Test, allocBogusHint4) {
     // supported lease. Allocation engine should ignore it and carry on
     // with the normal allocation
     Lease4Ptr lease = engine->allocateLease4(subnet_, clientid_, hwaddr_,
-                                               IOAddress("10.1.1.1"),
-                                               false, false, "",
-                                               false, CalloutHandlePtr(),
-                                               old_lease_);
+                                             IOAddress("10.1.1.1"),
+                                             false, false, "",
+                                             true, CalloutHandlePtr(),
+                                             old_lease_);
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -220,15 +217,11 @@ TEST_F(AllocEngine4Test, allocBogusHint4) {
     // Do all checks on the lease
     checkLease4(lease);
 
-    // Check that the lease is indeed in LeaseMgr
+    // Check that the lease is not in the LeaseMgr as it is a fake allocation.
     Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
-    ASSERT_TRUE(from_mgr);
-
-    // Now check that the lease in LeaseMgr has the same parameters
-    detailCompareLease(lease, from_mgr);
+    EXPECT_FALSE(from_mgr);
 }
 
-
 // This test checks that NULL values are handled properly
 TEST_F(AllocEngine4Test, allocateLease4Nulls) {
     boost::scoped_ptr<AllocEngine> engine;
@@ -603,6 +596,37 @@ TEST_F(AllocEngine4Test, renewLease4) {
     detailCompareLease(lease, from_mgr);
 }
 
+TEST_F(AllocEngine4Test, requestOtherClientLease) {
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+
+    Lease4Ptr lease2(new Lease4(IOAddress("192.0.2.102"), hwaddr2_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+
+    LeaseMgrFactory::instance().addLease(lease);
+    LeaseMgrFactory::instance().addLease(lease2);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    Lease4Ptr new_lease = engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                                IOAddress("192.0.2.102"),
+                                                false, false, "",
+                                                false, CalloutHandlePtr(),
+                                                old_lease_);
+
+    ASSERT_FALSE(new_lease);
+
+    new_lease = engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                                IOAddress("192.0.2.102"),
+                                                false, false, "",
+                                                true, CalloutHandlePtr(),
+                                                old_lease_);
+    ASSERT_TRUE(new_lease);
+
+}
+
 // This test checks the behavior of the allocation engine in the following
 // scenario:
 // - Client has no lease in the database.
@@ -802,8 +826,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLease) {
     EXPECT_EQ("192.0.2.123", allocated_lease->addr_.toText());
 
     // Make sure that the lease has been committed to the lease database.
-    Lease4Ptr from_mgr =
-        LeaseMgrFactory::instance().getLease4(allocated_lease->addr_);
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(allocated_lease->addr_);
     ASSERT_TRUE(from_mgr);
     detailCompareLease(allocated_lease, from_mgr);
 
@@ -879,13 +902,7 @@ TEST_F(AllocEngine4Test, reservedAddressHijacked) {
 // - Client B has a reservation for the address in use by client A.
 // - Client B sends a DHCPDISCOVER.
 // - Server determines that the reserved address is in use by a different client
-// and that it can't allocate a lease to the client B.
-//
-// In the scenario presented here, the allocation engine should return a
-// NULL lease to the server. When the server receives NULL pointer from the
-// allocation engine the proper action for the server will be to not
-// respond to the client. Instead it should report to the administrator
-// that it was unable to allocate the (reserved) lease.
+//   so it offers and address from the dynamic pool.
 TEST_F(AllocEngine4Test, reservedAddressHijackedFakeAllocation) {
     // Create a reservation for the client B.
     HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
@@ -912,9 +929,12 @@ TEST_F(AllocEngine4Test, reservedAddressHijackedFakeAllocation) {
                                                       true, CalloutHandlePtr(),
                                                       old_lease_);
     // The allocation engine should return no lease.
-    ASSERT_FALSE(allocated_lease);
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_NE(allocated_lease->addr_.toText(), "192.0.2.123");
+    EXPECT_TRUE(subnet_->inPool(Lease::TYPE_V4, allocated_lease->addr_));
     EXPECT_FALSE(old_lease_);
 
+
     // Do the same test. But, this time do not specify any address to be
     // allocated.
     allocated_lease = engine.allocateLease4(subnet_, clientid_,
@@ -923,7 +943,9 @@ TEST_F(AllocEngine4Test, reservedAddressHijackedFakeAllocation) {
                                             false, false, "",
                                             true, CalloutHandlePtr(),
                                             old_lease_);
-    EXPECT_FALSE(allocated_lease);
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_NE(allocated_lease->addr_.toText(), "192.0.2.123");
+    EXPECT_TRUE(subnet_->inPool(Lease::TYPE_V4, allocated_lease->addr_));
     EXPECT_FALSE(old_lease_);
 }
 
@@ -990,7 +1012,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) {
 // - Client has a reservation for a different address than the one for which it
 // has a lease.
 // - Client sends a DHCPDISCOVER and asks for a different address than reserved
-// and different from which it has a lease for.
+//   and different from which it has a lease for.
 // - Server ignores the client's hint and offers a reserved address.
 TEST_F(AllocEngine4Test, reservedAddressExistingLeaseFakeAllocation) {
     // Create a reservation for the client.
@@ -1179,12 +1201,15 @@ TEST_F(AllocEngine4Test, reservedAddressConflictResolution) {
 
 
     // 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_));
+    // allocation engine can't allocate a reserved lease for this client
+    // because this specific address is in use by the Client A.
+    Lease4Ptr offered_lease = engine.allocateLease4(subnet_, ClientIdPtr(),
+                                                    hwaddr2_,
+                                                    IOAddress("192.0.2.101"),
+                                                    false, false, "", false,
+                                                    CalloutHandlePtr(),
+                                                    old_lease_);
+    ASSERT_FALSE(offered_lease);
 
     // Client A tries to renew the lease. The renewal should fail because
     // server detects that Client A doesn't have reservation for this
@@ -1193,18 +1218,17 @@ TEST_F(AllocEngine4Test, reservedAddressConflictResolution) {
                                        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());
+    ASSERT_FALSE(old_lease_);
 
     // 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_);
+    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");