Skip to content

Commit

Permalink
[spinel] do not update the sent frame if the frame has been updated b…
Browse files Browse the repository at this point in the history
…efore sending (openthread#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.
  • Loading branch information
zhanglongxia authored Dec 11, 2024
1 parent b92bd46 commit c427043
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 9 deletions.
3 changes: 2 additions & 1 deletion src/core/diags/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Done

### diag frame

Usage: `diag frame [-b MaxCsmaBackoffs] [-c] [-C RxChannelAfterTxDone] [-d TxDelay] [-p TxPower] [-r MaxFrameRetries] [-s] <frame>`
Usage: `diag frame [-b MaxCsmaBackoffs] [-c] [-C RxChannelAfterTxDone] [-d TxDelay] [-p TxPower] [-r MaxFrameRetries] [-s] [-u] <frame>`

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`.

Expand All @@ -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
Expand Down
16 changes: 15 additions & 1 deletion src/core/diags/factory_diags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
{
Expand All @@ -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:
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions src/core/diags/factory_diags.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,10 @@ class Diags : public InstanceLocator, private NonCopyable
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;
Expand Down
8 changes: 4 additions & 4 deletions src/lib/spinel/radio_spinel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1583,10 +1583,8 @@ void RadioSpinel::HandleTransmitDone(uint32_t aCommand,
error = SpinelStatusToOtError(status);
}

static_cast<Mac::TxFrame *>(mTransmitFrame)->SetIsHeaderUpdated(headerUpdated);

if ((sRadioCaps & OT_RADIO_CAPS_TRANSMIT_SEC) && headerUpdated &&
static_cast<Mac::TxFrame *>(mTransmitFrame)->GetSecurityEnabled())
if ((sRadioCaps & OT_RADIO_CAPS_TRANSMIT_SEC) && (!mTransmitFrame->mInfo.mTxInfo.mIsHeaderUpdated) &&
headerUpdated && static_cast<Mac::TxFrame *>(mTransmitFrame)->GetSecurityEnabled())
{
uint8_t keyId;
uint32_t frameCounter;
Expand All @@ -1603,6 +1601,8 @@ void RadioSpinel::HandleTransmitDone(uint32_t aCommand,
#endif
}

static_cast<Mac::TxFrame *>(mTransmitFrame)->SetIsHeaderUpdated(headerUpdated);

exit:
// A parse error indicates an RCP misbehavior, so recover the RCP immediately.
mState = kStateTransmitDone;
Expand Down

0 comments on commit c427043

Please sign in to comment.