From 40484c94687163e60e44e3f23855b49e7dd911df Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Mon, 30 May 2022 22:06:00 -0700
Subject: [PATCH] [routing-manager] directly remove on-link prefix from
 discovered list on advert start (#7764)

This commit changes `EvaluteOnLinkPrefix()` so that when we decide to
start advertising the local on-link prefix, we go through the list of
discovered prefixes to check if the local prefix was previously discovered
and included in the list, and remove it from the list (without
unpublishing it from Thread Network Data). This change allows us to
simplify `InvalidateDiscoveredPrefixes()` which was earlier called and
was in charge of removing the local prefix.

This helps align the logic that when we are advertising the on-link
prefix, we do not allow it to be added in the discovered prefix list
from `UpdateDiscoveredOnLinkPrefix()`.
---
 src/core/border_router/routing_manager.cpp | 39 +++++++++++++---------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp
index b0712c50c..6581da52e 100644
--- a/src/core/border_router/routing_manager.cpp
+++ b/src/core/border_router/routing_manager.cpp
@@ -578,11 +578,25 @@ void RoutingManager::EvaluateOnLinkPrefix(void)
             LogInfo("Start advertising on-link prefix %s on %s", mLocalOnLinkPrefix.ToString().AsCString(),
                     mInfraIf.ToString().AsCString());
 
-            // Call `InvalidateDiscoveredPrefixes()` after setting
-            // `mIsAdvertisingLocalOnLinkPrefix` to remove on-link
-            // prefix in case it was discovered and included in
-            // `mDiscoveredPrefixes` list earlier.
-            InvalidateDiscoveredPrefixes();
+            // We go through `mDiscoveredPrefixes` list to check if the
+            // local on-link prefix was previously discovered and
+            // included in the list and if so we remove it from list.
+            //
+            // Note that `UpdateDiscoveredOnLinkPrefix()` will also
+            // check and not add local on-link prefix in the discovered
+            // prefix list while we are advertising the local on-link
+            // prefix.
+
+            for (ExternalPrefix &prefix : mDiscoveredPrefixes)
+            {
+                if (prefix.IsOnLinkPrefix() && mLocalOnLinkPrefix == prefix.GetPrefix())
+                {
+                    // To remove the prefix from the list, we copy the
+                    // popped last entry into `prefix` entry.
+                    prefix = *mDiscoveredPrefixes.PopBack();
+                    break;
+                }
+            }
         }
 
         mOnLinkPrefixDeprecateTimer.Stop();
@@ -1297,23 +1311,16 @@ void RoutingManager::InvalidateDiscoveredPrefixes(void)
     for (ExternalPrefixArray::IndexType index = 0; index < mDiscoveredPrefixes.GetLength();)
     {
         ExternalPrefix &prefix = mDiscoveredPrefixes[index];
-        bool            isAdvertisedLocalOnLinkPrefix =
-            mIsAdvertisingLocalOnLinkPrefix && prefix.IsOnLinkPrefix() && mLocalOnLinkPrefix == prefix.GetPrefix();
 
         // We invalidate expired prefixes, or local OMR prefixes
         // (either in `mAdvertisedOmrPrefixes` or in Thread Network
-        // Data), or if the prefix matches the local on-link prefix
-        // (when local on-link prefix is being advertised).
+        // Data).
 
         if ((prefix.GetExpireTime() <= now) ||
-            (!prefix.IsOnLinkPrefix() && (mAdvertisedOmrPrefixes.Contains(prefix.GetPrefix()) ||
-                                          NetworkDataContainsOmrPrefix(prefix.GetPrefix()))) ||
-            isAdvertisedLocalOnLinkPrefix)
+            (!prefix.IsOnLinkPrefix() &&
+             (mAdvertisedOmrPrefixes.Contains(prefix.GetPrefix()) || NetworkDataContainsOmrPrefix(prefix.GetPrefix()))))
         {
-            if (!isAdvertisedLocalOnLinkPrefix)
-            {
-                UnpublishExternalRoute(prefix.GetPrefix());
-            }
+            UnpublishExternalRoute(prefix.GetPrefix());
 
             // Remove the prefix from the array by replacing it with
             // last entry in the array (we copy the popped last entry