Skip to content

Commit

Permalink
[mrp] Align MRP backoff time calculation with spec (#21356)
Browse files Browse the repository at this point in the history 
* [mrp] Align ReliableMessageMgr::GetBackoff() naming with spec

Make variables and comments in the function more aligned
with the current spec version so that it is easier to
compare the code with the spec.

Signed-off-by: Damian Krolik <[email protected]>

* [mrp] Implement MRP_BACKOFF_MARGIN

Base interval (active or idle) should be multiplied by
MRP_BACKOFF_MARGIN = 1.1 before putting it in the MRP
backoff formula.

Signed-off-by: Damian Krolik <[email protected]>

* [mrp] Increase MRP backoff time for SED sender

When sender is sleepy, it may take more time until it
receives an acknowledgment from a peer, so MRP backoff time
should be extended by the sleepy active interval.

Signed-off-by: Damian Krolik <[email protected]>
  • Loading branch information
@Damian-Nordic @pull
Damian-Nordic authored and pull[bot] committed Sep 13, 2023
1 parent 6ff3d5b commit 1879889
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 48 deletions.
76 changes: 51 additions & 25 deletions src/messaging/ReliableMessageMgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <messaging/ExchangeMgr.h>
#include <messaging/Flags.h>
#include <messaging/ReliableMessageContext.h>
#include <platform/ConnectivityManager.h>

using namespace chip::System::Clock::Literals;

Expand Down Expand Up @@ -202,40 +203,65 @@ CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, Re
return CHIP_NO_ERROR;
}

System::Clock::Timestamp ReliableMessageMgr::GetBackoff(System::Clock::Timestamp backoffBase, uint8_t sendCount)
System::Clock::Timestamp ReliableMessageMgr::GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount)
{
static constexpr uint32_t MRP_BACKOFF_JITTER_BASE = 1024;
static constexpr uint32_t MRP_BACKOFF_BASE_NUMERATOR = 16;
static constexpr uint32_t MRP_BACKOFF_BASE_DENOMENATOR = 10;
static constexpr uint32_t MRP_BACKOFF_THRESHOLD = 1;

System::Clock::Timestamp backoff = backoffBase;

// Implement `t = i⋅MRP_BACKOFF_BASE^max(0,n−MRP_BACKOFF_THRESHOLD)` from Section 4.11.2.1. Retransmissions

// Generate fixed point equivalent of `retryCount = max(0,n−MRP_BACKOFF_THRESHOLD)`
int retryCount = sendCount - MRP_BACKOFF_THRESHOLD;
if (retryCount < 0)
retryCount = 0; // Enforce floor
if (retryCount > 4)
retryCount = 4; // Enforce reasonable maximum after 5 tries

// Generate fixed point equivalent of `backoff = i⋅1.6^retryCount`
// See section "4.11.8. Parameters and Constants" for the parameters below:
// MRP_BACKOFF_JITTER = 0.25
constexpr uint32_t MRP_BACKOFF_JITTER_BASE = 1024;
// MRP_BACKOFF_MARGIN = 1.1
constexpr uint32_t MRP_BACKOFF_MARGIN_NUMERATOR = 1127;
constexpr uint32_t MRP_BACKOFF_MARGIN_DENOMINATOR = 1024;
// MRP_BACKOFF_BASE = 1.6
constexpr uint32_t MRP_BACKOFF_BASE_NUMERATOR = 16;
constexpr uint32_t MRP_BACKOFF_BASE_DENOMINATOR = 10;
constexpr int MRP_BACKOFF_THRESHOLD = 1;

// Implement `i = MRP_BACKOFF_MARGIN * i` from section "4.11.2.1. Retransmissions", where:
// i == baseInterval
baseInterval = baseInterval * MRP_BACKOFF_MARGIN_NUMERATOR / MRP_BACKOFF_MARGIN_DENOMINATOR;

// Implement:
// mrpBackoffTime = i * MRP_BACKOFF_BASE^(max(0,n-MRP_BACKOFF_THRESHOLD)) * (1.0 + random(0,1) * MRP_BACKOFF_JITTER)
// from section "4.11.2.1. Retransmissions", where:
// i == baseInterval
// n == sendCount

// 1. Calculate exponent `max(0,n−MRP_BACKOFF_THRESHOLD)`
int exponent = sendCount - MRP_BACKOFF_THRESHOLD;
if (exponent < 0)
exponent = 0; // Enforce floor
if (exponent > 4)
exponent = 4; // Enforce reasonable maximum after 5 tries

// 2. Calculate `mrpBackoffTime = i * MRP_BACKOFF_BASE^(max(0,n-MRP_BACKOFF_THRESHOLD))`
uint32_t backoffNum = 1;
uint32_t backoffDenom = 1;
for (int i = 0; i < retryCount; i++)

for (int i = 0; i < exponent; i++)
{
backoffNum *= MRP_BACKOFF_BASE_NUMERATOR;
backoffDenom *= MRP_BACKOFF_BASE_DENOMENATOR;
backoffDenom *= MRP_BACKOFF_BASE_DENOMINATOR;
}
backoff = backoff * backoffNum / backoffDenom;

// Implement jitter scaler: `t *= (1.0+random(0,1)⋅MRP_BACKOFF_JITTER)`
// where jitter is random multiplier from 1.000 to 1.250:
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();
backoff = backoff * jitter / MRP_BACKOFF_JITTER_BASE;
mrpBackoffTime = mrpBackoffTime * jitter / MRP_BACKOFF_JITTER_BASE;

#if CHIP_DEVICE_CONFIG_ENABLE_SED
// Implement:
// "A sleepy sender SHOULD increase t to also account for its own sleepy interval
// required to receive the acknowledgment"
DeviceLayer::ConnectivityManager::SEDIntervalsConfig sedIntervals;

if (DeviceLayer::ConnectivityMgr().GetSEDIntervalsConfig(sedIntervals) == CHIP_NO_ERROR)
{
mrpBackoffTime += System::Clock::Timestamp(sedIntervals.ActiveIntervalMS);
}
#endif

return backoff;
return mrpBackoffTime;
}

void ReliableMessageMgr::StartRetransmision(RetransTableEntry * entry)
Expand Down
4 changes: 2 additions & 2 deletions src/messaging/ReliableMessageMgr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ class ReliableMessageMgr
/**
* Calculate the backoff timer for the retransmission.
*
* @param[in] backoffBase The base interval to use for the backoff calculation, either the active or idle interval.
* @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).
*
* @retval The backoff time value, including jitter.
*/
static System::Clock::Timestamp GetBackoff(System::Clock::Timestamp backoffBase, uint8_t sendCount);
static System::Clock::Timestamp GetBackoff(System::Clock::Timestamp baseInterval, uint8_t sendCount);

/**
* Start retranmisttion of cached encryped packet for current entry.
Expand Down
83 changes: 63 additions & 20 deletions src/messaging/tests/TestReliableMessageProtocol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,44 +183,86 @@ struct BackoffComplianceTestVector theBackoffComplianceTestVector[] = {
{
.sendCount = 0,
.backoffBase = System::Clock::Timeout(300),
.backoffMin = System::Clock::Timeout(300),
.backoffMax = System::Clock::Timeout(375),
.backoffMin = System::Clock::Timeout(330),
.backoffMax = System::Clock::Timeout(413),
},
{
.sendCount = 1,
.backoffBase = System::Clock::Timeout(300),
.backoffMin = System::Clock::Timeout(300),
.backoffMax = System::Clock::Timeout(375),
.backoffMin = System::Clock::Timeout(330),
.backoffMax = System::Clock::Timeout(413),
},
{
.sendCount = 2,
.backoffBase = System::Clock::Timeout(300),
.backoffMin = System::Clock::Timeout(480),
.backoffMax = System::Clock::Timeout(600),
.backoffMin = System::Clock::Timeout(528),
.backoffMax = System::Clock::Timeout(660),
},
{
.sendCount = 3,
.backoffBase = System::Clock::Timeout(300),
.backoffMin = System::Clock::Timeout(768),
.backoffMax = System::Clock::Timeout(960),
.backoffMin = System::Clock::Timeout(844),
.backoffMax = System::Clock::Timeout(1057),
},
{
.sendCount = 4,
.backoffBase = System::Clock::Timeout(300),
.backoffMin = System::Clock::Timeout(1228),
.backoffMax = System::Clock::Timeout(1536),
.backoffMin = System::Clock::Timeout(1351),
.backoffMax = System::Clock::Timeout(1690),
},
{
.sendCount = 5,
.backoffBase = System::Clock::Timeout(300),
.backoffMin = System::Clock::Timeout(1966),
.backoffMax = System::Clock::Timeout(2458),
.backoffMin = System::Clock::Timeout(2162),
.backoffMax = System::Clock::Timeout(2704),
},
{
.sendCount = 6,
.backoffBase = System::Clock::Timeout(300),
.backoffMin = System::Clock::Timeout(1966),
.backoffMax = System::Clock::Timeout(2458),
.backoffMin = System::Clock::Timeout(2162),
.backoffMax = System::Clock::Timeout(2704),
},
{
.sendCount = 0,
.backoffBase = System::Clock::Timeout(4000),
.backoffMin = System::Clock::Timeout(4400),
.backoffMax = System::Clock::Timeout(5500),
},
{
.sendCount = 1,
.backoffBase = System::Clock::Timeout(4000),
.backoffMin = System::Clock::Timeout(4400),
.backoffMax = System::Clock::Timeout(5500),
},
{
.sendCount = 2,
.backoffBase = System::Clock::Timeout(4000),
.backoffMin = System::Clock::Timeout(7040),
.backoffMax = System::Clock::Timeout(8800),
},
{
.sendCount = 3,
.backoffBase = System::Clock::Timeout(4000),
.backoffMin = System::Clock::Timeout(11264),
.backoffMax = System::Clock::Timeout(14081),
},
{
.sendCount = 4,
.backoffBase = System::Clock::Timeout(4000),
.backoffMin = System::Clock::Timeout(18022),
.backoffMax = System::Clock::Timeout(22529),
},
{
.sendCount = 5,
.backoffBase = System::Clock::Timeout(4000),
.backoffMin = System::Clock::Timeout(28835),
.backoffMax = System::Clock::Timeout(36045),
},
{
.sendCount = 6,
.backoffBase = System::Clock::Timeout(4000),
.backoffMin = System::Clock::Timeout(28835),
.backoffMax = System::Clock::Timeout(36045),
},
};

Expand Down Expand Up @@ -323,7 +365,7 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 1);
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1);

// Wait for the initial message to fail (should take 300-375ms)
// Wait for the initial message to fail (should take 330-413ms)
ctx.GetIOContext().DriveIOUntil(1000_ms32, [&] { return loopback.mSentMessageCount >= 2; });
now = System::SystemClock().GetMonotonicTimestamp();
timeoutTime = now - startTime;
Expand All @@ -340,7 +382,7 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 2);
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1);

// Wait for the 1st retry to fail (should take 300-375ms)
// Wait for the 1st retry to fail (should take 330-413ms)
ctx.GetIOContext().DriveIOUntil(1000_ms32, [&] { return loopback.mSentMessageCount >= 3; });
now = System::SystemClock().GetMonotonicTimestamp();
timeoutTime = now - startTime;
Expand All @@ -357,7 +399,7 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 3);
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1);

// Wait for the 2nd retry to fail (should take 480-600msms)
// Wait for the 2nd retry to fail (should take 528-660ms)
ctx.GetIOContext().DriveIOUntil(1000_ms32, [&] { return loopback.mSentMessageCount >= 4; });
now = System::SystemClock().GetMonotonicTimestamp();
timeoutTime = now - startTime;
Expand All @@ -374,8 +416,8 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 4);
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1);

// Wait for the 3rd retry to fail (should take 768-960ms)
ctx.GetIOContext().DriveIOUntil(1000_ms32, [&] { return loopback.mSentMessageCount >= 5; });
// Wait for the 3rd retry to fail (should take 845-1056ms)
ctx.GetIOContext().DriveIOUntil(1500_ms32, [&] { return loopback.mSentMessageCount >= 5; });
now = System::SystemClock().GetMonotonicTimestamp();
timeoutTime = now - startTime;
ChipLogProgress(Test, "Attempt #4 Timeout : %d ms", timeoutTime.count());
Expand Down Expand Up @@ -1613,7 +1655,8 @@ void CheckGetBackoff(nlTestSuite * inSuite, void * inContext)
{
struct BackoffComplianceTestVector * test = &theBackoffComplianceTestVector[i];
System::Clock::Timeout backoff = ReliableMessageMgr::GetBackoff(test->backoffBase, test->sendCount);
ChipLogProgress(Test, "Backoff # %d: %" PRIu32, test->sendCount, (uint32_t) backoff.count());
ChipLogProgress(Test, "Backoff base %" PRIu32 " # %d: %" PRIu32, test->backoffBase.count(), test->sendCount,
backoff.count());

NL_TEST_ASSERT(inSuite, backoff >= test->backoffMin);
NL_TEST_ASSERT(inSuite, backoff <= test->backoffMax);
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/secure_channel/tests/TestPASESession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, S

while (delegate.mMessageDropped)
{
chip::test_utils::SleepMillis(85);
chip::test_utils::SleepMillis(100);
delegate.mMessageDropped = false;
ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), ctx.GetExchangeManager().GetReliableMessageMgr());
ctx.DrainAndServiceIO();
Expand Down

0 comments on commit 1879889

Please sign in to comment.