[Dns-sd] Added Operational Discovery and made browse delegate common (#32750)
* Added operational discovery support
* Restyled by whitespace
* Restyled by clang-format
* Updates as per review feedback
* Restyled by whitespace
* Restyled by clang-format
* Updates based on additional feedback
* Restyled by whitespace
* updates as per feedback from review
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/src/lib/dnssd/IncrementalResolve.cpp b/src/lib/dnssd/IncrementalResolve.cpp
index f083f85..867b1a8 100644
--- a/src/lib/dnssd/IncrementalResolve.cpp
+++ b/src/lib/dnssd/IncrementalResolve.cpp
@@ -137,7 +137,8 @@
return SerializedQNameIterator(BytesRange(mNameBuffer, mNameBuffer + sizeof(mNameBuffer)), mNameBuffer);
}
-CHIP_ERROR IncrementalResolver::InitializeParsing(mdns::Minimal::SerializedQNameIterator name, const mdns::Minimal::SrvRecord & srv)
+CHIP_ERROR IncrementalResolver::InitializeParsing(mdns::Minimal::SerializedQNameIterator name, const uint64_t ttl,
+ const mdns::Minimal::SrvRecord & srv)
{
AutoInactiveResetter inactiveReset(*this);
@@ -183,6 +184,7 @@
{
return err;
}
+ mSpecificResolutionData.Get<OperationalNodeData>().hasZeroTTL = (ttl == 0);
}
LogFoundOperationalSrvRecord(mSpecificResolutionData.Get<OperationalNodeData>().peerId, mTargetHostName.Get());
@@ -304,7 +306,7 @@
}
}
- if (IsActiveBrowseParse())
+ if (IsActiveCommissionParse())
{
TxtParser<CommissionNodeData> delegate(mSpecificResolutionData.Get<CommissionNodeData>());
if (!ParseTxtRecord(data.GetData(), &delegate))
@@ -343,14 +345,17 @@
CHIP_ERROR IncrementalResolver::Take(DiscoveredNodeData & outputData)
{
- VerifyOrReturnError(IsActiveBrowseParse(), CHIP_ERROR_INCORRECT_STATE);
+ VerifyOrReturnError(IsActiveCommissionParse(), CHIP_ERROR_INCORRECT_STATE);
IPAddressSorter::Sort(mCommonResolutionData.ipAddress, mCommonResolutionData.numIPs, mCommonResolutionData.interfaceId);
- outputData.Set<CommissionNodeData>();
- CommissionNodeData & nodeData = outputData.Get<CommissionNodeData>();
- nodeData = mSpecificResolutionData.Get<CommissionNodeData>();
- CommonResolutionData & resolutionData = nodeData;
+ // Set the DiscoveredNodeData with CommissionNodeData info specific to commissionable/commisssioner
+ // node available in mSpecificResolutionData.
+ outputData.Set<CommissionNodeData>(mSpecificResolutionData.Get<CommissionNodeData>());
+
+ // IncrementalResolver stored CommonResolutionData separately in mCommonResolutionData hence copy the
+ // CommonResolutionData info from mCommonResolutionData, to CommissionNodeData within DiscoveredNodeData
+ CommonResolutionData & resolutionData = outputData.Get<CommissionNodeData>();
resolutionData = mCommonResolutionData;
ResetToInactive();
diff --git a/src/lib/dnssd/IncrementalResolve.h b/src/lib/dnssd/IncrementalResolve.h
index 4a8bc0c..319a080 100644
--- a/src/lib/dnssd/IncrementalResolve.h
+++ b/src/lib/dnssd/IncrementalResolve.h
@@ -100,7 +100,7 @@
/// method.
bool IsActive() const { return mSpecificResolutionData.Valid(); }
- bool IsActiveBrowseParse() const { return mSpecificResolutionData.Is<CommissionNodeData>(); }
+ bool IsActiveCommissionParse() const { return mSpecificResolutionData.Is<CommissionNodeData>(); }
bool IsActiveOperationalParse() const { return mSpecificResolutionData.Is<OperationalNodeData>(); }
ServiceNameType GetCurrentType() const { return mServiceNameType; }
@@ -115,7 +115,8 @@
/// interested on, after which TXT and A/AAAA are looked for.
///
/// If this function returns with error, the object will be in an inactive state.
- CHIP_ERROR InitializeParsing(mdns::Minimal::SerializedQNameIterator name, const mdns::Minimal::SrvRecord & srv);
+ CHIP_ERROR InitializeParsing(mdns::Minimal::SerializedQNameIterator name, const uint64_t ttl,
+ const mdns::Minimal::SrvRecord & srv);
/// Notify that a new record is being processed.
/// Will handle filtering and processing of data to determine if the entry is relevant for
@@ -150,7 +151,7 @@
/// Take the current value of the object and clear it once returned.
///
- /// Object must be in `IsActiveBrowseParse()` for this to succeed.
+ /// Object must be in `IsActive()` for this to succeed.
/// Data will be returned (and cleared) even if not yet complete based
/// on `GetMissingRequiredInformation()`. This method takes as much data as
/// it was parsed so far.
diff --git a/src/lib/dnssd/ResolverProxy.cpp b/src/lib/dnssd/ResolverProxy.cpp
index e43028e..2fbf5b1 100644
--- a/src/lib/dnssd/ResolverProxy.cpp
+++ b/src/lib/dnssd/ResolverProxy.cpp
@@ -55,6 +55,13 @@
return mResolver.StartDiscovery(DiscoveryType::kCommissionerNode, filter, *mContext);
}
+CHIP_ERROR ResolverProxy::DiscoverOperationalNodes(DiscoveryFilter filter)
+{
+ VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_INCORRECT_STATE);
+
+ return mResolver.StartDiscovery(DiscoveryType::kOperational, filter, *mContext);
+}
+
CHIP_ERROR ResolverProxy::StopDiscovery()
{
VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_INCORRECT_STATE);
diff --git a/src/lib/dnssd/ResolverProxy.h b/src/lib/dnssd/ResolverProxy.h
index 58617b0..d1888c3 100644
--- a/src/lib/dnssd/ResolverProxy.h
+++ b/src/lib/dnssd/ResolverProxy.h
@@ -54,6 +54,7 @@
CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter());
CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter());
+ CHIP_ERROR DiscoverOperationalNodes(DiscoveryFilter filter = DiscoveryFilter());
CHIP_ERROR StopDiscovery();
private:
diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
index 8e17ce2..efcdece 100644
--- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
+++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
@@ -213,7 +213,7 @@
continue;
}
- CHIP_ERROR err = resolver.InitializeParsing(data.GetName(), srv);
+ CHIP_ERROR err = resolver.InitializeParsing(data.GetName(), data.GetTtlSeconds(), srv);
if (err != CHIP_NO_ERROR)
{
// Receiving records that we do not need to parse is normal:
@@ -363,6 +363,7 @@
{
if (!resolver->IsActive())
{
+ ChipLogError(Discovery, "resolver inactive, continue to next");
continue;
}
@@ -370,7 +371,7 @@
if (missing.Has(IncrementalResolver::RequiredInformationBitFlags::kIpAddress))
{
- if (resolver->IsActiveBrowseParse())
+ if (resolver->IsActiveCommissionParse())
{
// Browse wants IP addresses
ScheduleIpAddressResolve(resolver->GetTargetHostName());
@@ -399,7 +400,7 @@
}
// SUCCESS. Call the delegates
- if (resolver->IsActiveBrowseParse())
+ if (resolver->IsActiveCommissionParse())
{
MATTER_TRACE_SCOPE("Active commissioning delegate call", "MinMdnsResolver");
DiscoveredNodeData nodeData;
@@ -452,19 +453,39 @@
else if (resolver->IsActiveOperationalParse())
{
MATTER_TRACE_SCOPE("Active operational delegate call", "MinMdnsResolver");
- ResolvedNodeData nodeData;
+ ResolvedNodeData nodeResolvedData;
+ CHIP_ERROR err = resolver->Take(nodeResolvedData);
- CHIP_ERROR err = resolver->Take(nodeData);
if (err != CHIP_NO_ERROR)
{
- ChipLogError(Discovery, "Failed to take discovery result: %" CHIP_ERROR_FORMAT, err.Format());
+ ChipLogError(Discovery, "Failed to take NodeData - result: %" CHIP_ERROR_FORMAT, err.Format());
continue;
}
- mActiveResolves.Complete(nodeData.operationalData.peerId);
+ if (mActiveResolves.HasBrowseFor(chip::Dnssd::DiscoveryType::kOperational))
+ {
+ if (mDiscoveryContext != nullptr)
+ {
+ DiscoveredNodeData nodeData;
+ OperationalNodeBrowseData opNodeData;
+
+ opNodeData.peerId = nodeResolvedData.operationalData.peerId;
+ opNodeData.hasZeroTTL = nodeResolvedData.operationalData.hasZeroTTL;
+ nodeData.Set<OperationalNodeBrowseData>(opNodeData);
+ mDiscoveryContext->OnNodeDiscovered(nodeData);
+ }
+ else
+ {
+#if CHIP_MINMDNS_HIGH_VERBOSITY
+ ChipLogError(Discovery, "No delegate to report operational node discovery");
+#endif
+ }
+ }
+
+ mActiveResolves.Complete(nodeResolvedData.operationalData.peerId);
if (mOperationalDelegate != nullptr)
{
- mOperationalDelegate->OnOperationalNodeResolved(nodeData);
+ mOperationalDelegate->OnOperationalNodeResolved(nodeResolvedData);
}
else
{
diff --git a/src/lib/dnssd/Types.h b/src/lib/dnssd/Types.h
index c88dd0a..1b9b19c 100644
--- a/src/lib/dnssd/Types.h
+++ b/src/lib/dnssd/Types.h
@@ -197,10 +197,21 @@
struct OperationalNodeData
{
PeerId peerId;
-
+ bool hasZeroTTL;
void Reset() { peerId = PeerId(); }
};
+struct OperationalNodeBrowseData : public OperationalNodeData
+{
+ OperationalNodeBrowseData() { Reset(); };
+ void LogDetail() const
+ {
+ ChipLogDetail(Discovery, "Discovered Operational node:\r\n");
+ ChipLogDetail(Discovery, "\tNode Instance: " ChipLogFormatPeerId, ChipLogValuePeerId(peerId));
+ ChipLogDetail(Discovery, "\thasZeroTTL: %s\r\n", hasZeroTTL ? "true" : "false");
+ }
+};
+
inline constexpr size_t kMaxDeviceNameLen = 32;
inline constexpr size_t kMaxRotatingIdLen = 50;
inline constexpr size_t kMaxPairingInstructionLen = 128;
@@ -297,12 +308,13 @@
}
};
-using DiscoveredNodeData = Variant<CommissionNodeData>;
+using DiscoveredNodeData = Variant<CommissionNodeData, OperationalNodeBrowseData>;
-/// Callbacks for discovering nodes advertising non-operational status:
+/// Callbacks for discovering nodes advertising both operational and non-operational status:
/// - Commissioners
/// - Nodes in commissioning modes over IP (e.g. ethernet devices, devices already
/// connected to thread/wifi or devices with a commissioning window open)
+/// - Operational nodes
class DiscoverNodeDelegate
{
public:
diff --git a/src/lib/dnssd/tests/TestIncrementalResolve.cpp b/src/lib/dnssd/tests/TestIncrementalResolve.cpp
index 0b5f26a..053d0dd 100644
--- a/src/lib/dnssd/tests/TestIncrementalResolve.cpp
+++ b/src/lib/dnssd/tests/TestIncrementalResolve.cpp
@@ -20,6 +20,7 @@
#include <string.h>
+#include <lib/dnssd/ServiceNaming.h>
#include <lib/dnssd/minimal_mdns/core/tests/QNameStrings.h>
#include <lib/dnssd/minimal_mdns/records/IP.h>
#include <lib/dnssd/minimal_mdns/records/Ptr.h>
@@ -161,7 +162,7 @@
IncrementalResolver resolver;
EXPECT_FALSE(resolver.IsActive());
- EXPECT_FALSE(resolver.IsActiveBrowseParse());
+ EXPECT_FALSE(resolver.IsActiveCommissionParse());
EXPECT_FALSE(resolver.IsActiveOperationalParse());
EXPECT_TRUE(
resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kSrvInitialization));
@@ -177,10 +178,10 @@
PreloadSrvRecord(srvRecord);
// test host name is not a 'matter' name
- EXPECT_NE(resolver.InitializeParsing(kTestHostName.Serialized(), srvRecord), CHIP_NO_ERROR);
+ EXPECT_NE(resolver.InitializeParsing(kTestHostName.Serialized(), 0, srvRecord), CHIP_NO_ERROR);
EXPECT_FALSE(resolver.IsActive());
- EXPECT_FALSE(resolver.IsActiveBrowseParse());
+ EXPECT_FALSE(resolver.IsActiveCommissionParse());
EXPECT_FALSE(resolver.IsActiveOperationalParse());
}
@@ -193,10 +194,10 @@
SrvRecord srvRecord;
PreloadSrvRecord(srvRecord);
- EXPECT_EQ(resolver.InitializeParsing(kTestOperationalName.Serialized(), srvRecord), CHIP_NO_ERROR);
+ EXPECT_EQ(resolver.InitializeParsing(kTestOperationalName.Serialized(), 1, srvRecord), CHIP_NO_ERROR);
EXPECT_TRUE(resolver.IsActive());
- EXPECT_FALSE(resolver.IsActiveBrowseParse());
+ EXPECT_FALSE(resolver.IsActiveCommissionParse());
EXPECT_TRUE(resolver.IsActiveOperationalParse());
EXPECT_TRUE(resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
EXPECT_EQ(resolver.GetTargetHostName(), kTestHostName.Serialized());
@@ -211,10 +212,10 @@
SrvRecord srvRecord;
PreloadSrvRecord(srvRecord);
- EXPECT_EQ(resolver.InitializeParsing(kTestCommissionableNode.Serialized(), srvRecord), CHIP_NO_ERROR);
+ EXPECT_EQ(resolver.InitializeParsing(kTestCommissionableNode.Serialized(), 0, srvRecord), CHIP_NO_ERROR);
EXPECT_TRUE(resolver.IsActive());
- EXPECT_TRUE(resolver.IsActiveBrowseParse());
+ EXPECT_TRUE(resolver.IsActiveCommissionParse());
EXPECT_FALSE(resolver.IsActiveOperationalParse());
EXPECT_TRUE(resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
EXPECT_EQ(resolver.GetTargetHostName(), kTestHostName.Serialized());
@@ -229,10 +230,10 @@
SrvRecord srvRecord;
PreloadSrvRecord(srvRecord);
- EXPECT_EQ(resolver.InitializeParsing(kTestCommissionerNode.Serialized(), srvRecord), CHIP_NO_ERROR);
+ EXPECT_EQ(resolver.InitializeParsing(kTestCommissionerNode.Serialized(), 0, srvRecord), CHIP_NO_ERROR);
EXPECT_TRUE(resolver.IsActive());
- EXPECT_TRUE(resolver.IsActiveBrowseParse());
+ EXPECT_TRUE(resolver.IsActiveCommissionParse());
EXPECT_FALSE(resolver.IsActiveOperationalParse());
EXPECT_TRUE(resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
EXPECT_EQ(resolver.GetTargetHostName(), kTestHostName.Serialized());
@@ -247,7 +248,7 @@
SrvRecord srvRecord;
PreloadSrvRecord(srvRecord);
- EXPECT_EQ(resolver.InitializeParsing(kTestOperationalName.Serialized(), srvRecord), CHIP_NO_ERROR);
+ EXPECT_EQ(resolver.InitializeParsing(kTestOperationalName.Serialized(), 1, srvRecord), CHIP_NO_ERROR);
// once initialized, parsing should be ready however no IP address is available
EXPECT_TRUE(resolver.IsActiveOperationalParse());
@@ -304,6 +305,7 @@
// validate data as it was passed in
EXPECT_EQ(nodeData.operationalData.peerId,
PeerId().SetCompressedFabricId(0x1234567898765432LL).SetNodeId(0xABCDEFEDCBAABCDELL));
+ EXPECT_FALSE(nodeData.operationalData.hasZeroTTL);
EXPECT_EQ(nodeData.resolutionData.numIPs, 1u);
EXPECT_EQ(nodeData.resolutionData.port, 0x1234);
EXPECT_FALSE(nodeData.resolutionData.supportsTcp);
@@ -324,10 +326,10 @@
SrvRecord srvRecord;
PreloadSrvRecord(srvRecord);
- EXPECT_EQ(resolver.InitializeParsing(kTestCommissionableNode.Serialized(), srvRecord), CHIP_NO_ERROR);
+ EXPECT_EQ(resolver.InitializeParsing(kTestCommissionableNode.Serialized(), 0, srvRecord), CHIP_NO_ERROR);
// once initialized, parsing should be ready however no IP address is available
- EXPECT_TRUE(resolver.IsActiveBrowseParse());
+ EXPECT_TRUE(resolver.IsActiveCommissionParse());
EXPECT_TRUE(resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
EXPECT_EQ(resolver.GetTargetHostName(), kTestHostName.Serialized());
diff --git a/src/lib/shell/commands/Dns.cpp b/src/lib/shell/commands/Dns.cpp
index 7fac018..c6a9365 100644
--- a/src/lib/shell/commands/Dns.cpp
+++ b/src/lib/shell/commands/Dns.cpp
@@ -83,15 +83,23 @@
AddressResolve::NodeLookupHandle & Handle() { return mSelfHandle; }
+ void LogOperationalNodeDiscovered(const Dnssd::OperationalNodeBrowseData & nodeData)
+ {
+ streamer_printf(streamer_get(), "DNS browse operational succeeded: \r\n");
+ streamer_printf(streamer_get(), " Node Instance: " ChipLogFormatPeerId, ChipLogValuePeerId(nodeData.peerId));
+ streamer_printf(streamer_get(), " hasZeroTTL: %s\r\n", nodeData.hasZeroTTL ? "true" : "false");
+ }
+
void OnNodeDiscovered(const Dnssd::DiscoveredNodeData & discNodeData) override
{
- if (!discNodeData.Is<Dnssd::CommissionNodeData>())
+ if (discNodeData.Is<Dnssd::OperationalNodeBrowseData>())
{
- streamer_printf(streamer_get(), "DNS browse failed - not commission type node \r\n");
+ LogOperationalNodeDiscovered(discNodeData.Get<Dnssd::OperationalNodeBrowseData>());
return;
}
- Dnssd::CommissionNodeData nodeData = discNodeData.Get<Dnssd::CommissionNodeData>();
+ const auto & nodeData = discNodeData.Get<Dnssd::CommissionNodeData>();
+
if (!nodeData.IsValid())
{
streamer_printf(streamer_get(), "DNS browse failed - not found valid services \r\n");
@@ -237,6 +245,16 @@
return sResolverProxy.DiscoverCommissioners(filter);
}
+CHIP_ERROR BrowseOperationalHandler(int argc, char ** argv)
+{
+ Dnssd::DiscoveryFilter filter;
+ VerifyOrReturnError(ParseSubType(argc, argv, filter), CHIP_ERROR_INVALID_ARGUMENT);
+
+ streamer_printf(streamer_get(), "Browsing operational...\r\n");
+
+ return sResolverProxy.DiscoverOperationalNodes(filter);
+}
+
CHIP_ERROR BrowseHandler(int argc, char ** argv)
{
if (argc == 0)
@@ -271,13 +289,14 @@
"Browse Matter commissionable nodes. Usage: dns browse commissionable [subtype]" },
{ &BrowseCommissionerHandler, "commissioner",
"Browse Matter commissioner nodes. Usage: dns browse commissioner [subtype]" },
+ { &BrowseOperationalHandler, "operational", "Browse Matter operational nodes. Usage: dns browse operational" },
};
static const shell_command_t sDnsSubCommands[] = {
{ &ResolveHandler, "resolve",
"Resolve the DNS service. Usage: dns resolve <fabric-id> <node-id> (e.g. dns resolve 5544332211 1)" },
{ &BrowseHandler, "browse",
- "Browse DNS services published by Matter nodes. Usage: dns browse <commissionable|commissioner>" },
+ "Browse DNS services published by Matter nodes. Usage: dns browse <commissionable|commissioner|operational>" },
};
static const shell_command_t sDnsCommand = { &DnsHandler, "dns", "Dns client commands" };
