From 412352318cb5bb0b6f1cbe078d35ee64571493ab Mon Sep 17 00:00:00 2001 From: Pradip De Date: Fri, 19 Jul 2024 19:21:16 -0700 Subject: [PATCH] Fix DNS-SD TCP advertisement to use a bitmap for Client and Server. (#34289) When TCP is enabled, a node, currently, advertises as both a Client and Server. --- src/app/server/Dnssd.cpp | 3 +++ src/app/server/Dnssd.h | 1 + src/lib/dnssd/Advertiser.h | 17 ++++++++++++----- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 6 +++--- src/lib/dnssd/Discovery_ImplPlatform.cpp | 3 ++- .../dnssd/minimal_mdns/tests/TestAdvertiser.cpp | 5 +++-- 6 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 3cdcd35f9a7e98..2fc56614e3ab9e 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -213,6 +213,9 @@ CHIP_ERROR DnssdServer::AdvertiseOperational() AddICDKeyToAdvertisement(advertiseParameters); #endif +#if INET_CONFIG_ENABLE_TCP_ENDPOINT + advertiseParameters.SetTCPSupportModes(chip::Dnssd::TCPModeAdvertise::kTCPClientServer); +#endif auto & mdnsAdvertiser = chip::Dnssd::ServiceAdvertiser::Instance(); ChipLogProgress(Discovery, "Advertise operational node " ChipLogFormatX64 "-" ChipLogFormatX64, diff --git a/src/app/server/Dnssd.h b/src/app/server/Dnssd.h index 105318ca5a08be..3103a2010c03f6 100644 --- a/src/app/server/Dnssd.h +++ b/src/app/server/Dnssd.h @@ -96,6 +96,7 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver void SetICDManager(ICDManager * manager) { mICDManager = manager; }; #endif + /// Start operational advertising CHIP_ERROR AdvertiseOperational(); diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h index 02888181f98181..88b86ef871b91d 100644 --- a/src/lib/dnssd/Advertiser.h +++ b/src/lib/dnssd/Advertiser.h @@ -55,6 +55,14 @@ enum class ICDModeAdvertise : uint8_t kLIT, // The ICD is currently operating as a LIT. ICD=1 in DNS-SD key/value pairs. }; +enum class TCPModeAdvertise : uint16_t +{ + kNone = 0, // The device does not support TCP. + kTCPClient = 1 << 1, // The device supports the TCP client. + kTCPServer = 1 << 2, // The device supports the TCP server. + kTCPClientServer = (kTCPClient | kTCPServer), // The device supports both the TCP client and server. +}; + template class BaseAdvertisingParams { @@ -102,13 +110,12 @@ class BaseAdvertisingParams } const std::optional & GetLocalMRPConfig() const { return mLocalMRPConfig; } - // NOTE: The SetTcpSupported API is deprecated and not compliant with 1.3. T flag should not be set. - Derived & SetTcpSupported(std::optional tcpSupported) + Derived & SetTCPSupportModes(TCPModeAdvertise tcpSupportModes) { - mTcpSupported = tcpSupported; + mTcpSupportModes = tcpSupportModes; return *reinterpret_cast(this); } - std::optional GetTcpSupported() const { return mTcpSupported; } + TCPModeAdvertise GetTCPSupportModes() const { return mTcpSupportModes; } Derived & SetICDModeToAdvertise(ICDModeAdvertise operatingMode) { @@ -124,7 +131,7 @@ class BaseAdvertisingParams uint8_t mMacStorage[kMaxMacSize] = {}; size_t mMacLength = 0; std::optional mLocalMRPConfig; - std::optional mTcpSupported; + TCPModeAdvertise mTcpSupportModes = TCPModeAdvertise::kNone; ICDModeAdvertise mICDModeAdvertise = ICDModeAdvertise::kNone; }; diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 5ca002673cd4c9..ce46137c9a00e2 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -278,10 +278,10 @@ class AdvertiserMinMdns : public ServiceAdvertiser, } } - if (const auto & tcpSupported = params.GetTcpSupported(); tcpSupported.has_value()) + if (params.GetTCPSupportModes() != TCPModeAdvertise::kNone) { - size_t writtenCharactersNumber = - static_cast(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *tcpSupported)); + size_t writtenCharactersNumber = static_cast(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), + "T=%d", static_cast(params.GetTCPSupportModes()))); VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)), CHIP_ERROR_INVALID_STRING_LENGTH); txtFields[numTxtFields++] = storage.tcpSupportedBuf; diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index beb733b1276a3f..e214aed4f80da6 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -240,7 +240,8 @@ CHIP_ERROR CopyTxtRecord(TxtFieldKey key, char * buffer, size_t bufferLen, const switch (key) { case TxtFieldKey::kTcpSupported: - return CopyTextRecordValue(buffer, bufferLen, params.GetTcpSupported()); + VerifyOrReturnError(params.GetTCPSupportModes() != TCPModeAdvertise::kNone, CHIP_ERROR_UNINITIALIZED); + return CopyTextRecordValue(buffer, bufferLen, to_underlying(params.GetTCPSupportModes())); case TxtFieldKey::kSessionIdleInterval: #if CHIP_CONFIG_ENABLE_ICD_SERVER // A ICD operating as a LIT should not advertise its slow polling interval diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp index 1a66865c143a2e..ea85f74d2be11a 100644 --- a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp +++ b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp @@ -81,7 +81,8 @@ OperationalAdvertisingParameters operationalParams1 = .SetPort(CHIP_PORT) .EnableIpV4(true) .SetLocalMRPConfig(std::make_optional( - 32_ms32, 30_ms32)); // Match SII, SAI. SAT not provided so it uses default 4000ms + 32_ms32, 30_ms32)) // Match SII, SAI. SAT not provided so it uses default 4000ms + .SetTCPSupportModes(chip::Dnssd::TCPModeAdvertise::kTCPClientServer); OperationalAdvertisingParameters operationalParams2 = OperationalAdvertisingParameters().SetPeerId(kPeerId2).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); OperationalAdvertisingParameters operationalParams3 = @@ -92,7 +93,7 @@ OperationalAdvertisingParameters operationalParams5 = OperationalAdvertisingParameters().SetPeerId(kPeerId5).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); OperationalAdvertisingParameters operationalParams6 = OperationalAdvertisingParameters().SetPeerId(kPeerId6).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); -const QNamePart txtOperational1Parts[] = { "SII=32", "SAI=30", "SAT=4000" }; +const QNamePart txtOperational1Parts[] = { "SII=32", "SAI=30", "SAT=4000", "T=6" }; PtrResourceRecord ptrOperationalService = PtrResourceRecord(kDnsSdQueryName, kMatterOperationalQueryName); PtrResourceRecord ptrOperational1 = PtrResourceRecord(kMatterOperationalQueryName, kInstanceName1); SrvResourceRecord srvOperational1 = SrvResourceRecord(kInstanceName1, kHostnameName, CHIP_PORT);