From 9c8374a44c9cc08c76273aca96442c796ff3f916 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 16 Jun 2026 06:39:05 -0700 Subject: [PATCH] [netdiag] extract answer sending logic into `AnswerSender` (#13242) This commit introduces the `NetDiag::AnswerSender` class to manage the transmission of multipart CoAP answer messages for Network Diagnostics and History Tracker queries. Previously, both `NetDiag::Server` and `HistoryTracker::Server` contained duplicated logic to queue allocated answer messages, track responses, and send subsequent messages one by one using an internal CoAP message queue. By extracting this into the new `AnswerSender` class, it eliminates the redundancy in message queue management, freeing of related answers, and CoAP response handling across both modules. The `AnswerSender` takes ownership of the messages generated by an `AnswerBuilder` and manages the asynchronous transmission lifecycle. --- src/core/thread/net_diag.cpp | 104 +------------- src/core/thread/net_diag.hpp | 10 +- src/core/thread/net_diag_types.cpp | 162 ++++++++++++++++++++++ src/core/thread/net_diag_types.hpp | 52 +++++++ src/core/utils/history_tracker_server.cpp | 108 ++------------- src/core/utils/history_tracker_server.hpp | 9 +- 6 files changed, 231 insertions(+), 214 deletions(-) diff --git a/src/core/thread/net_diag.cpp b/src/core/thread/net_diag.cpp index 29523c78d..6f6a7e44f 100644 --- a/src/core/thread/net_diag.cpp +++ b/src/core/thread/net_diag.cpp @@ -46,6 +46,9 @@ namespace NetDiag { Server::Server(Instance &aInstance) : InstanceLocator(aInstance) +#if OPENTHREAD_FTD + , mAnswerSender(aInstance, kAnswerTypeNetDiag) +#endif , mNonPreferredChannels(0) { } @@ -533,51 +536,12 @@ exit: #if OPENTHREAD_FTD -bool Server::IsLastAnswer(const Coap::Message &aAnswer) const -{ - // Indicates whether `aAnswer` is the last one associated with - // the same query. - - bool isLast = true; - AnswerTlvValue answerTlvValue; - - // If there is no Answer TLV, we assume it is the last answer. - - SuccessOrExit(Tlv::Find(aAnswer, answerTlvValue)); - isLast = answerTlvValue.IsLast(); - -exit: - return isLast; -} - -void Server::FreeAllRelatedAnswers(Coap::Message &aFirstAnswer) -{ - // This method dequeues and frees all answer messages related to - // same query as `aFirstAnswer`. Note that related answers are - // enqueued in order. - - Coap::Message *answer = &aFirstAnswer; - - while (answer != nullptr) - { - Coap::Message *next = IsLastAnswer(*answer) ? nullptr : answer->GetNextCoapMessage(); - - mAnswerQueue.DequeueAndFree(*answer); - answer = next; - } -} - void Server::PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap::Message &aRequest) { - AnswerBuilder answerBuilder(GetInstance(), kAnswerTypeNetDiag); - Coap::Message *firstAnswer; + AnswerBuilder answerBuilder(GetInstance(), kAnswerTypeNetDiag); SuccessOrExit(PrepareAnswers(aRequest, answerBuilder)); - - firstAnswer = answerBuilder.GetAnswers().GetHead(); - mAnswerQueue.EnqueueAllFrom(answerBuilder.GetAnswers()); - - SendNextAnswer(*firstAnswer, aDestination); + mAnswerSender.Send(answerBuilder, aDestination); exit: return; @@ -620,64 +584,6 @@ exit: return error; } -void Server::SendNextAnswer(Coap::Message &aAnswer, const Ip6::Address &aDestination) -{ - // This method send the given next `aAnswer` associated with - // a query to the `aDestination`. - - Error error = kErrorNone; - Coap::Message *nextAnswer = IsLastAnswer(aAnswer) ? nullptr : aAnswer.GetNextCoapMessage(); - - mAnswerQueue.Dequeue(aAnswer); - - // When sending the message, we pass `nextAnswer` as `aContext` - // to be used when invoking callback `HandleAnswerResponse()`. - - error = Get().SendMessageAllowMulticastLoop(aAnswer, aDestination, HandleAnswerResponse, nextAnswer); - - if (error != kErrorNone) - { - // If the `SendMessage()` fails, we `Free` the dequeued - // `aAnswer` and all the related next answers in the queue. - - aAnswer.Free(); - - if (nextAnswer != nullptr) - { - FreeAllRelatedAnswers(*nextAnswer); - } - } -} - -void Server::HandleAnswerResponse(void *aContext, Coap::Msg *aMsg, Error aResult) -{ - Coap::Message *nextAnswer = static_cast(aContext); - - VerifyOrExit(nextAnswer != nullptr); - - nextAnswer->Get().HandleAnswerResponse(*nextAnswer, aMsg, aResult); - -exit: - return; -} - -void Server::HandleAnswerResponse(Coap::Message &aNextAnswer, Coap::Msg *aResponse, Error aResult) -{ - Error error = aResult; - - SuccessOrExit(error); - VerifyOrExit(aResponse != nullptr, error = kErrorDrop); - VerifyOrExit(aResponse->GetCode() == Coap::kCodeChanged, error = kErrorDrop); - - SendNextAnswer(aNextAnswer, aResponse->mMessageInfo.GetPeerAddr()); - -exit: - if (error != kErrorNone) - { - FreeAllRelatedAnswers(aNextAnswer); - } -} - Error Server::AppendChildTableAsChildTlvs(AnswerBuilder &aAnswerBuilder) { Error error = kErrorNone; diff --git a/src/core/thread/net_diag.hpp b/src/core/thread/net_diag.hpp index 6de24f8ed..8a3314410 100644 --- a/src/core/thread/net_diag.hpp +++ b/src/core/thread/net_diag.hpp @@ -80,6 +80,7 @@ class Server : public InstanceLocator, private NonCopyable friend class Tmf::Agent; friend class MeshCoP::TcatAgent; friend class Client; + friend class AnswerSender; public: /** @@ -150,26 +151,19 @@ private: #if OPENTHREAD_MTD void SendAnswer(const Ip6::Address &aDestination, const Message &aRequest); #elif OPENTHREAD_FTD - bool IsLastAnswer(const Coap::Message &aAnswer) const; - void FreeAllRelatedAnswers(Coap::Message &aFirstAnswer); void PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap::Message &aRequest); Error PrepareAnswers(const Coap::Message &aRequest, AnswerBuilder &aAnswerBuilder); - void SendNextAnswer(Coap::Message &aAnswer, const Ip6::Address &aDestination); Error AppendChildTable(Message &aMessage); Error AppendChildTableAsChildTlvs(AnswerBuilder &aAnswerBuilder); Error AppendRouterNeighborTlvs(AnswerBuilder &aAnswerBuilder); Error AppendChildTableIp6AddressList(AnswerBuilder &aAnswerBuilder); Error AppendChildIp6AddressListTlv(Message &aAnswer, const Child &aChild); Error AppendEnhancedRoute(Message &aMessage); - #if OPENTHREAD_CONFIG_BLE_TCAT_ENABLE Error AppendChildTableAsChildTlvs(Message &aMessage); Error AppendRouterNeighborTlvs(Message &aMessage); Error AppendChildTableIp6AddressList(Message &aMessage); #endif - - static void HandleAnswerResponse(void *aContext, Coap::Msg *aMsg, Error aResult); - void HandleAnswerResponse(Coap::Message &aNextAnswer, Coap::Msg *aResponse, Error aResult); #endif #if OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE Error AppendBorderRouterIfAddrs(Message &aMessage); @@ -179,7 +173,7 @@ private: template void HandleTmf(Coap::Msg &aMsg); #if OPENTHREAD_FTD - Coap::MessageQueue mAnswerQueue; + AnswerSender mAnswerSender; #endif uint32_t mNonPreferredChannels; Callback mNonPreferredChannelsResetCallback; diff --git a/src/core/thread/net_diag_types.cpp b/src/core/thread/net_diag_types.cpp index dd58c7778..94ec7ebd0 100644 --- a/src/core/thread/net_diag_types.cpp +++ b/src/core/thread/net_diag_types.cpp @@ -38,6 +38,9 @@ namespace ot { namespace NetDiag { +//--------------------------------------------------------------------------------------------------------------------- +// AnswerBuilder + AnswerBuilder::AnswerBuilder(Instance &aInstance, AnswerType aType) : InstanceLocator(aInstance) , mAnswer(nullptr) @@ -150,5 +153,164 @@ exit: return error; } +//--------------------------------------------------------------------------------------------------------------------- +// AnswerSender + +AnswerSender::AnswerSender(Instance &aInstance, AnswerType aType) + : InstanceLocator(aInstance) + , mType(aType) +{ +} + +void AnswerSender::Send(AnswerBuilder &aAnswerBuilder, const Ip6::Address &aDestination) +{ + Coap::Message *firstAnswer = aAnswerBuilder.GetAnswers().GetHead(); + + VerifyOrExit(firstAnswer != nullptr); + + mQueue.EnqueueAllFrom(aAnswerBuilder.GetAnswers()); + SendNext(*firstAnswer, aDestination); + +exit: + return; +} + +bool AnswerSender::IsLast(const Coap::Message &aAnswer) const +{ + // Indicates whether `aAnswer` is the last one associated with + // the same query. + + bool isLast = true; + Error error = kErrorNotFound; + AnswerTlvValue answerTlvValue; + + switch (mType) + { + case kAnswerTypeNetDiag: + error = Tlv::Find(aAnswer, answerTlvValue); + break; +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE + case kAnswerTypeHistoryTracker: + error = Tlv::Find(aAnswer, answerTlvValue); + break; +#endif + } + + // If there is no Answer TLV, we assume it is the last answer. + + SuccessOrExit(error); + + isLast = answerTlvValue.IsLast(); + +exit: + return isLast; +} + +void AnswerSender::FreeAllRelated(Coap::Message &aAnswer) +{ + // This method dequeues and frees all answer messages related to + // same query as `aAnswer`. Note that related answers are always + // enqueued in order. + + Coap::Message *answer = &aAnswer; + + while (answer != nullptr) + { + Coap::Message *next = IsLast(*answer) ? nullptr : answer->GetNextCoapMessage(); + + mQueue.DequeueAndFree(*answer); + answer = next; + } +} + +void AnswerSender::SendNext(Coap::Message &aAnswer, const Ip6::Address &aDestination) +{ + // This method sends the given next `aAnswer` associated with + // a query to the `aDestination`. + + Error error = kErrorFailed; + Coap::Message *nextAnswer = IsLast(aAnswer) ? nullptr : aAnswer.GetNextCoapMessage(); + Coap::ResponseHandler responseHandler = nullptr; + + mQueue.Dequeue(aAnswer); + + switch (mType) + { + case kAnswerTypeNetDiag: +#if OPENTHREAD_FTD + responseHandler = AnswerSender::HandleResponse; +#endif + break; +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE + case kAnswerTypeHistoryTracker: + responseHandler = AnswerSender::HandleResponse; + break; +#endif + } + + VerifyOrExit(responseHandler != nullptr); + + // When sending the message, we pass `nextAnswer` as `aContext` + // to be used when invoking callback `responseHandler` + + error = Get().SendMessageAllowMulticastLoop(aAnswer, aDestination, responseHandler, nextAnswer); + +exit: + if (error != kErrorNone) + { + // If the `SendMessage()` fails, we `Free` the dequeued + // `aAnswer` and all the related next answers in the queue. + + aAnswer.Free(); + + if (nextAnswer != nullptr) + { + FreeAllRelated(*nextAnswer); + } + } +} + +#if OPENTHREAD_FTD +template <> AnswerSender &AnswerSender::GetSender(Instance &aInstance) +{ + return aInstance.Get().mAnswerSender; +} +#endif + +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE +template <> AnswerSender &AnswerSender::GetSender(Instance &aInstance) +{ + return aInstance.Get().mAnswerSender; +} +#endif + +template void AnswerSender::HandleResponse(void *aContext, Coap::Msg *aMsg, Error aResult) +{ + Coap::Message *nextAnswer = static_cast(aContext); + + VerifyOrExit(nextAnswer != nullptr); + GetSender(nextAnswer->GetInstance()).HandleResponse(*nextAnswer, aMsg, aResult); + +exit: + return; +} + +void AnswerSender::HandleResponse(Coap::Message &aNextAnswer, Coap::Msg *aResponse, Error aResult) +{ + Error error = aResult; + + SuccessOrExit(error); + VerifyOrExit(aResponse != nullptr, error = kErrorDrop); + VerifyOrExit(aResponse->GetCode() == Coap::kCodeChanged, error = kErrorDrop); + + SendNext(aNextAnswer, aResponse->mMessageInfo.GetPeerAddr()); + +exit: + if (error != kErrorNone) + { + FreeAllRelated(aNextAnswer); + } +} + } // namespace NetDiag } // namespace ot diff --git a/src/core/thread/net_diag_types.hpp b/src/core/thread/net_diag_types.hpp index b89fc27db..54a22ce8f 100644 --- a/src/core/thread/net_diag_types.hpp +++ b/src/core/thread/net_diag_types.hpp @@ -149,6 +149,58 @@ private: bool mHasQueryId; }; +/** + * Represents an `AnswerSender` for sending Network Diagnostics or History Tracker answer messages. + * + * This class takes ownership of the answer messages generated by an `AnswerBuilder` and manages sending them to a + * destination address one by one. It handles confirmable CoAP POST messages and tracks the responses. If any message + * fails to send or receives an error response, all remaining related answers are automatically freed. + */ +class AnswerSender : public InstanceLocator +{ +public: + /** + * Initializes the `AnswerSender`. + * + * @param[in] aInstance The OpenThread instance. + * @param[in] aType The `AnswerType` specifying either Network Diagnostics or History Tracker. + */ + AnswerSender(Instance &aInstance, AnswerType aType); + + /** + * Takes over all answer messages from @p aAnswerBuilder and starts sending them to @p aDestination. + * + * The `AnswerSender` enqueues all messages from the builder into its own queue. It then begins sending the first + * answer message in the queue. + * + * @param[in] aAnswerBuilder The `AnswerBuilder` containing the generated answer messages. + * @param[in] aDestination The destination IP address to send the answers to. + */ + void Send(AnswerBuilder &aAnswerBuilder, const Ip6::Address &aDestination); + +private: + bool IsLast(const Coap::Message &aAnswer) const; + void FreeAllRelated(Coap::Message &aAnswer); + void SendNext(Coap::Message &aAnswer, const Ip6::Address &aDestination); + void HandleResponse(Coap::Message &aNextAnswer, Coap::Msg *aResponse, Error aResult); + + template static void HandleResponse(void *aContext, Coap::Msg *aMsg, Error aResult); + template static AnswerSender &GetSender(Instance &aInstance); + + Coap::MessageQueue mQueue; + AnswerType mType; +}; + +// Declare template specializations. + +#if OPENTHREAD_FTD +template <> AnswerSender &AnswerSender::GetSender(Instance &aInstance); +#endif + +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE +template <> AnswerSender &AnswerSender::GetSender(Instance &aInstance); +#endif + } // namespace NetDiag } // namespace ot diff --git a/src/core/utils/history_tracker_server.cpp b/src/core/utils/history_tracker_server.cpp index a08667675..5e1b9389e 100644 --- a/src/core/utils/history_tracker_server.cpp +++ b/src/core/utils/history_tracker_server.cpp @@ -44,6 +44,7 @@ RegisterLogModule("HistoryServer"); Server::Server(Instance &aInstance) : InstanceLocator(aInstance) + , mAnswerSender(aInstance, NetDiag::kAnswerTypeHistoryTracker) { } @@ -60,49 +61,14 @@ template <> void Server::HandleTmf(Coap::Msg &aMsg) PrepareAndSendAnswers(aMsg.mMessageInfo.GetPeerAddr(), aMsg.mMessage); } -bool Server::IsLastAnswer(const Coap::Message &aAnswer) const -{ - // Indicates whether `aAnswer` is the last one associated with - // the same query. - - bool isLast = true; - AnswerTlvValue answerTlvValue; - - // If there is no Answer TLV, we assume it is the last answer. - - SuccessOrExit(Tlv::Find(aAnswer, answerTlvValue)); - isLast = answerTlvValue.IsLast(); - -exit: - return isLast; -} - -void Server::FreeAllRelatedAnswers(Coap::Message &aFirstAnswer) -{ - // Dequeues and frees all answer messages related to the same query - // as `aFirstAnswer`. Note that related answers are enqueued in - // order. - - Coap::Message *answer = &aFirstAnswer; - - while (answer != nullptr) - { - Coap::Message *next = IsLastAnswer(*answer) ? nullptr : answer->GetNextCoapMessage(); - - mAnswerQueue.DequeueAndFree(*answer); - answer = next; - } -} - void Server::PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap::Message &aRequest) { - Error error; - AnswerBuilder answerBuilder(GetInstance(), NetDiag::kAnswerTypeHistoryTracker); - OffsetRange offsetRange; - Tlv::Info tlvInfo; - RequestTlv requestTlv; - TimeMilli now = TimerMilli::GetNow(); - Coap::Message *firstAnswer; + Error error; + AnswerBuilder answerBuilder(GetInstance(), NetDiag::kAnswerTypeHistoryTracker); + OffsetRange offsetRange; + Tlv::Info tlvInfo; + RequestTlv requestTlv; + TimeMilli now = TimerMilli::GetNow(); SuccessOrExit(error = answerBuilder.Start(aRequest)); @@ -138,70 +104,12 @@ void Server::PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap: SuccessOrExit(error = answerBuilder.Finish()); - firstAnswer = answerBuilder.GetAnswers().GetHead(); - mAnswerQueue.EnqueueAllFrom(answerBuilder.GetAnswers()); - - SendNextAnswer(*firstAnswer, aDestination); + mAnswerSender.Send(answerBuilder, aDestination); exit: return; } -void Server::SendNextAnswer(Coap::Message &aAnswer, const Ip6::Address &aDestination) -{ - Error error = kErrorNone; - Coap::Message *nextAnswer = IsLastAnswer(aAnswer) ? nullptr : aAnswer.GetNextCoapMessage(); - - mAnswerQueue.Dequeue(aAnswer); - - // When sending the message, we pass `nextAnswer` as `aContext` - // to be used when invoking callback `HandleAnswerResponse()`. - - error = Get().SendMessageAllowMulticastLoop(aAnswer, aDestination, HandleAnswerResponse, nextAnswer); - - if (error != kErrorNone) - { - // If the `SendMessage()` fails, we `Free` the dequeued - // `aAnswer` and all the related next answers in the queue. - - aAnswer.Free(); - - if (nextAnswer != nullptr) - { - FreeAllRelatedAnswers(*nextAnswer); - } - } -} - -void Server::HandleAnswerResponse(void *aContext, Coap::Msg *aMsg, Error aResult) -{ - Coap::Message *nextAnswer = static_cast(aContext); - - VerifyOrExit(nextAnswer != nullptr); - - nextAnswer->Get().HandleAnswerResponse(*nextAnswer, aMsg, aResult); - -exit: - return; -} - -void Server::HandleAnswerResponse(Coap::Message &aNextAnswer, Coap::Msg *aResponse, Error aResult) -{ - Error error = aResult; - - SuccessOrExit(error); - VerifyOrExit(aResponse != nullptr, error = kErrorDrop); - VerifyOrExit(aResponse->GetCode() == Coap::kCodeChanged, error = kErrorDrop); - - SendNextAnswer(aNextAnswer, aResponse->mMessageInfo.GetPeerAddr()); - -exit: - if (error != kErrorNone) - { - FreeAllRelatedAnswers(aNextAnswer); - } -} - Error Server::AppendNetworkInfo(AnswerBuilder &aAnswerBuilder, const RequestTlv &aRequestTlv, TimeMilli aNow) { Error error = kErrorNone; diff --git a/src/core/utils/history_tracker_server.hpp b/src/core/utils/history_tracker_server.hpp index 63f38266d..30dae9ae0 100644 --- a/src/core/utils/history_tracker_server.hpp +++ b/src/core/utils/history_tracker_server.hpp @@ -58,6 +58,7 @@ namespace HistoryTracker { class Server : public InstanceLocator { friend class Tmf::Agent; + friend class NetDiag::AnswerSender; public: explicit Server(Instance &aInstance); @@ -65,18 +66,12 @@ public: private: typedef NetDiag::AnswerBuilder AnswerBuilder; - bool IsLastAnswer(const Coap::Message &aAnswer) const; - void FreeAllRelatedAnswers(Coap::Message &aFirstAnswer); void PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap::Message &aRequest); - void SendNextAnswer(Coap::Message &aAnswer, const Ip6::Address &aDestination); Error AppendNetworkInfo(AnswerBuilder &aAnswerBuilder, const RequestTlv &aRequestTlv, TimeMilli aNow); - static void HandleAnswerResponse(void *aContext, Coap::Msg *aMsg, Error aResult); - void HandleAnswerResponse(Coap::Message &aNextAnswer, Coap::Msg *aResponse, Error aResult); - template void HandleTmf(Coap::Msg &aMsg); - Coap::MessageQueue mAnswerQueue; + NetDiag::AnswerSender mAnswerSender; }; DeclareTmfHandler(Server, kUriHistoryQuery);