[3563] v6 lease allocation restructured.

src/lib/dhcpsrv/alloc_engine.cc

@@ -297,13 +297,6 @@ Lease6Collection
 AllocEngine::allocateLeases6(ClientContext6& ctx) {
 
     try {
-        AllocatorPtr allocator = getAllocator(ctx.type_);
-
-        if (!allocator) {
-            isc_throw(InvalidOperation, "No allocator specified for "
-                      << Lease6::typeToText(ctx.type_));
-        }
-
         if (!ctx.subnet_) {
             isc_throw(InvalidOperation, "Subnet is required for allocation");
         }
@@ -312,153 +305,137 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
             isc_throw(InvalidOperation, "DUID is mandatory for allocation");
         }
 
-        // Check if there's existing lease for that subnet/duid/iaid
-        // combination.
-        /// @todo: Make this generic (cover temp. addrs and prefixes)
-        ctx.old_leases_ = LeaseMgrFactory::instance().getLeases6(ctx.type_,
-                              *ctx.duid_, ctx.iaid_, ctx.subnet_->getID());
+        // Check if there's a host reservation for this client.
+        ctx.host_ = HostMgr::instance().get6(ctx.subnet_->getID(), ctx.duid_,
+                                             ctx.hwaddr_);
 
-        // There is at least one lease for this client. We will return these
-        // leases for the client, but we may need to update FQDN information.
-        if (!ctx.old_leases_.empty()) {
-            // Return old leases so the server can see what has changed.
-            return (updateFqdnData(ctx.old_leases_, ctx.fwd_dns_update_,
-                                   ctx.rev_dns_update_,
-                                   ctx.hostname_, ctx.fake_allocation_));
-        }
+        // Check if there are existing leases for that subnet/duid/iaid
+        // combination.
+        Lease6Collection leases =
+            LeaseMgrFactory::instance().getLeases6(ctx.type_, *ctx.duid_, ctx.iaid_,
+                                                   ctx.subnet_->getID());
+
+        // Now do the checks:
+        // Case 1. if there are no leases, and there are reservations...
+        //   1.1. are the reserved addresses are used by someone else?
+        //       yes: we have a problem
+        //       no: assign them => done
+        // Case 2. if there are leases and there are no reservations...
+        //   2.1 are the leases reserved for someone else?
+        //       yes: release them, assign something else
+        //       no: renew them => done
+        // Case 3. if there are leases and there are reservations...
+        //   3.1 are the leases matching reservations?
+        //       yes: renew them => done
+        //       no: release existing leases, assign new ones based on reservations
+        // Case 4. if there are no leases and no reservations...
+        //       assign new leases
+        //
+        // We could implement those checks as nested ifs, but the performance
+        // gain would be minimal and the code readibility loss would be substantial.
+        // Hence independent checks.
 
-        IOAddress hint("::");
-        if (!ctx.hints_.empty()) {
-            /// @todo: We support only one hint for now
-            hint = ctx.hints_[0];
-        }
+        // Case 1: There are no leases and there's a reservation for this host.
+        if (leases.empty() && ctx.host_) {
 
-        // check if the hint is in pool and is available
-        // This is equivalent of subnet->inPool(hint), but returns the pool
-        Pool6Ptr pool = boost::dynamic_pointer_cast<
-            Pool6>(ctx.subnet_->getPool(ctx.type_, hint, false));
-
-        if (pool) {
-            /// @todo: We support only one hint for now
-            Lease6Ptr lease = LeaseMgrFactory::instance().getLease6(ctx.type_, hint);
-            if (!lease) {
-                /// @todo: check if the hint is reserved once we have host
-                /// support implemented
-
-                // The hint is valid and not currently used, let's create a
-                // lease for it
-                lease = createLease6(ctx,
-                                     hint,
-                                     pool->getLength());
-
-                // It can happen that the lease allocation failed (we could
-                // have lost the race condition. That means that the hint is
-                // lo longer usable and we need to continue the regular
-                // allocation path.
-                if (lease) {
-                    // We are allocating a new lease (not renewing). So, the
-                    // old lease should be NULL.
-                    ctx.old_leases_.clear();
-
-                    /// @todo: We support only one lease per ia for now
-                    Lease6Collection collection;
-                    collection.push_back(lease);
-                    return (collection);
-                }
-            } else {
-                if (lease->expired()) {
-                    // Copy an existing, expired lease so as it can be returned
-                    // to the caller.
-                    Lease6Ptr old_lease(new Lease6(*lease));
-                    ctx.old_leases_.push_back(old_lease);
-
-                    /// We found a lease and it is expired, so we can reuse it
-                    lease = reuseExpiredLease(lease,
-                                              ctx,
-                                              pool->getLength());
-
-                    /// @todo: We support only one lease per ia for now
-                    Lease6Collection collection;
-                    collection.push_back(lease);
-                    return (collection);
-                }
+            // Try to allocate leases that match reservations. Typically this will
+            // succeed, except cases where the reserved addresses are used by
+            // someone else.
+            allocateReservedLeases6(ctx, leases);
 
+            // If we got at least one lease, we're good to go.
+            if (!leases.empty()) {
+                return (leases);
             }
+
+            // If not, we'll need to continue and will eventually fall into case 4:
+            // getting a regular lease. That could happen when we're processing
+            // request from client X, there's a reserved address A for X, but
+            // A is currently used by client Y. We can't immediately reassign A
+            // from X to Y, because Y keeps using it, so X would send Decline right
+            // away. Need to wait till Y renews, then we can release A, so it
+            // will become available for X.
         }
 
-        // Hint is in the pool but is not available. 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 number of tries
+        // Case 2: There are existing leases and there are no reservations.
         //
-        // @todo: Current code does not handle pool exhaustion well. It will be
-        // improved. Current problems:
-        // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
-        // 10 addresses), we will iterate over it 100 times before giving up
-        // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
-        // 3. the whole concept of infinite attempts is just asking for infinite loop
-        // We may consider some form or reference counting (this pool has X addresses
-        // left), but this has one major problem. We exactly control allocation
-        // moment, but we currently do not control expiration time at all
+        // 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.
+        if (!leases.empty() && !ctx.host_) {
 
-        unsigned int i = attempts_;
-        do {
-            IOAddress candidate = allocator->pickAddress(ctx.subnet_, ctx.duid_, hint);
-
-            /// @todo: check if the address is reserved once we have host support
-            /// implemented
-
-            // The first step is to find out prefix length. It is 128 for
-            // non-PD leases.
-            uint8_t prefix_len = 128;
-            if (ctx.type_ == Lease::TYPE_PD) {
-                Pool6Ptr pool = boost::dynamic_pointer_cast<Pool6>(
-                    ctx.subnet_->getPool(ctx.type_, candidate, false));
-                prefix_len = pool->getLength();
+            // Check if the existing leases are reserved for someone else.
+            // If they're not, we're ok to keep using them.
+            removeNonmatchingReservedLeases6(ctx, leases);
+
+            if (!leases.empty()) {
+            // Return old leases so the server can see what has changed.
+                return (updateFqdnData(leases, ctx.fwd_dns_update_,
+                                       ctx.rev_dns_update_,
+                                       ctx.hostname_, ctx.fake_allocation_));
             }
 
-            Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(ctx.type_,
-                                 candidate);
-            if (!existing) {
+            // If leases is empty at this stage, it means that we used to have
+            // leases for this client, but we checked and those leases are reserved
+            // for someone else, so we lost them. We will need to continue and
+            // will finally end up in case 4 (no leases, no reservations), so we'll
+            // assign something new.
+        }
 
-                // there's no existing lease for selected candidate, so it is
-                // free. Let's allocate it.
+        // Case 3: There are leases and there are reservations.
+        if (!leases.empty() && ctx.host_) {
 
-                Lease6Ptr lease = createLease6(ctx, candidate, prefix_len);
-                if (lease) {
-                    // We are allocating a new lease (not renewing). So, the
-                    // old lease should be NULL.
-                    ctx.old_leases_.clear();
+            // First, check if have leases matching reservations, and add new
+            // leases if we don't have them.
+            allocateReservedLeases6(ctx, leases);
 
-                    Lease6Collection collection;
-                    collection.push_back(lease);
-                    return (collection);
-                }
+            // leases now contain both existing and new leases that were created
+            // from reservations.
 
-                // 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()) {
-                    // Copy an existing, expired lease so as it can be returned
-                    // to the caller.
-                    Lease6Ptr old_lease(new Lease6(*existing));
-                    ctx.old_leases_.push_back(old_lease);
-
-                    existing = reuseExpiredLease(existing,
-                                                 ctx,
-                                                 prefix_len);
-                    Lease6Collection collection;
-                    collection.push_back(existing);
-                    return (collection);
-                }
+            // Second, let's remove leases that are reserved for someone else.
+            // This applies to any existing leases.
+            removeNonmatchingReservedLeases6(ctx, leases);
+
+            // leases now contain existing and new leases, but we removed those
+            // leases that are reserved for someone else (non-matching reserved).
+
+            // There's one more check to do. Let's remove leases that are not
+            // matching reservations, i.e. if client X has address A, but there's
+            // a reservation for address B, we should release A and reassign B.
+            // Caveat: do this only if we have at least one reserved address.
+            removeNonreservedLeases6(ctx, leases);
+
+            // All checks are done. Let's hope we do have any leases left.
+
+            // If we have any leases left, let's return them and we're done.
+            if (!leases.empty()) {
+                return (leases);
             }
 
-            // Continue trying allocation until we run out of attempts
-            // (or attempts are set to 0, which means infinite)
-            --i;
-        } while ((i > 0) || !attempts_);
+            // If we don't have any leases at this stage, it means that we hit
+            // one of the following cases:
+            // - we have a reservation, but it's not for this IAID/ia-type and
+            //   we had to return the address we were using
+            // - we have a reservation for this iaid/ia-type, but the reserved
+            //   address is currently used by someone else. We can't assign it
+            //   yet.
+            // - we had an address, but we just discovered that it's reserved for
+            //   someone else, so we released it.
+        }
+
+        // Case 4: One of the following is true:
+        // - we don't have leases and there are no reservations
+        // - we used to have leases, but we lost them, because they are now
+        //   reserved for someone else
+        // - we have a reservation, but it is not usable yet, because the address
+        //   is still used by someone else
+        //
+        // In any case, we need to go through normal lease assignment process
+        // for now.
+
+        leases = allocateUnreservedLeases6(ctx);
+
+        return (leases);
 
         // Unable to allocate an address, return an empty lease.
         LOG_WARN(dhcpsrv_logger, DHCPSRV_ADDRESS6_ALLOC_FAIL).arg(attempts_);
@@ -472,6 +449,171 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
     return (Lease6Collection());
 }
 
+Lease6Collection
+AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
+
+    AllocatorPtr allocator = getAllocator(ctx.type_);
+
+    if (!allocator) {
+        isc_throw(InvalidOperation, "No allocator specified for "
+                  << Lease6::typeToText(ctx.type_));
+    }
+
+    Lease6Collection leases;
+
+    IOAddress hint("::");
+    if (!ctx.hints_.empty()) {
+        /// @todo: We support only one hint for now
+        hint = ctx.hints_[0];
+    }
+
+    // check if the hint is in pool and is available
+    // This is equivalent of subnet->inPool(hint), but returns the pool
+    Pool6Ptr pool = boost::dynamic_pointer_cast<
+        Pool6>(ctx.subnet_->getPool(ctx.type_, hint, false));
+
+    if (pool) {
+        /// @todo: We support only one hint for now
+        Lease6Ptr lease = LeaseMgrFactory::instance().getLease6(ctx.type_, hint);
+        if (!lease) {
+            /// @todo: check if the hint is reserved once we have host
+            /// support implemented
+
+            // The hint is valid and not currently used, let's create a
+            // lease for it
+            lease = createLease6(ctx, hint, pool->getLength());
+
+            // It can happen that the lease allocation failed (we could
+            // have lost the race condition. That means that the hint is
+            // lo longer usable and we need to continue the regular
+            // allocation path.
+            if (lease) {
+
+                /// @todo: We support only one lease per ia for now
+                Lease6Collection collection;
+                collection.push_back(lease);
+                return (collection);
+            }
+        } else {
+            if (lease->expired()) {
+                // Copy an existing, expired lease so as it can be returned
+                // to the caller.
+                Lease6Ptr old_lease(new Lease6(*lease));
+                ctx.old_leases_.push_back(old_lease);
+
+                /// We found a lease and it is expired, so we can reuse it
+                lease = reuseExpiredLease(lease, ctx, pool->getLength());
+
+                /// @todo: We support only one lease per ia for now
+                leases.push_back(lease);
+                return (leases);
+            }
+
+        }
+    }
+
+    // Hint is in the pool but is not available. 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 number of tries
+    //
+    // @todo: Current code does not handle pool exhaustion well. It will be
+    // improved. Current problems:
+    // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
+    // 10 addresses), we will iterate over it 100 times before giving up
+    // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
+    // 3. the whole concept of infinite attempts is just asking for infinite loop
+    // We may consider some form or reference counting (this pool has X addresses
+    // left), but this has one major problem. We exactly control allocation
+    // moment, but we currently do not control expiration time at all
+
+    unsigned int i = attempts_;
+    do {
+        IOAddress candidate = allocator->pickAddress(ctx.subnet_, ctx.duid_, hint);
+
+        /// @todo: check if the address is reserved once we have host support
+        /// implemented
+
+        // The first step is to find out prefix length. It is 128 for
+        // non-PD leases.
+        uint8_t prefix_len = 128;
+        if (ctx.type_ == Lease::TYPE_PD) {
+            Pool6Ptr pool = boost::dynamic_pointer_cast<Pool6>(
+                ctx.subnet_->getPool(ctx.type_, candidate, false));
+            prefix_len = pool->getLength();
+        }
+
+        Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(ctx.type_,
+                                                                   candidate);
+        if (!existing) {
+
+            // there's no existing lease for selected candidate, so it is
+            // free. Let's allocate it.
+
+            Lease6Ptr lease = createLease6(ctx, candidate, prefix_len);
+            if (lease) {
+                // We are allocating a new lease (not renewing). So, the
+                // old lease should be NULL.
+                ctx.old_leases_.clear();
+
+                Lease6Collection collection;
+                collection.push_back(lease);
+                return (collection);
+            }
+
+            // 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()) {
+                // Copy an existing, expired lease so as it can be returned
+                // to the caller.
+                Lease6Ptr old_lease(new Lease6(*existing));
+                ctx.old_leases_.push_back(old_lease);
+
+                existing = reuseExpiredLease(existing,
+                                             ctx,
+                                             prefix_len);
+                Lease6Collection collection;
+                collection.push_back(existing);
+                return (collection);
+            }
+        }
+
+        // Continue trying allocation until we run out of attempts
+        // (or attempts are set to 0, which means infinite)
+        --i;
+    } while ((i > 0) || !attempts_);
+
+    // We failed to allocate anything. Let's return empty collection.
+    return (Lease6Collection());
+}
+
+void
+AllocEngine::allocateReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases) {
+    /// @todo
+    if (!ctx.host_ || existing_leases.empty()) {
+        return;
+    }
+}
+
+void
+AllocEngine::removeNonmatchingReservedLeases6(ClientContext6& ctx,
+                                              Lease6Collection& existing_leases) {
+    /// @todo
+    if (!ctx.host_ || existing_leases.empty()) {
+    }
+}
+
+void
+AllocEngine::removeNonreservedLeases6(ClientContext6& ctx,
+                                      Lease6Collection& existing_leases) {
+    /// @todo
+    if (!ctx.host_ || existing_leases.empty()) {
+    }
+}
+
 Lease4Ptr
 AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid,
                             const HWAddrPtr& hwaddr, const IOAddress& hint,

src/lib/dhcpsrv/alloc_engine.h

@@ -372,7 +372,8 @@ protected:
         /// update existing lease.
         bool fake_allocation_;
 
-        /// @brief A pointer to any old leases that the client had before update.
+        /// @brief A pointer to any old leases that the client had before update
+        ///        but are no longer valid after the update/allocation.
         ///
         /// This collection is typically empty, except cases when we are doing
         /// address reassignment, e.g. because there is a host reservation that
@@ -701,6 +702,58 @@ private:
                            const isc::asiolink::IOAddress& addr,
                            const uint8_t prefix_len);
 
+    /// @brief Allocates a normal, in-pool, unreserved lease from the pool.
+    ///
+    /// It attempts to pick a hint first, then uses allocator iteratively until
+    /// an available (not used, not reserved) lease is found. In principle, it
+    /// may return more than one lease, but we currently handle only one.
+    /// This may change in the future.
+    ///
+    /// @param ctx client context that contains all details (subnet, client-id, etc.)
+    /// @return collection of newly allocated leases
+    Lease6Collection allocateUnreservedLeases6(ClientContext6& ctx);
+
+    /// @brief Creates new leases based on reservations.
+    ///
+    /// This method allocates new leases, based on host reservation. Existing
+    /// leases are specified in existing_leases parameter. A new lease is not created,
+    /// if there is a lease for specified address on existing_leases list or there is
+    /// a lease used by someone else.
+    ///
+    /// @param ctx client context that contains all details (subnet, client-id, etc.)
+    /// @param existing_leases leases that are already associated with the client
+    void
+    allocateReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases);
+
+    /// @brief Removes leases that are reserved for someone else.
+    ///
+    /// Goes through the list specified in existing_leases and removes those that
+    /// are reserved by someone else. The removed leases are added to the
+    /// ctx.removed_leases_ collection.
+    ///
+    /// @param ctx client context that contains all details (subnet, client-id, etc.)
+    /// @param existing_leases [in/out] leases that should be checked
+    void
+    removeNonmatchingReservedLeases6(ClientContext6& ctx,
+                                     Lease6Collection& existing_leases);
+
+    /// @brief Removed leases that are not reserved for this client
+    ///
+    /// This method iterates over existing_leases and will remove leases that are
+    /// not reserved for this client. It will leave at least one lease on the list,
+    /// if possible. The reason to run this method is that if there is a reservation
+    /// for address A for client X and client X already has a lease for a
+    /// diffierent address B, we should assign A and release B. However,
+    /// if for some reason we can't assign A, keeping B would be better than
+    /// not having a lease at all. Hence we may keep B if that's the only lease
+    /// left.
+    ///
+    /// @param ctx client context that contains all details (subnet, client-id, etc.)
+    /// @param existing_leases [in/out] leases that should be checked
+    void
+    removeNonreservedLeases6(ClientContext6& ctx,
+                             Lease6Collection& existing_leases);
+
     /// @brief Reuses expired DHCPv4 lease.
     ///
     /// Makes new allocation using an expired lease. The lease is updated with

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -533,6 +533,7 @@ TEST_F(AllocEngine6Test, allocWithValidHint6) {
 
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::15"),
                                        false);
+    ASSERT_TRUE(lease);
 
     // We should get what we asked for
     EXPECT_EQ("2001:db8:1::15", lease->addr_.toText());