From: Thomas Markwalder Date: Wed, 17 Dec 2025 15:59:21 +0000 (-0500) Subject: [#4176] Further comments X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cb61b66bd6447697d46c62644a67f1291e1989dc;p=thirdparty%2Fkea.git [#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 5782f3783a..caf7ffcb07 100644 --- 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 ce57c64071..9344c7a25c 100644 --- 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) {