From e79155731309aeaf9561543166c5801642d291d9 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 26 Mar 2026 15:30:29 -0700 Subject: [PATCH] [routing-manager] decouple `OmrPrefixManager` to accelerate OMR publication (#12753) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit refactors the `OmrPrefixManager` to decouple it from the main `RoutingManager` policy evaluation cycle. This allows the OMR prefix to be managed independently and published faster into the Network Data. Previously, `OmrPrefixManager` relied on its `Evaluate()` method being called during the main `RoutingManager::EvaluateRoutingPolicy()` cycle. This meant it had to wait for other components to be ready — such as sending Router Solicitations to discover other routers on the Adjacent Infrastructure Link (AIL)—before taking action. With this change, `OmrPrefixManager` operates independently. It can evaluate its state as soon as the Border Router function is enabled and `Start()` is called. Additional improvements supporting this independent operation include: - Replaces the `mIsLocalAddedInNetData` boolean with a `LocalPrefixState` enum (`kNotAdded`, `kToAdd`, `kAdded`) to manage addition state and support delayed updates. - Introduces a random delay (`kMinDelayToAdd` to `kMaxDelayToAdd`) before adding a self-generated OMR prefix to Network Data. This gives the network time to settle, allowing other BRs or the `PdPrefixManager` time to establish a prefix. - Implements a retry mechanism with jitter for Network Data addition failures, rather than silently ignoring them. - Refactors `PdPrefixManager` to batch state changes via an `mEvents` bitmask and process them through `mEventTask`. Changes are now handled explicitly by `OmrPrefixManager::HandlePdPrefixManagerEvent()`, further reducing unnecessary main routing policy evaluations. --- src/core/border_router/routing_manager.cpp | 233 +++++++++++++++------ src/core/border_router/routing_manager.hpp | 58 +++-- tests/unit/test_routing_manager.cpp | 6 +- 3 files changed, 216 insertions(+), 81 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 602dd18dc..405dd5893 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -360,6 +360,7 @@ void RoutingManager::HandleNotifierEvents(Events aEvents) if (mIsRunning && aEvents.Contains(kEventThreadNetdataChanged)) { + mOmrPrefixManager.HandleNetDataChange(); mOnLinkPrefixManager.HandleNetDataChange(); ScheduleRoutingPolicyEvaluation(kAfterRandomDelay); } @@ -721,8 +722,11 @@ exit: RoutingManager::OmrPrefixManager::OmrPrefixManager(Instance &aInstance) : InstanceLocator(aInstance) , mConfig(kOmrConfigAuto) - , mIsLocalAddedInNetData(false) + , mTimer(aInstance) + , mLocalInNetDataState(kNotAdded) , mDefaultRoute(false) + , mIsInitialized(false) + , mIsRunning(false) { } @@ -733,22 +737,44 @@ void RoutingManager::OmrPrefixManager::Init(const Ip6::Prefix &aBrUlaPrefix) mGeneratedPrefix.SetLength(kOmrPrefixLength); LogInfo("Generated local OMR prefix: %s", mGeneratedPrefix.ToString().AsCString()); + + mIsInitialized = true; } void RoutingManager::OmrPrefixManager::Start(void) { - FavoredOmrPrefix favoredPrefix; + VerifyOrExit(mIsInitialized && !mIsRunning); - DetermineFavoredPrefixInNetData(favoredPrefix); - SetFavoredPrefix(favoredPrefix); + mIsRunning = true; + Evaluate(); + +exit: + return; } void RoutingManager::OmrPrefixManager::Stop(void) { + VerifyOrExit(mIsRunning); + RemoveLocalFromNetData(); ClearFavoredPrefix(); + + mIsRunning = false; + +exit: + return; } +void RoutingManager::OmrPrefixManager::HandleNetDataChange(void) { Evaluate(); } + +#if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE +void RoutingManager::OmrPrefixManager::HandlePdPrefixManagerEvent(void) +{ + // Callback from `PdPrefixManager`. + Evaluate(); +} +#endif + bool RoutingManager::OmrPrefixManager::IsInitialEvaluationDone(void) const { // This method indicates whether or not we are done with the @@ -756,7 +782,7 @@ bool RoutingManager::OmrPrefixManager::IsInitialEvaluationDone(void) const // we have discovered a favored OMR prefix (added by us or another BR) // or if `OmrConfig` is set to disable OMR prefix management. - return !mFavoredPrefix.IsEmpty() || (mConfig == kOmrConfigDisabled); + return mIsRunning && (!mFavoredPrefix.IsEmpty() || (mConfig == kOmrConfigDisabled)); } RoutingManager::OmrConfig RoutingManager::OmrPrefixManager::GetConfig(Ip6::Prefix *aPrefix, @@ -805,7 +831,7 @@ Error RoutingManager::OmrPrefixManager::SetConfig(OmrConfig aConfig, mConfig = aConfig; mCustomPrefix = customPrefix; - Get().ScheduleRoutingPolicyEvaluation(kImmediately); + Evaluate(); exit: return error; @@ -917,13 +943,14 @@ void RoutingManager::OmrPrefixManager::Evaluate(void) { FavoredOmrPrefix favoredPrefix; - OT_ASSERT(Get().IsRunning()); + VerifyOrExit(mIsRunning); DetermineFavoredPrefixInNetData(favoredPrefix); UpdateLocalPrefix(); - // Decide if we need to add or remove our local OMR prefix. + // Determine whether to add or remove our local OMR prefix in the + // Network Data. if (mLocalPrefix.IsEmpty()) { @@ -931,24 +958,73 @@ void RoutingManager::OmrPrefixManager::Evaluate(void) ExitNow(); } - if (favoredPrefix.IsEmpty() || favoredPrefix.GetPreference() < mLocalPrefix.GetPreference()) + if (favoredPrefix.GetPrefix() == mLocalPrefix.GetPrefix()) { - SuccessOrExit(AddLocalToNetData()); - SetFavoredPrefix(mLocalPrefix); + // If `favoredPrefix` matches our local prefix, add it to the + // Network Data (if not already added). This also handles the + // case where a restarting BR, which was previously publishing + // the OMR prefix, sees its own local prefix in the restored + // Network Data. In such cases, we want to re-add it quickly. + + AddLocalToNetData(); ExitNow(); } - SetFavoredPrefix(favoredPrefix); + if (favoredPrefix.IsEmpty()) + { + if (mLocalPrefixOrigin != kSelfGenerated) + { + AddLocalToNetData(); + ExitNow(); + } - if (favoredPrefix.GetPrefix() == mLocalPrefix.GetPrefix()) - { - IgnoreError(AddLocalToNetData()); + // Apply a random delay before adding a self-generated ULA OMR + // prefix. This allows other BRs time to publish their prefixes, + // or provides time for the `PdPrefixManager` to be delegated a + // prefix. + + switch (mLocalInNetDataState) + { + case kNotAdded: + { + uint32_t delay = Random::NonCrypto::GetUint32InRange(kMinDelayToAdd, kMaxDelayToAdd); + + mLocalInNetDataState = kToAdd; + mTimer.Start(delay); + + LogInfo("Will add %s in %lu msec to NetData", LocalToString().AsCString(), ToUlong(delay)); + break; + } + + case kAdded: + case kToAdd: + break; + } + + ExitNow(); } - else if (mIsLocalAddedInNetData) + + if (favoredPrefix.GetPreference() < mLocalPrefix.GetPreference()) { - RemoveLocalFromNetData(); + AddLocalToNetData(); + ExitNow(); } + // Since there is a `favoredPrefix` different from our local prefix, + // we remove our local prefix if it is added or scheduled to be added. + + SetFavoredPrefix(favoredPrefix); + RemoveLocalFromNetData(); + +exit: + return; +} + +void RoutingManager::OmrPrefixManager::HandleTimer(void) +{ + VerifyOrExit(mLocalInNetDataState == kToAdd); + AddLocalToNetData(); + exit: return; } @@ -968,19 +1044,42 @@ bool RoutingManager::OmrPrefixManager::ShouldAdvertiseLocalAsRio(void) const // may still be present in Network Data for a short interval due // to delays in registering changes with the leader. - return mIsLocalAddedInNetData && Get().ContainsOmrPrefix(mLocalPrefix.GetPrefix()); + bool shouldAdv = false; + + switch (mLocalInNetDataState) + { + case kAdded: + case kToAdd: + shouldAdv = Get().ContainsOmrPrefix(mLocalPrefix.GetPrefix()); + break; + case kNotAdded: + break; + } + + return shouldAdv; } -Error RoutingManager::OmrPrefixManager::AddLocalToNetData(void) +void RoutingManager::OmrPrefixManager::AddLocalToNetData(void) { - Error error = kErrorNone; + VerifyOrExit(mLocalInNetDataState != kAdded); - VerifyOrExit(!mIsLocalAddedInNetData); - SuccessOrExit(error = AddOrUpdateLocalInNetData()); - mIsLocalAddedInNetData = true; + if (AddOrUpdateLocalInNetData() != kErrorNone) + { + uint32_t delay = Random::NonCrypto::AddJitter(kRetryDelay, kRetryJitter); + + LogInfo("Will retry adding %s in %lu msec to NetData", LocalToString().AsCString(), ToUlong(delay)); + mLocalInNetDataState = kToAdd; + mTimer.Start(delay); + ExitNow(); + } + + mLocalInNetDataState = kAdded; + mTimer.Stop(); + + SetFavoredPrefix(mLocalPrefix); exit: - return error; + return; } Error RoutingManager::OmrPrefixManager::AddOrUpdateLocalInNetData(void) @@ -1003,30 +1102,33 @@ Error RoutingManager::OmrPrefixManager::AddOrUpdateLocalInNetData(void) SuccessOrExit(error = Get().AddOnMeshPrefix(config)); Get().HandleServerDataUpdated(); - LogInfo("%s %s in Thread Network Data", !mIsLocalAddedInNetData ? "Added" : "Updated", LocalToString().AsCString()); + LogInfo("%s %s in NetData", !IsLocalAddedInNetData() ? "Added" : "Updated", LocalToString().AsCString()); exit: - LogWarnOnError(error, "%s %s in Thread Network Data", !mIsLocalAddedInNetData ? "add" : "update", - LocalToString().AsCString()); + LogWarnOnError(error, "%s %s in NetData", !IsLocalAddedInNetData() ? "add" : "update", LocalToString().AsCString()); return error; } void RoutingManager::OmrPrefixManager::RemoveLocalFromNetData(void) { - Error error = kErrorNone; + switch (mLocalInNetDataState) + { + case kAdded: + IgnoreError(Get().RemoveOnMeshPrefix(mLocalPrefix.GetPrefix())); + Get().HandleServerDataUpdated(); + LogInfo("Removed %s from NetData", LocalToString().AsCString()); + mLocalInNetDataState = kNotAdded; + break; - VerifyOrExit(mIsLocalAddedInNetData); + case kToAdd: + LogInfo("Canceled adding %s to NetData", LocalToString().AsCString()); + mTimer.Stop(); + mLocalInNetDataState = kNotAdded; + break; - error = Get().RemoveOnMeshPrefix(mLocalPrefix.GetPrefix()); - - LogWarnOnError(error, "remove %s from Thread Network Data", LocalToString().AsCString()); - - mIsLocalAddedInNetData = false; - Get().HandleServerDataUpdated(); - LogInfo("Removed %s from Thread Network Data", LocalToString().AsCString()); - -exit: - return; + case kNotAdded: + break; + } } void RoutingManager::OmrPrefixManager::UpdateDefaultRouteFlag(bool aDefaultRoute) @@ -1035,7 +1137,7 @@ void RoutingManager::OmrPrefixManager::UpdateDefaultRouteFlag(bool aDefaultRoute mDefaultRoute = aDefaultRoute; - VerifyOrExit(mIsLocalAddedInNetData); + VerifyOrExit(IsLocalAddedInNetData()); IgnoreError(AddOrUpdateLocalInNetData()); exit: @@ -2517,15 +2619,14 @@ void RoutingManager::TxRaInfo::CalculateHash(const RouterAdvert::RxMessage &aRaM RoutingManager::PdPrefixManager::PdPrefixManager(Instance &aInstance) : InstanceLocator(aInstance) , mState(kDhcp6PdStateDisabled) + , mEvents(0) , mOnLinkPrefixConflict(false) , mRoutePrefixConflict(false) , mNumPlatformPioProcessed(0) , mNumPlatformRaReceived(0) , mLastPlatformRaTime(0) , mTimer(aInstance) -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - , mRecordHistoryTask(aInstance) -#endif + , mEventTask(aInstance) { } @@ -2614,11 +2715,7 @@ void RoutingManager::PdPrefixManager::SetState(State aState) break; } -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - mRecordHistoryTask.Post(); -#endif - - mStateCallback.InvokeIfSet(MapEnum(mState)); + SignalEvent(kEventStateChanged); exit: return; @@ -2665,11 +2762,7 @@ void RoutingManager::PdPrefixManager::WithdrawPrefix(void) mTimer.Stop(); - Get().ScheduleRoutingPolicyEvaluation(kImmediately); - -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - mRecordHistoryTask.Post(); -#endif + SignalEvent(kEventPdPrefixChanged); exit: return; @@ -2758,11 +2851,7 @@ void RoutingManager::PdPrefixManager::ApplyFavoredPrefix(const PdPrefix &aFavore LogInfo("DHCPv6 PD prefix set to %s", mPrefix.GetPrefix().ToString().AsCString()); CheckConflict(kPdPrefixChanged); - Get().ScheduleRoutingPolicyEvaluation(kImmediately); - -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - mRecordHistoryTask.Post(); -#endif + SignalEvent(kEventPdPrefixChanged); } if (HasPrefix()) @@ -2835,7 +2924,7 @@ void RoutingManager::PdPrefixManager::CheckConflict(ConflictCheckEvent aEvent) if (hadConflict != HasConflict()) { - Get().ScheduleRoutingPolicyEvaluation(kImmediately); + SignalEvent(kEventConflictStateChanged); } exit: @@ -2894,15 +2983,33 @@ exit: return; } -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - -void RoutingManager::PdPrefixManager::HandleRecordHistoryTask(void) +void RoutingManager::PdPrefixManager::SignalEvent(Event aEvent) { - Get().RecordDhcp6Pd(mState, mPrefix.GetPrefix()); + mEvents |= aEvent; + mEventTask.Post(); } +void RoutingManager::PdPrefixManager::HandleEventTask(void) +{ + Events events = mEvents; + + mEvents = 0; + + Get().mOmrPrefixManager.HandlePdPrefixManagerEvent(); + +#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE + if (events & (kEventStateChanged | kEventPdPrefixChanged)) + { + Get().RecordDhcp6Pd(mState, mPrefix.GetPrefix()); + } #endif + if (events & kEventStateChanged) + { + mStateCallback.InvokeIfSet(MapEnum(mState)); + } +} + bool RoutingManager::PdPrefixManager::PdPrefix::IsValidPdPrefix(void) const { // We should accept ULA prefix since it could be used by the internet infrastructure like NAT64. diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index 7fe77618f..551491f71 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -617,6 +617,8 @@ private: //------------------------------------------------------------------------------------------------------------------ // Nested types + void HandleOmrPrefixManagerTimer(void) { mOmrPrefixManager.HandleTimer(); } + class OmrPrefixManager : public InstanceLocator { public: @@ -634,8 +636,18 @@ private: const Ip6::Prefix &GetGeneratedPrefix(void) const { return mGeneratedPrefix; } const OmrPrefix &GetLocalPrefix(void) const { return mLocalPrefix; } const FavoredOmrPrefix &GetFavoredPrefix(void) const { return mFavoredPrefix; } + void HandleTimer(void); + void HandleNetDataChange(void); +#if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE + void HandlePdPrefixManagerEvent(void); +#endif private: + // All times are in msec + static constexpr uint32_t kMinDelayToAdd = 250; + static constexpr uint32_t kMaxDelayToAdd = kMinDelayToAdd + 3500; + static constexpr uint32_t kRetryDelay = 1500; + static constexpr uint16_t kRetryJitter = 150; static constexpr uint16_t kInfoStringSize = 85; typedef String InfoString; @@ -647,11 +659,19 @@ private: kDhcp6Pd, }; + enum LocalPrefixState : uint8_t // State of `mLocalPrefix` in Network Data + { + kNotAdded, + kToAdd, + kAdded, + }; + void SetFavoredPrefix(const OmrPrefix &aOmrPrefix); void ClearFavoredPrefix(void) { SetFavoredPrefix(OmrPrefix()); } void DetermineFavoredPrefixInNetData(FavoredOmrPrefix &aFavoredPrefix); void UpdateLocalPrefix(void); - Error AddLocalToNetData(void); + bool IsLocalAddedInNetData(void) const { return (mLocalInNetDataState == kAdded); } + void AddLocalToNetData(void); Error AddOrUpdateLocalInNetData(void); void RemoveLocalFromNetData(void); InfoString LocalToString(void) const; @@ -660,14 +680,19 @@ private: static const char *OmrConfigToString(OmrConfig aConfig); static const char *PrefixOriginToString(PrefixOrigin aOrigin); + using DelayTimer = TimerMilliIn; + OmrConfig mConfig; OmrPrefix mLocalPrefix; OmrPrefix mCustomPrefix; Ip6::Prefix mGeneratedPrefix; FavoredOmrPrefix mFavoredPrefix; + DelayTimer mTimer; PrefixOrigin mLocalPrefixOrigin; - bool mIsLocalAddedInNetData; - bool mDefaultRoute; + LocalPrefixState mLocalInNetDataState; + bool mDefaultRoute : 1; + bool mIsInitialized : 1; + bool mIsRunning : 1; }; //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -949,9 +974,7 @@ private: #if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE void HandlePdPrefixManagerTimer(void) { mPdPrefixManager.HandleTimer(); } -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - void HandlePdPrefixManagerTask(void) { mPdPrefixManager.HandleRecordHistoryTask(); } -#endif + void HandlePdPrefixManagerEventTask(void) { mPdPrefixManager.HandleEventTask(); } class PdPrefixManager : public InstanceLocator { @@ -970,6 +993,15 @@ private: kRxRaPrefixTableChanged, }; + enum Event : uint8_t + { + kEventStateChanged = 1 << 0, + kEventPdPrefixChanged = 1 << 1, + kEventConflictStateChanged = 1 << 2, + }; + + typedef uint8_t Events; + explicit PdPrefixManager(Instance &aInstance); void SetEnabled(bool aEnabled); @@ -988,9 +1020,7 @@ private: void HandleTimer(void) { WithdrawPrefix(); } void SetStateCallback(Dhcp6PdCallback aCallback, void *aContext) { mStateCallback.Set(aCallback, aContext); } void Evaluate(void); -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - void HandleRecordHistoryTask(void); -#endif + void HandleEventTask(void); private: class PdPrefix : public OnLinkPrefix @@ -1004,6 +1034,7 @@ private: void UpdateState(void); void SetState(State aState); + void SignalEvent(Event aEvent); void EvaluateCandidatePrefix(PdPrefix &aPrefix, PdPrefix &aFavoredPrefix); void ApplyFavoredPrefix(const PdPrefix &aFavoredPrefix); void WithdrawPrefix(void); @@ -1015,11 +1046,10 @@ private: using PrefixTimer = TimerMilliIn; using StateCallback = Callback; -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - using RecordHistoryTask = TaskletIn; -#endif + using EventTask = TaskletIn; State mState; + Events mEvents; bool mOnLinkPrefixConflict; bool mRoutePrefixConflict; uint32_t mNumPlatformPioProcessed; @@ -1027,10 +1057,8 @@ private: TimeMilli mLastPlatformRaTime; StateCallback mStateCallback; PrefixTimer mTimer; + EventTask mEventTask; PdPrefix mPrefix; -#if OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE - RecordHistoryTask mRecordHistoryTask; -#endif }; #endif // OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp index fb5c476e8..d0e9801b3 100644 --- a/tests/unit/test_routing_manager.cpp +++ b/tests/unit/test_routing_manager.cpp @@ -5186,7 +5186,7 @@ void TestDhcp6PdConflict(void) // Now Advertise the PD prefix as on-link from a router first. Log("Router A advertises PD prefix as on-link before delegating the prefix"); - SendRouterAdvert(routerAddressA, {Pio(pdPrefix, 200, 200)}); + SendRouterAdvert(routerAddressA, {Pio(pdPrefix, 350, 350)}); // Check that local OMR is used. @@ -5208,7 +5208,7 @@ void TestDhcp6PdConflict(void) sExpectedRios.Clear(); sExpectedRios.Add(localOmr); - AdvanceTime(100 * 1000); + AdvanceTime(200 * 1000); VerifyOrQuit(sExpectedRios.SawAll()); VerifyOmrPrefixInNetData(localOmr, /* aDefaultRoute */ true); @@ -5266,7 +5266,7 @@ void TestDhcp6PdConflict(void) sExpectedRios.Clear(); sExpectedRios.Add(localOmr); - AdvanceTime(30 * 1000); + AdvanceTime(200 * 1000); VerifyOrQuit(sExpectedRios.SawAll()); VerifyOmrPrefixInNetData(localOmr, /* aDefaultRoute */ false);