From 87a919fd0a96e779a17e5cdd7bafc6c5b9e744e2 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Tue, 8 Jul 2025 22:50:40 -0700
Subject: [PATCH] [mle] enhance role restoration logic in `Start()` (#11679)

This commit refactors `Mle::Start()` by moving the logic for restoring
the previous role into a new method, `RestorePrevRole()`.

This change simplifies the startup process and improves robustness by
adding more validation checks within `RestorePrevRole()`. The new
method ensures that the previously saved state information is
consistent before it is applied. For example, it verifies that
`mLastSavedRole` matches the saved RLOC16 and that parent information
is valid if the saved role was Child. These checks protect against
loading invalid settings and allow the device to start more quickly
by ignoring inconsistent state.
---
 src/core/thread/mle.cpp | 64 +++++++++++++++++++++++++++++------------
 src/core/thread/mle.hpp |  1 +
 2 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp
index df7c0802a..7551a5ea6 100644
--- a/src/core/thread/mle.cpp
+++ b/src/core/thread/mle.cpp
@@ -225,29 +225,23 @@ Error Mle::Start(StartMode aMode)
 
     Get<KeyManager>().Start();
 
-    if (aMode == kNormalAttach)
+    switch (aMode)
     {
+    case kNormalAttach:
         mReattachState =
             (Get<MeshCoP::ActiveDatasetManager>().Restore() == kErrorNone) ? kReattachActive : kReattachStop;
+
+        if (RestorePrevRole() == kErrorNone)
+        {
+            ExitNow();
+        }
+        break;
+
+    case kAnnounceAttach:
+        break;
     }
 
-    if ((aMode == kAnnounceAttach) || (GetRloc16() == kInvalidRloc16))
-    {
-        Attach(kAnyPartition);
-    }
-#if OPENTHREAD_FTD
-    else if (IsRouterRloc16(GetRloc16()))
-    {
-        if (BecomeRouter(ThreadStatusTlv::kTooFewRouters) != kErrorNone)
-        {
-            Attach(kAnyPartition);
-        }
-    }
-#endif
-    else
-    {
-        IgnoreError(SendChildUpdateRequestToParent());
-    }
+    Attach(kAnyPartition);
 
 exit:
     return error;
@@ -286,6 +280,40 @@ exit:
     }
 }
 
+Error Mle::RestorePrevRole(void)
+{
+    // Restores the device to its previously saved role after MLE
+    // start.
+    //
+    // Returns `kErrorNone` if the restoration process begins
+    // successfully. An error is returned if:
+    // - No previous role was saved.
+    // - The saved role info is inconsistent (e.g., RLOC16 indicates a
+    //   router/leader, but `mLastSavedRole` doesn't match).
+    // - The last role is "Child," but no valid parent is available.
+
+    Error error = kErrorFailed;
+
+    VerifyOrExit(GetRloc16() != kInvalidRloc16);
+
+    if (IsRouterRloc16(GetRloc16()))
+    {
+#if OPENTHREAD_FTD
+        VerifyOrExit(mLastSavedRole == kRoleRouter || mLastSavedRole == kRoleLeader);
+        error = BecomeRouter(ThreadStatusTlv::kTooFewRouters);
+#endif
+        ExitNow();
+    }
+
+    VerifyOrExit(mLastSavedRole == kRoleChild);
+    VerifyOrExit(mParent.IsStateValidOrRestoring());
+    IgnoreError(SendChildUpdateRequestToParent());
+    error = kErrorNone;
+
+exit:
+    return error;
+}
+
 const Counters &Mle::GetCounters(void)
 {
     UpdateRoleTimeCounters(mRole);
diff --git a/src/core/thread/mle.hpp b/src/core/thread/mle.hpp
index 1763d564f..4f5de6063 100644
--- a/src/core/thread/mle.hpp
+++ b/src/core/thread/mle.hpp
@@ -1958,6 +1958,7 @@ private:
 
     Error      Start(StartMode aMode);
     void       Stop(StopMode aMode);
+    Error      RestorePrevRole(void);
     TxMessage *NewMleMessage(Command aCommand);
     void       SetRole(DeviceRole aRole);
     void       Attach(AttachMode aMode);