From: Francis Dupont Date: Sun, 16 Oct 2022 23:16:32 +0000 (+0200) Subject: [#2596] Checkpoint for #2595 update X-Git-Tag: Kea-2.3.3~75 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=72fa5ea9382b72031d96e5489728a48a6b0eb529;p=thirdparty%2Fkea.git [#2596] Checkpoint for #2595 update --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 9fcdcce2ec..86996ebf90 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -1924,7 +1924,7 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx, if (!ctx.fake_allocation_) { // Add(update) the extended information on the lease. - static_cast(updateLease6ExtendedInfo(expired, ctx)); + updateLease6ExtendedInfo(expired, ctx); // for REQUEST we do update the lease LeaseMgrFactory::instance().updateLease6(expired); @@ -2085,7 +2085,7 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx, if (!ctx.fake_allocation_) { // Add(update) the extended information on the lease. - static_cast(updateLease6ExtendedInfo(lease, ctx)); + updateLease6ExtendedInfo(lease, ctx); // That is a real (REQUEST) allocation bool status = LeaseMgrFactory::instance().addLease(lease); @@ -2401,7 +2401,8 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) { } // @todo should we call storeLease6ExtendedInfo() here ? - if (updateLease6ExtendedInfo(lease, ctx)) { + updateLease6ExtendedInfo(lease, ctx); + if (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE) { changed = true; } @@ -3145,7 +3146,10 @@ void AllocEngine::reclaimLeaseInDatabase(const LeasePtrType& lease, lease->fqdn_fwd_ = false; lease->fqdn_rev_ = false; lease->state_ = Lease::STATE_EXPIRED_RECLAIMED; - lease->setContext(ElementPtr()); + if (lease->getContext()) { + lease->extended_info_action_ = ExtendedInfoAction::ACTION_DELETE; + lease->setContext(ElementPtr()); + } lease_update_fun(lease); } else { @@ -4043,7 +4047,7 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr, lease->hostname_ = ctx.hostname_; // Add(update) the extended information on the lease. - static_cast(updateLease4ExtendedInfo(lease, ctx)); + updateLease4ExtendedInfo(lease, ctx); // Let's execute all callouts registered for lease4_select if (ctx.callout_handle_ && @@ -4623,28 +4627,27 @@ AllocEngine::updateLease4Information(const Lease4Ptr& lease, } // Add(update) the extended information on the lease. - if (updateLease4ExtendedInfo(lease, ctx)) { + updateLease4ExtendedInfo(lease, ctx); + if (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE) { changed = true; } return (changed); } -bool +void AllocEngine::updateLease4ExtendedInfo(const Lease4Ptr& lease, const AllocEngine::ClientContext4& ctx) const { - bool changed = false; - // If storage is not enabled then punt. if (!ctx.subnet_->getStoreExtendedInfo()) { - return (changed); + return; } // Look for relay agent information option (option 82) OptionPtr rai = ctx.query_->getOption(DHO_DHCP_AGENT_OPTIONS); if (!rai) { // Pkt4 doesn't have it, so nothing to store (or update). - return (changed); + return; } // Create a StringElement with the hex string for relay-agent-info. @@ -4693,30 +4696,26 @@ AllocEngine::updateLease4ExtendedInfo(const Lease4Ptr& lease, // Add/replace the extended info entry. ConstElementPtr old_extended_info = mutable_isc->get("relay-agent-info"); if (!old_extended_info || (*old_extended_info != *extended_info)) { - changed = true; + lease->extended_info_action_ = ExtendedInfoAction::ACTION_UPDATE; mutable_isc->set("relay-agent-info", extended_info); mutable_user_context->set("ISC", mutable_isc); } // Update the lease's user_context. lease->setContext(mutable_user_context); - - return (changed); } bool AllocEngine::updateLease6ExtendedInfo(const Lease6Ptr& lease, const AllocEngine::ClientContext6& ctx) const { - bool changed = false; - // If storage is not enabled then punt. if (!ctx.subnet_->getStoreExtendedInfo()) { - return (changed); + return; } // If we do not have relay information, then punt. if (ctx.query_->relay_info_.empty()) { - return (changed); + return; } // We need to convert the vector of RelayInfo instances in @@ -4802,15 +4801,13 @@ AllocEngine::updateLease6ExtendedInfo(const Lease6Ptr& lease, // Add/replace the extended info entry. ConstElementPtr old_extended_info = mutable_isc->get("relay-info"); if (!old_extended_info || (*old_extended_info != *extended_info)) { - changed = true; + lease->extended_info_action_ = ExtendedInfoAction::ACTION_UPDATE; mutable_isc->set("relay-info", extended_info); mutable_user_context->set("ISC", mutable_isc); } // Update the lease's user context. lease->setContext(mutable_user_context); - - return (changed); } void diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index 681475f545..07257145a1 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -1898,9 +1898,7 @@ protected: /// @param [out] lease A pointer to the lease to be updated. /// @param ctx A context containing information from the server about the /// client and its message. - /// @return True if there was a significant (e.g. other than cltt) change, - /// false otherwise. - bool updateLease4ExtendedInfo(const Lease4Ptr& lease, + void updateLease4ExtendedInfo(const Lease4Ptr& lease, const ClientContext4& ctx) const; /// @brief Stores additional client query parameters on a V6 lease @@ -1918,9 +1916,7 @@ protected: /// @param [out] lease A pointer to the lease to be updated. /// @param ctx A context containing information from the server about the /// client and its message. - /// @return True if there was a significant (e.g. other than cltt) change, - /// false otherwise. - bool updateLease6ExtendedInfo(const Lease6Ptr& lease, + void updateLease6ExtendedInfo(const Lease6Ptr& lease, const ClientContext6& ctx) const; private: diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc index d14c53b273..0d529cdd20 100644 --- a/src/lib/dhcpsrv/lease.cc +++ b/src/lib/dhcpsrv/lease.cc @@ -44,7 +44,8 @@ Lease::Lease(const isc::asiolink::IOAddress& addr, reuseable_valid_lft_(0), cltt_(cltt), current_cltt_(cltt), subnet_id_(subnet_id), hostname_(boost::algorithm::to_lower_copy(hostname)), fqdn_fwd_(fqdn_fwd), - fqdn_rev_(fqdn_rev), hwaddr_(hwaddr), state_(STATE_DEFAULT) { + fqdn_rev_(fqdn_rev), hwaddr_(hwaddr), state_(STATE_DEFAULT), + extended_info_action_(ExtendedInfoAction::ACTION_IGNORE) { } diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index 0c2c791903..73a913e8d4 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/src/lib/dhcpsrv/lease.h @@ -184,6 +184,16 @@ struct Lease : public isc::data::UserContext, public isc::data::CfgToElement { /// belonging to this class. uint32_t state_; + /// @brief Action on extended info tables. + typedef enum { + ACTION_IGNORE, ///< ignore extended info, + ACTION_DELETE, ///< delete reference to the lease + ACTION_UPDATE ///< update extended info tables. + } ExtendedInfoAction; + + /// @brief Record the action on extended info tables in the lease. + ExtendedInfoAction extended_info_action_; + /// @brief Convert Lease to Printable Form /// /// @return String form of the lease diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index 7e6ec5dfba..3afce6887b 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -3851,8 +3851,10 @@ TEST_F(AllocEngine4Test, updateExtendedInfo4) { } // Call AllocEngine::updateLease4ExtendeInfo(). - bool ret = false; - ASSERT_NO_THROW_LOG(ret = engine.callUpdateLease4ExtendedInfo(lease, ctx)); + ASSERT_NO_THROW_LOG(engine.callUpdateLease4ExtendedInfo(lease, ctx)); + bool ret = (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE); + // Reset the lease action. + lease->extended_info_action_ = ExtendedInfoAction::ACTION_IGNORE; ASSERT_EQ(scenario.exp_ret, ret); // Verify the lease has the expected user context content. diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index 74182e145c..5f8f5d802e 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc @@ -4299,8 +4299,10 @@ TEST_F(AllocEngine6ExtendedInfoTest, updateExtendedInfo6) { ctx.query_->relay_info_ = scenario.relays_; // Call AllocEngine::updateLease6ExtendeInfo(). - bool ret; - ASSERT_NO_THROW_LOG(ret = engine_.callUpdateLease6ExtendedInfo(lease, ctx)); + ASSERT_NO_THROW_LOG(engine_.callUpdateLease6ExtendedInfo(lease, ctx)); + bool ret = (lease->extended_info_action_ == ExtendedInfoAction::ACTION_UPDATE); + // Reset the lease action. + lease->extended_info_action_ = ExtendedInfoAction::ACTION_IGNORE; ASSERT_EQ(scenario.exp_ret, ret); // Verify the lease has the expected user context content. diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h index 2e4ffbef10..be8152419e 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h @@ -93,8 +93,7 @@ public: /// @brief Wrapper method for invoking AllocEngine4::updateLease4ExtendedInfo(). /// @param lease lease to update /// @param ctx current packet processing context - /// @return true if extended information was changed - bool callUpdateLease4ExtendedInfo(const Lease4Ptr& lease, + void callUpdateLease4ExtendedInfo(const Lease4Ptr& lease, AllocEngine::ClientContext4& ctx) const { return (updateLease4ExtendedInfo(lease, ctx)); } @@ -102,8 +101,7 @@ public: /// @brief Wrapper method for invoking AllocEngine6::updateLease6ExtendedInfo(). /// @param lease lease to update /// @param ctx current packet processing context - /// @return true if extended information was changed - bool callUpdateLease6ExtendedInfo(const Lease6Ptr& lease, + void callUpdateLease6ExtendedInfo(const Lease6Ptr& lease, AllocEngine::ClientContext6& ctx) const { return (updateLease6ExtendedInfo(lease, ctx)); }