[sleepy] Fix #15800 - track idle/active of peer and use dynamic timeo…

…ut (#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.
project-chip · Apr 11, 2022 · 06c6e06 · 06c6e06
1 parent 60f338d
commit 06c6e06
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -54,7 +54,9 @@ class UnauthenticatedSession : public Session, public ReferenceCounted<Unauthent
 
     UnauthenticatedSession(SessionRole sessionRole, NodeId ephemeralInitiatorNodeID, const ReliableMessageProtocolConfig & config) :
         mEphemeralInitiatorNodeId(ephemeralInitiatorNodeID), mSessionRole(sessionRole),
-        mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()), mMRPConfig(config)
+        mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()),
+        mLastPeerActivityTime(System::Clock::kZero), // Start at zero to default to IDLE state
+        mMRPConfig(config)
     {}
     ~UnauthenticatedSession() override { NotifySessionReleased(); }
 
@@ -64,7 +66,13 @@ class UnauthenticatedSession : public Session, public ReferenceCounted<Unauthent
     UnauthenticatedSession & operator=(UnauthenticatedSession &&) = delete;
 
     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();
+    }
 
     Session::SessionType GetSessionType() const override { return Session::SessionType::kUnauthenticated; }
 #if CHIP_PROGRESS_LOGGING
@@ -112,6 +120,13 @@ class UnauthenticatedSession : public Session, public ReferenceCounted<Unauthent
     const PeerAddress & GetPeerAddress() const { return mPeerAddress; }
     void SetPeerAddress(const PeerAddress & peerAddress) { mPeerAddress = peerAddress; }
 
+    bool IsPeerActive() { return ((System::SystemClock().GetMonotonicTimestamp() - GetLastPeerActivityTime()) < kMinActiveTime); }
+
+    System::Clock::Timestamp GetMRPBaseTimeout() override
+    {
+        return IsPeerActive() ? GetMRPConfig().mActiveRetransTimeout : GetMRPConfig().mIdleRetransTimeout;
+    }
+
     void SetMRPConfig(const ReliableMessageProtocolConfig & config) { mMRPConfig = config; }
 
     const ReliableMessageProtocolConfig & GetMRPConfig() const override { return mMRPConfig; }
@@ -122,7 +137,8 @@ class UnauthenticatedSession : public Session, public ReferenceCounted<Unauthent
     const NodeId mEphemeralInitiatorNodeId;
     const SessionRole mSessionRole;
     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;
     PeerMessageCounter mPeerMessageCounter;
 };