From c427043c5ab1b0e99548eb705e9a3d9010915bf9 Mon Sep 17 00:00:00 2001 From: Zhanglong Xia Date: Thu, 12 Dec 2024 03:04:39 +0800 Subject: [PATCH] [spinel] do not update the sent frame if the frame has been updated before sending (#10976) When running the command `diag frame -s fd874f68f1ca00efbe00adde5d4f4913e953845a154d4cbab10000000001820e390005009bb8ea011c58a065c39fbd` and `diag send 20` to send the specified diag frame, we found that the frame is modified by the RadioSpinel module. Which causes the diag sent frames are not the same. This commit checks whether the frame header has been updated before sending, the RadioSpinel module won't update the frame header if the frame header has been updated before sending. This commit also adds an option `-u` to the `diag frame` command to specify the `mInfo.mTxInfo.mIsHeaderUpdated` field of the diag sent frame. --- src/core/diags/README.md | 3 ++- src/core/diags/factory_diags.cpp | 16 +++++++++++++++- src/core/diags/factory_diags.hpp | 7 ++++--- src/lib/spinel/radio_spinel.cpp | 8 ++++---- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/core/diags/README.md b/src/core/diags/README.md index 95cedf531..595a06e9e 100644 --- a/src/core/diags/README.md +++ b/src/core/diags/README.md @@ -80,7 +80,7 @@ Done ### diag frame -Usage: `diag frame [-b MaxCsmaBackoffs] [-c] [-C RxChannelAfterTxDone] [-d TxDelay] [-p TxPower] [-r MaxFrameRetries] [-s] ` +Usage: `diag frame [-b MaxCsmaBackoffs] [-c] [-C RxChannelAfterTxDone] [-d TxDelay] [-p TxPower] [-r MaxFrameRetries] [-s] [-u] ` Set the frame (hex encoded) to be used by `diag send` and `diag repeat`. The frame may be overwritten by `diag send` and `diag repeat`. @@ -91,6 +91,7 @@ Set the frame (hex encoded) to be used by `diag send` and `diag repeat`. The fra - Specify `-p` to specify the tx power in dBm for this frame. - Specify `-r` to specify the `mInfo.mTxInfo.mMaxFrameRetries` field for this frame. - Specify `-s` to indicate that tx security is already processed thus it should be skipped in the radio layer. +- Specify `-u` to specify the `mInfo.mTxInfo.mIsHeaderUpdated` field for this frame. ```bash > diag frame 11223344 diff --git a/src/core/diags/factory_diags.cpp b/src/core/diags/factory_diags.cpp index 784585bc2..914061b6c 100644 --- a/src/core/diags/factory_diags.cpp +++ b/src/core/diags/factory_diags.cpp @@ -212,6 +212,7 @@ Diags::Diags(Instance &aInstance) void Diags::ResetTxPacket(void) { + mIsHeaderUpdated = false; mTxPacket->mInfo.mTxInfo.mTxDelayBaseTime = 0; mTxPacket->mInfo.mTxInfo.mTxDelay = 0; mTxPacket->mInfo.mTxInfo.mMaxCsmaBackoffs = 0; @@ -231,6 +232,7 @@ Error Diags::ProcessFrame(uint8_t aArgsLength, char *aArgs[]) uint16_t size = OT_RADIO_FRAME_MAX_SIZE; bool securityProcessed = false; bool csmaCaEnabled = false; + bool isHeaderUpdated = false; int8_t txPower = OT_RADIO_POWER_INVALID; uint8_t maxFrameRetries = 0; uint8_t maxCsmaBackoffs = 0; @@ -289,6 +291,11 @@ Error Diags::ProcessFrame(uint8_t aArgsLength, char *aArgs[]) else if (StringMatch(aArgs[0], "-s")) { securityProcessed = true; + isHeaderUpdated = true; + } + else if (StringMatch(aArgs[0], "-u")) + { + isHeaderUpdated = true; } else { @@ -315,6 +322,7 @@ Error Diags::ProcessFrame(uint8_t aArgsLength, char *aArgs[]) mTxPacket->mInfo.mTxInfo.mMaxCsmaBackoffs = maxCsmaBackoffs; mTxPacket->mInfo.mTxInfo.mRxChannelAfterTxDone = rxChannelAfterTxDone; mTxPacket->mLength = size; + mIsHeaderUpdated = isHeaderUpdated; mIsTxPacketSet = true; exit: @@ -556,7 +564,13 @@ void Diags::TransmitPacket(void) { mTxPacket->mChannel = mChannel; - if (!mIsTxPacketSet) + if (mIsTxPacketSet) + { + // The `mInfo.mTxInfo.mIsHeaderUpdated` field may be updated by the radio driver after the frame is sent, + // set the `mInfo.mTxInfo.mIsHeaderUpdated` field before transmitting the frame. + mTxPacket->mInfo.mTxInfo.mIsHeaderUpdated = mIsHeaderUpdated; + } + else { ResetTxPacket(); mTxPacket->mLength = mTxLen; diff --git a/src/core/diags/factory_diags.hpp b/src/core/diags/factory_diags.hpp index 074006202..354ac78c6 100644 --- a/src/core/diags/factory_diags.hpp +++ b/src/core/diags/factory_diags.hpp @@ -217,9 +217,10 @@ private: uint8_t mChannel; int8_t mTxPower; uint8_t mTxLen; - bool mIsTxPacketSet; - bool mRepeatActive; - bool mDiagSendOn; + bool mIsHeaderUpdated : 1; + bool mIsTxPacketSet : 1; + bool mRepeatActive : 1; + bool mDiagSendOn : 1; #endif otDiagOutputCallback mOutputCallback; diff --git a/src/lib/spinel/radio_spinel.cpp b/src/lib/spinel/radio_spinel.cpp index 52fec803d..f467c1f28 100644 --- a/src/lib/spinel/radio_spinel.cpp +++ b/src/lib/spinel/radio_spinel.cpp @@ -1583,10 +1583,8 @@ void RadioSpinel::HandleTransmitDone(uint32_t aCommand, error = SpinelStatusToOtError(status); } - static_cast(mTransmitFrame)->SetIsHeaderUpdated(headerUpdated); - - if ((sRadioCaps & OT_RADIO_CAPS_TRANSMIT_SEC) && headerUpdated && - static_cast(mTransmitFrame)->GetSecurityEnabled()) + if ((sRadioCaps & OT_RADIO_CAPS_TRANSMIT_SEC) && (!mTransmitFrame->mInfo.mTxInfo.mIsHeaderUpdated) && + headerUpdated && static_cast(mTransmitFrame)->GetSecurityEnabled()) { uint8_t keyId; uint32_t frameCounter; @@ -1603,6 +1601,8 @@ void RadioSpinel::HandleTransmitDone(uint32_t aCommand, #endif } + static_cast(mTransmitFrame)->SetIsHeaderUpdated(headerUpdated); + exit: // A parse error indicates an RCP misbehavior, so recover the RCP immediately. mState = kStateTransmitDone;