From 31ac6266faa7652fcf35bb32ac684f6d7b007da3 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 22 May 2024 11:09:02 -0700 Subject: [PATCH] [routing-manager] track whether a discovered router is local device (#10284) This commit adds `mIsLocalDevice` in the `Router` class to track whether it represents the local device (e.g., another software entity on this device). This information is used to skip sending NS probes to this router. Tracking this information directly in the `Router` class is safer than checking the address using `mInfra.HasAddress()`, as the addresses may have changed since the last RA was received. This commit exposes this information through the public APIs and CLI command, and updates `test_routing_manager` unit test to validate the new flag. --- include/openthread/border_routing.h | 1 + include/openthread/instance.h | 2 +- src/cli/cli_br.cpp | 15 ++++++++------- src/core/border_router/routing_manager.cpp | 14 +++++++------- src/core/border_router/routing_manager.hpp | 1 + tests/unit/test_routing_manager.cpp | 14 ++++++++++---- 6 files changed, 28 insertions(+), 19 deletions(-) diff --git a/include/openthread/border_routing.h b/include/openthread/border_routing.h index e00c33f31..403c9b600 100644 --- a/include/openthread/border_routing.h +++ b/include/openthread/border_routing.h @@ -101,6 +101,7 @@ typedef struct otBorderRoutingRouterEntry bool mManagedAddressConfigFlag : 1; ///< The router's Managed Address Config flag (`M` flag). bool mOtherConfigFlag : 1; ///< The router's Other Config flag (`O` flag). bool mStubRouterFlag : 1; ///< The router's Stub Router flag. + bool mIsLocalDevice : 1; ///< This router is the local device (this BR). } otBorderRoutingRouterEntry; /** diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 1964b22d0..df87b06cb 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (414) +#define OPENTHREAD_API_VERSION (415) /** * @addtogroup api-instance diff --git a/src/cli/cli_br.cpp b/src/cli/cli_br.cpp index b24cf76d5..b76e76716 100644 --- a/src/cli/cli_br.cpp +++ b/src/cli/cli_br.cpp @@ -557,16 +557,17 @@ void Br::OutputRouterInfo(const otBorderRoutingRouterEntry &aEntry, RouterOutput OutputFormat(" (M:%u O:%u Stub:%u)", aEntry.mManagedAddressConfigFlag, aEntry.mOtherConfigFlag, aEntry.mStubRouterFlag); - switch (aMode) + if (aMode == kLongVersion) { - case kShortVersion: - OutputNewLine(); - break; + OutputFormat(" ms-since-rx:%lu", ToUlong(aEntry.mMsecSinceLastUpdate)); - case kLongVersion: - OutputLine(" ms-since-rx:%lu", ToUlong(aEntry.mMsecSinceLastUpdate)); - break; + if (aEntry.mIsLocalDevice) + { + OutputFormat(" (this BR)"); + } } + + OutputNewLine(); } template <> otError Br::Process(Arg aArgs[]) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index f03121b78..d11cbe632 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -1013,6 +1013,8 @@ void RoutingManager::RxRaTracker::ProcessRouterAdvertMessage(const RouterAdvert: } } + router->mIsLocalDevice = (aRaOrigin == kThisBrOtherEntity); + UpdateRouterOnRx(*router); RemoveRoutersWithNoEntriesOrFlags(); @@ -1614,14 +1616,11 @@ void RoutingManager::RxRaTracker::HandleRouterTimer(void) continue; } - // If the `router` emitting RA has an address belonging to - // infra interface, it indicates that the RAs are from - // same device. In this case we skip performing NS probes. - // This addresses situation where platform may not be - // be able to receive and pass the NA message response - // from device itself. + // Skip NS probes if the router is this device. This prevents + // issues where the platform might not be able to receive and + // process the NA messages from the local device itself. - if (Get().mInfraIf.HasAddress(router.mAddress)) + if (router.mIsLocalDevice) { continue; } @@ -1877,6 +1876,7 @@ void RoutingManager::RxRaTracker::Router::CopyInfoTo(RouterEntry &aEntry, TimeMi aEntry.mManagedAddressConfigFlag = mManagedAddressConfigFlag; aEntry.mOtherConfigFlag = mOtherConfigFlag; aEntry.mStubRouterFlag = mStubRouterFlag; + aEntry.mIsLocalDevice = mIsLocalDevice; } //--------------------------------------------------------------------------------------------------------------------- diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index a38eb8205..25b71f15c 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -831,6 +831,7 @@ private: bool mManagedAddressConfigFlag : 1; bool mOtherConfigFlag : 1; bool mStubRouterFlag : 1; + bool mIsLocalDevice : 1; }; //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp index 74413e43d..b6040b7af 100644 --- a/tests/unit/test_routing_manager.cpp +++ b/tests/unit/test_routing_manager.cpp @@ -1108,8 +1108,10 @@ struct InfraRouter InfraRouter(const Ip6::Address &aAddress, bool aManagedAddressConfigFlag, bool aOtherConfigFlag, - bool aStubRouterFlag) + bool aStubRouterFlag, + bool aIsLocalDevice = false) : mAddress(aAddress) + , mIsLocalDevice(aIsLocalDevice) { mFlags.Clear(); mFlags.mManagedAddressConfigFlag = aManagedAddressConfigFlag; @@ -1119,6 +1121,7 @@ struct InfraRouter Ip6::Address mAddress; RaFlags mFlags; + bool mIsLocalDevice; }; template void VerifyDiscoveredRouters(const InfraRouter (&aRouters)[kNumRouters]) @@ -1140,8 +1143,9 @@ void VerifyDiscoveredRouters(const InfraRouter *aRouters, uint16_t aNumRouters) { bool didFind = false; - Log(" address:%s, M:%u, O:%u, StubRouter:%u", AsCoreType(&entry.mAddress).ToString().AsCString(), - entry.mManagedAddressConfigFlag, entry.mOtherConfigFlag, entry.mStubRouterFlag); + Log(" address:%s, M:%u, O:%u, StubRouter:%u%s", AsCoreType(&entry.mAddress).ToString().AsCString(), + entry.mManagedAddressConfigFlag, entry.mOtherConfigFlag, entry.mStubRouterFlag, + entry.mIsLocalDevice ? " (this BR)" : ""); for (uint16_t index = 0; index < aNumRouters; index++) { @@ -1150,6 +1154,7 @@ void VerifyDiscoveredRouters(const InfraRouter *aRouters, uint16_t aNumRouters) VerifyOrQuit(entry.mManagedAddressConfigFlag == aRouters[index].mFlags.mManagedAddressConfigFlag); VerifyOrQuit(entry.mOtherConfigFlag == aRouters[index].mFlags.mOtherConfigFlag); VerifyOrQuit(entry.mStubRouterFlag == aRouters[index].mFlags.mStubRouterFlag); + VerifyOrQuit(entry.mIsLocalDevice == aRouters[index].mIsLocalDevice); didFind = true; } } @@ -3015,7 +3020,8 @@ void TestLearnRaHeader(void) SendRouterAdvert(sInfraIfAddress, DefaultRoute(1000, NetworkData::kRoutePreferenceLow)); AdvanceTime(1); - VerifyDiscoveredRouters({InfraRouter(sInfraIfAddress, /* M */ false, /* O */ false, /* StubRouter */ false)}); + VerifyDiscoveredRouters( + {InfraRouter(sInfraIfAddress, /* M */ false, /* O */ false, /* StubRouter */ false, /* IsLocalDevice */ true)}); //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // RoutingManager should learn the header from the