From: Francis Dupont Date: Mon, 14 Mar 2022 15:33:42 +0000 (+0100) Subject: [#2249] Addressed comments X-Git-Tag: Kea-2.1.4~39 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=88a932fbbf56cb006bda54c92479fb36f69ac89b;p=thirdparty%2Fkea.git [#2249] Addressed comments --- diff --git a/doc/examples/kea4/all-keys.json b/doc/examples/kea4/all-keys.json index 01b38b8464..808358fbd0 100644 --- a/doc/examples/kea4/all-keys.json +++ b/doc/examples/kea4/all-keys.json @@ -455,7 +455,7 @@ "re-detect": true }, - // Boolean parameter which controls whether an early global host + // Boolean parameter which controls whether an early global host // reservations lookup should be performed. This lookup takes place // before subnet selection and when a global reservation is found // with some client classes triggers a second phase classification. diff --git a/doc/examples/kea6/all-keys.json b/doc/examples/kea6/all-keys.json index 3c7ce10960..68356bb7b4 100644 --- a/doc/examples/kea6/all-keys.json +++ b/doc/examples/kea6/all-keys.json @@ -398,7 +398,7 @@ "re-detect": true }, - // Boolean parameter which controls whether an early global host + // Boolean parameter which controls whether an early global host // reservations lookup should be performed. This lookup takes place // before subnet selection and when a global reservation is found // with some client classes triggers a second phase classification. diff --git a/doc/sphinx/arm/classify.rst b/doc/sphinx/arm/classify.rst index ed7357433c..6675424766 100644 --- a/doc/sphinx/arm/classify.rst +++ b/doc/sphinx/arm/classify.rst @@ -91,9 +91,9 @@ The classification process is conducted in several steps: class. After a subnet is selected, the server determines whether there is a reservation for a given client. Therefore, it is not possible to use the ``UNKNOWN`` class to select a shared network or - a subnet, and for using the ``KNOWN`` class only global reservations - can be used and the ``early-global-reservations-lookup`` parameter - must be configured to true + a subnet. For the ``KNOWN`` class only global reservations only + global reservations are used and the ``early-global-reservations-lookup`` + parameter must be configured to true 10. When the incoming packet belongs to the special class ``DROP``, it is dropped and an informational message is logged with the packet diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index b8ce076cef..f8b16d93fa 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -603,7 +603,7 @@ protected: /// Returns ACK message, NAK message, or NULL /// /// @param request a message received from client - /// @param context pointer to the client context where allocated and + /// @param context pointer to the client context where allocated and /// deleted leases are stored. /// /// @return ACK or NAK message diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 76c0ba0274..8918c8142b 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -453,8 +453,8 @@ Dhcpv6Srv::earlyGHRLookup(const Pkt6Ptr& query, // Add classes from the global reservations. const ClientClasses& classes = global_host->getClientClasses6(); - for (ClientClasses::const_iterator cclass = classes.cbegin(); - cclass != classes.cend(); ++cclass) { + for (ClientClasses::const_iterator cclass = classes.cbegin(); + cclass != classes.cend(); ++cclass) { query->addClass(*cclass); } @@ -550,7 +550,7 @@ Dhcpv6Srv::initContext(const Pkt6Ptr& pkt, // time evaluate to false as desired. removeDependentEvaluatedClasses(pkt); setReservedClientClasses(pkt, ctx); - evaluateClasses(pkt, false); + evaluateClasses(pkt, false); } // Set KNOWN builtin class if something was found, UNKNOWN if not. diff --git a/src/bin/dhcp6/tests/classify_unittests.cc b/src/bin/dhcp6/tests/classify_unittests.cc index 8791a80654..ee68acfdea 100644 --- a/src/bin/dhcp6/tests/classify_unittests.cc +++ b/src/bin/dhcp6/tests/classify_unittests.cc @@ -2465,7 +2465,7 @@ TEST_F(ClassifyTest, earlyDrop) { ASSERT_NO_THROW(client2.doSolicit(true)); - // Not matchine so not dropped. + // Not matching so not dropped. EXPECT_TRUE(client2.getContext().response_); } diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc index 1cca731d92..683a71cf62 100644 --- a/src/bin/dhcp6/tests/fqdn_unittest.cc +++ b/src/bin/dhcp6/tests/fqdn_unittest.cc @@ -455,8 +455,10 @@ public: AllocEngine::ClientContext6 ctx; bool drop = !srv_->earlyGHRLookup(query, ctx); + ASSERT_FALSE(drop); srv_->initContext(query, ctx, drop); + ASSERT_FALSE(drop); Pkt6Ptr answer = generateMessageWithIds(DHCPV6_ADVERTISE); ASSERT_NO_THROW(srv_->processClientFqdn(query, answer, ctx)); diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index ef22751a41..a2973995ca 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -473,10 +473,10 @@ public: /// update existing lease. bool fake_allocation_; - /// @brief Indicates if early global reservation is looked for. - /// - /// This caches the early-global-reservations-lookup value. - bool early_global_reservations_lookup_; + /// @brief Indicates if early global reservation is looked for. + /// + /// This caches the early-global-reservations-lookup value. + bool early_global_reservations_lookup_; /// @brief Subnet selected for the client by the server. Subnet6Ptr subnet_; @@ -1004,9 +1004,9 @@ public: public: /// @brief Determines the preferred and valid v6 lease lifetimes. /// - /// A candidate triplet for both preferred and valid lifetimes will be + /// A candidate triplet for both preferred and valid lifetimes will be /// selected from the first class matched to the query which defines the - /// value or from the subnet if none do. Classes are searched in the order + /// value or from the subnet if none do. Classes are searched in the order /// they are assigned to the query. /// /// If the client requested a lifetime IA hint, then the @@ -1378,10 +1378,10 @@ public: /// information to the allocation engine methods is that adding /// new information doesn't modify the API of the allocation engine. struct ClientContext4 : public boost::noncopyable { - /// @brief Indicates if early global reservation is looked for. - /// - /// This caches the early-global-reservations-lookup value. - bool early_global_reservations_lookup_; + /// @brief Indicates if early global reservation is looked for. + /// + /// This caches the early-global-reservations-lookup value. + bool early_global_reservations_lookup_; /// @brief Subnet selected for the client by the server. Subnet4Ptr subnet_; diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index 36f5b929e5..0b490e87b9 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -1331,6 +1331,7 @@ TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkReservations) { lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease2)); ctx.subnet_ = subnet1_; + ctx.hosts_.clear(); AllocEngine::findReservation(ctx); lease = engine_.allocateLease4(ctx); ASSERT_TRUE(lease); @@ -1374,6 +1375,7 @@ TEST_F(SharedNetworkAlloc4Test, discoverSharedNetworkReservationsNoColl) { lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease2)); ctx.subnet_ = subnet1_; + ctx.hosts_.clear(); AllocEngine::findReservation(ctx); lease = engine_.allocateLease4(ctx); ASSERT_TRUE(lease); @@ -1647,6 +1649,7 @@ TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkReservations) { lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease2)); ctx.subnet_ = subnet1_; + ctx.hosts_.clear(); AllocEngine::findReservation(ctx); lease = engine_.allocateLease4(ctx); ASSERT_TRUE(lease); @@ -1693,6 +1696,7 @@ TEST_F(SharedNetworkAlloc4Test, requestSharedNetworkReservationsNoColl) { lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease2)); ctx.subnet_ = subnet1_; + ctx.hosts_.clear(); AllocEngine::findReservation(ctx); lease = engine_.allocateLease4(ctx); ASSERT_TRUE(lease); @@ -3066,18 +3070,21 @@ TEST_F(AllocEngine4Test, findReservation) { CfgMgr::instance().commit(); // This time the reservation should be returned. + ctx.hosts_.clear(); ASSERT_NO_THROW(engine.findReservation(ctx)); EXPECT_TRUE(ctx.currentHost()); EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); // It shouldn't be returned when reservations-in-subnet is disabled. subnet_->setReservationsInSubnet(false); + ctx.hosts_.clear(); ASSERT_NO_THROW(engine.findReservation(ctx)); EXPECT_FALSE(ctx.currentHost()); // Check the reservations-in-subnet and reservations-out-of-pool flags. subnet_->setReservationsInSubnet(true); subnet_->setReservationsOutOfPool(true); + ctx.hosts_.clear(); ASSERT_NO_THROW(engine.findReservation(ctx)); EXPECT_TRUE(ctx.currentHost()); EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); @@ -3090,6 +3097,7 @@ TEST_F(AllocEngine4Test, findReservation) { CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); CfgMgr::instance().commit(); + ctx.hosts_.clear(); ASSERT_NO_THROW(engine.findReservation(ctx)); EXPECT_TRUE(ctx.currentHost()); EXPECT_EQ(ctx.currentHost()->getIPv4Reservation(), host->getIPv4Reservation()); @@ -3097,6 +3105,7 @@ TEST_F(AllocEngine4Test, findReservation) { // Remove the subnet. Subnet id is required to find host reservations, so // if it is set to NULL, no reservation should be returned ctx.subnet_.reset(); + ctx.hosts_.clear(); ASSERT_NO_THROW(engine.findReservation(ctx)); EXPECT_FALSE(ctx.currentHost()); @@ -3109,6 +3118,7 @@ TEST_F(AllocEngine4Test, findReservation) { CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); CfgMgr::instance().commit(); + ctx.hosts_.clear(); ASSERT_NO_THROW(engine.findReservation(ctx)); EXPECT_FALSE(ctx.currentHost()); }