From 56730396866a411a09fe8dbfa94c91ea5c736570 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Fri, 11 Oct 2024 10:30:35 -0700 Subject: [PATCH] [message] track the MLE command type in `Message` (#10809) This commit adds a new field, `mMleCommand`, to `Message::Metadata` to track the MLE command type of the message. Helper methods, such as `IsMleCommand()`, are added to `Message` to get/set/check the MLE command type. This replaces the previous model where `Message::SubType` was used to track a subset of MLE commands. It helps simplify the code and allow us to track all MLE commands. --- src/core/common/message.cpp | 23 +--------- src/core/common/message.hpp | 58 +++++++++++++++++------- src/core/thread/discover_scanner.cpp | 2 +- src/core/thread/mesh_forwarder.cpp | 62 ++++++++++++-------------- src/core/thread/mesh_forwarder_ftd.cpp | 2 +- src/core/thread/mle.cpp | 51 ++++----------------- src/core/thread/mle.hpp | 28 +----------- src/core/thread/mle_router.cpp | 21 +-------- src/core/thread/mle_types.hpp | 29 ++++++++++++ tests/unit/test_message.cpp | 2 +- 10 files changed, 116 insertions(+), 162 deletions(-) diff --git a/src/core/common/message.cpp b/src/core/common/message.cpp index 7e34d8e39..58f6acfd9 100644 --- a/src/core/common/message.cpp +++ b/src/core/common/message.cpp @@ -318,28 +318,9 @@ void Message::SetOffset(uint16_t aOffset) GetMetadata().mOffset = aOffset; } -bool Message::IsSubTypeMle(void) const +bool Message::IsMleCommand(Mle::Command aMleCommand) const { - bool rval; - - switch (GetMetadata().mSubType) - { - case kSubTypeMleGeneral: - case kSubTypeMleAnnounce: - case kSubTypeMleDiscoverRequest: - case kSubTypeMleDiscoverResponse: - case kSubTypeMleChildUpdateRequest: - case kSubTypeMleDataResponse: - case kSubTypeMleChildIdRequest: - rval = true; - break; - - default: - rval = false; - break; - } - - return rval; + return (GetSubType() == kSubTypeMle) && (GetMetadata().mMleCommand == aMleCommand); } Error Message::SetPriority(Priority aPriority) diff --git a/src/core/common/message.hpp b/src/core/common/message.hpp index eaef26e42..fa056d6b5 100644 --- a/src/core/common/message.hpp +++ b/src/core/common/message.hpp @@ -202,6 +202,7 @@ protected: #endif uint8_t mType : 3; // The message type. uint8_t mSubType : 4; // The message sub type. + uint8_t mMleCommand; // The MLE command type (used when `mSubType is `Mle`). uint8_t mChannel; // The message channel (used for MLE Announce). #if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE uint8_t mTimeSyncSeq; // The time sync sequence. @@ -289,18 +290,12 @@ public: */ enum SubType : uint8_t { - kSubTypeNone = 0, ///< None - kSubTypeMleAnnounce = 1, ///< MLE Announce - kSubTypeMleDiscoverRequest = 2, ///< MLE Discover Request - kSubTypeMleDiscoverResponse = 3, ///< MLE Discover Response - kSubTypeJoinerEntrust = 4, ///< Joiner Entrust - kSubTypeMplRetransmission = 5, ///< MPL next retransmission message - kSubTypeMleGeneral = 6, ///< General MLE - kSubTypeJoinerFinalizeResponse = 7, ///< Joiner Finalize Response - kSubTypeMleChildUpdateRequest = 8, ///< MLE Child Update Request - kSubTypeMleDataResponse = 9, ///< MLE Data Response - kSubTypeMleChildIdRequest = 10, ///< MLE Child ID Request - kSubTypeMleDataRequest = 11, ///< MLE Data Request + kSubTypeNone = 0, ///< None + kSubTypeMle = 1, ///< MLE message + kSubTypeMplRetransmission = 2, ///< MPL next retransmission message + kSubTypeJoinerEntrust = 3, ///< Joiner Entrust + kSubTypeJoinerFinalizeResponse = 4, ///< Joiner Finalize Response + }; enum Priority : uint8_t @@ -566,12 +561,43 @@ public: void SetSubType(SubType aSubType) { GetMetadata().mSubType = aSubType; } /** - * Returns whether or not the message is of MLE subtype. + * Indicates whether or not the message is of MLE sub type. * - * @retval TRUE If message is of MLE subtype. - * @retval FALSE If message is not of MLE subtype. + * @retval TRUE The message is of MLE sub type. + * @retval FALSE The message is not of MLE sub type. */ - bool IsSubTypeMle(void) const; + bool IsSubTypeMle(void) const { return (GetSubType() == kSubTypeMle); } + + /** + * Indicates whether or not the message is a given MLE command. + * + * It checks `IsSubTypeMle()` and then if `GetMleCommand()` is the same as `aMleCommand`. + * + * @param[in] aMleCommand The MLE command type. + * + * @retval TRUE The message is an MLE command of @p aMleCommand type. + * @retval FALSE The message is not an MLE command of @p aMleCommand type. + */ + bool IsMleCommand(Mle::Command aMleCommand) const; + + /** + * Gets the MLE command type. + * + * Caller MUST ensure that message sub type is `kSubTypeMle` before calling this method. Otherwise the returned + * value is not meaningful. + * + * @returns The message's MLE command type. + */ + Mle::Command GetMleCommand(void) const { return static_cast(GetMetadata().mMleCommand); } + + /** + * Set the MLE command type of message. + * + * Caller should also set the sub type to `kSubTypeMle`. + * + * @param[in] aMleCommand The MLE command type. + */ + void SetMleCommand(Mle::Command aMleCommand) { GetMetadata().mMleCommand = aMleCommand; } /** * Checks whether this multicast message may be looped back. diff --git a/src/core/thread/discover_scanner.cpp b/src/core/thread/discover_scanner.cpp index 70fa6997b..9787b36d2 100644 --- a/src/core/thread/discover_scanner.cpp +++ b/src/core/thread/discover_scanner.cpp @@ -97,7 +97,7 @@ Error DiscoverScanner::Discover(const Mac::ChannelMask &aScanChannels, mScanChannels.Intersect(aScanChannels); } - VerifyOrExit((message = Get().NewMleMessage(Mle::kCommandDiscoveryRequest)) != nullptr, error = kErrorNoBufs); + VerifyOrExit((message = Get().NewMleMessage(kCommandDiscoveryRequest)) != nullptr, error = kErrorNoBufs); message->SetPanId(aPanId); // Prepare sub-TLV MeshCoP Discovery Request. diff --git a/src/core/thread/mesh_forwarder.cpp b/src/core/thread/mesh_forwarder.cpp index ee9e1226b..4e71febf2 100644 --- a/src/core/thread/mesh_forwarder.cpp +++ b/src/core/thread/mesh_forwarder.cpp @@ -778,7 +778,7 @@ Mac::TxFrame *MeshForwarder::HandleFrameRequest(Mac::TxFrames &aTxFrames) switch (mSendMessage->GetType()) { case Message::kTypeIp6: - if (mSendMessage->GetSubType() == Message::kSubTypeMleDiscoverRequest) + if (mSendMessage->IsMleCommand(Mle::kCommandDiscoveryRequest)) { frame = Get().PrepareDiscoveryRequestFrame(*frame); VerifyOrExit(frame != nullptr); @@ -792,7 +792,7 @@ Mac::TxFrame *MeshForwarder::HandleFrameRequest(Mac::TxFrames &aTxFrames) mMessageNextOffset = PrepareDataFrame(*frame, *mSendMessage, mMacAddrs, mAddMeshHeader, mMeshSource, mMeshDest, addFragHeader); - if ((mSendMessage->GetSubType() == Message::kSubTypeMleChildIdRequest) && mSendMessage->IsLinkSecurityEnabled()) + if (mSendMessage->IsMleCommand(Mle::kCommandChildIdRequest) && mSendMessage->IsLinkSecurityEnabled()) { LogNote("Child ID Request requires fragmentation, aborting tx"); mMessageNextOffset = mSendMessage->GetLength(); @@ -856,7 +856,7 @@ void MeshForwarder::PrepareMacHeaders(Mac::TxFrame &aTxFrame, Mac::TxFrame::Info #endif #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE if (Get().IsCslEnabled() && - !(aMessage != nullptr && aMessage->GetSubType() == Message::kSubTypeMleDiscoverRequest)) + !(aMessage != nullptr && aMessage->IsMleCommand(Mle::kCommandDiscoveryRequest))) { aTxFrameInfo.mAppendCslIe = true; aTxFrameInfo.mVersion = Mac::Frame::kVersion2015; @@ -934,39 +934,40 @@ start: { frameInfo.mSecurityLevel = Mac::Frame::kSecurityEncMic32; - switch (aMessage.GetSubType()) + if (aMessage.GetSubType() == Message::kSubTypeJoinerEntrust) { - case Message::kSubTypeJoinerEntrust: frameInfo.mKeyIdMode = Mac::Frame::kKeyIdMode0; - break; - - case Message::kSubTypeMleAnnounce: + } + else if (aMessage.IsMleCommand(Mle::kCommandAnnounce)) + { frameInfo.mKeyIdMode = Mac::Frame::kKeyIdMode2; - break; - - default: + } + else + { frameInfo.mKeyIdMode = Mac::Frame::kKeyIdMode1; - break; } } frameInfo.mPanIds.SetBothSourceDestination(Get().GetPanId()); - switch (aMessage.GetSubType()) + if (aMessage.IsSubTypeMle()) { - case Message::kSubTypeMleAnnounce: - aFrame.SetChannel(aMessage.GetChannel()); - aFrame.SetRxChannelAfterTxDone(Get().GetPanChannel()); - frameInfo.mPanIds.SetDestination(Mac::kPanIdBroadcast); - break; + switch (aMessage.GetMleCommand()) + { + case Mle::kCommandAnnounce: + aFrame.SetChannel(aMessage.GetChannel()); + aFrame.SetRxChannelAfterTxDone(Get().GetPanChannel()); + frameInfo.mPanIds.SetDestination(Mac::kPanIdBroadcast); + break; - case Message::kSubTypeMleDiscoverRequest: - case Message::kSubTypeMleDiscoverResponse: - frameInfo.mPanIds.SetDestination(aMessage.GetPanId()); - break; + case Mle::kCommandDiscoveryRequest: + case Mle::kCommandDiscoveryResponse: + frameInfo.mPanIds.SetDestination(aMessage.GetPanId()); + break; - default: - break; + default: + break; + } } frameInfo.mType = Mac::Frame::kTypeData; @@ -1345,20 +1346,15 @@ void MeshForwarder::FinalizeMessageDirectTx(Message &aMessage, Error aError) aMessage.GetTxSuccess() ? mIpCounters.mTxSuccess++ : mIpCounters.mTxFailure++; } - switch (aMessage.GetSubType()) + if (aMessage.IsMleCommand(Mle::kCommandDiscoveryRequest)) { - case Message::kSubTypeMleDiscoverRequest: // Note that `HandleDiscoveryRequestFrameTxDone()` may update // `aMessage` and mark it again for direct transmission. Get().HandleDiscoveryRequestFrameTxDone(aMessage, aError); - break; - - case Message::kSubTypeMleChildIdRequest: + } + else if (aMessage.IsMleCommand(Mle::kCommandChildIdRequest)) + { Get().HandleChildIdRequestTxDone(aMessage); - break; - - default: - break; } exit: diff --git a/src/core/thread/mesh_forwarder_ftd.cpp b/src/core/thread/mesh_forwarder_ftd.cpp index f91e53245..e2bd73a79 100644 --- a/src/core/thread/mesh_forwarder_ftd.cpp +++ b/src/core/thread/mesh_forwarder_ftd.cpp @@ -305,7 +305,7 @@ void MeshForwarder::RemoveDataResponseMessages(void) for (Message &message : mSendQueue) { - if (message.GetSubType() != Message::kSubTypeMleDataResponse) + if (!message.IsMleCommand(Mle::kCommandDataResponse)) { continue; } diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 06660a3ad..8d1b19b90 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -4354,7 +4354,7 @@ void Mle::DelayedSender::Send(TxMessage &aMessage, const Metadata &aMetadata) aMetadata.RemoveFrom(aMessage); - if (aMessage.GetSubType() == Message::kSubTypeMleDataRequest) + if (aMessage.IsMleCommand(kCommandDataRequest)) { SuccessOrExit(error = aMessage.AppendActiveAndPendingTimestampTlvs()); } @@ -4381,17 +4381,15 @@ exit: void Mle::DelayedSender::RemoveDataResponseMessage(void) { - RemoveMessage(Message::kSubTypeMleDataResponse, kTypeDataResponse, nullptr); + RemoveMessage(kCommandDataResponse, kTypeDataResponse, nullptr); } void Mle::DelayedSender::RemoveDataRequestMessage(const Ip6::Address &aDestination) { - RemoveMessage(Message::kSubTypeMleDataRequest, kTypeDataRequest, &aDestination); + RemoveMessage(kCommandDataRequest, kTypeDataRequest, &aDestination); } -void Mle::DelayedSender::RemoveMessage(Message::SubType aSubType, - MessageType aMessageType, - const Ip6::Address *aDestination) +void Mle::DelayedSender::RemoveMessage(Command aCommand, MessageType aMessageType, const Ip6::Address *aDestination) { for (Message &message : mQueue) { @@ -4399,8 +4397,7 @@ void Mle::DelayedSender::RemoveMessage(Message::SubType aSubType, metadata.ReadFrom(message); - if ((message.GetSubType() == aSubType) && - ((aDestination == nullptr) || (metadata.mDestination == *aDestination))) + if (message.IsMleCommand(aCommand) && ((aDestination == nullptr) || (metadata.mDestination == *aDestination))) { mQueue.DequeueAndFree(message); Log(kMessageRemoveDelayed, aMessageType, metadata.mDestination); @@ -4416,52 +4413,20 @@ Mle::TxMessage *Mle::NewMleMessage(Command aCommand) Error error = kErrorNone; TxMessage *message; Message::Settings settings(Message::kNoLinkSecurity, Message::kPriorityNet); - Message::SubType subType; uint8_t securitySuite; message = static_cast(mSocket.NewMessage(0, settings)); VerifyOrExit(message != nullptr, error = kErrorNoBufs); securitySuite = k154Security; - subType = Message::kSubTypeMleGeneral; - switch (aCommand) + if ((aCommand == kCommandDiscoveryRequest) || (aCommand == kCommandDiscoveryResponse)) { - case kCommandAnnounce: - subType = Message::kSubTypeMleAnnounce; - break; - - case kCommandDiscoveryRequest: - subType = Message::kSubTypeMleDiscoverRequest; securitySuite = kNoSecurity; - break; - - case kCommandDiscoveryResponse: - subType = Message::kSubTypeMleDiscoverResponse; - securitySuite = kNoSecurity; - break; - - case kCommandChildUpdateRequest: - subType = Message::kSubTypeMleChildUpdateRequest; - break; - - case kCommandDataResponse: - subType = Message::kSubTypeMleDataResponse; - break; - - case kCommandChildIdRequest: - subType = Message::kSubTypeMleChildIdRequest; - break; - - case kCommandDataRequest: - subType = Message::kSubTypeMleDataRequest; - break; - - default: - break; } - message->SetSubType(subType); + message->SetSubType(Message::kSubTypeMle); + message->SetMleCommand(aCommand); SuccessOrExit(error = message->Append(securitySuite)); diff --git a/src/core/thread/mle.hpp b/src/core/thread/mle.hpp index fa309ccf7..6dcc552de 100644 --- a/src/core/thread/mle.hpp +++ b/src/core/thread/mle.hpp @@ -792,32 +792,6 @@ private: //------------------------------------------------------------------------------------------------------------------ // Enumerations - enum Command : uint8_t - { - kCommandLinkRequest = 0, - kCommandLinkAccept = 1, - kCommandLinkAcceptAndRequest = 2, - kCommandLinkReject = 3, - kCommandAdvertisement = 4, - kCommandUpdate = 5, - kCommandUpdateRequest = 6, - kCommandDataRequest = 7, - kCommandDataResponse = 8, - kCommandParentRequest = 9, - kCommandParentResponse = 10, - kCommandChildIdRequest = 11, - kCommandChildIdResponse = 12, - kCommandChildUpdateRequest = 13, - kCommandChildUpdateResponse = 14, - kCommandAnnounce = 15, - kCommandDiscoveryRequest = 16, - kCommandDiscoveryResponse = 17, - kCommandLinkMetricsManagementRequest = 18, - kCommandLinkMetricsManagementResponse = 19, - kCommandLinkProbe = 20, - kCommandTimeSync = 99, - }; - enum AttachMode : uint8_t { kAnyPartition, // Attach to any Thread partition. @@ -1112,7 +1086,7 @@ private: }; void Send(TxMessage &aMessage, const Metadata &aMetadata); - void RemoveMessage(Message::SubType aSubType, MessageType aMessageType, const Ip6::Address *aDestination); + void RemoveMessage(Command aCommand, MessageType aMessageType, const Ip6::Address *aDestination); using DelayTimer = TimerMilliIn; diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index cb2aa6ead..930db6205 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -1921,28 +1921,11 @@ exit: } #endif // OPENTHREAD_CONFIG_TMF_PROXY_DUA_ENABLE -bool MleRouter::IsMessageMleSubType(const Message &aMessage) -{ - bool isMle = false; - - switch (aMessage.GetSubType()) - { - case Message::kSubTypeMleGeneral: - case Message::kSubTypeMleChildIdRequest: - case Message::kSubTypeMleChildUpdateRequest: - case Message::kSubTypeMleDataResponse: - isMle = true; - break; - default: - break; - } - - return isMle; -} +bool MleRouter::IsMessageMleSubType(const Message &aMessage) { return aMessage.IsSubTypeMle(); } bool MleRouter::IsMessageChildUpdateRequest(const Message &aMessage) { - return aMessage.GetSubType() == Message::kSubTypeMleChildUpdateRequest; + return aMessage.IsMleCommand(kCommandChildUpdateRequest); } void MleRouter::HandleChildIdRequest(RxInfo &aRxInfo) diff --git a/src/core/thread/mle_types.hpp b/src/core/thread/mle_types.hpp index 4d7e669f2..661c19b01 100644 --- a/src/core/thread/mle_types.hpp +++ b/src/core/thread/mle_types.hpp @@ -115,6 +115,35 @@ enum DeviceRole : uint8_t kRoleLeader = OT_DEVICE_ROLE_LEADER, ///< The Thread Leader role. }; +/** + * Represents MLE commands. + */ +enum Command : uint8_t +{ + kCommandLinkRequest = 0, ///< Link Request command + kCommandLinkAccept = 1, ///< Link Accept command + kCommandLinkAcceptAndRequest = 2, ///< Link Accept And Request command + kCommandLinkReject = 3, ///< Link Reject command + kCommandAdvertisement = 4, ///< Advertisement command + kCommandUpdate = 5, ///< Update command + kCommandUpdateRequest = 6, ///< Update Request command + kCommandDataRequest = 7, ///< Data Request command + kCommandDataResponse = 8, ///< Data Response command + kCommandParentRequest = 9, ///< Parent Request command + kCommandParentResponse = 10, ///< Parent Response command + kCommandChildIdRequest = 11, ///< Child ID Request command + kCommandChildIdResponse = 12, ///< Child ID Response command + kCommandChildUpdateRequest = 13, ///< Child Update Request command + kCommandChildUpdateResponse = 14, ///< Child Update Response command + kCommandAnnounce = 15, ///< Announce command + kCommandDiscoveryRequest = 16, ///< Discovery Request command + kCommandDiscoveryResponse = 17, ///< Discovery Response command + kCommandLinkMetricsManagementRequest = 18, ///< Link Metrics Management Request command + kCommandLinkMetricsManagementResponse = 19, ///< Link Metrics Management Response command + kCommandLinkProbe = 20, ///< Link Probe command + kCommandTimeSync = 99, ///< Time Sync command +}; + constexpr uint16_t kAloc16Leader = 0xfc00; constexpr uint16_t kAloc16DhcpAgentStart = 0xfc01; constexpr uint16_t kAloc16DhcpAgentEnd = 0xfc0f; diff --git a/tests/unit/test_message.cpp b/tests/unit/test_message.cpp index 666548cc7..fe7a62ea6 100644 --- a/tests/unit/test_message.cpp +++ b/tests/unit/test_message.cpp @@ -70,7 +70,7 @@ void TestMessage(void) message->SetLinkSecurityEnabled(Message::kWithLinkSecurity); SuccessOrQuit(message->SetPriority(Message::Priority::kPriorityNet)); message->SetType(Message::Type::kType6lowpan); - message->SetSubType(Message::SubType::kSubTypeMleChildIdRequest); + message->SetSubType(Message::SubType::kSubTypeJoinerEntrust); message->SetLoopbackToHostAllowed(true); message->SetOrigin(Message::kOriginHostUntrusted); SuccessOrQuit(message->SetLength(kMaxSize));