[#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 fcbe205e998f75235ec264c324cd3bbf6b75d568..96aedbb3017c94a738a71584c9c33f55742acbb3 100644 (file)
--- a/doc/guide/dhcp6-srv.xml
+++ b/doc/guide/dhcp6-srv.xml
@@ -2068,11 +2068,12 @@ should include options from the new option space:
           </simpara>
         </listitem>
       </itemizedlist>
-      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.
       </para>
       <para>
       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 a7cd23ce967dd1d54e6d6851ffc9ac011a6368eb..34c7e5a4d15c6fb01f97dff86d091ce5bc30a864 100644 (file)
--- 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<uint32_t>(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<uint32_t>(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 3428f3dd58cdbe1098132dce4b2842e2dcc2332a..63e9b4369d4dde044edfeebf7090a2413ceea0b1 100644 (file)
--- 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 92c4efeebccfe71a1e9077da912dd380fbc0e8f0..9bed7df50baabe76e6f796b42f241fdc6ad8a97e 100644 (file)
--- 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)",