[mac] simplify retry success histogram tracking (#13319)

This commit refactors MAC retry success histogram tracking to simplify
the code and make array indexing cleaner:

- Encapsulate direct and indirect retry success histogram arrays into a
  dedicated `RetryHistogram` struct inheriting from `Clearable`.
- Add `RecordDirectTx()` and `RecordIndirectTx()` helpers that use
  `Min<uint8_t>(aRetryCount, kMax - 1)` to ensure safe indexing and
  accumulate retries exceeding array bounds into the last bucket.
- Simplify `GetDirectRetrySuccessHistogram()` and
  `GetIndirectRetrySuccessHistogram()` implementations using `Min<>`.
- Update API Doxygen comments to document that the last histogram entry
  counts all retries at or above the array limit.
This commit is contained in:
Abtin Keshavarzian
2026-07-10 07:12:45 -07:00
committed by GitHub
parent e08ff3fab9
commit 1860864511
6 changed files with 66 additions and 72 deletions
+1 -1
View File
@@ -52,7 +52,7 @@ extern "C" {
*
* @note This number versions both OpenThread platform and user APIs.
*/
#define OPENTHREAD_API_VERSION (610)
#define OPENTHREAD_API_VERSION (611)
/**
* @addtogroup api-instance
+19 -9
View File
@@ -851,33 +851,43 @@ int8_t otLinkConvertLinkQualityToRss(otInstance *aInstance, uint8_t aLinkQuality
/**
* Gets histogram of retries for a single direct packet until success.
*
* Is valid when OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE configuration is enabled.
* Requires `OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE`.
*
* @param[in] aInstance A pointer to an OpenThread instance.
* @param[out] aNumberOfEntries A pointer to where the size of returned histogram array is placed.
* The configuration `OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_DIRECT` specifies the size of the
* direct TX histogram array.
*
* @param[in] aInstance A pointer to an OpenThread instance.
* @param[out] aSize A pointer to where the size of returned histogram array is placed.
*
* @returns A pointer to the histogram of retries (in a form of an array).
* The n-th element indicates that the packet has been sent with n-th retry.
* If the number of retries is larger than the histogram array max size, the last entry
* counts all retries at or above the limit.
*/
const uint32_t *otLinkGetTxDirectRetrySuccessHistogram(otInstance *aInstance, uint8_t *aNumberOfEntries);
const uint32_t *otLinkGetTxDirectRetrySuccessHistogram(otInstance *aInstance, uint16_t *aSize);
/**
* Gets histogram of retries for a single indirect packet until success.
*
* Is valid when OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE configuration is enabled.
* Requires `OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE`.
*
* @param[in] aInstance A pointer to an OpenThread instance.
* @param[out] aNumberOfEntries A pointer to where the size of returned histogram array is placed.
* The configuration `OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_INDIRECT` specifies the size of the
* indirect TX histogram array.
*
* @param[in] aInstance A pointer to an OpenThread instance.
* @param[out] aSize A pointer to where the size of returned histogram array is placed.
*
* @returns A pointer to the histogram of retries (in a form of an array).
* The n-th element indicates that the packet has been sent with n-th retry.
* If the number of retries is larger than the histogram array max size, the last entry
* counts all retries at or above the limit.
*/
const uint32_t *otLinkGetTxIndirectRetrySuccessHistogram(otInstance *aInstance, uint8_t *aNumberOfEntries);
const uint32_t *otLinkGetTxIndirectRetrySuccessHistogram(otInstance *aInstance, uint16_t *aSize);
/**
* Clears histogram statistics for direct and indirect transmissions.
*
* Is valid when OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE configuration is enabled.
* Requires `OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE`.
*
* @param[in] aInstance A pointer to an OpenThread instance.
*/
+7 -7
View File
@@ -307,24 +307,24 @@ int8_t otLinkConvertLinkQualityToRss(otInstance *aInstance, uint8_t aLinkQuality
}
#if OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
const uint32_t *otLinkGetTxDirectRetrySuccessHistogram(otInstance *aInstance, uint8_t *aNumberOfEntries)
const uint32_t *otLinkGetTxDirectRetrySuccessHistogram(otInstance *aInstance, uint16_t *aSize)
{
AssertPointerIsNotNull(aNumberOfEntries);
AssertPointerIsNotNull(aSize);
return AsCoreType(aInstance).Get<Mac::Mac>().GetDirectRetrySuccessHistogram(*aNumberOfEntries);
return AsCoreType(aInstance).Get<Mac::Mac>().GetDirectRetrySuccessHistogram(*aSize);
}
const uint32_t *otLinkGetTxIndirectRetrySuccessHistogram(otInstance *aInstance, uint8_t *aNumberOfEntries)
const uint32_t *otLinkGetTxIndirectRetrySuccessHistogram(otInstance *aInstance, uint16_t *aSize)
{
const uint32_t *histogram = nullptr;
AssertPointerIsNotNull(aNumberOfEntries);
AssertPointerIsNotNull(aSize);
#if OPENTHREAD_FTD
histogram = AsCoreType(aInstance).Get<Mac::Mac>().GetIndirectRetrySuccessHistogram(*aNumberOfEntries);
histogram = AsCoreType(aInstance).Get<Mac::Mac>().GetIndirectRetrySuccessHistogram(*aSize);
#else
OT_UNUSED_VARIABLE(aInstance);
*aNumberOfEntries = 0;
*aSize = 0;
#endif
return histogram;
+10 -28
View File
@@ -1408,9 +1408,9 @@ void Mac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError)
mCounters.mTxDirectMaxRetryExpiry++;
}
#if OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
else if (mLinks.GetTransmitRetries() < OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_DIRECT)
else
{
mRetryHistogram.mTxDirectRetrySuccess[mLinks.GetTransmitRetries()]++;
mRetryHistogram.RecordDirectTx(mLinks.GetTransmitRetries());
}
#endif
@@ -1444,9 +1444,9 @@ void Mac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError)
mCounters.mTxIndirectMaxRetryExpiry++;
}
#if OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
else if (mLinks.GetTransmitRetries() < OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_INDIRECT)
else
{
mRetryHistogram.mTxIndirectRetrySuccess[mLinks.GetTransmitRetries()]++;
mRetryHistogram.RecordIndirectTx(mLinks.GetTransmitRetries());
}
#endif
@@ -2210,37 +2210,19 @@ exit:
Error Mac::GetRegion(uint16_t &aRegionCode) const { return Get<Radio>().GetRegion(aRegionCode); }
#if OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
const uint32_t *Mac::GetDirectRetrySuccessHistogram(uint8_t &aNumberOfEntries)
const uint32_t *Mac::GetDirectRetrySuccessHistogram(uint16_t &aSize) const
{
if (mMaxFrameRetriesDirect >= OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_DIRECT)
{
aNumberOfEntries = OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_DIRECT;
}
else
{
aNumberOfEntries = mMaxFrameRetriesDirect + 1;
}
return mRetryHistogram.mTxDirectRetrySuccess;
aSize = Min<uint16_t>(RetryHistogram::kMaxDirect, static_cast<uint16_t>(mMaxFrameRetriesDirect) + 1);
return mRetryHistogram.mDirect;
}
#if OPENTHREAD_FTD
const uint32_t *Mac::GetIndirectRetrySuccessHistogram(uint8_t &aNumberOfEntries)
const uint32_t *Mac::GetIndirectRetrySuccessHistogram(uint16_t &aSize) const
{
if (mMaxFrameRetriesIndirect >= OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_INDIRECT)
{
aNumberOfEntries = OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_INDIRECT;
}
else
{
aNumberOfEntries = mMaxFrameRetriesIndirect + 1;
}
return mRetryHistogram.mTxIndirectRetrySuccess;
aSize = Min<uint16_t>(RetryHistogram::kMaxIndirect, static_cast<uint16_t>(mMaxFrameRetriesIndirect) + 1);
return mRetryHistogram.mIndirect;
}
#endif
void Mac::ResetRetrySuccessHistogram() { ClearAllBytes(mRetryHistogram); }
#endif // OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
uint8_t Mac::ComputeLinkMargin(int8_t aRss) const { return ot::ComputeLinkMargin(GetNoiseFloor(), aRss); }
+25 -23
View File
@@ -43,6 +43,7 @@
#include "common/locator.hpp"
#include "common/log.hpp"
#include "common/non_copyable.hpp"
#include "common/num_utils.hpp"
#include "common/tasklet.hpp"
#include "common/time.hpp"
#include "common/timer.hpp"
@@ -498,29 +499,33 @@ public:
/**
* Returns the MAC retry histogram for direct transmission.
*
* @param[out] aNumberOfEntries A reference to where the size of returned histogram array is placed.
* @param[out] aSize A reference to where the size of returned histogram array is placed.
*
* @returns A pointer to the histogram of retries (in a form of an array).
* The n-th element indicates that the packet has been sent with n-th retry.
* If the number of retries is larger than the histogram array max size, the last entry
* counts all retries at or above the limit.
*/
const uint32_t *GetDirectRetrySuccessHistogram(uint8_t &aNumberOfEntries);
const uint32_t *GetDirectRetrySuccessHistogram(uint16_t &aSize) const;
#if OPENTHREAD_FTD
/**
* Returns the MAC retry histogram for indirect transmission.
*
* @param[out] aNumberOfEntries A reference to where the size of returned histogram array is placed.
* @param[out] aSize A reference to where the size of returned histogram array is placed.
*
* @returns A pointer to the histogram of retries (in a form of an array).
* The n-th element indicates that the packet has been sent with n-th retry.
* If the number of retries is larger than the histogram array max size, the last entry
* counts all retries at or above the limit.
*/
const uint32_t *GetIndirectRetrySuccessHistogram(uint8_t &aNumberOfEntries);
const uint32_t *GetIndirectRetrySuccessHistogram(uint16_t &aSize) const;
#endif
/**
* Resets MAC retry histogram.
*/
void ResetRetrySuccessHistogram(void);
void ResetRetrySuccessHistogram(void) { mRetryHistogram.Clear(); }
#endif // OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
/**
@@ -796,27 +801,24 @@ private:
};
#if OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
struct RetryHistogram
struct RetryHistogram : public Clearable<RetryHistogram>
{
/**
* Histogram of number of retries for a single direct packet until success
* [0 retry: packet count, 1 retry: packet count, 2 retry : packet count ...
* until max retry limit: packet count]
*
* The size of the array is OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_DIRECT.
*/
uint32_t mTxDirectRetrySuccess[OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_DIRECT];
static constexpr uint16_t kMaxDirect = OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_DIRECT;
static constexpr uint16_t kMaxIndirect = OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_INDIRECT;
/**
* Histogram of number of retries for a single indirect packet until success
* [0 retry: packet count, 1 retry: packet count, 2 retry : packet count ...
* until max retry limit: packet count]
*
* The size of the array is OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_INDIRECT.
*/
uint32_t mTxIndirectRetrySuccess[OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_MAX_SIZE_COUNT_INDIRECT];
static_assert(kMaxDirect > 0, "kMaxDirect must be greater than 0");
uint32_t mDirect[kMaxDirect];
void RecordDirectTx(uint8_t aRetryCount) { mDirect[Min<uint16_t>(aRetryCount, kMaxDirect - 1)]++; }
#if OPENTHREAD_FTD
static_assert(kMaxIndirect > 0, "kMaxIndirect must be greater than 0");
uint32_t mIndirect[kMaxIndirect];
void RecordIndirectTx(uint8_t aRetryCount) { mIndirect[Min<uint16_t>(aRetryCount, kMaxIndirect - 1)]++; }
#endif
};
#endif // OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE
#endif
Error ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Neighbor *aNeighbor);
void ProcessTransmitSecurity(TxFrame &aFrame);
+4 -4
View File
@@ -2928,8 +2928,8 @@ template <> otError NcpBase::HandlePropertyGet<SPINEL_PROP_CNTR_MAC_RETRY_HISTOG
otError error = OT_ERROR_NONE;
const uint32_t *histogramDirect;
const uint32_t *histogramIndirect;
uint8_t histogramDirectEntries;
uint8_t histogramIndirectEntries;
uint16_t histogramDirectEntries;
uint16_t histogramIndirectEntries;
histogramDirect = otLinkGetTxDirectRetrySuccessHistogram(mInstance, &histogramDirectEntries);
histogramIndirect = otLinkGetTxIndirectRetrySuccessHistogram(mInstance, &histogramIndirectEntries);
@@ -2939,7 +2939,7 @@ template <> otError NcpBase::HandlePropertyGet<SPINEL_PROP_CNTR_MAC_RETRY_HISTOG
// Encode direct message retries histogram
SuccessOrExit(error = mEncoder.OpenStruct());
for (uint8_t i = 0; i < histogramDirectEntries; i++)
for (uint16_t i = 0; i < histogramDirectEntries; i++)
{
SuccessOrExit(error = mEncoder.WriteUint32(histogramDirect[i]));
}
@@ -2947,7 +2947,7 @@ template <> otError NcpBase::HandlePropertyGet<SPINEL_PROP_CNTR_MAC_RETRY_HISTOG
// Encode indirect message retries histogram
SuccessOrExit(error = mEncoder.OpenStruct());
for (uint8_t i = 0; i < histogramIndirectEntries; i++)
for (uint16_t i = 0; i < histogramIndirectEntries; i++)
{
SuccessOrExit(error = mEncoder.WriteUint32(histogramIndirect[i]));
}