From 6ecfb46e5998e7cc17998fe7301b50b19c7c2279 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 6 Oct 2023 15:41:58 -0400 Subject: [PATCH] Fix handling of a response that comes after MRP resends end. After #29173 we can get into the following situation: 1. A message is sent. 2. Before we get an ack or response, all MRP retries happen, MRP gives up, but the exchange response timer has not been hit yet. 3. We get an actual response. 4. Because our exchange is marked as having an un-acked message, but the incoming message is not treated as an ack (because the MRP state that would do that has been torn down), we do not clear our "have un-acked message" state and end up discarding the incoming message. The fix is as follows: * Rename things to make it clear that what we really have is "waiting for an ack" state, which in fact _does_ get cleared when we run out of MRP retries, not an "un-acked message" state. * Have a separate state bit for tracking that we ran out of MRP retries on a message we sent. --- src/messaging/ExchangeContext.cpp | 11 +- src/messaging/ReliableMessageContext.cpp | 12 +- src/messaging/ReliableMessageContext.h | 38 ++- src/messaging/ReliableMessageMgr.cpp | 12 +- src/messaging/tests/MessagingContext.cpp | 14 + src/messaging/tests/MessagingContext.h | 8 +- .../tests/TestReliableMessageProtocol.cpp | 297 +++++++++++++++++- 7 files changed, 363 insertions(+), 29 deletions(-) diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index 8c085025456ba8..3a0b8343301ce5 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -79,6 +79,7 @@ bool ExchangeContext::IsResponseExpected() const void ExchangeContext::SetResponseExpected(bool inResponseExpected) { mFlags.Set(Flags::kFlagResponseExpected, inResponseExpected); + SetWaitingForResponseOrAck(inResponseExpected); } void ExchangeContext::UseSuggestedResponseTimeout(Timeout applicationProcessingTimeout) @@ -489,6 +490,9 @@ void ExchangeContext::NotifyResponseTimeout(bool aCloseIfNeeded) } #endif // CONFIG_DEVICE_LAYER && CHIP_CONFIG_ENABLE_ICD_SERVER + // Grab the value of WaitingForResponseOrAck() before we mess with our state. + bool mrpTimedOut = WaitingForResponseOrAck(); + SetResponseExpected(false); // Hold a ref to ourselves so we can make calls into our delegate that might @@ -502,9 +506,8 @@ void ExchangeContext::NotifyResponseTimeout(bool aCloseIfNeeded) { // If we timed out _after_ getting an ack for the message, that means // the session is probably fine (since our message and the ack got - // through), so don't mark the session defunct unless we have an - // un-acked message here. - if (IsMessageNotAcked()) + // through), so don't mark the session defunct unless MRP also timed out. + if (mrpTimedOut) { if (mSession->IsSecureSession() && mSession->AsSecureSession()->IsCASESession()) { @@ -596,7 +599,7 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload return CHIP_NO_ERROR; } - if (IsMessageNotAcked()) + if (IsWaitingForAck()) { // The only way we can get here is a spec violation on the other side: // we sent a message that needs an ack, and the other side responded diff --git a/src/messaging/ReliableMessageContext.cpp b/src/messaging/ReliableMessageContext.cpp index ab717e34259111..4d3139d6945d2a 100644 --- a/src/messaging/ReliableMessageContext.cpp +++ b/src/messaging/ReliableMessageContext.cpp @@ -55,14 +55,14 @@ ReliableMessageMgr * ReliableMessageContext::GetReliableMessageMgr() return static_cast(this)->GetExchangeMgr()->GetReliableMessageMgr(); } -void ReliableMessageContext::SetMessageNotAcked(bool messageNotAcked) +void ReliableMessageContext::SetWaitingForAck(bool waitingForAck) { - mFlags.Set(Flags::kFlagMessageNotAcked, messageNotAcked); + mFlags.Set(Flags::kFlagWaitingForAck, waitingForAck); #if CONFIG_DEVICE_LAYER && CHIP_CONFIG_ENABLE_ICD_SERVER DeviceLayer::ChipDeviceEvent event; event.Type = DeviceLayer::DeviceEventType::kICDMsgAckSyncEvent; - event.AckSync.awaitingAck = messageNotAcked; + event.AckSync.awaitingAck = waitingForAck; CHIP_ERROR status = DeviceLayer::PlatformMgr().PostEvent(&event); if (status != CHIP_NO_ERROR) { @@ -103,7 +103,11 @@ CHIP_ERROR ReliableMessageContext::FlushAcks() void ReliableMessageContext::HandleRcvdAck(uint32_t ackMessageCounter) { // Msg is an Ack; Check Retrans Table and remove message context - if (!GetReliableMessageMgr()->CheckAndRemRetransTable(this, ackMessageCounter)) + if (GetReliableMessageMgr()->CheckAndRemRetransTable(this, ackMessageCounter)) + { + SetWaitingForResponseOrAck(false); + } + else { // This can happen quite easily due to a packet with a piggyback ack // being lost and retransmitted. diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index 2ee9fe50a2f763..7fb90bfcd54c36 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -114,11 +114,11 @@ class ReliableMessageContext */ bool IsAckPending() const; - /// Determine whether there is message hasn't been acknowledged. - bool IsMessageNotAcked() const; + /// Determine whether the reliable message context is waiting for an ack. + bool IsWaitingForAck() const; - /// Set whether there is a message hasn't been acknowledged. - void SetMessageNotAcked(bool messageNotAcked); + /// Set whether the reliable message context is waiting for an ack. + void SetWaitingForAck(bool waitingForAck); /// Set if this exchange is requesting Sleepy End Device active mode void SetRequestingActiveMode(bool activeMode); @@ -136,6 +136,9 @@ class ReliableMessageContext ReliableMessageMgr * GetReliableMessageMgr(); protected: + bool WaitingForResponseOrAck() const; + void SetWaitingForResponseOrAck(bool waitingForResponseOrAck); + enum class Flags : uint16_t { /// When set, signifies that this context is the initiator of the exchange. @@ -147,8 +150,10 @@ class ReliableMessageContext /// When set, automatically request an acknowledgment whenever a message is sent via UDP. kFlagAutoRequestAck = (1u << 2), - /// When set, signifies there is a message which hasn't been acknowledged. - kFlagMessageNotAcked = (1u << 3), + /// When set, signifies the reliable message context is waiting for an + /// ack: a message that needs an ack has been sent, no ack has been + /// received, and we have not yet run out of MRP retries. + kFlagWaitingForAck = (1u << 3), /// When set, signifies that there is an acknowledgment pending to be sent back. kFlagAckPending = (1u << 4), @@ -172,6 +177,13 @@ class ReliableMessageContext /// When set, ignore session being released, because we are releasing it ourselves. kFlagIgnoreSessionRelease = (1u << 10), + + /// When set: + /// + /// (1) We sent a message that expected a response (hence + /// IsResponseExpected() is true). + /// (2) We have received neither a response nor an ack for that message. + kFlagWaitingForResponseOrAck = (1u << 11), }; BitFlags mFlags; // Internal state flags @@ -216,9 +228,9 @@ inline bool ReliableMessageContext::IsAckPending() const return mFlags.Has(Flags::kFlagAckPending); } -inline bool ReliableMessageContext::IsMessageNotAcked() const +inline bool ReliableMessageContext::IsWaitingForAck() const { - return mFlags.Has(Flags::kFlagMessageNotAcked); + return mFlags.Has(Flags::kFlagWaitingForAck); } inline bool ReliableMessageContext::HasPiggybackAckPending() const @@ -251,5 +263,15 @@ inline bool ReliableMessageContext::IsEphemeralExchange() const return mFlags.Has(Flags::kFlagEphemeralExchange); } +inline bool ReliableMessageContext::WaitingForResponseOrAck() const +{ + return mFlags.Has(Flags::kFlagWaitingForResponseOrAck); +} + +inline void ReliableMessageContext::SetWaitingForResponseOrAck(bool waitingForResponseOrAck) +{ + mFlags.Set(Flags::kFlagWaitingForResponseOrAck, waitingForResponseOrAck); +} + } // namespace Messaging } // namespace chip diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index 68818585df3075..af5f3878e3cb03 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -49,12 +49,12 @@ namespace Messaging { ReliableMessageMgr::RetransTableEntry::RetransTableEntry(ReliableMessageContext * rc) : ec(*rc->GetExchangeContext()), nextRetransTime(0), sendCount(0) { - ec->SetMessageNotAcked(true); + ec->SetWaitingForAck(true); } ReliableMessageMgr::RetransTableEntry::~RetransTableEntry() { - ec->SetMessageNotAcked(false); + ec->SetWaitingForAck(false); } ReliableMessageMgr::ReliableMessageMgr(ObjectPool & contextPool) : @@ -158,12 +158,6 @@ void ReliableMessageMgr::ExecuteActions() // Do not StartTimer, we will schedule the timer at the end of the timer handler. mRetransTable.ReleaseObject(entry); - // Dropping our entry marked the exchange as not having an un-acked - // message... but of course it _does_ have an un-acked message and - // we have just given up on waiting for the ack. - - ec->GetReliableMessageContext()->SetMessageNotAcked(true); - return Loop::Continue; } @@ -204,7 +198,7 @@ void ReliableMessageMgr::Timeout(System::Layer * aSystemLayer, void * aAppState) CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, RetransTableEntry ** rEntry) { - VerifyOrDie(!rc->IsMessageNotAcked()); + VerifyOrDie(!rc->IsWaitingForAck()); *rEntry = mRetransTable.CreateObject(rc); if (*rEntry == nullptr) diff --git a/src/messaging/tests/MessagingContext.cpp b/src/messaging/tests/MessagingContext.cpp index f08b2d251bed2f..a7cf5369cccef1 100644 --- a/src/messaging/tests/MessagingContext.cpp +++ b/src/messaging/tests/MessagingContext.cpp @@ -163,12 +163,26 @@ CHIP_ERROR MessagingContext::CreateSessionBobToAlice() mBobFabricIndex, mAliceAddress, CryptoContext::SessionRole::kInitiator); } +CHIP_ERROR MessagingContext::CreateCASESessionBobToAlice() +{ + return mSessionManager.InjectCaseSessionWithTestKey(mSessionBobToAlice, kBobKeyId, kAliceKeyId, GetBobFabric()->GetNodeId(), + GetAliceFabric()->GetNodeId(), mBobFabricIndex, mAliceAddress, + CryptoContext::SessionRole::kInitiator); +} + CHIP_ERROR MessagingContext::CreateSessionAliceToBob() { return mSessionManager.InjectPaseSessionWithTestKey(mSessionAliceToBob, kAliceKeyId, GetBobFabric()->GetNodeId(), kBobKeyId, mAliceFabricIndex, mBobAddress, CryptoContext::SessionRole::kResponder); } +CHIP_ERROR MessagingContext::CreateCASESessionAliceToBob() +{ + return mSessionManager.InjectCaseSessionWithTestKey(mSessionAliceToBob, kAliceKeyId, kBobKeyId, GetAliceFabric()->GetNodeId(), + GetBobFabric()->GetNodeId(), mAliceFabricIndex, mBobAddress, + CryptoContext::SessionRole::kResponder); +} + CHIP_ERROR MessagingContext::CreatePASESessionCharlieToDavid() { return mSessionManager.InjectPaseSessionWithTestKey(mSessionCharlieToDavid, kCharlieKeyId, 0xdeadbeef, kDavidKeyId, diff --git a/src/messaging/tests/MessagingContext.h b/src/messaging/tests/MessagingContext.h index 156bb78bf3609e..ed6ead5d2da3ae 100644 --- a/src/messaging/tests/MessagingContext.h +++ b/src/messaging/tests/MessagingContext.h @@ -139,9 +139,11 @@ class MessagingContext : public PlatformMemoryUser const FabricInfo * GetAliceFabric() { return mFabricTable.FindFabricWithIndex(mAliceFabricIndex); } const FabricInfo * GetBobFabric() { return mFabricTable.FindFabricWithIndex(mBobFabricIndex); } - CHIP_ERROR CreateSessionBobToAlice(); - CHIP_ERROR CreateSessionAliceToBob(); - CHIP_ERROR CreateSessionBobToFriends(); + CHIP_ERROR CreateSessionBobToAlice(); // Creates PASE session + CHIP_ERROR CreateCASESessionBobToAlice(); + CHIP_ERROR CreateSessionAliceToBob(); // Creates PASE session + CHIP_ERROR CreateCASESessionAliceToBob(); + CHIP_ERROR CreateSessionBobToFriends(); // Creates PASE session CHIP_ERROR CreatePASESessionCharlieToDavid(); CHIP_ERROR CreatePASESessionDavidToCharlie(); diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index 7596071f07af8a..c8b318e3bd6c30 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -121,7 +121,7 @@ class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegat return CHIP_NO_ERROR; } - void OnResponseTimeout(ExchangeContext * ec) override {} + void OnResponseTimeout(ExchangeContext * ec) override { mResponseTimedOut = true; } void CloseExchangeIfNeeded() { @@ -147,6 +147,7 @@ class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegat bool IsOnMessageReceivedCalled = false; bool mReceivedPiggybackAck = false; bool mRetainExchange = false; + bool mResponseTimedOut = false; ExchangeContext * mExchange = nullptr; nlTestSuite * mTestSuite = nullptr; @@ -1685,6 +1686,297 @@ void CheckGetBackoff(nlTestSuite * inSuite, void * inContext) } } +void CheckApplicationResponseDelayed(nlTestSuite * inSuite, void * inContext) +{ + TestContext & ctx = *reinterpret_cast(inContext); + + CHIP_ERROR err = CHIP_NO_ERROR; + + // Make sure we are using CASE sessions, because there is no defunct-marking for PASE. + ctx.ExpireSessionBobToAlice(); + ctx.ExpireSessionAliceToBob(); + err = ctx.CreateCASESessionBobToAlice(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + err = ctx.CreateCASESessionAliceToBob(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + chip::System::PacketBufferHandle buffer = chip::MessagePacketBuffer::NewWithData(PAYLOAD, sizeof(PAYLOAD)); + NL_TEST_ASSERT(inSuite, !buffer.IsNull()); + + MockAppDelegate mockReceiver(ctx); + err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + mockReceiver.mTestSuite = inSuite; + mockReceiver.mRetainExchange = true; + + MockAppDelegate mockSender(ctx); + ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); + NL_TEST_ASSERT(inSuite, exchange != nullptr); + + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + NL_TEST_ASSERT(inSuite, rm != nullptr); + + exchange->GetSessionHandle()->AsSecureSession()->SetRemoteMRPConfig({ + 30_ms32, // CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL + 30_ms32, // CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL + }); + + constexpr uint32_t kMaxMRPTransmits = 5; // Counting the initial message. + + // Let's drop all but the last MRP transmit. + auto & loopback = ctx.GetLoopback(); + loopback.mSentMessageCount = 0; + loopback.mNumMessagesToDrop = kMaxMRPTransmits - 1; + loopback.mDroppedMessageCount = 0; + + // Ensure the retransmit table is empty right now + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); + + exchange->SetResponseTimeout(3000_ms32); + err = exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), SendMessageFlags::kExpectResponse); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + + // Ensure the message was dropped, and was added to retransmit table + NL_TEST_ASSERT(inSuite, loopback.mNumMessagesToDrop == kMaxMRPTransmits - 2); + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); + NL_TEST_ASSERT(inSuite, !mockReceiver.IsOnMessageReceivedCalled); + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); + + // Wait for all but the last retransmit to happen. + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mDroppedMessageCount >= kMaxMRPTransmits - 1; }); + ctx.DrainAndServiceIO(); + + // Ensure that nothing has been sent yet. + NL_TEST_ASSERT(inSuite, loopback.mNumMessagesToDrop == 0); + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); + NL_TEST_ASSERT(inSuite, !mockReceiver.IsOnMessageReceivedCalled); + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); + + // Now allow through the next message (our last retransmit), but make sure + // there is no standalone ack for it. + mockReceiver.SetDropAckResponse(true); + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mSentMessageCount >= kMaxMRPTransmits; }); + ctx.DrainAndServiceIO(); + + // Verify that message was sent and received but nothing else has been sent. + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); // We have no ack yet. + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got the message. + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); // We did not get a response. + + // Ensure there will be no more weirdness with acks and that our MRP timer is restarted properly. + mockReceiver.SetDropAckResponse(false); + + // Now send a response, but drop all but the last MRP retransmit. + loopback.mSentMessageCount = 0; + loopback.mNumMessagesToDrop = kMaxMRPTransmits - 1; + loopback.mDroppedMessageCount = 0; + + mockReceiver.mExchange->GetSessionHandle()->AsSecureSession()->SetRemoteMRPConfig({ + 30_ms32, // CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL + 30_ms32, // CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL + }); + + buffer = chip::MessagePacketBuffer::NewWithData(PAYLOAD, sizeof(PAYLOAD)); + NL_TEST_ASSERT(inSuite, !buffer.IsNull()); + + err = mockReceiver.mExchange->SendMessage(Echo::MsgType::EchoResponse, std::move(buffer)); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + + // At this point, we should have two MRP contexts pending. + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 2); // We have no ack yet. + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got original message. + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); // We did not get a response. + + // Now wait for all but the last retransmit to happen from the other side. + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mSentMessageCount >= kMaxMRPTransmits - 1; }); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + // We might have timed our MRP resends out, or not, but the other side is waiting for an ack. + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1 || rm->TestGetCountRetransTable() == 2); + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got original message. + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); // We did not get a response. + + // Now wait for us to time out our MRP context for sure. + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return rm->TestGetCountRetransTable() == 1; }); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); // We timed out our MRP context. + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got original message. + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); // We did not get a response. + NL_TEST_ASSERT(inSuite, !mockSender.mResponseTimedOut); // We did not time out yet. + + // Now wait for the last retransmit (and our ack) to to happen. + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mSentMessageCount >= kMaxMRPTransmits; }); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits + 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); // Everything has been acked. + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got original message. + NL_TEST_ASSERT(inSuite, mockSender.IsOnMessageReceivedCalled); // We got the response. + NL_TEST_ASSERT(inSuite, !mockSender.mResponseTimedOut); // We did not time out yet. + + mockReceiver.mTestSuite = nullptr; + + // Ensure that we did not mark any sessions defunct. + NL_TEST_ASSERT(inSuite, !ctx.GetSessionBobToAlice()->AsSecureSession()->IsDefunct()); + NL_TEST_ASSERT(inSuite, !ctx.GetSessionAliceToBob()->AsSecureSession()->IsDefunct()); + + // Reset our sessions, so other tests get the usual PASE session + ctx.ExpireSessionBobToAlice(); + ctx.ExpireSessionAliceToBob(); + err = ctx.CreateSessionBobToAlice(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + err = ctx.CreateSessionAliceToBob(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + err = ctx.GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); +} + +void CheckApplicationResponseNeverComes(nlTestSuite * inSuite, void * inContext) +{ + TestContext & ctx = *reinterpret_cast(inContext); + + CHIP_ERROR err = CHIP_NO_ERROR; + + // Make sure we are using CASE sessions, because there is no defunct-marking for PASE. + ctx.ExpireSessionBobToAlice(); + ctx.ExpireSessionAliceToBob(); + err = ctx.CreateCASESessionBobToAlice(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + err = ctx.CreateCASESessionAliceToBob(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + chip::System::PacketBufferHandle buffer = chip::MessagePacketBuffer::NewWithData(PAYLOAD, sizeof(PAYLOAD)); + NL_TEST_ASSERT(inSuite, !buffer.IsNull()); + + MockAppDelegate mockReceiver(ctx); + err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + mockReceiver.mTestSuite = inSuite; + + MockAppDelegate mockSender(ctx); + ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); + NL_TEST_ASSERT(inSuite, exchange != nullptr); + + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + NL_TEST_ASSERT(inSuite, rm != nullptr); + + exchange->GetSessionHandle()->AsSecureSession()->SetRemoteMRPConfig({ + 30_ms32, // CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL + 30_ms32, // CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL + }); + + constexpr uint32_t kMaxMRPTransmits = 5; // Counting the initial message. + + // Let's drop all but the last MRP transmit. + auto & loopback = ctx.GetLoopback(); + loopback.mSentMessageCount = 0; + loopback.mNumMessagesToDrop = kMaxMRPTransmits - 1; + loopback.mDroppedMessageCount = 0; + + // Ensure the retransmit table is empty right now + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); + + exchange->SetResponseTimeout(2500_ms32); + err = exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), SendMessageFlags::kExpectResponse); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + ctx.DrainAndServiceIO(); + + // Ensure the message was dropped, and was added to retransmit table + NL_TEST_ASSERT(inSuite, loopback.mNumMessagesToDrop == kMaxMRPTransmits - 2); + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); + NL_TEST_ASSERT(inSuite, !mockReceiver.IsOnMessageReceivedCalled); + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); + + // Wait for all but the last retransmit to happen. + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mDroppedMessageCount >= kMaxMRPTransmits - 1; }); + ctx.DrainAndServiceIO(); + + // Ensure that nothing has been sent yet. + NL_TEST_ASSERT(inSuite, loopback.mNumMessagesToDrop == 0); + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); + NL_TEST_ASSERT(inSuite, !mockReceiver.IsOnMessageReceivedCalled); + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); + + // Now allow through the next message (our last retransmit), but make sure + // there is no standalone ack for it. + mockReceiver.SetDropAckResponse(true); + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mSentMessageCount >= kMaxMRPTransmits; }); + ctx.DrainAndServiceIO(); + + // Verify that message was sent and received but nothing else has been sent. + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); // We have no ack yet. + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got the message. + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); // We did not get a response. + + // Ensure there will be no more weirdness with acks and that our MRP timer is restarted properly. + mockReceiver.SetDropAckResponse(false); + + // Now wait for us to time out our MRP context. + ctx.GetIOContext().DriveIOUntil(1000_ms32, [&] { return rm->TestGetCountRetransTable() == 0; }); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); // We timed out our MRP context. + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got original message. + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); // We did not get a response. + NL_TEST_ASSERT(inSuite, !mockSender.mResponseTimedOut); // We did not time out yet. + + // Now wait for our exchange to time out. + ctx.GetIOContext().DriveIOUntil(3000_ms32, [&] { return mockSender.mResponseTimedOut; }); + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == kMaxMRPTransmits); + NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == kMaxMRPTransmits - 1); + NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); // We timed this out long ago. + NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); // Other side got original message. + NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled); // We never got a response. + NL_TEST_ASSERT(inSuite, mockSender.mResponseTimedOut); // We tiemd out + + mockReceiver.mTestSuite = nullptr; + + // We should have marked out session defunct. + NL_TEST_ASSERT(inSuite, ctx.GetSessionBobToAlice()->AsSecureSession()->IsDefunct()); + // Other side had no reason to mark its session defunct. + NL_TEST_ASSERT(inSuite, !ctx.GetSessionAliceToBob()->AsSecureSession()->IsDefunct()); + + // Reset our sessions, so other tests get the usual PASE session + ctx.ExpireSessionBobToAlice(); + ctx.ExpireSessionAliceToBob(); + err = ctx.CreateSessionBobToAlice(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + err = ctx.CreateSessionAliceToBob(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + err = ctx.GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); +} + int InitializeTestCase(void * inContext) { TestContext & ctx = *static_cast(inContext); @@ -1734,6 +2026,9 @@ const nlTest sTests[] = { 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_DEF("Test an application response that comes after MRP retransmits run out", CheckApplicationResponseDelayed), + NL_TEST_DEF("Test an application response that never comes, so MRP retransmits run out and then exchange times out", + CheckApplicationResponseNeverComes), NL_TEST_SENTINEL(), };