From fa79f38c02787ae0887ba53433bae15c6fb67e4d Mon Sep 17 00:00:00 2001 From: Simon Lin Date: Tue, 1 Mar 2022 07:55:35 +0800 Subject: [PATCH] [routing-manager] fix small OMR prefix election issue (#7405) This commit fixes two small issues regarding OMR prefix election: - This commit handles the case where `mLocalOmrPrefix` is already in Leader's Network Data, but not locally added. It is possible when the BR restores the Network Data from Leader. In such case the BR should re-add `mLocalOmrPrefix` to local Network Data so that it keeps advertising OMR Prefix. Otherwise, the OMR Prefix will be removed from Leader's Network Data because of Network Data inconsistentency, and then all BRs start to re-elect OMR Prefix. So, the change will make the OMR Prefix more stable in some corner cases. - The change here checks if `mLocalOmrPrefix` was added to the Local Network Data, instead of checking if `mLocalOmrPrefix` is added to the Leader's Network Data. Here the BR has already decided that it should not be advertising `mLocalOmrPrefix`. Whether or not `mLocalOmrPrefix` is in Leader's Network Data is relevant. The BR only needs to remove `mLocalOmrPrefix` from Local Network Data and eventually Local Network Data will synchronize with Leader's Network Data. The original code also has a potential issue that the BR may send `NET_DATA.ntf` to Leader indefinitely when these conditions are met: - `mLocalOmrPrefix` was added to Local Network Data - `mLocalOmrPrefix` is not added to Leader' Network Data because there is no more space in Leader's Network Data --- src/core/border_router/routing_manager.cpp | 37 +++++++++++++++++----- src/core/border_router/routing_manager.hpp | 1 + src/core/thread/network_data_local.cpp | 14 ++++++++ src/core/thread/network_data_local.hpp | 11 +++++++ 4 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index d7686d5a6..69d3287d7 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -462,18 +462,29 @@ void RoutingManager::EvaluateOmrPrefix(OmrPrefixArray &aNewOmrPrefixes) // The `aNewOmrPrefixes` remains empty if we fail to publish // the local OMR prefix. } - else if (publishedLocalOmrPrefix != nullptr && smallestOmrPrefix != publishedLocalOmrPrefix) + else { - LogInfo("EvaluateOmrPrefix: There is already a smaller OMR prefix %s in the Thread network", - smallestOmrPrefix->ToString().AsCString()); + OT_ASSERT(smallestOmrPrefix != nullptr); - UnpublishLocalOmrPrefix(); + if (*smallestOmrPrefix == mLocalOmrPrefix) + { + IgnoreError(PublishLocalOmrPrefix()); + } + else if (IsOmrPrefixAddedToLocalNetworkData()) + { + LogInfo("EvaluateOmrPrefix: There is already a smaller OMR prefix %s in the Thread network", + smallestOmrPrefix->ToString().AsCString()); - // Remove the local OMR prefix from the list by overwriting it - // with the last element and then popping it from the list. + UnpublishLocalOmrPrefix(); - *publishedLocalOmrPrefix = *aNewOmrPrefixes.Back(); - aNewOmrPrefixes.PopBack(); + // Remove the local OMR prefix from the list by overwriting it + // with the last element and then popping it from the list. + if (publishedLocalOmrPrefix != nullptr) + { + *publishedLocalOmrPrefix = *aNewOmrPrefixes.Back(); + aNewOmrPrefixes.PopBack(); + } + } } } @@ -484,6 +495,8 @@ Error RoutingManager::PublishLocalOmrPrefix(void) OT_ASSERT(mIsRunning); + VerifyOrExit(!IsOmrPrefixAddedToLocalNetworkData()); + omrPrefixConfig.Clear(); omrPrefixConfig.mPrefix = mLocalOmrPrefix; omrPrefixConfig.mStable = true; @@ -505,6 +518,7 @@ Error RoutingManager::PublishLocalOmrPrefix(void) LogInfo("Publishing local OMR prefix %s in Thread network", mLocalOmrPrefix.ToString().AsCString()); } +exit: return error; } @@ -514,6 +528,8 @@ void RoutingManager::UnpublishLocalOmrPrefix(void) VerifyOrExit(mIsRunning); + VerifyOrExit(IsOmrPrefixAddedToLocalNetworkData()); + SuccessOrExit(error = Get().RemoveOnMeshPrefix(mLocalOmrPrefix)); Get().HandleServerDataUpdated(); @@ -527,6 +543,11 @@ exit: } } +bool RoutingManager::IsOmrPrefixAddedToLocalNetworkData(void) const +{ + return Get().ContainsOnMeshPrefix(mLocalOmrPrefix); +} + Error RoutingManager::AddExternalRoute(const Ip6::Prefix &aPrefix, RoutePreference aRoutePreference, bool aNat64) { Error error; diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index 01325f8f8..cd6e14bf9 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -305,6 +305,7 @@ private: void EvaluateOmrPrefix(OmrPrefixArray &aNewOmrPrefixes); Error PublishLocalOmrPrefix(void); void UnpublishLocalOmrPrefix(void); + bool IsOmrPrefixAddedToLocalNetworkData(void) const; Error AddExternalRoute(const Ip6::Prefix &aPrefix, RoutePreference aRoutePreference, bool aNat64 = false); void RemoveExternalRoute(const Ip6::Prefix &aPrefix); void StartRouterSolicitationDelay(void); diff --git a/src/core/thread/network_data_local.cpp b/src/core/thread/network_data_local.cpp index 8095a95bb..b558140fa 100644 --- a/src/core/thread/network_data_local.cpp +++ b/src/core/thread/network_data_local.cpp @@ -69,6 +69,20 @@ Error Local::RemoveOnMeshPrefix(const Ip6::Prefix &aPrefix) return RemovePrefix(aPrefix, NetworkDataTlv::kTypeBorderRouter); } +bool Local::ContainsOnMeshPrefix(const Ip6::Prefix &aPrefix) const +{ + const PrefixTlv *tlv; + bool contains = false; + + VerifyOrExit((tlv = FindPrefix(aPrefix)) != nullptr); + VerifyOrExit(tlv->FindSubTlv(NetworkDataTlv::kTypeBorderRouter) != nullptr); + + contains = true; + +exit: + return contains; +} + Error Local::AddHasRoutePrefix(const ExternalRouteConfig &aConfig) { Error error = kErrorInvalidArgs; diff --git a/src/core/thread/network_data_local.hpp b/src/core/thread/network_data_local.hpp index de2879435..1123af6b9 100644 --- a/src/core/thread/network_data_local.hpp +++ b/src/core/thread/network_data_local.hpp @@ -97,6 +97,17 @@ public: */ Error RemoveOnMeshPrefix(const Ip6::Prefix &aPrefix); + /** + * This method indicates whether or not the Thread Network Data contains a given on mesh prefix. + * + * @param[in] aPrefix The on mesh prefix to check. + * + * @retval TRUE if Network Data contains mesh prefix @p aPrefix. + * @retval FALSE if Network Data does not contain mesh prefix @p aPrefix. + * + */ + bool ContainsOnMeshPrefix(const Ip6::Prefix &aPrefix) const; + /** * This method adds a Has Route entry to the Thread Network data. *