diff --git a/src/core/mac/sub_mac.cpp b/src/core/mac/sub_mac.cpp index 5334f62e6..d3f248b00 100644 --- a/src/core/mac/sub_mac.cpp +++ b/src/core/mac/sub_mac.cpp @@ -279,7 +279,7 @@ void SubMac::HandleReceiveDone(RxFrame *aFrame, Error aError) if (!ShouldHandleTransmitSecurity() && aFrame != nullptr && aFrame->mInfo.mRxInfo.mAckedWithSecEnhAck) { - UpdateFrameCounter(aFrame->mInfo.mRxInfo.mAckFrameCounter); + SignalFrameCounterUsed(aFrame->mInfo.mRxInfo.mAckFrameCounter); } #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE @@ -362,7 +362,7 @@ void SubMac::ProcessTransmitSecurity(void) uint32_t frameCounter = GetFrameCounter(); mTransmitFrame.SetFrameCounter(frameCounter); - UpdateFrameCounter(frameCounter + 1); + SignalFrameCounterUsed(frameCounter); } extAddress = &GetExtAddress(); @@ -536,7 +536,7 @@ void SubMac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aErro OT_UNREACHABLE_CODE(ExitNow()); } - UpdateFrameCounterOnTxDone(aFrame); + SignalFrameCounterUsedOnTxDone(aFrame); // Determine whether a CSMA retry is required. @@ -571,7 +571,7 @@ exit: return; } -void SubMac::UpdateFrameCounterOnTxDone(const TxFrame &aFrame) +void SubMac::SignalFrameCounterUsedOnTxDone(const TxFrame &aFrame) { uint8_t keyIdMode; uint32_t frameCounter; @@ -598,7 +598,7 @@ void SubMac::UpdateFrameCounterOnTxDone(const TxFrame &aFrame) VerifyOrExit(keyIdMode == Frame::kKeyIdMode1); VerifyOrExit(aFrame.GetFrameCounter(frameCounter) == kErrorNone, OT_ASSERT(allowError)); - UpdateFrameCounter(frameCounter); + SignalFrameCounterUsed(frameCounter); exit: return; @@ -876,11 +876,23 @@ exit: return; } -void SubMac::UpdateFrameCounter(uint32_t aFrameCounter) +void SubMac::SignalFrameCounterUsed(uint32_t aFrameCounter) { - mFrameCounter = aFrameCounter; + mCallbacks.FrameCounterUsed(aFrameCounter); - mCallbacks.FrameCounterUpdated(aFrameCounter); + // It not always guaranteed that this method is invoked in order + // for different counter values (i.e., we may get it for a + // smaller counter value after a lager one). This may happen due + // to a new counter value being used for an enhanced-ack during + // tx of a frame. Note that the newer counter used for enhanced-ack + // is processed from `HandleReceiveDone()` which can happen before + // processing of the older counter value from `HandleTransmitDone()`. + + VerifyOrExit(mFrameCounter <= aFrameCounter); + mFrameCounter = aFrameCounter + 1; + +exit: + return; } void SubMac::SetFrameCounter(uint32_t aFrameCounter) diff --git a/src/core/mac/sub_mac.hpp b/src/core/mac/sub_mac.hpp index 9281e6fdb..64b7a3aa3 100644 --- a/src/core/mac/sub_mac.hpp +++ b/src/core/mac/sub_mac.hpp @@ -195,12 +195,15 @@ public: void EnergyScanDone(int8_t aMaxRssi); /** - * This method notifies user of `SubMac` that MAC frame counter is updated. + * This method notifies user of `SubMac` that a specific MAC frame counter is used for transmission. * - * @param[in] aFrameCounter The MAC frame counter value. + * It is possible that this callback is invoked out of order in terms of counter values (i.e., called for a + * smaller counter value after a call for a larger counter value). + * + * @param[in] aFrameCounter The MAC frame counter value which was used. * */ - void FrameCounterUpdated(uint32_t aFrameCounter); + void FrameCounterUsed(uint32_t aFrameCounter); }; /** @@ -622,7 +625,7 @@ private: bool ShouldHandleTransmitTargetTime(void) const; void ProcessTransmitSecurity(void); - void UpdateFrameCounter(uint32_t aFrameCounter); + void SignalFrameCounterUsed(uint32_t aFrameCounter); void StartCsmaBackoff(void); void BeginTransmit(void); void SampleRssi(void); @@ -630,7 +633,7 @@ private: void HandleReceiveDone(RxFrame *aFrame, Error aError); void HandleTransmitStarted(TxFrame &aFrame); void HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError); - void UpdateFrameCounterOnTxDone(const TxFrame &aFrame); + void SignalFrameCounterUsedOnTxDone(const TxFrame &aFrame); void HandleEnergyScanDone(int8_t aMaxRssi); static void HandleTimer(Timer &aTimer); diff --git a/src/core/mac/sub_mac_callbacks.cpp b/src/core/mac/sub_mac_callbacks.cpp index 35bb64ddc..b61ad283f 100644 --- a/src/core/mac/sub_mac_callbacks.cpp +++ b/src/core/mac/sub_mac_callbacks.cpp @@ -107,9 +107,9 @@ void SubMac::Callbacks::EnergyScanDone(int8_t aMaxRssi) } } -void SubMac::Callbacks::FrameCounterUpdated(uint32_t aFrameCounter) +void SubMac::Callbacks::FrameCounterUsed(uint32_t aFrameCounter) { - Get().MacFrameCounterUpdated(aFrameCounter); + Get().MacFrameCounterUsed(aFrameCounter); } #elif OPENTHREAD_RADIO @@ -142,7 +142,7 @@ void SubMac::Callbacks::EnergyScanDone(int8_t aMaxRssi) Get().InvokeEnergyScanDone(aMaxRssi); } -void SubMac::Callbacks::FrameCounterUpdated(uint32_t aFrameCounter) +void SubMac::Callbacks::FrameCounterUsed(uint32_t aFrameCounter) { OT_UNUSED_VARIABLE(aFrameCounter); } diff --git a/src/core/thread/key_manager.cpp b/src/core/thread/key_manager.cpp index 6d9a7d850..7ac5bfbdf 100644 --- a/src/core/thread/key_manager.cpp +++ b/src/core/thread/key_manager.cpp @@ -429,17 +429,26 @@ void KeyManager::SetAllMacFrameCounters(uint32_t aMacFrameCounter) } #if OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE -void KeyManager::MacFrameCounterUpdated(uint32_t aMacFrameCounter) +void KeyManager::MacFrameCounterUsed(uint32_t aMacFrameCounter) { - mMacFrameCounters.Set154(aMacFrameCounter); + // This is callback from `SubMac` to indicate that a frame + // counter value is used for tx. We ensure to handle it + // even if it is called out of order. + + VerifyOrExit(mMacFrameCounters.Get154() <= aMacFrameCounter); + + mMacFrameCounters.Set154(aMacFrameCounter + 1); if (mMacFrameCounters.Get154() >= mStoredMacFrameCounter) { IgnoreError(Get().Store()); } + +exit: + return; } #else -void KeyManager::MacFrameCounterUpdated(uint32_t) +void KeyManager::MacFrameCounterUsed(uint32_t) { } #endif diff --git a/src/core/thread/key_manager.hpp b/src/core/thread/key_manager.hpp index 407eab023..c1b77f553 100644 --- a/src/core/thread/key_manager.hpp +++ b/src/core/thread/key_manager.hpp @@ -541,12 +541,14 @@ public: void UpdateKeyMaterial(void); /** - * This method handles MAC frame counter change (callback from `SubMac` for 15.4 security frame change) + * This method handles MAC frame counter changes (callback from `SubMac` for 15.4 security frame change). + * + * This is called to indicate the @p aMacFrameCounter value is now used. * * @param[in] aMacFrameCounter The 15.4 link MAC frame counter value. * */ - void MacFrameCounterUpdated(uint32_t aMacFrameCounter); + void MacFrameCounterUsed(uint32_t aMacFrameCounter); private: static constexpr uint32_t kDefaultKeySwitchGuardTime = 624;