Skip to content

Commit a13da8b

Browse files
yunhanw-googlerestyled-commitsbzbarsky-apple
authored andcommitted
[nrf fromtree] Fix MRP baseTimeout calculation for LIT (#37696) (#38062)
* Fix MRP baseTimeout calculation when it is possible the other end fall asleep * use min(SAI, SII) for mrp timeout when clock runs into idle * Restyled by clang-format * update mrp calculation -- Use 15 seconds to determine whether device is in LIT mode, then if yes, set the timeout base as SAI if GetRemoteMRPConfig().mIdleRetransTimeout.count() > sitIcdSlowPollMaximum.count() * Address comments * address comments * Restyled by whitespace * address comments * Restyled by whitespace * Restyled by clang-format * Update src/transport/UnauthenticatedSessionTable.h * Update src/transport/Session.h * Update src/protocols/secure_channel/CASESession.cpp * Update src/transport/SecureSession.h * Update src/protocols/secure_channel/CASESession.cpp * Update src/app/ReadClient.cpp * Update src/messaging/ReliableMessageProtocolConfig.cpp * Update src/messaging/ReliableMessageProtocolConfig.h * address comments * address comments * update true isFirstMessageOnExchange for all ComputeRoundTripTimeout caller * Restyled by whitespace * Restyled by clang-format * fix compilation error * address comments * cleanup * add parameter isFirstMessageOnExchange in GetMessageReceiptTimeout * Restyled by clang-format --------- Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]> (cherry picked from commit 5fd234d4f14e1225533eaea85854f160bbd0fd55) Signed-off-by: Adrian Gielniewski <[email protected]>
1 parent 4f96fa4 commit a13da8b

File tree

17 files changed

+83
-51
lines changed

17 files changed

+83
-51
lines changed

src/app/CommandSender.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ CHIP_ERROR CommandSender::SendCommandRequestInternal(const SessionHandle & sessi
125125
mExchangeCtx.Grab(exchange);
126126
VerifyOrReturnError(!mExchangeCtx->IsGroupExchangeContext(), CHIP_ERROR_INVALID_MESSAGE_TYPE);
127127

128-
mExchangeCtx->SetResponseTimeout(timeout.ValueOr(session->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime)));
128+
mExchangeCtx->SetResponseTimeout(
129+
timeout.ValueOr(session->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime, true /*isFirstMessageOnExchange*/)));
129130

130131
if (mTimedInvokeTimeoutMs.HasValue())
131132
{

src/app/ReadClient.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -987,12 +987,15 @@ CHIP_ERROR ReadClient::ComputeLivenessCheckTimerTimeout(System::Clock::Timeout *
987987
// So recompute the round-trip timeout directly. Assume MRP, since in practice that is likely what is happening.
988988
auto & peerMRPConfig = mReadPrepareParams.mSessionHolder->GetRemoteMRPConfig();
989989
// Peer will assume we are idle (hence we pass kZero to GetMessageReceiptTimeout()), but will assume we treat it as active
990-
// for the response, so to match the retransmission timeout computation for the message back to the peeer, we should treat
991-
// it as active.
992-
auto roundTripTimeout = mReadPrepareParams.mSessionHolder->GetMessageReceiptTimeout(System::Clock::kZero) +
990+
// for the response, so to match the retransmission timeout computation for the message back to the peer, we should treat
991+
// it as active and handling non-initial message, isFirstMessageOnExchange needs to be set as false for
992+
// GetRetransmissionTimeout.
993+
auto roundTripTimeout =
994+
mReadPrepareParams.mSessionHolder->GetMessageReceiptTimeout(System::Clock::kZero, true /*isFirstMessageOnExchange*/) +
993995
kExpectedIMProcessingTime +
994996
GetRetransmissionTimeout(peerMRPConfig.mActiveRetransTimeout, peerMRPConfig.mIdleRetransTimeout,
995-
System::SystemClock().GetMonotonicTimestamp(), peerMRPConfig.mActiveThresholdTime);
997+
System::SystemClock().GetMonotonicTimestamp(), peerMRPConfig.mActiveThresholdTime,
998+
false /*isFirstMessageOnExchange*/);
996999
*aTimeout = System::Clock::Seconds16(mMaxInterval) + roundTripTimeout;
9971000
return CHIP_NO_ERROR;
9981001
}

src/app/TimedHandler.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,9 @@ CHIP_ERROR TimedHandler::HandleTimedRequestAction(Messaging::ExchangeContext * a
127127
// will send nothing and the other side will have to time out to realize
128128
// it's missed its window).
129129
auto delay = System::Clock::Milliseconds32(timeoutMs);
130-
aExchangeContext->SetResponseTimeout(
131-
std::max(delay, aExchangeContext->GetSessionHandle()->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime)));
130+
aExchangeContext->SetResponseTimeout(std::max(delay,
131+
aExchangeContext->GetSessionHandle()->ComputeRoundTripTimeout(
132+
app::kExpectedIMProcessingTime, false /*isFirstMessageOnExchange*/)));
132133
ReturnErrorOnFailure(StatusResponse::Send(Status::Success, aExchangeContext, /* aExpectResponse = */ true));
133134

134135
// Now just wait for the client.

src/controller/AutoCommissioner.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ Optional<System::Clock::Timeout> AutoCommissioner::GetCommandTimeout(DeviceProxy
651651
auto sessionHandle = device->GetSecureSession();
652652
if (sessionHandle.HasValue())
653653
{
654-
timeout = sessionHandle.Value()->ComputeRoundTripTimeout(timeout);
654+
timeout = sessionHandle.Value()->ComputeRoundTripTimeout(timeout, true /*isFirstMessageOnExchange*/);
655655
}
656656

657657
// Enforce the spec minimal timeout. Maybe this enforcement should live in

src/controller/python/chip/utils/DeviceProxyUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ uint32_t pychip_DeviceProxy_ComputeRoundTripTimeout(DeviceProxy * device, uint32
7171

7272
return deviceProxy->GetSecureSession()
7373
.Value()
74-
->ComputeRoundTripTimeout(System::Clock::Milliseconds32(upperLayerProcessingTimeoutMs))
74+
->ComputeRoundTripTimeout(System::Clock::Milliseconds32(upperLayerProcessingTimeoutMs), true /*isFirstMessageOnExchange*/)
7575
.count();
7676
}
7777

src/controller/tests/TestWriteChunking.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,9 +566,10 @@ void TestWriteChunking::RunTest(Instructions instructions)
566566
err = writeClient->SendWriteRequest(sessionHandle);
567567
EXPECT_EQ(err, CHIP_NO_ERROR);
568568

569-
GetIOContext().DriveIOUntil(sessionHandle->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime) +
570-
System::Clock::Seconds16(1),
571-
[&]() { return GetExchangeManager().GetNumActiveExchanges() == 0; });
569+
GetIOContext().DriveIOUntil(
570+
sessionHandle->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime, true /*isFirstMessageOnExchange*/) +
571+
System::Clock::Seconds16(1),
572+
[&]() { return GetExchangeManager().GetNumActiveExchanges() == 0; });
572573

573574
EXPECT_EQ(onGoingPath, app::ConcreteAttributePath());
574575
EXPECT_EQ(status.size(), instructions.expectedStatus.size());

src/controller/tests/data_model/TestRead.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4783,10 +4783,10 @@ System::Clock::Timeout TestRead::ComputeSubscriptionTimeout(System::Clock::Secon
47834783
// Add 1000ms of slack to our max interval to make sure we hit the
47844784
// subscription liveness timer. 100ms was tried in the past and is not
47854785
// sufficient: our process can easily lose the timeslice for 100ms.
4786-
const auto & ourMrpConfig = GetDefaultMRPConfig();
4787-
auto publisherTransmissionTimeout =
4788-
GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout,
4789-
System::SystemClock().GetMonotonicTimestamp(), ourMrpConfig.mActiveThresholdTime);
4786+
const auto & ourMrpConfig = GetDefaultMRPConfig();
4787+
auto publisherTransmissionTimeout = GetRetransmissionTimeout(
4788+
ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout, System::SystemClock().GetMonotonicTimestamp(),
4789+
ourMrpConfig.mActiveThresholdTime, true /* isFirstMessageOnExchange */);
47904790

47914791
return publisherTransmissionTimeout + aMaxInterval + System::Clock::Milliseconds32(1000);
47924792
}

src/darwin/Framework/CHIP/MTRBaseDevice.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1475,7 +1475,7 @@ - (void)_invokeCommandWithEndpointID:(NSNumber *)endpointID
14751475
// Clamp to a number of seconds that will not overflow 32-bit
14761476
// int when converted to ms.
14771477
auto serverTimeoutInSeconds = System::Clock::Seconds16(serverSideProcessingTimeout.unsignedShortValue);
1478-
invokeTimeout.SetValue(session->ComputeRoundTripTimeout(serverTimeoutInSeconds));
1478+
invokeTimeout.SetValue(session->ComputeRoundTripTimeout(serverTimeoutInSeconds, true /*isFirstMessageOnExchange*/));
14791479
}
14801480
ReturnErrorOnFailure(commandSender->SendCommandRequest(session, invokeTimeout));
14811481

src/messaging/ExchangeContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ void ExchangeContext::SetResponseExpected(bool inResponseExpected)
7777

7878
void ExchangeContext::UseSuggestedResponseTimeout(Timeout applicationProcessingTimeout)
7979
{
80-
SetResponseTimeout(mSession->ComputeRoundTripTimeout(applicationProcessingTimeout));
80+
SetResponseTimeout(mSession->ComputeRoundTripTimeout(applicationProcessingTimeout, !HasReceivedAtLeastOneMessage()));
8181
}
8282

8383
void ExchangeContext::SetResponseTimeout(Timeout timeout)

src/messaging/ReliableMessageProtocolConfig.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
126126
}
127127

128128
System::Clock::Timeout GetRetransmissionTimeout(System::Clock::Timeout activeInterval, System::Clock::Timeout idleInterval,
129-
System::Clock::Timeout lastActivityTime, System::Clock::Timeout activityThreshold)
129+
System::Clock::Timeout lastActivityTime, System::Clock::Timeout activityThreshold,
130+
bool isFirstMessageOnExchange)
130131
{
131132
auto timeSinceLastActivity = (System::SystemClock().GetMonotonicTimestamp() - lastActivityTime);
132133

@@ -135,7 +136,14 @@ System::Clock::Timeout GetRetransmissionTimeout(System::Clock::Timeout activeInt
135136
System::Clock::Timestamp timeout(0);
136137
for (uint8_t i = 0; i < CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1; i++)
137138
{
138-
auto baseInterval = ((timeSinceLastActivity + timeout) < activityThreshold) ? activeInterval : idleInterval;
139+
auto baseInterval = activeInterval;
140+
// If we are calculating the timeout for the initial message, we never know whether the peer is active or not, choose
141+
// active/idle interval from PeerActiveMode of session per 4.11.2.1. Retransmissions.
142+
// If we are calculating the timeout for response message, we know the peer is active, always choose active interval.
143+
if (isFirstMessageOnExchange)
144+
{
145+
baseInterval = ((timeSinceLastActivity + timeout) < activityThreshold) ? activeInterval : idleInterval;
146+
}
139147
timeout += Messaging::ReliableMessageMgr::GetBackoff(baseInterval, i, /* computeMaxPossible */ true);
140148
}
141149

0 commit comments

Comments
 (0)