From f83394276982777eb0aa3f530fb4511cf1f6989e Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 15 Jun 2026 07:30:22 -0700 Subject: [PATCH] [netdiag] extend `AnswerBuilder` to support `HistoryTracker` (#13235) This commit extends the `NetDiag::AnswerBuilder` class to support generating and managing answer messages for the `HistoryTracker` module. This refactor eliminates redundant implementation in `HistoryTracker::Server`, removing its internal `AnswerInfo` struct and related message allocation and length checking methods, replacing them with the unified `NetDiag::AnswerBuilder` workflow. --- src/core/thread/net_diag.cpp | 2 +- src/core/thread/net_diag_types.cpp | 58 ++++++++++++-- src/core/thread/net_diag_types.hpp | 32 ++++++-- src/core/utils/history_tracker_server.cpp | 93 +++++------------------ src/core/utils/history_tracker_server.hpp | 29 ++----- 5 files changed, 100 insertions(+), 114 deletions(-) diff --git a/src/core/thread/net_diag.cpp b/src/core/thread/net_diag.cpp index 3fd1969c3..29523c78d 100644 --- a/src/core/thread/net_diag.cpp +++ b/src/core/thread/net_diag.cpp @@ -569,7 +569,7 @@ void Server::FreeAllRelatedAnswers(Coap::Message &aFirstAnswer) void Server::PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap::Message &aRequest) { - AnswerBuilder answerBuilder(GetInstance()); + AnswerBuilder answerBuilder(GetInstance(), kAnswerTypeNetDiag); Coap::Message *firstAnswer; SuccessOrExit(PrepareAnswers(aRequest, answerBuilder)); diff --git a/src/core/thread/net_diag_types.cpp b/src/core/thread/net_diag_types.cpp index cbdecfcbe..dd58c7778 100644 --- a/src/core/thread/net_diag_types.cpp +++ b/src/core/thread/net_diag_types.cpp @@ -38,13 +38,14 @@ namespace ot { namespace NetDiag { -AnswerBuilder::AnswerBuilder(Instance &aInstance) +AnswerBuilder::AnswerBuilder(Instance &aInstance, AnswerType aType) : InstanceLocator(aInstance) , mAnswer(nullptr) , mAnswerIndex(0) , mQueryId(0) - , mHasQueryId(false) + , mType(aType) , mPriority(Message::kPriorityNormal) + , mHasQueryId(false) { } @@ -53,9 +54,24 @@ AnswerBuilder::~AnswerBuilder(void) { mAnswers.DequeueAndFreeAll(); } Error AnswerBuilder::Start(const Coap::Message &aRequest) { mAnswerIndex = 0; - mHasQueryId = (Tlv::Find(aRequest, mQueryId) == kErrorNone); mPriority = aRequest.GetPriority(); + mHasQueryId = false; + switch (mType) + { + case kAnswerTypeNetDiag: + SuccessOrExit(Tlv::Find(aRequest, mQueryId)); + break; +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE + case kAnswerTypeHistoryTracker: + SuccessOrExit(Tlv::Find(aRequest, mQueryId)); + break; +#endif + } + + mHasQueryId = true; + +exit: return Allocate(); } @@ -64,17 +80,34 @@ Error AnswerBuilder::Finish(void) { return AppendAnswerTlv(AnswerTlvValue::kIsLa Error AnswerBuilder::Allocate(void) { Error error = kErrorNone; + Uri uri = kUriDiagnosticGetAnswer; - mAnswer = Get().AllocateAndInitConfirmablePostMessage(kUriDiagnosticGetAnswer); +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE + if (mType == kAnswerTypeHistoryTracker) + { + uri = kUriHistoryAnswer; + } +#endif + + mAnswer = Get().AllocateAndInitConfirmablePostMessage(uri); VerifyOrExit(mAnswer != nullptr, error = kErrorNoBufs); IgnoreError(mAnswer->SetPriority(mPriority)); mAnswers.Enqueue(*mAnswer); - if (mHasQueryId) + VerifyOrExit(mHasQueryId); + + switch (mType) { + case kAnswerTypeNetDiag: SuccessOrExit(error = Tlv::Append(*mAnswer, mQueryId)); + break; +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE + case kAnswerTypeHistoryTracker: + SuccessOrExit(error = Tlv::Append(*mAnswer, mQueryId)); + break; +#endif } exit: @@ -83,11 +116,24 @@ exit: Error AnswerBuilder::AppendAnswerTlv(AnswerTlvValue::IsLastFlag aFlag) { + Error error = kErrorNone; AnswerTlvValue value; value.Init(mAnswerIndex++, aFlag); - return Tlv::Append(*mAnswer, value); + switch (mType) + { + case kAnswerTypeNetDiag: + error = Tlv::Append(*mAnswer, value); + break; +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE + case kAnswerTypeHistoryTracker: + error = Tlv::Append(*mAnswer, value); + break; +#endif + } + + return error; } Error AnswerBuilder::CheckAnswerLength(void) diff --git a/src/core/thread/net_diag_types.hpp b/src/core/thread/net_diag_types.hpp index e56b0932d..b89fc27db 100644 --- a/src/core/thread/net_diag_types.hpp +++ b/src/core/thread/net_diag_types.hpp @@ -47,11 +47,25 @@ namespace ot { namespace NetDiag { /** - * Represents an object for tracking and managing Network Diagnostic answer messages. + * Represents an Answer type (used in `AnswerBuilder`). + */ +enum AnswerType : uint8_t +{ + kAnswerTypeNetDiag, ///< A `NetDiag` answer. +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_SERVER_ENABLE + kAnswerTypeHistoryTracker, ///< A `HistoryTracker` answer. +#endif +}; + +/** + * Represents an object for tracking and managing `NetDiag` or `HistoryTracker` answer messages. * - * This class is used when the response to a Network Diagnostic query requires multiple CoAP answer messages . It - * manages the inclusion of the Query ID and the Answer TLV (providing message indexing and "more-to-follow" flags) - * in each allocated answer message, while maintaining all answer messages in a queue. + * This class is used when the response to a query requires multiple CoAP answer messages. It manages the inclusion of + * the Query ID and the Answer TLV (providing message indexing and "more-to-follow" flags) in each allocated answer + * message, while maintaining all answer messages in a queue. + * + * The `AnswerType` determines the specific TLV types and the CoAP URI used for the allocated answer messages, either + * the Network Diagnostics TLVs or the History Tracker TLVs are used. */ class AnswerBuilder : public InstanceLocator { @@ -60,8 +74,9 @@ public: * Initializes the `AnswerBuilder`. * * @param[in] aInstance The OpenThread instance. + * @param[in] aType The `AnswerType`. */ - explicit AnswerBuilder(Instance &aInstance); + AnswerBuilder(Instance &aInstance, AnswerType aType); /** * Destructor for `AnswerBuilder`. @@ -71,13 +86,13 @@ public: ~AnswerBuilder(void); /** - * Starts the building of answer messages based on a given Network Diagnostic request. + * Starts the building of answer messages based on a given request. * * This method searches for a Query ID TLV within @p aRequest. If present, the Query ID is captured and * automatically included in all allocated answer messages. It also captures the priority from @p aRequest and * uses it for all answer messages. * - * @param[in] aRequest The Network Diagnostic request message. + * @param[in] aRequest The request message. * * @retval kErrorNone Successfully started. * @retval kErrorNoBufs Insufficient message buffers to allocate the first answer. @@ -129,8 +144,9 @@ private: Coap::MessageQueue mAnswers; uint16_t mAnswerIndex; uint16_t mQueryId; - bool mHasQueryId; + AnswerType mType; Message::Priority mPriority; + bool mHasQueryId; }; } // namespace NetDiag diff --git a/src/core/utils/history_tracker_server.cpp b/src/core/utils/history_tracker_server.cpp index b694f544d..a08667675 100644 --- a/src/core/utils/history_tracker_server.cpp +++ b/src/core/utils/history_tracker_server.cpp @@ -60,34 +60,6 @@ template <> void Server::HandleTmf(Coap::Msg &aMsg) PrepareAndSendAnswers(aMsg.mMessageInfo.GetPeerAddr(), aMsg.mMessage); } -Error Server::AllocateAnswer(Coap::Message *&aAnswer, AnswerInfo &aInfo) -{ - // Allocates an `Answer` message, adds it to `mAnswerQueue`, - // updates the `aInfo.mFirstAnswer` if it is the first allocated - // messages, and appends `QueryIdTlv` to the message (if needed). - - Error error = kErrorNone; - - aAnswer = Get().AllocateAndInitConfirmablePostMessage(kUriHistoryAnswer); - VerifyOrExit(aAnswer != nullptr, error = kErrorNoBufs); - IgnoreError(aAnswer->SetPriority(aInfo.mPriority)); - - mAnswerQueue.Enqueue(*aAnswer); - - if (aInfo.mFirstAnswer == nullptr) - { - aInfo.mFirstAnswer = aAnswer; - } - - if (aInfo.mHasQueryId) - { - SuccessOrExit(error = Tlv::Append(*aAnswer, aInfo.mQueryId)); - } - -exit: - return error; -} - bool Server::IsLastAnswer(const Coap::Message &aAnswer) const { // Indicates whether `aAnswer` is the last one associated with @@ -122,24 +94,17 @@ void Server::FreeAllRelatedAnswers(Coap::Message &aFirstAnswer) } } -void Server::PrepareAndSendAnswers(const Ip6::Address &aDestination, const Message &aRequest) +void Server::PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap::Message &aRequest) { - Coap::Message *answer; Error error; - AnswerInfo info; + AnswerBuilder answerBuilder(GetInstance(), NetDiag::kAnswerTypeHistoryTracker); OffsetRange offsetRange; Tlv::Info tlvInfo; RequestTlv requestTlv; - AnswerTlvValue answerTlvValue; + TimeMilli now = TimerMilli::GetNow(); + Coap::Message *firstAnswer; - if (Tlv::Find(aRequest, info.mQueryId) == kErrorNone) - { - info.mHasQueryId = true; - } - - info.mPriority = aRequest.GetPriority(); - - SuccessOrExit(error = AllocateAnswer(answer, info)); + SuccessOrExit(error = answerBuilder.Start(aRequest)); offsetRange.InitFromMessageOffsetToEnd(aRequest); @@ -160,48 +125,26 @@ void Server::PrepareAndSendAnswers(const Ip6::Address &aDestination, const Messa switch (requestTlv.GetTlvType()) { case Tlv::kNetworkInfo: - SuccessOrExit(error = AppendNetworkInfo(answer, info, requestTlv)); + SuccessOrExit(error = AppendNetworkInfo(answerBuilder, requestTlv, now)); break; default: break; } - SuccessOrExit(error = CheckAnswerLength(answer, info)); + SuccessOrExit(error = answerBuilder.CheckAnswerLength()); } } - answerTlvValue.Init(info.mAnswerIndex, AnswerTlvValue::kIsLast); - SuccessOrExit(error = Tlv::Append(*answer, answerTlvValue)); + SuccessOrExit(error = answerBuilder.Finish()); - SendNextAnswer(*info.mFirstAnswer, aDestination); + firstAnswer = answerBuilder.GetAnswers().GetHead(); + mAnswerQueue.EnqueueAllFrom(answerBuilder.GetAnswers()); + + SendNextAnswer(*firstAnswer, aDestination); exit: - if ((error != kErrorNone) && (info.mFirstAnswer != nullptr)) - { - FreeAllRelatedAnswers(*info.mFirstAnswer); - } -} - -Error Server::CheckAnswerLength(Coap::Message *&aAnswer, AnswerInfo &aInfo) -{ - // Checks the length of the `aAnswer` message and if it is above - // the threshold, it enqueues the message for transmission after - // appending an Answer TLV with the current index to the message. - // In this case, it will also allocate a new answer message. - - Error error = kErrorNone; - AnswerTlvValue answerTlvValue; - - VerifyOrExit(aAnswer->GetLength() >= kAnswerMessageLengthThreshold); - - answerTlvValue.Init(aInfo.mAnswerIndex++, AnswerTlvValue::kMoreToFollow); - SuccessOrExit(error = Tlv::Append(*aAnswer, answerTlvValue)); - - error = AllocateAnswer(aAnswer, aInfo); - -exit: - return error; + return; } void Server::SendNextAnswer(Coap::Message &aAnswer, const Ip6::Address &aDestination) @@ -259,14 +202,14 @@ exit: } } -Error Server::AppendNetworkInfo(Coap::Message *&aAnswer, AnswerInfo &aInfo, const RequestTlv &aRequestTlv) +Error Server::AppendNetworkInfo(AnswerBuilder &aAnswerBuilder, const RequestTlv &aRequestTlv, TimeMilli aNow) { Error error = kErrorNone; Iterator iterator; uint32_t maxEntryAge = aRequestTlv.GetMaxEntryAge(); uint16_t maxCount = aRequestTlv.GetNumEntries(); - iterator.Init(aInfo.mNow); + iterator.Init(aNow); for (uint16_t count = 0; (maxCount == 0) || (count < maxCount); count++) { @@ -288,11 +231,11 @@ Error Server::AppendNetworkInfo(Coap::Message *&aAnswer, AnswerInfo &aInfo, cons networkInfoTlv.InitFrom(*networkInfo, entryAge); - SuccessOrExit(error = aAnswer->Append(networkInfoTlv)); - SuccessOrExit(error = CheckAnswerLength(aAnswer, aInfo)); + SuccessOrExit(error = aAnswerBuilder.GetAnswer().Append(networkInfoTlv)); + SuccessOrExit(error = aAnswerBuilder.CheckAnswerLength()); } - SuccessOrExit(error = Tlv::AppendEmpty(*aAnswer)); + SuccessOrExit(error = Tlv::AppendEmpty(aAnswerBuilder.GetAnswer())); exit: return error; diff --git a/src/core/utils/history_tracker_server.hpp b/src/core/utils/history_tracker_server.hpp index 73e8b4105..63f38266d 100644 --- a/src/core/utils/history_tracker_server.hpp +++ b/src/core/utils/history_tracker_server.hpp @@ -43,11 +43,13 @@ #include "common/locator.hpp" #include "common/non_copyable.hpp" #include "common/timer.hpp" +#include "thread/net_diag_types.hpp" #include "thread/tmf.hpp" #include "utils/history_tracker.hpp" #include "utils/history_tracker_tlvs.hpp" namespace ot { + namespace HistoryTracker { /** @@ -61,34 +63,13 @@ public: explicit Server(Instance &aInstance); private: - static constexpr uint16_t kAnswerMessageLengthThreshold = 800; + typedef NetDiag::AnswerBuilder AnswerBuilder; - struct AnswerInfo - { - AnswerInfo(void) - : mNow(TimerMilli::GetNow()) - , mAnswerIndex(0) - , mQueryId(0) - , mHasQueryId(false) - , mFirstAnswer(nullptr) - { - } - - TimeMilli mNow; - uint16_t mAnswerIndex; - uint16_t mQueryId; - bool mHasQueryId; - Message::Priority mPriority; - Coap::Message *mFirstAnswer; - }; - - Error AllocateAnswer(Coap::Message *&aAnswer, AnswerInfo &aInfo); bool IsLastAnswer(const Coap::Message &aAnswer) const; void FreeAllRelatedAnswers(Coap::Message &aFirstAnswer); - void PrepareAndSendAnswers(const Ip6::Address &aDestination, const Message &aRequest); - Error CheckAnswerLength(Coap::Message *&aAnswer, AnswerInfo &aInfo); + void PrepareAndSendAnswers(const Ip6::Address &aDestination, const Coap::Message &aRequest); void SendNextAnswer(Coap::Message &aAnswer, const Ip6::Address &aDestination); - Error AppendNetworkInfo(Coap::Message *&aAnswer, AnswerInfo &aInfo, const RequestTlv &aRequestTlv); + 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);