From 06c6e06db94e9ee4d7112564cca3eeffc7124afa Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Mon, 11 Apr 2022 13:09:15 -0700 Subject: [PATCH] [sleepy] Fix #15800 - track idle/active of peer and use dynamic timeout (#17003) * [mrp] Fix #15800 - Add start of peer active tracking * [sleepy] Add switching between active / idle based on peer tracking. * [sleepy] Use constexpr for kMinActiveTime. * [sleepy] Fix review comments + restyle. --- src/messaging/ReliableMessageMgr.cpp | 18 +++++++++--------- src/transport/GroupSession.h | 4 ++++ src/transport/SecureSession.h | 19 +++++++++++++++++-- src/transport/Session.h | 3 +++ src/transport/SessionManager.cpp | 4 ++-- src/transport/UnauthenticatedSessionTable.h | 20 ++++++++++++++++++-- 6 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index 5dcad89fde520e..c0f68a13272536 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -141,12 +141,12 @@ void ReliableMessageMgr::ExecuteActions() "Retransmitting MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange " Send Cnt %d", messageCounter, ChipLogValueExchange(&entry->ec.Get()), entry->sendCount); - // TODO(#15800): Choose active/idle timeout corresponding to the activity of exchanges of the session. - System::Clock::Timestamp backoff = - ReliableMessageMgr::GetBackoff(entry->ec->GetSessionHandle()->GetMRPConfig().mActiveRetransTimeout, entry->sendCount); - entry->nextRetransTime = System::SystemClock().GetMonotonicTimestamp() + backoff; + + // Choose active/idle timeout from PeerActiveMode of session per 4.11.2.1. Retransmissions. + System::Clock::Timestamp baseTimeout = entry->ec->GetSessionHandle()->GetMRPBaseTimeout(); + System::Clock::Timestamp backoff = ReliableMessageMgr::GetBackoff(baseTimeout, entry->sendCount); + entry->nextRetransTime = System::SystemClock().GetMonotonicTimestamp() + backoff; SendFromRetransTable(entry); - // For test not using async IO loop, the entry may have been removed after send, do not use entry below return Loop::Continue; }); @@ -223,10 +223,10 @@ System::Clock::Timestamp ReliableMessageMgr::GetBackoff(System::Clock::Timestamp void ReliableMessageMgr::StartRetransmision(RetransTableEntry * entry) { - // TODO(#15800): Choose active/idle timeout corresponding to the ActiveState of peer in session. - System::Clock::Timestamp backoff = - ReliableMessageMgr::GetBackoff(entry->ec->GetSessionHandle()->GetMRPConfig().mIdleRetransTimeout, entry->sendCount); - entry->nextRetransTime = System::SystemClock().GetMonotonicTimestamp() + backoff; + // Choose active/idle timeout from PeerActiveMode of session per 4.11.2.1. Retransmissions. + System::Clock::Timestamp baseTimeout = entry->ec->GetSessionHandle()->GetMRPBaseTimeout(); + System::Clock::Timestamp backoff = ReliableMessageMgr::GetBackoff(baseTimeout, entry->sendCount); + entry->nextRetransTime = System::SystemClock().GetMonotonicTimestamp() + backoff; StartTimer(); } diff --git a/src/transport/GroupSession.h b/src/transport/GroupSession.h index 6acf55a1d40d6b..c3118653b9e384 100644 --- a/src/transport/GroupSession.h +++ b/src/transport/GroupSession.h @@ -58,6 +58,8 @@ class IncomingGroupSession : public Session return cfg; } + System::Clock::Timestamp GetMRPBaseTimeout() override { return System::Clock::kZero; } + System::Clock::Milliseconds32 GetAckTimeout() const override { VerifyOrDie(false); @@ -100,6 +102,8 @@ class OutgoingGroupSession : public Session return cfg; } + System::Clock::Timestamp GetMRPBaseTimeout() override { return System::Clock::kZero; } + System::Clock::Milliseconds32 GetAckTimeout() const override { VerifyOrDie(false); diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index b8e8a423d14f29..8ae0b00bc94bda 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -75,7 +75,8 @@ class SecureSession : public Session FabricIndex fabric, const ReliableMessageProtocolConfig & config) : mSecureSessionType(secureSessionType), mPeerNodeId(peerNodeId), mPeerCATs(peerCATs), mLocalSessionId(localSessionId), mPeerSessionId(peerSessionId), - mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()), mMRPConfig(config) + mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()), + mLastPeerActivityTime(System::SystemClock().GetMonotonicTimestamp()), mMRPConfig(config) { SetFabricIndex(fabric); } @@ -169,7 +170,20 @@ class SecureSession : public Session } System::Clock::Timestamp GetLastActivityTime() const { return mLastActivityTime; } + System::Clock::Timestamp GetLastPeerActivityTime() const { return mLastPeerActivityTime; } void MarkActive() { mLastActivityTime = System::SystemClock().GetMonotonicTimestamp(); } + void MarkActiveRx() + { + mLastPeerActivityTime = System::SystemClock().GetMonotonicTimestamp(); + MarkActive(); + } + + bool IsPeerActive() { return ((System::SystemClock().GetMonotonicTimestamp() - GetLastPeerActivityTime()) < kMinActiveTime); } + + System::Clock::Timestamp GetMRPBaseTimeout() override + { + return IsPeerActive() ? GetMRPConfig().mActiveRetransTimeout : GetMRPConfig().mIdleRetransTimeout; + } CryptoContext & GetCryptoContext() { return mCryptoContext; } @@ -183,7 +197,8 @@ class SecureSession : public Session uint16_t mPeerSessionId; PeerAddress mPeerAddress; - System::Clock::Timestamp mLastActivityTime; + System::Clock::Timestamp mLastActivityTime; ///< Timestamp of last tx or rx + System::Clock::Timestamp mLastPeerActivityTime; ///< Timestamp of last rx ReliableMessageProtocolConfig mMRPConfig; CryptoContext mCryptoContext; SessionMessageCounter mSessionMessageCounter; diff --git a/src/transport/Session.h b/src/transport/Session.h index f10954b1f07801..b51c78964cf3d2 100644 --- a/src/transport/Session.h +++ b/src/transport/Session.h @@ -32,6 +32,8 @@ class UnauthenticatedSession; class IncomingGroupSession; class OutgoingGroupSession; +constexpr System::Clock::Milliseconds32 kMinActiveTime = System::Clock::Milliseconds32(4000); + class Session { public: @@ -71,6 +73,7 @@ class Session virtual Access::SubjectDescriptor GetSubjectDescriptor() const = 0; virtual bool RequireMRP() const = 0; virtual const ReliableMessageProtocolConfig & GetMRPConfig() const = 0; + virtual System::Clock::Timestamp GetMRPBaseTimeout() = 0; virtual System::Clock::Milliseconds32 GetAckTimeout() const = 0; FabricIndex GetFabricIndex() const { return mFabricIndex; } diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index a04b8402bf0a36..7f99ae8cd4fea5 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -556,7 +556,7 @@ void SessionManager::UnauthenticatedMessageDispatch(const PacketHeader & packetH unsecuredSession->SetPeerAddress(peerAddress); SessionMessageDelegate::DuplicateMessage isDuplicate = SessionMessageDelegate::DuplicateMessage::No; - unsecuredSession->MarkActive(); + unsecuredSession->MarkActiveRx(); PayloadHeader payloadHeader; ReturnOnFailure(payloadHeader.DecodeAndConsume(msg)); @@ -640,7 +640,7 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & packetHea return; } - secureSession->MarkActive(); + secureSession->MarkActiveRx(); if (isDuplicate == SessionMessageDelegate::DuplicateMessage::Yes && !payloadHeader.NeedsAck()) { diff --git a/src/transport/UnauthenticatedSessionTable.h b/src/transport/UnauthenticatedSessionTable.h index 5ac2211fd06d05..15a118d47ccdd1 100644 --- a/src/transport/UnauthenticatedSessionTable.h +++ b/src/transport/UnauthenticatedSessionTable.h @@ -54,7 +54,9 @@ class UnauthenticatedSession : public Session, public ReferenceCounted