diff --git a/examples/platforms/simulation/radio.c b/examples/platforms/simulation/radio.c index 7874ad5a5..4bf353ae8 100644 --- a/examples/platforms/simulation/radio.c +++ b/examples/platforms/simulation/radio.c @@ -643,7 +643,7 @@ static otError radioProcessTransmitSecurity(otRadioFrame *aFrame) aFrame->mInfo.mTxInfo.mAesKey = key; - if (!aFrame->mInfo.mTxInfo.mIsARetx) + if (!aFrame->mInfo.mTxInfo.mIsHeaderUpdated) { otMacFrameSetKeyId(aFrame, keyId); otMacFrameSetFrameCounter(aFrame, sMacFrameCounter++); @@ -678,7 +678,7 @@ void radioSendMessage(otInstance *aInstance) #endif // OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT && OPENTHREAD_CONFIG_TIME_SYNC_ENABLE #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE - if (sCslPeriod > 0 && !sTransmitFrame.mInfo.mTxInfo.mIsARetx) + if (sCslPeriod > 0 && !sTransmitFrame.mInfo.mTxInfo.mIsHeaderUpdated) { otMacFrameSetCslIe(&sTransmitFrame, (uint16_t)sCslPeriod, getCslPhase()); } diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 498ba5c54..1a1477f28 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 (144) +#define OPENTHREAD_API_VERSION (145) /** * @addtogroup api-instance diff --git a/include/openthread/platform/radio.h b/include/openthread/platform/radio.h index f0b21c378..822d91184 100644 --- a/include/openthread/platform/radio.h +++ b/include/openthread/platform/radio.h @@ -239,24 +239,28 @@ typedef struct otRadioFrame uint8_t mMaxFrameRetries; ///< Maximum number of retries allowed after a transmission failure. /** - * Indicates whether the frame is a retransmission or not. + * Indicates whether frame counter and CSL IEs are properly updated in the header. * * If the platform layer does not provide `OT_RADIO_CAPS_TRANSMIT_SEC` capability, it can ignore this flag. * * If the platform provides `OT_RADIO_CAPS_TRANSMIT_SEC` capability, then platform is expected to handle tx * security processing and assignment of frame counter. In this case the following behavior is expected: * - * When `mIsARetx` is set, it indicates that OpenThread core has already set the frame counter and key id - * (if security is enabled) in the prepared frame. The counter is ensured to match the counter value from - * the previous attempts of the same frame. The platform should not assign or change the frame counter (but - * may still need to perform security processing depending on `mIsSecurityProcessed` flag). + * When `mIsHeaderUpdated` is set, it indicates that OpenThread core has already set the frame counter and + * CSL IEs (if security is enabled) in the prepared frame. The counter is ensured to match the counter value + * from the previous attempts of the same frame. The platform should not assign or change the frame counter + * (but may still need to perform security processing depending on `mIsSecurityProcessed` flag). * - * If `mIsARetx` is not set, then the frame counter and key id are not set in the frame by OpenThread core - * and it is the responsibility of the radio platform to assign them. The platform should update the frame - * (assign counter and key id) even if the transmission gets aborted or fails (e.g., channel access error). + * If `mIsHeaderUpdated` is not set, then the frame counter and key CSL IE not set in the frame by + * OpenThread core and it is the responsibility of the radio platform to assign them. The platform + * must update the frame header (assign counter and CSL IE values) before sending the frame over the air, + * however if the the transmission gets aborted and the frame is never sent over the air (e.g., channel + * access error) the platform may choose to not update the header. If the platform updates the header, + * it must also set this flag before passing the frame back from the `otPlatRadioTxDone()` callback. * */ - bool mIsARetx : 1; + bool mIsHeaderUpdated : 1; + bool mIsARetx : 1; ///< Indicates whether the frame is a retransmission or not. bool mCsmaCaEnabled : 1; ///< Set to true to enable CSMA-CA for this packet, false otherwise. bool mCslPresent : 1; ///< Set to true if CSL header IE is present. bool mIsSecurityProcessed : 1; ///< True if SubMac should skip the AES processing of this frame. diff --git a/src/core/mac/data_poll_handler.cpp b/src/core/mac/data_poll_handler.cpp index f4d61eac8..81299cbff 100644 --- a/src/core/mac/data_poll_handler.cpp +++ b/src/core/mac/data_poll_handler.cpp @@ -242,8 +242,9 @@ void DataPollHandler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError, break; case kErrorNoAck: - aChild.IncrementIndirectTxAttempts(); + OT_ASSERT(!aFrame.GetSecurityEnabled() || aFrame.IsHeaderUpdated()); + aChild.IncrementIndirectTxAttempts(); otLogInfoMac("Indirect tx to child %04x failed, attempt %d/%d", aChild.GetRloc16(), aChild.GetIndirectTxAttempts(), kMaxPollTriggeredTxAttempts); @@ -268,7 +269,7 @@ void DataPollHandler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError, aChild.SetIndirectDataSequenceNumber(aFrame.GetSequence()); - if (aFrame.GetSecurityEnabled()) + if (aFrame.GetSecurityEnabled() && aFrame.IsHeaderUpdated()) { uint32_t frameCounter; uint8_t keyId; diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp index 59a60aebf..5a74c3ad3 100644 --- a/src/core/mac/mac.cpp +++ b/src/core/mac/mac.cpp @@ -950,7 +950,7 @@ void Mac::ProcessTransmitSecurity(TxFrame &aFrame) aFrame.SetAesKey(keyManager.GetKek()); extAddress = &GetExtAddress(); - if (!aFrame.IsARetransmission()) + if (!aFrame.IsHeaderUpdated()) { aFrame.SetFrameCounter(keyManager.GetKekFrameCounter()); keyManager.IncrementKekFrameCounter(); @@ -974,13 +974,13 @@ void Mac::ProcessTransmitSecurity(TxFrame &aFrame) aFrame.SetAesKey(*mLinks.GetCurrentMacKey(aFrame)); extAddress = &GetExtAddress(); - // If the frame is marked as a retransmission, `MeshForwarder` which + // If the frame header is marked as updated, `MeshForwarder` which // prepared the frame should set the frame counter and key id to the - // same values used in the earlier transmit attempt. For a new frame (not - // a retransmission), we get a new frame counter and key id from the key + // same values used in the earlier transmit attempt. For a new frame (header + // not updated), we get a new frame counter and key id from the key // manager. - if (!aFrame.IsARetransmission()) + if (!aFrame.IsHeaderUpdated()) { mLinks.SetMacFrameCounter(aFrame); aFrame.SetKeyId((keyManager.GetCurrentKeySequence() & 0x7f) + 1); @@ -1086,7 +1086,7 @@ void Mac::BeginTransmit(void) frame = Get().HandleFrameRequest(txFrames); VerifyOrExit(frame != nullptr); - // If the frame is marked as a retransmission, then data sequence number is already set. + // If the frame is marked as retransmission, then data sequence number is already set. if (!frame->IsARetransmission()) { frame->SetSequence(mDataSequence++); @@ -1100,7 +1100,7 @@ void Mac::BeginTransmit(void) frame = Get().HandleFrameRequest(txFrames); VerifyOrExit(frame != nullptr); - // If the frame is marked as a retransmission, then data sequence number is already set. + // If the frame is marked as retransmission, then data sequence number is already set. if (!frame->IsARetransmission()) { frame->SetSequence(mDataSequence++); diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp index cd1be5b10..c51e52336 100644 --- a/src/core/mac/mac_frame.cpp +++ b/src/core/mac/mac_frame.cpp @@ -509,6 +509,8 @@ void Frame::SetFrameCounter(uint32_t aFrameCounter) index += kSecurityControlSize; WriteUint32(aFrameCounter, &mPsdu[index]); + + static_cast(this)->SetIsHeaderUpdated(true); } const uint8_t *Frame::GetKeySource(void) const diff --git a/src/core/mac/mac_frame.hpp b/src/core/mac/mac_frame.hpp index ec111a76f..4fbaab821 100644 --- a/src/core/mac/mac_frame.hpp +++ b/src/core/mac/mac_frame.hpp @@ -1336,6 +1336,23 @@ public: mInfo.mTxInfo.mIsSecurityProcessed = aIsSecurityProcessed; } + /** + * This method indicates whether or not the frame header is updated. + * + * @retval TRUE The frame already has the header updated. + * @retval FALSE The frame does not have the header updated. + * + */ + bool IsHeaderUpdated(void) const { return mInfo.mTxInfo.mIsHeaderUpdated; } + + /** + * This method sets the header updated flag attribute. + * + * @param[in] aIsHeaderUpdated TRUE if the frame header is updated. + * + */ + void SetIsHeaderUpdated(bool aIsHeaderUpdated) { mInfo.mTxInfo.mIsHeaderUpdated = aIsHeaderUpdated; } + #if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE /** * This method sets the Time IE offset. diff --git a/src/core/mac/mac_links.hpp b/src/core/mac/mac_links.hpp index 967c2227c..21b0694e3 100644 --- a/src/core/mac/mac_links.hpp +++ b/src/core/mac/mac_links.hpp @@ -180,6 +180,7 @@ public: mTxFrame802154.SetIsARetransmission(false); mTxFrame802154.SetIsSecurityProcessed(false); mTxFrame802154.SetCsmaCaEnabled(true); // Set to true by default, only set to `false` for CSL transmission + mTxFrame802154.SetIsHeaderUpdated(false); #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE mTxFrame802154.SetTxDelay(0); mTxFrame802154.SetTxDelayBaseTime(0); @@ -190,6 +191,7 @@ public: mTxFrameTrel.SetIsARetransmission(false); mTxFrameTrel.SetIsSecurityProcessed(false); mTxFrameTrel.SetCsmaCaEnabled(true); + mTxFrameTrel.SetIsHeaderUpdated(false); #endif #if OPENTHREAD_CONFIG_MULTI_RADIO diff --git a/src/core/mac/sub_mac.cpp b/src/core/mac/sub_mac.cpp index a84696235..0217a2d50 100644 --- a/src/core/mac/sub_mac.cpp +++ b/src/core/mac/sub_mac.cpp @@ -333,7 +333,7 @@ void SubMac::ProcessTransmitSecurity(void) SuccessOrExit(mTransmitFrame.GetKeyIdMode(keyIdMode)); - if (!mTransmitFrame.IsARetransmission()) + if (!mTransmitFrame.IsHeaderUpdated()) { mTransmitFrame.SetKeyId(mKeyId); } @@ -343,7 +343,7 @@ void SubMac::ProcessTransmitSecurity(void) mTransmitFrame.SetAesKey(GetCurrentMacKey()); - if (!mTransmitFrame.IsARetransmission()) + if (!mTransmitFrame.IsHeaderUpdated()) { uint32_t frameCounter = GetFrameCounter(); diff --git a/src/core/thread/csl_tx_scheduler.cpp b/src/core/thread/csl_tx_scheduler.cpp index b70a895f0..552a429ed 100644 --- a/src/core/thread/csl_tx_scheduler.cpp +++ b/src/core/thread/csl_tx_scheduler.cpp @@ -249,8 +249,9 @@ void CslTxScheduler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError, C break; case kErrorNoAck: - aChild.IncrementCslTxAttempts(); + OT_ASSERT(!aFrame.GetSecurityEnabled() || aFrame.IsHeaderUpdated()); + aChild.IncrementCslTxAttempts(); otLogInfoMac("CSL tx to child %04x failed, attempt %d/%d", aChild.GetRloc16(), aChild.GetCslTxAttempts(), kMaxCslTriggeredTxAttempts); @@ -274,7 +275,7 @@ void CslTxScheduler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError, C { aChild.SetIndirectDataSequenceNumber(aFrame.GetSequence()); - if (aFrame.GetSecurityEnabled()) + if (aFrame.GetSecurityEnabled() && aFrame.IsHeaderUpdated()) { uint32_t frameCounter; uint8_t keyId; diff --git a/src/lib/spinel/radio_spinel_impl.hpp b/src/lib/spinel/radio_spinel_impl.hpp index dfeee4b55..29ee3d874 100644 --- a/src/lib/spinel/radio_spinel_impl.hpp +++ b/src/lib/spinel/radio_spinel_impl.hpp @@ -1843,9 +1843,10 @@ void RadioSpinel::HandleTransmitDone(uint32_t const uint8_t * aBuffer, uint16_t aLength) { - otError error = OT_ERROR_NONE; - spinel_status_t status = SPINEL_STATUS_OK; - bool framePending = false; + otError error = OT_ERROR_NONE; + spinel_status_t status = SPINEL_STATUS_OK; + bool framePending = false; + bool headerUpdated = false; spinel_ssize_t unpacked; VerifyOrExit(aCommand == SPINEL_CMD_PROP_VALUE_IS && aKey == SPINEL_PROP_LAST_STATUS, error = OT_ERROR_FAILED); @@ -1862,6 +1863,12 @@ void RadioSpinel::HandleTransmitDone(uint32_t aBuffer += unpacked; aLength -= static_cast(unpacked); + unpacked = spinel_datatype_unpack(aBuffer, aLength, SPINEL_DATATYPE_BOOL_S, &headerUpdated); + VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); + + aBuffer += unpacked; + aLength -= static_cast(unpacked); + if (status == SPINEL_STATUS_OK) { SuccessOrExit(error = ParseRadioFrame(mAckRadioFrame, aBuffer, aLength, unpacked)); @@ -1873,7 +1880,10 @@ void RadioSpinel::HandleTransmitDone(uint32_t error = SpinelStatusToOtError(status); } - if ((mRadioCaps & OT_RADIO_CAPS_TRANSMIT_SEC) && static_cast(mTransmitFrame)->GetSecurityEnabled()) + static_cast(mTransmitFrame)->SetIsHeaderUpdated(headerUpdated); + + if ((mRadioCaps & OT_RADIO_CAPS_TRANSMIT_SEC) && headerUpdated && + static_cast(mTransmitFrame)->GetSecurityEnabled()) { uint8_t keyId; uint32_t frameCounter; @@ -1905,20 +1915,21 @@ otError RadioSpinel::Transmit(otRadioFrame &a otPlatRadioTxStarted(mInstance, mTransmitFrame); error = Request(SPINEL_CMD_PROP_VALUE_SET, SPINEL_PROP_STREAM_RAW, - SPINEL_DATATYPE_DATA_WLEN_S // Frame data - SPINEL_DATATYPE_UINT8_S // Channel - SPINEL_DATATYPE_UINT8_S // MaxCsmaBackoffs - SPINEL_DATATYPE_UINT8_S // MaxFrameRetries - SPINEL_DATATYPE_BOOL_S // CsmaCaEnabled - SPINEL_DATATYPE_BOOL_S // IsARetx - SPINEL_DATATYPE_BOOL_S // SkipAes - SPINEL_DATATYPE_UINT32_S // TxDelay - SPINEL_DATATYPE_UINT32_S, // TxDelayBaseTime + SPINEL_DATATYPE_DATA_WLEN_S // Frame data + SPINEL_DATATYPE_UINT8_S // Channel + SPINEL_DATATYPE_UINT8_S // MaxCsmaBackoffs + SPINEL_DATATYPE_UINT8_S // MaxFrameRetries + SPINEL_DATATYPE_BOOL_S // CsmaCaEnabled + SPINEL_DATATYPE_BOOL_S // IsHeaderUpdated + SPINEL_DATATYPE_BOOL_S // IsARetx + SPINEL_DATATYPE_BOOL_S // SkipAes + SPINEL_DATATYPE_UINT32_S // TxDelay + SPINEL_DATATYPE_UINT32_S, // TxDelayBaseTime mTransmitFrame->mPsdu, mTransmitFrame->mLength, mTransmitFrame->mChannel, mTransmitFrame->mInfo.mTxInfo.mMaxCsmaBackoffs, mTransmitFrame->mInfo.mTxInfo.mMaxFrameRetries, - mTransmitFrame->mInfo.mTxInfo.mCsmaCaEnabled, mTransmitFrame->mInfo.mTxInfo.mIsARetx, - mTransmitFrame->mInfo.mTxInfo.mIsSecurityProcessed, mTransmitFrame->mInfo.mTxInfo.mTxDelay, - mTransmitFrame->mInfo.mTxInfo.mTxDelayBaseTime); + mTransmitFrame->mInfo.mTxInfo.mCsmaCaEnabled, mTransmitFrame->mInfo.mTxInfo.mIsHeaderUpdated, + mTransmitFrame->mInfo.mTxInfo.mIsARetx, mTransmitFrame->mInfo.mTxInfo.mIsSecurityProcessed, + mTransmitFrame->mInfo.mTxInfo.mTxDelay, mTransmitFrame->mInfo.mTxInfo.mTxDelayBaseTime); if (error == OT_ERROR_NONE) { diff --git a/src/lib/spinel/spinel.h b/src/lib/spinel/spinel.h index 9ddf6a969..77f98c785 100644 --- a/src/lib/spinel/spinel.h +++ b/src/lib/spinel/spinel.h @@ -377,7 +377,7 @@ * Please see section "Spinel definition compatibility guideline" for more details. * */ -#define SPINEL_RCP_API_VERSION 3 +#define SPINEL_RCP_API_VERSION 4 /** * @def SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION @@ -389,7 +389,7 @@ * Please see section "Spinel definition compatibility guideline" for more details. * */ -#define SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION 1 +#define SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION 4 /** * @def SPINEL_FRAME_MAX_SIZE diff --git a/src/ncp/ncp_base_radio.cpp b/src/ncp/ncp_base_radio.cpp index 3be08286f..b53a0db2c 100644 --- a/src/ncp/ncp_base_radio.cpp +++ b/src/ncp/ncp_base_radio.cpp @@ -148,8 +148,9 @@ void NcpBase::LinkRawTransmitDone(otRadioFrame *aFrame, otRadioFrame *aAckFrame, if (mCurTransmitTID) { - uint8_t header = SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0 | mCurTransmitTID; - bool framePending = (aAckFrame != nullptr && static_cast(aAckFrame)->GetFramePending()); + uint8_t header = SPINEL_HEADER_FLAG | SPINEL_HEADER_IID_0 | mCurTransmitTID; + bool framePending = (aAckFrame != nullptr && static_cast(aAckFrame)->GetFramePending()); + bool headerUpdated = static_cast(aFrame)->IsHeaderUpdated(); // Clear cached transmit TID mCurTransmitTID = 0; @@ -157,13 +158,14 @@ void NcpBase::LinkRawTransmitDone(otRadioFrame *aFrame, otRadioFrame *aAckFrame, SuccessOrExit(mEncoder.BeginFrame(header, SPINEL_CMD_PROP_VALUE_IS, SPINEL_PROP_LAST_STATUS)); SuccessOrExit(mEncoder.WriteUintPacked(ThreadErrorToSpinelStatus(aError))); SuccessOrExit(mEncoder.WriteBool(framePending)); + SuccessOrExit(mEncoder.WriteBool(headerUpdated)); if (aError == OT_ERROR_NONE) { SuccessOrExit(PackRadioFrame(aAckFrame, aError)); } - if (static_cast(aFrame)->GetSecurityEnabled()) + if (static_cast(aFrame)->GetSecurityEnabled() && headerUpdated) { uint8_t keyId; uint32_t frameCounter; @@ -386,6 +388,7 @@ otError NcpBase::DecodeStreamRawTxRequest(otRadioFrame &aFrame) uint16_t payloadLen; bool csmaEnable; bool isARetx; + bool isHeaderUpdated; bool isSecurityProcessed; SuccessOrExit(error = mDecoder.ReadDataWithLen(payloadPtr, payloadLen)); @@ -403,6 +406,7 @@ otError NcpBase::DecodeStreamRawTxRequest(otRadioFrame &aFrame) aFrame.mInfo.mTxInfo.mMaxCsmaBackoffs = OPENTHREAD_CONFIG_MAC_MAX_CSMA_BACKOFFS_DIRECT; aFrame.mInfo.mTxInfo.mMaxFrameRetries = OPENTHREAD_CONFIG_MAC_DEFAULT_MAX_FRAME_RETRIES_DIRECT; aFrame.mInfo.mTxInfo.mCsmaCaEnabled = true; + aFrame.mInfo.mTxInfo.mIsHeaderUpdated = false; aFrame.mInfo.mTxInfo.mIsARetx = false; aFrame.mInfo.mTxInfo.mIsSecurityProcessed = false; aFrame.mInfo.mTxInfo.mTxDelay = 0; @@ -416,11 +420,13 @@ otError NcpBase::DecodeStreamRawTxRequest(otRadioFrame &aFrame) SuccessOrExit(mDecoder.ReadUint8(aFrame.mInfo.mTxInfo.mMaxCsmaBackoffs)); SuccessOrExit(mDecoder.ReadUint8(aFrame.mInfo.mTxInfo.mMaxFrameRetries)); SuccessOrExit(mDecoder.ReadBool(csmaEnable)); + SuccessOrExit(mDecoder.ReadBool(isHeaderUpdated)); SuccessOrExit(mDecoder.ReadBool(isARetx)); SuccessOrExit(mDecoder.ReadBool(isSecurityProcessed)); SuccessOrExit(mDecoder.ReadUint32(aFrame.mInfo.mTxInfo.mTxDelay)); SuccessOrExit(mDecoder.ReadUint32(aFrame.mInfo.mTxInfo.mTxDelayBaseTime)); aFrame.mInfo.mTxInfo.mCsmaCaEnabled = csmaEnable; + aFrame.mInfo.mTxInfo.mIsHeaderUpdated = isHeaderUpdated; aFrame.mInfo.mTxInfo.mIsARetx = isARetx; aFrame.mInfo.mTxInfo.mIsSecurityProcessed = isSecurityProcessed;