From: Marcin Siodelski Date: Wed, 19 Apr 2023 14:08:11 +0000 (+0200) Subject: [#2823] Addressed review comments X-Git-Tag: Kea-2.3.7~22 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0c30fc802cc84d85ca425f7aa417c3e69250590c;p=thirdparty%2Fkea.git [#2823] Addressed review comments --- diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc index ca1c9d7366..5ba211f519 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc @@ -221,10 +221,10 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, // Allocator selection at the global level can affect subnets and shared networks // for which the allocator hasn't been specified explicitly. Let's see if the // allocator has been specified at the global level. - std::string default_allocator; + std::string global_allocator; auto allocator = external_cfg->getConfiguredGlobal(CfgGlobals::ALLOCATOR); if (allocator && (allocator->getType() == Element::string)) { - default_allocator = allocator->stringValue(); + global_allocator = allocator->stringValue(); } // If we're fetching the changes from the config backend we also want @@ -234,10 +234,9 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, // We're only affected by the allocator change if this is the update from // the configuration backend. if (cb_update) { - std::string current_default_allocator; auto allocator = CfgMgr::instance().getCurrentCfg()->getConfiguredGlobal(CfgGlobals::ALLOCATOR); if (allocator && (allocator->getType() == Element::string)) { - allocator_changed = (default_allocator != allocator->stringValue()); + allocator_changed = (global_allocator != allocator->stringValue()); } } @@ -268,7 +267,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, (*network)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr { return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals()); }); - (*network)->setDefaultAllocatorType(default_allocator); + (*network)->setDefaultAllocatorType(global_allocator); external_cfg->getCfgSharedNetworks4()->add((*network)); } @@ -279,7 +278,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, Subnet4Collection subnets; if (allocator_changed || reconfig) { // A change of the allocator or the server reconfiguration can affect all - // shared networks. Get all subnets. + // subnets. Get all subnets. subnets = getMgr().getPool()->getAllSubnets4(backend_selector, server_selector); } else if (!updated_entries.empty()) { @@ -300,7 +299,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, (*subnet)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr { return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals()); }); - (*subnet)->setDefaultAllocatorType(default_allocator); + (*subnet)->setDefaultAllocatorType(global_allocator); external_cfg->getCfgSubnets4()->add((*subnet)); } diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc index 4624bf495b..91b482ebad 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc @@ -220,17 +220,17 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector // Allocator selection at the global level can affect subnets and shared networks // for which the allocator hasn't been specified explicitly. Let's see if the // allocator has been specified at the global level. - std::string default_allocator; + std::string global_allocator; auto allocator = external_cfg->getConfiguredGlobal(CfgGlobals::ALLOCATOR); if (allocator && (allocator->getType() == Element::string)) { - default_allocator = allocator->stringValue(); + global_allocator = allocator->stringValue(); } // Also, get the PD allocator. - std::string default_pd_allocator; + std::string global_pd_allocator; allocator = external_cfg->getConfiguredGlobal(CfgGlobals::PD_ALLOCATOR); if (allocator && (allocator->getType() == Element::string)) { - default_pd_allocator = allocator->stringValue(); + global_pd_allocator = allocator->stringValue(); } // If we're fetching the changes from the config backend we also want @@ -242,7 +242,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector if (cb_update) { auto allocator = CfgMgr::instance().getCurrentCfg()->getConfiguredGlobal(CfgGlobals::ALLOCATOR); if (allocator && (allocator->getType() == Element::string)) { - allocator_changed = (default_allocator != allocator->stringValue()); + allocator_changed = (global_allocator != allocator->stringValue()); } // The address allocator hasn't changed. So, let's check if the PD allocator @@ -250,7 +250,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector if (!allocator_changed) { auto allocator = CfgMgr::instance().getCurrentCfg()->getConfiguredGlobal(CfgGlobals::PD_ALLOCATOR); if (allocator && (allocator->getType() == Element::string)) { - allocator_changed = (default_pd_allocator != allocator->stringValue()); + allocator_changed = (global_pd_allocator != allocator->stringValue()); } } } @@ -278,8 +278,8 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector (*network)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr { return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals()); }); - (*network)->setDefaultAllocatorType(default_allocator); - (*network)->setDefaultPdAllocatorType(default_pd_allocator); + (*network)->setDefaultAllocatorType(global_allocator); + (*network)->setDefaultPdAllocatorType(global_pd_allocator); external_cfg->getCfgSharedNetworks6()->add((*network)); } @@ -290,7 +290,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector Subnet6Collection subnets; if (allocator_changed || reconfig) { // A change of the allocator or the server reconfiguration can affect all - // shared networks. Get all subnets. + // subnets. Get all subnets. subnets = getMgr().getPool()->getAllSubnets6(backend_selector, server_selector); } else if (!updated_entries.empty()) { @@ -311,8 +311,8 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector (*subnet)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr { return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals()); }); - (*subnet)->setDefaultAllocatorType(default_allocator); - (*subnet)->setDefaultPdAllocatorType(default_pd_allocator); + (*subnet)->setDefaultAllocatorType(global_allocator); + (*subnet)->setDefaultPdAllocatorType(global_pd_allocator); external_cfg->getCfgSubnets6()->add((*subnet)); } diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h index 3eda94f6a3..9d2a8995e1 100644 --- a/src/lib/dhcpsrv/network.h +++ b/src/lib/dhcpsrv/network.h @@ -1237,6 +1237,9 @@ protected: util::Optional allocator_type_; /// @brief Default allocator type. + /// + /// This value is not configurable by the user. It is used by the configuration + /// backend internally. util::Optional default_allocator_type_; /// @brief Pointer to another network that this network belongs to. diff --git a/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc b/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc index 33ae382901..350106f11b 100644 --- a/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc +++ b/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc @@ -361,8 +361,11 @@ public: // Test that the allocator returns the correct type. TEST_F(FreeLeaseQueueAllocatorTest6, getType) { - FreeLeaseQueueAllocator alloc(Lease::TYPE_NA, subnet_); - EXPECT_EQ("flq", alloc.getType()); + FreeLeaseQueueAllocator allocNA(Lease::TYPE_NA, subnet_); + EXPECT_EQ("flq", allocNA.getType()); + + FreeLeaseQueueAllocator allocPD(Lease::TYPE_PD, subnet_); + EXPECT_EQ("flq", allocPD.getType()); } // Test populating free DHCPv6 address leases to the queue. diff --git a/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc b/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc index 3d28607e45..bde1805a11 100644 --- a/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc +++ b/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc @@ -119,8 +119,11 @@ using IterativeAllocatorTest6 = AllocEngine6Test; // Test that the allocator returns the correct type. TEST_F(IterativeAllocatorTest6, getType) { - IterativeAllocator alloc(Lease::TYPE_NA, subnet_); - EXPECT_EQ("iterative", alloc.getType()); + IterativeAllocator allocNA(Lease::TYPE_NA, subnet_); + EXPECT_EQ("iterative", allocNA.getType()); + + IterativeAllocator allocPD(Lease::TYPE_PD, subnet_); + EXPECT_EQ("iterative", allocPD.getType()); } // This test verifies that the allocator picks addresses that belong to the diff --git a/src/lib/dhcpsrv/tests/random_allocator_unittest.cc b/src/lib/dhcpsrv/tests/random_allocator_unittest.cc index 55fec78d2f..e8009eed3a 100644 --- a/src/lib/dhcpsrv/tests/random_allocator_unittest.cc +++ b/src/lib/dhcpsrv/tests/random_allocator_unittest.cc @@ -186,8 +186,11 @@ using RandomAllocatorTest6 = AllocEngine6Test; // Test that the allocator returns the correct type. TEST_F(RandomAllocatorTest6, getType) { - RandomAllocator alloc(Lease::TYPE_NA, subnet_); - EXPECT_EQ("random", alloc.getType()); + RandomAllocator allocNA(Lease::TYPE_NA, subnet_); + EXPECT_EQ("random", allocNA.getType()); + + RandomAllocator allocPD(Lease::TYPE_PD, subnet_); + EXPECT_EQ("random", allocPD.getType()); } // Test allocating IPv6 addresses when a subnet has a single pool.