Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update GetAckTimeout to use the backoff algorithm #23307

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/controller/python/test/test_scripts/network_commissioning.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ async def test_wifi(self, endpointId):
logger.info(f"Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
ssid=b'', breadcrumb=self.with_breadcrumb())
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
logger.info(f"Received response: {res}")
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
Expand Down Expand Up @@ -198,7 +199,8 @@ async def test_wifi(self, endpointId):
logger.info(f"Connect to a network")
req = Clusters.NetworkCommissioning.Commands.ConnectNetwork(
networkID=TEST_WIFI_SSID.encode(), breadcrumb=self.with_breadcrumb())
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
logger.info(f"Got response: {res}")
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
Expand Down Expand Up @@ -277,7 +279,8 @@ async def test_thread(self, endpointId):
logger.info(f"Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
ssid=b'', breadcrumb=self.with_breadcrumb())
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
logger.info(f"Received response: {res}")
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
Expand Down Expand Up @@ -332,7 +335,8 @@ async def test_thread(self, endpointId):
logger.info(f"Connect to a network")
req = Clusters.NetworkCommissioning.Commands.ConnectNetwork(
networkID=TEST_THREAD_NETWORK_IDS[0], breadcrumb=self.with_breadcrumb())
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req, interactionTimeoutMs=interactionTimeoutMs)
logger.info(f"Got response: {res}")
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
Expand Down
5 changes: 3 additions & 2 deletions src/messaging/ReliableMessageMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, Re
return CHIP_NO_ERROR;
}

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

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

#if CHIP_DEVICE_CONFIG_ENABLE_SED
Expand Down
14 changes: 8 additions & 6 deletions src/messaging/ReliableMessageMgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,17 @@ class ReliableMessageMgr
/**
* Calculate the backoff timer for the retransmission.
*
* @param[in] baseInterval The base interval to use for the backoff calculation, either the active or idle interval.
* @param[in] sendCount Count of how many times this message
* has been retransmitted so far (0 if it has
* been sent only once with no retransmits,
* 1 if it has been sent twice, etc).
* @param[in] baseInterval The base interval to use for the backoff calculation, either the active or idle interval.
* @param[in] sendCount Count of how many times this message
* has been retransmitted so far (0 if it has
* been sent only once with no retransmits,
* 1 if it has been sent twice, etc).
* @param[in] computeMaxPossible Disable randomness such that the maximum value is used instead.
*
* @retval The backoff time value, including jitter.
*/
static System::Clock::Timestamp GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount);
static System::Clock::Timestamp GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount,
bool computeMaxPossible = false);

/**
* Start retranmisttion of cached encryped packet for current entry.
Expand Down
20 changes: 19 additions & 1 deletion src/messaging/ReliableMessageProtocolConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*
*/

#include <messaging/ReliableMessageProtocolConfig.h>
#include <messaging/ReliableMessageMgr.h>

#include <platform/CHIPDeviceLayer.h>
#include <system/SystemClock.h>
Expand Down Expand Up @@ -88,4 +88,22 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
: Optional<ReliableMessageProtocolConfig>::Value(config);
}

System::Clock::Timestamp GetRetransmissionTimeout(System::Clock::Timestamp activeInterval, System::Clock::Timestamp idleInterval,
System::Clock::Timestamp lastActivityTime,
System::Clock::Timestamp activityThreshold)
{
auto timeSinceLastActivity = (System::SystemClock().GetMonotonicTimestamp() - lastActivityTime);

// Calculate the retransmission timeout and take into account that an active/idle state change can happen
// in the middle.
System::Clock::Timestamp timeout(0);
for (uint8_t i = 0; i < CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1; i++)
{
auto baseInterval = ((timeSinceLastActivity + timeout) < activityThreshold) ? activeInterval : idleInterval;
timeout += Messaging::ReliableMessageMgr::GetBackoff(baseInterval, i, /* computeMaxPossible */ true);
}

return timeout;
}

} // namespace chip
15 changes: 15 additions & 0 deletions src/messaging/ReliableMessageProtocolConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,21 @@ ReliableMessageProtocolConfig GetDefaultMRPConfig();
*/
Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig();

/**
* @brief
* Returns the maximum transmission time depending on the last activity time.
*
* @param[in] activeInterval The active interval to use for the backoff calculation.
* @param[in] idleInterval The idle interval to use for the backoff calculation.
* @param[in] lastActivityTime The last time some activity has been recorded.
* @param[in] activityThreshold The activity threshold for a node to be considered active.
*
* @return The maximum transmission time
*/
System::Clock::Timestamp GetRetransmissionTimeout(System::Clock::Timestamp activeInterval, System::Clock::Timestamp idleInterval,
System::Clock::Timestamp lastActivityTime,
System::Clock::Timestamp activityThreshold);

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST

/**
Expand Down
3 changes: 2 additions & 1 deletion src/transport/SecureSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ class SecureSession : public Session, public ReferenceCounted<SecureSession, Sec
switch (mPeerAddress.GetTransportType())
{
case Transport::Type::kUdp:
return GetRemoteMRPConfig().mIdleRetransTimeout * (CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1);
return GetRetransmissionTimeout(mRemoteMRPConfig.mActiveRetransTimeout, mRemoteMRPConfig.mIdleRetransTimeout,
GetLastPeerActivityTime(), kMinActiveTime);
vivien-apple marked this conversation as resolved.
Show resolved Hide resolved
case Transport::Type::kTcp:
return System::Clock::Seconds16(30);
case Transport::Type::kBle:
Expand Down
3 changes: 2 additions & 1 deletion src/transport/UnauthenticatedSessionTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class UnauthenticatedSession : public Session,
switch (mPeerAddress.GetTransportType())
{
case Transport::Type::kUdp:
return GetRemoteMRPConfig().mIdleRetransTimeout * (CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1);
return GetRetransmissionTimeout(mRemoteMRPConfig.mActiveRetransTimeout, mRemoteMRPConfig.mIdleRetransTimeout,
GetLastPeerActivityTime(), kMinActiveTime);
case Transport::Type::kTcp:
return System::Clock::Seconds16(30);
default:
Expand Down