From dbe1f7019c5e8b0a76c8724f9a6ca4b1dcaa9ced Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 19 Jan 2023 09:12:07 -0800 Subject: [PATCH] [radio] new API to update frame counter only if new value is larger (#8607) This commit adds a new radio platform API which updates the MAC Frame Counter only if the new value is larger than current value being used by radio platform, i.e., `otPlatRadioSetMacFrameCounterIfLarger()`. OT stack provides a default weak implementation of this API which calls the existing `otPlatRadioSetMacFrameCouter()`. This API is used when radio provides `OT_RADIO_CAPS_TRANSMIT_SEC` capability and thereby is responsible for assigning the frame counter to tx frames. It helps address an edge-case where OT stack may not yet know of the latest frame counter values used by radio platform (e.g., due to enhanced acks processed by radio platform directly or due to delay between RCP and host) and then setting the value from OT stack may cause the counter value on radio to move back (OT stack will set the counter after appending it in `LinkFrameCounterTlv` on all radios when multi-radio links feature is enabled). This commit also updates spinel and RCP and `radio_spinel` implementations to support the new API. This is added in a fully compatible way, i.e., a new host firmware works with older RCP, and similarly an old host can work with new RCP firmware. --- include/openthread/instance.h | 2 +- include/openthread/link_raw.h | 15 +++++++++++++++ include/openthread/platform/radio.h | 11 +++++++++++ src/core/api/link_raw_api.cpp | 7 ++++++- src/core/mac/link_raw.cpp | 4 ++-- src/core/mac/link_raw.hpp | 6 ++++-- src/core/mac/sub_mac.cpp | 16 +++++++++++++--- src/core/mac/sub_mac.hpp | 4 +++- src/core/radio/radio.hpp | 12 ++++++++++++ src/core/radio/radio_platform.cpp | 22 ++++++++++++++++++++++ src/core/thread/key_manager.cpp | 10 ++++++---- src/core/thread/key_manager.hpp | 8 +++++--- src/core/thread/mle.cpp | 4 ++-- src/lib/spinel/radio_spinel.hpp | 6 ++++-- src/lib/spinel/radio_spinel_impl.hpp | 5 +++-- src/lib/spinel/spinel.h | 5 ++++- src/ncp/ncp_base_radio.cpp | 15 ++++++++++++++- src/posix/platform/radio.cpp | 8 +++++++- 18 files changed, 134 insertions(+), 26 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index fc36d1aa1..3f12693ae 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (276) +#define OPENTHREAD_API_VERSION (277) /** * @addtogroup api-instance diff --git a/include/openthread/link_raw.h b/include/openthread/link_raw.h index d3544d580..461b81d7a 100644 --- a/include/openthread/link_raw.h +++ b/include/openthread/link_raw.h @@ -356,6 +356,9 @@ otError otLinkRawSetMacKey(otInstance *aInstance, /** * Sets the current MAC frame counter value. * + * This function always sets the MAC counter to the new given value @p aMacFrameCounter independent of the current + * value. + * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aMacFrameCounter The MAC frame counter value. * @@ -365,6 +368,18 @@ otError otLinkRawSetMacKey(otInstance *aInstance, */ otError otLinkRawSetMacFrameCounter(otInstance *aInstance, uint32_t aMacFrameCounter); +/** + * Sets the current MAC frame counter value only if the new value is larger than the current one. + * + * @param[in] aInstance A pointer to an OpenThread instance. + * @param[in] aMacFrameCounter The MAC frame counter value. + * + * @retval OT_ERROR_NONE If successful. + * @retval OT_ERROR_INVALID_STATE If the raw link-layer isn't enabled. + * + */ +otError otLinkRawSetMacFrameCounterIfLarger(otInstance *aInstance, uint32_t aMacFrameCounter); + /** * Get current platform time (64bits width) of the radio chip. * diff --git a/include/openthread/platform/radio.h b/include/openthread/platform/radio.h index 70ceddc5c..5d90e501a 100644 --- a/include/openthread/platform/radio.h +++ b/include/openthread/platform/radio.h @@ -620,6 +620,17 @@ void otPlatRadioSetMacKey(otInstance *aInstance, */ void otPlatRadioSetMacFrameCounter(otInstance *aInstance, uint32_t aMacFrameCounter); +/** + * This method sets the current MAC frame counter value only if the new given value is larger than the current value. + * + * This function is used when radio provides `OT_RADIO_CAPS_TRANSMIT_SEC` capability. + * + * @param[in] aInstance A pointer to an OpenThread instance. + * @param[in] aMacFrameCounter The MAC frame counter value. + * + */ +void otPlatRadioSetMacFrameCounterIfLarger(otInstance *aInstance, uint32_t aMacFrameCounter); + /** * Get the current estimated time (in microseconds) of the radio chip. * diff --git a/src/core/api/link_raw_api.cpp b/src/core/api/link_raw_api.cpp index a8c944822..12a76d53d 100644 --- a/src/core/api/link_raw_api.cpp +++ b/src/core/api/link_raw_api.cpp @@ -223,7 +223,12 @@ otError otLinkRawSetMacKey(otInstance *aInstance, otError otLinkRawSetMacFrameCounter(otInstance *aInstance, uint32_t aMacFrameCounter) { - return AsCoreType(aInstance).Get().SetMacFrameCounter(aMacFrameCounter); + return AsCoreType(aInstance).Get().SetMacFrameCounter(aMacFrameCounter, /* aSetIfLarger */ false); +} + +otError otLinkRawSetMacFrameCounterIfLarger(otInstance *aInstance, uint32_t aMacFrameCounter) +{ + return AsCoreType(aInstance).Get().SetMacFrameCounter(aMacFrameCounter, /* aSetIfLarger */ true); } uint64_t otLinkRawGetRadioTime(otInstance *aInstance) diff --git a/src/core/mac/link_raw.cpp b/src/core/mac/link_raw.cpp index 5857da877..ca87f4147 100644 --- a/src/core/mac/link_raw.cpp +++ b/src/core/mac/link_raw.cpp @@ -258,12 +258,12 @@ exit: return error; } -Error LinkRaw::SetMacFrameCounter(uint32_t aMacFrameCounter) +Error LinkRaw::SetMacFrameCounter(uint32_t aFrameCounter, bool aSetIfLarger) { Error error = kErrorNone; VerifyOrExit(IsEnabled(), error = kErrorInvalidState); - mSubMac.SetFrameCounter(aMacFrameCounter); + mSubMac.SetFrameCounter(aFrameCounter, aSetIfLarger); exit: return error; diff --git a/src/core/mac/link_raw.hpp b/src/core/mac/link_raw.hpp index 29b6d528b..d630ebe63 100644 --- a/src/core/mac/link_raw.hpp +++ b/src/core/mac/link_raw.hpp @@ -271,13 +271,15 @@ public: /** * This method sets the current MAC frame counter value. * - * @param[in] aMacFrameCounter The MAC frame counter value. + * @param[in] aFrameCounter The MAC frame counter value. + * @param[in] aSetIfLarger If `true`, set only if the new value @p aFrameCounter is larger than current value. + * If `false`, set the new value independent of the current value. * * @retval kErrorNone If successful. * @retval kErrorInvalidState If the raw link-layer isn't enabled. * */ - Error SetMacFrameCounter(uint32_t aMacFrameCounter); + Error SetMacFrameCounter(uint32_t aFrameCounter, bool aSetIfLarger); #if OT_SHOULD_LOG_AT(OT_LOG_LEVEL_INFO) /** diff --git a/src/core/mac/sub_mac.cpp b/src/core/mac/sub_mac.cpp index 8f401a3eb..ba16d12c8 100644 --- a/src/core/mac/sub_mac.cpp +++ b/src/core/mac/sub_mac.cpp @@ -966,13 +966,23 @@ exit: return; } -void SubMac::SetFrameCounter(uint32_t aFrameCounter) +void SubMac::SetFrameCounter(uint32_t aFrameCounter, bool aSetIfLarger) { - mFrameCounter = aFrameCounter; + if (!aSetIfLarger || (aFrameCounter > mFrameCounter)) + { + mFrameCounter = aFrameCounter; + } VerifyOrExit(!ShouldHandleTransmitSecurity()); - Get().SetMacFrameCounter(aFrameCounter); + if (aSetIfLarger) + { + Get().SetMacFrameCounterIfLarger(aFrameCounter); + } + else + { + Get().SetMacFrameCounter(aFrameCounter); + } exit: return; diff --git a/src/core/mac/sub_mac.hpp b/src/core/mac/sub_mac.hpp index 2822a4be4..7d3c952ee 100644 --- a/src/core/mac/sub_mac.hpp +++ b/src/core/mac/sub_mac.hpp @@ -489,9 +489,11 @@ public: * This method sets the current MAC Frame Counter value. * * @param[in] aFrameCounter The MAC Frame Counter value. + * @param[in] aSetIfLarger If `true`, set only if the new value @p aFrameCounter is larger than the current value. + * If `false`, set the new value independent of the current value. * */ - void SetFrameCounter(uint32_t aFrameCounter); + void SetFrameCounter(uint32_t aFrameCounter, bool aSetIfLarger); #if OPENTHREAD_CONFIG_MAC_FILTER_ENABLE /** diff --git a/src/core/radio/radio.hpp b/src/core/radio/radio.hpp index 4d31e5e1d..8e0494f1d 100644 --- a/src/core/radio/radio.hpp +++ b/src/core/radio/radio.hpp @@ -301,6 +301,18 @@ public: otPlatRadioSetMacFrameCounter(GetInstancePtr(), aMacFrameCounter); } + /** + * This method sets the current MAC Frame Counter value only if the new given value is larger than the current + * value. + * + * @param[in] aMacFrameCounter The MAC Frame Counter value. + * + */ + void SetMacFrameCounterIfLarger(uint32_t aMacFrameCounter) + { + otPlatRadioSetMacFrameCounterIfLarger(GetInstancePtr(), aMacFrameCounter); + } + /** * This method gets the radio's transmit power in dBm. * diff --git a/src/core/radio/radio_platform.cpp b/src/core/radio/radio_platform.cpp index 80787d137..7840a867c 100644 --- a/src/core/radio/radio_platform.cpp +++ b/src/core/radio/radio_platform.cpp @@ -212,6 +212,28 @@ OT_TOOL_WEAK void otPlatRadioSetMacFrameCounter(otInstance *aInstance, uint32_t OT_UNUSED_VARIABLE(aMacFrameCounter); } +OT_TOOL_WEAK void otPlatRadioSetMacFrameCounterIfLarger(otInstance *aInstance, uint32_t aMacFrameCounter) +{ + // Radio platforms that support `OT_RADIO_CAPS_TRANSMIT_SEC` should + // provide this radio platform function. + // + // This function helps address an edge-case where OT stack may not + // yet know the latest frame counter values used by radio platform + // (e.g., due to enhanced acks processed by radio platform directly + // or due to delay between RCP and host) and then setting the value + // from OT stack may cause the counter value on radio to move back + // (OT stack will set the counter after appending it in + // `LinkFrameCounterTlv` on all radios when multi-radio links + // feature is enabled). + // + // The weak implementation here is intended as a solution to ensure + // temporary compatibility with radio platforms that may not yet + // implement it. If this weak implementation is used, the edge-case + // above may still happen. + + otPlatRadioSetMacFrameCounter(aInstance, aMacFrameCounter); +} + OT_TOOL_WEAK uint64_t otPlatTimeGet(void) { return UINT64_MAX; } OT_TOOL_WEAK uint64_t otPlatRadioGetNow(otInstance *aInstance) diff --git a/src/core/thread/key_manager.cpp b/src/core/thread/key_manager.cpp index 168899997..642688952 100644 --- a/src/core/thread/key_manager.cpp +++ b/src/core/thread/key_manager.cpp @@ -381,7 +381,7 @@ void KeyManager::SetCurrentKeySequence(uint32_t aKeySequence) mKeySequence = aKeySequence; UpdateKeyMaterial(); - SetAllMacFrameCounters(0); + SetAllMacFrameCounters(0, /* aSetIfLarger */ false); mMleFrameCounter = 0; Get().Signal(kEventThreadKeySeqCounterChanged); @@ -412,12 +412,14 @@ const Mac::KeyMaterial &KeyManager::GetTemporaryTrelMacKey(uint32_t aKeySequence } #endif -void KeyManager::SetAllMacFrameCounters(uint32_t aMacFrameCounter) +void KeyManager::SetAllMacFrameCounters(uint32_t aFrameCounter, bool aSetIfLarger) { - mMacFrameCounters.SetAll(aMacFrameCounter); + mMacFrameCounters.SetAll(aFrameCounter); #if OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE - Get().SetFrameCounter(aMacFrameCounter); + Get().SetFrameCounter(aFrameCounter, aSetIfLarger); +#else + OT_UNUSED_VARIABLE(aSetIfLarger); #endif } diff --git a/src/core/thread/key_manager.hpp b/src/core/thread/key_manager.hpp index 048c22355..cba93e1c5 100644 --- a/src/core/thread/key_manager.hpp +++ b/src/core/thread/key_manager.hpp @@ -401,10 +401,12 @@ public: /** * This method sets the current MAC Frame Counter value for all radio links. * - * @param[in] aMacFrameCounter The MAC Frame Counter value. - * + * @param[in] aFrameCounter The MAC Frame Counter value. + * @param[in] aSetIfLarger If `true`, set only if the new value @p aFrameCounter is larger than current value. + * If `false`, set the new value independent of the current value. + */ - void SetAllMacFrameCounters(uint32_t aMacFrameCounter); + void SetAllMacFrameCounters(uint32_t aFrameCounter, bool aSetIfLarger); /** * This method sets the MAC Frame Counter value which is stored in non-volatile memory. diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index a47104d7f..0dc90cda4 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -365,7 +365,7 @@ void Mle::Restore(void) Get().SetCurrentKeySequence(networkInfo.GetKeySequence()); Get().SetMleFrameCounter(networkInfo.GetMleFrameCounter()); - Get().SetAllMacFrameCounters(networkInfo.GetMacFrameCounter()); + Get().SetAllMacFrameCounters(networkInfo.GetMacFrameCounter(), /* aSetIfLarger */ false); #if OPENTHREAD_MTD mDeviceMode.Set(networkInfo.GetDeviceMode() & ~DeviceMode::kModeFullThreadDevice); @@ -4511,7 +4511,7 @@ Error Mle::TxMessage::AppendLinkFrameCounterTlv(void) counter = Get().GetMaximumMacFrameCounter(); #if OPENTHREAD_CONFIG_MULTI_RADIO - Get().SetAllMacFrameCounters(counter); + Get().SetAllMacFrameCounters(counter, /* aSetIfLarger */ true); #endif return Tlv::Append(*this, counter); diff --git a/src/lib/spinel/radio_spinel.hpp b/src/lib/spinel/radio_spinel.hpp index 65468c28f..a7701811b 100644 --- a/src/lib/spinel/radio_spinel.hpp +++ b/src/lib/spinel/radio_spinel.hpp @@ -648,10 +648,12 @@ public: /** * This method sets the current MAC Frame Counter value. * - * @param[in] aMacFrameCounter The MAC Frame Counter value. + * @param[in] aMacFrameCounter The MAC Frame Counter value. + * @param[in] aSetIfLarger If `true`, set only if the new value is larger than the current value. + * If `false`, set the new value independent of the current value. * */ - otError SetMacFrameCounter(uint32_t aMacFrameCounter); + otError SetMacFrameCounter(uint32_t aMacFrameCounter, bool aSetIfLarger); /** * This method sets the radio region code. diff --git a/src/lib/spinel/radio_spinel_impl.hpp b/src/lib/spinel/radio_spinel_impl.hpp index 22793081d..61b8f545d 100644 --- a/src/lib/spinel/radio_spinel_impl.hpp +++ b/src/lib/spinel/radio_spinel_impl.hpp @@ -1222,11 +1222,12 @@ exit: } template -otError RadioSpinel::SetMacFrameCounter(uint32_t aMacFrameCounter) +otError RadioSpinel::SetMacFrameCounter(uint32_t aMacFrameCounter, bool aSetIfLarger) { otError error; - SuccessOrExit(error = Set(SPINEL_PROP_RCP_MAC_FRAME_COUNTER, SPINEL_DATATYPE_UINT32_S, aMacFrameCounter)); + SuccessOrExit(error = Set(SPINEL_PROP_RCP_MAC_FRAME_COUNTER, SPINEL_DATATYPE_UINT32_S SPINEL_DATATYPE_BOOL_S, + aMacFrameCounter, aSetIfLarger)); exit: return error; diff --git a/src/lib/spinel/spinel.h b/src/lib/spinel/spinel.h index bf6a5ad1d..433b0e9dd 100644 --- a/src/lib/spinel/spinel.h +++ b/src/lib/spinel/spinel.h @@ -4772,9 +4772,12 @@ enum SPINEL_PROP_RCP_MAC_KEY = SPINEL_PROP_RCP_EXT__BEGIN + 0, /// MAC Frame Counter - /** Format: `L`. + /** Format: `L` for read and `Lb` or `L` for write * * `L`: MAC frame counter + * 'b': Optional boolean used only during write. If not provided, `false` is assumed. + * If `true` counter is set only if the new value is larger than current value. + * If `false` the new value is set as frame counter independent of the current value. * * The Spinel property is used to set MAC frame counter to RCP. * diff --git a/src/ncp/ncp_base_radio.cpp b/src/ncp/ncp_base_radio.cpp index 443762730..4cd679981 100644 --- a/src/ncp/ncp_base_radio.cpp +++ b/src/ncp/ncp_base_radio.cpp @@ -508,10 +508,23 @@ template <> otError NcpBase::HandlePropertySet