From 0db2bade371535ba5cd92726cedd213591fb9ded Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 18 Mar 2026 17:44:37 -0700 Subject: [PATCH] [network-diag] simplify `MacCountersTlv` and its usage (#12711) This commit simplifies `MacCountersTlv` by replacing its individual getter and setter methods with bulk operations: - Adds an `Init()` method that takes a `Mac::Counters` to directly populate the TLV fields from the MAC layer counters. - Adds a `Read()` method to parse the TLV and populate a given `NetworkDiagnostic::MacCounters` structure. - Updates `NetworkDiagnostic::Server` and `Client` to use these new methods, allowing the removal of their local helper methods `AppendMacCounters()` and `ParseMacCounters()`. - Introduces `Counters` as an alias for `otMacCounters` within the `Mac` namespace. --- src/core/mac/mac.hpp | 4 +- src/core/mac/mac_types.hpp | 5 + src/core/thread/network_diagnostic.cpp | 44 +----- src/core/thread/network_diagnostic.hpp | 3 - src/core/thread/network_diagnostic_tlvs.cpp | 50 +++++++ src/core/thread/network_diagnostic_tlvs.hpp | 147 ++------------------ 6 files changed, 76 insertions(+), 177 deletions(-) diff --git a/src/core/mac/mac.hpp b/src/core/mac/mac.hpp index b61b2b6be..9d6904d65 100644 --- a/src/core/mac/mac.hpp +++ b/src/core/mac/mac.hpp @@ -492,7 +492,7 @@ public: * * @returns A reference to the MAC counter. */ - otMacCounters &GetCounters(void) { return mCounters; } + Counters &GetCounters(void) { return mCounters; } #if OPENTHREAD_CONFIG_MAC_RETRY_SUCCESS_HISTOGRAM_ENABLE /** @@ -931,7 +931,7 @@ private: Links mLinks; OperationTask mOperationTask; MacTimer mTimer; - otMacCounters mCounters; + Counters mCounters; uint32_t mKeyIdMode2FrameCounter; SuccessRateTracker mCcaSuccessRateTracker; uint16_t mCcaSampleCount; diff --git a/src/core/mac/mac_types.hpp b/src/core/mac/mac_types.hpp index 4055d0a95..d667fd747 100644 --- a/src/core/mac/mac_types.hpp +++ b/src/core/mac/mac_types.hpp @@ -86,6 +86,11 @@ constexpr ShortAddress kShortAddrInvalid = OT_RADIO_INVALID_SHORT_ADDR; ///< */ typedef otWakeupId WakeupId; +/** + * Represents the MAC layer counters. + */ +typedef otMacCounters Counters; + /** * Generates a random IEEE 802.15.4 PAN ID. * diff --git a/src/core/thread/network_diagnostic.cpp b/src/core/thread/network_diagnostic.cpp index 05ef7bbc6..8d2293c43 100644 --- a/src/core/thread/network_diagnostic.cpp +++ b/src/core/thread/network_diagnostic.cpp @@ -207,28 +207,6 @@ exit: #endif // OPENTHREAD_CONFIG_BLE_TCAT_ENABLE #endif // OPENTHREAD_FTD -Error Server::AppendMacCounters(Message &aMessage) -{ - MacCountersTlv tlv; - const otMacCounters &counters = Get().GetCounters(); - - ClearAllBytes(tlv); - - tlv.Init(); - tlv.SetIfInUnknownProtos(counters.mRxOther); - tlv.SetIfInErrors(counters.mRxErrNoFrame + counters.mRxErrUnknownNeighbor + counters.mRxErrInvalidSrcAddr + - counters.mRxErrSec + counters.mRxErrFcs + counters.mRxErrOther); - tlv.SetIfOutErrors(counters.mTxErrCca); - tlv.SetIfInUcastPkts(counters.mRxUnicast); - tlv.SetIfInBroadcastPkts(counters.mRxBroadcast); - tlv.SetIfInDiscards(counters.mRxAddressFiltered + counters.mRxDestAddrFiltered + counters.mRxDuplicated); - tlv.SetIfOutUcastPkts(counters.mTxUnicast); - tlv.SetIfOutBroadcastPkts(counters.mTxBroadcast); - tlv.SetIfOutDiscards(counters.mTxErrBusyChannel); - - return tlv.AppendTo(aMessage); -} - #if OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE Error Server::AppendBorderRouterIfAddrs(Message &aMessage) @@ -402,8 +380,13 @@ Error Server::AppendDiagTlv(uint8_t aTlvType, Message &aMessage) break; case Tlv::kMacCounters: - error = AppendMacCounters(aMessage); + { + MacCountersTlv tlv; + + tlv.Init(Get().GetCounters()); + error = tlv.AppendTo(aMessage); break; + } case Tlv::kMleCounters: { @@ -1166,19 +1149,6 @@ void Client::ReadDiagData(DiagData &aDiagData, const Message &aMessage, const Tl aDiagData.mCount = static_cast(aMessage.ReadBytes(offsetRange, aDiagData.m8)); } -void Client::ParseMacCounters(const MacCountersTlv &aMacCountersTlv, otNetworkDiagMacCounters &aMacCounters) -{ - aMacCounters.mIfInUnknownProtos = aMacCountersTlv.GetIfInUnknownProtos(); - aMacCounters.mIfInErrors = aMacCountersTlv.GetIfInErrors(); - aMacCounters.mIfOutErrors = aMacCountersTlv.GetIfOutErrors(); - aMacCounters.mIfInUcastPkts = aMacCountersTlv.GetIfInUcastPkts(); - aMacCounters.mIfInBroadcastPkts = aMacCountersTlv.GetIfInBroadcastPkts(); - aMacCounters.mIfInDiscards = aMacCountersTlv.GetIfInDiscards(); - aMacCounters.mIfOutUcastPkts = aMacCountersTlv.GetIfOutUcastPkts(); - aMacCounters.mIfOutBroadcastPkts = aMacCountersTlv.GetIfOutBroadcastPkts(); - aMacCounters.mIfOutDiscards = aMacCountersTlv.GetIfOutDiscards(); -} - void Client::ParseIp6AddrList(Ip6AddrList &aIp6Addrs, const Message &aMessage, OffsetRange aOffsetRange) { aIp6Addrs.mCount = 0; @@ -1279,7 +1249,7 @@ Error Client::GetNextDiagTlv(const Coap::Message &aMessage, Iterator &aIterator, SuccessOrExit(error = aMessage.Read(offset, macCountersTlv)); VerifyOrExit(macCountersTlv.IsValid(), error = kErrorParse); - ParseMacCounters(macCountersTlv, aDiagTlv.mData.mMacCounters); + macCountersTlv.Read(aDiagTlv.mData.mMacCounters); break; } diff --git a/src/core/thread/network_diagnostic.hpp b/src/core/thread/network_diagnostic.hpp index 544f783c2..3bbb7527b 100644 --- a/src/core/thread/network_diagnostic.hpp +++ b/src/core/thread/network_diagnostic.hpp @@ -160,7 +160,6 @@ private: Error AppendDiagTlv(uint8_t aTlvType, Message &aMessage); Error AppendIp6AddressList(Message &aMessage); - Error AppendMacCounters(Message &aMessage); Error AppendRequestedTlvs(const Message &aRequest, Message &aResponse); #if OPENTHREAD_CONFIG_BLE_TCAT_ENABLE @@ -283,7 +282,6 @@ public: private: typedef otNetworkDiagData DiagData; typedef otNetworkDiagIp6AddrList Ip6AddrList; - typedef otNetworkDiagMacCounters MacCounters; Error SendCommand(Uri aUri, Message::Priority aPriority, @@ -305,7 +303,6 @@ private: static void ReadDiagData(DiagData &aDiagData, const Message &aMessage, const Tlv::Info &aTlvInfo); static void ParseIp6AddrList(Ip6AddrList &aIp6Addrs, const Message &aMessage, OffsetRange aOffsetRange); - static void ParseMacCounters(const MacCountersTlv &aMacCountersTlv, MacCounters &aMacCounters); #if OT_SHOULD_LOG_AT(OT_LOG_LEVEL_INFO) static const char *UriToString(Uri aUri); diff --git a/src/core/thread/network_diagnostic_tlvs.cpp b/src/core/thread/network_diagnostic_tlvs.cpp index 3cec7464f..ca84fbeb7 100644 --- a/src/core/thread/network_diagnostic_tlvs.cpp +++ b/src/core/thread/network_diagnostic_tlvs.cpp @@ -38,6 +38,9 @@ namespace ot { namespace NetworkDiagnostic { +//--------------------------------------------------------------------------------------------------------------------- +// EnhancedRouteTlvEntry + void EnhancedRouteTlvEntry::InitFrom(const Router &aRouter) { uint16_t data = 0; @@ -69,6 +72,9 @@ void EnhancedRouteTlvEntry::Parse(ParseInfo &aParseInfo) const #if OPENTHREAD_FTD +//--------------------------------------------------------------------------------------------------------------------- +// ChildTlv + void ChildTlv::InitFrom(const Child &aChild) { Clear(); @@ -103,6 +109,9 @@ void ChildTlv::InitFrom(const Child &aChild) #endif } +//--------------------------------------------------------------------------------------------------------------------- +// RouterNeighborTlv + void RouterNeighborTlv::InitFrom(const Router &aRouter) { Clear(); @@ -124,6 +133,9 @@ void RouterNeighborTlv::InitFrom(const Router &aRouter) #endif // OPENTHREAD_FTD +//--------------------------------------------------------------------------------------------------------------------- +// AnswerTlv + void AnswerTlv::Init(uint16_t aIndex, IsLastFlag aIsLastFlag) { SetType(kAnswer); @@ -132,6 +144,44 @@ void AnswerTlv::Init(uint16_t aIndex, IsLastFlag aIsLastFlag) SetFlagsIndex((aIndex & kIndexMask) | (aIsLastFlag == kIsLast ? kIsLastFlag : 0)); } +//--------------------------------------------------------------------------------------------------------------------- +// MacCountersTlv + +void MacCountersTlv::Init(const Mac::Counters &aMacCounters) +{ + SetType(kMacCounters); + SetLength(sizeof(*this) - sizeof(Tlv)); + + mIfInUnknownProtos = BigEndian::HostSwap32(aMacCounters.mRxOther); + mIfInErrors = BigEndian::HostSwap32(aMacCounters.mRxErrNoFrame + aMacCounters.mRxErrUnknownNeighbor + + aMacCounters.mRxErrInvalidSrcAddr + aMacCounters.mRxErrSec + + aMacCounters.mRxErrFcs + aMacCounters.mRxErrOther); + mIfOutErrors = BigEndian::HostSwap32(aMacCounters.mTxErrCca); + mIfInUcastPkts = BigEndian::HostSwap32(aMacCounters.mRxUnicast); + mIfInBroadcastPkts = BigEndian::HostSwap32(aMacCounters.mRxBroadcast); + mIfInDiscards = BigEndian::HostSwap32(aMacCounters.mRxAddressFiltered + aMacCounters.mRxDestAddrFiltered + + aMacCounters.mRxDuplicated); + mIfOutUcastPkts = BigEndian::HostSwap32(aMacCounters.mTxUnicast); + mIfOutBroadcastPkts = BigEndian::HostSwap32(aMacCounters.mTxBroadcast); + mIfOutDiscards = BigEndian::HostSwap32(aMacCounters.mTxErrBusyChannel); +} + +void MacCountersTlv::Read(MacCounters &aDiagMacCounters) const +{ + aDiagMacCounters.mIfInUnknownProtos = BigEndian::HostSwap32(mIfInUnknownProtos); + aDiagMacCounters.mIfInErrors = BigEndian::HostSwap32(mIfInErrors); + aDiagMacCounters.mIfOutErrors = BigEndian::HostSwap32(mIfOutErrors); + aDiagMacCounters.mIfInUcastPkts = BigEndian::HostSwap32(mIfInUcastPkts); + aDiagMacCounters.mIfInBroadcastPkts = BigEndian::HostSwap32(mIfInBroadcastPkts); + aDiagMacCounters.mIfInDiscards = BigEndian::HostSwap32(mIfInDiscards); + aDiagMacCounters.mIfOutUcastPkts = BigEndian::HostSwap32(mIfOutUcastPkts); + aDiagMacCounters.mIfOutBroadcastPkts = BigEndian::HostSwap32(mIfOutBroadcastPkts); + aDiagMacCounters.mIfOutDiscards = BigEndian::HostSwap32(mIfOutDiscards); +} + +//--------------------------------------------------------------------------------------------------------------------- +// MleCountersTlv + void MleCountersTlv::Init(const Mle::Counters &aMleCounters) { SetType(kMleCounters); diff --git a/src/core/thread/network_diagnostic_tlvs.hpp b/src/core/thread/network_diagnostic_tlvs.hpp index 3f7dfdb6a..e6eb7978d 100644 --- a/src/core/thread/network_diagnostic_tlvs.hpp +++ b/src/core/thread/network_diagnostic_tlvs.hpp @@ -44,6 +44,7 @@ #include "common/encoding.hpp" #include "common/message.hpp" #include "common/tlvs.hpp" +#include "mac/mac_types.hpp" #include "net/ip6_address.hpp" #include "radio/radio.hpp" #include "thread/child.hpp" @@ -321,6 +322,11 @@ typedef Mle::LeaderDataTlvValue LeaderDataTlvValue; */ typedef SimpleTlvInfo LeaderDataTlv; +/** + * Represents the Mac Counters. + */ +typedef otNetworkDiagMacCounters MacCounters; + /** * Implements Mac Counters TLV generation and parsing. */ @@ -330,12 +336,10 @@ class MacCountersTlv : public Tlv, public TlvInfo public: /** * Initializes the TLV. + * + * @param[in] aMacCounters The MAC counters to initialize the TLV with. */ - void Init(void) - { - SetType(kMacCounters); - SetLength(sizeof(*this) - sizeof(Tlv)); - } + void Init(const Mac::Counters &aMacCounters); /** * Indicates whether or not the TLV appears to be well-formed. @@ -346,138 +350,11 @@ public: bool IsValid(void) const { return GetLength() >= sizeof(*this) - sizeof(Tlv); } /** - * Returns the IfInUnknownProtos counter. + * Reads the counters from TLV. * - * @returns The IfInUnknownProtos counter + * @param[out] aDiagMacCounters A reference to `NetworkDiagnostic::MacCounters` to populate. */ - uint32_t GetIfInUnknownProtos(void) const { return BigEndian::HostSwap32(mIfInUnknownProtos); } - - /** - * Sets the IfInUnknownProtos counter. - * - * @param[in] aIfInUnknownProtos The IfInUnknownProtos counter - */ - void SetIfInUnknownProtos(const uint32_t aIfInUnknownProtos) - { - mIfInUnknownProtos = BigEndian::HostSwap32(aIfInUnknownProtos); - } - - /** - * Returns the IfInErrors counter. - * - * @returns The IfInErrors counter - */ - uint32_t GetIfInErrors(void) const { return BigEndian::HostSwap32(mIfInErrors); } - - /** - * Sets the IfInErrors counter. - * - * @param[in] aIfInErrors The IfInErrors counter - */ - void SetIfInErrors(const uint32_t aIfInErrors) { mIfInErrors = BigEndian::HostSwap32(aIfInErrors); } - - /** - * Returns the IfOutErrors counter. - * - * @returns The IfOutErrors counter - */ - uint32_t GetIfOutErrors(void) const { return BigEndian::HostSwap32(mIfOutErrors); } - - /** - * Sets the IfOutErrors counter. - * - * @param[in] aIfOutErrors The IfOutErrors counter. - */ - void SetIfOutErrors(const uint32_t aIfOutErrors) { mIfOutErrors = BigEndian::HostSwap32(aIfOutErrors); } - - /** - * Returns the IfInUcastPkts counter. - * - * @returns The IfInUcastPkts counter - */ - uint32_t GetIfInUcastPkts(void) const { return BigEndian::HostSwap32(mIfInUcastPkts); } - - /** - * Sets the IfInUcastPkts counter. - * - * @param[in] aIfInUcastPkts The IfInUcastPkts counter. - */ - void SetIfInUcastPkts(const uint32_t aIfInUcastPkts) { mIfInUcastPkts = BigEndian::HostSwap32(aIfInUcastPkts); } - /** - * Returns the IfInBroadcastPkts counter. - * - * @returns The IfInBroadcastPkts counter - */ - uint32_t GetIfInBroadcastPkts(void) const { return BigEndian::HostSwap32(mIfInBroadcastPkts); } - - /** - * Sets the IfInBroadcastPkts counter. - * - * @param[in] aIfInBroadcastPkts The IfInBroadcastPkts counter. - */ - void SetIfInBroadcastPkts(const uint32_t aIfInBroadcastPkts) - { - mIfInBroadcastPkts = BigEndian::HostSwap32(aIfInBroadcastPkts); - } - - /** - * Returns the IfInDiscards counter. - * - * @returns The IfInDiscards counter - */ - uint32_t GetIfInDiscards(void) const { return BigEndian::HostSwap32(mIfInDiscards); } - - /** - * Sets the IfInDiscards counter. - * - * @param[in] aIfInDiscards The IfInDiscards counter. - */ - void SetIfInDiscards(const uint32_t aIfInDiscards) { mIfInDiscards = BigEndian::HostSwap32(aIfInDiscards); } - - /** - * Returns the IfOutUcastPkts counter. - * - * @returns The IfOutUcastPkts counter - */ - uint32_t GetIfOutUcastPkts(void) const { return BigEndian::HostSwap32(mIfOutUcastPkts); } - - /** - * Sets the IfOutUcastPkts counter. - * - * @param[in] aIfOutUcastPkts The IfOutUcastPkts counter. - */ - void SetIfOutUcastPkts(const uint32_t aIfOutUcastPkts) { mIfOutUcastPkts = BigEndian::HostSwap32(aIfOutUcastPkts); } - - /** - * Returns the IfOutBroadcastPkts counter. - * - * @returns The IfOutBroadcastPkts counter - */ - uint32_t GetIfOutBroadcastPkts(void) const { return BigEndian::HostSwap32(mIfOutBroadcastPkts); } - - /** - * Sets the IfOutBroadcastPkts counter. - * - * @param[in] aIfOutBroadcastPkts The IfOutBroadcastPkts counter. - */ - void SetIfOutBroadcastPkts(const uint32_t aIfOutBroadcastPkts) - { - mIfOutBroadcastPkts = BigEndian::HostSwap32(aIfOutBroadcastPkts); - } - - /** - * Returns the IfOutDiscards counter. - * - * @returns The IfOutDiscards counter - */ - uint32_t GetIfOutDiscards(void) const { return BigEndian::HostSwap32(mIfOutDiscards); } - - /** - * Sets the IfOutDiscards counter. - * - * @param[in] aIfOutDiscards The IfOutDiscards counter. - */ - void SetIfOutDiscards(const uint32_t aIfOutDiscards) { mIfOutDiscards = BigEndian::HostSwap32(aIfOutDiscards); } + void Read(MacCounters &aDiagMacCounters) const; private: uint32_t mIfInUnknownProtos;