From 4f692464d521a73ece4cd0bfe42f86dcd546f532 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 20 May 2025 11:53:55 -0700 Subject: [PATCH] [trel] implement delayed peer removal in `PeerTable` (#11511) This commit updates `Trel::Peer` and `PeerTable` to allow scheduling a `Peer` for removal from the table after a given delay. This enables smoother peer removal, especially for transient issues, by avoiding abrupt disconnections. To implement this, a `State` has been added to the `Peer` class, tracking whether a peer is in `kStateValid` or `kStateRemoving`. If a peer scheduled for removal is discovered again, it will be re-added and marked as valid. Logging is also updated to show these new state transitions. Additionally, `EvictPeer()` is updated to prioritize evicting peers already scheduled for removal. To ensure consistent API behavior, peers that are scheduled for removal are skipped when iterating over the peer table using `otTrelGetNextPeer()` or `otTrelGetNumberOfPeers()`. --- src/core/radio/trel_peer.cpp | 74 ++++++++++++++++++++++--- src/core/radio/trel_peer.hpp | 63 ++++++++++++--------- src/core/radio/trel_peer_discoverer.cpp | 25 ++++++--- src/core/radio/trel_peer_discoverer.hpp | 2 + 4 files changed, 123 insertions(+), 41 deletions(-) diff --git a/src/core/radio/trel_peer.cpp b/src/core/radio/trel_peer.cpp index 250c276e0..6a56759c5 100644 --- a/src/core/radio/trel_peer.cpp +++ b/src/core/radio/trel_peer.cpp @@ -51,11 +51,13 @@ void Peer::Init(Instance &aInstance) AsCoreType(&mExtAddress).Clear(); AsCoreType(&mExtPanId).Clear(); AsCoreType(&mSockAddr).Clear(); + + mState = kStateValid; } void Peer::Free(void) { - Log(kRemoving); + Log(kDeleted); #if OPENTHREAD_CONFIG_TREL_USE_HEAP_ENABLE Heap::Allocatable::Free(); @@ -65,6 +67,22 @@ void Peer::Free(void) #endif } +void Peer::ScheduleToRemoveAfter(uint32_t aDelay) +{ + VerifyOrExit(IsStateValid()); + + mRemoveTime = TimerMilli::GetNow() + aDelay; + SetState(kStateRemoving); + + Get().mTimer.FireAtIfEarlier(mRemoveTime); + + Log(kRemoving); + LogInfo(" after %u msec", aDelay); + +exit: + return; +} + bool Peer::Matches(const NonNeighborMatcher &aMatcher) const { // Matches only if the peer is not a neighbor. This is used when @@ -96,17 +114,21 @@ const char *Peer::ActionToString(Action aAction) { static const char *const kActionStrings[] = { "Added", // (0) kAdded - "Updated", // (1) kUpdated - "Removing", // (2) kRemoving - "Evicting", // (3) kEvicting + "Re-added", // (1) kReAdded, + "Updated", // (2) kUpdated + "Removing", // (3) kRemoving + "Deleted", // (4) kDeleted + "Evicting", // (5) kEvicting }; struct EnumCheck { InitEnumValidatorCounter(); ValidateNextEnum(kAdded); + ValidateNextEnum(kReAdded); ValidateNextEnum(kUpdated); ValidateNextEnum(kRemoving); + ValidateNextEnum(kDeleted); ValidateNextEnum(kEvicting); }; @@ -120,6 +142,7 @@ const char *Peer::ActionToString(Action aAction) PeerTable::PeerTable(Instance &aInstance) : InstanceLocator(aInstance) + , mTimer(aInstance) { } @@ -164,10 +187,16 @@ Error PeerTable::EvictPeer(void) Error error = kErrorNotFound; OwnedPtr peerToEvict; - // We first try to evict a peer belonging to a different PAN. - // If not found, we evict a non-neighbor peer. + // We first try to evict a peer already scheduled to be removed. + // Then try to evict a peer belonging to a different PAN. If not + // found, we evict a non-neighbor peer. - peerToEvict = RemoveMatching(Peer::OtherExtPanIdMatcher(Get().GetExtPanId())); + peerToEvict = RemoveMatching(Peer::kStateRemoving); + + if (peerToEvict == nullptr) + { + peerToEvict = RemoveMatching(Peer::OtherExtPanIdMatcher(Get().GetExtPanId())); + } if (peerToEvict == nullptr) { @@ -183,11 +212,36 @@ exit: return error; } +void PeerTable::HandleTimer(void) +{ + TimeMilli now = TimerMilli::GetNow(); + NextFireTime nextFireTime(now); + + RemoveAndFreeAllMatching(Peer::ExpireChecker(now)); + + for (const Peer &peer : *this) + { + if (peer.IsStateRemoving()) + { + nextFireTime.UpdateIfEarlier(peer.mRemoveTime); + } + } + + mTimer.FireAtIfEarlier(nextFireTime); +} + const Peer *PeerTable::GetNextPeer(PeerIterator &aIterator) const { const Peer *entry = static_cast(aIterator); VerifyOrExit(entry != nullptr); + + while (!entry->IsStateValid()) + { + entry = entry->GetNext(); + VerifyOrExit(entry != nullptr); + } + aIterator = entry->GetNext(); exit: @@ -200,8 +254,10 @@ uint16_t PeerTable::GetNumberOfPeers(void) const for (const Peer &peer : *this) { - OT_UNUSED_VARIABLE(peer); - count++; + if (peer.IsStateValid()) + { + count++; + } } return count; diff --git a/src/core/radio/trel_peer.hpp b/src/core/radio/trel_peer.hpp index 58f24ccdc..16a63d40a 100644 --- a/src/core/radio/trel_peer.hpp +++ b/src/core/radio/trel_peer.hpp @@ -48,6 +48,7 @@ #include "common/non_copyable.hpp" #include "common/owning_list.hpp" #include "common/pool.hpp" +#include "common/timer.hpp" #include "mac/mac_types.hpp" #include "meshcop/extended_panid.hpp" #include "net/socket.hpp" @@ -110,33 +111,21 @@ public: */ void SetSockAddr(const Ip6::SockAddr &aSockAddr) { mSockAddr = aSockAddr; } - /** - * Indicates whether the peer matches a given Extended Address. - * - * @param[in] aExtAddress A Extended Address to match with. - * - * @retval TRUE if the peer matches @p aExtAddress. - * @retval FALSE if the peer does not match @p aExtAddress. - */ - bool Matches(const Mac::ExtAddress &aExtAddress) const { return GetExtAddress() == aExtAddress; } - - /** - * Indicates whether the peer matches a given Socket Address. - * - * @param[in] aSockAddr A Socket Address to match with. - * - * @retval TRUE if the peer matches @p aSockAddr. - * @retval FALSE if the peer does not match @p aSockAddr. - */ - bool Matches(const Ip6::SockAddr &aSockAddr) const { return GetSockAddr() == aSockAddr; } - private: + enum State : uint8_t + { + kStateValid, + kStateRemoving, + }; + enum Action : uint8_t { - kAdded, - kUpdated, - kRemoving, - kEvicting, + kAdded, // Added a new peer. + kReAdded, // Re-added a peer (discovered again) that was scheduled for removal. + kUpdated, // Updated an existing peer. + kRemoving, // Scheduling a peer to be removed after delay. + kDeleted, // Fully removing and deleting the peer from the table. + kEvicting, // Evicting the peer to make space for new one. }; struct OtherExtPanIdMatcher // Matches if Ext PAN ID is different. @@ -159,12 +148,30 @@ private: NeighborTable &mNeighborTable; }; + struct ExpireChecker // Matches if the peer is in `kStateRemoving` and already expired. + { + explicit ExpireChecker(TimeMilli aNow) + : mNow(aNow) + { + } + + TimeMilli mNow; + }; + void Init(Instance &aInstance); void Free(void); + void SetState(State aState) { mState = aState; } + bool IsStateValid(void) const { return mState == kStateValid; } + bool IsStateRemoving(void) const { return mState == kStateRemoving; } void SetExtAddress(const Mac::ExtAddress &aExtAddress) { mExtAddress = aExtAddress; } void SetExtPanId(const MeshCoP::ExtendedPanId &aExtPanId) { mExtPanId = aExtPanId; } + void ScheduleToRemoveAfter(uint32_t aDelay); + bool Matches(const Mac::ExtAddress &aExtAddress) const { return GetExtAddress() == aExtAddress; } + bool Matches(const Ip6::SockAddr &aSockAddr) const { return GetSockAddr() == aSockAddr; } + bool Matches(State aState) const { return mState == aState; } bool Matches(const OtherExtPanIdMatcher &aMatcher) const { return GetExtPanId() != aMatcher.mExtPanId; } bool Matches(const NonNeighborMatcher &aMatcher) const; + bool Matches(const ExpireChecker &aChecker) const { return IsStateRemoving() && (aChecker.mNow >= mRemoveTime); } #if OT_SHOULD_LOG_AT(OT_LOG_LEVEL_INFO) void Log(Action aAction) const; @@ -173,7 +180,9 @@ private: void Log(Action) const {} #endif - Peer *mNext; + Peer *mNext; + State mState; + TimeMilli mRemoveTime; }; //--------------------------------------------------------------------------------------------------------------------- @@ -248,7 +257,11 @@ private: Peer *AllocatePeer(void); Error EvictPeer(void); + void HandleTimer(void); + using PeerTimer = TimerMilliIn; + + PeerTimer mTimer; #if !OPENTHREAD_CONFIG_TREL_USE_HEAP_ENABLE Pool mPool; #endif diff --git a/src/core/radio/trel_peer_discoverer.cpp b/src/core/radio/trel_peer_discoverer.cpp index e9c8e73bf..15b9c466f 100644 --- a/src/core/radio/trel_peer_discoverer.cpp +++ b/src/core/radio/trel_peer_discoverer.cpp @@ -118,7 +118,8 @@ void PeerDiscoverer::HandleDiscoveredPeerInfo(const PeerInfo &aInfo) Peer *peer; TxtData txtData; TxtData::Info txtInfo; - bool isNew = false; + Peer::Action action = Peer::kUpdated; + bool shouldLog = true; VerifyOrExit(mIsRunning); @@ -129,7 +130,9 @@ void PeerDiscoverer::HandleDiscoveredPeerInfo(const PeerInfo &aInfo) if (aInfo.IsRemoved()) { - Get().RemoveAndFreeAllMatching(txtInfo.mExtAddress); + peer = Get().FindMatching(txtInfo.mExtAddress); + VerifyOrExit(peer != nullptr); + peer->ScheduleToRemoveAfter(kRemoveDelay); ExitNow(); } @@ -158,18 +161,26 @@ void PeerDiscoverer::HandleDiscoveredPeerInfo(const PeerInfo &aInfo) VerifyOrExit(peer != nullptr); peer->SetExtAddress(txtInfo.mExtAddress); - isNew = true; + action = Peer::kAdded; } - - if (!isNew) + else if (!peer->IsStateValid()) { - VerifyOrExit((peer->GetExtPanId() != txtInfo.mExtPanId) || (peer->GetSockAddr() != aInfo.GetSockAddr())); + action = Peer::kReAdded; + } + else + { + shouldLog = (peer->GetExtPanId() != txtInfo.mExtPanId) || (peer->GetSockAddr() != aInfo.GetSockAddr()); + action = Peer::kUpdated; } + peer->SetState(Peer::kStateValid); peer->SetExtPanId(txtInfo.mExtPanId); peer->SetSockAddr(aInfo.GetSockAddr()); - peer->Log(isNew ? Peer::kAdded : Peer::kUpdated); + if (shouldLog) + { + peer->Log(action); + } exit: return; diff --git a/src/core/radio/trel_peer_discoverer.hpp b/src/core/radio/trel_peer_discoverer.hpp index 79641133a..9b94ee4c8 100644 --- a/src/core/radio/trel_peer_discoverer.hpp +++ b/src/core/radio/trel_peer_discoverer.hpp @@ -91,6 +91,8 @@ public: void NotifyPeerSocketAddressDifference(const Ip6::SockAddr &aPeerSockAddr, const Ip6::SockAddr &aRxSockAddr); private: + static constexpr uint32_t kRemoveDelay = 7 * Time::kOneSecondInMsec; + class TxtData { public: