From 7dfde1f12923f03c9680be4d838b94b7a2320324 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 31 Mar 2022 22:03:52 -0700 Subject: [PATCH] [mesh-forwarder] fix and enhance `LogMessage()` (#7556) This commit updates `LogMessage()` in `MeshForwarder`: - Fixes `MessageActionToString()` so when there is a passed-in error it correctly returns the related action string (and only when `aAction == kMessageTransmit` it returned "Failed to send"). - Changes the order of parameters in `LogMessage()` and uses default value for parameters to simplify its use. --- src/core/net/ip6.cpp | 2 +- src/core/thread/indirect_sender.cpp | 4 ++-- src/core/thread/mesh_forwarder.cpp | 28 +++++++++++++++++--------- src/core/thread/mesh_forwarder.hpp | 6 +++++- src/core/thread/mesh_forwarder_ftd.cpp | 6 +++--- 5 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/core/net/ip6.cpp b/src/core/net/ip6.cpp index d555b10ec..0509eccad 100644 --- a/src/core/net/ip6.cpp +++ b/src/core/net/ip6.cpp @@ -1214,7 +1214,7 @@ start: { // Remove encapsulating header and start over. aMessage.RemoveHeader(aMessage.GetOffset()); - Get().LogMessage(MeshForwarder::kMessageReceive, aMessage, nullptr, kErrorNone); + Get().LogMessage(MeshForwarder::kMessageReceive, aMessage); goto start; } diff --git a/src/core/thread/indirect_sender.cpp b/src/core/thread/indirect_sender.cpp index 8fa0fd820..6b4e68c8e 100644 --- a/src/core/thread/indirect_sender.cpp +++ b/src/core/thread/indirect_sender.cpp @@ -320,7 +320,7 @@ void IndirectSender::UpdateIndirectMessage(Child &aChild) mDataPollHandler.HandleNewFrame(aChild); aChild.GetMacAddress(childAddress); - Get().LogMessage(MeshForwarder::kMessagePrepareIndirect, *message, &childAddress, kErrorNone); + Get().LogMessage(MeshForwarder::kMessagePrepareIndirect, *message, kErrorNone, &childAddress); } } @@ -519,7 +519,7 @@ void IndirectSender::HandleSentFrameToChild(const Mac::TxFrame &aFrame, if (!aFrame.IsEmpty()) { IgnoreError(aFrame.GetDstAddr(macDest)); - Get().LogMessage(MeshForwarder::kMessageTransmit, *message, &macDest, txError); + Get().LogMessage(MeshForwarder::kMessageTransmit, *message, txError, &macDest); } if (message->GetType() == Message::kTypeIp6) diff --git a/src/core/thread/mesh_forwarder.cpp b/src/core/thread/mesh_forwarder.cpp index e527fbf5d..9edf7eb2d 100644 --- a/src/core/thread/mesh_forwarder.cpp +++ b/src/core/thread/mesh_forwarder.cpp @@ -224,7 +224,7 @@ void MeshForwarder::RemoveMessage(Message &aMessage) } } - LogMessage(kMessageEvict, aMessage, nullptr, kErrorNoBufs); + LogMessage(kMessageEvict, aMessage, kErrorNoBufs); queue->DequeueAndFree(aMessage); } @@ -320,7 +320,7 @@ Message *MeshForwarder::PrepareNextDirectTransmission(void) #endif default: - LogMessage(kMessageDrop, *curMessage, nullptr, error); + LogMessage(kMessageDrop, *curMessage, error); mSendQueue.DequeueAndFree(*curMessage); continue; } @@ -1071,7 +1071,7 @@ void MeshForwarder::UpdateSendMessage(Error aFrameTxError, Mac::Address &aMacDes Get().RecordTxMessage(*mSendMessage, aMacDest); #endif - LogMessage(kMessageTransmit, *mSendMessage, &aMacDest, txError); + LogMessage(kMessageTransmit, *mSendMessage, txError, &aMacDest); if (mSendMessage->GetType() == Message::kTypeIp6) { @@ -1348,7 +1348,7 @@ void MeshForwarder::ClearReassemblyList(void) { for (Message &message : mReassemblyList) { - LogMessage(kMessageReassemblyDrop, message, nullptr, kErrorNoFrameReceived); + LogMessage(kMessageReassemblyDrop, message, kErrorNoFrameReceived); if (message.GetType() == Message::kTypeIp6) { @@ -1385,7 +1385,7 @@ bool MeshForwarder::UpdateReassemblyList(void) } else { - LogMessage(kMessageReassemblyDrop, message, nullptr, kErrorReassemblyTimeout); + LogMessage(kMessageReassemblyDrop, message, kErrorReassemblyTimeout); if (message.GetType() == Message::kTypeIp6) { @@ -1475,7 +1475,7 @@ Error MeshForwarder::HandleDatagram(Message &aMessage, const ThreadLinkInfo &aLi Get().RecordRxMessage(aMessage, aMacSource); #endif - LogMessage(kMessageReceive, aMessage, &aMacSource, kErrorNone); + LogMessage(kMessageReceive, aMessage, kErrorNone, &aMacSource); if (aMessage.GetType() == Message::kTypeIp6) { @@ -1713,6 +1713,8 @@ const char *MeshForwarder::MessageActionToString(MessageAction aAction, Error aE "Evicting", // (5) kMessageEvict }; + const char *string = kMessageActionStrings[aAction]; + static_assert(kMessageReceive == 0, "kMessageReceive value is incorrect"); static_assert(kMessageTransmit == 1, "kMessageTransmit value is incorrect"); static_assert(kMessagePrepareIndirect == 2, "kMessagePrepareIndirect value is incorrect"); @@ -1720,7 +1722,12 @@ const char *MeshForwarder::MessageActionToString(MessageAction aAction, Error aE static_assert(kMessageReassemblyDrop == 4, "kMessageReassemblyDrop value is incorrect"); static_assert(kMessageEvict == 5, "kMessageEvict value is incorrect"); - return (aError == kErrorNone) ? kMessageActionStrings[aAction] : "Failed to send"; + if ((aAction == kMessageTransmit) && (aError != kErrorNone)) + { + string = "Failed to send"; + } + + return string; } const char *MeshForwarder::MessagePriorityToString(const Message &aMessage) @@ -1803,8 +1810,9 @@ exit: void MeshForwarder::LogMessage(MessageAction aAction, const Message & aMessage, - const Mac::Address *aMacAddress, - Error aError) + Error aError, + const Mac::Address *aMacAddress) + { LogLevel logLevel = kLogLevelInfo; @@ -1882,7 +1890,7 @@ void MeshForwarder::LogLowpanHcFrameDrop(Error aError, #else // #if OT_SHOULD_LOG_AT( OT_LOG_LEVEL_NOTE) -void MeshForwarder::LogMessage(MessageAction, const Message &, const Mac::Address *, Error) +void MeshForwarder::LogMessage(MessageAction, const Message &, Error, const Mac::Address *) { } diff --git a/src/core/thread/mesh_forwarder.hpp b/src/core/thread/mesh_forwarder.hpp index 86594f5ab..476c7d055 100644 --- a/src/core/thread/mesh_forwarder.hpp +++ b/src/core/thread/mesh_forwarder.hpp @@ -512,7 +512,11 @@ private: void PauseMessageTransmissions(void) { mTxPaused = true; } void ResumeMessageTransmissions(void); - void LogMessage(MessageAction aAction, const Message &aMessage, const Mac::Address *aAddress, Error aError); + void LogMessage(MessageAction aAction, + const Message & aMessage, + Error aError = kErrorNone, + const Mac::Address *aAddress = nullptr); + void LogFrame(const char *aActionText, const Mac::Frame &aFrame, Error aError); void LogFragmentFrameDrop(Error aError, uint16_t aFrameLength, diff --git a/src/core/thread/mesh_forwarder_ftd.cpp b/src/core/thread/mesh_forwarder_ftd.cpp index 2468f5c0f..fb3df525e 100644 --- a/src/core/thread/mesh_forwarder_ftd.cpp +++ b/src/core/thread/mesh_forwarder_ftd.cpp @@ -169,7 +169,7 @@ void MeshForwarder::HandleResolved(const Ip6::Address &aEid, Error aError) } else { - LogMessage(kMessageDrop, message, nullptr, aError); + LogMessage(kMessageDrop, message, aError); message.Free(); } } @@ -349,7 +349,7 @@ void MeshForwarder::RemoveDataResponseMessages(void) mSendMessage = nullptr; } - LogMessage(kMessageDrop, message, nullptr, kErrorNone); + LogMessage(kMessageDrop, message); mSendQueue.DequeueAndFree(message); } } @@ -807,7 +807,7 @@ void MeshForwarder::HandleMesh(uint8_t * aFrame, message->SetRadioType(static_cast(aLinkInfo.mRadioType)); #endif - LogMessage(kMessageReceive, *message, &aMacSource, kErrorNone); + LogMessage(kMessageReceive, *message, kErrorNone, &aMacSource); #if OPENTHREAD_CONFIG_MULTI_RADIO // Since the message will be forwarded, we clear the radio