From 7b4f9e26fa096c89d6ad93f70f5cab03361b904e Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Wed, 11 Jan 2023 23:19:28 -0800
Subject: [PATCH] [mle] always check for stale parent in `HandleAdv()` (#8646)

This commit contains smaller fixes and enhancements in `Mle` method
`Mle::HandleAdvertisement()`
- We add `VerifyOrExit(IsAttached())` at the top of the method.
- We always perform the check for stale parent i.e., check if we
  got an MLE Advertisement from parent but with a different RLOC16.
  The `MleRouter` method also does the same check, however in certain
  cases (e.g., when there is Partition ID or leader ID mismatch) the
  `MleRouter` method will exit early and expect the `Mle` method
  to handle these cases.
- When on `Mle` method we handle Partition ID or leader ID mismatch
  and then process `RouteTlv` we now also call `UpdateRoutesOnFed()`.
---
 src/core/thread/mle.cpp        | 57 +++++++++++++++-------------------
 src/core/thread/mle_router.cpp | 18 ++++++++++-
 2 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp
index 9b12340de..45debf3ec 100644
--- a/src/core/thread/mle.cpp
+++ b/src/core/thread/mle.cpp
@@ -2706,52 +2706,47 @@ void Mle::HandleAdvertisement(RxInfo &aRxInfo)
     LeaderData leaderData;
     uint16_t   delay;
 
-    // Source Address
+    VerifyOrExit(IsAttached());
+
     SuccessOrExit(error = Tlv::Find<SourceAddressTlv>(aRxInfo.mMessage, sourceAddress));
 
     Log(kMessageReceive, kTypeAdvertisement, aRxInfo.mMessageInfo.GetPeerAddr(), sourceAddress);
 
-    // Leader Data
     SuccessOrExit(error = aRxInfo.mMessage.ReadLeaderDataTlv(leaderData));
 
-    if (IsAttached())
-    {
 #if OPENTHREAD_FTD
-        if (IsFullThreadDevice())
-        {
-            SuccessOrExit(error = Get<MleRouter>().HandleAdvertisement(aRxInfo, sourceAddress, leaderData));
-        }
-        else
-#endif
-        {
-            if ((aRxInfo.mNeighbor == &mParent) && (mParent.GetRloc16() != sourceAddress))
-            {
-                // Remove stale parent.
-                IgnoreError(BecomeDetached());
-            }
-        }
-    }
-
-    switch (mRole)
+    if (IsFullThreadDevice())
     {
-    case kRoleDisabled:
-    case kRoleDetached:
-        ExitNow();
+        SuccessOrExit(error = Get<MleRouter>().HandleAdvertisement(aRxInfo, sourceAddress, leaderData));
+    }
+#endif
 
-    case kRoleChild:
+    if (IsChild())
+    {
         VerifyOrExit(aRxInfo.mNeighbor == &mParent);
 
-        if ((mParent.GetRloc16() == sourceAddress) && (leaderData.GetPartitionId() != mLeaderData.GetPartitionId() ||
-                                                       leaderData.GetLeaderRouterId() != GetLeaderId()))
+        if (mParent.GetRloc16() != sourceAddress)
+        {
+            // Remove stale parent.
+            IgnoreError(BecomeDetached());
+            ExitNow();
+        }
+
+        if ((leaderData.GetPartitionId() != mLeaderData.GetPartitionId()) ||
+            (leaderData.GetLeaderRouterId() != GetLeaderId()))
         {
             SetLeaderData(leaderData.GetPartitionId(), leaderData.GetWeighting(), leaderData.GetLeaderRouterId());
 
 #if OPENTHREAD_FTD
             if (IsFullThreadDevice())
             {
-                switch (Get<MleRouter>().ProcessRouteTlv(aRxInfo))
+                RouteTlv routeTlv;
+
+                switch (Get<MleRouter>().ProcessRouteTlv(aRxInfo, routeTlv))
                 {
                 case kErrorNone:
+                    Get<RouterTable>().UpdateRoutesOnFed(routeTlv, mParent.GetRouterId());
+                    break;
                 case kErrorNotFound:
                     break;
                 default:
@@ -2764,12 +2759,10 @@ void Mle::HandleAdvertisement(RxInfo &aRxInfo)
         }
 
         mParent.SetLastHeard(TimerMilli::GetNow());
-        break;
-
-    case kRoleRouter:
-    case kRoleLeader:
+    }
+    else // Device is router or leader
+    {
         VerifyOrExit(aRxInfo.IsNeighborStateValid());
-        break;
     }
 
     if (mRetrieveNewNetworkData || IsNetworkDataNewer(leaderData))
diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp
index 11a58006c..8473ca40a 100644
--- a/src/core/thread/mle_router.cpp
+++ b/src/core/thread/mle_router.cpp
@@ -1158,6 +1158,9 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo, uint16_t aSourceAddress, c
         routeTlv.SetLength(0); // Mark that a Route TLV was not included
     }
 
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Handle Partition ID mismatch
+
     if (aLeaderData.GetPartitionId() != mLeaderData.GetPartitionId())
     {
         LogNote("Different partition (peer:%lu, local:%lu)", ToUlong(aLeaderData.GetPartitionId()),
@@ -1189,7 +1192,11 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo, uint16_t aSourceAddress, c
 
         ExitNow(error = kErrorDrop);
     }
-    else if (aLeaderData.GetLeaderRouterId() != GetLeaderId())
+
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Handle Leader Router ID mismatch
+
+    if (aLeaderData.GetLeaderRouterId() != GetLeaderId())
     {
         VerifyOrExit(aRxInfo.IsNeighborStateValid());
 
@@ -1210,6 +1217,9 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo, uint16_t aSourceAddress, c
     Get<TimeSync>().HandleTimeSyncMessage(aRxInfo.mMessage);
 #endif
 
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Process `RouteTlv`
+
     if (aRxInfo.IsNeighborStateValid() &&
         ((mRouterTable.GetActiveRouterCount() == 0) ||
          SerialNumber::IsGreater(routeTlv.GetRouterIdSequence(), mRouterTable.GetRouterIdSequence())))
@@ -1243,6 +1253,9 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo, uint16_t aSourceAddress, c
         }
     }
 
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Update routers as a child
+
     if (IsChild())
     {
         if (aRxInfo.mNeighbor == &mParent)
@@ -1288,6 +1301,9 @@ Error MleRouter::HandleAdvertisement(RxInfo &aRxInfo, uint16_t aSourceAddress, c
         ExitNow();
     }
 
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Update routers as a router or leader.
+
     if (IsRouter())
     {
         if (mLinkRequestDelay > 0 && routeTlv.IsRouterIdSet(mRouterId))