From: Thomas Markwalder Date: Wed, 24 Apr 2019 18:41:25 +0000 (-0400) Subject: [#365,!296] Changed to use preferred lease time X-Git-Tag: Kea-1.6.0-beta~187 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=239bbbb295c54ee57a17bc6cd0d5b1932f93452e;p=thirdparty%2Fkea.git [#365,!296] Changed to use preferred lease time doc/guide/dhcp6-srv.xml Updated to reflect the use to preferred life time vs valid life time src/bin/dhcp6/dhcp6_srv.* Updated to use preferred lease time for tee time calculation Dhcpv6Srv::setTeeTimes() - updated to allow values to less than or equal, rathe than the just less than src/bin/dhcp6/tests/dhcp6_srv_unittest.cc src/bin/dhcp6/tests/hooks_unittest.cc src/bin/dhcp6/tests/tee_times_unittest.cc updated unit tests accordingly --- diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index ad191e6698..fcbe205e99 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -2051,7 +2051,7 @@ should include options from the new option space: servers should send values for T1 and T2 that are 50% and 80% of the preferred lease time, repsectively. Kea can be configured to send values that are specified explicitly or that are calculated as - percentages of the valid lease time. The server's behavior is governed + percentages of the preferred lease time. The server's behavior is governed by combination of configuration parameters, two of which have already been mentioned. @@ -2068,9 +2068,11 @@ should include options from the new option space: - The server will only use T2 if it is less than valid lease time, otherwise it will - be set to 0. T1 will only be use if it is less than T2, otherwise it will be - set to 0. + The server will only use T2 if it is less than or equal preferred lease time, + otherwise it will be set to 0. If T2 is not zero then T1 must be less than or + equalt to T2, otherwise T1 will be set to zero. If T2 is zero, then T1 must + be less than or equal to the preferred lease time, otherwise T1 will be set + to zero. Calculating the values is controlled by the following three parameters. @@ -2085,14 +2087,14 @@ should include options from the new option space: t1-percent - the percentage of the valid lease time to use for T1. It is expressed as a real number between 0.0 and 1.0 and must - be less than t2-percent. The default value is 0.5 per RFC 3315. + be less than t2-percent. The default value is 0.5 per RFC 8415. t2-percent - the percentage of the valid lease time to use for T2. It is expressed as a real number between 0.0 and 1.0 and must - be greater than t1-percent. The default value is 0.8 per RFC 3315. + be greater than t1-percent. The default value is 0.8 per RFC 8415. diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 5014868081..a7cd23ce96 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1833,7 +1833,7 @@ Dhcpv6Srv::assignIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer, .arg(lease->toText()); // Set the values for T1 and T2. - setTeeTimes(lease->valid_lft_, subnet, ia_rsp); + setTeeTimes(lease->preferred_lft_, subnet, ia_rsp); Option6IAAddrPtr addr(new Option6IAAddr(D6O_IAADDR, lease->addr_, lease->preferred_lft_, @@ -1917,17 +1917,17 @@ Dhcpv6Srv::assignIA_PD(const Pkt6Ptr& query, const Pkt6Ptr& /*answer*/, if (!leases.empty()) { - // Need to retain the shortest valid lease time to use + // Need to retain the shortest preferred lease time to use // for calculating T1 and T2. - uint32_t min_valid_lft = (*leases.begin())->valid_lft_; + uint32_t min_preferred_lft = (*leases.begin())->preferred_lft_; const bool pd_exclude_requested = requestedInORO(query, D6O_PD_EXCLUDE); for (Lease6Collection::iterator l = leases.begin(); l != leases.end(); ++l) { // Check for new minimum lease time - if (((*l)->valid_lft_ > 0) && (min_valid_lft > (*l)->valid_lft_)) { - min_valid_lft = (*l)->valid_lft_; + if (((*l)->preferred_lft_ > 0) && (min_preferred_lft > (*l)->preferred_lft_)) { + min_preferred_lft = (*l)->preferred_lft_; } // We have a lease! Let's wrap its content into IA_PD option @@ -1959,8 +1959,8 @@ Dhcpv6Srv::assignIA_PD(const Pkt6Ptr& query, const Pkt6Ptr& /*answer*/, } } - // Set T1 and T2, using the shortest valid lifetime among the leases. - setTeeTimes(min_valid_lft, subnet, ia_rsp); + // Set T1 and T2, using the shortest preferred lifetime among the leases. + setTeeTimes(min_preferred_lft, subnet, ia_rsp); // It would be possible to insert status code=0(success) as well, // but this is considered waste of bandwidth as absence of status @@ -2068,7 +2068,7 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer, // Retains the shortest valid lease time to use // for calculating T1 and T2. - uint32_t min_valid_lft = std::numeric_limits::max(); + uint32_t min_preferred_lft = std::numeric_limits::max(); // For all leases we have now, add the IAADDR with non-zero lifetimes. for (Lease6Collection::const_iterator l = leases.begin(); l != leases.end(); ++l) { @@ -2077,8 +2077,8 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer, ia_rsp->addOption(iaaddr); // Check for new minimum lease time - if (((*l)->valid_lft_ > 0) && (min_valid_lft > (*l)->valid_lft_)) { - min_valid_lft = (*l)->valid_lft_; + if (((*l)->preferred_lft_ > 0) && (min_preferred_lft > (*l)->preferred_lft_)) { + min_preferred_lft = (*l)->preferred_lft_; } LOG_INFO(lease6_logger, DHCP6_PD_LEASE_RENEW) @@ -2137,7 +2137,7 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer, if (!leases.empty()) { // We allocated leases so we need to update T1 and T2. - setTeeTimes(min_valid_lft, subnet, ia_rsp); + setTeeTimes(min_preferred_lft, subnet, ia_rsp); } else { // The server wasn't able allocate new lease and renew an existing // lease. In that case, the server sends NoAddrsAvail per RFC 8415. @@ -2248,7 +2248,7 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query, // Retains the shortest valid lease time to use // for calculating T1 and T2. - uint32_t min_valid_lft = std::numeric_limits::max(); + uint32_t min_preferred_lft = std::numeric_limits::max(); for (Lease6Collection::const_iterator l = leases.begin(); l != leases.end(); ++l) { @@ -2273,8 +2273,8 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query, } // Check for new minimum lease time - if ((*l)->valid_lft_ < min_valid_lft) { - min_valid_lft = (*l)->valid_lft_; + if (((*l)->preferred_lft_ > 0) && ((*l)->preferred_lft_ < min_preferred_lft)) { + min_preferred_lft = (*l)->preferred_lft_; } LOG_INFO(lease6_logger, DHCP6_PD_LEASE_RENEW) @@ -2324,7 +2324,7 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query, if (!leases.empty()) { // We allocated leases so we need to update T1 and T2. - setTeeTimes(min_valid_lft, subnet, ia_rsp); + setTeeTimes(min_preferred_lft, subnet, ia_rsp); } else { // All is left is to insert the status code. // The server wasn't able allocate new lease and renew an existing @@ -3891,22 +3891,23 @@ void Dhcpv6Srv::discardPackets() { } void -Dhcpv6Srv::setTeeTimes(uint32_t valid_lft, const Subnet6Ptr& subnet, Option6IAPtr& resp) { +Dhcpv6Srv::setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6IAPtr& resp) { uint32_t t2_time = 0; // If T2 is explicitly configured we'll use try value. if (!subnet->getT2().unspecified()) { t2_time = subnet->getT2(); } else if (subnet->getCalculateTeeTimes()) { // Calculating tee times is enabled, so calculated it. - t2_time = static_cast(subnet->getT2Percent() * valid_lft); + t2_time = static_cast(subnet->getT2Percent() * preferred_lft); } - // Send the T2 candidate value only if it's sane: to be sane it must be less than - // the valid life time. - uint32_t timer_ceiling = valid_lft; - if (t2_time > 0 && t2_time < timer_ceiling) { + // The T2 candidate value is sane if it less than or equal to preferred lease time. + // If not, we set it to 0. We allow it to be equal to support the use case that + // clients can be told not to rebind. + uint32_t timer_ceiling = preferred_lft; + if (t2_time > 0 && t2_time <= timer_ceiling) { resp->setT2(t2_time); - // When we send T2, timer ceiling for T1 becomes T2. + // When we use T2, the timer ceiling for T1 becomes T2. timer_ceiling = t2_time; } else { // It's either explicitly 0 or insane, leave it to the client @@ -3919,12 +3920,13 @@ Dhcpv6Srv::setTeeTimes(uint32_t valid_lft, const Subnet6Ptr& subnet, Option6IAPt t1_time = subnet->getT1(); } else if (subnet->getCalculateTeeTimes()) { // Calculating tee times is enabled, so calculate it. - t1_time = static_cast(subnet->getT1Percent() * valid_lft); + t1_time = static_cast(subnet->getT1Percent() * preferred_lft); } - // Send T1 if it's sane: If we sent T2, T1 must be less than that. If not it must be - // less than the valid life time. - if (t1_time > 0 && t1_time < timer_ceiling) { + // T1 is sane if it is less than or equal to T2 if T2 is > 0, otherwise + // it must be less than of equal to preferred lease time. We let it + // equal to the ceiling to support the use case of client not renewing. + if (t1_time > 0 && t1_time <= timer_ceiling) { resp->setT1(t1_time); } else { // It's either explicitly 0 or insane, leave it to the client diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index 20bb91a1c5..3428f3dd58 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -692,7 +692,7 @@ protected: /// is specified then use it. If not and calculate-tee-times is true, then /// use the value given by: valid lease time * t2-percent. /// - /// If the T2 candidate is less than the valid lease time use it, + /// If the T2 candidate is less or equal than the preferred lease time use it, /// otherwise set T2 to zero. /// /// T1: @@ -701,14 +701,15 @@ protected: /// is specified then use it. If not and calculate-tee-times is true, then /// use the value given by: valid lease time * t1-percent. /// - /// The T1 candidate will be used provided it less than the T2 when T2 is - /// is greater than zero. When T2 is zero then the T1 candidate must be - /// less than the valid lease time, otherwise T1 will be set to zero. + /// The T1 candidate will be used provided it less than or equal to T2 + /// when T2 is greater than zero, otherwise it must be less than or equal to + /// the preferred lease time. If the candiate value cannot be used the we + /// set T1 to zero. /// - /// @param lease lease being assigned to the client + /// @param preferred_lft preferred lease time of the lease being assigned to the client /// @param subnet the subnet to which the lease belongs /// @param resp outbound IA option in which the timers are set. - void setTeeTimes(uint32_t valid_lft, const Subnet6Ptr& subnet, Option6IAPtr& resp); + void setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6IAPtr& resp); /// @brief Attempts to release received addresses /// diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index c2d80a85ec..92c4efeebc 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -2480,7 +2480,7 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { // Handy constants Triplet unspecified; - Triplet valid_lft(1000); + Triplet preferred_lft(1000); bool calculate_enabled = true; // Test scenarios @@ -2495,17 +2495,17 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { }, { "T1 and T2 specified insane", - valid_lft + 1, valid_lft + 2, + preferred_lft + 1, preferred_lft + 2, calculate_enabled, 0.4, 0.8, 0, 0 }, { "T1 should be calculated, T2 specified", - unspecified, valid_lft - 1, + unspecified, preferred_lft - 1, calculate_enabled, 0.4, 0.8, - 400, valid_lft - 1 + 400, preferred_lft - 1 }, { "T1 specified, T2 should be calculated", @@ -2516,7 +2516,7 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { }, { "T1 specified insane (> T2), T2 should be calculated", - valid_lft - 1, unspecified, + preferred_lft - 1, unspecified, calculate_enabled, 0.4, 0.8, 0, 800 @@ -2531,28 +2531,35 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { }, { "T1 specified, T2 unspecified (no calculation)", - valid_lft - 1, unspecified, + preferred_lft - 1, unspecified, !calculate_enabled, 0.4, 0.8, - valid_lft - 1, 0 + preferred_lft - 1, 0 }, { "both T1 and T2 specified (no calculation)", - valid_lft - 2, valid_lft - 1, + preferred_lft - 2, preferred_lft - 1, !calculate_enabled, 0.4, 0.8, - valid_lft - 2, valid_lft - 1 + preferred_lft - 2, preferred_lft - 1 }, { - "T1 specified insane (>= lease T2), T2 specified (no calculation)", - valid_lft - 1, valid_lft - 1, + "both T1 and T2 specified equal to preferred", + preferred_lft, preferred_lft, !calculate_enabled, 0.4, 0.8, - 0, valid_lft - 1 + preferred_lft, preferred_lft }, { - "T1 specified insane (>= lease time), T2 not specified (no calculation)", - valid_lft, unspecified, + "T1 specified insane (> lease T2), T2 specified (no calculation)", + preferred_lft - 1, preferred_lft - 2, + !calculate_enabled, + 0.4, 0.8, + 0, preferred_lft - 2 + }, + { + "T1 specified insane (> lease time), T2 not specified (no calculation)", + preferred_lft + 1, unspecified, !calculate_enabled, 0.4, 0.8, 0, 0 @@ -2560,7 +2567,7 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { }; // Calculation is enabled for all the scenarios. - subnet_->setValid(valid_lft); + subnet_->setPreferred(preferred_lft); // Create a discover packet to use Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234)); diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index 33ec8e4444..3a92dfbfd0 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/src/bin/dhcp6/tests/hooks_unittest.cc @@ -2834,7 +2834,7 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Renew) { ASSERT_TRUE(tmp); // Check that IA_NA was returned and that there's an address included - boost::shared_ptr addr_opt = checkIA_NA(reply, 1000, 602, 803); + boost::shared_ptr addr_opt = checkIA_NA(reply, 1000, 601, 802); ASSERT_TRUE(addr_opt); // Check that the lease is really in the database @@ -3951,7 +3951,7 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Rebind) { // Check that IA_NA was returned and that there's an address included // Note we also verify that T1 and T2 were calculated correctly. - boost::shared_ptr addr_opt = checkIA_NA(reply, 1000, 602, 803); + boost::shared_ptr addr_opt = checkIA_NA(reply, 1000, 601, 802); ASSERT_TRUE(addr_opt); // Check that the lease is really in the database diff --git a/src/bin/dhcp6/tests/tee_times_unittest.cc b/src/bin/dhcp6/tests/tee_times_unittest.cc index e3b042686c..367433d2d6 100644 --- a/src/bin/dhcp6/tests/tee_times_unittest.cc +++ b/src/bin/dhcp6/tests/tee_times_unittest.cc @@ -175,24 +175,24 @@ TEST_F(TeeTest, defaultTimers) { uint32_t actual_t2; ASSERT_TRUE(client.getTeeTimes(na_iaid, actual_t1, actual_t2)); - EXPECT_EQ(2000, actual_t1); - EXPECT_EQ(3200, actual_t2); + EXPECT_EQ(1500, actual_t1); + EXPECT_EQ(2400, actual_t2); ASSERT_TRUE(client.getTeeTimes(pd_iaid, actual_t1, actual_t2)); - EXPECT_EQ(2000, actual_t1); - EXPECT_EQ(3200, actual_t2); + EXPECT_EQ(1500, actual_t1); + EXPECT_EQ(2400, actual_t2); // Let's renew the leases. ASSERT_NO_THROW(client.doRenew()); // Now check the timers again. ASSERT_TRUE(client.getTeeTimes(na_iaid, actual_t1, actual_t2)); - EXPECT_EQ(2000, actual_t1); - EXPECT_EQ(3200, actual_t2); + EXPECT_EQ(1500, actual_t1); + EXPECT_EQ(2400, actual_t2); ASSERT_TRUE(client.getTeeTimes(pd_iaid, actual_t1, actual_t2)); - EXPECT_EQ(2000, actual_t1); - EXPECT_EQ(3200, actual_t2); + EXPECT_EQ(1500, actual_t1); + EXPECT_EQ(2400, actual_t2); } // This test verifies that custom percentags for T1 and T2 @@ -217,24 +217,24 @@ TEST_F(TeeTest, calculateTimers) { uint32_t actual_t2; ASSERT_TRUE(client.getTeeTimes(na_iaid, actual_t1, actual_t2)); - EXPECT_EQ(1800, actual_t1); - EXPECT_EQ(2800, actual_t2); + EXPECT_EQ(1350, actual_t1); + EXPECT_EQ(2100, actual_t2); ASSERT_TRUE(client.getTeeTimes(pd_iaid, actual_t1, actual_t2)); - EXPECT_EQ(1800, actual_t1); - EXPECT_EQ(2800, actual_t2); + EXPECT_EQ(1350, actual_t1); + EXPECT_EQ(2100, actual_t2); // Let's renew the leases. ASSERT_NO_THROW(client.doRenew()); // Now check the timers again. ASSERT_TRUE(client.getTeeTimes(na_iaid, actual_t1, actual_t2)); - EXPECT_EQ(1800, actual_t1); - EXPECT_EQ(2800, actual_t2); + EXPECT_EQ(1350, actual_t1); + EXPECT_EQ(2100, actual_t2); ASSERT_TRUE(client.getTeeTimes(pd_iaid, actual_t1, actual_t2)); - EXPECT_EQ(1800, actual_t1); - EXPECT_EQ(2800, actual_t2); + EXPECT_EQ(1350, actual_t1); + EXPECT_EQ(2100, actual_t2); }