From dd0d8311f83ebdde7544e57d82ffab6eb75509ae Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 4 Feb 2026 12:55:55 -0800 Subject: [PATCH] [border-router] detect DHCPv6-PD prefix conflict with route prefixes (#12361) This commit enhances the DHCPv6 PD prefix conflict detection logic to check against Route Information Options (RIOs) present in received Router Advertisements, in addition to the existing check against on-link prefixes (PIOs). The conflict detection behavior is event-driven to correctly handle network propagation delays and valid advertisements: 1. On new prefix assignment (`kPdPrefixChanged`): A strict check is performed. If the prefix matches any existing RIO from another router, it is flagged as a conflict. 2. On RA table updates (`kRxRaPrefixTableChanged`): The check focuses on conflict resolution. Crucially, it ignores new RIO matches appearing after the prefix has been adopted. This is necessary because once the BR publishes the PD prefix in Thread Network Data as the OMR prefix, other BRs will naturally start advertising it as a RIO to announce reachability. The unit test `TestDhcp6PdConflict` is updated to verify both the detection of conflict, its resolution, and that the subsequent RIO advertisements do not cause a conflict after PD prefix is published as OMR. --- src/core/border_router/routing_manager.cpp | 83 ++++++++++++--- src/core/border_router/routing_manager.hpp | 16 ++- src/core/border_router/rx_ra_tracker.cpp | 24 ++++- src/core/border_router/rx_ra_tracker.hpp | 10 ++ tests/unit/test_routing_manager.cpp | 113 +++++++++++++++++++++ 5 files changed, 224 insertions(+), 22 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 094e5ef82..5edb0b808 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -865,8 +865,7 @@ void RoutingManager::OmrPrefixManager::UpdateLocalPrefix(void) { case kOmrConfigAuto: #if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE - if (Get().mPdPrefixManager.HasPrefix() && - !Get().mPdPrefixManager.HasConflictWithOnLinkPrefixes()) + if (Get().mPdPrefixManager.HasPrefix() && !Get().mPdPrefixManager.HasConflict()) { if (mLocalPrefix.GetPrefix() != Get().mPdPrefixManager.GetPrefix()) { @@ -2506,7 +2505,8 @@ void RoutingManager::TxRaInfo::CalculateHash(const RouterAdvert::RxMessage &aRaM RoutingManager::PdPrefixManager::PdPrefixManager(Instance &aInstance) : InstanceLocator(aInstance) , mState(kDhcp6PdStateDisabled) - , mConflicted(false) + , mOnLinkPrefixConflict(false) + , mRoutePrefixConflict(false) , mNumPlatformPioProcessed(0) , mNumPlatformRaReceived(0) , mLastPlatformRaTime(0) @@ -2639,7 +2639,8 @@ void RoutingManager::PdPrefixManager::WithdrawPrefix(void) LogInfo("Withdrew DHCPv6 PD prefix %s", mPrefix.GetPrefix().ToString().AsCString()); mPrefix.Clear(); - mConflicted = false; + mOnLinkPrefixConflict = false; + mRoutePrefixConflict = false; mTimer.Stop(); @@ -2734,7 +2735,7 @@ void RoutingManager::PdPrefixManager::ApplyFavoredPrefix(const PdPrefix &aFavore mPrefix = aFavoredPrefix; LogInfo("DHCPv6 PD prefix set to %s", mPrefix.GetPrefix().ToString().AsCString()); - CheckConflictWithOnLinkPrefixes(); + CheckConflict(kPdPrefixChanged); Get().ScheduleRoutingPolicyEvaluation(kImmediately); @@ -2791,10 +2792,10 @@ exit: return; } -void RoutingManager::PdPrefixManager::CheckConflictWithOnLinkPrefixes(void) +void RoutingManager::PdPrefixManager::CheckConflict(ConflictCheckEvent aEvent) { - // Checks if the delegated PD prefix is also seen as an on-link - // prefix. This protects against DHCPv6-PD server misbehavior + // Checks if the delegated PD prefix is also seen as an on-link or + // route prefix. This protects against DHCPv6-PD server misbehavior // assigning the same prefix to multiple requesters. // // If a conflict is detected, the delegated PD prefix is no longer @@ -2802,19 +2803,71 @@ void RoutingManager::PdPrefixManager::CheckConflictWithOnLinkPrefixes(void) // prefix. Once the conflict is resolved, the PD prefix can be // used as OMR prefix again. - bool conflicted; + bool hadConflict; VerifyOrExit(HasPrefix()); - conflicted = Get().IsPrefixOnLink(mPrefix.GetPrefix()); - VerifyOrExit(conflicted != mConflicted); + hadConflict = HasConflict(); - mConflicted = conflicted; + CheckConflictWithOnLinkPrefixes(); + CheckConflictWithRoutePrefixes(aEvent); - LogInfo("DHCPv6 PD prefix %s %sconflicts with the advertised on-link prefixes", - mPrefix.GetPrefix().ToString().AsCString(), mConflicted ? "" : "no longer "); + if (hadConflict != HasConflict()) + { + Get().ScheduleRoutingPolicyEvaluation(kImmediately); + } - Get().ScheduleRoutingPolicyEvaluation(kImmediately); +exit: + return; +} + +void RoutingManager::PdPrefixManager::CheckConflictWithOnLinkPrefixes(void) +{ + UpdateConflictFlag(mOnLinkPrefixConflict, Get().IsPrefixOnLink(mPrefix.GetPrefix()), "on-link"); +} + +void RoutingManager::PdPrefixManager::CheckConflictWithRoutePrefixes(ConflictCheckEvent aEvent) +{ + // Conflict detection for route prefixes depends on the triggering + // event and the current state. + // + // If a new PD prefix is assigned (`kPdPrefixChanged`), any matching + // route prefix (RIO) is flagged as a conflict. + // + // If `RxRaTracker` is changed (`kRxRaPrefixTableChanged`), we check + // only for conflict resolution: + // - If conflicted, we check if the interfering RIO is removed. + // - We ignore any new RIO route matches. Once the PD prefix is + // adopted, it is published in Thread Network Data as the OMR + // prefix. Other BRs connected to the same mesh will see this and + // advertise it as a RIO in their emitted RAs to announce + // reachability to the Thread mesh. We must not treat these + // expected advertisements as conflicts. + + switch (aEvent) + { + case kPdPrefixChanged: + break; + case kRxRaPrefixTableChanged: + VerifyOrExit(mRoutePrefixConflict); + break; + } + + UpdateConflictFlag(mRoutePrefixConflict, Get().ContainsRoutePrefix(mPrefix.GetPrefix()), "route"); + +exit: + return; +} + +void RoutingManager::PdPrefixManager::UpdateConflictFlag(bool &aConflictFlag, bool aNewFlag, const char *aPrefixType) +{ + OT_UNUSED_VARIABLE(aPrefixType); + + VerifyOrExit(aConflictFlag != aNewFlag); + aConflictFlag = aNewFlag; + + LogInfo("DHCPv6 PD prefix %s %sconflicts with the advertised %s prefixes", + mPrefix.GetPrefix().ToString().AsCString(), aNewFlag ? "" : "no longer ", aPrefixType); exit: return; diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index e6a1a66d0..fb437077f 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -955,16 +955,22 @@ private: static constexpr RoutePreference kPdRoutePreference = RoutePreference::kRoutePreferenceMedium; + enum ConflictCheckEvent : uint8_t + { + kPdPrefixChanged, + kRxRaPrefixTableChanged, + }; + explicit PdPrefixManager(Instance &aInstance); void SetEnabled(bool aEnabled); void Start(void) { Evaluate(); } void Stop(void) { Evaluate(); } bool HasPrefix(void) const { return !mPrefix.IsEmpty(); } - bool HasConflictWithOnLinkPrefixes(void) const { return mConflicted; } + bool HasConflict(void) const { return mOnLinkPrefixConflict || mRoutePrefixConflict; } const Ip6::Prefix &GetPrefix(void) const { return mPrefix.GetPrefix(); } State GetState(void) const { return mState; } - void CheckConflictWithOnLinkPrefixes(void); + void CheckConflict(ConflictCheckEvent aEvent); void ProcessPrefixesFromRa(const InfraIf::Icmp6Packet &aRaPacket); void ProcessPrefix(const Dhcp6PdPrefix &aPrefix); @@ -992,6 +998,9 @@ private: void EvaluateCandidatePrefix(PdPrefix &aPrefix, PdPrefix &aFavoredPrefix); void ApplyFavoredPrefix(const PdPrefix &aFavoredPrefix); void WithdrawPrefix(void); + void CheckConflictWithOnLinkPrefixes(void); + void CheckConflictWithRoutePrefixes(ConflictCheckEvent aEvent); + void UpdateConflictFlag(bool &aConflictFlag, bool aNewFlag, const char *aPrefixType); static const char *StateToString(State aState); @@ -1002,7 +1011,8 @@ private: #endif State mState; - bool mConflicted; + bool mOnLinkPrefixConflict; + bool mRoutePrefixConflict; uint32_t mNumPlatformPioProcessed; uint32_t mNumPlatformRaReceived; TimeMilli mLastPlatformRaTime; diff --git a/src/core/border_router/rx_ra_tracker.cpp b/src/core/border_router/rx_ra_tracker.cpp index dd0291ffe..673297a9d 100644 --- a/src/core/border_router/rx_ra_tracker.cpp +++ b/src/core/border_router/rx_ra_tracker.cpp @@ -859,11 +859,11 @@ void RxRaTracker::Evaluate(void) #if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // Check for possible conflict between delegated DHCPv6-PD prefix - // and any of the observed on-link prefixes. This protects against - // DHCPv6-PD server misbehavior (assigning same prefix to multiple - // requesters). + // and any of the observed on-link or route prefixes. This protects + // against DHCPv6-PD server misbehavior (assigning same prefix to + // multiple requesters). - Get().mPdPrefixManager.CheckConflictWithOnLinkPrefixes(); + Get().mPdPrefixManager.CheckConflict(RoutingManager::PdPrefixManager::kRxRaPrefixTableChanged); #endif //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1206,6 +1206,22 @@ exit: return isOnLink; } +bool RxRaTracker::ContainsRoutePrefix(const Ip6::Prefix &aPrefix) const +{ + bool contains = false; + + for (const Router &router : mRouters) + { + if (router.mRoutePrefixes.ContainsMatching(aPrefix)) + { + contains = true; + break; + } + } + + return contains; +} + bool RxRaTracker::IsAddressReachableThroughExplicitRoute(const Ip6::Address &aAddress) const { // Checks whether the `aAddress` matches any discovered route diff --git a/src/core/border_router/rx_ra_tracker.hpp b/src/core/border_router/rx_ra_tracker.hpp index 85b049e9d..c641d6548 100644 --- a/src/core/border_router/rx_ra_tracker.hpp +++ b/src/core/border_router/rx_ra_tracker.hpp @@ -317,6 +317,16 @@ public: */ bool IsPrefixOnLink(const Ip6::Prefix &aPrefix) const; + /** + * Indicates whether a given prefix is seen as a route prefix advertised by any router. + * + * @param[in] aPrefix The IPv6 prefix to check. + * + * @retval TRUE The prefix is seen as a route prefix. + * @retval FALSE The prefix is not seen as a route prefix. + */ + bool ContainsRoutePrefix(const Ip6::Prefix &aPrefix) const; + // Callbacks notifying of changes void HandleLocalOnLinkPrefixChanged(void); diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp index adec1ecd9..5a825968a 100644 --- a/tests/unit/test_routing_manager.cpp +++ b/tests/unit/test_routing_manager.cpp @@ -5209,6 +5209,119 @@ void TestDhcp6PdConflict(void) VerifyOmrPrefixInNetData(localOmr, /* aDefaultRoute */ true); + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Wait for the PIO from Router A to expire. We renew the PD + // prefix during this time to ensure it stays valid. + + Log("Wait for Router A PIO to expire"); + + AdvanceTime(200 * 1000); + ReportPdPrefixesAsRa({Pio(pdPrefix, kValidLitime, kPreferredLifetime)}); + AdvanceTime(200 * 1000); + + // Router A entry should be expired and removed. The PD prefix is still + // valid (renewed). The conflict should be resolved. Validate that PD + // prefix is again being use as OMR prefix. + + AdvanceTime(100 * 1000); + + VerifyPdOmrPrefix(pdPrefix); + VerifyOmrPrefixInNetData(pdPrefix, /* aDefaultRoute */ false); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Remove the PD prefix + + ReportPdPrefixesAsRa({Pio(pdPrefix, 0, 0)}); + + AdvanceTime(1 * 1000); + VerifyNoPdOmrPrefix(); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Now Advertise the PD prefix as RIO from a router first. + + Log("Router A advertises PD prefix as RIO before delegating the prefix"); + SendRouterAdvert(routerAddressA, {Rio(pdPrefix, kValidLitime, NetworkData::kRoutePreferenceMedium)}); + + // Check that local OMR is used. + + sExpectedRios.Clear(); + sExpectedRios.Add(localOmr); + + AdvanceTime(10 * 1000); + VerifyOrQuit(sExpectedRios.SawAll()); + + VerifyOmrPrefixInNetData(localOmr, /* aDefaultRoute */ false); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Report the same PD prefix. Validate that local OMR prefix is + // still being used (due to RIO conflict). + + Log("Delegate PD prefix which conflicts with already advertised RIO prefix from router A"); + ReportPdPrefixesAsRa({Pio(pdPrefix, kValidLitime, kPreferredLifetime)}); + + sExpectedRios.Clear(); + sExpectedRios.Add(localOmr); + + AdvanceTime(30 * 1000); + VerifyOrQuit(sExpectedRios.SawAll()); + + VerifyOmrPrefixInNetData(localOmr, /* aDefaultRoute */ false); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Remove the RIO from Router A. Validate that the PD prefix is + // now accepted and used as OMR. + + Log("Router A removes RIO, conflict should be resolved"); + SendRouterAdvert(routerAddressA, {Rio(pdPrefix, 0, NetworkData::kRoutePreferenceMedium)}); + + sExpectedRios.Clear(); + sExpectedRios.Add(pdPrefix); + + AdvanceTime(10 * 1000); + VerifyOrQuit(sExpectedRios.SawAll()); + + VerifyOmrPrefixInNetData(pdPrefix, /* aDefaultRoute */ false); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Remove the PD prefix + + ReportPdPrefixesAsRa({Pio(pdPrefix, 0, 0)}); + + AdvanceTime(1 * 1000); + VerifyNoPdOmrPrefix(); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Report the PD prefix again. Validate that it is used as OMR prefix. + + Log("Delegate PD prefix"); + ReportPdPrefixesAsRa({Pio(pdPrefix, kValidLitime, kPreferredLifetime)}); + + sExpectedRios.Clear(); + sExpectedRios.Add(pdPrefix); + + AdvanceTime(10 * 1000); + VerifyOrQuit(sExpectedRios.SawAll()); + + VerifyOmrPrefixInNetData(pdPrefix, /* aDefaultRoute */ false); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // Advertise the same prefix as RIO from a router. This should NOT + // be treated as a conflict since the PD prefix is already adopted + // and used. + + Log("Router A advertises PD prefix as RIO after it is delegated"); + SendRouterAdvert(routerAddressA, {Rio(pdPrefix, kValidLitime, NetworkData::kRoutePreferenceMedium)}); + + AdvanceTime(1 * 1000); + + sExpectedRios.Clear(); + sExpectedRios.Add(pdPrefix); + + AdvanceTime(300 * 1000); + VerifyOrQuit(sExpectedRios.SawAll()); + + VerifyOmrPrefixInNetData(pdPrefix, /* aDefaultRoute */ false); + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - SuccessOrQuit(sInstance->Get().SetEnabled(false));