From a6432f7daa97365cbf0c7d55993f1f4b4528cd82 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 18 Feb 2025 09:17:50 -0800 Subject: [PATCH] [routing-manager] simplify state management in `PdPrefixManager` (#11250) This commit simplifies state tracking and updates within `PdPrefixManager`. Previously, three boolean flags (`mEnabled`, `mStarted`, and `mPaused`) and a `GetState()` method were used to represent the state as a `Dhcp6PdState`. This commit replaces the boolean flags with a single `mState` variable of type `Dhcp6PdState`. New `UpdateState()` and `SetState()` methods handle all state transitions, ensuring PD prefix withdrawal (OMR prefix removal from Network Data) and state change signaling. --- src/core/border_router/routing_manager.cpp | 115 +++++++++------------ src/core/border_router/routing_manager.hpp | 17 ++- 2 files changed, 55 insertions(+), 77 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 18a452c69..eaac06679 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -3946,9 +3946,7 @@ exit: RoutingManager::PdPrefixManager::PdPrefixManager(Instance &aInstance) : InstanceLocator(aInstance) - , mEnabled(false) - , mIsStarted(false) - , mIsPaused(false) + , mState(kDhcp6PdStateDisabled) , mNumPlatformPioProcessed(0) , mNumPlatformRaReceived(0) , mLastPlatformRaTime(0) @@ -3958,83 +3956,68 @@ RoutingManager::PdPrefixManager::PdPrefixManager(Instance &aInstance) void RoutingManager::PdPrefixManager::SetEnabled(bool aEnabled) { - State oldState = GetState(); - - VerifyOrExit(mEnabled != aEnabled); - mEnabled = aEnabled; - EvaluateStateChange(oldState); - -exit: - return; -} - -void RoutingManager::PdPrefixManager::StartStop(bool aStart) -{ - State oldState = GetState(); - - VerifyOrExit(aStart != mIsStarted); - mIsStarted = aStart; - EvaluateStateChange(oldState); - -exit: - return; -} - -void RoutingManager::PdPrefixManager::PauseResume(bool aPause) -{ - State oldState = GetState(); - - VerifyOrExit(aPause != mIsPaused); - mIsPaused = aPause; - EvaluateStateChange(oldState); - -exit: - return; -} - -RoutingManager::PdPrefixManager::State RoutingManager::PdPrefixManager::GetState(void) const -{ - State state = kDhcp6PdStateDisabled; - - if (mEnabled) + if (aEnabled) { - state = mIsStarted ? (mIsPaused ? kDhcp6PdStateIdle : kDhcp6PdStateRunning) : kDhcp6PdStateStopped; + VerifyOrExit(mState == kDhcp6PdStateDisabled); + UpdateState(); + } + else + { + SetState(kDhcp6PdStateDisabled); } - return state; +exit: + return; } void RoutingManager::PdPrefixManager::Evaluate(void) { - const FavoredOmrPrefix &favoredPrefix = Get().mOmrPrefixManager.GetFavoredPrefix(); - bool shouldPause = favoredPrefix.IsInfrastructureDerived() && (favoredPrefix.GetPrefix() != mPrefix.GetPrefix()); + VerifyOrExit(mState != kDhcp6PdStateDisabled); + UpdateState(); - PauseResume(/* aPause */ shouldPause); +exit: + return; } -void RoutingManager::PdPrefixManager::EvaluateStateChange(Dhcp6PdState aOldState) +void RoutingManager::PdPrefixManager::UpdateState(void) { - State newState = GetState(); + if (!Get().IsRunning()) + { + SetState(kDhcp6PdStateStopped); + } + else + { + const FavoredOmrPrefix &favoredOmrPrefix = Get().mOmrPrefixManager.GetFavoredPrefix(); - VerifyOrExit(aOldState != newState); - LogInfo("PdPrefixManager: %s -> %s", StateToString(aOldState), StateToString(newState)); + // We request a PD prefix (enter `kDhcp6PdStateRunning`), + // unless we see a favored infrastructure-derived OMR prefix + // which differs from our prefix. In this case, we can + // withdraw our prefix and enter `kDhcp6PdStateIdle`. - switch (newState) + if (favoredOmrPrefix.IsInfrastructureDerived() && (favoredOmrPrefix.GetPrefix() != mPrefix.GetPrefix())) + { + SetState(kDhcp6PdStateIdle); + } + else + { + SetState(kDhcp6PdStateRunning); + } + } +} + +void RoutingManager::PdPrefixManager::SetState(State aState) +{ + VerifyOrExit(aState != mState); + + LogInfo("PdPrefixManager: %s -> %s", StateToString(mState), StateToString(aState)); + mState = aState; + + if (mState != kDhcp6PdStateRunning) { - case kDhcp6PdStateDisabled: - case kDhcp6PdStateStopped: - case kDhcp6PdStateIdle: WithdrawPrefix(); - break; - case kDhcp6PdStateRunning: - break; } - // When the prefix is replaced, there will be a short period when the old prefix is still in the netdata, and PD - // manager will refuse to request the prefix. - // TODO: Either update the comment for the state callback or add a random delay when notifing the upper layer for - // state change. - mStateCallback.InvokeIfSet(MapEnum(newState)); + mStateCallback.InvokeIfSet(MapEnum(mState)); exit: return; @@ -4044,7 +4027,7 @@ Error RoutingManager::PdPrefixManager::GetPrefixInfo(PrefixTableEntry &aInfo) co { Error error = kErrorNone; - VerifyOrExit(IsRunning() && HasPrefix(), error = kErrorNotFound); + VerifyOrExit(HasPrefix(), error = kErrorNotFound); aInfo.mPrefix = mPrefix.GetPrefix(); aInfo.mValidLifetime = mPrefix.GetValidLifetime(); @@ -4059,7 +4042,7 @@ Error RoutingManager::PdPrefixManager::GetProcessedRaInfo(PdProcessedRaInfo &aPd { Error error = kErrorNone; - VerifyOrExit(IsRunning() && HasPrefix(), error = kErrorNotFound); + VerifyOrExit(HasPrefix(), error = kErrorNotFound); aPdProcessedRaInfo.mNumPlatformRaReceived = mNumPlatformRaReceived; aPdProcessedRaInfo.mNumPlatformPioProcessed = mNumPlatformPioProcessed; @@ -4119,7 +4102,7 @@ void RoutingManager::PdPrefixManager::Process(const InfraIf::Icmp6Packet *aRaPac PdPrefix favoredPrefix; PdPrefix prefix; - VerifyOrExit(mEnabled, error = kErrorInvalidState); + VerifyOrExit(mState != kDhcp6PdStateDisabled, error = kErrorInvalidState); if (aRaPacket != nullptr) { diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index c360c3017..ea09cdd12 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -1471,12 +1471,11 @@ private: explicit PdPrefixManager(Instance &aInstance); void SetEnabled(bool aEnabled); - void Start(void) { StartStop(/* aStart= */ true); } - void Stop(void) { StartStop(/* aStart= */ false); } - bool IsRunning(void) const { return GetState() == kDhcp6PdStateRunning; } + void Start(void) { Evaluate(); } + void Stop(void) { Evaluate(); } bool HasPrefix(void) const { return !mPrefix.IsEmpty(); } const Ip6::Prefix &GetPrefix(void) const { return mPrefix.GetPrefix(); } - State GetState(void) const; + State GetState(void) const { return mState; } void ProcessRa(const uint8_t *aRouterAdvert, uint16_t aLength); void ProcessPrefix(const PrefixTableEntry &aPrefixTableEntry); @@ -1496,22 +1495,18 @@ private: bool IsFavoredOver(const PdPrefix &aOther) const; }; + void UpdateState(void); + void SetState(State aState); void Process(const InfraIf::Icmp6Packet *aRaPacket, const PrefixTableEntry *aPrefixTableEntry); void ProcessPdPrefix(PdPrefix &aPrefix, PdPrefix &aFavoredPrefix); - void EvaluateStateChange(State aOldState); void WithdrawPrefix(void); - void StartStop(bool aStart); - void PauseResume(bool aPause); static const char *StateToString(State aState); using PrefixTimer = TimerMilliIn; using StateCallback = Callback; - bool mEnabled; // Whether PdPrefixManager is enabled. (guards the overall PdPrefixManager functions) - bool mIsStarted; // Whether PdPrefixManager is started. (monitoring prefixes) - bool mIsPaused; // Whether PdPrefixManager is paused. (when there is another BR advertising another prefix) - + State mState; uint32_t mNumPlatformPioProcessed; uint32_t mNumPlatformRaReceived; TimeMilli mLastPlatformRaTime;