From 16d7c2a46895b453957ea17c9956911acdcaac32 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 10 Oct 2024 08:05:01 -0700 Subject: [PATCH] [indirect-sender] add `HasQueuedMessageForSleepyChild()` (#10806) This commit adds new helper methods in `IndirectSender` to find a queued message for transmission to a given sleepy child that also satisfies a certain condition (using a general-purpose predicate `MessageChecker` function pointer). These helpers are then used in `IndirectSender` and `MleRouter` to simplify the code, particularly in `SendChildUpdateRequest()`, where sending multiple "Child Update Request" messages to a sleepy child being restored (after a parent restart/reboot) should be avoided. --- src/core/thread/indirect_sender.cpp | 53 +++++++++++++++---------- src/core/thread/indirect_sender.hpp | 60 ++++++++++++++++++++++++++++- src/core/thread/mesh_forwarder.hpp | 14 +------ src/core/thread/mle_router.cpp | 27 ++++--------- 4 files changed, 100 insertions(+), 54 deletions(-) diff --git a/src/core/thread/indirect_sender.cpp b/src/core/thread/indirect_sender.cpp index a629155a5..6c499f3d9 100644 --- a/src/core/thread/indirect_sender.cpp +++ b/src/core/thread/indirect_sender.cpp @@ -97,7 +97,7 @@ void IndirectSender::AddMessageForSleepyChild(Message &aMessage, Child &aChild) if ((aMessage.GetType() != Message::kTypeSupervision) && (aChild.GetIndirectMessageCount() > 1)) { - Message *supervisionMessage = FindIndirectMessage(aChild, /* aSupervisionTypeOnly */ true); + Message *supervisionMessage = FindQueuedMessageForSleepyChild(aChild, AcceptSupervisionMessage); if (supervisionMessage != nullptr) { @@ -151,6 +151,23 @@ exit: return; } +const Message *IndirectSender::FindQueuedMessageForSleepyChild(const Child &aChild, MessageChecker aChecker) const +{ + const Message *match = nullptr; + uint16_t childIndex = Get().GetChildIndex(aChild); + + for (const Message &message : Get().mSendQueue) + { + if (message.GetChildMask(childIndex) && aChecker(message)) + { + match = &message; + break; + } + } + + return match; +} + void IndirectSender::SetChildUseShortAddress(Child &aChild, bool aUseShortAddress) { VerifyOrExit(aChild.IsIndirectSourceMatchShort() != aUseShortAddress); @@ -201,24 +218,6 @@ void IndirectSender::HandleChildModeChange(Child &aChild, Mle::DeviceMode aOldMo // case. } -Message *IndirectSender::FindIndirectMessage(Child &aChild, bool aSupervisionTypeOnly) -{ - Message *msg = nullptr; - uint16_t childIndex = Get().GetChildIndex(aChild); - - for (Message &message : Get().mSendQueue) - { - if (message.GetChildMask(childIndex) && - (!aSupervisionTypeOnly || (message.GetType() == Message::kTypeSupervision))) - { - msg = &message; - break; - } - } - - return msg; -} - void IndirectSender::RequestMessageUpdate(Child &aChild) { Message *curMessage = aChild.GetIndirectMessage(); @@ -253,7 +252,7 @@ void IndirectSender::RequestMessageUpdate(Child &aChild) VerifyOrExit(!aChild.IsWaitingForMessageUpdate()); - newMessage = FindIndirectMessage(aChild); + newMessage = FindQueuedMessageForSleepyChild(aChild, AcceptAnyMessage); VerifyOrExit(curMessage != newMessage); @@ -296,7 +295,7 @@ exit: void IndirectSender::UpdateIndirectMessage(Child &aChild) { - Message *message = FindIndirectMessage(aChild); + Message *message = FindQueuedMessageForSleepyChild(aChild, AcceptAnyMessage); aChild.SetWaitingForMessageUpdate(false); aChild.SetIndirectMessage(message); @@ -555,6 +554,18 @@ void IndirectSender::ClearMessagesForRemovedChildren(void) } } +bool IndirectSender::AcceptAnyMessage(const Message &aMessage) +{ + OT_UNUSED_VARIABLE(aMessage); + + return true; +} + +bool IndirectSender::AcceptSupervisionMessage(const Message &aMessage) +{ + return aMessage.GetType() == Message::kTypeSupervision; +} + } // namespace ot #endif // #if OPENTHREAD_FTD diff --git a/src/core/thread/indirect_sender.hpp b/src/core/thread/indirect_sender.hpp index 4adc5a5fc..9e810e7de 100644 --- a/src/core/thread/indirect_sender.hpp +++ b/src/core/thread/indirect_sender.hpp @@ -130,6 +130,16 @@ public: "mQueuedMessageCount cannot fit max required!"); }; + /** + * Represents a predicate function for checking if a given `Message` meets specific criteria. + * + * @param[in] aMessage The message to evaluate. + * + * @retval TRUE If the @p aMessage satisfies the predicate condition. + * @retval FALSE If the @p aMessage does not satisfy the predicate condition. + */ + typedef bool (&MessageChecker)(const Message &aMessage); + /** * Initializes the object. * @@ -175,6 +185,52 @@ public: */ void ClearAllMessagesForSleepyChild(Child &aChild); + /** + * Finds the first queued message for a given sleepy child that also satisfies the conditions of a given + * `MessageChecker`. + * + * The caller MUST ensure that @p aChild is sleepy. + * + * @param[in] aChild The sleepy child to check. + * @param[in] aChecker The predicate function to apply. + * + * @returns A pointer to the matching queued message, or `nullptr` if none is found. + */ + Message *FindQueuedMessageForSleepyChild(const Child &aChild, MessageChecker aChecker) + { + return AsNonConst(AsConst(this)->FindQueuedMessageForSleepyChild(aChild, aChecker)); + } + + /** + * Finds the first queued message for a given sleepy child that also satisfies the conditions of a given + * `MessageChecker`. + * + * The caller MUST ensure that @p aChild is sleepy. + * + * @param[in] aChild The sleepy child to check. + * @param[in] aChecker The predicate function to apply. + * + * @returns A pointer to the matching queued message, or `nullptr` if none is found. + */ + const Message *FindQueuedMessageForSleepyChild(const Child &aChild, MessageChecker aChecker) const; + + /** + * Indicates whether there is any queued message for a given sleepy child that also satisfies the conditions of a + * given `MessageChecker`. + * + * The caller MUST ensure that @p aChild is sleepy. + * + * @param[in] aChild The sleepy child to check for. + * @param[in] aChecker The predicate function to apply. + * + * @retval TRUE There is a queued message satisfying @p aChecker for sleepy child @p aChild. + * @retval FALSE There is no queued message satisfying @p aChecker for sleepy child @p aChild. + */ + bool HasQueuedMessageForSleepyChild(const Child &aChild, MessageChecker aChecker) const + { + return (FindQueuedMessageForSleepyChild(aChild, aChecker) != nullptr); + } + /** * Sets whether to use the extended or short address for a child. * @@ -198,12 +254,14 @@ private: void HandleFrameChangeDone(Child &aChild); void UpdateIndirectMessage(Child &aChild); - Message *FindIndirectMessage(Child &aChild, bool aSupervisionTypeOnly = false); void RequestMessageUpdate(Child &aChild); uint16_t PrepareDataFrame(Mac::TxFrame &aFrame, Child &aChild, Message &aMessage); void PrepareEmptyFrame(Mac::TxFrame &aFrame, Child &aChild, bool aAckRequest); void ClearMessagesForRemovedChildren(void); + static bool AcceptAnyMessage(const Message &aMessage); + static bool AcceptSupervisionMessage(const Message &aMessage); + bool mEnabled; SourceMatchController mSourceMatchController; DataPollHandler mDataPollHandler; diff --git a/src/core/thread/mesh_forwarder.hpp b/src/core/thread/mesh_forwarder.hpp index 077820f47..8609f7a20 100644 --- a/src/core/thread/mesh_forwarder.hpp +++ b/src/core/thread/mesh_forwarder.hpp @@ -222,16 +222,7 @@ public: void SetRxOnWhenIdle(bool aRxOnWhenIdle); #if OPENTHREAD_FTD - - /** - * Represents a predicate function for checking if a given `Message` meets specific criteria. - * - * @param[in] aMessage The message to evaluate. - * - * @return TRUE If the @p aMessage satisfies the predicate condition. - * @return FALSE If the @p aMessage does not satisfy the predicate condition. - */ - typedef bool (&MessageChecker)(const Message &aMessage); + typedef IndirectSender::MessageChecker MessageChecker; ///< General predicate function checking a message. /** * Removes and frees messages queued for a child, based on a given predicate. @@ -242,8 +233,7 @@ public: * @param[in] aMessageChecker The predicate function to filter messages. */ void RemoveMessagesForChild(Child &aChild, MessageChecker aMessageChecker); - -#endif // OPENTHREAD_FTD +#endif /** * Frees unicast/multicast MLE Data Responses from Send Message Queue if any. diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index d2d6d0989..a98cb6171 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -2883,30 +2883,17 @@ Error MleRouter::SendChildUpdateRequest(Child &aChild) Ip6::Address destination; TxMessage *message = nullptr; - if (!aChild.IsRxOnWhenIdle()) + if (!aChild.IsRxOnWhenIdle() && aChild.IsStateRestoring()) { - uint16_t childIndex = Get().GetChildIndex(aChild); + // No need to send the resync "Child Update Request" + // to the sleepy child if there is one already + // queued. - for (const Message &msg : Get().GetSendQueue()) - { - if (msg.GetChildMask(childIndex) && msg.GetSubType() == Message::kSubTypeMleChildUpdateRequest) - { - // No need to send the resync "Child Update Request" - // to the sleepy child if there is one already - // queued. - if (aChild.IsStateRestoring()) - { - ExitNow(); - } - - // Remove queued outdated "Child Update Request" when - // there is newer Network Data is to send. - Get().RemoveMessagesForChild(aChild, IsMessageChildUpdateRequest); - break; - } - } + VerifyOrExit(!Get().HasQueuedMessageForSleepyChild(aChild, IsMessageChildUpdateRequest)); } + Get().RemoveMessagesForChild(aChild, IsMessageChildUpdateRequest); + VerifyOrExit((message = NewMleMessage(kCommandChildUpdateRequest)) != nullptr, error = kErrorNoBufs); SuccessOrExit(error = message->AppendSourceAddressTlv()); SuccessOrExit(error = message->AppendLeaderDataTlv());