From b378c704d0abfa4a0ae63df78d39e1727e580269 Mon Sep 17 00:00:00 2001
From: Song GUO <songguo@google.com>
Date: Fri, 11 Oct 2024 12:32:48 +0800
Subject: [PATCH] [routing-manager] fix for updating prefix without deprecating
 the old one (#10811)

In some cases, the platform may send the new prefix without
deprecating the existing prefix when renewing the prefix allocated by
PD.

The existing code will stop the timer, causing the prefix to be active
forever.

This commit fixes this issue.
---
 .github/workflows/unit.yml                 |  2 +-
 src/core/border_router/routing_manager.cpp |  2 +-
 tests/unit/test_routing_manager.cpp        | 51 ++++++++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml
index d7d11c790..bb0e9d826 100644
--- a/.github/workflows/unit.yml
+++ b/.github/workflows/unit.yml
@@ -79,7 +79,7 @@ jobs:
         sudo rm /etc/apt/sources.list.d/* && sudo apt-get update
         sudo apt-get --no-install-recommends install -y ninja-build lcov
     - name: Build Simulation
-      run: ./script/cmake-build simulation -DOT_BUILD_GTEST=ON -DOT_BORDER_ROUTING=ON -DOT_NCP_INFRA_IF=ON
+      run: ./script/cmake-build simulation -DOT_BUILD_GTEST=ON -DOT_BORDER_ROUTING=ON -DOT_BORDER_ROUTING_DHCP6_PD=ON -DOT_NCP_INFRA_IF=ON
     - name: Test Simulation
       run: cd build/simulation && ninja test
     - name: Build Multipan Simulation
diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp
index df7a9beab..3f4d065df 100644
--- a/src/core/border_router/routing_manager.cpp
+++ b/src/core/border_router/routing_manager.cpp
@@ -4068,7 +4068,7 @@ void RoutingManager::PdPrefixManager::Process(const RouterAdvert::Icmp6Packet *a
         Get<RoutingManager>().ScheduleRoutingPolicyEvaluation(kImmediately);
     }
 
-    if (HasPrefix() && currentPrefixUpdated)
+    if (HasPrefix())
     {
         mTimer.FireAt(mPrefix.GetDeprecationTime());
     }
diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp
index 4e65383f5..44fa71115 100644
--- a/tests/unit/test_routing_manager.cpp
+++ b/tests/unit/test_routing_manager.cpp
@@ -4287,6 +4287,57 @@ void TestBorderRoutingProcessPlatfromGeneratedNd(void)
         VerifyOmrPrefixInNetData(localOmr, /* aDefaultRoute */ false);
     }
 
+    // 6. Replace a prefix, on some platforms, there might be no messages to deprecate the old prefix, instead, they
+    // send new prefixes directly.
+    //    In this case, we still use the old prefix as long as the old prefix preferred lifetime is not exceeded, and
+    //    replace it with the new prefix when expired.
+    Log("6. Replace prefix.");
+    {
+        Ip6::Prefix raPrefix    = PrefixFromString("2001:db8:1:2::", 64);
+        Ip6::Prefix newRaPrefix = PrefixFromString("2001:db8:3:4::", 64);
+
+        SendRouterAdvertToBorderRoutingProcessIcmp6Ra({Pio(raPrefix, kValidLitime, kPreferredLifetime)});
+
+        sExpectedRios.Add(raPrefix);
+        sExpectedRios.Clear();
+        AdvanceTime(10 * 1000);
+
+        VerifyPdOmrPrefix(raPrefix);
+        VerifyOmrPrefixInNetData(raPrefix, /* aDefaultRoute */ false);
+
+        AdvanceTime(1000 * 1000);
+        VerifyPdOmrPrefix(raPrefix);
+
+        // Send new prefix without deprecating old prefix.
+        // The old prefix should be preferred for another (1800 - 10 - 1000) = 790s
+        SendRouterAdvertToBorderRoutingProcessIcmp6Ra({Pio(newRaPrefix, kValidLitime, kPreferredLifetime)});
+        AdvanceTime(500 * 1000);
+        VerifyPdOmrPrefix(raPrefix);
+
+        AdvanceTime(300 * 1000);
+        // Old Prefix should be removed now.
+        VerifyNoPdOmrPrefix();
+
+        sExpectedRios.Add(newRaPrefix);
+
+        // When the prefix is replaced, there will be a short period when the old prefix is still in the netdata, and PD
+        // manager will refuse to request the prefix.
+        SendRouterAdvertToBorderRoutingProcessIcmp6Ra({Pio(newRaPrefix, kValidLitime, kPreferredLifetime)});
+        // Advance a short period of time to wait for a stable PD state.
+        AdvanceTime(5000);
+        VerifyOrQuit(sInstance->Get<BorderRouter::RoutingManager>().GetDhcp6PdState() ==
+                     BorderRouter::RoutingManager::kDhcp6PdStateRunning);
+        SendRouterAdvertToBorderRoutingProcessIcmp6Ra({Pio(newRaPrefix, kValidLitime, kPreferredLifetime)});
+
+        AdvanceTime(1000 * 1000);
+        VerifyOrQuit(sExpectedRios.SawAll());
+        VerifyPdOmrPrefix(newRaPrefix);
+
+        AdvanceTime(1000 * 1000);
+        VerifyNoPdOmrPrefix();
+        VerifyOmrPrefixInNetData(localOmr, /* aDefaultRoute */ false);
+    }
+
     SuccessOrQuit(otBorderRoutingSetEnabled(sInstance, false));
     VerifyOrQuit(sHeapAllocatedPtrs.GetLength() <= heapAllocations);