From 83272e7eae3f15cc7057c6392d8cd848971ed085 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Fri, 29 Aug 2025 17:30:00 -0700 Subject: [PATCH] [nat64] use `OwningList` to manage active mappings (#11878) This change updates the management of active NAT64 mappings to use an `OwningList`. To support this, the `Mapping` struct now inherits from `InstanceLocatorInit` and includes a new `Free()` method. This method encapsulates the logic for releasing the associated IPv4 address and the `Mapping` entry back to their corresponding pools. This new design simplifies the `Translator` class by removing the redundant `ReleaseMapping()`, `ReleaseMappings()`, and `ReleaseExpiredMappings()` methods. All mapping cleanup operations are now handled by the `OwningList` class (which invokes `Mapping::Free()` on entries as they are removed). --- src/core/net/nat64_translator.cpp | 55 ++++++++----------------------- src/core/net/nat64_translator.hpp | 11 +++---- 2 files changed, 18 insertions(+), 48 deletions(-) diff --git a/src/core/net/nat64_translator.cpp b/src/core/net/nat64_translator.cpp index 4e6170c30..d38a34f64 100644 --- a/src/core/net/nat64_translator.cpp +++ b/src/core/net/nat64_translator.cpp @@ -66,6 +66,7 @@ const char *StateToString(State aState) Translator::Translator(Instance &aInstance) : InstanceLocator(aInstance) , mState(State::kStateDisabled) + , mMappingPool(aInstance) , mTimer(aInstance) { Random::NonCrypto::Fill(mNextMappingId); @@ -372,10 +373,12 @@ void Translator::Mapping::CopyTo(AddressMapping &aMapping, TimeMilli aNow) const aMapping.mRemainingTimeMs = (mExpiry < aNow) ? 0 : mExpiry - aNow; } -void Translator::ReleaseMapping(Mapping &aMapping) +void Translator::Mapping::Free(void) { + LogInfo("Mapping removed: %s", ToString().AsCString()); + #if OPENTHREAD_CONFIG_NAT64_PORT_TRANSLATION_ENABLE - if (mIp4Cidr.mLength > kAddressMappingCidrLimit) + if (Get().mIp4Cidr.mLength > kAddressMappingCidrLimit) { // If `CONFIG_NAT64_PORT_TRANSLATION_ENABLE` is enabled // IPv4 addresses are allocated from the pool only when the @@ -385,35 +388,12 @@ void Translator::ReleaseMapping(Mapping &aMapping) else #endif { - IgnoreError(mIp4AddressPool.PushBack(aMapping.mIp4Address)); + IgnoreError(Get().mIp4AddressPool.PushBack(mIp4Address)); } - mMappingPool.Free(aMapping); - - LogInfo("Mapping removed: %s", aMapping.ToString().AsCString()); + Get().mMappingPool.Free(*this); } -uint16_t Translator::ReleaseMappings(LinkedList &aMappings) -{ - uint16_t numRemoved = 0; - - for (Mapping *mapping = aMappings.Pop(); mapping != nullptr; mapping = aMappings.Pop()) - { - numRemoved++; - ReleaseMapping(*mapping); - } - - return numRemoved; -} - -uint16_t Translator::ReleaseExpiredMappings(void) -{ - LinkedList idleMappings; - - mActiveMappings.RemoveAllMatching(idleMappings, TimerMilli::GetNow()); - - return ReleaseMappings(idleMappings); -} #if OPENTHREAD_CONFIG_NAT64_PORT_TRANSLATION_ENABLE uint16_t Translator::AllocateSourcePort(uint16_t aSrcPort) { @@ -471,12 +451,10 @@ Translator::Mapping *Translator::AllocateMapping(const Ip6::Headers &aIp6Headers { if (mIp4AddressPool.IsEmpty()) { - // `ReleaseExpiredMappings()` returns the number of - // mappings removed. - - VerifyOrExit(ReleaseExpiredMappings() > 0); + mActiveMappings.RemoveAndFreeAllMatching(TimerMilli::GetNow()); } + VerifyOrExit(!mIp4AddressPool.IsEmpty()); ip4Addr = *mIp4AddressPool.PopBack(); } @@ -646,8 +624,7 @@ Error Translator::SetIp4Cidr(const Ip4::Cidr &aCidr) numberOfHosts = OT_MIN(numberOfHosts, kPoolSize); - mMappingPool.FreeAll(); - mActiveMappings.Clear(); + mActiveMappings.Free(); mIp4AddressPool.Clear(); for (uint32_t i = 0; i < numberOfHosts; i++) @@ -675,8 +652,7 @@ exit: void Translator::ClearIp4Cidr(void) { mIp4Cidr.Clear(); - mMappingPool.FreeAll(); - mActiveMappings.Clear(); + mActiveMappings.Free(); mIp4AddressPool.Clear(); UpdateState(); @@ -709,13 +685,8 @@ exit: void Translator::HandleTimer(void) { - uint16_t numReleased = ReleaseExpiredMappings(); - - LogInfo("Released %u expired mappings", numReleased); - + mActiveMappings.RemoveAndFreeAllMatching(TimerMilli::GetNow()); mTimer.Start(Min(kIcmpTimeout, kIdleTimeout)); - - OT_UNUSED_VARIABLE(numReleased); } void Translator::AddressMappingIterator::Init(Instance &aInstance) @@ -845,7 +816,7 @@ void Translator::SetEnabled(bool aEnabled) if (!aEnabled) { - ReleaseMappings(mActiveMappings); + mActiveMappings.Free(); } UpdateState(); diff --git a/src/core/net/nat64_translator.hpp b/src/core/net/nat64_translator.hpp index c0f08f697..fc49ff0db 100644 --- a/src/core/net/nat64_translator.hpp +++ b/src/core/net/nat64_translator.hpp @@ -37,8 +37,8 @@ #include "openthread-core-config.h" #include "common/array.hpp" -#include "common/linked_list.hpp" #include "common/locator.hpp" +#include "common/owning_list.hpp" #include "common/pool.hpp" #include "common/timer.hpp" #include "net/ip4_types.hpp" @@ -321,12 +321,14 @@ private: static constexpr DropReason kReasonUnsupportedProto = OT_NAT64_DROP_REASON_UNSUPPORTED_PROTO; static constexpr DropReason kReasonNoMapping = OT_NAT64_DROP_REASON_NO_MAPPING; - struct Mapping : public LinkedListEntry + struct Mapping : public InstanceLocatorInit, public LinkedListEntry { static constexpr uint16_t kInfoStringSize = 70; typedef String InfoString; + void Init(Instance &aInstance) { InstanceLocatorInit::Init(aInstance); } + void Free(void); void Touch(uint8_t aProtocol); InfoString ToString(void) const; void CopyTo(AddressMapping &aMapping, TimeMilli aNow) const; @@ -351,9 +353,6 @@ private: Error TranslateIcmp4(Message &aMessage, uint16_t aOriginalId); Error TranslateIcmp6(Message &aMessage, uint16_t aTranslatedId); - uint16_t ReleaseMappings(LinkedList &aMappings); - void ReleaseMapping(Mapping &aMapping); - uint16_t ReleaseExpiredMappings(void); Mapping *AllocateMapping(const Ip6::Headers &aIp6Headers); void HandleTimer(void); void UpdateState(void); @@ -371,7 +370,7 @@ private: uint64_t mNextMappingId; Array mIp4AddressPool; Pool mMappingPool; - LinkedList mActiveMappings; + OwningList mActiveMappings; Ip6::Prefix mNat64Prefix; Ip4::Cidr mIp4Cidr; TranslatorTimer mTimer;