[#1418] Checkpoint: did v6 core code

src/lib/dhcpsrv/alloc_engine.cc

@@ -484,7 +484,7 @@ AllocEngine::ClientContext6::ClientContext6(const Subnet6Ptr& subnet,
 
 AllocEngine::ClientContext6::IAContext::IAContext()
     : iaid_(0), type_(Lease::TYPE_NA), hints_(), old_leases_(),
-      changed_leases_(), ia_rsp_() {
+      changed_leases_(), new_resources_(), ia_rsp_() {
 }
 
 void
@@ -516,6 +516,21 @@ IAContext::addHint(const Option6IAPrefixPtr& iaprefix) {
             iaprefix->getPreferred(), iaprefix->getValid());
 }
 
+void
+AllocEngine::ClientContext6::
+IAContext::addNewResource(const asiolink::IOAddress& prefix,
+                          const uint8_t prefix_len) {
+    static_cast<void>(new_resources_.insert(Resource(prefix, prefix_len)));
+}
+
+bool
+AllocEngine::ClientContext6::
+IAContext::isNewResource(const asiolink::IOAddress& prefix,
+                         const uint8_t prefix_len) const {
+    return (static_cast<bool>(new_resources_.count(Resource(prefix,
+                                                            prefix_len))));
+}
+
 void
 AllocEngine::ClientContext6::
 addAllocatedResource(const asiolink::IOAddress& prefix,
@@ -749,6 +764,8 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
             // someone else.
             allocateReservedLeases6(ctx, leases);
 
+            leases = updateLeaseData(ctx, 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
@@ -1233,12 +1250,7 @@ AllocEngine::allocateReservedLeases6(ClientContext6& ctx,
                     }
                 }
 
-                // If this is a real allocation, we may need to extend the lease
+                // Got a lease for a reservation in this IA.
-                // lifetime.
-                if (!ctx.fake_allocation_) {
-                    lease->cltt_ = time(NULL);
-                    LeaseMgrFactory::instance().updateLease6(lease);
-                }
                 return;
             }
         }
@@ -1390,14 +1402,8 @@ AllocEngine::allocateGlobalReservedLeases6(ClientContext6& ctx,
                                                 static_cast<bool>(fqdn));
             }
 
-            // If this is a real allocation, we may need to extend the lease
+            // Got a lease for a reservation in this IA.
-            // lifetime.
+            return(true);
-            if (!ctx.fake_allocation_) {
-                lease->cltt_ = time(NULL);
-                LeaseMgrFactory::instance().updateLease6(lease);
-            }
-
-            return;
         }
     }
 
@@ -1816,7 +1822,7 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
 
     if (!ctx.fake_allocation_) {
         // Add(update) the extended information on the lease.
-        updateLease6ExtendedInfo(expired, ctx);
+        static_cast<void>(updateLease6ExtendedInfo(expired, ctx));
 
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease6(expired);
@@ -1925,7 +1931,7 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
 
     if (!ctx.fake_allocation_) {
         // Add(update) the extended information on the lease.
-        updateLease6ExtendedInfo(lease, ctx);
+        static_cast<void>(updateLease6ExtendedInfo(lease, ctx));
 
         // That is a real (REQUEST) allocation
         bool status = LeaseMgrFactory::instance().addLease(lease);
@@ -1951,6 +1957,9 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
                                               static_cast<int64_t>(1));
             }
 
+            // Record it so it won't be updated twice.
+            ctx.currentIA().addNewResource(addr, prefix_len);
+
             return (lease);
         } else {
             // One of many failures with LeaseMgr (e.g. lost connection to the
@@ -2034,6 +2043,11 @@ AllocEngine::renewLeases6(ClientContext6& ctx) {
 
         // Extend all existing leases that passed all checks.
         for (Lease6Collection::iterator l = leases.begin(); l != leases.end(); ++l) {
+            if (ctx.currentIA().isNewResource((*l)->addr_,
+                                              (*l)->prefixlen_)) {
+                // This lease was just created so is already extended.
+                continue;
+            }
             LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE_DETAIL,
                       ALLOC_ENGINE_V6_EXTEND_LEASE)
                 .arg(ctx.query_->getLabel())
@@ -2123,6 +2137,8 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
     // Keep the old data in case the callout tells us to skip update.
     Lease6Ptr old_data(new Lease6(*lease));
 
+    bool changed = false;
+    lease->remaining_preferred_lft_ = lease->preferred_lft_;
     if (!ctx.currentIA().hints_.empty() &&
         ctx.currentIA().hints_[0].getPreferred()) {
         uint32_t preferred = ctx.currentIA().hints_[0].getPreferred();
@@ -2130,6 +2146,9 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
     } else {
         lease->preferred_lft_ = ctx.subnet_->getPreferred();
     }
+    if (lease->preferred_lft_ < lease->remaining_preferred_lft_) {
+        changed = true;
+    }
     if (!ctx.currentIA().hints_.empty() &&
         ctx.currentIA().hints_[0].getValid()) {
         uint32_t valid = ctx.currentIA().hints_[0].getValid();
@@ -2137,13 +2156,29 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
     } else {
         lease->valid_lft_ = ctx.subnet_->getValid();
     }
-    lease->cltt_ = time(NULL);
+    if (lease->valid_lft_ < lease->current_valid_lft_) {
-    lease->hostname_ = ctx.hostname_;
+        changed = true;
-    lease->fqdn_fwd_ = ctx.fwd_dns_update_;
+    }
-    lease->fqdn_rev_ = ctx.rev_dns_update_;
-    lease->hwaddr_ = ctx.hwaddr_;
-    lease->state_ = Lease::STATE_DEFAULT;
 
+    lease->cltt_ = time(NULL);
+    if ((lease->fqdn_fwd_ != ctx.fwd_dns_update_) ||
+        (lease->fqdn_rev_ != ctx.rev_dns_update_) ||
+        (lease->hostname_ != ctx.hostname_)) {
+        changed = true;
+        lease->hostname_ = ctx.hostname_;
+        lease->fqdn_fwd_ = ctx.fwd_dns_update_;
+        lease->fqdn_rev_ = ctx.rev_dns_update_;
+    }
+    if ((!ctx.hwaddr_ && lease->hwaddr_) ||
+        (ctx.hwaddr_ &&
+         (!lease->hwaddr_ || (*ctx.hwaddr_ != *lease->hwaddr_)))) {
+        changed = true;
+        lease->hwaddr_ = ctx.hwaddr_;
+    }
+    if (lease->state_ != Lease::STATE_DEFAULT) {
+        changed = true;
+        lease->state_ = Lease::STATE_DEFAULT;
+    }
     LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE_DETAIL_DATA,
               ALLOC_ENGINE_V6_EXTEND_NEW_LEASE_DATA)
         .arg(ctx.query_->getLabel())
@@ -2207,14 +2242,26 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
             if (ctx.subnet_->inPool(ctx.currentIA().type_, old_data->addr_)) {
                 update_stats = true;
             }
+            changed = true;
         }
 
         // @todo should we call storeLease6ExtendedInfo() here ?
-        updateLease6ExtendedInfo(lease, ctx);
+        if (updateLease6ExtendedInfo(lease, ctx)) {
+            changed = true;
+        }
+
+        // Try to reuse the lease.
+        lease->remaining_valid_lft_ = 0;
+        if (!changed) {
+            setLeaseRemainingLife(lease, ctx);
+        }
+
 
         // Now that the lease has been reclaimed, we can go ahead and update it
         // in the lease database.
-        LeaseMgrFactory::instance().updateLease6(lease);
+        if (lease->remaining_valid_lft_ == 0) {
+            LeaseMgrFactory::instance().updateLease6(lease);
+        }
 
         if (update_stats) {
             StatsMgr::instance().addValue(
@@ -2253,6 +2300,12 @@ AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases
     for (Lease6Collection::const_iterator lease_it = leases.begin();
          lease_it != leases.end(); ++lease_it) {
         Lease6Ptr lease(new Lease6(**lease_it));
+        if (ctx.currentIA().isNewResource(lease->addr_, lease->prefixlen_)) {
+            // This lease was just created so is already up to date.
+            updated_leases.push_back(lease);
+            continue;
+        }
+
         lease->fqdn_fwd_ = ctx.fwd_dns_update_;
         lease->fqdn_rev_ = ctx.rev_dns_update_;
         lease->hostname_ = ctx.hostname_;
@@ -2271,9 +2324,19 @@ AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases
                 }
             }
 
+            bool fqdn_changed = ((lease->type_ != Lease::TYPE_PD) &&
+                                 !(lease->hasIdenticalFqdn(**lease_it)));
+
             lease->cltt_ = time(NULL);
-            ctx.currentIA().changed_leases_.push_back(*lease_it);
+            lease->remaining_valid_lft_ = 0;
-            LeaseMgrFactory::instance().updateLease6(lease);
+            if (!fqdn_changed) {
+                lease->remaining_preferred_lft_ = lease->preferred_lft_;
+                setLeaseRemainingLife(lease, ctx);
+            }
+            if (lease->remaining_valid_lft_ == 0) {
+                ctx.currentIA().changed_leases_.push_back(*lease_it);
+                LeaseMgrFactory::instance().updateLease6(lease);
+            }
 
             if (update_stats) {
                 StatsMgr::instance().addValue(
@@ -3870,6 +3933,7 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
 
     // Update the lease with the information from the context.
     // If there was no significant changes, try reuse.
+    lease->remaining_valid_lft_ = 0;
     if (!updateLease4Information(lease, ctx)) {
         setLeaseRemainingLife(lease, ctx);
     }
@@ -4235,8 +4299,8 @@ AllocEngine::updateLease4Information(const Lease4Ptr& lease,
     if ((!ctx.hwaddr_ && lease->hwaddr_) ||
         (ctx.hwaddr_ &&
          (!lease->hwaddr_ || (*ctx.hwaddr_ != *lease->hwaddr_)))) {
-            changed = true;
+        changed = true;
-            lease->hwaddr_ = ctx.hwaddr_;
+        lease->hwaddr_ = ctx.hwaddr_;
     }
     if (ctx.subnet_->getMatchClientId() && ctx.clientid_) {
         if (!lease->client_id_ || (*ctx.clientid_ != *lease->client_id_)) {

src/lib/dhcpsrv/alloc_engine.h

@@ -558,6 +558,11 @@ public:
             /// FQDN has changed.
             Lease6Collection changed_leases_;
 
+            /// @brief Holds addresses and prefixes allocated for this IA.
+            ///
+            /// This collection is used to update at most once new leases.
+            ResourceContainer new_resources_;
+
             /// @brief A pointer to the IA_NA/IA_PD option to be sent in
             /// response
             Option6IAPtr ia_rsp_;
@@ -591,6 +596,20 @@ public:
             ///
             /// @throw BadValue if iaprefix is null.
             void addHint(const Option6IAPrefixPtr& iaprefix);
+
+            /// @brief Convenience method adding new prefix or address.
+            ///
+            /// @param prefix Prefix or address
+            /// @param prefix_len Prefix length. Default is 128 for addresses.
+            void addNewResource(const asiolink::IOAddress& prefix,
+                                const uint8_t prefix_len = 128);
+
+            /// @brief Checks if specified address or prefix was new.
+            ///
+            /// @param prefix Prefix or address
+            /// @param prefix_len Prefix length. Default is 128 for addresses.
+            bool isNewResource(const asiolink::IOAddress& prefix,
+                               const uint8_t prefix_len = 128) const;
         };
 
         /// @brief Container holding IA specific contexts.

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc

@@ -3606,14 +3606,8 @@ TEST_F(AllocEngine6Test, globalHostReservedAddress) {
     EXPECT_EQ("3001::1", lease->addr_.toText());
 
     // We're going to rollback the clock a little so we can verify a renewal.
-    // We verify the "time" change in case the lease returned to us
-    // by expectOneLease ever becomes a copy and not what is in the lease mgr.
     --lease->cltt_;
-    lease->updateCurrentExpirationTime();
+    EXPECT_NO_THROW(LeaseMgrFactory::instance().updateLease6(lease));
-    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
-                                                               lease->addr_);
-    ASSERT_TRUE(from_mgr);
-    EXPECT_EQ(from_mgr->cltt_, lease->cltt_);
 
     // This is what the client will send in his renew message.
     AllocEngine::HintContainer hints;
@@ -3671,14 +3665,8 @@ TEST_F(AllocEngine6Test, globalHostReservedPrefix) {
     EXPECT_EQ("3001::", lease->addr_.toText());
 
     // We're going to rollback the clock a little so we can verify a renewal.
-    // We verify the "time" change in case the lease returned to us
-    // by expectOneLease ever becomes a copy and not what is in the lease mgr.
     --lease->cltt_;
-    lease->updateCurrentExpirationTime();
+    EXPECT_NO_THROW(LeaseMgrFactory::instance().updateLease6(lease));
-    Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->type_,
-                                                               lease->addr_);
-    ASSERT_TRUE(from_mgr);
-    EXPECT_EQ(from_mgr->cltt_, lease->cltt_);
 
     // This is what the client will send in his renew message.
     AllocEngine::HintContainer hints;