From 4181dcd42cca86657c1f508d24508a2a8720b5bd Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 20 Aug 2026 12:49:29 -0700 Subject: [PATCH] [mac] refactor Frame `Type` and `Version` enums and FCF helpers (#13516) This commit refactors `Mac::Frame::Type` and `Mac::Frame::Version` enums and adds helper methods for Frame Control Field (FCF) construction and field extraction. Specifically: - Changes `Type` and `Version` underlying type from `uint16_t` to `uint8_t`. - Updates `Version` enum constants (`kVersion2003`, `kVersion2006`, `kVersion2015`) to store raw unshifted 2-bit values (`0, 1, 2`). - Updates `Frame::GetVersion()` to return `uint8_t`. - Introduces `Frame::ConstructFrameControlField(Type, uint8_t)` to encapsulate FCF construction from Frame Type and Version. - Renames and standardizes static FCF readers `ReadType()` and `ReadVersion()` using `ReadBits` for consistency with other FCF sub-field helpers (`ReadDstAddrMode()`, `ReadSecurityLevel()`, etc.). - Updates all call sites across `mac_frame.cpp` to use the new helpers. --- src/core/mac/mac_frame.cpp | 21 +++++++++++++-------- src/core/mac/mac_frame.hpp | 29 ++++++++++++++++------------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp index fb577d0cd..079ea3974 100644 --- a/src/core/mac/mac_frame.cpp +++ b/src/core/mac/mac_frame.cpp @@ -52,7 +52,7 @@ void TxFrame::BuildInfo::PrepareHeadersIn(TxFrame &aTxFrame) const FrameBuilder builder; uint8_t micSize = 0; - fcf = static_cast(mType) | static_cast(mVersion); + fcf = ConstructFrameControlField(mType, mVersion); fcf |= static_cast(DetermineAddrMode(mAddrs.mSource) << kFcfSrcAddrShift); fcf |= static_cast(DetermineAddrMode(mAddrs.mDestination) << kFcfDstAddrShift); @@ -291,8 +291,8 @@ Error Frame::ParseInfo::ParseFrom(const Frame &aFrame, ParseMode aMode) // Also restrict frame version to 2003, 2006, 2015. Future frame // versions can alter the MAC header layout. - VerifyOrExit(GetType(mFcf) <= kTypeMacCmd); - VerifyOrExit(GetVersion(mFcf) <= kVersion2015); + VerifyOrExit(ReadType(mFcf) <= kTypeMacCmd); + VerifyOrExit(ReadVersion(mFcf) <= kVersion2015); if (IsSeqPresent(mFcf)) { @@ -404,7 +404,7 @@ Error Frame::ParseInfo::ParseFrom(const Frame &aFrame, ParseMode aMode) //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // MAC Command - if (GetType(mFcf) == kTypeMacCmd) + if (ReadType(mFcf) == kTypeMacCmd) { VerifyOrExit(frameData.CanRead(sizeof(mCommandId))); @@ -470,6 +470,11 @@ Error Frame::ValidatePsdu(void) const return info.ParseFrom(*this, kParseFully); } +uint16_t Frame::ConstructFrameControlField(Type aType, uint8_t aVersion) +{ + return static_cast(aType | (aVersion << kFcfVersionShift)); +} + void Frame::UpdateFcfFlag(bool aSet, uint16_t aBitFlag) { uint16_t fcf = GetFrameControlField(); @@ -784,7 +789,7 @@ Error Frame::GetCommandId(uint8_t &aCommandId) const ParseInfo info; SuccessOrExit(error = info.ParseFrom(*this, kParseFully)); - VerifyOrExit(GetType(info.mFcf) == kTypeMacCmd, error = kErrorNotFound); + VerifyOrExit(ReadType(info.mFcf) == kTypeMacCmd, error = kErrorNotFound); aCommandId = info.mCommandId; exit: @@ -1063,7 +1068,7 @@ exit: void TxFrame::GenerateImmAck(const RxFrame &aFrame, bool aIsFramePending) { - uint16_t fcf = static_cast(kTypeAck) | aFrame.GetVersion(); + uint16_t fcf = ConstructFrameControlField(kTypeAck, aFrame.GetVersion()); mChannel = aFrame.mChannel; ClearAllBytes(mInfo.mTxInfo); @@ -1228,7 +1233,7 @@ Frame::InfoString Frame::ToInfoString(void) const { InfoString string; ParseInfo info; - uint16_t type; + uint8_t type; string.Append("len:%u", mLength); @@ -1245,7 +1250,7 @@ Frame::InfoString Frame::ToInfoString(void) const string.Append(", type:"); - type = GetType(info.mFcf); + type = ReadType(info.mFcf); switch (type) { diff --git a/src/core/mac/mac_frame.hpp b/src/core/mac/mac_frame.hpp index 44677d700..f81ccb335 100644 --- a/src/core/mac/mac_frame.hpp +++ b/src/core/mac/mac_frame.hpp @@ -67,9 +67,9 @@ public: /** * Represents the MAC frame type. * - * Values match the Frame Type field in Frame Control Field (FCF) as an `uint16_t`. + * Values match the Frame Type field in Frame Control Field (FCF). */ - enum Type : uint16_t + enum Type : uint8_t { kTypeBeacon = 0, ///< Beacon Frame Type. kTypeData = 1, ///< Data Frame Type. @@ -80,13 +80,14 @@ public: /** * Represents the MAC frame version. * - * Values match the Version field in Frame Control Field (FCF) as an `uint16_t`. + * Values match the raw (unshifted) Version sub-field (2-bit wide) in Frame Control Field (FCF). The enum does + * not cover all possible 2-bit values. */ - enum Version : uint16_t + enum Version : uint8_t { - kVersion2003 = 0 << 12, ///< 2003 Frame Version. - kVersion2006 = 1 << 12, ///< 2006 Frame Version. - kVersion2015 = 2 << 12, ///< 2015 Frame Version. + kVersion2003 = 0, ///< 2003 Frame Version. + kVersion2006 = 1, ///< 2006 Frame Version. + kVersion2015 = 2, ///< 2015 Frame Version. }; /** @@ -173,7 +174,7 @@ public: * * @returns The IEEE 802.15.4 Frame Type. */ - uint8_t GetType(void) const { return GetPsdu()[0] & kFcfFrameTypeMask; } + uint8_t GetType(void) const { return ReadType(GetFrameControlField()); } /** * Returns whether the frame is an Ack frame. @@ -196,7 +197,7 @@ public: * * @returns The IEEE 802.15.4 Frame Version. */ - uint16_t GetVersion(void) const { return GetVersion(GetFrameControlField()); } + uint8_t GetVersion(void) const { return ReadVersion(GetFrameControlField()); } /** * Returns if this IEEE 802.15.4 frame's version is 2015. @@ -588,7 +589,8 @@ protected: static constexpr uint16_t kFcfDstAddrShort = kAddrModeShort << kFcfDstAddrShift; static constexpr uint16_t kFcfDstAddrExt = kAddrModeExt << kFcfDstAddrShift; static constexpr uint16_t kFcfDstAddrMask = kFcfAddrMask << kFcfDstAddrShift; - static constexpr uint16_t kFcfFrameVersionMask = 3 << 12; + static constexpr uint16_t kFcfVersionShift = 12; + static constexpr uint16_t kFcfVersionMask = 3 << kFcfVersionShift; static constexpr uint16_t kFcfSrcAddrShift = 14; static constexpr uint16_t kFcfSrcAddrNone = kAddrModeNone << kFcfSrcAddrShift; static constexpr uint16_t kFcfSrcAddrShort = kAddrModeShort << kFcfSrcAddrShift; @@ -660,7 +662,7 @@ protected: void UpdateFcfFlag(bool aSet, uint16_t aBitFlag); - static uint16_t GetType(uint16_t aFcf) { return (aFcf & kFcfFrameTypeMask); } + static uint8_t ReadType(uint16_t aFcf) { return As(ReadBits(aFcf)); } static AddrMode ReadDstAddrMode(uint16_t aFcf) { return As(ReadBits(aFcf)); } static AddrMode ReadSrcAddrMode(uint16_t aFcf) { return As(ReadBits(aFcf)); } static bool IsSeqSuppressed(uint16_t aFcf) { return IsVersion2015(aFcf) && ((aFcf & kFcfSeqSuppression) != 0); } @@ -671,13 +673,14 @@ protected: static bool IsFramePending(uint16_t aFcf) { return (aFcf & kFcfFramePending) != 0; } static bool IsIePresent(uint16_t aFcf) { return IsVersion2015(aFcf) && ((aFcf & kFcfIePresent) != 0); } static bool IsAckRequest(uint16_t aFcf) { return (aFcf & kFcfAckRequest) != 0; } - static uint16_t GetVersion(uint16_t aFcf) { return (aFcf & kFcfFrameVersionMask); } - static bool IsVersion2015(uint16_t aFcf) { return GetVersion(aFcf) == kVersion2015; } + static uint8_t ReadVersion(uint16_t aFcf) { return As(ReadBits(aFcf)); } + static bool IsVersion2015(uint16_t aFcf) { return ReadVersion(aFcf) == kVersion2015; } static bool IsDstPanIdPresent(uint16_t aFcf); static bool IsSrcPanIdPresent(uint16_t aFcf); static AddrMode DetermineAddrMode(const Address &aAddress); static uint8_t CalculateKeySourceSize(KeyIdMode aKeyIdMode); static uint8_t CalculateMicSize(SecurityLevel aSecurityLevel); + static uint16_t ConstructFrameControlField(Type aType, uint8_t aVersion); // Security Control fields static SecurityLevel ReadSecurityLevel(uint8_t aSecCtl);