Skip to content

Commit 9d3d8d2

Browse files
hnnajhrestyled-commitsbzbarsky-apple
authored
Plumb Peer DNS-SD advertisement for TCP support into SessionSetup context #30392 (#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>
1 parent 2bab37b commit 9d3d8d2

File tree

19 files changed

+205
-56
lines changed

19 files changed

+205
-56
lines changed

‎examples/chip-tool/commands/common/RemoteDataModelLogger.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,8 @@ CHIP_ERROR LogDiscoveredNodeData(const chip::Dnssd::CommissionNodeData & nodeDat
241241
value["rotatingIdLen"] = static_cast<uint64_t>(commissionData.rotatingIdLen);
242242
value["pairingHint"] = commissionData.pairingHint;
243243
value["pairingInstruction"] = commissionData.pairingInstruction;
244-
value["supportsTcp"] = resolutionData.supportsTcp;
244+
value["supportsTcpClient"] = resolutionData.supportsTcpClient;
245+
value["supportsTcpServer"] = resolutionData.supportsTcpServer;
245246
value["port"] = resolutionData.port;
246247
value["numIPs"] = static_cast<uint8_t>(resolutionData.numIPs);
247248

‎examples/chip-tool/commands/discover/Commands.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ class Resolve : public DiscoverCommand, public chip::AddressResolve::NodeListene
5353
result.mrpRemoteConfig.mIdleRetransTimeout.count());
5454
ChipLogProgress(chipTool, " MRP retry interval (active): %" PRIu32 "ms",
5555
result.mrpRemoteConfig.mActiveRetransTimeout.count());
56-
ChipLogProgress(chipTool, " Supports TCP: %s", result.supportsTcp ? "yes" : "no");
56+
ChipLogProgress(chipTool, " Supports TCP Client: %s", result.supportsTcpClient ? "yes" : "no");
57+
ChipLogProgress(chipTool, " Supports TCP Server: %s", result.supportsTcpServer ? "yes" : "no");
5758
ChipLogProgress(chipTool, " ICD is operating as: %s", result.isICDOperatingAsLIT ? "LIT" : "SIT");
5859
SetCommandExitStatus(CHIP_NO_ERROR);
5960
}

‎examples/fabric-admin/commands/common/RemoteDataModelLogger.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,8 @@ CHIP_ERROR LogDiscoveredNodeData(const chip::Dnssd::CommissionNodeData & nodeDat
241241
value["rotatingIdLen"] = static_cast<uint64_t>(commissionData.rotatingIdLen);
242242
value["pairingHint"] = commissionData.pairingHint;
243243
value["pairingInstruction"] = commissionData.pairingInstruction;
244-
value["supportsTcp"] = resolutionData.supportsTcp;
244+
value["supportsTcpClient"] = resolutionData.supportsTcpClient;
245+
value["supportsTcpServer"] = resolutionData.supportsTcpServer;
245246
value["port"] = resolutionData.port;
246247
value["numIPs"] = static_cast<uint8_t>(resolutionData.numIPs);
247248

‎scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/discovery_commands.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@
6868
<arg name="rotatingIdLen" type="int64u"/>
6969
<arg name="pairingHint" type="int16u"/>
7070
<arg name="pairingInstruction" type="char_string"/>
71-
<arg name="supportsTcp" type="boolean"/>
71+
<arg name="supportsTcpClient" type="boolean"/>
72+
<arg name="supportsTcpServer" type="boolean"/>
7273
<arg name="numIPs" type="int8u"/>
7374
<arg name="port" type="int16u"/>
7475
<arg name="mrpRetryIntervalIdle" type="int32u" optional="true"/>

‎src/app/OperationalSessionSetup.cpp

+21-8
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,10 @@ void OperationalSessionSetup::Connect(Callback::Callback<OnDeviceConnected> * on
197197
Connect(onConnection, nullptr, onSetupFailure, transportPayloadCapability);
198198
}
199199

200-
void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config)
200+
void OperationalSessionSetup::UpdateDeviceData(const ResolveResult & result)
201201
{
202+
auto & config = result.mrpRemoteConfig;
203+
auto addr = result.address;
202204
#if CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
203205
// Make sure to clear out our reason for trying the next result first thing,
204206
// so it does not stick around in various error cases.
@@ -248,7 +250,7 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
248250
return;
249251
}
250252

251-
CHIP_ERROR err = EstablishConnection(config);
253+
CHIP_ERROR err = EstablishConnection(result);
252254
LogErrorOnFailure(err);
253255
if (err == CHIP_NO_ERROR)
254256
{
@@ -292,15 +294,26 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
292294
// Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
293295
}
294296

295-
CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ReliableMessageProtocolConfig & config)
297+
CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ResolveResult & result)
296298
{
299+
auto & config = result.mrpRemoteConfig;
297300
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
298-
// TODO: Combine LargePayload flag with DNS-SD advertisements from peer.
299-
// Issue #32348.
300301
if (mTransportPayloadCapability == TransportPayloadCapability::kLargePayload)
301302
{
302-
// Set the transport type for carrying large payloads
303-
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
303+
if (result.supportsTcpServer)
304+
{
305+
// Set the transport type for carrying large payloads
306+
mDeviceAddress.SetTransportType(chip::Transport::Type::kTcp);
307+
}
308+
else
309+
{
310+
// we should not set the large payload while the TCP support is not enabled
311+
ChipLogError(
312+
Discovery,
313+
"LargePayload session requested but peer does not support TCP server, PeerNodeId=" ChipLogFormatScopedNodeId,
314+
ChipLogValueScopedNodeId(mPeerId));
315+
return CHIP_ERROR_INTERNAL;
316+
}
304317
}
305318
#endif
306319

@@ -627,7 +640,7 @@ void OperationalSessionSetup::PerformAddressUpdate()
627640

628641
void OperationalSessionSetup::OnNodeAddressResolved(const PeerId & peerId, const ResolveResult & result)
629642
{
630-
UpdateDeviceData(result.address, result.mrpRemoteConfig);
643+
UpdateDeviceData(result);
631644
}
632645

633646
void OperationalSessionSetup::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP_ERROR reason)

‎src/app/OperationalSessionSetup.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
347347

348348
void MoveToState(State aTargetState);
349349

350-
CHIP_ERROR EstablishConnection(const ReliableMessageProtocolConfig & config);
350+
CHIP_ERROR EstablishConnection(const AddressResolve::ResolveResult & result);
351351

352352
/*
353353
* 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,
419419
/**
420420
* This function will set new IP address, port and MRP retransmission intervals of the device.
421421
*/
422-
void UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config);
422+
void UpdateDeviceData(const AddressResolve::ResolveResult & result);
423423

424424
#if CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
425425
/**

‎src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,9 @@ void pychip_CommissionableNodeController_PrintDiscoveredCommissioners(
117117
{
118118
ChipLogProgress(Discovery, "\tMrp Interval active\tNot present");
119119
}
120-
ChipLogProgress(Discovery, "\tSupports TCP\t\t%d", dnsSdInfo->supportsTcp);
120+
121+
ChipLogProgress(Discovery, "\tSupports TCP Client\t\t%d", dnsSdInfo->supportsTcpClient);
122+
ChipLogProgress(Discovery, "\tSupports TCP Server\t\t%d", dnsSdInfo->supportsTcpServer);
121123

122124
if (dnsSdInfo->isICDOperatingAsLIT.has_value())
123125
{

‎src/controller/python/ChipDeviceController-Discovery.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,9 @@ void pychip_DeviceController_IterateDiscoveredCommissionableNodes(Controller::De
142142
{
143143
jsonVal["mrpRetryActiveThreshold"] = activeThreshold->count();
144144
}
145-
jsonVal["supportsTcp"] = dnsSdInfo->supportsTcp;
145+
146+
jsonVal["supportsTcpClient"] = dnsSdInfo->supportsTcpClient;
147+
jsonVal["supportsTcpServer"] = dnsSdInfo->supportsTcpServer;
146148
{
147149
Json::Value addresses;
148150
for (unsigned j = 0; j < dnsSdInfo->numIPs; ++j)

‎src/controller/python/chip/discovery/__init__.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ class CommissionableNode():
9292
mrpRetryIntervalIdle: int = None
9393
mrpRetryIntervalActive: int = None
9494
mrpRetryActiveThreshold: int = None
95-
supportsTcp: bool = None
95+
supportsTcpClient: bool = None
96+
supportsTcpServer: bool = None
9697
isICDOperatingAsLIT: bool = None
9798
addresses: List[str] = None
9899
rotatingId: Optional[str] = None

‎src/controller/python/chip/yaml/runner.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,8 @@ def decode(self, result: _ActionResult):
913913
'mrpRetryIntervalIdle': response.mrpRetryIntervalIdle,
914914
'mrpRetryIntervalActive': response.mrpRetryIntervalActive,
915915
'mrpRetryActiveThreshold': response.mrpRetryActiveThreshold,
916-
'supportsTcp': response.supportsTcp,
916+
'supportsTcpClient': response.supportsTcpClient,
917+
'supportsTcpServer': response.supportsTcpServer,
917918
'isICDOperatingAsLIT': response.isICDOperatingAsLIT,
918919
'addresses': response.addresses,
919920
'rotatingId': response.rotatingId,

‎src/lib/address_resolve/AddressResolve.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ struct ResolveResult
3333
{
3434
Transport::PeerAddress address;
3535
ReliableMessageProtocolConfig mrpRemoteConfig;
36-
bool supportsTcp = false;
36+
bool supportsTcpServer = false;
37+
bool supportsTcpClient = false;
3738
bool isICDOperatingAsLIT = false;
3839

3940
ResolveResult() : address(Transport::Type::kUdp), mrpRemoteConfig(GetDefaultMRPConfig()) {}

‎src/lib/address_resolve/AddressResolve_DefaultImpl.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,9 @@ void Resolver::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeDat
287287

288288
result.address.SetPort(nodeData.resolutionData.port);
289289
result.address.SetInterface(nodeData.resolutionData.interfaceId);
290-
result.mrpRemoteConfig = nodeData.resolutionData.GetRemoteMRPConfig();
291-
result.supportsTcp = nodeData.resolutionData.supportsTcp;
290+
result.mrpRemoteConfig = nodeData.resolutionData.GetRemoteMRPConfig();
291+
result.supportsTcpClient = nodeData.resolutionData.supportsTcpClient;
292+
result.supportsTcpServer = nodeData.resolutionData.supportsTcpServer;
292293

293294
if (nodeData.resolutionData.isICDOperatingAsLIT.has_value())
294295
{

‎src/lib/address_resolve/tool.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ class PrintOutNodeListener : public chip::AddressResolve::NodeListener
5656
result.address.ToString(addr_string);
5757

5858
ChipLogProgress(Discovery, "Resolve completed: %s", addr_string);
59-
ChipLogProgress(Discovery, " Supports TCP: %s", result.supportsTcp ? "YES" : "NO");
59+
ChipLogProgress(Discovery, " Supports TCP Client: %s", result.supportsTcpClient ? "YES" : "NO");
60+
ChipLogProgress(Discovery, " Supports TCP Server: %s", result.supportsTcpServer ? "YES" : "NO");
6061
ChipLogProgress(Discovery, " MRP IDLE retransmit timeout: %u ms", result.mrpRemoteConfig.mIdleRetransTimeout.count());
6162
ChipLogProgress(Discovery, " MRP ACTIVE retransmit timeout: %u ms", result.mrpRemoteConfig.mActiveRetransTimeout.count());
6263
ChipLogProgress(Discovery, " MRP ACTIVE Threshold time: %u ms", result.mrpRemoteConfig.mActiveThresholdTime.count());

‎src/lib/dnssd/TxtFields.cpp

+9-2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ namespace Dnssd {
3838

3939
namespace Internal {
4040

41+
constexpr uint8_t kTCPClient = 1;
42+
constexpr uint8_t kTCPServer = 2;
43+
4144
namespace {
4245

4346
char SafeToLower(uint8_t ch)
@@ -276,9 +279,13 @@ void FillNodeDataFromTxt(const ByteSpan & key, const ByteSpan & value, CommonRes
276279
case TxtFieldKey::kSessionActiveThreshold:
277280
nodeData.mrpRetryActiveThreshold = Internal::GetRetryActiveThreshold(value);
278281
break;
279-
case TxtFieldKey::kTcpSupported:
280-
nodeData.supportsTcp = Internal::MakeBoolFromAsciiDecimal(value);
282+
case TxtFieldKey::kTcpSupported: {
283+
// bit 0 is reserved and deprecated
284+
uint8_t support = Internal::MakeU8FromAsciiDecimal(value);
285+
nodeData.supportsTcpClient = (support & (1 << Internal::kTCPClient)) != 0;
286+
nodeData.supportsTcpServer = (support & (1 << Internal::kTCPServer)) != 0;
281287
break;
288+
}
282289
case TxtFieldKey::kLongIdleTimeICD:
283290
nodeData.isICDOperatingAsLIT = Internal::MakeOptionalBoolFromAsciiDecimal(value);
284291
break;

‎src/lib/dnssd/Types.h

+6-3
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ struct CommonResolutionData
9090

9191
uint16_t port = 0;
9292
char hostName[kHostNameMaxLength + 1] = {};
93-
bool supportsTcp = false;
93+
bool supportsTcpClient = false;
94+
bool supportsTcpServer = false;
9495
std::optional<bool> isICDOperatingAsLIT;
9596
std::optional<System::Clock::Milliseconds32> mrpRetryIntervalIdle;
9697
std::optional<System::Clock::Milliseconds32> mrpRetryIntervalActive;
@@ -131,7 +132,8 @@ struct CommonResolutionData
131132
isICDOperatingAsLIT = std::nullopt;
132133
numIPs = 0;
133134
port = 0;
134-
supportsTcp = false;
135+
supportsTcpClient = false;
136+
supportsTcpServer = false;
135137
interfaceId = Inet::InterfaceId::Null();
136138
for (auto & addr : ipAddress)
137139
{
@@ -181,7 +183,8 @@ struct CommonResolutionData
181183
{
182184
ChipLogDetail(Discovery, "\tMrp Active Threshold: not present");
183185
}
184-
ChipLogDetail(Discovery, "\tTCP Supported: %d", supportsTcp);
186+
ChipLogDetail(Discovery, "\tTCP Client Supported: %d", supportsTcpClient);
187+
ChipLogDetail(Discovery, "\tTCP Server Supported: %d", supportsTcpServer);
185188
if (isICDOperatingAsLIT.has_value())
186189
{
187190
ChipLogDetail(Discovery, "\tThe ICD operates in %s", *isICDOperatingAsLIT ? "LIT" : "SIT");

‎src/lib/dnssd/tests/TestIncrementalResolve.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,8 @@ TEST(TestIncrementalResolve, TestParseOperational)
308308
EXPECT_FALSE(nodeData.operationalData.hasZeroTTL);
309309
EXPECT_EQ(nodeData.resolutionData.numIPs, 1u);
310310
EXPECT_EQ(nodeData.resolutionData.port, 0x1234);
311-
EXPECT_FALSE(nodeData.resolutionData.supportsTcp);
311+
EXPECT_FALSE(nodeData.resolutionData.supportsTcpServer);
312+
EXPECT_FALSE(nodeData.resolutionData.supportsTcpClient);
312313
EXPECT_FALSE(nodeData.resolutionData.GetMrpRetryIntervalActive().has_value());
313314
EXPECT_EQ(nodeData.resolutionData.GetMrpRetryIntervalIdle(), std::make_optional(chip::System::Clock::Milliseconds32(23)));
314315

@@ -396,7 +397,8 @@ TEST(TestIncrementalResolve, TestParseCommissionable)
396397
// validate data as it was passed in
397398
EXPECT_EQ(nodeData.numIPs, 2u);
398399
EXPECT_EQ(nodeData.port, 0x1234);
399-
EXPECT_FALSE(nodeData.supportsTcp);
400+
EXPECT_FALSE(nodeData.supportsTcpClient);
401+
EXPECT_FALSE(nodeData.supportsTcpServer);
400402
EXPECT_EQ(nodeData.GetMrpRetryIntervalActive(), std::make_optional(chip::System::Clock::Milliseconds32(321)));
401403
EXPECT_FALSE(nodeData.GetMrpRetryIntervalIdle().has_value());
402404

0 commit comments

Comments
 (0)