From 54195e0ecf086e5400755e7b50053e2977fe620f Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 28 Aug 2025 08:33:37 -0700 Subject: [PATCH] [nat64] improve `AddressMappingIterator` & expiration time calculation (#11855) This commit improves the NAT64 address mapping iterator to ensure the remaining lifetime for all mapping entries is reported consistently. The iterator now internally stores a timestamp upon initialization. This timestamp is then used as a common reference to calculate the remaining lifetime for each `otNat64AddressMapping` entry, ensuring consistent values throughout a single iteration. The public C APIs remain unchanged, while the underlying implementation and the `otNat64AddressMappingIterator` struct are updated. --- include/openthread/instance.h | 2 +- include/openthread/nat64.h | 33 +++++++++++----- src/core/api/nat64_api.cpp | 8 ++-- src/core/net/nat64_translator.cpp | 17 ++++---- src/core/net/nat64_translator.hpp | 66 ++++++++++++++++++------------- tests/unit/test_nat64.cpp | 10 +++-- 6 files changed, 81 insertions(+), 55 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index c1fde84d1..eb59a917c 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -52,7 +52,7 @@ extern "C" { * * @note This number versions both OpenThread platform and user APIs. */ -#define OPENTHREAD_API_VERSION (529) +#define OPENTHREAD_API_VERSION (530) /** * @addtogroup api-instance diff --git a/include/openthread/nat64.h b/include/openthread/nat64.h index 17ed89121..bcaec32d1 100644 --- a/include/openthread/nat64.h +++ b/include/openthread/nat64.h @@ -177,9 +177,16 @@ typedef struct otNat64AddressMapping * OPENTHREAD_CONFIG_NAT64_PORT_TRANSLATION_ENABLE is true. */ uint16_t mTranslatedPortOrId; - uint32_t mRemainingTimeMs; ///< Remaining time before expiry in milliseconds. - otNat64ProtocolCounters mCounters; + /** + * Remaining time in milliseconds before the entry expires. + * + * The remaining time is relative to the initialization of the `otNat64AddressMappingIterator`, i.e., when + * `otNat64InitAddressMappingIterator()` was called. + */ + uint32_t mRemainingTimeMs; + + otNat64ProtocolCounters mCounters; ///< Counters. } otNat64AddressMapping; /** @@ -188,21 +195,30 @@ typedef struct otNat64AddressMapping * The fields in this type are opaque (intended for use by OpenThread core only) and therefore should not be * accessed or used by caller. * - * Before using an iterator, it MUST be initialized using `otNat64AddressMappingIteratorInit()`. + * Before using an iterator, it MUST be initialized using `otNat64InitAddressMappingIterator()`. + * + * The member fields in this struct are for internal OpenThread stack use and should not be accessed directly. */ typedef struct otNat64AddressMappingIterator { - void *mPtr; + const void *mPtr; + uint32_t mData32; } otNat64AddressMappingIterator; /** * Initializes an `otNat64AddressMappingIterator`. * - * An iterator MUST be initialized before it is used. + * Available when `OPENTHREAD_CONFIG_NAT64_TRANSLATOR_ENABLE` is enabled. * - * An iterator can be initialized again to restart from the beginning of the mapping info. + * An iterator MUST be initialized before it is used. An iterator can be initialized again to restart from the + * beginning of the mapping info list. * - * @param[in] aInstance The OpenThread instance. + * The iterator initialization time is used to report the `mRemainingTimeMs` in the `otNat64AddressMapping` retrieved + * when calling `otNat64GetNextAddressMapping()` to iterate over the list. This ensures that all entry + * `mRemainingTimeMs` values are consistent and are from the same time origin, regardless of how or when + * `otNat64GetNextAddressMapping()` is called. + * + * @param[in] aInstance A pointer to the OpenThread instance. * @param[out] aIterator A pointer to the iterator to initialize. */ void otNat64InitAddressMappingIterator(otInstance *aInstance, otNat64AddressMappingIterator *aIterator); @@ -214,8 +230,7 @@ void otNat64InitAddressMappingIterator(otInstance *aInstance, otNat64AddressMapp * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in,out] aIterator A pointer to the iterator. On success the iterator will be updated to point to next - * NAT64 address mapping record. To get the first entry the iterator should be set to - * OT_NAT64_ADDRESS_MAPPING_ITERATOR_INIT. + * NAT64 address mapping record. * @param[out] aMapping A pointer to an `otNat64AddressMapping` where information of next NAT64 address * mapping record is placed (on success). * diff --git a/src/core/api/nat64_api.cpp b/src/core/api/nat64_api.cpp index 7bef7ac14..6ceab789e 100644 --- a/src/core/api/nat64_api.cpp +++ b/src/core/api/nat64_api.cpp @@ -68,19 +68,17 @@ void otNat64SetReceiveIp4Callback(otInstance *aInstance, otNat64ReceiveIp4Callba void otNat64InitAddressMappingIterator(otInstance *aInstance, otNat64AddressMappingIterator *aIterator) { - AssertPointerIsNotNull(aIterator); - - AsCoreType(aInstance).Get().InitAddressMappingIterator(*aIterator); + AsCoreType(aIterator).Init(AsCoreType(aInstance)); } otError otNat64GetNextAddressMapping(otInstance *aInstance, otNat64AddressMappingIterator *aIterator, otNat64AddressMapping *aMapping) { - AssertPointerIsNotNull(aIterator); + OT_UNUSED_VARIABLE(aInstance); AssertPointerIsNotNull(aMapping); - return AsCoreType(aInstance).Get().GetNextAddressMapping(*aIterator, *aMapping); + return AsCoreType(aIterator).GetNext(*aMapping); } void otNat64GetCounters(otInstance *aInstance, otNat64ProtocolCounters *aCounters) diff --git a/src/core/net/nat64_translator.cpp b/src/core/net/nat64_translator.cpp index b7ec64fb0..8580e36cd 100644 --- a/src/core/net/nat64_translator.cpp +++ b/src/core/net/nat64_translator.cpp @@ -717,21 +717,20 @@ void Translator::HandleTimer(void) OT_UNUSED_VARIABLE(numReleased); } -void Translator::InitAddressMappingIterator(AddressMappingIterator &aIterator) +void Translator::AddressMappingIterator::Init(Instance &aInstance) { - aIterator.mPtr = mActiveMappings.GetHead(); + SetMapping(aInstance.Get().mActiveMappings.GetHead()); + SetInitTime(TimerMilli::GetNow()); } -Error Translator::GetNextAddressMapping(AddressMappingIterator &aIterator, AddressMapping &aMapping) +Error Translator::AddressMappingIterator::GetNext(AddressMapping &aMapping) { - Error error = kErrorNotFound; - Mapping *mapping = static_cast(aIterator.mPtr); + Error error = kErrorNone; - VerifyOrExit(mapping != nullptr); + VerifyOrExit(GetMapping() != nullptr, error = kErrorNotFound); - mapping->CopyTo(aMapping, TimerMilli::GetNow()); - aIterator.mPtr = mapping->GetNext(); - error = kErrorNone; + GetMapping()->CopyTo(aMapping, GetInitTime()); + SetMapping(GetMapping()->GetNext()); exit: return error; diff --git a/src/core/net/nat64_translator.hpp b/src/core/net/nat64_translator.hpp index aff44ce64..b7b99dbcf 100644 --- a/src/core/net/nat64_translator.hpp +++ b/src/core/net/nat64_translator.hpp @@ -71,11 +71,12 @@ const char *StateToString(State aState); */ class Translator : public InstanceLocator, private NonCopyable { + struct Mapping; + public: - typedef otNat64AddressMapping AddressMapping; ///< Address mapping. - typedef otNat64AddressMappingIterator AddressMappingIterator; ///< Address mapping Iterator. - typedef otNat64DropReason DropReason; ///< Drop reason. - typedef otNat64ErrorCounters ErrorCounters; ///< Error counters. + typedef otNat64AddressMapping AddressMapping; ///< Address mapping. + typedef otNat64DropReason DropReason; ///< Drop reason. + typedef otNat64ErrorCounters ErrorCounters; ///< Error counters. /** * The possible results of NAT64 translation. @@ -87,6 +88,39 @@ public: kDrop, ///< Silently drop the message. }; + /** + * An iterator to iterate over `AddressMapping` entries. + */ + class AddressMappingIterator : public otNat64AddressMappingIterator + { + public: + /** + * Initializes an `AddressMappingIterator`. + * + * An iterator MUST be initialized before it is used. An iterator can be initialized again to start from the + * beginning of the mapping info list. + * + * @param[in] aInstance The OpenThread instance. + */ + void Init(Instance &aInstance); + + /** + * Gets the next `AddressMapping` info using the iterator. + * + * @param[out] aMapping An `AddressMapping` to output to next NAT64 address mapping. + * + * @retval kErrorNone Successfully found the next NAT64 address mapping info. + * @retval kErrorNotFound No subsequent NAT64 address mapping info was found. + */ + Error GetNext(AddressMapping &aMapping); + + private: + void SetMapping(const Mapping *aMapping) { mPtr = aMapping; } + const Mapping *GetMapping(void) const { return static_cast(mPtr); } + void SetInitTime(TimeMilli aNow) { mData32 = aNow.GetValue(); } + TimeMilli GetInitTime(void) const { return TimeMilli(mData32); } + }; + /** * Represents the counters for the protocols supported by NAT64. */ @@ -219,29 +253,6 @@ public: */ void ClearNat64Prefix(void); - /** - * Initializes an `AddressMappingIterator`. - * - * An iterator MUST be initialized before it is used. - * - * An iterator can be initialized again to restart from the beginning of the mapping info. - * - * @param[out] aIterator An iterator to initialize. - */ - void InitAddressMappingIterator(AddressMappingIterator &aIterator); - - /** - * Gets the next AddressMapping info (using an iterator). - * - * @param[in,out] aIterator The iterator. - * @param[out] aMapping An `AddressMapping` to output to next NAT64 address mapping. - * - * @retval kErrorNone Successfully found the next NAT64 address mapping info (@p aMapping and @p aIterator - * are updated. - * @retval kErrorNotFound No subsequent NAT64 address mapping info was found. - */ - Error GetNextAddressMapping(AddressMappingIterator &aIterator, AddressMapping &aMapping); - /** * Gets the NAT64 translator counters. * @@ -361,6 +372,7 @@ private: DefineMapEnum(otNat64State, Nat64::State); #if OPENTHREAD_CONFIG_NAT64_TRANSLATOR_ENABLE +DefineCoreType(otNat64AddressMappingIterator, Nat64::Translator::AddressMappingIterator); DefineCoreType(otNat64ProtocolCounters, Nat64::Translator::ProtocolCounters); #endif diff --git a/tests/unit/test_nat64.cpp b/tests/unit/test_nat64.cpp index e2d5c6355..ac7b56918 100644 --- a/tests/unit/test_nat64.cpp +++ b/tests/unit/test_nat64.cpp @@ -360,8 +360,9 @@ void TestPacketCounter(void) otNat64AddressMapping mapping; size_t totalMappingCount = 0; - sInstance->Get().InitAddressMappingIterator(iter); - while (sInstance->Get().GetNextAddressMapping(iter, mapping) == kErrorNone) + iter.Init(*sInstance); + + while (iter.GetNext(mapping) == kErrorNone) { totalMappingCount++; VerifyCounters(otNat64ProtocolCounters{.mTotal = @@ -432,8 +433,9 @@ void TestPacketCounter(void) otNat64AddressMapping mapping; size_t totalMappingCount = 0; - sInstance->Get().InitAddressMappingIterator(iter); - while (sInstance->Get().GetNextAddressMapping(iter, mapping) == kErrorNone) + iter.Init(*sInstance); + + while (iter.GetNext(mapping) == kErrorNone) { totalMappingCount++; VerifyCounters(otNat64ProtocolCounters{.mTotal =