[#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
This commit is contained in:
Thomas Markwalder
2025-12-17 10:59:21 -05:00
parent 9b945ae17c
commit cb61b66bd6
2 changed files with 30 additions and 4 deletions

View File

@@ -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;
}

View File

@@ -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) {