From 26b7416045287b4c4866d3969e8a07e4fbe1f2e2 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 3 Jun 2024 10:18:36 -0700 Subject: [PATCH] [routing-manager] fix and enhance prefix stale time calculation (#10317) This commit updates the determination of stale times for discovered on-link or route prefixes. The stale time is now calculated per unique prefix. If multiple routers advertise the same on-link or route prefix, the stale time for the prefix is set to the latest among all corresponding entries. This addresses an issue in the previous implementation where, for on-link prefixes, the stale time was determined as the latest stale time over all on-link entries, regardless of the actual prefixes. For route prefixes, the earliest stale time was used, also disregarding the possibility of multiple routers advertising the same prefix. This commit also updates the `test_routing_manager` unit test to validate the corrected stale time calculation. --- src/core/border_router/routing_manager.cpp | 103 +++++++++++++--- src/core/border_router/routing_manager.hpp | 6 + tests/unit/test_routing_manager.cpp | 137 +++++++++++++++++++++ 3 files changed, 231 insertions(+), 15 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 2afa73cac..312067fcb 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -1492,36 +1492,50 @@ void RoutingManager::RxRaTracker::RemoveOrDeprecateEntriesFromInactiveRouters(vo void RoutingManager::RxRaTracker::ScheduleStaleTimer(void) { - NextFireTime staleTime; - TimeMilli onLinkStaleTime = staleTime.GetNow(); - bool foundOnLink = false; + // If multiple routers advertise the same on-link or route prefix, + // the stale time for the prefix is determined by the latest stale + // time among all corresponding entries. + // + // The "StaleTimeCalculated" flag is used to ensure stale time is + // calculated only once for each unique prefix. Initially, this + // flag is cleared on all entries. As we iterate over routers and + // their entries, `DetermineStaleTimeFor()` will consider all + // matching entries and mark "StaleTimeCalculated" flag on them. - // For on-link prefixes, we consider stale time as when all on-link - // prefixes become stale (the latest stale time) but for route - // prefixes we consider the earliest stale time. + NextFireTime staleTime; + + for (Router &router : mRouters) + { + for (OnLinkPrefix &entry : router.mOnLinkPrefixes) + { + entry.SetStaleTimeCalculated(false); + } + + for (RoutePrefix &entry : router.mRoutePrefixes) + { + entry.SetStaleTimeCalculated(false); + } + } for (const Router &router : mRouters) { for (const OnLinkPrefix &entry : router.mOnLinkPrefixes) { - if (!entry.IsDeprecated()) + if (!entry.IsStaleTimeCalculated()) { - onLinkStaleTime = Max(onLinkStaleTime, Max(staleTime.GetNow(), entry.GetStaleTime())); - foundOnLink = true; + DetermineStaleTimeFor(entry, staleTime); } } for (const RoutePrefix &entry : router.mRoutePrefixes) { - staleTime.UpdateIfEarlier(entry.GetStaleTime()); + if (!entry.IsStaleTimeCalculated()) + { + DetermineStaleTimeFor(entry, staleTime); + } } } - if (foundOnLink) - { - staleTime.UpdateIfEarlier(onLinkStaleTime); - } - if (mLocalRaHeader.IsValid()) { uint16_t interval = kRtrAdvStaleTime; @@ -1537,6 +1551,65 @@ void RoutingManager::RxRaTracker::ScheduleStaleTimer(void) mStaleTimer.FireAt(staleTime); } +void RoutingManager::RxRaTracker::DetermineStaleTimeFor(const OnLinkPrefix &aPrefix, NextFireTime &aStaleTime) +{ + TimeMilli prefixStaleTime = aStaleTime.GetNow(); + bool found = false; + + for (Router &router : mRouters) + { + for (OnLinkPrefix &entry : router.mOnLinkPrefixes) + { + if (!entry.Matches(aPrefix.GetPrefix())) + { + continue; + } + + entry.SetStaleTimeCalculated(true); + + if (entry.IsDeprecated()) + { + continue; + } + + prefixStaleTime = Max(prefixStaleTime, Max(aStaleTime.GetNow(), entry.GetStaleTime())); + found = true; + } + } + + if (found) + { + aStaleTime.UpdateIfEarlier(prefixStaleTime); + } +} + +void RoutingManager::RxRaTracker::DetermineStaleTimeFor(const RoutePrefix &aPrefix, NextFireTime &aStaleTime) +{ + TimeMilli prefixStaleTime = aStaleTime.GetNow(); + bool found = false; + + for (Router &router : mRouters) + { + for (RoutePrefix &entry : router.mRoutePrefixes) + { + if (!entry.Matches(aPrefix.GetPrefix())) + { + continue; + } + + entry.SetStaleTimeCalculated(true); + + prefixStaleTime = Max(prefixStaleTime, Max(aStaleTime.GetNow(), entry.GetStaleTime())); + found = true; + } + } + + if (found) + { + aStaleTime.UpdateIfEarlier(prefixStaleTime); + } +} + void RoutingManager::RxRaTracker::HandleStaleTimer(void) { VerifyOrExit(Get().IsRunning()); diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index 23959753e..4dcd1c223 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -675,12 +675,16 @@ private: bool Matches(const UlaChecker &aIsUla) const { return (mPrefix.IsUniqueLocal() == aIsUla); } bool Matches(const ExpirationChecker &aChecker) const { return (GetExpireTime() <= aChecker.mNow); } + void SetStaleTimeCalculated(bool aFlag) { mStaleTimeCalculated = aFlag; } + bool IsStaleTimeCalculated(void) const { return mStaleTimeCalculated; } + protected: LifetimedPrefix(void) = default; TimeMilli CalculateExpirationTime(uint32_t aLifetime) const; Ip6::Prefix mPrefix; + bool mStaleTimeCalculated : 1; uint32_t mValidLifetime; TimeMilli mLastUpdateTime; }; @@ -910,6 +914,8 @@ private: void RemoveExpiredEntries(void); void SignalTableChanged(void); void ScheduleStaleTimer(void); + void DetermineStaleTimeFor(const OnLinkPrefix &aPrefix, NextFireTime &aStaleTime); + void DetermineStaleTimeFor(const RoutePrefix &aPrefix, NextFireTime &aStaleTime); void UpdateRouterOnRx(Router &aRouter); void SendNeighborSolicitToRouter(const Router &aRouter); #if OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp index cf80ca3b3..c2bafc363 100644 --- a/tests/unit/test_routing_manager.cpp +++ b/tests/unit/test_routing_manager.cpp @@ -2648,6 +2648,142 @@ void TestExtPanIdChange(void) FinalizeTest(); } +void TestPrefixStaleTime(void) +{ + Ip6::Prefix localOnLink; + Ip6::Prefix localOmr; + Ip6::Prefix onLinkPrefixA = PrefixFromString("2000:abba:baba:aaaa::", 64); + Ip6::Prefix onLinkPrefixB = PrefixFromString("2000:abba:baba:bbbb::", 64); + Ip6::Prefix routePrefix = PrefixFromString("2000:1234:5678::", 64); + Ip6::Address routerAddressA = AddressFromString("fd00::aaaa"); + Ip6::Address routerAddressB = AddressFromString("fd00::bbbb"); + uint16_t heapAllocations; + + Log("--------------------------------------------------------------------------------------------"); + Log("TestPrefixStaleTime"); + + InitTest(); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Start Routing Manager. Check emitted RS and RA messages. + + sRsEmitted = false; + sRaValidated = false; + sExpectedPio = kPioAdvertisingLocalOnLink; + sExpectedRios.Clear(); + + heapAllocations = sHeapAllocatedPtrs.GetLength(); + SuccessOrQuit(sInstance->Get().SetEnabled(true)); + + SuccessOrQuit(sInstance->Get().GetOnLinkPrefix(localOnLink)); + SuccessOrQuit(sInstance->Get().GetOmrPrefix(localOmr)); + + Log("Local on-link prefix is %s", localOnLink.ToString().AsCString()); + Log("Local OMR prefix is %s", localOmr.ToString().AsCString()); + + sExpectedRios.Add(localOmr); + + AdvanceTime(30000); + + VerifyOrQuit(sRsEmitted); + VerifyOrQuit(sRaValidated); + VerifyOrQuit(sExpectedRios.SawAll()); + Log("Received RA was validated"); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Advertise a route prefix with 200 seconds lifetime from router A. + // Advertise the same route prefix with 500 seconds lifetime from + // router B. + + SendRouterAdvert(routerAddressA, {Rio(routePrefix, 200, NetworkData::kRoutePreferenceMedium)}); + SendRouterAdvert(routerAddressB, {Rio(routePrefix, 500, NetworkData::kRoutePreferenceMedium)}); + + AdvanceTime(10); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Check the discovered prefix table and ensure info from router A and B + // is present in the table. + + VerifyPrefixTable({RoutePrefix(routePrefix, 200, NetworkData::kRoutePreferenceMedium, routerAddressA), + RoutePrefix(routePrefix, 500, NetworkData::kRoutePreferenceMedium, routerAddressB)}); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Wait for a period exceeding the 200-second lifetime of the route + // advertised by router A. Confirm that the stale timer does not expire + // during this time, and no RS messages sent. This verifies that the + // presence of the matching entry from router B successfully extended + // the stale time for the route prefix. + + sRsEmitted = false; + + AdvanceTime(490 * 1000); + + VerifyOrQuit(!sRsEmitted); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Check the discovered prefix table and ensure router A entry is + // expired and removed. + + VerifyPrefixTable({RoutePrefix(routePrefix, 500, NetworkData::kRoutePreferenceMedium, routerAddressB)}); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Wait longer than 500-second lifetime of prefix advertised by + // router B. Now we should see RS messages emitted. + + AdvanceTime(20 * 1000); + + VerifyOrQuit(sRsEmitted); + + VerifyPrefixTableIsEmpty(); + + AdvanceTime(5 * 000); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Advertise the same on-link prefix A with different lifetimes from routers A and B. + // Advertise a different on-link prefix from router A. + + SendRouterAdvert(routerAddressA, {Pio(onLinkPrefixA, 1800, 200), Pio(onLinkPrefixB, 2000, 2000)}); + SendRouterAdvert(routerAddressB, {Pio(onLinkPrefixA, 1800, 500)}); + + AdvanceTime(10); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Check the discovered prefix table and ensure info from router A and B + // is present in the table. + + VerifyPrefixTable({OnLinkPrefix(onLinkPrefixA, 1800, 200, routerAddressA), + OnLinkPrefix(onLinkPrefixB, 2000, 2000, routerAddressA), + OnLinkPrefix(onLinkPrefixA, 1800, 500, routerAddressB)}); + + sRsEmitted = false; + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Wait for a period exceeding the 200-second lifetime of the on-link prefix. + // Confirm stale timer is not expired and no RS is emitted. + + sRsEmitted = false; + + AdvanceTime(490 * 1000); + + VerifyOrQuit(!sRsEmitted); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Wait for a 500-second lifetime for prefix advertised by router B. Now + // we should see RS messages emitted. + + AdvanceTime(20 * 1000); + + VerifyOrQuit(sRsEmitted); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + SuccessOrQuit(sInstance->Get().SetEnabled(false)); + VerifyOrQuit(heapAllocations == sHeapAllocatedPtrs.GetLength()); + + Log("End of TestPrefixStaleTime"); + FinalizeTest(); +} + void TestRouterNsProbe(void) { Ip6::Prefix localOnLink; @@ -4055,6 +4191,7 @@ int main(void) #endif ot::TestExtPanIdChange(); ot::TestConflictingPrefix(); + ot::TestPrefixStaleTime(); ot::TestRouterNsProbe(); ot::TestLearningAndCopyingOfFlags(); ot::TestLearnRaHeader();