[mle] Refactor RoleTransitioner for a state-driven design

This commit refactors the RoleTransitioner with an enumerated state
and state transition functions to clarify role transitions and
prepare for additional parameters configured in an upcoming
Router Administration feature.

This commit is intended to not change any functionality, though some expected
enhancements have been described in several TODO comments.

Key changes include:
- Increased scope of the `RoleTransitioner` class for all operations directly
  related to determining the transition state or timing for FTDs.
  - This includes logic on values that can be passed with simple parameters to check
    functions, but excludes InstanceLocator operations, so that the
    class can remain focused on determining the role transition state.
- `Signal` methods of the `RoleTransitioner` class have been defined to clarify
  events affecting state transitions.
- The conditions related to beginning REED advertisements has been separated from
- Enumerated tracking of substates of the `DeviceRole` related to
  role transitions for FTDs.
- The `AddressSolicitState` has become specific cases of the `RoleTransitionerState`
- The `RoleTransitioner` now controls the extra delay when downgrading
  from the leader role.
- Parameters affecting the `mRouterRoleAllowed` state are tracked more independently
  - Separate bits track the state for `mRouterEligible`, `mUsingFtdMode`, and
  `mRouterRoleAllowedBySecurityPolicy`, which all update `mRouterRoleAllowed` when changed
  - `mCcmEnabled` and `mThreadVersionCheckEnabled` are saved with the `RoleTransitioner`
    but are used with getters to determine `mRouterRoleAllowedBySecurityPolicy`
  - SetRouterEligible() no longer returns an error code
- Different timer states within `RoleTransitionerState` control cases where the
  timer is used, to differentiate the state to restore if the timer is canceled:
  `kChildUpgradeTimerStarted`, `kRouterDowngradeTimerStarted`,
  `kRouterDisallowedTimerStarted`, and `kLeaderDisallowedTimerStarted`
- Some related switch cases have been replaced by short if-else blocks where it has a
  measurable improvement on codespace for the size-check build.

Author: jthompson-lutron

src/core/thread/mle.cpp

@@ -80,10 +80,6 @@ Mle::Mle(Instance &aInstance)
     , mWedAttachTimer(aInstance)
 #endif
 #if OPENTHREAD_FTD
-#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
-    , mCcmEnabled(false)
-    , mThreadVersionCheckEnabled(true)
-#endif
     , mNetworkIdTimeout(kNetworkIdTimeout)
     , mPreviousPartitionRouterIdSequence(0)
     , mPreviousPartitionIdTimeout(0)
@@ -295,6 +291,10 @@ void Mle::SetRole(DeviceRole aRole)
 
     SuccessOrExit(Get<Notifier>().Update(mRole, aRole, kEventThreadRoleChanged));
 
+#if OPENTHREAD_FTD
+    mRoleTransitioner.SignalRoleChanged(mRole);
+#endif
+
     LogNote("Role %s -> %s", RoleToString(oldRole), RoleToString(mRole));
 
     if ((oldRole == kRoleDetached) && IsAttached())
@@ -447,10 +447,6 @@ void Mle::Restore(void)
     // non-volatile settings after boot.
     mHasRestored = true;
 
-#if OPENTHREAD_FTD
-    UpdateRouterRoleAllowed(kReasonMleInit);
-#endif
-
 exit:
     return;
 }
@@ -599,7 +595,6 @@ void Mle::SetStateDetached(void)
     Get<MeshForwarder>().SetRxOnWhenIdle(true);
     Get<Mac::Mac>().SetBeaconEnabled(false);
 #if OPENTHREAD_FTD
-    mRoleTransitioner.SetDowngradeBlocked(false);
     ClearAlternateRloc16();
     HandleDetachStart();
 #endif
@@ -720,7 +715,7 @@ Error Mle::SetDeviceMode(DeviceMode aDeviceMode)
         ClearAlternateRloc16();
     }
 
-    UpdateRouterRoleAllowed(kReasonDeviceModeChanged);
+    mRoleTransitioner.SignalFtdModeChanged(IsFullThreadDevice());
 #endif
 
     if (IsAttached())
@@ -1045,7 +1040,7 @@ void Mle::HandleNotifierEvents(Events aEvents)
 #if OPENTHREAD_FTD
     if (aEvents.Contains(kEventSecurityPolicyChanged))
     {
-        UpdateRouterRoleAllowed(kReasonSecurityPolicyChanged);
+        HandleRouterRoleAllowedFromSecurityPolicy();
     }
 
     if (mRoleTransitioner.IsDowngradeBlocked() && aEvents.Contains(kEventThreadChildRemoved))
@@ -1061,7 +1056,7 @@ void Mle::HandleNotifierEvents(Events aEvents)
             }
         }
 
-        mRoleTransitioner.SetDowngradeBlocked(shouldBlock);
+        mRoleTransitioner.SignalDowngradeBlocked(shouldBlock);
     }
 #endif
 

src/core/thread/mle.hpp

@@ -1022,7 +1022,7 @@ class Mle : public InstanceLocator, private NonCopyable
      * @retval TRUE   If the REED is going to become a Router soon.
      * @retval FALSE  If the REED is not going to become a Router soon.
      */
-    bool MayBecomeRouterSoon(void) const { return IsChild() && mRoleTransitioner.MayTransitionRoleSoon(); }
+    bool MayBecomeRouterSoon(void) const { return mRoleTransitioner.MayBecomeRouterSoon(); }
 
     /**
      * Removes a link to a neighbor.
@@ -1346,7 +1346,6 @@ class Mle : public InstanceLocator, private NonCopyable
     static constexpr uint8_t  kNetworkIdTimeout              = 120; // (in sec)
     static constexpr uint16_t kDiscoveryMaxJitter            = 250; // Max jitter delay Discovery Responses (in msec).
     static constexpr uint16_t kUnsolicitedDataResponseJitter = 500; // Max delay for unsol Data Response (in msec).
-    static constexpr uint8_t  kLeaderDowngradeExtraDelay     = 10;  // Extra delay to downgrade leader (in sec).
     static constexpr uint8_t  kDefaultLeaderWeight           = 64;
     static constexpr uint8_t  kAlternateRloc16Timeout        = 8; // Time to use alternate RLOC16 (in sec).
 
@@ -1471,16 +1470,6 @@ class Mle : public InstanceLocator, private NonCopyable
         kAddrSolicitUnrecognizedReason = 6,
     };
 
-#if OPENTHREAD_FTD
-    enum UpdateRouterRoleAllowedReason : uint8_t // Used in `UpdateRouterRoleAllowed()`
-    {
-        kReasonMleInit,
-        kReasonDeviceModeChanged,
-        kReasonConfigParameterChanged,
-        kReasonSecurityPolicyChanged,
-    };
-#endif
-
     enum MessageAction : uint8_t
     {
         kMessageSend,
@@ -2214,61 +2203,111 @@ class Mle : public InstanceLocator, private NonCopyable
     class RoleTransitioner
     {
     public:
-        enum AddressSolicitState : uint8_t
-        {
-            kAddressSolicitStateIdle,
-            kAddressSolicitStatePending,
-            kAddressSolicitStateRejected,
-        };
-
         explicit RoleTransitioner(void);
 
+        bool IsAddressSolicitPending(void) const { return mTransitionerState == kChildAddressSolicitStatePending; }
+        bool IsDowngradeBlocked(void) const { return mTransitionerState == kRouterDowngradeBlocked; }
         bool IsRouterEligible(void) const { return mRouterEligible; }
         bool IsRouterRoleAllowed(void) const { return mRouterRoleAllowed; }
-        bool IsTransitionPending(void) const { return (mTimeout != 0); }
-        bool IsDowngradeBlocked(void) const { return mDowngradeBlocked; }
+        bool IsTransitionPending(void) const;
+
+#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
+        bool IsCcmEnabled(void) const { return mCcmEnabled; }
+        bool IsThreadVersionCheckEnabled(void) const { return mThreadVersionCheckEnabled; }
+#endif
 
         bool IsRouterCountBelowUpgradeThreshold(uint8_t aCount) const { return aCount < mUpgradeThreshold; }
-        bool IsRouterCountAboveDowngradeThreshold(uint8_t aCount) const { return aCount > mDowngradeThreshold; }
-        bool MayDowngrade(uint8_t aRouterCount, uint16_t aValidChildren) const;
-        bool MayTransitionRoleSoon(void) const;
+
+        bool MayBeginReedAdvertisments(void) const;
+        bool MayBeginDowngradeTimer(uint8_t aRouterCount, uint16_t aValidChildren) const;
+        bool MayCompleteTimedDowngrade(uint8_t aRouterCount) const;
+        bool MayUpgrade(uint8_t aRouterCount) const;
+        bool MayBecomeRouterSoon(void) const;
 
         uint8_t GetTimeout(void) const { return mTimeout; }
         uint8_t GetJitter(void) const { return mJitter; }
         uint8_t GetUpgradeThreshold(void) const { return mUpgradeThreshold; }
         uint8_t GetDowngradeThreshold(void) const { return mDowngradeThreshold; }
 
-        AddressSolicitState GetAddressSolicitState(void) { return mAddressSolicitState; }
-
-        void SetAddressSolicitState(AddressSolicitState aState) { mAddressSolicitState = aState; }
-
-        bool SetRouterEligibleAndGetChanged(bool aStatus);
-        bool SetRouterRoleAllowedAndGetChanged(bool aStatus);
-
         void SetJitter(uint8_t aJitter) { mJitter = aJitter; }
         void SetUpgradeThreshold(uint8_t aThreshold) { mUpgradeThreshold = aThreshold; }
         void SetDowngradeThreshold(uint8_t aThreshold) { mDowngradeThreshold = aThreshold; }
-        void SetDowngradeBlocked(bool aBlocked) { mDowngradeBlocked = aBlocked; }
 
-        void StartTimeout(void);
-        void StopTimeout(void) { mTimeout = 0; }
-        void IncreaseTimeout(uint8_t aIncrement) { mTimeout += aIncrement; }
+        void HandleTimeTick(void) { mTimeout = (mTimeout > 0) ? mTimeout - 1 : mTimeout; }
+
+        // ------------------------
+        // State transition signals
+        void SignalAddressSolicitFinished(void);
+        void SignalAddressSolicitPending(void) { mTransitionerState = kChildAddressSolicitStatePending; }
+        void SignalAddressSolicitRejected(void) { mTransitionerState = kChildAddressSolicitStateRejected; }
+        void SignalBeginTimedRouterDowngrade(void);
+        void SignalChildUpgradeStart(void);
+        void SignalDowngradeAborted(void);
+        void SignalDowngradeBlocked(bool aShouldBlock);
+        void SignalRoleChanged(DeviceRole aRole);
+        void SignalUpgradeAttemptFailed(void) { mTransitionerState = kChildIdle; }
+
+        // ------------------------
+        // State transition signals affecting the Router role allowed status
+        void SignalFtdModeChanged(bool aIsFtdMode);
+        void SignalRouterEligible(bool aEligible);
+        void SignalRouterRoleAllowedBySecurityPolicy(bool aAllowed);
 
-        bool HandleTimeTick(void);
-        void StartUpgradeIfConditionsAllow(uint8_t aRouterCount);
+#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
+        void SetCcmEnabled(bool aCcmEnabled) { mCcmEnabled = aCcmEnabled; }
+        void SetThreadVersionCheckEnabled(bool aThreadVersionCheckEnabled)
+        {
+            mThreadVersionCheckEnabled = aThreadVersionCheckEnabled;
+        }
+#endif
+
+        // ---------------------------------------------------
+        // State transition signals with state change feedback
+
+        // Polling signals that may result in a transition
+        void StartRouterUpgradeIfConditionsAllow(uint8_t aRouterCount);
 
     private:
+        /** Transition substates of @ref DeviceRole */
+        enum RoleTransitionerState : uint8_t
+        {
+            kInit,
+            kDetached,
+            kChildIdle,
+            kChildUpgradeTimerStarted,
+            kChildAddressSolicitStatePending,
+            kChildAddressSolicitStateRejected,
+            kRouterIdle,
+            kRouterDowngradeBlocked,      ///< Downgrades from the Router role are blocked
+            kRouterDowngradeTimerStarted, ///< The downgrade timer has been started
+            kRouterDisallowedTimerStarted,
+            kLeaderIdle,
+            kLeaderDisallowedTimerStarted,
+        };
+
+        void BeginTimer(void);
+        void BeginLeaderDowngradeTimer(void);
+        void SignalRouterRoleAllowedUpdate(void);
+
         static constexpr uint8_t kMaxDelayToTransitionSoon = 10;
 
-        static constexpr uint8_t kRouterSelectionJitterDefault    = 120; // (in sec)
+        static constexpr uint8_t kRouterSelectionJitterDefault = 120; ///< (in sec)
+        static constexpr uint8_t kLeaderDowngradeExtraDelay    = 10;  ///< Extra delay to downgrade leader (in sec).
+
         static constexpr uint8_t kRouterUpgradeThresholdDefault   = 16;
         static constexpr uint8_t kRouterDowngradeThresholdDefault = 23;
 
-        bool mDowngradeBlocked : 1;
-        bool mRouterEligible : 1;
-        bool mRouterRoleAllowed : 1;
+        // Router Role Allowed statuses
+        bool mRouterRoleAllowed : 1; ///< Cached state of the router role allowed status
+        bool mRouterEligible : 1;    ///< Router Eligible configuration affecting the role allowed status
+        bool mUsingFtdMode : 1;      ///< FTD mode
+        bool mRouterRoleAllowedBySecurityPolicy : 1; ///< Cached state of the security policy router role allowed status
+#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
+        bool mCcmEnabled : 1;                ///< Whether to allow the security policy CCM commissioned check
+        bool mThreadVersionCheckEnabled : 1; ///< Whether to allow the security policy version check
+#endif
 
-        AddressSolicitState mAddressSolicitState;
+        RoleTransitionerState mTransitionerState;
 
         uint8_t mTimeout;
         uint8_t mJitter;
@@ -2460,8 +2499,8 @@ class Mle : public InstanceLocator, private NonCopyable
     void     ClearAlternateRloc16(void);
     uint8_t  SelectLeaderId(void) const;
     uint32_t SelectPartitionId(void) const;
-    bool     DetermineIfRouterRoleAllowed(void) const;
-    void     UpdateRouterRoleAllowed(UpdateRouterRoleAllowedReason aReason);
+    void     HandleRouterRoleAllowedFromSecurityPolicy(void);
+    void     HandleRoleAllowedChangedFromConfig(void);
     void     DetermineConnectivity(Connectivity &aConnectivity) const;
     void     HandleDetachStart(void);
     void     HandleChildStart(void);
@@ -2509,7 +2548,7 @@ class Mle : public InstanceLocator, private NonCopyable
     void     SetChildStateToValid(Child &aChild);
     bool     HasChildren(void);
     void     RemoveChildren(void);
-    bool     ShouldDowngrade(uint8_t aNeighborId, const RouteTlv &aRouteTlv) const;
+    void     CheckOrBeginRouterDowngrade(uint8_t aNeighborId, const RouteTlv &aRouteTlv);
     bool     NeighborHasComparableConnectivity(const RouteTlv &aRouteTlv, uint8_t aNeighborId) const;
     void     HandleAdvertiseTrickleTimer(void);
     void     HandleTimeTick(void);
@@ -2589,10 +2628,6 @@ class Mle : public InstanceLocator, private NonCopyable
 
 #if OPENTHREAD_FTD
 
-#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
-    bool mCcmEnabled : 1;
-    bool mThreadVersionCheckEnabled : 1;
-#endif
     uint8_t mRouterId;
     uint8_t mPreviousRouterId;
     uint8_t mNetworkIdTimeout;