From 82f5ab298b0237007ae049a436ca093b2b031f42 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 18 Apr 2022 19:53:43 -0700 Subject: [PATCH] [message] update and simplify `otBufferInfo` (#7596) This commit updates the `otMessageGetBufferInfo()` and its related types. In particular, this commit adds `otMessageQueueInfo` to indicate number of messages, number of data buffers, and total message length (number of bytes) for all messages in the queue. --- include/openthread/instance.h | 2 +- include/openthread/message.h | 57 +++++++++++++++--------------- src/cli/README.md | 24 ++++++++----- src/cli/cli.cpp | 25 +++++++------ src/core/common/instance.cpp | 32 ++++++----------- src/core/common/message.cpp | 20 +++++------ src/core/common/message.hpp | 42 ++++++++++++++-------- src/ncp/ncp_base_mtd.cpp | 31 ++++++++-------- tests/unit/test_message_queue.cpp | 11 +++--- tests/unit/test_priority_queue.cpp | 7 ++-- 10 files changed, 129 insertions(+), 122 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index c57b31274..3605499d7 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (202) +#define OPENTHREAD_API_VERSION (203) /** * @addtogroup api-instance diff --git a/include/openthread/message.h b/include/openthread/message.h index 0b1430ec2..c39d99d53 100644 --- a/include/openthread/message.h +++ b/include/openthread/message.h @@ -58,34 +58,6 @@ extern "C" { */ typedef struct otMessage otMessage; -/** - * This structure represents the message buffer information. - * - */ -typedef struct otBufferInfo -{ - uint16_t mTotalBuffers; ///< The number of buffers in the pool. - uint16_t mFreeBuffers; ///< The number of free message buffers. - uint16_t m6loSendMessages; ///< The number of messages in the 6lo send queue. - uint16_t m6loSendBuffers; ///< The number of buffers in the 6lo send queue. - uint16_t m6loReassemblyMessages; ///< The number of messages in the 6LoWPAN reassembly queue. - uint16_t m6loReassemblyBuffers; ///< The number of buffers in the 6LoWPAN reassembly queue. - uint16_t mIp6Messages; ///< The number of messages in the IPv6 send queue. - uint16_t mIp6Buffers; ///< The number of buffers in the IPv6 send queue. - uint16_t mMplMessages; ///< The number of messages in the MPL send queue. - uint16_t mMplBuffers; ///< The number of buffers in the MPL send queue. - uint16_t mMleMessages; ///< The number of messages in the MLE send queue. - uint16_t mMleBuffers; ///< The number of buffers in the MLE send queue. - uint16_t mArpMessages; ///< The number of messages in the ARP send queue. - uint16_t mArpBuffers; ///< The number of buffers in the ARP send queue. - uint16_t mCoapMessages; ///< The number of messages in the CoAP send queue. - uint16_t mCoapBuffers; ///< The number of buffers in the CoAP send queue. - uint16_t mCoapSecureMessages; ///< The number of messages in the CoAP secure send queue. - uint16_t mCoapSecureBuffers; ///< The number of buffers in the CoAP secure send queue. - uint16_t mApplicationCoapMessages; ///< The number of messages in the application CoAP send queue. - uint16_t mApplicationCoapBuffers; ///< The number of buffers in the application CoAP send queue. -} otBufferInfo; - /** * This enumeration defines the OpenThread message priority levels. * @@ -298,6 +270,35 @@ typedef struct void *mData; ///< Opaque data used by the implementation. } otMessageQueue; +/** + * This structure represents information about a message queue. + * + */ +typedef struct otMessageQueueInfo +{ + uint16_t mNumMessages; ///< Number of messages in the queue. + uint16_t mNumBuffers; ///< Number of data buffers used by messages in the queue. + uint32_t mTotalBytes; ///< Total number of bytes used by all messages in the queue. +} otMessageQueueInfo; + +/** + * This structure represents the message buffer information for different queues used by OpenThread stack. + * + */ +typedef struct otBufferInfo +{ + uint16_t mTotalBuffers; ///< The total number of buffers in the messages pool. + uint16_t mFreeBuffers; ///< The number of free buffers. + otMessageQueueInfo m6loSendQueue; ///< Info about 6LoWPAN send queue. + otMessageQueueInfo m6loReassemblyQueue; ///< Info about 6LoWPAN reassembly queue. + otMessageQueueInfo mIp6Queue; ///< Info about IPv6 send queue. + otMessageQueueInfo mMplQueue; ///< Info about MPL send queue. + otMessageQueueInfo mMleQueue; ///< Info about MLE delayed message queue. + otMessageQueueInfo mCoapQueue; ///< Info about CoAP/TMF send queue. + otMessageQueueInfo mCoapSecureQueue; ///< Info about CoAP secure send queue. + otMessageQueueInfo mApplicationCoapQueue; ///< Info about application CoAP send queue. +} otBufferInfo; + /** * Initialize the message queue. * diff --git a/src/cli/README.md b/src/cli/README.md index 1d0696baf..b2c4056cd 100644 --- a/src/cli/README.md +++ b/src/cli/README.md @@ -399,19 +399,25 @@ Done Show the current message buffer information. +- The `total` shows total number of message buffers in pool. +- The `free` shows the number of free message buffers. +- This is then followed by info about different queues used by OpenThread stack, each line representing info about a queue. + - The first number shows number messages in the queue. + - The second number shows number of buffers used by all messages in the queue. + - The third number shows total number of bytes of all messages in the queue. + ```bash > bufferinfo total: 40 free: 40 -6lo send: 0 0 -6lo reas: 0 0 -ip6: 0 0 -mpl: 0 0 -mle: 0 0 -arp: 0 0 -coap: 0 0 -coap secure: 0 0 -application coap: 0 0 +6lo send: 0 0 0 +6lo reas: 0 0 0 +ip6: 0 0 0 +mpl: 0 0 0 +mle: 0 0 0 +coap: 0 0 0 +coap secure: 0 0 0 +application coap: 0 0 0 Done ``` diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index b159577d1..8ebfa47fb 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -756,21 +756,19 @@ template <> otError Interpreter::Process(Arg aArgs[]) struct BufferInfoName { - const uint16_t otBufferInfo::*mNumMessagesPtr; - const uint16_t otBufferInfo::*mNumBuffersPtr; - const char * mName; + const otMessageQueueInfo otBufferInfo::*mQueuePtr; + const char * mName; }; static const BufferInfoName kBufferInfoNames[] = { - {&otBufferInfo::m6loSendMessages, &otBufferInfo::m6loSendBuffers, "6lo send"}, - {&otBufferInfo::m6loReassemblyMessages, &otBufferInfo::m6loReassemblyBuffers, "6lo reas"}, - {&otBufferInfo::mIp6Messages, &otBufferInfo::mIp6Buffers, "ip6"}, - {&otBufferInfo::mMplMessages, &otBufferInfo::mMplBuffers, "mpl"}, - {&otBufferInfo::mMleMessages, &otBufferInfo::mMleBuffers, "mle"}, - {&otBufferInfo::mArpMessages, &otBufferInfo::mArpBuffers, "arp"}, - {&otBufferInfo::mCoapMessages, &otBufferInfo::mCoapBuffers, "coap"}, - {&otBufferInfo::mCoapSecureMessages, &otBufferInfo::mCoapSecureBuffers, "coap secure"}, - {&otBufferInfo::mApplicationCoapMessages, &otBufferInfo::mApplicationCoapBuffers, "application coap"}, + {&otBufferInfo::m6loSendQueue, "6lo send"}, + {&otBufferInfo::m6loReassemblyQueue, "6lo reas"}, + {&otBufferInfo::mIp6Queue, "ip6"}, + {&otBufferInfo::mMplQueue, "mpl"}, + {&otBufferInfo::mMleQueue, "mle"}, + {&otBufferInfo::mCoapQueue, "coap"}, + {&otBufferInfo::mCoapSecureQueue, "coap secure"}, + {&otBufferInfo::mApplicationCoapQueue, "application coap"}, }; otBufferInfo bufferInfo; @@ -782,7 +780,8 @@ template <> otError Interpreter::Process(Arg aArgs[]) for (const BufferInfoName &info : kBufferInfoNames) { - OutputLine("%s: %d %d", info.mName, bufferInfo.*info.mNumMessagesPtr, bufferInfo.*info.mNumBuffersPtr); + OutputLine("%s: %u %u %u", info.mName, (bufferInfo.*info.mQueuePtr).mNumMessages, + (bufferInfo.*info.mQueuePtr).mNumBuffers, (bufferInfo.*info.mQueuePtr).mTotalBytes); } return OT_ERROR_NONE; diff --git a/src/core/common/instance.cpp b/src/core/common/instance.cpp index 122eb4327..af453ae63 100644 --- a/src/core/common/instance.cpp +++ b/src/core/common/instance.cpp @@ -253,42 +253,32 @@ exit: void Instance::GetBufferInfo(BufferInfo &aInfo) { - uint16_t messages, buffers; - aInfo.Clear(); aInfo.mTotalBuffers = Get().GetTotalBufferCount(); aInfo.mFreeBuffers = Get().GetFreeBufferCount(); - Get().GetSendQueue().GetInfo(aInfo.m6loSendMessages, aInfo.m6loSendBuffers); - - Get().GetReassemblyQueue().GetInfo(aInfo.m6loReassemblyMessages, aInfo.m6loReassemblyBuffers); - - Get().GetSendQueue().GetInfo(aInfo.mIp6Messages, aInfo.mIp6Buffers); + Get().GetSendQueue().GetInfo(aInfo.m6loSendQueue); + Get().GetReassemblyQueue().GetInfo(aInfo.m6loReassemblyQueue); + Get().GetSendQueue().GetInfo(aInfo.mIp6Queue); #if OPENTHREAD_FTD - Get().GetBufferedMessageSet().GetInfo(aInfo.mMplMessages, aInfo.mMplBuffers); + Get().GetBufferedMessageSet().GetInfo(aInfo.mMplQueue); #endif - Get().GetMessageQueue().GetInfo(aInfo.mMleMessages, aInfo.mMleBuffers); + Get().GetMessageQueue().GetInfo(aInfo.mMleQueue); - Get().GetRequestMessages().GetInfo(aInfo.mCoapMessages, aInfo.mCoapBuffers); - Get().GetCachedResponses().GetInfo(messages, buffers); - aInfo.mCoapMessages += messages; - aInfo.mCoapBuffers += buffers; + Get().GetRequestMessages().GetInfo(aInfo.mCoapQueue); + Get().GetCachedResponses().GetInfo(aInfo.mCoapQueue); #if OPENTHREAD_CONFIG_DTLS_ENABLE - Get().GetRequestMessages().GetInfo(aInfo.mCoapSecureMessages, aInfo.mCoapSecureBuffers); - Get().GetCachedResponses().GetInfo(messages, buffers); - aInfo.mCoapSecureMessages += messages; - aInfo.mCoapSecureBuffers += buffers; + Get().GetRequestMessages().GetInfo(aInfo.mCoapSecureQueue); + Get().GetCachedResponses().GetInfo(aInfo.mCoapSecureQueue); #endif #if OPENTHREAD_CONFIG_COAP_API_ENABLE - GetApplicationCoap().GetRequestMessages().GetInfo(aInfo.mApplicationCoapMessages, aInfo.mApplicationCoapBuffers); - GetApplicationCoap().GetCachedResponses().GetInfo(messages, buffers); - aInfo.mApplicationCoapMessages += messages; - aInfo.mApplicationCoapBuffers += buffers; + GetApplicationCoap().GetRequestMessages().GetInfo(aInfo.mApplicationCoapQueue); + GetApplicationCoap().GetCachedResponses().GetInfo(aInfo.mApplicationCoapQueue); #endif } diff --git a/src/core/common/message.cpp b/src/core/common/message.cpp index d3681f20b..126d4548d 100644 --- a/src/core/common/message.cpp +++ b/src/core/common/message.cpp @@ -832,15 +832,13 @@ Message::ConstIterator MessageQueue::begin(void) const return Message::ConstIterator(GetHead()); } -void MessageQueue::GetInfo(uint16_t &aMessageCount, uint16_t &aBufferCount) const +void MessageQueue::GetInfo(Info &aInfo) const { - aMessageCount = 0; - aBufferCount = 0; - for (const Message &message : *this) { - aMessageCount++; - aBufferCount += message.GetBufferCount(); + aInfo.mNumMessages++; + aInfo.mNumBuffers += message.GetBufferCount(); + aInfo.mTotalBytes += message.GetLength(); } } @@ -992,15 +990,13 @@ Message::ConstIterator PriorityQueue::begin(void) const return Message::ConstIterator(GetHead()); } -void PriorityQueue::GetInfo(uint16_t &aMessageCount, uint16_t &aBufferCount) const +void PriorityQueue::GetInfo(Info &aInfo) const { - aMessageCount = 0; - aBufferCount = 0; - for (const Message &message : *this) { - aMessageCount++; - aBufferCount += message.GetBufferCount(); + aInfo.mNumMessages++; + aInfo.mNumBuffers += message.GetBufferCount(); + aInfo.mTotalBytes += message.GetLength(); } } diff --git a/src/core/common/message.hpp b/src/core/common/message.hpp index 69af1e3a3..4a78413e3 100644 --- a/src/core/common/message.hpp +++ b/src/core/common/message.hpp @@ -1345,6 +1345,8 @@ class MessageQueue : public otMessageQueue friend class PriorityQueue; public: + typedef otMessageQueueInfo Info; ///< This struct represents info (number of messages/buffers) about a queue. + /** * This enumeration represents a position (head or tail) in the queue. This is used to specify where a new message * should be added in the queue. @@ -1418,13 +1420,17 @@ public: void DequeueAndFreeAll(void); /** - * This method returns the number of messages and buffers enqueued. + * This method gets the information about number of messages and buffers in the queue. * - * @param[out] aMessageCount Returns the number of messages enqueued. - * @param[out] aBufferCount Returns the number of buffers enqueued. + * This method updates `aInfo` and adds number of message/buffers in the message queue to the corresponding member + * variable in `aInfo`. The caller needs to make sure `aInfo` is initialized before calling this method (e.g., + * clearing `aInfo`). Same `aInfo` can be passed in multiple calls of `GetInfo(aInfo)` on different queues to add + * up the number of messages/buffers on different queues. + * + * @param[out] aInfo A reference to `Info` structure to update.ni * */ - void GetInfo(uint16_t &aMessageCount, uint16_t &aBufferCount) const; + void GetInfo(Info &aInfo) const; // The following methods are intended to support range-based `for` // loop iteration over the queue entries and should not be used @@ -1454,6 +1460,8 @@ class PriorityQueue : private Clearable friend class MessagePool; public: + typedef otMessageQueueInfo Info; ///< This struct represents info (number of messages/buffers) about a queue. + /** * This constructor initializes the priority queue. * @@ -1532,16 +1540,7 @@ public: void DequeueAndFreeAll(void); /** - * This method returns the number of messages and buffers enqueued. - * - * @param[out] aMessageCount Returns the number of messages enqueued. - * @param[out] aBufferCount Returns the number of buffers enqueued. - * - */ - void GetInfo(uint16_t &aMessageCount, uint16_t &aBufferCount) const; - - /** - * This method returns the tail of the list (last message in the list) + * This method returns the tail of the list (last message in the list). * * @returns A pointer to the tail of the list. * @@ -1549,13 +1548,26 @@ public: Message *GetTail(void) { return AsNonConst(AsConst(this)->GetTail()); } /** - * This method returns the tail of the list (last message in the list) + * This method returns the tail of the list (last message in the list). * * @returns A pointer to the tail of the list. * */ const Message *GetTail(void) const; + /** + * This method gets the information about number of messages and buffers in the priority queue. + * + * This method updates `aInfo` array and adds number of message/buffers in the message queue to the corresponding + * member variable in `aInfo`. The caller needs to make sure `aInfo` is initialized before calling this method + * (e.g., clearing `aInfo`). Same `aInfo` can be passed in multiple calls of `GetInfo(aInfo)` on different queues + * to add up the number of messages/buffers on different queues. + * + * @param[out] aInfo A reference to an `Info` structure to update. + * + */ + void GetInfo(Info &aInfo) const; + // The following methods are intended to support range-based `for` // loop iteration over the queue entries and should not be used // directly. The range-based `for` works correctly even if the diff --git a/src/ncp/ncp_base_mtd.cpp b/src/ncp/ncp_base_mtd.cpp index 1f6e703e3..bd1f227cf 100644 --- a/src/ncp/ncp_base_mtd.cpp +++ b/src/ncp/ncp_base_mtd.cpp @@ -2682,27 +2682,28 @@ template <> otError NcpBase::HandlePropertyGet(v template <> otError NcpBase::HandlePropertyGet(void) { - otError error; + otError error = OT_ERROR_NONE; otBufferInfo bufferInfo; otMessageGetBufferInfo(mInstance, &bufferInfo); SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mTotalBuffers)); SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mFreeBuffers)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loSendMessages)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loSendBuffers)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loReassemblyMessages)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loReassemblyBuffers)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mIp6Messages)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mIp6Buffers)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMplMessages)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMplBuffers)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMleMessages)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMleBuffers)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mArpMessages)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mArpBuffers)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mCoapMessages)); - SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mCoapBuffers)); + + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loSendQueue.mNumMessages)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loSendQueue.mNumBuffers)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loReassemblyQueue.mNumMessages)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.m6loReassemblyQueue.mNumBuffers)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mIp6Queue.mNumMessages)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mIp6Queue.mNumBuffers)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMplQueue.mNumMessages)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMplQueue.mNumBuffers)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMleQueue.mNumMessages)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mMleQueue.mNumBuffers)); + SuccessOrExit(error = mEncoder.WriteUint16(0)); // Write zero for ARP for backward compatibility. + SuccessOrExit(error = mEncoder.WriteUint16(0)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mCoapQueue.mNumMessages)); + SuccessOrExit(error = mEncoder.WriteUint16(bufferInfo.mCoapQueue.mNumBuffers)); exit: return error; diff --git a/tests/unit/test_message_queue.cpp b/tests/unit/test_message_queue.cpp index 424702971..b8cedc6bf 100644 --- a/tests/unit/test_message_queue.cpp +++ b/tests/unit/test_message_queue.cpp @@ -102,9 +102,9 @@ void VerifyMessageQueueContent(ot::MessageQueue &aMessageQueue, int aExpectedLen void TestMessageQueue(void) { - ot::MessageQueue messageQueue; - ot::Message * messages[kNumTestMessages]; - uint16_t msgCount, bufferCount; + ot::MessageQueue messageQueue; + ot::Message * messages[kNumTestMessages]; + ot::MessageQueue::Info info; sInstance = testInitInstance(); VerifyOrQuit(sInstance != nullptr); @@ -144,8 +144,9 @@ void TestMessageQueue(void) VerifyMessageQueueContent(messageQueue, 5, messages[0], messages[1], messages[2], messages[3], messages[4]); // Check the GetInfo() - messageQueue.GetInfo(msgCount, bufferCount); - VerifyOrQuit(msgCount == 5, "MessageQueue::GetInfo() failed."); + memset(&info, 0, sizeof(info)); + messageQueue.GetInfo(info); + VerifyOrQuit(info.mNumMessages == 5, "MessageQueue::GetInfo() failed."); // Remove from head messageQueue.Dequeue(*messages[0]); diff --git a/tests/unit/test_priority_queue.cpp b/tests/unit/test_priority_queue.cpp index 5d7be54ef..51f6b720b 100644 --- a/tests/unit/test_priority_queue.cpp +++ b/tests/unit/test_priority_queue.cpp @@ -47,11 +47,12 @@ void VerifyPriorityQueueContent(ot::PriorityQueue &aPriorityQueue, int aExpected ot::Message * message; ot::Message * msgArg; int8_t curPriority = ot::Message::kNumPriorities; - uint16_t msgCount, bufCount; + ot::PriorityQueue::Info info; // Check the `GetInfo` - aPriorityQueue.GetInfo(msgCount, bufCount); - VerifyOrQuit(msgCount == aExpectedLength, "GetInfo() result does not match expected len."); + memset(&info, 0, sizeof(info)); + aPriorityQueue.GetInfo(info); + VerifyOrQuit(info.mNumMessages == aExpectedLength, "GetInfo() result does not match expected len."); va_start(args, aExpectedLength);