From: Thomas Markwalder Date: Thu, 25 Apr 2019 18:19:26 +0000 (-0400) Subject: [#365,!296] Relaxed sanity checks X-Git-Tag: Kea-1.6.0-beta~186 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=590446377e01642c8bc12fa8e046ccf2bb4c01cd;p=thirdparty%2Fkea.git [#365,!296] Relaxed sanity checks T2 may now be specified as any value >= 0, T1 must be less than T2. This allows both values to exceed the valid life time. --- diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index fcbe205e99..96aedbb301 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -2068,11 +2068,12 @@ should include options from the new option space: - 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. + You may specify any value for T2 greater than or equal to zero. When specifying + T1 it must be less than T2. This flexibility is allowed to support a use case + where admins want to suppress client renewals and rebinds by deferring them beyond + the life span of the lease. This should cause the lease to expire, rather than + get renewed by clients. If T1 is specified as larger than T2, it will be set to + zero in the outbound IA. Calculating the values is controlled by the following three parameters. diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index a7cd23ce96..34c7e5a4d1 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -3892,29 +3892,23 @@ void Dhcpv6Srv::discardPackets() { void Dhcpv6Srv::setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6IAPtr& resp) { + // Default T2 time to zero. uint32_t t2_time = 0; - // If T2 is explicitly configured we'll use try value. + + // If T2 is explicitly configured we'll use that value. if (!subnet->getT2().unspecified()) { t2_time = subnet->getT2(); } else if (subnet->getCalculateTeeTimes()) { - // Calculating tee times is enabled, so calculated it. + // Calculating tee times is enabled, so calculate it. t2_time = static_cast(subnet->getT2Percent() * preferred_lft); } - // 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 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 - resp->setT2(0); - } + // We allow T2 to be any value. + resp->setT2(t2_time); + // Default T1 time to zero. uint32_t t1_time = 0; + // If T1 is explicitly configured we'll use try value. if (!subnet->getT1().unspecified()) { t1_time = subnet->getT1(); @@ -3923,10 +3917,8 @@ Dhcpv6Srv::setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6 t1_time = static_cast(subnet->getT1Percent() * preferred_lft); } - // 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) { + // T1 is sane if it is less than or equal to T2. + if (t1_time < t2_time) { 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 3428f3dd58..63e9b4369d 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -688,23 +688,18 @@ protected: /// /// T2: /// - /// The candidate value for T2 defaults to zero. If the rebind-timer value - /// 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 or equal than the preferred lease time use it, - /// otherwise set T2 to zero. + /// The value for T2 defaults to zero. If the rebind-timer value is + /// specified then use it. If not and calculate-tee-times is true, then + /// use the value given by: preferred lease time * t2-percent. /// /// T1: /// /// The candidate value for T1 defaults to zero. If the renew-timer value /// is specified then use it. If not and calculate-tee-times is true, then - /// use the value given by: valid lease time * t1-percent. + /// use the value given by: preferred lease time * t1-percent. /// - /// 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. + /// The T1 candidate will be used provided it less than to T2, + /// otherwise it will be set T1 to zero. /// /// @param preferred_lft preferred lease time of the lease being assigned to the client /// @param subnet the subnet to which the lease belongs diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index 92c4efeebc..9bed7df50b 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -2494,11 +2494,11 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { 400, 800 }, { - "T1 and T2 specified insane", + "preferred < T1 specified < T2 specified", preferred_lft + 1, preferred_lft + 2, calculate_enabled, 0.4, 0.8, - 0, 0 + preferred_lft + 1, preferred_lft + 2 }, { "T1 should be calculated, T2 specified", @@ -2530,14 +2530,16 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { 0, 0 }, { + // cannot set T1 > 0 when T2 is 0 "T1 specified, T2 unspecified (no calculation)", preferred_lft - 1, unspecified, !calculate_enabled, 0.4, 0.8, - preferred_lft - 1, 0 + //preferred_lft - 1, 0 + 0, 0 }, { - "both T1 and T2 specified (no calculation)", + "both T1 and T2 specified sane (no calculation)", preferred_lft - 2, preferred_lft - 1, !calculate_enabled, 0.4, 0.8, @@ -2548,7 +2550,8 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) { preferred_lft, preferred_lft, !calculate_enabled, 0.4, 0.8, - preferred_lft, preferred_lft + // T1 must be less than T2 + 0, preferred_lft }, { "T1 specified insane (> lease T2), T2 specified (no calculation)",