Skip to content

Commit

Permalink
Fix DNS-SD TCP advertisement to use a bitmap for Client and Server. (#…
Browse files Browse the repository at this point in the history
…34289)

When TCP is enabled, a node, currently, advertises as both a Client and
Server.
  • Loading branch information
pidarped authored and pull[bot] committed Dec 18, 2024
1 parent 5ce1b3f commit 4123523
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 11 deletions.
3 changes: 3 additions & 0 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/app/server/Dnssd.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver

void SetICDManager(ICDManager * manager) { mICDManager = manager; };
#endif

/// Start operational advertising
CHIP_ERROR AdvertiseOperational();

Expand Down
17 changes: 12 additions & 5 deletions src/lib/dnssd/Advertiser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 Derived>
class BaseAdvertisingParams
{
Expand Down Expand Up @@ -102,13 +110,12 @@ class BaseAdvertisingParams
}
const std::optional<ReliableMessageProtocolConfig> & 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<bool> tcpSupported)
Derived & SetTCPSupportModes(TCPModeAdvertise tcpSupportModes)
{
mTcpSupported = tcpSupported;
mTcpSupportModes = tcpSupportModes;
return *reinterpret_cast<Derived *>(this);
}
std::optional<bool> GetTcpSupported() const { return mTcpSupported; }
TCPModeAdvertise GetTCPSupportModes() const { return mTcpSupportModes; }

Derived & SetICDModeToAdvertise(ICDModeAdvertise operatingMode)
{
Expand All @@ -124,7 +131,7 @@ class BaseAdvertisingParams
uint8_t mMacStorage[kMaxMacSize] = {};
size_t mMacLength = 0;
std::optional<ReliableMessageProtocolConfig> mLocalMRPConfig;
std::optional<bool> mTcpSupported;
TCPModeAdvertise mTcpSupportModes = TCPModeAdvertise::kNone;
ICDModeAdvertise mICDModeAdvertise = ICDModeAdvertise::kNone;
};

Expand Down
6 changes: 3 additions & 3 deletions src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<size_t>(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *tcpSupported));
size_t writtenCharactersNumber = static_cast<size_t>(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf),
"T=%d", static_cast<int>(params.GetTCPSupportModes())));
VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)),
CHIP_ERROR_INVALID_STRING_LENGTH);
txtFields[numTxtFields++] = storage.tcpSupportedBuf;
Expand Down
3 changes: 2 additions & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ OperationalAdvertisingParameters operationalParams1 =
.SetPort(CHIP_PORT)
.EnableIpV4(true)
.SetLocalMRPConfig(std::make_optional<ReliableMessageProtocolConfig>(
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 =
Expand All @@ -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);
Expand Down

0 comments on commit 4123523

Please sign in to comment.