Skip to content

Commit c6477fc

Browse files
vivien-appleadbridge
authored andcommitted
Update GetAckTimeout to use the backoff algorithm (project-chip#23307)
* [MRP] Expose GetRetransmissionTimeout from ReliableMessageProtocolConfig such that an accurate maximum retransmission time could be calculated from the external world * [MRP] Update GetAckTimeout to take into account the PeerActive status and use GetRetransmissionTimeout to be more accurate * Update src/controller/python/test/test_scripts/network_commissioning.py ScanNetwork and ConnectNetwork tests with an explicit interactionTimeoutMs instead of relying on GetAckTimeout behavior
1 parent 1da6a9e commit c6477fc

7 files changed

+57
-15
lines changed

src/controller/python/test/test_scripts/network_commissioning.py

+8-4
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ async def test_wifi(self, endpointId):
143143
logger.info(f"Scan networks")
144144
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
145145
ssid=b'', breadcrumb=self.with_breadcrumb())
146-
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
146+
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
147+
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
147148
logger.info(f"Received response: {res}")
148149
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
149150
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
@@ -198,7 +199,8 @@ async def test_wifi(self, endpointId):
198199
logger.info(f"Connect to a network")
199200
req = Clusters.NetworkCommissioning.Commands.ConnectNetwork(
200201
networkID=TEST_WIFI_SSID.encode(), breadcrumb=self.with_breadcrumb())
201-
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
202+
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
203+
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
202204
logger.info(f"Got response: {res}")
203205
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
204206
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
@@ -277,7 +279,8 @@ async def test_thread(self, endpointId):
277279
logger.info(f"Scan networks")
278280
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
279281
ssid=b'', breadcrumb=self.with_breadcrumb())
280-
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
282+
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
283+
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
281284
logger.info(f"Received response: {res}")
282285
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
283286
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
@@ -332,7 +335,8 @@ async def test_thread(self, endpointId):
332335
logger.info(f"Connect to a network")
333336
req = Clusters.NetworkCommissioning.Commands.ConnectNetwork(
334337
networkID=TEST_THREAD_NETWORK_IDS[0], breadcrumb=self.with_breadcrumb())
335-
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
338+
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
339+
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
336340
logger.info(f"Got response: {res}")
337341
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
338342
raise AssertionError(f"Unexpected result: {res.networkingStatus}")

src/messaging/ReliableMessageMgr.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@ CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, Re
204204
return CHIP_NO_ERROR;
205205
}
206206

207-
System::Clock::Timestamp ReliableMessageMgr::GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount)
207+
System::Clock::Timestamp ReliableMessageMgr::GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount,
208+
bool computeMaxPossible)
208209
{
209210
// See section "4.11.8. Parameters and Constants" for the parameters below:
210211
// MRP_BACKOFF_JITTER = 0.25
@@ -247,7 +248,7 @@ System::Clock::Timestamp ReliableMessageMgr::GetBackoff(System::Clock::Timestamp
247248
System::Clock::Timestamp mrpBackoffTime = baseInterval * backoffNum / backoffDenom;
248249

249250
// 3. Calculate `mrpBackoffTime *= (1.0 + random(0,1) * MRP_BACKOFF_JITTER)`
250-
uint32_t jitter = MRP_BACKOFF_JITTER_BASE + Crypto::GetRandU8();
251+
uint32_t jitter = MRP_BACKOFF_JITTER_BASE + (computeMaxPossible ? UINT8_MAX : Crypto::GetRandU8());
251252
mrpBackoffTime = mrpBackoffTime * jitter / MRP_BACKOFF_JITTER_BASE;
252253

253254
#if CHIP_DEVICE_CONFIG_ENABLE_SED

src/messaging/ReliableMessageMgr.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,17 @@ class ReliableMessageMgr
102102
/**
103103
* Calculate the backoff timer for the retransmission.
104104
*
105-
* @param[in] baseInterval The base interval to use for the backoff calculation, either the active or idle interval.
106-
* @param[in] sendCount Count of how many times this message
107-
* has been retransmitted so far (0 if it has
108-
* been sent only once with no retransmits,
109-
* 1 if it has been sent twice, etc).
105+
* @param[in] baseInterval The base interval to use for the backoff calculation, either the active or idle interval.
106+
* @param[in] sendCount Count of how many times this message
107+
* has been retransmitted so far (0 if it has
108+
* been sent only once with no retransmits,
109+
* 1 if it has been sent twice, etc).
110+
* @param[in] computeMaxPossible Disable randomness such that the maximum value is used instead.
110111
*
111112
* @retval The backoff time value, including jitter.
112113
*/
113-
static System::Clock::Timestamp GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount);
114+
static System::Clock::Timestamp GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount,
115+
bool computeMaxPossible = false);
114116

115117
/**
116118
* Start retranmisttion of cached encryped packet for current entry.

src/messaging/ReliableMessageProtocolConfig.cpp

+19-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*
2323
*/
2424

25-
#include <messaging/ReliableMessageProtocolConfig.h>
25+
#include <messaging/ReliableMessageMgr.h>
2626

2727
#include <platform/CHIPDeviceLayer.h>
2828
#include <system/SystemClock.h>
@@ -88,4 +88,22 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
8888
: Optional<ReliableMessageProtocolConfig>::Value(config);
8989
}
9090

91+
System::Clock::Timestamp GetRetransmissionTimeout(System::Clock::Timestamp activeInterval, System::Clock::Timestamp idleInterval,
92+
System::Clock::Timestamp lastActivityTime,
93+
System::Clock::Timestamp activityThreshold)
94+
{
95+
auto timeSinceLastActivity = (System::SystemClock().GetMonotonicTimestamp() - lastActivityTime);
96+
97+
// Calculate the retransmission timeout and take into account that an active/idle state change can happen
98+
// in the middle.
99+
System::Clock::Timestamp timeout(0);
100+
for (uint8_t i = 0; i < CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1; i++)
101+
{
102+
auto baseInterval = ((timeSinceLastActivity + timeout) < activityThreshold) ? activeInterval : idleInterval;
103+
timeout += Messaging::ReliableMessageMgr::GetBackoff(baseInterval, i, /* computeMaxPossible */ true);
104+
}
105+
106+
return timeout;
107+
}
108+
91109
} // namespace chip

src/messaging/ReliableMessageProtocolConfig.h

+15
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,21 @@ ReliableMessageProtocolConfig GetDefaultMRPConfig();
156156
*/
157157
Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig();
158158

159+
/**
160+
* @brief
161+
* Returns the maximum transmission time depending on the last activity time.
162+
*
163+
* @param[in] activeInterval The active interval to use for the backoff calculation.
164+
* @param[in] idleInterval The idle interval to use for the backoff calculation.
165+
* @param[in] lastActivityTime The last time some activity has been recorded.
166+
* @param[in] activityThreshold The activity threshold for a node to be considered active.
167+
*
168+
* @return The maximum transmission time
169+
*/
170+
System::Clock::Timestamp GetRetransmissionTimeout(System::Clock::Timestamp activeInterval, System::Clock::Timestamp idleInterval,
171+
System::Clock::Timestamp lastActivityTime,
172+
System::Clock::Timestamp activityThreshold);
173+
159174
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
160175

161176
/**

src/transport/SecureSession.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ class SecureSession : public Session, public ReferenceCounted<SecureSession, Sec
162162
switch (mPeerAddress.GetTransportType())
163163
{
164164
case Transport::Type::kUdp:
165-
return GetRemoteMRPConfig().mIdleRetransTimeout * (CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1);
165+
return GetRetransmissionTimeout(mRemoteMRPConfig.mActiveRetransTimeout, mRemoteMRPConfig.mIdleRetransTimeout,
166+
GetLastPeerActivityTime(), kMinActiveTime);
166167
case Transport::Type::kTcp:
167168
return System::Clock::Seconds16(30);
168169
case Transport::Type::kBle:

src/transport/UnauthenticatedSessionTable.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ class UnauthenticatedSession : public Session,
8787
switch (mPeerAddress.GetTransportType())
8888
{
8989
case Transport::Type::kUdp:
90-
return GetRemoteMRPConfig().mIdleRetransTimeout * (CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1);
90+
return GetRetransmissionTimeout(mRemoteMRPConfig.mActiveRetransTimeout, mRemoteMRPConfig.mIdleRetransTimeout,
91+
GetLastPeerActivityTime(), kMinActiveTime);
9192
case Transport::Type::kTcp:
9293
return System::Clock::Seconds16(30);
9394
default:

0 commit comments

Comments
 (0)