[mesh-forwarder] finalize message direct tx on drop or eviction (open…

…thread#9682)

This commit adds `FinalizeMessageDirectTx()`, which clears the
`DirectTx` flag on a given message, updates the IPv6 counter, and
signals other modules about the transmission status(particularly for
`MleDiscoverRequest` and `MleChildIdRequest` messages so their
internal state can be updated).

This commit ensures `FinalizeMessageDirectTx()` is called in various
scenarios:
- Successful message delivery (all fragments reach destination).
- Any fragment transmission failure (frame tx failure)
- Message drop due to address query failure or malformed message.
- Message drop by queue management.
- Message eviction to prioritize higher-priority messages.

This commit also updates `DiscoverScanner` to stop an ongoing discover
scan if the "MLE Discover Request" message transmission fails with error
other than CSMA error. This is necessary because the `DiscoverScanner`
reuses the same `Message` instance for tx on different scan channels.
If the `Message` is freed (e.g., evicted), the `Message` cannot be
reused.

Finally, this commit renames `RemoveMessage()` to `EvictMessage()` to
clarify the purpose and usage of this method.

src/core/thread/discover_scanner.cpp

@@ -205,23 +205,36 @@ Mac::TxFrame *DiscoverScanner::PrepareDiscoveryRequestFrame(Mac::TxFrame &aFrame
     return frame;
 }
 
-void DiscoverScanner::HandleDiscoveryRequestFrameTxDone(Message &aMessage)
+void DiscoverScanner::HandleDiscoveryRequestFrameTxDone(Message &aMessage, Error aError)
 {
     switch (mState)
     {
     case kStateIdle:
         break;
 
     case kStateScanning:
-        // Mark the Discovery Request message for direct tx to ensure it
-        // is not dequeued and freed by `MeshForwarder` and is ready for
-        // the next scan channel. Also pause message tx on `MeshForwarder`
-        // while listening to receive Discovery Responses.
-        aMessage.SetDirectTransmission();
-        aMessage.SetTimestampToNow();
-        Get<MeshForwarder>().PauseMessageTransmissions();
-        mTimer.Start(kDefaultScanDuration);
-        break;
+        if ((aError == kErrorNone) || (aError == kErrorChannelAccessFailure))
+        {
+            // Mark the Discovery Request message for direct tx to ensure it
+            // is not dequeued and freed by `MeshForwarder` and is ready for
+            // the next scan channel. Also pause message tx on `MeshForwarder`
+            // while listening to receive Discovery Responses.
+            aMessage.SetDirectTransmission();
+            aMessage.SetTimestampToNow();
+            Get<MeshForwarder>().PauseMessageTransmissions();
+            mTimer.Start(kDefaultScanDuration);
+            break;
+        }
+
+        // If we encounter other error failures (e.g., `kErrorDrop` due
+        // to queue management dropping the message or if message being
+        // evicted), `aMessage` may be immediately freed. This prevents
+        // us from reusing it to request a scan on the next scan channel.
+        // As a result, we stop the scan operation in such cases.
+
+        mState = kStateScanDone;
+
+        OT_FALL_THROUGH;
 
     case kStateScanDone:
         HandleDiscoverComplete();

src/core/thread/discover_scanner.hpp

@@ -165,7 +165,7 @@ class DiscoverScanner : public InstanceLocator, private NonCopyable
 
     // Methods used by `MeshForwarder`
     Mac::TxFrame *PrepareDiscoveryRequestFrame(Mac::TxFrame &aFrame);
-    void          HandleDiscoveryRequestFrameTxDone(Message &aMessage);
+    void          HandleDiscoveryRequestFrameTxDone(Message &aMessage, Error aError);
     void          Stop(void) { HandleDiscoverComplete(); }
 
     // Methods used from `Mle`

src/core/thread/mesh_forwarder.cpp

@@ -202,7 +202,7 @@ void MeshForwarder::PrepareEmptyFrame(Mac::TxFrame &aFrame, const Mac::Address &
     aFrame.SetPayloadLength(0);
 }
 
-void MeshForwarder::RemoveMessage(Message &aMessage)
+void MeshForwarder::EvictMessage(Message &aMessage)
 {
     PriorityQueue *queue = aMessage.GetPriorityQueue();
 
@@ -217,6 +217,8 @@ void MeshForwarder::RemoveMessage(Message &aMessage)
         }
 #endif
 
+        FinalizeMessageDirectTx(aMessage, kErrorNoBufs);
+
         if (mSendMessage == &aMessage)
         {
             mSendMessage = nullptr;
@@ -387,7 +389,7 @@ Error MeshForwarder::UpdateEcnOrDrop(Message &aMessage, bool aPreparingToSend)
         mTxQueueStats.UpdateFor(aMessage);
 #endif
         LogMessage(kMessageQueueMgmtDrop, aMessage);
-        aMessage.ClearDirectTransmission();
+        FinalizeMessageDirectTx(aMessage, kErrorDrop);
         RemoveMessageIfNoPendingTx(aMessage);
     }
 
@@ -518,7 +520,7 @@ void MeshForwarder::ApplyDirectTxQueueLimit(Message &aMessage)
 #endif
 
     LogMessage(kMessageFullQueueDrop, aMessage);
-    aMessage.ClearDirectTransmission();
+    FinalizeMessageDirectTx(aMessage, kErrorDrop);
     RemoveMessageIfNoPendingTx(aMessage);
 
 exit:
@@ -642,6 +644,7 @@ Message *MeshForwarder::PrepareNextDirectTransmission(void)
             mTxQueueStats.UpdateFor(*curMessage);
 #endif
             LogMessage(kMessageDrop, *curMessage, error);
+            FinalizeMessageDirectTx(*curMessage, error);
             mSendQueue.DequeueAndFree(*curMessage);
             continue;
         }
@@ -1261,9 +1264,6 @@ void MeshForwarder::UpdateSendMessage(Error aFrameTxError, Mac::Address &aMacDes
 
     txError = aFrameTxError;
 
-    mSendMessage->ClearDirectTransmission();
-    mSendMessage->SetOffset(0);
-
     if (aNeighbor != nullptr)
     {
         aNeighbor->GetLinkInfo().AddMessageTxStatus(mSendMessage->GetTxSuccess());
@@ -1289,39 +1289,54 @@ void MeshForwarder::UpdateSendMessage(Error aFrameTxError, Mac::Address &aMacDes
 #endif
 
     LogMessage(kMessageTransmit, *mSendMessage, txError, &aMacDest);
+    FinalizeMessageDirectTx(*mSendMessage, txError);
+    RemoveMessageIfNoPendingTx(*mSendMessage);
+
+exit:
+    mScheduleTransmissionTask.Post();
+}
 
-    if (mSendMessage->GetType() == Message::kTypeIp6)
+void MeshForwarder::FinalizeMessageDirectTx(Message &aMessage, Error aError)
+{
+    // Finalizes the direct transmission of `aMessage`. This can be
+    // triggered by successful delivery (all fragments reaching the
+    // destination), failure of any fragment, queue management
+    // dropping the message, or eviction of message to accommodate
+    // higher priority messages.
+
+    VerifyOrExit(aMessage.IsDirectTransmission());
+
+    aMessage.ClearDirectTransmission();
+    aMessage.SetOffset(0);
+
+    if (aError != kErrorNone)
     {
-        if (mSendMessage->GetTxSuccess())
-        {
-            mIpCounters.mTxSuccess++;
-        }
-        else
-        {
-            mIpCounters.mTxFailure++;
-        }
+        aMessage.SetTxSuccess(false);
+    }
+
+    if (aMessage.GetType() == Message::kTypeIp6)
+    {
+        aMessage.GetTxSuccess() ? mIpCounters.mTxSuccess++ : mIpCounters.mTxFailure++;
     }
 
-    switch (mSendMessage->GetSubType())
+    switch (aMessage.GetSubType())
     {
     case Message::kSubTypeMleDiscoverRequest:
         // Note that `HandleDiscoveryRequestFrameTxDone()` may update
-        // `mSendMessage` and mark it again for direct transmission.
-        Get<Mle::DiscoverScanner>().HandleDiscoveryRequestFrameTxDone(*mSendMessage);
+        // `aMessage` and mark it again for direct transmission.
+        Get<Mle::DiscoverScanner>().HandleDiscoveryRequestFrameTxDone(aMessage, aError);
         break;
 
     case Message::kSubTypeMleChildIdRequest:
-        Get<Mle::Mle>().HandleChildIdRequestTxDone(*mSendMessage);
+        Get<Mle::Mle>().HandleChildIdRequestTxDone(aMessage);
         break;
 
     default:
         break;
     }
 
-    RemoveMessageIfNoPendingTx(*mSendMessage);
-
 exit:
-    mScheduleTransmissionTask.Post();
+    return;
 }
 
 bool MeshForwarder::RemoveMessageIfNoPendingTx(Message &aMessage)

src/core/thread/mesh_forwarder.hpp

@@ -552,7 +552,7 @@ class MeshForwarder : public InstanceLocator, private NonCopyable
                                  Message::Priority       aPriority);
     Error HandleDatagram(Message &aMessage, const ThreadLinkInfo &aLinkInfo, const Mac::Address &aMacSource);
     void  ClearReassemblyList(void);
-    void  RemoveMessage(Message &aMessage);
+    void  EvictMessage(Message &aMessage);
     void  HandleDiscoverComplete(void);
 
     void          HandleReceivedFrame(Mac::RxFrame &aFrame);
@@ -564,6 +564,7 @@ class MeshForwarder : public InstanceLocator, private NonCopyable
     void UpdateNeighborLinkFailures(Neighbor &aNeighbor, Error aError, bool aAllowNeighborRemove, uint8_t aFailLimit);
     void HandleSentFrame(Mac::TxFrame &aFrame, Error aError);
     void UpdateSendMessage(Error aFrameTxError, Mac::Address &aMacDest, Neighbor *aNeighbor);
+    void FinalizeMessageDirectTx(Message &aMessage, Error aError);
     bool RemoveMessageIfNoPendingTx(Message &aMessage);
 
     void HandleTimeTick(void);

src/core/thread/mesh_forwarder_ftd.cpp

@@ -165,6 +165,7 @@ void MeshForwarder::HandleResolved(const Ip6::Address &aEid, Error aError)
         if (aError != kErrorNone)
         {
             LogMessage(kMessageDrop, message, kErrorAddressQuery);
+            FinalizeMessageDirectTx(message, kErrorAddressQuery);
             mSendQueue.DequeueAndFree(message);
             continue;
         }
@@ -259,7 +260,7 @@ Error MeshForwarder::EvictMessage(Message::Priority aPriority)
 exit:
     if ((error == kErrorNone) && (evict != nullptr))
     {
-        RemoveMessage(*evict);
+        EvictMessage(*evict);
     }
 
     return error;
@@ -342,6 +343,7 @@ void MeshForwarder::RemoveDataResponseMessages(void)
         }
 
         LogMessage(kMessageDrop, message);
+        FinalizeMessageDirectTx(message, kErrorDrop);
         mSendQueue.DequeueAndFree(message);
     }
 }

src/core/thread/mesh_forwarder_mtd.cpp

@@ -68,7 +68,7 @@ Error MeshForwarder::EvictMessage(Message::Priority aPriority)
 
     if (message->GetPriority() < static_cast<uint8_t>(aPriority))
     {
-        RemoveMessage(*message);
+        EvictMessage(*message);
         ExitNow(error = kErrorNone);
     }