Plumb Peer DNS-SD advertisement for TCP support into SessionSetup co…

…ntext project-chip#30392 (project-chip#33481) * Plumb Peer DNS-SD advertisement for TCP support into SessionSetup context * Restyled by clang-format * Fix unit tests * Fix build error * address Boris comments * reflect mdns tcp support in session estabishment * Restyled by clang-format * Update src/app/OperationalSessionSetup.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/OperationalSessionSetup.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * fixing minor comments * Restyled by clang-format * add an extra check on tcp support * Restyled by clang-format * add a check on tcp server enablement * add peer info in log * Restyled by clang-format * update log message --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
austina-csa · Jul 10, 2024 · 30d6fd7 · 30d6fd7
1 parent d883136
commit 30d6fd7
Show file tree

Hide file tree

Showing 19 changed files with 205 additions and 56 deletions.
diff --git a/examples/chip-tool/commands/common/RemoteDataModelLogger.cpp b/examples/chip-tool/commands/common/RemoteDataModelLogger.cpp
@@ -241,7 +241,8 @@ CHIP_ERROR LogDiscoveredNodeData(const chip::Dnssd::CommissionNodeData & nodeDat
     value["rotatingIdLen"]      = static_cast<uint64_t>(commissionData.rotatingIdLen);
     value["pairingHint"]        = commissionData.pairingHint;
     value["pairingInstruction"] = commissionData.pairingInstruction;
-    value["supportsTcp"]        = resolutionData.supportsTcp;
+    value["supportsTcpClient"]  = resolutionData.supportsTcpClient;
+    value["supportsTcpServer"]  = resolutionData.supportsTcpServer;
     value["port"]               = resolutionData.port;
     value["numIPs"]             = static_cast<uint8_t>(resolutionData.numIPs);
 

diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h
@@ -53,7 +53,8 @@ class Resolve : public DiscoverCommand, public chip::AddressResolve::NodeListene
                         result.mrpRemoteConfig.mIdleRetransTimeout.count());
         ChipLogProgress(chipTool, "   MRP retry interval (active): %" PRIu32 "ms",
                         result.mrpRemoteConfig.mActiveRetransTimeout.count());
-        ChipLogProgress(chipTool, "   Supports TCP: %s", result.supportsTcp ? "yes" : "no");
+        ChipLogProgress(chipTool, "   Supports TCP Client: %s", result.supportsTcpClient ? "yes" : "no");
+        ChipLogProgress(chipTool, "   Supports TCP Server: %s", result.supportsTcpServer ? "yes" : "no");
         ChipLogProgress(chipTool, "   ICD is operating as: %s", result.isICDOperatingAsLIT ? "LIT" : "SIT");
         SetCommandExitStatus(CHIP_NO_ERROR);
     }

diff --git a/examples/fabric-admin/commands/common/RemoteDataModelLogger.cpp b/examples/fabric-admin/commands/common/RemoteDataModelLogger.cpp
@@ -241,7 +241,8 @@ CHIP_ERROR LogDiscoveredNodeData(const chip::Dnssd::CommissionNodeData & nodeDat
     value["rotatingIdLen"]      = static_cast<uint64_t>(commissionData.rotatingIdLen);
     value["pairingHint"]        = commissionData.pairingHint;
     value["pairingInstruction"] = commissionData.pairingInstruction;
-    value["supportsTcp"]        = resolutionData.supportsTcp;
+    value["supportsTcpClient"]  = resolutionData.supportsTcpClient;
+    value["supportsTcpServer"]  = resolutionData.supportsTcpServer;
     value["port"]               = resolutionData.port;
     value["numIPs"]             = static_cast<uint8_t>(resolutionData.numIPs);
 

diff --git a/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/discovery_commands.py b/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/discovery_commands.py
@@ -68,7 +68,8 @@
         <arg name="rotatingIdLen" type="int64u"/>
         <arg name="pairingHint" type="int16u"/>
         <arg name="pairingInstruction" type="char_string"/>
-        <arg name="supportsTcp" type="boolean"/>
+        <arg name="supportsTcpClient" type="boolean"/>
+        <arg name="supportsTcpServer" type="boolean"/>
         <arg name="numIPs" type="int8u"/>
         <arg name="port" type="int16u"/>
         <arg name="mrpRetryIntervalIdle" type="int32u" optional="true"/>

diff --git a/src/app/OperationalSessionSetup.cpp b/src/app/OperationalSessionSetup.cpp
@@ -197,8 +197,10 @@ void OperationalSessionSetup::Connect(Callback::Callback<OnDeviceConnected> * on
     Connect(onConnection, nullptr, onSetupFailure, transportPayloadCapability);
 }
 
-void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config)
+void OperationalSessionSetup::UpdateDeviceData(const ResolveResult & result)
 {
+    auto & config = result.mrpRemoteConfig;
+    auto addr     = result.address;
 #if CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
     // Make sure to clear out our reason for trying the next result first thing,
     // so it does not stick around in various error cases.
@@ -248,7 +250,7 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
         return;
     }
 
-    CHIP_ERROR err = EstablishConnection(config);
+    CHIP_ERROR err = EstablishConnection(result);
     LogErrorOnFailure(err);
     if (err == CHIP_NO_ERROR)
     {
@@ -292,15 +294,26 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
     // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
 }
 
-CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ReliableMessageProtocolConfig & config)
+CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ResolveResult & result)
 {
+    auto & config = result.mrpRemoteConfig;
 #if INET_CONFIG_ENABLE_TCP_ENDPOINT
-    // TODO: Combine LargePayload flag with DNS-SD advertisements from peer.
-    // Issue #32348.
     if (mTransportPayloadCapability == TransportPayloadCapability::kLargePayload)
     {
-        // Set the transport type for carrying large payloads
-        mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
+        if (result.supportsTcpServer)
+        {
+            // Set the transport type for carrying large payloads
+            mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
+        }
+        else
+        {
+            // we should not set the large payload while the TCP support is not enabled
+            ChipLogError(
+                Discovery,
+                "LargePayload session requested but peer does not support TCP server, PeerNodeId=" ChipLogFormatScopedNodeId,
+                ChipLogValueScopedNodeId(mPeerId));
+            return CHIP_ERROR_INTERNAL;
+        }
     }
 #endif
 
@@ -627,7 +640,7 @@ void OperationalSessionSetup::PerformAddressUpdate()
 
 void OperationalSessionSetup::OnNodeAddressResolved(const PeerId & peerId, const ResolveResult & result)
 {
-    UpdateDeviceData(result.address, result.mrpRemoteConfig);
+    UpdateDeviceData(result);
 }
 
 void OperationalSessionSetup::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP_ERROR reason)

diff --git a/src/app/OperationalSessionSetup.h b/src/app/OperationalSessionSetup.h
@@ -347,7 +347,7 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
 
     void MoveToState(State aTargetState);
 
-    CHIP_ERROR EstablishConnection(const ReliableMessageProtocolConfig & config);
+    CHIP_ERROR EstablishConnection(const AddressResolve::ResolveResult & result);
 
     /*
      * This checks to see if an existing CASE session exists to the peer within the SessionManager
@@ -419,7 +419,7 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
     /**
      * This function will set new IP address, port and MRP retransmission intervals of the device.
      */
-    void UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config);
+    void UpdateDeviceData(const AddressResolve::ResolveResult & result);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
     /**

diff --git a/src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp b/src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp
@@ -117,7 +117,9 @@ void pychip_CommissionableNodeController_PrintDiscoveredCommissioners(
         {
             ChipLogProgress(Discovery, "\tMrp Interval active\tNot present");
         }
-        ChipLogProgress(Discovery, "\tSupports TCP\t\t%d", dnsSdInfo->supportsTcp);
+
+        ChipLogProgress(Discovery, "\tSupports TCP Client\t\t%d", dnsSdInfo->supportsTcpClient);
+        ChipLogProgress(Discovery, "\tSupports TCP Server\t\t%d", dnsSdInfo->supportsTcpServer);
 
         if (dnsSdInfo->isICDOperatingAsLIT.has_value())
         {

diff --git a/src/controller/python/ChipDeviceController-Discovery.cpp b/src/controller/python/ChipDeviceController-Discovery.cpp
@@ -142,7 +142,9 @@ void pychip_DeviceController_IterateDiscoveredCommissionableNodes(Controller::De
         {
             jsonVal["mrpRetryActiveThreshold"] = activeThreshold->count();
         }
-        jsonVal["supportsTcp"] = dnsSdInfo->supportsTcp;
+
+        jsonVal["supportsTcpClient"] = dnsSdInfo->supportsTcpClient;
+        jsonVal["supportsTcpServer"] = dnsSdInfo->supportsTcpServer;
         {
             Json::Value addresses;
             for (unsigned j = 0; j < dnsSdInfo->numIPs; ++j)

diff --git a/src/controller/python/chip/discovery/__init__.py b/src/controller/python/chip/discovery/__init__.py
@@ -92,7 +92,8 @@ class CommissionableNode():
     mrpRetryIntervalIdle: int = None
     mrpRetryIntervalActive: int = None
     mrpRetryActiveThreshold: int = None
-    supportsTcp: bool = None
+    supportsTcpClient: bool = None
+    supportsTcpServer: bool = None
     isICDOperatingAsLIT: bool = None
     addresses: List[str] = None
     rotatingId: Optional[str] = None

diff --git a/src/controller/python/chip/yaml/runner.py b/src/controller/python/chip/yaml/runner.py
@@ -913,7 +913,8 @@ def decode(self, result: _ActionResult):
                 'mrpRetryIntervalIdle': response.mrpRetryIntervalIdle,
                 'mrpRetryIntervalActive': response.mrpRetryIntervalActive,
                 'mrpRetryActiveThreshold': response.mrpRetryActiveThreshold,
-                'supportsTcp': response.supportsTcp,
+                'supportsTcpClient': response.supportsTcpClient,
+                'supportsTcpServer': response.supportsTcpServer,
                 'isICDOperatingAsLIT': response.isICDOperatingAsLIT,
                 'addresses': response.addresses,
                 'rotatingId': response.rotatingId,

diff --git a/src/lib/address_resolve/AddressResolve.h b/src/lib/address_resolve/AddressResolve.h
@@ -33,7 +33,8 @@ struct ResolveResult
 {
     Transport::PeerAddress address;
     ReliableMessageProtocolConfig mrpRemoteConfig;
-    bool supportsTcp         = false;
+    bool supportsTcpServer   = false;
+    bool supportsTcpClient   = false;
     bool isICDOperatingAsLIT = false;
 
     ResolveResult() : address(Transport::Type::kUdp), mrpRemoteConfig(GetDefaultMRPConfig()) {}

diff --git a/src/lib/address_resolve/AddressResolve_DefaultImpl.cpp b/src/lib/address_resolve/AddressResolve_DefaultImpl.cpp
@@ -287,8 +287,9 @@ void Resolver::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeDat
 
         result.address.SetPort(nodeData.resolutionData.port);
         result.address.SetInterface(nodeData.resolutionData.interfaceId);
-        result.mrpRemoteConfig = nodeData.resolutionData.GetRemoteMRPConfig();
-        result.supportsTcp     = nodeData.resolutionData.supportsTcp;
+        result.mrpRemoteConfig   = nodeData.resolutionData.GetRemoteMRPConfig();
+        result.supportsTcpClient = nodeData.resolutionData.supportsTcpClient;
+        result.supportsTcpServer = nodeData.resolutionData.supportsTcpServer;
 
         if (nodeData.resolutionData.isICDOperatingAsLIT.has_value())
         {

diff --git a/src/lib/address_resolve/tool.cpp b/src/lib/address_resolve/tool.cpp
@@ -56,7 +56,8 @@ class PrintOutNodeListener : public chip::AddressResolve::NodeListener
         result.address.ToString(addr_string);
 
         ChipLogProgress(Discovery, "Resolve completed: %s", addr_string);
-        ChipLogProgress(Discovery, "   Supports TCP:                  %s", result.supportsTcp ? "YES" : "NO");
+        ChipLogProgress(Discovery, " Supports TCP Client: %s", result.supportsTcpClient ? "YES" : "NO");
+        ChipLogProgress(Discovery, " Supports TCP Server: %s", result.supportsTcpServer ? "YES" : "NO");
         ChipLogProgress(Discovery, "   MRP IDLE retransmit timeout:   %u ms", result.mrpRemoteConfig.mIdleRetransTimeout.count());
         ChipLogProgress(Discovery, "   MRP ACTIVE retransmit timeout: %u ms", result.mrpRemoteConfig.mActiveRetransTimeout.count());
         ChipLogProgress(Discovery, "   MRP ACTIVE Threshold time:     %u ms", result.mrpRemoteConfig.mActiveThresholdTime.count());

diff --git a/src/lib/dnssd/TxtFields.cpp b/src/lib/dnssd/TxtFields.cpp
@@ -38,6 +38,9 @@ namespace Dnssd {
 
 namespace Internal {
 
+constexpr uint8_t kTCPClient = 1;
+constexpr uint8_t kTCPServer = 2;
+
 namespace {
 
 char SafeToLower(uint8_t ch)
@@ -276,9 +279,13 @@ void FillNodeDataFromTxt(const ByteSpan & key, const ByteSpan & value, CommonRes
     case TxtFieldKey::kSessionActiveThreshold:
         nodeData.mrpRetryActiveThreshold = Internal::GetRetryActiveThreshold(value);
         break;
-    case TxtFieldKey::kTcpSupported:
-        nodeData.supportsTcp = Internal::MakeBoolFromAsciiDecimal(value);
+    case TxtFieldKey::kTcpSupported: {
+        // bit 0 is reserved and deprecated
+        uint8_t support            = Internal::MakeU8FromAsciiDecimal(value);
+        nodeData.supportsTcpClient = (support & (1 << Internal::kTCPClient)) != 0;
+        nodeData.supportsTcpServer = (support & (1 << Internal::kTCPServer)) != 0;
         break;
+    }
     case TxtFieldKey::kLongIdleTimeICD:
         nodeData.isICDOperatingAsLIT = Internal::MakeOptionalBoolFromAsciiDecimal(value);
         break;

diff --git a/src/lib/dnssd/Types.h b/src/lib/dnssd/Types.h
@@ -90,7 +90,8 @@ struct CommonResolutionData
 
     uint16_t port                         = 0;
     char hostName[kHostNameMaxLength + 1] = {};
-    bool supportsTcp                      = false;
+    bool supportsTcpClient                = false;
+    bool supportsTcpServer                = false;
     std::optional<bool> isICDOperatingAsLIT;
     std::optional<System::Clock::Milliseconds32> mrpRetryIntervalIdle;
     std::optional<System::Clock::Milliseconds32> mrpRetryIntervalActive;
@@ -131,7 +132,8 @@ struct CommonResolutionData
         isICDOperatingAsLIT     = std::nullopt;
         numIPs                  = 0;
         port                    = 0;
-        supportsTcp             = false;
+        supportsTcpClient       = false;
+        supportsTcpServer       = false;
         interfaceId             = Inet::InterfaceId::Null();
         for (auto & addr : ipAddress)
         {
@@ -181,7 +183,8 @@ struct CommonResolutionData
         {
             ChipLogDetail(Discovery, "\tMrp Active Threshold: not present");
         }
-        ChipLogDetail(Discovery, "\tTCP Supported: %d", supportsTcp);
+        ChipLogDetail(Discovery, "\tTCP Client Supported: %d", supportsTcpClient);
+        ChipLogDetail(Discovery, "\tTCP Server Supported: %d", supportsTcpServer);
         if (isICDOperatingAsLIT.has_value())
         {
             ChipLogDetail(Discovery, "\tThe ICD operates in %s", *isICDOperatingAsLIT ? "LIT" : "SIT");

diff --git a/src/lib/dnssd/tests/TestIncrementalResolve.cpp b/src/lib/dnssd/tests/TestIncrementalResolve.cpp
@@ -308,7 +308,8 @@ TEST(TestIncrementalResolve, TestParseOperational)
     EXPECT_FALSE(nodeData.operationalData.hasZeroTTL);
     EXPECT_EQ(nodeData.resolutionData.numIPs, 1u);
     EXPECT_EQ(nodeData.resolutionData.port, 0x1234);
-    EXPECT_FALSE(nodeData.resolutionData.supportsTcp);
+    EXPECT_FALSE(nodeData.resolutionData.supportsTcpServer);
+    EXPECT_FALSE(nodeData.resolutionData.supportsTcpClient);
     EXPECT_FALSE(nodeData.resolutionData.GetMrpRetryIntervalActive().has_value());
     EXPECT_EQ(nodeData.resolutionData.GetMrpRetryIntervalIdle(), std::make_optional(chip::System::Clock::Milliseconds32(23)));
 
@@ -396,7 +397,8 @@ TEST(TestIncrementalResolve, TestParseCommissionable)
     // validate data as it was passed in
     EXPECT_EQ(nodeData.numIPs, 2u);
     EXPECT_EQ(nodeData.port, 0x1234);
-    EXPECT_FALSE(nodeData.supportsTcp);
+    EXPECT_FALSE(nodeData.supportsTcpClient);
+    EXPECT_FALSE(nodeData.supportsTcpServer);
     EXPECT_EQ(nodeData.GetMrpRetryIntervalActive(), std::make_optional(chip::System::Clock::Milliseconds32(321)));
     EXPECT_FALSE(nodeData.GetMrpRetryIntervalIdle().has_value());