Skip to content

Commit d1a1509

Browse files
authored
[mac] simplify and refactor HandleTransmitDone() (openthread#13336)
This commit refactors `Mac::HandleTransmitDone()` by extracting link-layer and multi-radio post-tx processing into two focused helper methods: - `ProcessTxDone()`: Handles IEEE 802.15.4 link-level post-transmission actions, including broadcast frame re-transmissions, Enh-ACK security and MIC verification, MAC address filtering, and updating Neighbor table link info and CSL metrics. - `ProcessMultiRadioTxDone()`: Handles multi-radio link completion coordination (`#if OPENTHREAD_CONFIG_MULTI_RADIO`), updating the `RadioSelector` and tracking pending radio links. This change replaces deeply nested conditional blocks and preprocessor guards with simpler, linear `VerifyOrExit()` and `SuccessOrExit()` early exits. Flattening the control flow removes multiple layers of indentation, making `TxDone` logic much more readable and easier to follow. Both methods separate execution flow control from the frame transmission outcome: - The return value (`Error`) controls execution flow in `HandleTransmitDone()`. Returning `kErrorPending` indicates that the overall tx is not yet finished (due to a scheduled broadcast frame retransmission or pending multi-radio link), deferring MAC state machine advancement (`switch (mOperation)`). - The `aError` reference parameter captures the frame transmission status (`kErrorNone`, `kErrorNoAck`, etc.) and can be updated by (e.g., if Enh-ACK security verification or MAC address filtering fails).
1 parent 7eac857 commit d1a1509

2 files changed

Lines changed: 143 additions & 85 deletions

File tree

src/core/mac/mac.cpp

Lines changed: 137 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,130 +1239,182 @@ void Mac::RecordFrameTransmitStatus(const TxFrame &aFrame, Error aError, uint8_t
12391239
return;
12401240
}
12411241

1242-
void Mac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError)
1242+
#if OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE
1243+
1244+
Error Mac::ProcessTxDone(TxFrame &aFrame, RxFrame *aAckFrame, Error &aError)
12431245
{
1244-
bool ackRequested = aFrame.GetAckRequest();
1246+
// Process post-transmission actions on IEEE 802.15.4 link
1247+
// (handling broadcast retransmissions and ACK processing).
1248+
//
1249+
// Returns `kErrorPending` if a broadcast frame is scheduled for
1250+
// retransmission (indicating overall frame transmission is not yet
1251+
// finished). Returns `kErrorNone` otherwise.
1252+
//
1253+
// May update `aError` (e.g., setting it to `kErrorNoAck` if Enh-ACK
1254+
// security or MAC filter checks fail).
1255+
1256+
Error error = kErrorNone;
1257+
Address dstAddr;
1258+
Neighbor *neighbor;
1259+
1260+
VerifyOrExit(!aFrame.IsEmpty());
12451261

1246-
#if OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE
1247-
if (!aFrame.IsEmpty()
12481262
#if OPENTHREAD_CONFIG_MULTI_RADIO
1249-
&& (aFrame.GetRadioType() == kRadioTypeIeee802154)
1263+
VerifyOrExit(aFrame.GetRadioType() == kRadioTypeIeee802154);
12501264
#endif
1251-
)
1252-
{
1253-
Address dstAddr;
1265+
IgnoreError(aFrame.GetDstAddr(dstAddr));
12541266

1255-
IgnoreError(aFrame.GetDstAddr(dstAddr));
1267+
// Determine whether to re-transmit a broadcast frame.
12561268

1257-
// Determine whether to re-transmit a broadcast frame.
1258-
if (dstAddr.IsBroadcast())
1259-
{
1260-
mBroadcastTransmitCount++;
1269+
if (dstAddr.IsBroadcast())
1270+
{
1271+
mBroadcastTransmitCount++;
12611272

1262-
if (mBroadcastTransmitCount < kTxNumBcast)
1263-
{
1273+
if (mBroadcastTransmitCount < kTxNumBcast)
1274+
{
12641275
#if OPENTHREAD_CONFIG_MULTI_RADIO
1265-
{
1266-
RadioTypes radioTypes;
1267-
radioTypes.Add(kRadioTypeIeee802154);
1268-
mLinks.Send(aFrame, radioTypes);
1269-
}
1276+
{
1277+
RadioTypes radioTypes;
1278+
radioTypes.Add(kRadioTypeIeee802154);
1279+
mLinks.Send(aFrame, radioTypes);
1280+
}
12701281
#else
1271-
mLinks.Send();
1282+
mLinks.Send();
12721283
#endif
1273-
ExitNow();
1274-
}
1275-
1276-
mBroadcastTransmitCount = 0;
1284+
ExitNow(error = kErrorPending);
12771285
}
12781286

1279-
if (ackRequested && (aAckFrame != nullptr))
1280-
{
1281-
Neighbor *neighbor = Get<NeighborTable>().FindNeighbor(dstAddr);
1287+
mBroadcastTransmitCount = 0;
1288+
}
1289+
1290+
// If an ACK was requested and received, process the ACK frame
1291+
// (verifying MAC filter, Enh-ACK security, and updating
1292+
// neighbor link info and CSL).
1293+
1294+
VerifyOrExit(aFrame.GetAckRequest() && (aAckFrame != nullptr));
1295+
1296+
SuccessOrExit(aError);
1297+
1298+
neighbor = Get<NeighborTable>().FindNeighbor(dstAddr);
12821299

12831300
#if OPENTHREAD_CONFIG_MAC_FILTER_ENABLE
1284-
if ((aError == kErrorNone) && (neighbor != nullptr) &&
1285-
(mFilter.ApplyToRxFrame(*aAckFrame, neighbor->GetExtAddress(), neighbor) != kErrorNone))
1286-
{
1287-
aError = kErrorNoAck;
1288-
}
1301+
if ((neighbor != nullptr) && mFilter.ApplyToRxFrame(*aAckFrame, neighbor->GetExtAddress(), neighbor) != kErrorNone)
1302+
{
1303+
aError = kErrorNoAck;
1304+
ExitNow();
1305+
}
12891306
#endif
12901307

12911308
#if OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_2
1292-
// Verify Enh-ACK integrity by checking its MIC
1293-
if ((aError == kErrorNone) && (ProcessEnhAckSecurity(aFrame, *aAckFrame) != kErrorNone))
1294-
{
1295-
aError = kErrorNoAck;
1296-
}
1309+
if (ProcessEnhAckSecurity(aFrame, *aAckFrame) != kErrorNone)
1310+
{
1311+
aError = kErrorNoAck;
1312+
ExitNow();
1313+
}
12971314
#endif
12981315

1299-
if ((aError == kErrorNone) && (neighbor != nullptr))
1300-
{
1301-
UpdateNeighborLinkInfo(*neighbor, *aAckFrame);
1316+
VerifyOrExit(neighbor != nullptr);
1317+
1318+
UpdateNeighborLinkInfo(*neighbor, *aAckFrame);
13021319

13031320
#if OPENTHREAD_CONFIG_MLE_LINK_METRICS_INITIATOR_ENABLE
1304-
ProcessEnhAckProbing(*aAckFrame, *neighbor);
1321+
ProcessEnhAckProbing(*aAckFrame, *neighbor);
13051322
#endif
13061323
#if OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
1307-
ProcessCsl(*aAckFrame, dstAddr);
1324+
ProcessCsl(*aAckFrame, dstAddr);
13081325
#endif
13091326
#if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE
1310-
if (!mRxOnWhenIdle && aFrame.Has<CslIe>())
1311-
{
1312-
Get<DataPollSender>().ResetKeepAliveTimer();
1313-
}
1314-
#endif
1315-
}
1316-
}
1327+
if (!mRxOnWhenIdle && aFrame.Has<CslIe>())
1328+
{
1329+
Get<DataPollSender>().ResetKeepAliveTimer();
13171330
}
1331+
#endif
1332+
1333+
exit:
1334+
return error;
1335+
}
1336+
13181337
#endif // OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE
13191338

13201339
#if OPENTHREAD_CONFIG_MULTI_RADIO
1321-
if (!aFrame.IsEmpty())
1322-
{
1323-
RadioType radio = aFrame.GetRadioType();
1324-
RadioTypes requiredRadios = mLinks.GetTxFramesRequiredRadioTypes();
13251340

1326-
Get<RadioSelector>().UpdateOnSendDone(aFrame, aError);
1341+
Error Mac::ProcessMultiRadioTxDone(TxFrame &aFrame, Error &aError)
1342+
{
1343+
// Process post-transmission actions under multi-radio config
1344+
// (updating radio selector and tracking transmission across
1345+
// multiple radio links).
1346+
//
1347+
// Returns `kErrorPending` if transmissions on other radio links are
1348+
// still pending. Returns `kErrorNone` once all radio links have
1349+
// completed and updates `aError` with the overall transmission
1350+
// result (`mTxError`).
13271351

1328-
if (requiredRadios.IsEmpty())
1329-
{
1330-
// If the "required radio type set" is empty, successful
1331-
// tx over any radio link is sufficient for overall tx to
1332-
// be considered successful. In this case `mTxError`
1333-
// starts as `kErrorAbort` and we update it only when
1334-
// it is not already `kErrorNone`.
1352+
Error error = kErrorNone;
1353+
RadioType radio;
1354+
RadioTypes requiredRadios;
13351355

1336-
if (mTxError != kErrorNone)
1337-
{
1338-
mTxError = aError;
1339-
}
1340-
}
1341-
else
1342-
{
1343-
// When the "required radio type set" is not empty we
1344-
// expect the successful frame tx on all links in this set
1345-
// to consider the overall tx successful. In this case,
1346-
// `mTxError` starts as `kErrorNone` and we update it
1347-
// if tx over any link in the set fails.
1356+
VerifyOrExit(!aFrame.IsEmpty());
13481357

1349-
if (requiredRadios.Contains(radio) && (aError != kErrorNone))
1350-
{
1351-
LogDebgOnError(aError, "tx frame on required radio link %s", RadioTypeToString(radio));
1352-
mTxError = aError;
1353-
}
1358+
radio = aFrame.GetRadioType();
1359+
requiredRadios = mLinks.GetTxFramesRequiredRadioTypes();
1360+
1361+
Get<RadioSelector>().UpdateOnSendDone(aFrame, aError);
1362+
1363+
if (requiredRadios.IsEmpty())
1364+
{
1365+
// If the "required radio type set" is empty, successful
1366+
// tx over any radio link is sufficient for overall tx to
1367+
// be considered successful. In this case `mTxError`
1368+
// starts as `kErrorAbort` and we update it only when
1369+
// it is not already `kErrorNone`.
1370+
1371+
if (mTxError != kErrorNone)
1372+
{
1373+
mTxError = aError;
13541374
}
1375+
}
1376+
else
1377+
{
1378+
// When the "required radio type set" is not empty we
1379+
// expect the successful frame tx on all links in this set
1380+
// to consider the overall tx successful. In this case,
1381+
// `mTxError` starts as `kErrorNone` and we update it
1382+
// if tx over any link in the set fails.
13551383

1356-
// Keep track of radio links on which the frame is sent
1357-
// and wait for all radio links to finish.
1358-
mTxPendingRadioLinks.Remove(radio);
1384+
if (requiredRadios.Contains(radio) && (aError != kErrorNone))
1385+
{
1386+
LogDebgOnError(aError, "tx frame on required radio link %s", RadioTypeToString(radio));
1387+
mTxError = aError;
1388+
}
1389+
}
13591390

1360-
VerifyOrExit(mTxPendingRadioLinks.IsEmpty());
1391+
// Keep track of radio links on which the frame is sent
1392+
// and wait for all radio links to finish.
1393+
mTxPendingRadioLinks.Remove(radio);
13611394

1362-
aError = mTxError;
1395+
if (!mTxPendingRadioLinks.IsEmpty())
1396+
{
1397+
ExitNow(error = kErrorPending);
13631398
}
1399+
1400+
aError = mTxError;
1401+
1402+
exit:
1403+
return error;
1404+
}
1405+
13641406
#endif // OPENTHREAD_CONFIG_MULTI_RADIO
13651407

1408+
void Mac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError)
1409+
{
1410+
#if OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE
1411+
SuccessOrExit(ProcessTxDone(aFrame, aAckFrame, aError));
1412+
#endif
1413+
1414+
#if OPENTHREAD_CONFIG_MULTI_RADIO
1415+
SuccessOrExit(ProcessMultiRadioTxDone(aFrame, aError));
1416+
#endif
1417+
13661418
// Determine next action based on current operation.
13671419

13681420
switch (mOperation)
@@ -1379,7 +1431,7 @@ void Mac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError)
13791431
break;
13801432

13811433
case kOperationTransmitPoll:
1382-
OT_ASSERT(aFrame.IsEmpty() || ackRequested);
1434+
OT_ASSERT(aFrame.IsEmpty() || aFrame.GetAckRequest());
13831435

13841436
if ((aError == kErrorNone) && (aAckFrame != nullptr))
13851437
{

src/core/mac/mac.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,12 @@ class Mac : public InstanceLocator, private NonCopyable
843843
void UpdateNeighborLinkInfo(Neighbor &aNeighbor, const RxFrame &aRxFrame);
844844
bool HandleMacCommand(RxFrame &aFrame);
845845
void HandleTimer(void);
846+
#if OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE
847+
Error ProcessTxDone(TxFrame &aFrame, RxFrame *aAckFrame, Error &aError);
848+
#endif
849+
#if OPENTHREAD_CONFIG_MULTI_RADIO
850+
Error ProcessMultiRadioTxDone(TxFrame &aFrame, Error &aError);
851+
#endif
846852

847853
void Scan(Operation aScanOperation, uint32_t aScanChannels, uint16_t aScanDuration);
848854
Error UpdateScanChannel(void);

0 commit comments

Comments
 (0)