From fc3672e51d37245509554928d6b224a542bd7f32 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Wed, 30 Mar 2022 14:36:36 -0700 Subject: [PATCH] [mrp] Fix #15799 - add exponential backoff. (#16026) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #15799 - Add exponential backoff. - Add unit test for exponential backoff calculation. - Add spec refs to backoff implementation. Implements the backoff algorithm as specified but in fixed point: ``` t = i⋅MRP_BACKOFF_BASE^max(0,n−MRP_BACKOFF_THRESHOLD) ⋅ (1.0+random(0,1)⋅MRP_BACKOFF_JITTER) Where: t = the resultant retransmission timeout for this transmission n = the transmission count of the message, starting with 0 i = the base retry interval for the Exchange (either IDLE or ACTIVE) MRP_BACKOFF_BASE | 1.6 | The base number for the exponential backoff equation. MRP_BACKOFF_JITTER | 0.25 | The scaler for random jitter in the backoff equation. MRP_BACKOFF_THRESHOLD | 1 | # of retransmissions before transition from linear to exponential backoff. ``` --- src/messaging/ReliableMessageMgr.cpp | 50 +++++++++++-- src/messaging/ReliableMessageMgr.h | 13 ++++ .../tests/TestReliableMessageProtocol.cpp | 74 +++++++++++++++++++ .../secure_channel/tests/TestPASESession.cpp | 2 +- 4 files changed, 132 insertions(+), 7 deletions(-) diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index 61df79998ef7e9..fcf5d0a6078c7e 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -144,9 +144,10 @@ void ReliableMessageMgr::ExecuteActions() "Retransmitting MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange " Send Cnt %d", messageCounter, ChipLogValueExchange(&entry->ec.Get()), entry->sendCount); - // TODO: Choose active/idle timeout corresponding to the activity of exchanges of the session. - entry->nextRetransTime = - System::SystemClock().GetMonotonicTimestamp() + entry->ec->GetSessionHandle()->GetMRPConfig().mActiveRetransTimeout; + // 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; SendFromRetransTable(entry); // For test not using async IO loop, the entry may have been removed after send, do not use entry below @@ -187,11 +188,48 @@ CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, Re return CHIP_NO_ERROR; } +System::Clock::Timestamp ReliableMessageMgr::GetBackoff(System::Clock::Timestamp backoffBase, 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` + uint32_t backoffNum = 1; + uint32_t backoffDenom = 1; + for (int i = 0; i < retryCount; i++) + { + backoffNum *= MRP_BACKOFF_BASE_NUMERATOR; + backoffDenom *= MRP_BACKOFF_BASE_DENOMENATOR; + } + 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: + uint32_t jitter = MRP_BACKOFF_JITTER_BASE + Crypto::GetRandU8(); + backoff = backoff * jitter / MRP_BACKOFF_JITTER_BASE; + + return backoff; +} + void ReliableMessageMgr::StartRetransmision(RetransTableEntry * entry) { - // TODO: Choose active/idle timeout corresponding to the activity of exchanges of the session. - entry->nextRetransTime = - System::SystemClock().GetMonotonicTimestamp() + entry->ec->GetSessionHandle()->GetMRPConfig().mIdleRetransTimeout; + // 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; StartTimer(); } diff --git a/src/messaging/ReliableMessageMgr.h b/src/messaging/ReliableMessageMgr.h index 8d6034ec825a0b..13266060487a5f 100644 --- a/src/messaging/ReliableMessageMgr.h +++ b/src/messaging/ReliableMessageMgr.h @@ -102,6 +102,19 @@ class ReliableMessageMgr */ CHIP_ERROR AddToRetransTable(ReliableMessageContext * rc, RetransTableEntry ** rEntry); + /** + * 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] 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); + /** * Start retranmisttion of cached encryped packet for current entry. * diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index 36e46e39a0362c..aabee8338cb6d3 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -1443,6 +1443,79 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); } +struct BackoffComplianceTestVector +{ + uint8_t sendCount; + System::Clock::Timestamp backoffBase; + System::Clock::Timestamp backoffMin; + System::Clock::Timestamp backoffMax; +}; + +struct BackoffComplianceTestVector theBackoffComplianceTestVector[] = { + { + .sendCount = 0, + .backoffBase = System::Clock::Timestamp(300), + .backoffMin = System::Clock::Timestamp(300), + .backoffMax = System::Clock::Timestamp(375), + }, + { + .sendCount = 1, + .backoffBase = System::Clock::Timestamp(300), + .backoffMin = System::Clock::Timestamp(300), + .backoffMax = System::Clock::Timestamp(375), + }, + { + .sendCount = 2, + .backoffBase = System::Clock::Timestamp(300), + .backoffMin = System::Clock::Timestamp(480), + .backoffMax = System::Clock::Timestamp(600), + }, + { + .sendCount = 3, + .backoffBase = System::Clock::Timestamp(300), + .backoffMin = System::Clock::Timestamp(768), + .backoffMax = System::Clock::Timestamp(960), + }, + { + .sendCount = 4, + .backoffBase = System::Clock::Timestamp(300), + .backoffMin = System::Clock::Timestamp(1229), + .backoffMax = System::Clock::Timestamp(1536), + }, + { + .sendCount = 5, + .backoffBase = System::Clock::Timestamp(300), + .backoffMin = System::Clock::Timestamp(1966), + .backoffMax = System::Clock::Timestamp(2458), + }, + { + .sendCount = 6, + .backoffBase = System::Clock::Timestamp(300), + .backoffMin = System::Clock::Timestamp(1966), + .backoffMax = System::Clock::Timestamp(2458), + }, +}; + +const unsigned theBackoffComplianceTestVectorLength = + sizeof(theBackoffComplianceTestVector) / sizeof(struct BackoffComplianceTestVector); + +void CheckGetBackoff(nlTestSuite * inSuite, void * inContext) +{ + // Run 3x iterations to thoroughly test random jitter always results in backoff within bounds. + for (uint32_t j = 0; j < 3; j++) + { + for (uint32_t i = 0; i < theBackoffComplianceTestVectorLength; i++) + { + struct BackoffComplianceTestVector * test = &theBackoffComplianceTestVector[i]; + System::Clock::Timestamp backoff = ReliableMessageMgr::GetBackoff(test->backoffBase, test->sendCount); + ChipLogProgress(Test, "Backoff # %d: %d", test->sendCount, (uint32_t) backoff.count()); + + NL_TEST_ASSERT(inSuite, backoff >= test->backoffMin); + NL_TEST_ASSERT(inSuite, backoff <= test->backoffMax); + } + } +} + int InitializeTestCase(void * inContext) { TestContext & ctx = *static_cast(inContext); @@ -1491,6 +1564,7 @@ const nlTest sTests[] = NL_TEST_DEF("Test that unencrypted message is dropped if exchange requires encryption", CheckUnencryptedMessageReceiveFailure), NL_TEST_DEF("Test that dropping an application-level message with a piggyback ack works ok once both sides retransmit", CheckLostResponseWithPiggyback), NL_TEST_DEF("Test that an application-level response-to-response after a lost standalone ack to the initial message works", CheckLostStandaloneAck), + NL_TEST_DEF("Test MRP backoff algorithm", CheckGetBackoff), NL_TEST_SENTINEL() }; diff --git a/src/protocols/secure_channel/tests/TestPASESession.cpp b/src/protocols/secure_channel/tests/TestPASESession.cpp index 11ada996512c61..27fb6a9ac875ee 100644 --- a/src/protocols/secure_channel/tests/TestPASESession.cpp +++ b/src/protocols/secure_channel/tests/TestPASESession.cpp @@ -237,7 +237,7 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, P while (gLoopback.mMessageDropped) { - chip::test_utils::SleepMillis(65); + chip::test_utils::SleepMillis(85); gLoopback.mMessageDropped = false; ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), ctx.GetExchangeManager().GetReliableMessageMgr()); ctx.DrainAndServiceIO();