From b492922a9c229e8f9260b6bd19b478685885fb37 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 23 Oct 2025 15:55:11 -0700 Subject: [PATCH] [border-router] centralize infra-if management in `InfraIf` (#12046) This change moves the management of the infrastructure interface state out of the `RoutingManager` and centralizes it within the `InfraIf` class. This makes `InfraIf` a more self-contained component and simplifies the logic in `RoutingManager`. The `RoutingManager` now depends on an initialized `InfraIf`. Its `Init()` method is simplified and is now called from `InfraIf::Init()`. The public API `otBorderRoutingInit()` now directly initializes the `InfraIf`. The `InfraIf::Init()` method is updated to support re-initialization, allowing to switch to a new interface. When switching, it ensures that components on the previous interface are stopped before restarting on the new one. --- include/openthread/border_routing.h | 13 ++-- include/openthread/instance.h | 2 +- src/core/api/border_routing_api.cpp | 17 +++-- src/core/border_router/infra_if.cpp | 35 ++++++--- src/core/border_router/infra_if.hpp | 12 +-- src/core/border_router/routing_manager.cpp | 87 +++++----------------- src/core/border_router/routing_manager.hpp | 35 ++------- tests/fuzz/cli.cpp | 2 +- tests/fuzz/icmp6.cpp | 2 +- tests/fuzz/ip6.cpp | 2 +- tests/fuzz/mdns.cpp | 2 +- tests/fuzz/radio-one-node.cpp | 2 +- tests/fuzz/trel.cpp | 2 +- 13 files changed, 86 insertions(+), 127 deletions(-) diff --git a/include/openthread/border_routing.h b/include/openthread/border_routing.h index d70493d74..2cf31d24c 100644 --- a/include/openthread/border_routing.h +++ b/include/openthread/border_routing.h @@ -211,19 +211,18 @@ typedef enum /** * Initializes the Border Routing Manager on given infrastructure interface. * - * @note This method MUST be called before any other otBorderRouting* APIs. - * @note This method can be re-called to change the infrastructure interface, but the Border Routing Manager should be - * disabled first, and re-enabled after. + * This function MUST be called before any other otBorderRouting* APIs. + * + * This function can also be used to re-initialize and switch the infrastructure interface index to a new one. + * Switching the interface index will trigger all components running on the previous interface (Border Routing, + * mDNS, etc) to be stopped (as if the previous if-index is no longer running) before restarting operations on the + * new interface. * * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aInfraIfIndex The infrastructure interface index. * @param[in] aInfraIfIsRunning A boolean that indicates whether the infrastructure - * interface is running. * * @retval OT_ERROR_NONE Successfully started the Border Routing Manager on given infrastructure. - * @retval OT_ERROR_INVALID_STATE The Border Routing Manager is in a state other than disabled or uninitialized. - * @retval OT_ERROR_INVALID_ARGS The index of the infrastructure interface is not valid. - * @retval OT_ERROR_FAILED Internal failure. Usually due to failure in generating random prefixes. * * @sa otPlatInfraIfStateChanged. * @sa otBorderRoutingSetEnabled. diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 42995e885..450d18993 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -52,7 +52,7 @@ extern "C" { * * @note This number versions both OpenThread platform and user APIs. */ -#define OPENTHREAD_API_VERSION (544) +#define OPENTHREAD_API_VERSION (545) /** * @addtogroup api-instance diff --git a/src/core/api/border_routing_api.cpp b/src/core/api/border_routing_api.cpp index c5b064f16..60e27f25b 100644 --- a/src/core/api/border_routing_api.cpp +++ b/src/core/api/border_routing_api.cpp @@ -41,21 +41,28 @@ using namespace ot; otError otBorderRoutingInit(otInstance *aInstance, uint32_t aInfraIfIndex, bool aInfraIfIsRunning) { - return AsCoreType(aInstance).Get().Init(aInfraIfIndex, aInfraIfIsRunning); + AsCoreType(aInstance).Get().Init(aInfraIfIndex, aInfraIfIsRunning); + + return kErrorNone; } otError otBorderRoutingGetInfraIfInfo(otInstance *aInstance, uint32_t *aInfraIfIndex, bool *aInfraIfIsRunning) { - bool isRunning; + Error error = kErrorNone; AssertPointerIsNotNull(aInfraIfIndex); - if (aInfraIfIsRunning == nullptr) + VerifyOrExit(AsCoreType(aInstance).Get().IsInitialized(), error = kErrorInvalidState); + + *aInfraIfIndex = AsCoreType(aInstance).Get().GetIfIndex(); + + if (aInfraIfIsRunning != nullptr) { - aInfraIfIsRunning = &isRunning; + *aInfraIfIsRunning = AsCoreType(aInstance).Get().IsRunning(); } - return AsCoreType(aInstance).Get().GetInfraIfInfo(*aInfraIfIndex, *aInfraIfIsRunning); +exit: + return error; } otError otBorderRoutingSetEnabled(otInstance *aInstance, bool aEnabled) diff --git a/src/core/border_router/infra_if.cpp b/src/core/border_router/infra_if.cpp index d0eac1b7e..02d5e48e4 100644 --- a/src/core/border_router/infra_if.cpp +++ b/src/core/border_router/infra_if.cpp @@ -50,28 +50,45 @@ InfraIf::InfraIf(Instance &aInstance) { } -Error InfraIf::Init(uint32_t aIfIndex) +void InfraIf::Init(uint32_t aInfraIfIndex, bool aInfraIfIsRunning) { - Error error = kErrorNone; + if (mInitialized) + { + VerifyOrExit(aInfraIfIndex != mIfIndex); - VerifyOrExit(!mInitialized, error = kErrorInvalidState); + LogInfo("Switching previously configured %s to %lu", ToString().AsCString(), ToUlong(aInfraIfIndex)); - mIfIndex = aIfIndex; + // When switching interface index, we `Deinit()` to signal + // that the previous `mIfIndex` is down so that all modules + // operating on this interface are stopped before restarting + // operation on the new interface. + + Deinit(); + } + + mIfIndex = aInfraIfIndex; mInitialized = true; LogInfo("Init %s", ToString().AsCString()); + Get().Init(); + exit: - return error; + IgnoreError(HandleStateChanged(mIfIndex, aInfraIfIsRunning)); } void InfraIf::Deinit(void) { - mInitialized = false; - mIsRunning = false; - mIfIndex = 0; + VerifyOrExit(mInitialized); - LogInfo("Deinit"); + LogInfo("Deinit %s", ToString().AsCString()); + + IgnoreError(HandleStateChanged(mIfIndex, /* aIsRunning */ false)); + + mInitialized = false; + +exit: + return; } bool InfraIf::HasAddress(const Ip6::Address &aAddress) const diff --git a/src/core/border_router/infra_if.hpp b/src/core/border_router/infra_if.hpp index b7057a01c..1f1aed7f0 100644 --- a/src/core/border_router/infra_if.hpp +++ b/src/core/border_router/infra_if.hpp @@ -101,13 +101,15 @@ public: /** * Initializes the `InfraIf`. * - * @param[in] aIfIndex The infrastructure interface index. + * This method can also be used to re-initialize and switch the infrastructure interface index to a new one. + * Switching the interface index will trigger all components running on the previous interface (Border Routing, + * mDNS, etc) to be stopped (as if the previous if-index is no longer running) before restarting operations on the + * new interface. * - * @retval kErrorNone Successfully initialized the `InfraIf`. - * @retval kErrorInvalidArgs The index of the infra interface is not valid. - * @retval kErrorInvalidState The `InfraIf` is already initialized. + * @param[in] aInfraIfIndex The infrastructure network interface index. + * @param[in] aInfraIfIsRunning A boolean that indicates whether the infrastructure interface is running. */ - Error Init(uint32_t aIfIndex); + void Init(uint32_t aInfraIfIndex, bool aInfraIfIsRunning); /** * Deinitilaizes the `InfraIf`. diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 0f972ef50..0bda7cad1 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -73,65 +73,26 @@ RoutingManager::RoutingManager(Instance &aInstance) mBrUlaPrefix.Clear(); } -Error RoutingManager::Init(uint32_t aInfraIfIndex, bool aInfraIfIsRunning) +void RoutingManager::Init(void) { - Error error; + VerifyOrExit(Get().IsInitialized()); - VerifyOrExit(GetState() == kStateUninitialized || GetState() == kStateDisabled, error = kErrorInvalidState); - - if (!Get().IsInitialized()) - { - LogInfo("Initializing - InfraIfIndex:%lu", ToUlong(aInfraIfIndex)); - SuccessOrExit(error = Get().Init(aInfraIfIndex)); - SuccessOrExit(error = LoadOrGenerateRandomBrUlaPrefix()); - mOmrPrefixManager.Init(mBrUlaPrefix); + LoadOrGenerateRandomBrUlaPrefix(); + mOmrPrefixManager.Init(mBrUlaPrefix); #if OPENTHREAD_CONFIG_NAT64_BORDER_ROUTING_ENABLE - mNat64PrefixManager.GenerateLocalPrefix(mBrUlaPrefix); + mNat64PrefixManager.GenerateLocalPrefix(mBrUlaPrefix); #endif - mOnLinkPrefixManager.Init(); - } - else if (aInfraIfIndex != Get().GetIfIndex()) - { - LogInfo("Reinitializing - InfraIfIndex:%lu -> %lu", ToUlong(Get().GetIfIndex()), - ToUlong(aInfraIfIndex)); - -#if OPENTHREAD_CONFIG_MULTICAST_DNS_ENABLE && OPENTHREAD_CONFIG_MULTICAST_DNS_AUTO_ENABLE_ON_INFRA_IF - IgnoreError(Get().SetEnabled(false, Get().GetIfIndex())); -#endif - - Get().SetIfIndex(aInfraIfIndex); - } - - error = Get().HandleStateChanged(Get().GetIfIndex(), aInfraIfIsRunning); + mOnLinkPrefixManager.Init(); exit: - if (error != kErrorNone) - { - Get().Deinit(); - } - - return error; -} - -bool RoutingManager::IsInitialized(void) const { return Get().IsInitialized(); } - -Error RoutingManager::GetInfraIfInfo(uint32_t &aInfraIfIndex, bool &aInfraIfIsRunning) const -{ - Error error = kErrorNone; - - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); - aInfraIfIndex = Get().GetIfIndex(); - aInfraIfIsRunning = Get().IsRunning(); - -exit: - return error; + return; } Error RoutingManager::SetEnabled(bool aEnabled) { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); VerifyOrExit(aEnabled != mIsEnabled); @@ -147,7 +108,7 @@ RoutingManager::State RoutingManager::GetState(void) const { State state = kStateUninitialized; - VerifyOrExit(IsInitialized()); + VerifyOrExit(Get().IsInitialized()); VerifyOrExit(IsEnabled(), state = kStateDisabled); state = IsRunning() ? kStateRunning : kStateStopped; @@ -160,7 +121,7 @@ Error RoutingManager::GetOmrPrefix(Ip6::Prefix &aPrefix) const { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); aPrefix = mOmrPrefixManager.GetGeneratedPrefix(); exit: @@ -172,7 +133,7 @@ Error RoutingManager::GetDhcp6PdOmrPrefix(Dhcp6PdPrefix &aPrefix) const { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); error = mPdPrefixManager.GetPrefix(aPrefix); exit: @@ -183,7 +144,7 @@ Error RoutingManager::GetDhcp6PdCounters(Dhcp6PdCounters &aCounters) { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); error = mPdPrefixManager.GetCounters(aCounters); exit: @@ -207,7 +168,7 @@ Error RoutingManager::GetOnLinkPrefix(Ip6::Prefix &aPrefix) const { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); aPrefix = mOnLinkPrefixManager.GetLocalPrefix(); exit: @@ -218,7 +179,7 @@ Error RoutingManager::GetFavoredOnLinkPrefix(Ip6::Prefix &aPrefix) const { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); aPrefix = mOnLinkPrefixManager.GetFavoredPrefix(); exit: @@ -236,7 +197,7 @@ Error RoutingManager::GetNat64Prefix(Ip6::Prefix &aPrefix) { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); aPrefix = mNat64PrefixManager.GetLocalPrefix(); exit: @@ -247,7 +208,7 @@ Error RoutingManager::GetFavoredNat64Prefix(Ip6::Prefix &aPrefix, RoutePreferenc { Error error = kErrorNone; - VerifyOrExit(IsInitialized(), error = kErrorInvalidState); + VerifyOrExit(Get().IsInitialized(), error = kErrorInvalidState); aPrefix = mNat64PrefixManager.GetFavoredPrefix(aRoutePreference); exit: @@ -255,10 +216,9 @@ exit: } #endif -Error RoutingManager::LoadOrGenerateRandomBrUlaPrefix(void) +void RoutingManager::LoadOrGenerateRandomBrUlaPrefix(void) { - Error error = kErrorNone; - bool generated = false; + bool generated = false; if (Get().Read(mBrUlaPrefix) != kErrorNone || !IsValidBrUlaPrefix(mBrUlaPrefix)) { @@ -266,7 +226,7 @@ Error RoutingManager::LoadOrGenerateRandomBrUlaPrefix(void) LogNote("No valid /48 BR ULA prefix found in settings, generating new one"); - SuccessOrExit(error = randomUlaPrefix.GenerateRandomUla()); + SuccessOrAssert(randomUlaPrefix.GenerateRandomUla()); mBrUlaPrefix.Set(randomUlaPrefix); mBrUlaPrefix.SetSubnetId(0); @@ -279,13 +239,6 @@ Error RoutingManager::LoadOrGenerateRandomBrUlaPrefix(void) OT_UNUSED_VARIABLE(generated); LogNote("BR ULA prefix: %s (%s)", mBrUlaPrefix.ToString().AsCString(), generated ? "generated" : "loaded"); - -exit: - if (error != kErrorNone) - { - LogCrit("Failed to generate random /48 BR ULA prefix"); - } - return error; } void RoutingManager::EvaluateState(void) @@ -435,7 +388,7 @@ void RoutingManager::HandleNotifierEvents(Events aEvents) mRoutePublisher.HandleNotifierEvents(aEvents); - VerifyOrExit(IsInitialized() && IsEnabled()); + VerifyOrExit(Get().IsInitialized() && IsEnabled()); if (aEvents.Contains(kEventThreadRoleChanged)) { diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index 3a513edbc..7be7a61d8 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -156,27 +156,9 @@ public: explicit RoutingManager(Instance &aInstance); /** - * Initializes the routing manager on given infrastructure interface. - * - * @param[in] aInfraIfIndex An infrastructure network interface index. - * @param[in] aInfraIfIsRunning A boolean that indicates whether the infrastructure - * interface is running. - * - * @retval kErrorNone Successfully started the routing manager. - * @retval kErrorInvalidArgs The index of the infra interface is not valid. + * Initializes the routing manager. */ - Error Init(uint32_t aInfraIfIndex, bool aInfraIfIsRunning); - - /** - * Gets the interface index of the currently configured infrastructure interface. - * - * @param[out] aInfraIfIndex A reference to output the interface index. - * @param[out] aInfraIfIsRunning A reference to output whether the interface is running. - * - * @retval kErrorNone Successfully retrieved the interface information. - * @retval kErrorInvalidState The Border Routing Manager is not initialized. - */ - Error GetInfraIfInfo(uint32_t &aInfraIfIndex, bool &aInfraIfIsRunning) const; + void Init(void); /** * Enables/disables the Border Routing Manager. @@ -1103,13 +1085,12 @@ private: //------------------------------------------------------------------------------------------------------------------ // Methods - void EvaluateState(void); - void Start(void); - void Stop(void); - void HandleNotifierEvents(Events aEvents); - bool IsInitialized(void) const; - bool IsEnabled(void) const { return mIsEnabled; } - Error LoadOrGenerateRandomBrUlaPrefix(void); + void EvaluateState(void); + void Start(void); + void Stop(void); + void HandleNotifierEvents(Events aEvents); + bool IsEnabled(void) const { return mIsEnabled; } + void LoadOrGenerateRandomBrUlaPrefix(void); void EvaluateRoutingPolicy(void); bool IsInitialPolicyEvaluationDone(void) const; diff --git a/tests/fuzz/cli.cpp b/tests/fuzz/cli.cpp index 3bf4b64e3..2c73e7ab3 100644 --- a/tests/fuzz/cli.cpp +++ b/tests/fuzz/cli.cpp @@ -111,7 +111,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) otCliInit(&node.GetInstance(), CliOutput, nullptr); - node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); + node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); node.GetInstance().Get().SetEnabled(true); node.GetInstance().Get().SetAutoEnableMode(true); node.GetInstance().Get().SetDhcp6PdEnabled(true); diff --git a/tests/fuzz/icmp6.cpp b/tests/fuzz/icmp6.cpp index 3c4d54e61..faf6c6acc 100644 --- a/tests/fuzz/icmp6.cpp +++ b/tests/fuzz/icmp6.cpp @@ -100,7 +100,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) node.GetInstance().SetLogLevel(kLogLevelInfo); - node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); + node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); node.GetInstance().Get().SetEnabled(true); node.GetInstance().Get().SetAutoEnableMode(true); node.GetInstance().Get().SetDhcp6PdEnabled(true); diff --git a/tests/fuzz/ip6.cpp b/tests/fuzz/ip6.cpp index bc6b946bf..56eee3705 100644 --- a/tests/fuzz/ip6.cpp +++ b/tests/fuzz/ip6.cpp @@ -112,7 +112,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) node.GetInstance().SetLogLevel(kLogLevelInfo); - node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); + node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); node.GetInstance().Get().SetEnabled(true); node.GetInstance().Get().SetAutoEnableMode(true); node.GetInstance().Get().SetDhcp6PdEnabled(true); diff --git a/tests/fuzz/mdns.cpp b/tests/fuzz/mdns.cpp index 49fb47fe8..940983ff4 100644 --- a/tests/fuzz/mdns.cpp +++ b/tests/fuzz/mdns.cpp @@ -111,7 +111,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) node.GetInstance().SetLogLevel(kLogLevelInfo); - node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); + node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); node.GetInstance().Get().SetEnabled(true); node.GetInstance().Get().SetAutoEnableMode(true); node.GetInstance().Get().SetDhcp6PdEnabled(true); diff --git a/tests/fuzz/radio-one-node.cpp b/tests/fuzz/radio-one-node.cpp index b4535f339..aa4e229c2 100644 --- a/tests/fuzz/radio-one-node.cpp +++ b/tests/fuzz/radio-one-node.cpp @@ -110,7 +110,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) node.GetInstance().SetLogLevel(kLogLevelInfo); - node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); + node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); node.GetInstance().Get().SetEnabled(true); node.GetInstance().Get().SetAutoEnableMode(true); node.GetInstance().Get().SetDhcp6PdEnabled(true); diff --git a/tests/fuzz/trel.cpp b/tests/fuzz/trel.cpp index 6601059dc..997598df3 100644 --- a/tests/fuzz/trel.cpp +++ b/tests/fuzz/trel.cpp @@ -101,7 +101,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) node.GetInstance().SetLogLevel(kLogLevelInfo); - node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); + node.GetInstance().Get().Init(/* aInfraIfIndex */ 1, /* aInfraIfIsRunning */ true); node.GetInstance().Get().SetEnabled(true); node.GetInstance().Get().SetAutoEnableMode(true); node.GetInstance().Get().SetDhcp6PdEnabled(true);