Disable CRMP if we send/receive messages not over UDP transport (#6407)

* Disable CRMP if we send/receive messages not over UDP transport * Declare reliableTransmissionRequested when it is first use
project-chip · Jun 7, 2021 · 5241395 · 5241395
1 parent 3770818
commit 5241395
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 10 deletions.
diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp
@@ -123,7 +123,17 @@ CHIP_ERROR ExchangeContext::SendMessageImpl(Protocols::Id protocolId, uint8_t ms
     // an error arising below. at the end, we have to close it.
     Retain();
 
-    bool reliableTransmissionRequested = !sendFlags.Has(SendMessageFlags::kNoAutoRequestAck);
+    bool reliableTransmissionRequested = true;
+
+    // If sending via UDP and NoAutoRequestAck send flag is not specificed, request reliable transmission.
+    if (state && state->GetPeerAddress().GetTransportType() != Transport::Type::kUdp)
+    {
+        reliableTransmissionRequested = false;
+    }
+    else
+    {
+        reliableTransmissionRequested = !sendFlags.Has(SendMessageFlags::kNoAutoRequestAck);
+    }
 
     ExchangeMessageDispatch * dispatch = GetMessageDispatch();
     ApplicationExchangeDispatch defaultDispatch;

diff --git a/src/messaging/ExchangeMessageDispatch.cpp b/src/messaging/ExchangeMessageDispatch.cpp
@@ -65,7 +65,7 @@ CHIP_ERROR ExchangeMessageDispatch::SendMessage(SecureSessionHandle session, uin
 #endif
     }
 
-    if (!IsTransportReliable() && reliableMessageContext->AutoRequestAck() && mReliableMessageMgr != nullptr &&
+    if (IsReliableTransmissionAllowed() && reliableMessageContext->AutoRequestAck() && mReliableMessageMgr != nullptr &&
         isReliableTransmission)
     {
         payloadHeader.SetNeedsAck(true);
@@ -105,7 +105,7 @@ CHIP_ERROR ExchangeMessageDispatch::OnMessageReceived(const PayloadHeader & payl
     ReturnErrorCodeIf(!MessagePermitted(payloadHeader.GetProtocolID().GetProtocolId(), payloadHeader.GetMessageType()),
                       CHIP_ERROR_INVALID_ARGUMENT);
 
-    if (!IsTransportReliable())
+    if (IsReliableTransmissionAllowed())
     {
         if (payloadHeader.IsAckMsg() && payloadHeader.GetAckId().HasValue())
         {

diff --git a/src/messaging/ExchangeMessageDispatch.h b/src/messaging/ExchangeMessageDispatch.h
@@ -63,7 +63,7 @@ class ExchangeMessageDispatch
     virtual CHIP_ERROR SendMessageImpl(SecureSessionHandle session, PayloadHeader & payloadHeader,
                                        System::PacketBufferHandle && message, EncryptedPacketBufferHandle * retainedMessage) = 0;
 
-    virtual bool IsTransportReliable() { return false; }
+    virtual bool IsReliableTransmissionAllowed() { return true; }
 
 private:
     ReliableMessageMgr * mReliableMessageMgr = nullptr;

diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp
@@ -344,11 +344,11 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry)
 
     VerifyOrReturnError(rc != nullptr, err);
 
-    const ExchangeMessageDispatch * transport = rc->GetExchangeContext()->GetMessageDispatch();
-    VerifyOrExit(transport != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
+    const ExchangeMessageDispatch * dispatcher = rc->GetExchangeContext()->GetMessageDispatch();
+    VerifyOrExit(dispatcher != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
 
     err =
-        transport->ResendMessage(rc->GetExchangeContext()->GetSecureSession(), std::move(entry->retainedBuf), &entry->retainedBuf);
+        dispatcher->ResendMessage(rc->GetExchangeContext()->GetSecureSession(), std::move(entry->retainedBuf), &entry->retainedBuf);
     SuccessOrExit(err);
 
     // Update the counters

diff --git a/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.h b/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.h
@@ -57,10 +57,10 @@ class SessionEstablishmentExchangeDispatch : public Messaging::ExchangeMessageDi
 
     bool MessagePermitted(uint16_t protocol, uint8_t type) override;
 
-    bool IsTransportReliable() override
+    bool IsReliableTransmissionAllowed() override
     {
-        // If the underlying transport is not UDP.
-        return (mPeerAddress.GetTransportType() != Transport::Type::kUdp);
+        // If the underlying transport is UDP.
+        return (mPeerAddress.GetTransportType() == Transport::Type::kUdp);
     }
 
 private: