From 69bfc4b1c0b5f6597ce4744d68e501bf063c9ee6 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 2 Sep 2021 13:50:26 -0700 Subject: [PATCH] [message] simplify `Metadata` (#6879) This commit simplifies the `Metadata` definition in `Message` by rearranging the member variables (to avoid alignment gaps). It moves the definition to be sub-type of `Buffer`. It also removes the doxygen documentation for `private` or `protected` definitions (mainly for simple getters/setters). --- src/core/coap/coap_message.cpp | 5 +- src/core/coap/coap_message.hpp | 4 +- src/core/common/message.cpp | 48 +++-- src/core/common/message.hpp | 321 +++++++++------------------------ 4 files changed, 119 insertions(+), 259 deletions(-) diff --git a/src/core/coap/coap_message.cpp b/src/core/coap/coap_message.cpp index 56c90d210..5753bf7b5 100644 --- a/src/core/coap/coap_message.cpp +++ b/src/core/coap/coap_message.cpp @@ -356,9 +356,8 @@ Error Message::ParseHeader(void) Error error = kErrorNone; Option::Iterator iterator; - OT_ASSERT(mBuffer.mHead.mMetadata.mReserved >= - sizeof(GetHelpData()) + - static_cast((reinterpret_cast(&GetHelpData()) - mBuffer.mHead.mData))); + OT_ASSERT(GetReserved() >= + sizeof(HelpData) + static_cast((reinterpret_cast(&GetHelpData()) - GetFirstData()))); GetHelpData().Clear(); diff --git a/src/core/coap/coap_message.hpp b/src/core/coap/coap_message.hpp index 98a76b97b..db7e1bd84 100644 --- a/src/core/coap/coap_message.hpp +++ b/src/core/coap/coap_message.hpp @@ -961,10 +961,10 @@ private: const HelpData &GetHelpData(void) const { - static_assert(sizeof(mBuffer.mHead.mMetadata) + sizeof(HelpData) + kHelpDataAlignment <= sizeof(mBuffer), + static_assert(sizeof(HelpData) + kHelpDataAlignment <= kHeadBufferDataSize, "Insufficient buffer size for CoAP processing!"); - return *static_cast(OT_ALIGN(mBuffer.mHead.mData, kHelpDataAlignment)); + return *static_cast(OT_ALIGN(GetFirstData(), kHelpDataAlignment)); } HelpData &GetHelpData(void) { return AsNonConst(AsConst(this)->GetHelpData()); } diff --git a/src/core/common/message.cpp b/src/core/common/message.cpp index 40ded9a26..333584c98 100644 --- a/src/core/common/message.cpp +++ b/src/core/common/message.cpp @@ -53,6 +53,9 @@ namespace ot { +//--------------------------------------------------------------------------------------------------------------------- +// MessagePool + MessagePool::MessagePool(Instance &aInstance) : InstanceLocator(aInstance) #if !OPENTHREAD_CONFIG_PLATFORM_MESSAGE_MANAGEMENT && !OPENTHREAD_CONFIG_MESSAGE_USE_HEAP_ENABLE @@ -187,6 +190,9 @@ uint16_t MessagePool::GetTotalBufferCount(void) const #endif } +//--------------------------------------------------------------------------------------------------------------------- +// Message::Settings + const Message::Settings Message::Settings::kDefault(Message::kWithLinkSecurity, Message::kPriorityNormal); Message::Settings::Settings(LinkSecurityMode aSecurityMode, Priority aPriority) @@ -201,11 +207,15 @@ Message::Settings::Settings(const otMessageSettings *aSettings) { } +//--------------------------------------------------------------------------------------------------------------------- +// Message + Error Message::ResizeMessage(uint16_t aLength) { - Error error = kErrorNone; + // This method adds or frees message buffers to meet the + // requested length. - // add buffers + Error error = kErrorNone; Buffer * curBuffer = this; Buffer * lastBuffer; uint16_t curLength = kHeadBufferDataSize; @@ -222,7 +232,6 @@ Error Message::ResizeMessage(uint16_t aLength) curLength += kBufferDataSize; } - // remove buffers lastBuffer = curBuffer; curBuffer = curBuffer->GetNextBuffer(); lastBuffer->SetNextBuffer(nullptr); @@ -272,7 +281,7 @@ Error Message::SetLength(uint16_t aLength) SuccessOrExit(error = ResizeMessage(totalLengthRequest)); GetMetadata().mLength = aLength; - // Correct offset in case shorter length is set. + // Correct the offset in case shorter length is set. if (GetOffset() > aLength) { SetOffset(aLength); @@ -333,18 +342,19 @@ bool Message::IsSubTypeMle(void) const Error Message::SetPriority(Priority aPriority) { - Error error = kErrorNone; - uint8_t priority = static_cast(aPriority); - PriorityQueue *priorityQueue = nullptr; + Error error = kErrorNone; + uint8_t priority = static_cast(aPriority); + PriorityQueue *priorityQueue; VerifyOrExit(priority < kNumPriorities, error = kErrorInvalidArgs); VerifyOrExit(IsInAQueue(), GetMetadata().mPriority = priority); VerifyOrExit(GetMetadata().mPriority != priority); - if (GetMetadata().mInPriorityQ) + priorityQueue = GetPriorityQueue(); + + if (priorityQueue != nullptr) { - priorityQueue = GetMetadata().mQueue.mPriority; priorityQueue->Dequeue(*this); } @@ -726,16 +736,19 @@ bool Message::IsTimeSync(void) const void Message::SetMessageQueue(MessageQueue *aMessageQueue) { - GetMetadata().mQueue.mMessage = aMessageQueue; - GetMetadata().mInPriorityQ = false; + GetMetadata().mQueue = aMessageQueue; + GetMetadata().mInPriorityQ = false; } void Message::SetPriorityQueue(PriorityQueue *aPriorityQueue) { - GetMetadata().mQueue.mPriority = aPriorityQueue; - GetMetadata().mInPriorityQ = true; + GetMetadata().mQueue = aPriorityQueue; + GetMetadata().mInPriorityQ = true; } +//--------------------------------------------------------------------------------------------------------------------- +// MessageQueue + MessageQueue::MessageQueue(void) { SetTail(nullptr); @@ -829,6 +842,9 @@ void MessageQueue::GetInfo(uint16_t &aMessageCount, uint16_t &aBufferCount) cons } } +//--------------------------------------------------------------------------------------------------------------------- +// PriorityQueue + PriorityQueue::PriorityQueue(void) { for (Message *&tail : mTails) @@ -839,6 +855,10 @@ PriorityQueue::PriorityQueue(void) Message *PriorityQueue::FindFirstNonNullTail(Message::Priority aStartPriorityLevel) const { + // Find the first non-nullptr tail starting from the given priority + // level and moving forward (wrapping from priority value + // `kNumPriorities` -1 back to 0). + Message *tail = nullptr; uint8_t priority; @@ -953,7 +973,7 @@ void PriorityQueue::Dequeue(Message &aMessage) aMessage.Next() = nullptr; aMessage.Prev() = nullptr; - aMessage.SetMessageQueue(nullptr); + aMessage.SetPriorityQueue(nullptr); } void PriorityQueue::DequeueAndFree(Message &aMessage) diff --git a/src/core/common/message.hpp b/src/core/common/message.hpp index 53615bb3d..b2a3fe02a 100644 --- a/src/core/common/message.hpp +++ b/src/core/common/message.hpp @@ -144,62 +144,6 @@ class MessageQueue; class PriorityQueue; class ThreadLinkInfo; -/** - * This structure contains metadata about a Message. - * - */ -struct MessageMetadata -{ - Message * mNext; ///< A pointer to the next Message in a doubly linked list. - Message * mPrev; ///< A pointer to the previous Message in a doubly linked list. - MessagePool *mMessagePool; ///< Identifies the message pool for this message. - union - { - MessageQueue * mMessage; ///< Identifies the message queue (if any) where this message is queued. - PriorityQueue *mPriority; ///< Identifies the priority queue (if any) where this message is queued. - } mQueue; ///< Identifies the queue (if any) where this message is queued. - - uint32_t mDatagramTag; ///< The datagram tag used for 6LoWPAN fragmentation or identification used for IPv6 - ///< fragmentation. - uint16_t mReserved; ///< Number of header bytes reserved for the message. - uint16_t mLength; ///< Number of bytes within the message. - uint16_t mOffset; ///< A byte offset within the message. - RssAverager mRssAverager; ///< The averager maintaining the received signal strength (RSS) average. -#if OPENTHREAD_CONFIG_MLE_LINK_METRICS_SUBJECT_ENABLE - LqiAverager mLqiAverager; ///< The averager maintaining the Link quality indicator (LQI) average. -#endif - - ChildMask mChildMask; ///< A ChildMask to indicate which sleepy children need to receive this. - uint16_t mMeshDest; ///< Used for unicast non-link-local messages. - uint8_t mTimeout; ///< Seconds remaining before dropping the message. - union - { - uint16_t mPanId; ///< Used for MLE Discover Request and Response messages. - uint8_t mChannel; ///< Used for MLE Announce. - } mPanIdChannel; ///< Used for MLE Discover Request, Response, and Announce messages. - - uint8_t mType : 3; ///< Identifies the type of message. - uint8_t mSubType : 4; ///< Identifies the message sub type. - bool mDirectTx : 1; ///< Used to indicate whether a direct transmission is required. - bool mLinkSecurity : 1; ///< Indicates whether or not link security is enabled. - uint8_t mPriority : 2; ///< Identifies the message priority level (higher value is higher priority). - bool mInPriorityQ : 1; ///< Indicates whether the message is queued in normal or priority queue. - bool mTxSuccess : 1; ///< Indicates whether the direct tx of the message was successful. - bool mDoNotEvict : 1; ///< Indicates whether or not this message may be evicted. - bool mMulticastLoop : 1; ///< Indicates whether or not this multicast message may be looped back. -#if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE - bool mTimeSync : 1; ///< Indicates whether the message is also used for time sync purpose. - int64_t mNetworkTimeOffset; ///< The time offset to the Thread network time, in microseconds. - uint8_t mTimeSyncSeq; ///< The time sync sequence. -#endif -#if OPENTHREAD_CONFIG_MULTI_RADIO - uint8_t mRadioType : 2; ///< The radio link type the message was received on, or should be sent on. - bool mIsRadioTypeSet : 1; ///< Indicates whether the radio type is set. - - static_assert(Mac::kNumRadioTypes <= (1 << 2), "mRadioType bitfield cannot store all radio type values"); -#endif -}; - /** * This class represents a Message buffer. * @@ -207,7 +151,6 @@ struct MessageMetadata class Buffer : public otMessageBuffer, public LinkedListEntry { friend class Message; - friend class LinkedListEntry; public: /** @@ -234,70 +177,76 @@ public: */ void SetNextBuffer(Buffer *aNext) { SetNext(aNext); } -private: - /** - * This method returns the message metadata in the first message buffer. - * - * @returns The message metadata structure. - * - */ - MessageMetadata &GetMetadata(void) { return mBuffer.mHead.mMetadata; } +protected: + struct Metadata + { + Message * mNext; // Next message in a doubly linked list. + Message * mPrev; // Previous message in a doubly linked list. + MessagePool *mMessagePool; // Message pool for this message. + void * mQueue; // The queue where message is queued (if any). Queue type from `mInPriorityQ`. + uint32_t mDatagramTag; // The datagram tag used for 6LoWPAN frags or IPv6fragmentation. + uint16_t mReserved; // Number of reserved bytes (for header). + uint16_t mLength; // Current message length (number of bytes). + uint16_t mOffset; // A byte offset within the message. + uint16_t mMeshDest; // Used for unicast non-link-local messages. + uint16_t mPanId; // PAN ID (used for MLE Discover Request and Response). + uint8_t mChannel; // The message channel (used for MLE Announce). + uint8_t mTimeout; // Seconds remaining before dropping the message. + RssAverager mRssAverager; // The averager maintaining the received signal strength (RSS) average. +#if OPENTHREAD_CONFIG_MLE_LINK_METRICS_SUBJECT_ENABLE + LqiAverager mLqiAverager; // The averager maintaining the Link quality indicator (LQI) average. +#endif + ChildMask mChildMask; // ChildMask to indicate which sleepy children need to receive this. - /** - * This method returns the message metadata in the first message buffer. - * - * @returns The message metadata structure. - * - */ - const MessageMetadata &GetMetadata(void) const { return mBuffer.mHead.mMetadata; } + uint8_t mType : 3; // The message type. + uint8_t mSubType : 4; // The message sub type. + bool mDirectTx : 1; // Whether a direct transmission is required. + bool mLinkSecurity : 1; // Whether link security is enabled. + uint8_t mPriority : 2; // The message priority level (higher value is higher priority). + bool mInPriorityQ : 1; // Whether the message is queued in normal or priority queue. + bool mTxSuccess : 1; // Whether the direct tx of the message was successful. + bool mDoNotEvict : 1; // Whether this message may be evicted. + bool mMulticastLoop : 1; // Whether this multicast message may be looped back. +#if OPENTHREAD_CONFIG_MULTI_RADIO + uint8_t mRadioType : 2; // The radio link type the message was received on, or should be sent on. + bool mIsRadioTypeSet : 1; // Whether the radio type is set. + static_assert(Mac::kNumRadioTypes <= (1 << 2), "mRadioType bitfield cannot store all radio type values"); +#endif +#if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE + bool mTimeSync : 1; // Whether the message is also used for time sync purpose. + int64_t mNetworkTimeOffset; // The time offset to the Thread network time, in microseconds. + uint8_t mTimeSyncSeq; // The time sync sequence. +#endif + }; - /** - * This method returns a pointer to the first byte of data in the first message buffer. - * - * @returns A pointer to the first data byte. - * - */ - uint8_t *GetFirstData(void) { return mBuffer.mHead.mData; } - - /** - * This method returns a pointer to the first byte of data in the first message buffer. - * - * @returns A pointer to the first data byte. - * - */ - const uint8_t *GetFirstData(void) const { return mBuffer.mHead.mData; } - - /** - * This method returns a pointer to the first data byte of a subsequent message buffer. - * - * @returns A pointer to the first data byte. - * - */ - uint8_t *GetData(void) { return mBuffer.mData; } - - /** - * This method returns a pointer to the first data byte of a subsequent message buffer. - * - * @returns A pointer to the first data byte. - * - */ - const uint8_t *GetData(void) const { return mBuffer.mData; } + static_assert(kBufferSize > sizeof(Metadata) + sizeof(otMessageBuffer), "Metadata does not fit in a single buffer"); static constexpr uint16_t kBufferDataSize = kBufferSize - sizeof(otMessageBuffer); - static constexpr uint16_t kHeadBufferDataSize = kBufferDataSize - sizeof(MessageMetadata); + static constexpr uint16_t kHeadBufferDataSize = kBufferDataSize - sizeof(Metadata); -protected: + Metadata & GetMetadata(void) { return mBuffer.mHead.mMetadata; } + const Metadata &GetMetadata(void) const { return mBuffer.mHead.mMetadata; } + + uint8_t * GetFirstData(void) { return mBuffer.mHead.mData; } + const uint8_t *GetFirstData(void) const { return mBuffer.mHead.mData; } + + uint8_t * GetData(void) { return mBuffer.mData; } + const uint8_t *GetData(void) const { return mBuffer.mData; } + +private: union { struct { - MessageMetadata mMetadata; - uint8_t mData[kHeadBufferDataSize]; + Metadata mMetadata; + uint8_t mData[kHeadBufferDataSize]; } mHead; uint8_t mData[kBufferDataSize]; } mBuffer; }; +static_assert(sizeof(Buffer) >= kBufferSize, "Buffer size if not valid"); + /** * This class represents a message. * @@ -947,7 +896,7 @@ public: * @returns The IEEE 802.15.4 Destination PAN ID. * */ - uint16_t GetPanId(void) const { return GetMetadata().mPanIdChannel.mPanId; } + uint16_t GetPanId(void) const { return GetMetadata().mPanId; } /** * This method sets the IEEE 802.15.4 Destination PAN ID. @@ -957,7 +906,7 @@ public: * @param[in] aPanId The IEEE 802.15.4 Destination PAN ID. * */ - void SetPanId(uint16_t aPanId) { GetMetadata().mPanIdChannel.mPanId = aPanId; } + void SetPanId(uint16_t aPanId) { GetMetadata().mPanId = aPanId; } /** * This method returns the IEEE 802.15.4 Channel to use for transmission. @@ -967,7 +916,7 @@ public: * @returns The IEEE 802.15.4 Channel to use for transmission. * */ - uint8_t GetChannel(void) const { return GetMetadata().mPanIdChannel.mChannel; } + uint8_t GetChannel(void) const { return GetMetadata().mChannel; } /** * This method sets the IEEE 802.15.4 Channel to use for transmission. @@ -977,7 +926,7 @@ public: * @param[in] aChannel The IEEE 802.15.4 Channel to use for transmission. * */ - void SetChannel(uint8_t aChannel) { GetMetadata().mPanIdChannel.mChannel = aChannel; } + void SetChannel(uint8_t aChannel) { GetMetadata().mChannel = aChannel; } /** * This method returns the timeout used for 6LoWPAN reassembly. @@ -1121,9 +1070,9 @@ public: uint8_t GetAverageLqi(void) const { return GetMetadata().mLqiAverager.GetAverage(); } /** - * This mehod returns the count of frames counted so far. + * This method returns the count of frames counted so far. * - * @retruns The count of frames that have been counted. + * @returns The count of frames that have been counted. * */ uint8_t GetPsduCount(void) const { return GetMetadata().mLqiAverager.GetCount(); } @@ -1145,7 +1094,7 @@ public: */ MessageQueue *GetMessageQueue(void) const { - return (!GetMetadata().mInPriorityQ) ? GetMetadata().mQueue.mMessage : nullptr; + return !GetMetadata().mInPriorityQ ? static_cast(GetMetadata().mQueue) : nullptr; } /** @@ -1156,7 +1105,7 @@ public: */ PriorityQueue *GetPriorityQueue(void) const { - return (GetMetadata().mInPriorityQ) ? GetMetadata().mQueue.mPriority : nullptr; + return GetMetadata().mInPriorityQ ? static_cast(GetMetadata().mQueue) : nullptr; } /** @@ -1254,98 +1203,9 @@ public: #endif // #if OPENTHREAD_CONFIG_MULTI_RADIO -private: - /** - * This method returns a pointer to the message pool to which this message belongs - * - * @returns A pointer to the message pool. - * - */ - MessagePool *GetMessagePool(void) const { return GetMetadata().mMessagePool; } - - /** - * This method sets the message pool this message to which this message belongs. - * - * @param[in] aMessagePool A pointer to the message pool - * - */ - void SetMessagePool(MessagePool *aMessagePool) { GetMetadata().mMessagePool = aMessagePool; } - - /** - * This method returns `true` if the message is enqueued in any queue (`MessageQueue` or `PriorityQueue`). - * - * @returns `true` if the message is in any queue, `false` otherwise. - * - */ - bool IsInAQueue(void) const { return (GetMetadata().mQueue.mMessage != nullptr); } - - /** - * This method sets the message queue information for the message. - * - * @param[in] aMessageQueue A pointer to the message queue where this message is queued. - * - */ - void SetMessageQueue(MessageQueue *aMessageQueue); - - /** - * This method sets the message queue information for the message. - * - * @param[in] aPriorityQueue A pointer to the priority queue where this message is queued. - * - */ - void SetPriorityQueue(PriorityQueue *aPriorityQueue); - - /** - * This method returns a reference to the `mNext` pointer. - * - * @returns A reference to the mNext pointer. - * - */ - Message *&Next(void) { return GetMetadata().mNext; } - - /** - * This method returns a reference to the `mNext` pointer (const pointer). - * - * - * @returns A reference to the mNext pointer. - * - */ - Message *const &Next(void) const { return GetMetadata().mNext; } - - /** - * This method returns a reference to the `mPrev` pointer. - * - * @returns A reference to the mPrev pointer. - * - */ - Message *&Prev(void) { return GetMetadata().mPrev; } - - /** - * This method returns the number of reserved header bytes. - * - * @returns The number of reserved header bytes. - * - */ +protected: uint16_t GetReserved(void) const { return GetMetadata().mReserved; } - - /** - * This method sets the number of reserved header bytes. - * - * @param[in] aReservedHeader The number of header bytes to reserve. - * - */ - void SetReserved(uint16_t aReservedHeader) { GetMetadata().mReserved = aReservedHeader; } - - /** - * This method adds or frees message buffers to meet the requested length. - * - * @param[in] aLength The number of bytes that the message buffer needs to handle. - * - * @retval kErrorNone Successfully resized the message. - * @retval kErrorNoBufs Could not grow the message due to insufficient available message buffers. - * - */ - Error ResizeMessage(uint16_t aLength); + void SetReserved(uint16_t aReservedHeader) { GetMetadata().mReserved = aReservedHeader; } private: struct Chunk @@ -1375,6 +1235,19 @@ private: { AsConst(this)->GetNextChunk(aLength, static_cast(aChunk)); } + + MessagePool *GetMessagePool(void) const { return GetMetadata().mMessagePool; } + void SetMessagePool(MessagePool *aMessagePool) { GetMetadata().mMessagePool = aMessagePool; } + + bool IsInAQueue(void) const { return (GetMetadata().mQueue != nullptr); } + void SetMessageQueue(MessageQueue *aMessageQueue); + void SetPriorityQueue(PriorityQueue *aPriorityQueue); + + Message *& Next(void) { return GetMetadata().mNext; } + Message *const &Next(void) const { return GetMetadata().mNext; } + Message *& Prev(void) { return GetMetadata().mPrev; } + + Error ResizeMessage(uint16_t aLength); }; /** @@ -1461,21 +1334,8 @@ public: void GetInfo(uint16_t &aMessageCount, uint16_t &aBufferCount) const; private: - /** - * This method returns the tail of the list (last message in the list) - * - * @returns A pointer to the tail of the list. - * - */ Message *GetTail(void) const { return static_cast(mData); } - - /** - * This method set the tail of the list. - * - * @param[in] aMessage A pointer to the message to set as new tail. - * - */ - void SetTail(Message *aMessage) { mData = aMessage; } + void SetTail(Message *aMessage) { mData = aMessage; } }; /** @@ -1562,32 +1422,14 @@ public: Message *GetTail(void) const; private: - /** - * This method increases (moves forward) the given priority while ensuring to wrap from - * priority value `kNumPriorities` -1 back to 0. - * - * @param[in] aPriority A given priority level - * - * @returns Increased/Moved forward priority level - */ uint8_t PrevPriority(uint8_t aPriority) const { return (aPriority == Message::kNumPriorities - 1) ? 0 : (aPriority + 1); } - /** - * This private method finds the first non-nullptr tail starting from the given priority level and moving forward. - * It wraps from priority value `kNumPriorities` -1 back to 0. - * - * aStartPriorityLevel Starting priority level. - * - * @returns The first non-nullptr tail pointer, or nullptr if all the - * - */ Message *FindFirstNonNullTail(Message::Priority aStartPriorityLevel) const; -private: - Message *mTails[Message::kNumPriorities]; ///< Tail pointers associated with different priority levels. + Message *mTails[Message::kNumPriorities]; // Tail pointers associated with different priority levels. }; /** @@ -1621,7 +1463,6 @@ public: */ Message *New(Message::Type aType, uint16_t aReserveHeader, Message::Priority aPriority); -public: /** * This method is used to obtain a new message with specified settings. *