[#4176] Further comments

modified:   src/lib/dhcpsrv/lease_mgr.cc
    LeaseMgr::updateStatsOnUpdate(const Lease4Ptr...)
        Do nothing if either lease state is REGISTERED

modified:   src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.cc
    Verify REGISTERED ignored for v4

diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc
index 5782f3783ac9bbf95321fddcc80d69db02ad5b7f..caf7ffcb0744e51401f28788fba62b54129b05e8 100644 (file)
--- a/src/lib/dhcpsrv/lease_mgr.cc
+++ b/src/lib/dhcpsrv/lease_mgr.cc
@@ -1498,6 +1498,12 @@ constexpr uint16_t REGISTERED_REGISTERED = STATE_MASK(Lease::STATE_REGISTERED,
 void
 LeaseMgr::updateStatsOnUpdate(const Lease4Ptr& existing,
                               const Lease4Ptr& lease) {
+    if (existing->state_ == Lease::STATE_REGISTERED ||
+         lease->state_ == Lease::STATE_REGISTERED) {
+        // Registered is not valid for v4.
+        return;
+    }
+
     if (existing->subnet_id_ == lease->subnet_id_) {
         if (existing->state_ == lease->state_) {
             // Same subnet, same state, nothing to do.
@@ -1629,7 +1635,6 @@ LeaseMgr::updateStatsOnUpdate(const Lease6Ptr& existing,
                               const Lease6Ptr& lease) {
     if (existing->type_ != lease->type_) {
         // Something is fishy, mismatched types.
-        /// @todo maybe we should throw?
         return;
     }
 
@@ -1639,7 +1644,6 @@ LeaseMgr::updateStatsOnUpdate(const Lease6Ptr& existing,
          existing->state_ == Lease::STATE_REGISTERED ||
          lease->state_ == Lease::STATE_REGISTERED)) {
         // Something is fishy. Invalid states for PDs.
-        /// @todo maybe we should throw?
         return;
     }
 

diff --git a/src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.cc
index ce57c6407174086871eb47658d92128c9c190693..9344c7a25c0ffc40ba02d53ba1cc14cd5e496683 100644 (file)
--- a/src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.cc
@@ -5111,24 +5111,35 @@ GenericLeaseMgrTest::testUpdateStatsOn4SameSubnet() {
     { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_DECLINED, 1, 0 },
     { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_EXPIRED_RECLAIMED, 2, 1 },
     { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_RELEASED, 2, 1 },
+    { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_REGISTERED, 1, 1 },
 
     // New state DECLINED
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_DEFAULT, 1, 2 },
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_DECLINED, 1, 1 },
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_EXPIRED_RECLAIMED, 2, 2 },
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_RELEASED, 2, 2 },
+    { __LINE__, Lease::STATE_DECLINED, Lease::STATE_REGISTERED, 1, 1 },
 
     // New state EXPIRED_RECLAIMED
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_DEFAULT, 0, 1 },
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_DECLINED, 0, 0 },
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_EXPIRED_RECLAIMED, 1, 1 },
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_RELEASED, 1, 1 },
+    { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_REGISTERED, 1, 1 },
 
     // New state RELEASED
     { __LINE__, Lease::STATE_RELEASED, Lease::STATE_DEFAULT, 0, 1 },
     { __LINE__, Lease::STATE_RELEASED, Lease::STATE_DECLINED, 0, 0 },
     { __LINE__, Lease::STATE_RELEASED, Lease::STATE_EXPIRED_RECLAIMED, 1, 1 },
-    { __LINE__, Lease::STATE_RELEASED, Lease::STATE_RELEASED, 1, 1 }
+    { __LINE__, Lease::STATE_RELEASED, Lease::STATE_RELEASED, 1, 1 },
+    { __LINE__, Lease::STATE_RELEASED, Lease::STATE_REGISTERED, 1, 1 },
+
+    // New state REGISTERED - not valid for v4, no stats should change
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_DEFAULT, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_DECLINED, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_EXPIRED_RECLAIMED, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_RELEASED, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_REGISTERED, 1, 1 }
     };
 
     for (auto scenario : scenarios) {
@@ -5220,24 +5231,35 @@ GenericLeaseMgrTest::testUpdateStatsOn4DifferentSubnet() {
     { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_DECLINED, 0, 0, 2, 1 },
     { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_EXPIRED_RECLAIMED, 1, 1, 2, 1 },
     { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_RELEASED, 1, 1, 2, 1 },
+    { __LINE__, Lease::STATE_DEFAULT, Lease::STATE_REGISTERED, 1, 1, 1, 1 },
 
     // New state DECLINED
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_DEFAULT, 0, 1, 2, 2 },
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_DECLINED, 0, 0, 2, 2 },
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_EXPIRED_RECLAIMED, 1, 1, 2, 2 },
     { __LINE__, Lease::STATE_DECLINED, Lease::STATE_RELEASED, 1, 1, 2, 2 },
+    { __LINE__, Lease::STATE_DECLINED, Lease::STATE_REGISTERED, 1, 1, 1, 1 },
 
     // New state EXPIRED_RECLAIMED
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_DEFAULT, 0, 1, 1, 1 },
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_DECLINED, 0, 0, 1, 1 },
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_EXPIRED_RECLAIMED, 1, 1, 1, 1 },
     { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_RELEASED, 1, 1, 1, 1 },
+    { __LINE__, Lease::STATE_EXPIRED_RECLAIMED, Lease::STATE_REGISTERED, 1, 1, 1, 1 },
 
     // New state RELEASED
     { __LINE__, Lease::STATE_RELEASED, Lease::STATE_DEFAULT, 0, 1, 1, 1 },
     { __LINE__, Lease::STATE_RELEASED, Lease::STATE_DECLINED, 0, 0, 1, 1 },
     { __LINE__, Lease::STATE_RELEASED, Lease::STATE_EXPIRED_RECLAIMED, 1, 1, 1, 1 },
-    { __LINE__, Lease::STATE_RELEASED, Lease::STATE_RELEASED, 1, 1, 1, 1 }
+    { __LINE__, Lease::STATE_RELEASED, Lease::STATE_RELEASED, 1, 1, 1, 1 },
+    { __LINE__, Lease::STATE_RELEASED, Lease::STATE_REGISTERED, 1, 1, 1, 1 },
+
+    // New state REGISTERED - not valid for v4, no stats should change
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_DEFAULT, 1, 1, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_DECLINED, 1, 1, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_EXPIRED_RECLAIMED, 1, 1, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_RELEASED, 1, 1, 1, 1 },
+    { __LINE__, Lease::STATE_REGISTERED, Lease::STATE_REGISTERED, 1, 1, 1, 1 }
     };
 
     for (auto scenario : scenarios) {