]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#4466] Addressed review comments
authorThomas Markwalder <tmark@isc.org>
Mon, 4 May 2026 13:57:52 +0000 (09:57 -0400)
committerThomas Markwalder <tmark@isc.org>
Mon, 4 May 2026 15:32:33 +0000 (15:32 +0000)
modified:   src/hooks/dhcp/mysql/mysql_lb_messages.cc
modified:   src/hooks/dhcp/mysql/mysql_lb_messages.mes
modified:   src/hooks/dhcp/mysql/mysql_lease_mgr.cc
modified:   src/hooks/dhcp/mysql/mysql_lease_mgr.h
modified:   src/hooks/dhcp/pgsql/pgsql_lb_messages.cc
modified:   src/hooks/dhcp/pgsql/pgsql_lb_messages.mes
modified:   src/hooks/dhcp/pgsql/pgsql_lease_mgr.cc
modified:   src/hooks/dhcp/pgsql/pgsql_lease_mgr.h
modified:   src/lib/dhcpsrv/lease_mgr.cc
modified:   src/lib/dhcpsrv/lease_mgr.h
modified:   src/lib/dhcpsrv/sflq_allocator.h
modified:   src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.cc
modified:   src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.h

13 files changed:
src/hooks/dhcp/mysql/mysql_lb_messages.cc
src/hooks/dhcp/mysql/mysql_lb_messages.mes
src/hooks/dhcp/mysql/mysql_lease_mgr.cc
src/hooks/dhcp/mysql/mysql_lease_mgr.h
src/hooks/dhcp/pgsql/pgsql_lb_messages.cc
src/hooks/dhcp/pgsql/pgsql_lb_messages.mes
src/hooks/dhcp/pgsql/pgsql_lease_mgr.cc
src/hooks/dhcp/pgsql/pgsql_lease_mgr.h
src/lib/dhcpsrv/lease_mgr.cc
src/lib/dhcpsrv/lease_mgr.h
src/lib/dhcpsrv/sflq_allocator.h
src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.cc
src/lib/dhcpsrv/testutils/generic_lease_mgr_unittest.h

index 13b70cb8476084dfe3d317c4fec95ffef6097a24..0e28ad6cda65a121af130f2cdab41cef9efaab77 100644 (file)
@@ -137,11 +137,11 @@ const char* values[] = {
     "MYSQL_LB_SFLQ_CREATE_POOL6", "creating shared-flq pool for address range %1 - %2, type %3, delegated length: %4, subnet id %5, recreate %6, capacity %7",
     "MYSQL_LB_SFLQ_PICK_LEASE4", "picking a free lease from address range %1 - %2",
     "MYSQL_LB_SFLQ_PICK_LEASE6", "picking a free lease from address range %1 - %2",
-    "MYSQL_LB_SFLQ_POOL4_DELETE", "delete the V4 SFLQ pool with start address %1 and end address %2, forece = %3",
+    "MYSQL_LB_SFLQ_POOL4_DELETE", "delete the V4 SFLQ pool with start address %1 and end address %2, force = %3",
     "MYSQL_LB_SFLQ_POOL4_GET_ALL", "fetch all V4 SFLQ pools",
     "MYSQL_LB_SFLQ_POOL4_GET_BY_RANGE", "fetch all V4 SFLQ pools that overlap the range %1 and %2",
     "MYSQL_LB_SFLQ_POOL4_GET_BY_SUBNET", "fetch all V4 SFLQ pools for subnet-id %1",
-    "MYSQL_LB_SFLQ_POOL6_DELETE", "delete the V6 SFLQ pool with start address %1 and end address %2, forece = %3",
+    "MYSQL_LB_SFLQ_POOL6_DELETE", "delete the V6 SFLQ pool with start address %1 and end address %2, force = %3",
     "MYSQL_LB_SFLQ_POOL6_GET_ALL", "fetch all V6 SFLQ pools",
     "MYSQL_LB_SFLQ_POOL6_GET_BY_RANGE", "fetch all V6 SFLQ pools that overlap the range %1 and %2",
     "MYSQL_LB_SFLQ_POOL6_GET_BY_SUBNET", "fetch all V6 SFLQ pools for subnet-id %1",
index 117e0ed1b4825d7406ab53d59d35945e5c87ca95..323572ab9c5acf308067968a99386efa838aa956 100644 (file)
@@ -374,7 +374,7 @@ Logged at debug log level 50.
 This debug message is issued when the server asks the lease back end for
 the v4 SFLQ pool that overlap the given address range.
 
-% MYSQL_LB_SFLQ_POOL4_DELETE delete the V4 SFLQ pool with start address %1 and end address %2, forece = %3
+% MYSQL_LB_SFLQ_POOL4_DELETE delete the V4 SFLQ pool with start address %1 and end address %2, force = %3
 Logged at debug log level 50.
 This debug message is issued when the server asks the lease back to delete
 the v4 SFLQ pool (and it's free lease data) that match the given start and end
@@ -395,7 +395,7 @@ Logged at debug log level 50.
 This debug message is issued when the server asks the lease back end for
 the v6 SFLQ pool that overlap the given address range.
 
-% MYSQL_LB_SFLQ_POOL6_DELETE delete the V6 SFLQ pool with start address %1 and end address %2, forece = %3
+% MYSQL_LB_SFLQ_POOL6_DELETE delete the V6 SFLQ pool with start address %1 and end address %2, force = %3
 Logged at debug log level 50.
 This debug message is issued when the server asks the lease back to delete
 the v6 SFLQ pool (and it's free lease data) that match the given start and end
index f15988cafa1508bc074234b267ae5f94fba67cf6..1dea5c8750aa6560a3fc6dc204e56ef6e7545162 100644 (file)
@@ -591,7 +591,7 @@ tagged_statements = { {
                     "    LEFT JOIN free_lease4 AS f "
                     "    ON f.address >= q.start_address AND f.address <= q.end_address "
                     "    GROUP BY q.id "
-                    "    ORDER BY q.subnet_id, q.start_address"},
+                    "    ORDER BY q.subnet_id ASC, q.start_address ASC"},
 
     {MySqlLeaseMgr::SFLQ_POOL4_GET_BY_SUBNET,
                     "SELECT q.id, q.subnet_id, 3 as lease_type, "
@@ -5352,7 +5352,7 @@ MySqlLeaseMgr::sflqPickFreeLease6(IOAddress start_address, IOAddress end_address
     MYSQL_BIND obind[1];
     memset(obind, 0, sizeof(obind));
 
-    char b_addr_buffer[45];
+    char b_addr_buffer[POOL_ADDRESS6_BUF_LENGTH];
     unsigned long b_addr_length = sizeof(b_addr_buffer);
     obind[0].buffer_type = MYSQL_TYPE_STRING;
     obind[0].buffer = reinterpret_cast<char*>(b_addr_buffer);
@@ -5403,7 +5403,7 @@ MySqlLeaseMgr::sflqPool4Get(SubnetID subnet_id) {
               .arg(subnet_id);
 
     MySqlBindingCollection in_bindings = {
-        MySqlBinding::createInteger<int64_t>(subnet_id)
+        MySqlBinding::createInteger<int32_t>(subnet_id)
     };
 
     return (sflqPoolGetCommon(SFLQ_POOL4_GET_BY_SUBNET, in_bindings));
@@ -5537,7 +5537,7 @@ MySqlLeaseMgr::sflqPoolGetCommon(StatementIndex stindex,
     SflqPoolInfoCollectionPtr pools(new SflqPoolInfoCollection());
 
     ctx->conn_.selectQuery(stindex, where_bindings, out_bindings,
-                           [this, &pools]
+                           [&pools]
                            (MySqlBindingCollection& out_bindings) {
 
         SflqPoolInfoPtr info(new SflqPoolInfo());
index 80dff3b7ebe10c8af56b41c5d69a0f6b6dcfc3cd..e5ca36cbed40cbe020ce23a989585f53be99a3e9 100644 (file)
@@ -1365,7 +1365,7 @@ public:
     /// @brief Delete the SFLQ V4 pool that matches a start and end address.
     ///
     /// Deletes the flq_pool4 entry along with its free_lease4 data.
-    /// Fails If there are multiple pools that overalap the given range 
+    /// Fails If there are multiple pools that overlap the given range 
     /// unless force is true.
     ///
     /// @param start_address start address of the pool to delete.
@@ -1407,7 +1407,7 @@ public:
     /// @brief Delete the SFLQ V6 pool that matches a start and end address.
     ///
     /// Deletes the flq_pool6 entry along with its free_lease6 data.
-    /// Fails If there are multiple pools that overalap the given range 
+    /// Fails If there are multiple pools that overlap the given range 
     /// unless force is true.
     ///
     /// @param start_address start address of the pool to delete.
@@ -1432,7 +1432,7 @@ private:
     /// @param stindex index of the SQL statement to execute.
     /// @param where_bindings input bindings holding the where clause parameter
     /// values (if any)
-    /// @param family protocl family AF_INET or AF_INET6
+    /// @param family protocol family AF_INET or AF_INET6
     ///
     /// @return A collection of the SFLQ V4 pools.
     SflqPoolInfoCollectionPtr sflqPoolGetCommon(StatementIndex stindex,
@@ -1448,7 +1448,7 @@ private:
     /// @param start_address start address of the pool to delete.
     /// @param end_address end address of the pool to delete.
     /// @param force overrides check for overlapping pools when true.
-    /// @param family protocl family AF_INET or AF_INET6
+    /// @param family protocol family AF_INET or AF_INET6
     ///
     /// @return True a pool was deleted.
     /// @throw InvalidOperation if force is false and overlapping pools are
index 2d926f0081898f1a3ddc509eb41821873264c3ea..e83bf75244931d23cad029b8040c19f52416292e 100644 (file)
@@ -132,11 +132,11 @@ const char* values[] = {
     "PGSQL_LB_SFLQ_CREATE_POOL6", "creating shared-flq pool for address range %1 - %2, type %3, delegated length: %4, subnet id %5, recreate %6, capacity %7",
     "PGSQL_LB_SFLQ_PICK_LEASE4", "picking a free lease from address range %1 - %2",
     "PGSQL_LB_SFLQ_PICK_LEASE6", "picking a free lease from address range %1 - %2",
-    "PGSQL_LB_SFLQ_POOL4_DELETE", "delete the V4 SFLQ pool with start address %1 and end address %2, forece = %3",
+    "PGSQL_LB_SFLQ_POOL4_DELETE", "delete the V4 SFLQ pool with start address %1 and end address %2, force = %3",
     "PGSQL_LB_SFLQ_POOL4_GET_ALL", "fetch all V4 SFLQ pools",
     "PGSQL_LB_SFLQ_POOL4_GET_BY_RANGE", "fetch all V4 SFLQ pools that overlap the range %1 and %2",
     "PGSQL_LB_SFLQ_POOL4_GET_BY_SUBNET", "fetch all V4 SFLQ pools for subnet-id %1",
-    "PGSQL_LB_SFLQ_POOL6_DELETE", "delete the V6 SFLQ pool with start address %1 and end address %2, forece = %3",
+    "PGSQL_LB_SFLQ_POOL6_DELETE", "delete the V6 SFLQ pool with start address %1 and end address %2, force = %3",
     "PGSQL_LB_SFLQ_POOL6_GET_ALL", "fetch all V6 SFLQ pools",
     "PGSQL_LB_SFLQ_POOL6_GET_BY_RANGE", "fetch all V6 SFLQ pools that overlap the range %1 and %2",
     "PGSQL_LB_SFLQ_POOL6_GET_BY_SUBNET", "fetch all V6 SFLQ pools for subnet-id %1",
index 26db35ed0f38999088e59f03c685fc2c7e8231b2..f7f41facbd43e37cae78602a499bde6e2eb311c0 100644 (file)
@@ -356,7 +356,7 @@ Logged at debug log level 50.
 This debug message is issued when the server asks the lease back end for
 the v4 SFLQ pool that overlap the given address range.
 
-% PGSQL_LB_SFLQ_POOL4_DELETE delete the V4 SFLQ pool with start address %1 and end address %2, forece = %3
+% PGSQL_LB_SFLQ_POOL4_DELETE delete the V4 SFLQ pool with start address %1 and end address %2, force = %3
 Logged at debug log level 50.
 This debug message is issued when the server asks the lease back to delete
 the v4 SFLQ pool (and it's free lease data) that match the given start and end
@@ -377,7 +377,7 @@ Logged at debug log level 50.
 This debug message is issued when the server asks the lease back end for
 the v6 SFLQ pool that overlap the given address range.
 
-% PGSQL_LB_SFLQ_POOL6_DELETE delete the V6 SFLQ pool with start address %1 and end address %2, forece = %3
+% PGSQL_LB_SFLQ_POOL6_DELETE delete the V6 SFLQ pool with start address %1 and end address %2, force = %3
 Logged at debug log level 50.
 This debug message is issued when the server asks the lease back to delete
 the v6 SFLQ pool (and it's free lease data) that match the given start and end
index 75c55d1941aa03aeed8b1fdb183e1365728bc1c5..191aebf8917f1a4094a35397c5279f482aad2263 100644 (file)
@@ -752,7 +752,7 @@ PgSqlTaggedStatement tagged_statements[] = {
       "sflqDeleteLease6",
       "SELECT sflqDeleteLease6(cast($1 as inet), $2)" },
 
-    // SFLQ_SFLQ_POOL4_GET_ALL
+    // SFLQ_POOL4_GET_ALL
     { 0, { },
       "sflqPool4GetAll",
       "SELECT q.id, q.subnet_id, 3 as lease_type, "
@@ -764,9 +764,9 @@ PgSqlTaggedStatement tagged_statements[] = {
       "    LEFT JOIN free_lease4 AS f "
       "    ON f.address >= q.start_address AND f.address <= q.end_address "
       "    GROUP BY q.id "
-      "    ORDER BY q.subnet_id, q.start_address"},
+      "    ORDER BY q.subnet_id ASC, q.start_address ASC"},
 
-    // SFLQ_SFLQ_POOL4_GET_BY_SUBNET
+    // SFLQ_POOL4_GET_BY_SUBNET
     { 1, { OID_INT8 },
       "sflqPool4GetBySubnet",
       "SELECT q.id, q.subnet_id, 3 as lease_type, "
@@ -779,7 +779,7 @@ PgSqlTaggedStatement tagged_statements[] = {
       "    ON f.address >= q.start_address AND f.address <= q.end_address "
       "    WHERE q.subnet_id = $1 "
       "    GROUP BY q.id "
-      "    ORDER BY q.subnet_id, q.start_address"},
+      "    ORDER BY q.subnet_id ASC, q.start_address ASC"},
 
     // SFLQ_POOL4_GET_BY_RANGE
     { 2, { OID_INT8, OID_INT8 },
@@ -813,7 +813,7 @@ PgSqlTaggedStatement tagged_statements[] = {
       "     USING deleted_pool p "
       "     WHERE f.address BETWEEN p.start_address AND p.end_address"},
 
-    // SFLQ_SFLQ_POOL6_GET_ALL
+    // SFLQ_POOL6_GET_ALL
     { 0, { },
       "sflqPool6GetAll",
       "SELECT q.id, q.subnet_id, q.lease_type, "
@@ -828,7 +828,7 @@ PgSqlTaggedStatement tagged_statements[] = {
       "    GROUP BY q.id "
       "    ORDER BY q.subnet_id ASC, inetToBytea(q.start_address) ASC "},
 
-    // SFLQ_SFLQ_POOL6_GET_BY_SUBNET
+    // SFLQ_POOL6_GET_BY_SUBNET
     { 1, { OID_INT8 },
       "sflqPool6GetBySubnet",
       "SELECT q.id, q.subnet_id, q.lease_type, "
@@ -4155,7 +4155,7 @@ PgSqlLeaseMgr::sflqCreateFlqPool4(IOAddress start_address, IOAddress end_address
             .arg(recreate)
             .arg(capacity);
     } catch (const std::exception& ex) {
-        isc_throw(BadValue, "PgqlLeasMgr::sflqCreateFlqPool4 " << ex.what());
+        isc_throw(BadValue, "PgSqlLeaseMgr::sflqCreateFlqPool4 " << ex.what());
     }
 
     // Get a context
index 8d9e292d2a1ad18dc63f33000b9d206846ef6928..56876d4fab54f9925863ca441b8c3b14577f9794 100644 (file)
@@ -1322,7 +1322,7 @@ public:
     /// @brief Delete the SFLQ V4 pool that matches a start and end address.
     ///
     /// Deletes the flq_pool4 entry along with its free_lease4 data.
-    /// Fails If there are multiple pools that overalap the given range
+    /// Fails If there are multiple pools that overlap the given range
     /// unless force is true.
     ///
     /// @param start_address start address of the pool to delete.
@@ -1364,7 +1364,7 @@ public:
     /// @brief Delete the SFLQ V6 pool that matches a start and end address.
     ///
     /// Deletes the flq_pool6 entry along with its free_lease6 data.
-    /// Fails If there are multiple pools that overalap the given range
+    /// Fails If there are multiple pools that overlap the given range
     /// unless force is true.
     ///
     /// @param start_address start address of the pool to delete.
@@ -1388,7 +1388,7 @@ private:
     /// @param stindex index of the SQL statement to execute.
     /// @param where_bindings input bindings holding the where clause parameter
     /// values (if any)
-    /// @param family protocl family AF_INET or AF_INET6
+    /// @param family protocol family AF_INET or AF_INET6
     ///
     /// @return A collection of the SFLQ V4 pools.
     SflqPoolInfoCollectionPtr sflqPoolGetCommon(const StatementIndex& stindex,
@@ -1402,7 +1402,7 @@ private:
     /// @param start_address start address of the pool to delete.
     /// @param end_address end address of the pool to delete.
     /// @param force overrides check for overlapping pools when true.
-    /// @param family protocl family AF_INET or AF_INET6
+    /// @param family protocol family AF_INET or AF_INET6
     ///
     /// @return True a pool was deleted.
     /// @throw InvalidOperation if force is false and overlapping pools are
index 539f9622fe185d4c8446e70e00a3978009317673..9df33ef49550cf24985fd433e50428d67cad02d1 100644 (file)
@@ -13,6 +13,7 @@
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/sflq_allocator.h>
+#include <dhcpsrv/subnet_id.h>
 #include <exceptions/exceptions.h>
 #include <stats/stats_mgr.h>
 #include <util/encode/encode.h>
@@ -1968,7 +1969,7 @@ SflqPoolInfo::SflqPoolInfo():
     start_address_(IOAddress::IPV4_ZERO_ADDRESS()),
     end_address_(IOAddress::IPV4_ZERO_ADDRESS()),
     delegated_len_(128),
-    subnet_id_(0),
+    subnet_id_(SUBNET_ID_UNUSED),
     free_leases_(0),
     created_ts_(),
     modified_ts_() {
@@ -2041,7 +2042,7 @@ LeaseMgr::sflqPool6Get(SubnetID) {
 
 SflqPoolInfoCollectionPtr
 LeaseMgr::sflqPool6Get(asiolink::IOAddress, asiolink::IOAddress) {
-    isc_throw(NotImplemented, "LeaseMgr::sflqPool6Get(IOAdress,IOAddress) called");
+    isc_throw(NotImplemented, "LeaseMgr::sflqPool6Get(IOAddress,IOAddress) called");
 }
 
 bool
index 999c14222d23734333dacdefd15e01ac73a43334..5b5dc3d64a6dc904021cc9b34378851c5d20f760 100644 (file)
@@ -1206,7 +1206,7 @@ public:
     /// @brief Delete the SFLQ V4 pool that matches a start and end address.
     ///
     /// Deletes the flq_pool4 entry along with its free_lease4 data.
-    /// Fails If there are multiple pools that overalap the given range 
+    /// Fails If there are multiple pools that overlap the given range 
     /// unless force is true.
     ///
     /// @param start_address start address of the pool to delete.
@@ -1248,7 +1248,7 @@ public:
     /// @brief Delete the SFLQ V6 pool that matches a start and end address.
     ///
     /// Deletes the flq_pool6 entry along with its free_lease6 data.
-    /// Fails If there are multiple pools that overalap the given range 
+    /// Fails If there are multiple pools that overlap the given range 
     /// unless force is true.
     ///
     /// @param start_address start address of the pool to delete.
index faff8a8e8abda56c65c979ace36c2fbae82b30b1..6c95c54c66cb689f794be91ddcf2db62d9d74de6 100644 (file)
@@ -20,8 +20,8 @@ namespace dhcp {
 /// This allocator is part of the Shared Free Lease Queue (SFLQ) Allocation
 /// scheme. The concept is similar to FLQ Allocation but rather than the
 /// creating and maintaining free lease data locally, it is created and
-/// maintained in the lease back end (MySql and PosgreSQL only) where it
-/// can be shared by other servers.
+/// maintained in the lease back end where it can be shared by other 
+/// servers.
 ///
 /// Th allocator relies on stored procedures in the lease back end to
 /// return free leases for a given IP address range (i.e. pool).  This can
index 164d5716ee3cb648de948856a26f91f6bfed5ca8..6b965eafeadf363ad54f0e00000c6bf9a73d6fe7 100644 (file)
@@ -6169,7 +6169,7 @@ GenericLeaseMgrTest::testSflqAPIFuncs4() {
                                        test_pool->subnet_id_, false));
     }
 
-    // Fetching all pools should find none.
+    // Fetching all pools should find all three.
     ASSERT_NO_THROW_LOG(pool_infos = lmptr_->sflqPool4GetAll());
     ASSERT_TRUE(pool_infos);
     ASSERT_EQ(3, pool_infos->size());
@@ -6324,7 +6324,7 @@ GenericLeaseMgrTest::testSflqAPIFuncs6(Lease::Type lease_type) {
                                        test_pool->subnet_id_, false));
     }
 
-    // Fetching all pools should find none.
+    // Fetching all pools should find all three. 
     ASSERT_NO_THROW_LOG(pool_infos = lmptr_->sflqPool6GetAll());
     ASSERT_TRUE(pool_infos);
     ASSERT_EQ(3, pool_infos->size());
@@ -6606,7 +6606,6 @@ GenericLeaseMgrTest::sflqCreateFlqPool4Concurrent() {
                 lmptr_->sflqCreateFlqPool4(start_address, end_address, false));
     });
 
-    usleep(1000);
     thread th2([this, &ret2, start_address, end_address]() {
             ASSERT_NO_THROW_LOG(ret2 =
                 lmptr_->sflqCreateFlqPool4(start_address, end_address, false));
index f08dceeca99cb9770b6f912e90418308b3d1f44c..30cdd4ae75c7584c1096b1c8f6655253561a816f 100644 (file)
@@ -197,7 +197,7 @@ public:
     /// equal to their rhs counterparts.  Asserts if they are not "equal".
     /// 
     /// @param lhs left-side instance to compare 
-    /// @param rhs reft-side instance to compare
+    /// @param rhs right-side instance to compare
     /// @param lineno source line of invocation (pass in __LINE__)
     void checkPoolInfos(const SflqPoolInfo& lhs, const SflqPoolInfo& rhs, int lineno);