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));