From 10d1409921e4ea41393394027b8254fb01b5f377 Mon Sep 17 00:00:00 2001 From: C Freeman Date: Mon, 12 Feb 2024 19:20:57 -0500 Subject: [PATCH] DNS-SD: Remove calls to set T flag (#32083) * DNS-SD: Remove calls to set T flag Nothing currently supports TCP, therefore nothing should be setting this flag. Per the new spec text, it is now forbidden to have the lowest bit marked. This will bring the SDK examples into compliance with 1.3. Note that this PR does not fix the advertising parameters API. We will need a new API for this flag when TCP support lands. Leaving this for a follow up PR as changing the API should happen separately from bringing the examples into conformance as it may affect the platforms, who have implementations outside of the SDK. Test: tested with all-clusters and avahi. Also fixed tests to omit this flag and ensure it does not appear in the TXT record. Automated cert test for operational records is in progress and requires this PR to pass. * Restyled by clang-format --------- Co-authored-by: Restyled.io --- src/app/server/Dnssd.cpp | 3 +-- src/lib/dnssd/Advertiser.h | 2 ++ src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp | 10 +++------- src/lib/dnssd/platform/tests/TestPlatform.cpp | 4 ---- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index dfb1fc4d6062a4..a8040564956a3e 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -181,7 +181,6 @@ CHIP_ERROR DnssdServer::AdvertiseOperational() .SetPort(GetSecuredPort()) .SetInterfaceId(GetInterfaceId()) .SetLocalMRPConfig(GetLocalMRPConfig()) - .SetTcpSupported(Optional(INET_CONFIG_ENABLE_TCP_ENDPOINT)) .EnableIpV4(true); #if CHIP_CONFIG_ENABLE_ICD_SERVER @@ -255,7 +254,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi advertiseParameters.SetDeviceName(chip::Optional::Value(deviceName)); } - advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional(INET_CONFIG_ENABLE_TCP_ENDPOINT)); + advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()); #if CHIP_CONFIG_ENABLE_ICD_SERVER AddICDKeyToAdvertisement(advertiseParameters); diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h index 0a98a90fa89a7e..7c6e2953dd78d5 100644 --- a/src/lib/dnssd/Advertiser.h +++ b/src/lib/dnssd/Advertiser.h @@ -94,6 +94,8 @@ class BaseAdvertisingParams return *reinterpret_cast(this); } const 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(Optional tcpSupported) { mTcpSupported = tcpSupported; diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp index 8f3e39c026eb6d..aaf7fc6aa716ab 100644 --- a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp +++ b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp @@ -80,7 +80,6 @@ OperationalAdvertisingParameters operationalParams1 = .SetMac(ByteSpan(kMac)) .SetPort(CHIP_PORT) .EnableIpV4(true) - .SetTcpSupported(chip::Optional(false)) .SetLocalMRPConfig(chip::Optional::Value( 32_ms32, 30_ms32)); // Match SII, SAI. SAT not provided so it uses default 4000ms OperationalAdvertisingParameters operationalParams2 = @@ -93,7 +92,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", "T=0" }; +const QNamePart txtOperational1Parts[] = { "SII=32", "SAI=30", "SAT=4000" }; PtrResourceRecord ptrOperationalService = PtrResourceRecord(kDnsSdQueryName, kMatterOperationalQueryName); PtrResourceRecord ptrOperational1 = PtrResourceRecord(kMatterOperationalQueryName, kInstanceName1); SrvResourceRecord srvOperational1 = SrvResourceRecord(kInstanceName1, kHostnameName, CHIP_PORT); @@ -181,14 +180,12 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced = .SetPairingInstruction(chip::Optional("Pair me")) .SetProductId(chip::Optional(897)) .SetRotatingDeviceId(chip::Optional("id_that_spins")) - .SetTcpSupported(chip::Optional(true)) .SetICDOperatingAsLIT(chip::Optional(false)) // 3600005 is more than the max so should be adjusted down .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32, 65535_ms16)); QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000", "DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3", - "SAI=3600000", "SII=3600000", "SAT=65535", "T=1", - "ICD=0" }; + "SAI=3600000", "SII=3600000", "SAT=65535", "ICD=0" }; FullQName txtCommissionableNodeParamsLargeEnhancedName = FullQName(txtCommissionableNodeParamsLargeEnhancedParts); TxtResourceRecord txtCommissionableNodeParamsLargeEnhanced = TxtResourceRecord(instanceName, txtCommissionableNodeParamsLargeEnhancedName); @@ -207,13 +204,12 @@ CommissionAdvertisingParameters commissionableNodeParamsEnhancedAsICDLIT = .SetPairingHint(chip::Optional(3)) .SetPairingInstruction(chip::Optional("Pair me")) .SetProductId(chip::Optional(897)) - .SetTcpSupported(chip::Optional(true)) .SetICDOperatingAsLIT(chip::Optional(true)) .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600000_ms32, 65535_ms16)); // With ICD Operation as LIT, SII key will not be added to the advertisement QNamePart txtCommissionableNodeParamsEnhancedAsICDLITParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000", "DN=testy-test", "PI=Pair me", "PH=3", "SAI=3600000", - "SAT=65535", "T=1", "ICD=1" }; + "SAT=65535", "ICD=1" }; FullQName txtCommissionableNodeParamsEnhancedAsICDLITName = FullQName(txtCommissionableNodeParamsEnhancedAsICDLITParts); TxtResourceRecord txtCommissionableNodeParamsEnhancedAsICDLIT = TxtResourceRecord(instanceName, txtCommissionableNodeParamsEnhancedAsICDLITName); diff --git a/src/lib/dnssd/platform/tests/TestPlatform.cpp b/src/lib/dnssd/platform/tests/TestPlatform.cpp index 93141ca403dc6a..f0628d0e691ff1 100644 --- a/src/lib/dnssd/platform/tests/TestPlatform.cpp +++ b/src/lib/dnssd/platform/tests/TestPlatform.cpp @@ -53,7 +53,6 @@ OperationalAdvertisingParameters operationalParams2 = .SetPort(CHIP_PORT) .EnableIpV4(true) .SetLocalMRPConfig(Optional::Value(32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below - .SetTcpSupported(Optional(true)) .SetICDOperatingAsLIT(Optional(false)); test::ExpectedCall operationalCall2 = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp) @@ -64,7 +63,6 @@ test::ExpectedCall operationalCall2 = test::ExpectedCall() .AddTxt("SII", "32") .AddTxt("SAI", "30") .AddTxt("SAT", "10") - .AddTxt("T", "1") .AddTxt("ICD", "0"); CommissionAdvertisingParameters commissionableNodeParamsSmall = @@ -97,7 +95,6 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic = .SetPairingInstruction(Optional("Pair me")) .SetProductId(Optional(897)) .SetRotatingDeviceId(Optional("id_that_spins")) - .SetTcpSupported(Optional(true)) .SetICDOperatingAsLIT(Optional(false)) // 3600005 is over the max, so this should be adjusted by the platform .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32, 65535_ms16)); @@ -114,7 +111,6 @@ test::ExpectedCall commissionableLargeBasic = test::ExpectedCall() .AddTxt("RI", "id_that_spins") .AddTxt("PI", "Pair me") .AddTxt("PH", "3") - .AddTxt("T", "1") .AddTxt("ICD", "0") .AddTxt("SII", "3600000") .AddTxt("SAI", "3600000")