From 6bc02986f08e5c407eb993e75b7205cda2010229 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 16 Oct 2023 11:31:08 -0700 Subject: [PATCH] [ip6] use `OwnedPtr` to simplify message lifetime (#9533) This commit updates the `Ip6` class to use `OwnedPtr`. This smart pointer automates the freeing of messages and helps to simplify the code by removing all the logic that tracks whether a message needs to be freed. It also makes the transfer of ownership of `Message` instances between methods more clear. --- src/core/api/ip6_api.cpp | 2 +- src/core/net/ip6.cpp | 237 ++++++++++--------------- src/core/net/ip6.hpp | 34 ++-- src/core/net/nat64_translator.cpp | 2 +- src/core/thread/child_supervision.cpp | 15 +- src/core/thread/mesh_forwarder.cpp | 15 +- src/core/thread/mesh_forwarder.hpp | 9 +- src/core/thread/mesh_forwarder_ftd.cpp | 67 ++++--- src/core/thread/mesh_forwarder_mtd.cpp | 18 +- 9 files changed, 172 insertions(+), 227 deletions(-) diff --git a/src/core/api/ip6_api.cpp b/src/core/api/ip6_api.cpp index d545fd38f..d70557069 100644 --- a/src/core/api/ip6_api.cpp +++ b/src/core/api/ip6_api.cpp @@ -141,7 +141,7 @@ otError otIp6Send(otInstance *aInstance, otMessage *aMessage) VerifyOrExit(!AsCoreType(aMessage).IsOriginThreadNetif(), error = kErrorInvalidArgs); - error = AsCoreType(aInstance).Get().SendRaw(AsCoreType(aMessage)); + error = AsCoreType(aInstance).Get().SendRaw(OwnedPtr(AsCoreTypePtr(aMessage))); exit: return error; diff --git a/src/core/net/ip6.cpp b/src/core/net/ip6.cpp index 61926c5d9..673217bff 100644 --- a/src/core/net/ip6.cpp +++ b/src/core/net/ip6.cpp @@ -494,7 +494,7 @@ void Ip6::HandleSendQueue(void) while ((message = mSendQueue.GetHead()) != nullptr) { mSendQueue.Dequeue(*message); - IgnoreError(HandleDatagram(*message)); + IgnoreError(HandleDatagram(OwnedPtr(message))); } } @@ -703,7 +703,8 @@ Error Ip6::HandleFragment(Message &aMessage, MessageInfo &aMessageInfo) mReassemblyList.Dequeue(*message); - IgnoreError(HandleDatagram(*message, aMessageInfo.mLinkInfo, /* aIsReassembled */ true)); + IgnoreError(HandleDatagram(OwnedPtr(message), aMessageInfo.mLinkInfo, + /* aIsReassembled */ true)); } exit: @@ -805,11 +806,11 @@ exit: } #endif // OPENTHREAD_CONFIG_IP6_FRAGMENTATION_ENABLE -Error Ip6::HandleExtensionHeaders(Message &aMessage, - MessageInfo &aMessageInfo, - Header &aHeader, - uint8_t &aNextHeader, - bool &aReceive) +Error Ip6::HandleExtensionHeaders(OwnedPtr &aMessagePtr, + MessageInfo &aMessageInfo, + Header &aHeader, + uint8_t &aNextHeader, + bool &aReceive) { Error error = kErrorNone; @@ -817,22 +818,22 @@ Error Ip6::HandleExtensionHeaders(Message &aMessage, while (aReceive || aNextHeader == kProtoHopOpts) { - SuccessOrExit(error = aMessage.Read(aMessage.GetOffset(), extHeader)); + SuccessOrExit(error = aMessagePtr->Read(aMessagePtr->GetOffset(), extHeader)); switch (aNextHeader) { case kProtoHopOpts: - SuccessOrExit(error = HandleOptions(aMessage, aHeader, aReceive)); + SuccessOrExit(error = HandleOptions(*aMessagePtr, aHeader, aReceive)); break; case kProtoFragment: - IgnoreError(PassToHost(aMessage, aMessageInfo, aNextHeader, + IgnoreError(PassToHost(aMessagePtr, aMessageInfo, aNextHeader, /* aApplyFilter */ false, aReceive, Message::kCopyToUse)); - SuccessOrExit(error = HandleFragment(aMessage, aMessageInfo)); + SuccessOrExit(error = HandleFragment(*aMessagePtr, aMessageInfo)); break; case kProtoDstOpts: - SuccessOrExit(error = HandleOptions(aMessage, aHeader, aReceive)); + SuccessOrExit(error = HandleOptions(*aMessagePtr, aHeader, aReceive)); break; case kProtoIp6: @@ -853,8 +854,26 @@ exit: return error; } +Error Ip6::TakeOrCopyMessagePtr(OwnedPtr &aTargetPtr, + OwnedPtr &aMessagePtr, + Message::Ownership aMessageOwnership) +{ + switch (aMessageOwnership) + { + case Message::kTakeCustody: + aTargetPtr = aMessagePtr.PassOwnership(); + break; + + case Message::kCopyToUse: + aTargetPtr.Reset(aMessagePtr->Clone()); + break; + } + + return (aTargetPtr != nullptr) ? kErrorNone : kErrorNoBufs; +} + Error Ip6::HandlePayload(Header &aIp6Header, - Message &aMessage, + OwnedPtr &aMessagePtr, MessageInfo &aMessageInfo, uint8_t aIpProto, Message::Ownership aMessageOwnership) @@ -863,8 +882,8 @@ Error Ip6::HandlePayload(Header &aIp6Header, OT_UNUSED_VARIABLE(aIp6Header); #endif - Error error = kErrorNone; - Message *message = (aMessageOwnership == Message::kTakeCustody) ? &aMessage : nullptr; + Error error = kErrorNone; + OwnedPtr messagePtr; switch (aIpProto) { @@ -879,18 +898,13 @@ Error Ip6::HandlePayload(Header &aIp6Header, ExitNow(); } - if (aMessageOwnership == Message::kCopyToUse) - { - message = aMessage.Clone(); - } - - VerifyOrExit(message != nullptr, error = kErrorNoBufs); + SuccessOrExit(error = TakeOrCopyMessagePtr(messagePtr, aMessagePtr, aMessageOwnership)); switch (aIpProto) { #if OPENTHREAD_CONFIG_TCP_ENABLE case kProtoTcp: - error = mTcp.HandleMessage(aIp6Header, *message, aMessageInfo); + error = mTcp.HandleMessage(aIp6Header, *messagePtr, aMessageInfo); if (error == kErrorDrop) { LogNote("Error TCP Checksum"); @@ -898,7 +912,7 @@ Error Ip6::HandlePayload(Header &aIp6Header, break; #endif case kProtoUdp: - error = mUdp.HandleMessage(*message, aMessageInfo); + error = mUdp.HandleMessage(*messagePtr, aMessageInfo); if (error == kErrorDrop) { LogNote("Error UDP Checksum"); @@ -906,7 +920,7 @@ Error Ip6::HandlePayload(Header &aIp6Header, break; case kProtoIcmp6: - error = mIcmp.HandleMessage(*message, aMessageInfo); + error = mIcmp.HandleMessage(*messagePtr, aMessageInfo); break; default: @@ -919,12 +933,10 @@ exit: LogNote("Failed to handle payload: %s", ErrorToString(error)); } - FreeMessage(message); - return error; } -Error Ip6::PassToHost(Message &aMessage, +Error Ip6::PassToHost(OwnedPtr &aMessagePtr, const MessageInfo &aMessageInfo, uint8_t aIpProto, bool aApplyFilter, @@ -936,28 +948,15 @@ Error Ip6::PassToHost(Message &aMessage, // may also perform translation and invoke IPv4 receive // callback. - Error error = kErrorNone; - Message *message = nullptr; + Error error = kErrorNone; + OwnedPtr messagePtr; - // `message` points to the `Message` instance we own in this - // method. If we can take ownership of `aMessage`, we use it as - // `message`. Otherwise, we may create a clone of it and use as - // `message`. `message` variable will be set to `nullptr` if the - // message ownership is transferred to an invoked callback. At - // the end of this method we free `message` if it is not `nullptr` - // indicating it was not passed to a callback. - - if (aMessageOwnership == Message::kTakeCustody) - { - message = &aMessage; - } - - VerifyOrExit(aMessage.IsLoopbackToHostAllowed(), error = kErrorNoRoute); + VerifyOrExit(aMessagePtr->IsLoopbackToHostAllowed(), error = kErrorNoRoute); VerifyOrExit(mReceiveIp6DatagramCallback.IsSet(), error = kErrorNoRoute); // Do not pass IPv6 packets that exceed kMinimalMtu. - VerifyOrExit(aMessage.GetLength() <= kMinimalMtu, error = kErrorDrop); + VerifyOrExit(aMessagePtr->GetLength() <= kMinimalMtu, error = kErrorDrop); // If the sender used mesh-local address as source, do not pass to // host unless this message is intended for this device itself. @@ -986,7 +985,7 @@ Error Ip6::PassToHost(Message &aMessage, { Icmp::Header icmp; - IgnoreError(aMessage.Read(aMessage.GetOffset(), icmp)); + IgnoreError(aMessagePtr->Read(aMessagePtr->GetOffset(), icmp)); VerifyOrExit(icmp.GetType() != Icmp::Header::kTypeEchoRequest, error = kErrorDrop); } @@ -996,7 +995,7 @@ Error Ip6::PassToHost(Message &aMessage, { Udp::Header udp; - IgnoreError(aMessage.Read(aMessage.GetOffset(), udp)); + IgnoreError(aMessagePtr->Read(aMessagePtr->GetOffset(), udp)); VerifyOrExit(Get().ShouldUsePlatformUdp(udp.GetDestinationPort()) && !Get().IsPortInUse(udp.GetDestinationPort()), error = kErrorNoRoute); @@ -1016,27 +1015,12 @@ Error Ip6::PassToHost(Message &aMessage, } } - switch (aMessageOwnership) - { - case Message::kTakeCustody: - break; + SuccessOrExit(error = TakeOrCopyMessagePtr(messagePtr, aMessagePtr, aMessageOwnership)); - case Message::kCopyToUse: - message = aMessage.Clone(); - - if (message == nullptr) - { - LogWarn("No buff to clone msg (len: %d) to pass to host", aMessage.GetLength()); - ExitNow(error = kErrorNoBufs); - } - - break; - } - - IgnoreError(RemoveMplOption(*message)); + IgnoreError(RemoveMplOption(*messagePtr)); #if OPENTHREAD_CONFIG_NAT64_TRANSLATOR_ENABLE - switch (Get().TranslateFromIp6(aMessage)) + switch (Get().TranslateFromIp6(*messagePtr)) { case Nat64::Translator::kNotTranslated: break; @@ -1047,37 +1031,33 @@ Error Ip6::PassToHost(Message &aMessage, case Nat64::Translator::kForward: VerifyOrExit(mReceiveIp4DatagramCallback.IsSet(), error = kErrorNoRoute); // Pass message to callback transferring its ownership. - mReceiveIp4DatagramCallback.Invoke(message); - message = nullptr; + mReceiveIp4DatagramCallback.Invoke(messagePtr.Release()); ExitNow(); } #endif - // Pass message to callback transferring its ownership. - mReceiveIp6DatagramCallback.Invoke(message); - message = nullptr; - #if OPENTHREAD_CONFIG_IP6_BR_COUNTERS_ENABLE { Header header; - IgnoreError(header.ParseFrom(aMessage)); - UpdateBorderRoutingCounters(header, aMessage.GetLength(), /* aIsInbound */ false); + IgnoreError(header.ParseFrom(*messagePtr)); + UpdateBorderRoutingCounters(header, messagePtr->GetLength(), /* aIsInbound */ false); } #endif + // Pass message to callback transferring its ownership. + mReceiveIp6DatagramCallback.Invoke(messagePtr.Release()); + exit: - FreeMessage(message); return error; } -Error Ip6::SendRaw(Message &aMessage) +Error Ip6::SendRaw(OwnedPtr aMessagePtr) { Error error = kErrorNone; Header header; - bool freed = false; - SuccessOrExit(error = header.ParseFrom(aMessage)); + SuccessOrExit(error = header.ParseFrom(*aMessagePtr)); VerifyOrExit(!header.GetSource().IsMulticast(), error = kErrorInvalidSourceAddress); #if OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE @@ -1096,27 +1076,20 @@ Error Ip6::SendRaw(Message &aMessage) if (header.GetDestination().IsMulticast()) { - SuccessOrExit(error = InsertMplOption(aMessage, header)); + SuccessOrExit(error = InsertMplOption(*aMessagePtr, header)); } - error = HandleDatagram(aMessage); - freed = true; - #if OPENTHREAD_CONFIG_IP6_BR_COUNTERS_ENABLE - UpdateBorderRoutingCounters(header, aMessage.GetLength(), /* aIsInbound */ true); + UpdateBorderRoutingCounters(header, aMessagePtr->GetLength(), /* aIsInbound */ true); #endif + error = HandleDatagram(aMessagePtr.PassOwnership()); + exit: - - if (!freed) - { - aMessage.Free(); - } - return error; } -Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool aIsReassembled) +Error Ip6::HandleDatagram(OwnedPtr aMessagePtr, const void *aLinkMessageInfo, bool aIsReassembled) { Error error; MessageInfo messageInfo; @@ -1124,15 +1097,13 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool bool receive; bool forwardThread; bool forwardHost; - bool shouldFreeMessage; uint8_t nextHeader; - receive = false; - forwardThread = false; - forwardHost = false; - shouldFreeMessage = true; + receive = false; + forwardThread = false; + forwardHost = false; - SuccessOrExit(error = header.ParseFrom(aMessage)); + SuccessOrExit(error = header.ParseFrom(*aMessagePtr)); messageInfo.Clear(); messageInfo.SetPeerAddr(header.GetSource()); @@ -1148,10 +1119,10 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool { // Destination is multicast - forwardThread = !aMessage.IsOriginThreadNetif(); + forwardThread = !aMessagePtr->IsOriginThreadNetif(); #if OPENTHREAD_FTD - if (aMessage.IsOriginThreadNetif() && header.GetDestination().IsMulticastLargerThanRealmLocal() && + if (aMessagePtr->IsOriginThreadNetif() && header.GetDestination().IsMulticastLargerThanRealmLocal() && Get().HasSleepyChildWithAddress(header.GetDestination())) { forwardThread = true; @@ -1160,7 +1131,7 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool forwardHost = header.GetDestination().IsMulticastLargerThanRealmLocal(); - if ((aMessage.IsOriginThreadNetif() || aMessage.GetMulticastLoop()) && + if ((aMessagePtr->IsOriginThreadNetif() || aMessagePtr->GetMulticastLoop()) && Get().IsMulticastSubscribed(header.GetDestination())) { receive = true; @@ -1178,7 +1149,7 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool { receive = true; } - else if (!aMessage.IsOriginThreadNetif() || !header.GetDestination().IsLinkLocal()) + else if (!aMessagePtr->IsOriginThreadNetif() || !header.GetDestination().IsLinkLocal()) { if (header.GetDestination().IsLinkLocal()) { @@ -1187,7 +1158,7 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool else if (IsOnLink(header.GetDestination())) { #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_BACKBONE_ROUTER_DUA_NDPROXYING_ENABLE - forwardThread = (!aMessage.IsLoopbackToHostAllowed() || + forwardThread = (!aMessagePtr->IsLoopbackToHostAllowed() || !Get().ShouldForwardDuaToBackbone(header.GetDestination())); #else forwardThread = true; @@ -1202,11 +1173,11 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool } } - aMessage.SetOffset(sizeof(header)); + aMessagePtr->SetOffset(sizeof(header)); // Process IPv6 Extension Headers nextHeader = static_cast(header.GetNextHeader()); - SuccessOrExit(error = HandleExtensionHeaders(aMessage, messageInfo, header, nextHeader, receive)); + SuccessOrExit(error = HandleExtensionHeaders(aMessagePtr, messageInfo, header, nextHeader, receive)); if (receive && (nextHeader == kProtoIp6)) { @@ -1216,25 +1187,17 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool // itself and use it directly. The encapsulating header is // then removed before processing the embedded message. - Message *messageCopy; + OwnedPtr messagePtr; + bool multicastLoop = aMessagePtr->GetMulticastLoop(); - if (forwardThread) - { - messageCopy = aMessage.Clone(); - VerifyOrExit(messageCopy != nullptr, error = kErrorNoBufs); - messageCopy->SetMulticastLoop(aMessage.GetMulticastLoop()); - } - else - { - messageCopy = &aMessage; - shouldFreeMessage = false; - } + SuccessOrExit(error = TakeOrCopyMessagePtr(messagePtr, aMessagePtr, + forwardThread ? Message::kCopyToUse : Message::kTakeCustody)); + messagePtr->SetMulticastLoop(multicastLoop); + messagePtr->RemoveHeader(messagePtr->GetOffset()); - messageCopy->RemoveHeader(messageCopy->GetOffset()); + Get().LogMessage(MeshForwarder::kMessageReceive, *messagePtr); - Get().LogMessage(MeshForwarder::kMessageReceive, *messageCopy); - - IgnoreError(HandleDatagram(*messageCopy, aLinkMessageInfo, aIsReassembled)); + IgnoreError(HandleDatagram(messagePtr.PassOwnership(), aLinkMessageInfo, aIsReassembled)); receive = false; forwardHost = false; @@ -1242,28 +1205,20 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool if ((forwardHost || receive) && !aIsReassembled) { - error = PassToHost(aMessage, messageInfo, nextHeader, + error = PassToHost(aMessagePtr, messageInfo, nextHeader, /* aApplyFilter */ !forwardHost, receive, (receive || forwardThread) ? Message::kCopyToUse : Message::kTakeCustody); - - // Need to free the message if we did not pass its - // ownership in the call to `PassToHost()` - shouldFreeMessage = (receive || forwardThread); } if (receive) { - error = HandlePayload(header, aMessage, messageInfo, nextHeader, + error = HandlePayload(header, aMessagePtr, messageInfo, nextHeader, forwardThread ? Message::kCopyToUse : Message::kTakeCustody); - - // Need to free the message if we did not pass its - // ownership in the call to `HandlePayload()` - shouldFreeMessage = forwardThread; } if (forwardThread) { - if (aMessage.IsOriginThreadNetif()) + if (aMessagePtr->IsOriginThreadNetif()) { VerifyOrExit(Get().IsRouterOrLeader()); header.SetHopLimit(header.GetHopLimit() - 1); @@ -1271,13 +1226,13 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool VerifyOrExit(header.GetHopLimit() > 0, error = kErrorDrop); - aMessage.Write(Header::kHopLimitFieldOffset, header.GetHopLimit()); + aMessagePtr->Write(Header::kHopLimitFieldOffset, header.GetHopLimit()); if (nextHeader == kProtoIcmp6) { uint8_t icmpType; - SuccessOrExit(error = aMessage.Read(aMessage.GetOffset(), icmpType)); + SuccessOrExit(error = aMessagePtr->Read(aMessagePtr->GetOffset(), icmpType)); error = kErrorDrop; @@ -1293,11 +1248,12 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool SuccessOrExit(error); } - if (aMessage.IsOriginHostUntrusted() && (nextHeader == kProtoUdp)) + if (aMessagePtr->IsOriginHostUntrusted() && (nextHeader == kProtoUdp)) { uint16_t destPort; - SuccessOrExit(error = aMessage.Read(aMessage.GetOffset() + Udp::Header::kDestPortFieldOffset, destPort)); + SuccessOrExit( + error = aMessagePtr->Read(aMessagePtr->GetOffset() + Udp::Header::kDestPortFieldOffset, destPort)); destPort = HostSwap16(destPort); if (destPort == Tmf::kUdpPort) @@ -1308,11 +1264,12 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool } #if !OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE - if (aMessage.IsOriginHostTrusted() && !aMessage.IsLoopbackToHostAllowed() && (nextHeader == kProtoUdp)) + if (aMessagePtr->IsOriginHostTrusted() && !aMessagePtr->IsLoopbackToHostAllowed() && (nextHeader == kProtoUdp)) { uint16_t destPort; - SuccessOrExit(error = aMessage.Read(aMessage.GetOffset() + Udp::Header::kDestPortFieldOffset, destPort)); + SuccessOrExit( + error = aMessagePtr->Read(aMessagePtr->GetOffset() + Udp::Header::kDestPortFieldOffset, destPort)); destPort = HostSwap16(destPort); if (nextHeader == kProtoUdp) @@ -1327,21 +1284,13 @@ Error Ip6::HandleDatagram(Message &aMessage, const void *aLinkMessageInfo, bool // type on the message to allow the radio type for tx to be // selected later (based on the radios supported by the next // hop). - aMessage.ClearRadioType(); + aMessagePtr->ClearRadioType(); #endif - // `SendMessage()` takes custody of message in the success case - SuccessOrExit(error = Get().SendMessage(aMessage)); - shouldFreeMessage = false; + Get().SendMessage(aMessagePtr.PassOwnership()); } exit: - - if (shouldFreeMessage) - { - aMessage.Free(); - } - return error; } diff --git a/src/core/net/ip6.hpp b/src/core/net/ip6.hpp index c2a2e530e..945b0d041 100644 --- a/src/core/net/ip6.hpp +++ b/src/core/net/ip6.hpp @@ -49,6 +49,7 @@ #include "common/log.hpp" #include "common/message.hpp" #include "common/non_copyable.hpp" +#include "common/owned_ptr.hpp" #include "common/time_ticker.hpp" #include "common/timer.hpp" #include "net/icmp6.hpp" @@ -194,10 +195,7 @@ public: /** * Sends a raw IPv6 datagram with a fully formed IPv6 header. * - * The caller transfers ownership of @p aMessage when making this call. OpenThread will free @p aMessage when - * processing is complete, including when a value other than `kErrorNone` is returned. - * - * @param[in] aMessage A reference to the message. + * @param[in] aMessage An owned pointer to a message (ownership is transferred to the method). * * @retval kErrorNone Successfully processed the message. * @retval kErrorDrop Message was well-formed but not fully processed due to packet processing rules. @@ -206,12 +204,12 @@ public: * @retval kErrorParse Encountered a malformed header when processing the message. * */ - Error SendRaw(Message &aMessage); + Error SendRaw(OwnedPtr aMessage); /** * Processes a received IPv6 datagram. * - * @param[in] aMessage A reference to the message. + * @param[in] aMessage An owned pointer to a message. * @param[in] aLinkMessageInfo A pointer to link-specific message information. * * @retval kErrorNone Successfully processed the message. @@ -221,7 +219,9 @@ public: * @retval kErrorParse Encountered a malformed header when processing the message. * */ - Error HandleDatagram(Message &aMessage, const void *aLinkMessageInfo = nullptr, bool aIsReassembled = false); + Error HandleDatagram(OwnedPtr aMessagePtr, + const void *aLinkMessageInfo = nullptr, + bool aIsReassembled = false); /** * Registers a callback to provide received raw IPv6 datagrams. @@ -362,22 +362,24 @@ private: static constexpr uint16_t kMinimalMtu = 1280; - void HandleSendQueue(void); - static uint8_t PriorityToDscp(Message::Priority aPriority); + static Error TakeOrCopyMessagePtr(OwnedPtr &aTargetPtr, + OwnedPtr &aMessagePtr, + Message::Ownership aMessageOwnership); void EnqueueDatagram(Message &aMessage); - Error PassToHost(Message &aMessage, + void HandleSendQueue(void); + Error PassToHost(OwnedPtr &aMessagePtr, const MessageInfo &aMessageInfo, uint8_t aIpProto, bool aApplyFilter, bool aReceive, Message::Ownership aMessageOwnership); - Error HandleExtensionHeaders(Message &aMessage, - MessageInfo &aMessageInfo, - Header &aHeader, - uint8_t &aNextHeader, - bool &aReceive); + Error HandleExtensionHeaders(OwnedPtr &aMessagePtr, + MessageInfo &aMessageInfo, + Header &aHeader, + uint8_t &aNextHeader, + bool &aReceive); Error FragmentDatagram(Message &aMessage, uint8_t aIpProto); Error HandleFragment(Message &aMessage, MessageInfo &aMessageInfo); #if OPENTHREAD_CONFIG_IP6_FRAGMENTATION_ENABLE @@ -392,7 +394,7 @@ private: Error RemoveMplOption(Message &aMessage); Error HandleOptions(Message &aMessage, Header &aHeader, bool &aReceive); Error HandlePayload(Header &aIp6Header, - Message &aMessage, + OwnedPtr &aMessagePtr, MessageInfo &aMessageInfo, uint8_t aIpProto, Message::Ownership aMessageOwnership); diff --git a/src/core/net/nat64_translator.cpp b/src/core/net/nat64_translator.cpp index 0415f909f..5afe9bfb0 100644 --- a/src/core/net/nat64_translator.cpp +++ b/src/core/net/nat64_translator.cpp @@ -99,7 +99,7 @@ Error Translator::SendMessage(Message &aMessage) VerifyOrExit(result == kForward); - error = Get().SendRaw(aMessage); + error = Get().SendRaw(OwnedPtr(&aMessage).PassOwnership()); freed = true; exit: diff --git a/src/core/thread/child_supervision.cpp b/src/core/thread/child_supervision.cpp index 02ba64ada..f4bcd580c 100644 --- a/src/core/thread/child_supervision.cpp +++ b/src/core/thread/child_supervision.cpp @@ -67,13 +67,13 @@ exit: void ChildSupervisor::SendMessage(Child &aChild) { - Message *message = nullptr; - uint16_t childIndex; + OwnedPtr messagePtr; + uint16_t childIndex; VerifyOrExit(aChild.GetIndirectMessageCount() == 0); - message = Get().Allocate(Message::kTypeSupervision, sizeof(uint8_t)); - VerifyOrExit(message != nullptr); + messagePtr.Reset(Get().Allocate(Message::kTypeSupervision, sizeof(uint8_t))); + VerifyOrExit(messagePtr != nullptr); // Supervision message is an empty payload 15.4 data frame. // The child index is stored here in the message content to allow @@ -81,15 +81,14 @@ void ChildSupervisor::SendMessage(Child &aChild) // `ChildSupervisor::GetDestination(message)`. childIndex = Get().GetChildIndex(aChild); - SuccessOrExit(message->Append(childIndex)); + SuccessOrExit(messagePtr->Append(childIndex)); - SuccessOrExit(Get().SendMessage(*message)); - message = nullptr; + Get().SendMessage(messagePtr.PassOwnership()); LogInfo("Sending supervision message to child 0x%04x", aChild.GetRloc16()); exit: - FreeMessage(message); + return; } void ChildSupervisor::UpdateOnSend(Child &aChild) { aChild.ResetSecondsSinceLastSupervision(); } diff --git a/src/core/thread/mesh_forwarder.cpp b/src/core/thread/mesh_forwarder.cpp index 413321c28..6b70245c4 100644 --- a/src/core/thread/mesh_forwarder.cpp +++ b/src/core/thread/mesh_forwarder.cpp @@ -1666,7 +1666,7 @@ Error MeshForwarder::HandleDatagram(Message &aMessage, const ThreadLinkInfo &aLi aMessage.SetLoopbackToHostAllowed(true); aMessage.SetOrigin(Message::kOriginThreadNetif); - return Get().HandleDatagram(aMessage, &aLinkInfo); + return Get().HandleDatagram(OwnedPtr(&aMessage), &aLinkInfo); } Error MeshForwarder::GetFramePriority(const FrameData &aFrameData, @@ -1707,24 +1707,23 @@ exit: #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE Error MeshForwarder::SendEmptyMessage(void) { - Error error = kErrorNone; - Message *message = nullptr; + Error error = kErrorNone; + OwnedPtr messagePtr; VerifyOrExit(mEnabled && !Get().GetRxOnWhenIdle() && Get().GetParent().IsStateValidOrRestoring(), error = kErrorInvalidState); - message = Get().Allocate(Message::kTypeMacEmptyData); - VerifyOrExit(message != nullptr, error = kErrorNoBufs); + messagePtr.Reset(Get().Allocate(Message::kTypeMacEmptyData)); + VerifyOrExit(messagePtr != nullptr, error = kErrorNoBufs); - SuccessOrExit(error = SendMessage(*message)); + SendMessage(messagePtr.PassOwnership()); exit: - FreeMessageOnError(message, error); LogDebg("Send empty message, error:%s", ErrorToString(error)); return error; } -#endif // OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE +#endif bool MeshForwarder::CalcIePresent(const Message *aMessage) { diff --git a/src/core/thread/mesh_forwarder.hpp b/src/core/thread/mesh_forwarder.hpp index dfdc749fd..c27029d32 100644 --- a/src/core/thread/mesh_forwarder.hpp +++ b/src/core/thread/mesh_forwarder.hpp @@ -42,6 +42,7 @@ #include "common/locator.hpp" #include "common/log.hpp" #include "common/non_copyable.hpp" +#include "common/owned_ptr.hpp" #include "common/tasklet.hpp" #include "common/time_ticker.hpp" #include "mac/channel_mask.hpp" @@ -196,14 +197,10 @@ public: /** * Submits a message to the mesh forwarder for forwarding. * - * @param[in] aMessage A reference to the message. - * - * @retval kErrorNone Successfully enqueued the message. - * @retval kErrorAlready The message was already enqueued. - * @retval kErrorDrop The message could not be sent and should be dropped. + * @param[in] aMessagePtr An owned pointer to a message (transfer ownership). * */ - Error SendMessage(Message &aMessage); + void SendMessage(OwnedPtr aMessagePtr); #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE /** diff --git a/src/core/thread/mesh_forwarder_ftd.cpp b/src/core/thread/mesh_forwarder_ftd.cpp index 46cb31e0a..8ac143804 100644 --- a/src/core/thread/mesh_forwarder_ftd.cpp +++ b/src/core/thread/mesh_forwarder_ftd.cpp @@ -46,23 +46,23 @@ namespace ot { RegisterLogModule("MeshForwarder"); -Error MeshForwarder::SendMessage(Message &aMessage) +void MeshForwarder::SendMessage(OwnedPtr aMessagePtr) { - Error error = kErrorNone; + Message &message = *aMessagePtr.Release(); - aMessage.SetOffset(0); - aMessage.SetDatagramTag(0); - aMessage.SetTimestampToNow(); - mSendQueue.Enqueue(aMessage); + message.SetOffset(0); + message.SetDatagramTag(0); + message.SetTimestampToNow(); + mSendQueue.Enqueue(message); - switch (aMessage.GetType()) + switch (message.GetType()) { case Message::kTypeIp6: { Ip6::Header ip6Header; const Ip6::Address &destination = ip6Header.GetDestination(); - IgnoreError(aMessage.Read(0, ip6Header)); + IgnoreError(message.Read(0, ip6Header)); if (destination.IsMulticast()) { @@ -72,10 +72,10 @@ Error MeshForwarder::SendMessage(Message &aMessage) // device's sleepy child, thus there should be no direct transmission. if (!destination.IsMulticastLargerThanRealmLocal()) { - aMessage.SetDirectTransmission(); + message.SetDirectTransmission(); } - if (aMessage.GetSubType() != Message::kSubTypeMplRetransmission) + if (message.GetSubType() != Message::kSubTypeMplRetransmission) { // Check if we need to forward the multicast message // to any sleepy child. This is skipped for MPL retx @@ -89,7 +89,7 @@ Error MeshForwarder::SendMessage(Message &aMessage) { if (!child.IsRxOnWhenIdle() && (destinedForAll || child.HasIp6Address(destination))) { - mIndirectSender.AddMessageForSleepyChild(aMessage, child); + mIndirectSender.AddMessageForSleepyChild(message, child); } } } @@ -98,14 +98,14 @@ Error MeshForwarder::SendMessage(Message &aMessage) { Neighbor *neighbor = Get().FindNeighbor(destination); - if ((neighbor != nullptr) && !neighbor->IsRxOnWhenIdle() && !aMessage.IsDirectTransmission() && + if ((neighbor != nullptr) && !neighbor->IsRxOnWhenIdle() && !message.IsDirectTransmission() && Get().Contains(*neighbor)) { - mIndirectSender.AddMessageForSleepyChild(aMessage, *static_cast(neighbor)); + mIndirectSender.AddMessageForSleepyChild(message, *static_cast(neighbor)); } else { - aMessage.SetDirectTransmission(); + message.SetDirectTransmission(); } } @@ -114,33 +114,33 @@ Error MeshForwarder::SendMessage(Message &aMessage) case Message::kTypeSupervision: { - Child *child = Get().GetDestination(aMessage); + Child *child = Get().GetDestination(message); OT_ASSERT((child != nullptr) && !child->IsRxOnWhenIdle()); - mIndirectSender.AddMessageForSleepyChild(aMessage, *child); + mIndirectSender.AddMessageForSleepyChild(message, *child); break; } default: - aMessage.SetDirectTransmission(); + message.SetDirectTransmission(); break; } // Ensure that the message is marked for direct tx and/or for indirect tx // to a sleepy child. Otherwise, remove the message. - if (RemoveMessageIfNoPendingTx(aMessage)) + if (RemoveMessageIfNoPendingTx(message)) { ExitNow(); } #if (OPENTHREAD_CONFIG_MAX_FRAMES_IN_DIRECT_TX_QUEUE > 0) - ApplyDirectTxQueueLimit(aMessage); + ApplyDirectTxQueueLimit(message); #endif mScheduleTransmissionTask.Post(); exit: - return error; + return; } void MeshForwarder::HandleResolved(const Ip6::Address &aEid, Error aError) @@ -186,7 +186,7 @@ void MeshForwarder::HandleResolved(const Ip6::Address &aEid, Error aError) message.SetLoopbackToHostAllowed(true); message.SetOrigin(Message::kOriginHostTrusted); - IgnoreError(Get().HandleDatagram(message)); + IgnoreError(Get().HandleDatagram(OwnedPtr(&message))); continue; } #endif @@ -655,8 +655,7 @@ void MeshForwarder::SendDestinationUnreachable(uint16_t aMeshSource, const Ip6:: void MeshForwarder::HandleMesh(FrameData &aFrameData, const Mac::Address &aMacSource, const ThreadLinkInfo &aLinkInfo) { - Error error = kErrorNone; - Message *message = nullptr; + Error error = kErrorNone; Mac::Addresses meshAddrs; Lowpan::MeshHeader meshHeader; @@ -688,6 +687,7 @@ void MeshForwarder::HandleMesh(FrameData &aFrameData, const Mac::Address &aMacSo } else if (meshHeader.GetHopsLeft() > 0) { + OwnedPtr messagePtr; Message::Priority priority = Message::kPriorityNormal; Get().ResolveRoutingLoops(aMacSource.GetShort(), meshAddrs.mDestination.GetShort()); @@ -697,32 +697,32 @@ void MeshForwarder::HandleMesh(FrameData &aFrameData, const Mac::Address &aMacSo meshHeader.DecrementHopsLeft(); GetForwardFramePriority(aFrameData, meshAddrs, priority); - message = - Get().Allocate(Message::kType6lowpan, /* aReserveHeader */ 0, Message::Settings(priority)); - VerifyOrExit(message != nullptr, error = kErrorNoBufs); + messagePtr.Reset( + Get().Allocate(Message::kType6lowpan, /* aReserveHeader */ 0, Message::Settings(priority))); + VerifyOrExit(messagePtr != nullptr, error = kErrorNoBufs); - SuccessOrExit(error = meshHeader.AppendTo(*message)); - SuccessOrExit(error = message->AppendData(aFrameData)); + SuccessOrExit(error = meshHeader.AppendTo(*messagePtr)); + SuccessOrExit(error = messagePtr->AppendData(aFrameData)); - message->SetLinkInfo(aLinkInfo); + messagePtr->SetLinkInfo(aLinkInfo); #if OPENTHREAD_CONFIG_MULTI_RADIO // We set the received radio type on the message in order for it // to be logged correctly from LogMessage(). - message->SetRadioType(static_cast(aLinkInfo.mRadioType)); + messagePtr->SetRadioType(static_cast(aLinkInfo.mRadioType)); #endif - LogMessage(kMessageReceive, *message, kErrorNone, &aMacSource); + LogMessage(kMessageReceive, *messagePtr, kErrorNone, &aMacSource); #if OPENTHREAD_CONFIG_MULTI_RADIO // Since the message will be forwarded, we clear the radio // type on the message to allow the radio type for tx to be // selected later (based on the radios supported by the next // hop). - message->ClearRadioType(); + messagePtr->ClearRadioType(); #endif - IgnoreError(SendMessage(*message)); + SendMessage(messagePtr.PassOwnership()); } exit: @@ -731,7 +731,6 @@ exit: { LogInfo("Dropping rx mesh frame, error:%s, len:%d, src:%s, sec:%s", ErrorToString(error), aFrameData.GetLength(), aMacSource.ToString().AsCString(), ToYesNo(aLinkInfo.IsLinkSecurityEnabled())); - FreeMessage(message); } } diff --git a/src/core/thread/mesh_forwarder_mtd.cpp b/src/core/thread/mesh_forwarder_mtd.cpp index 5e697056b..b88c773cd 100644 --- a/src/core/thread/mesh_forwarder_mtd.cpp +++ b/src/core/thread/mesh_forwarder_mtd.cpp @@ -37,21 +37,21 @@ namespace ot { -Error MeshForwarder::SendMessage(Message &aMessage) +void MeshForwarder::SendMessage(OwnedPtr aMessagePtr) { - aMessage.SetDirectTransmission(); - aMessage.SetOffset(0); - aMessage.SetDatagramTag(0); - aMessage.SetTimestampToNow(); + Message &message = *aMessagePtr.Release(); - mSendQueue.Enqueue(aMessage); + message.SetDirectTransmission(); + message.SetOffset(0); + message.SetDatagramTag(0); + message.SetTimestampToNow(); + + mSendQueue.Enqueue(message); mScheduleTransmissionTask.Post(); #if (OPENTHREAD_CONFIG_MAX_FRAMES_IN_DIRECT_TX_QUEUE > 0) - ApplyDirectTxQueueLimit(aMessage); + ApplyDirectTxQueueLimit(message); #endif - - return kErrorNone; } Error MeshForwarder::EvictMessage(Message::Priority aPriority)