[Dns-sd] Added Operational Discovery and made browse delegate common (p…

…roject-chip#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 <[email protected]>
SiliconLabs · May 30, 2024 · ebac430 · ebac430
1 parent c0b635a
commit ebac430
Show file tree

Hide file tree

Showing 8 changed files with 109 additions and 36 deletions.
diff --git a/src/lib/dnssd/IncrementalResolve.cpp b/src/lib/dnssd/IncrementalResolve.cpp
@@ -137,7 +137,8 @@ SerializedQNameIterator StoredServerName::Get() const
     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 @@ CHIP_ERROR IncrementalResolver::InitializeParsing(mdns::Minimal::SerializedQName
             {
                 return err;
             }
+            mSpecificResolutionData.Get<OperationalNodeData>().hasZeroTTL = (ttl == 0);
         }
 
         LogFoundOperationalSrvRecord(mSpecificResolutionData.Get<OperationalNodeData>().peerId, mTargetHostName.Get());
@@ -304,7 +306,7 @@ CHIP_ERROR IncrementalResolver::OnTxtRecord(const ResourceData & data, BytesRang
         }
     }
 
-    if (IsActiveBrowseParse())
+    if (IsActiveCommissionParse())
     {
         TxtParser<CommissionNodeData> delegate(mSpecificResolutionData.Get<CommissionNodeData>());
         if (!ParseTxtRecord(data.GetData(), &delegate))
@@ -343,14 +345,17 @@ CHIP_ERROR IncrementalResolver::OnIpAddress(Inet::InterfaceId interface, const I
 
 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
@@ -99,7 +99,7 @@ class IncrementalResolver
     /// 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; }
@@ -108,7 +108,8 @@ class IncrementalResolver
     /// 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
@@ -143,7 +144,7 @@ class IncrementalResolver
 
     /// 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
@@ -55,6 +55,13 @@ CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
     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
@@ -54,6 +54,7 @@ class ResolverProxy
 
     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
@@ -215,7 +215,7 @@ void PacketParser::ParseSRVResource(const ResourceData & data)
             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:
@@ -365,6 +365,7 @@ void MinMdnsResolver::AdvancePendingResolverStates()
     {
         if (!resolver->IsActive())
         {
+            ChipLogError(Discovery, "resolver inactive, continue to next");
             continue;
         }
 
@@ -385,7 +386,7 @@ void MinMdnsResolver::AdvancePendingResolverStates()
         }
 
         // SUCCESS. Call the delegates
-        if (resolver->IsActiveBrowseParse())
+        if (resolver->IsActiveCommissionParse())
         {
             MATTER_TRACE_SCOPE("Active commissioning delegate call", "MinMdnsResolver");
             DiscoveredNodeData nodeData;
@@ -438,18 +439,39 @@ void MinMdnsResolver::AdvancePendingResolverStates()
         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;
+            }
+
+            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(nodeData.operationalData.peerId);
+            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
@@ -197,10 +197,23 @@ struct CommonResolutionData
 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: " ChipLogFormatX64 ":" ChipLogFormatX64 "\r\n",
+                        ChipLogValueX64(peerId.GetCompressedFabricId()),
+                        ChipLogValueX64(peerId.GetNodeId()));
+        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;
@@ -301,12 +314,13 @@ struct ResolvedNodeData
     }
 };
 
-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
@@ -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>
@@ -163,7 +164,7 @@ void TestCreation(nlTestSuite * inSuite, void * inContext)
     IncrementalResolver resolver;
 
     NL_TEST_ASSERT(inSuite, !resolver.IsActive());
-    NL_TEST_ASSERT(inSuite, !resolver.IsActiveBrowseParse());
+    NL_TEST_ASSERT(inSuite, !resolver.IsActiveCommissionParse());
     NL_TEST_ASSERT(inSuite, !resolver.IsActiveOperationalParse());
     NL_TEST_ASSERT(
         inSuite,
@@ -180,10 +181,10 @@ void TestInactiveResetOnInitError(nlTestSuite * inSuite, void * inContext)
     PreloadSrvRecord(inSuite, srvRecord);
 
     // test host name is not a 'matter' name
-    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestHostName.Serialized(), srvRecord) != CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestHostName.Serialized(), 0, srvRecord) != CHIP_NO_ERROR);
 
     NL_TEST_ASSERT(inSuite, !resolver.IsActive());
-    NL_TEST_ASSERT(inSuite, !resolver.IsActiveBrowseParse());
+    NL_TEST_ASSERT(inSuite, !resolver.IsActiveCommissionParse());
     NL_TEST_ASSERT(inSuite, !resolver.IsActiveOperationalParse());
 }
 
@@ -196,10 +197,10 @@ void TestStartOperational(nlTestSuite * inSuite, void * inContext)
     SrvRecord srvRecord;
     PreloadSrvRecord(inSuite, srvRecord);
 
-    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestOperationalName.Serialized(), srvRecord) == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestOperationalName.Serialized(), 1, srvRecord) == CHIP_NO_ERROR);
 
     NL_TEST_ASSERT(inSuite, resolver.IsActive());
-    NL_TEST_ASSERT(inSuite, !resolver.IsActiveBrowseParse());
+    NL_TEST_ASSERT(inSuite, !resolver.IsActiveCommissionParse());
     NL_TEST_ASSERT(inSuite, resolver.IsActiveOperationalParse());
     NL_TEST_ASSERT(inSuite,
                    resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
@@ -215,10 +216,10 @@ void TestStartCommissionable(nlTestSuite * inSuite, void * inContext)
     SrvRecord srvRecord;
     PreloadSrvRecord(inSuite, srvRecord);
 
-    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestCommissionableNode.Serialized(), srvRecord) == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestCommissionableNode.Serialized(), 0, srvRecord) == CHIP_NO_ERROR);
 
     NL_TEST_ASSERT(inSuite, resolver.IsActive());
-    NL_TEST_ASSERT(inSuite, resolver.IsActiveBrowseParse());
+    NL_TEST_ASSERT(inSuite, resolver.IsActiveCommissionParse());
     NL_TEST_ASSERT(inSuite, !resolver.IsActiveOperationalParse());
     NL_TEST_ASSERT(inSuite,
                    resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
@@ -234,10 +235,10 @@ void TestStartCommissioner(nlTestSuite * inSuite, void * inContext)
     SrvRecord srvRecord;
     PreloadSrvRecord(inSuite, srvRecord);
 
-    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestCommissionerNode.Serialized(), srvRecord) == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestCommissionerNode.Serialized(), 0, srvRecord) == CHIP_NO_ERROR);
 
     NL_TEST_ASSERT(inSuite, resolver.IsActive());
-    NL_TEST_ASSERT(inSuite, resolver.IsActiveBrowseParse());
+    NL_TEST_ASSERT(inSuite, resolver.IsActiveCommissionParse());
     NL_TEST_ASSERT(inSuite, !resolver.IsActiveOperationalParse());
     NL_TEST_ASSERT(inSuite,
                    resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
@@ -253,7 +254,7 @@ void TestParseOperational(nlTestSuite * inSuite, void * inContext)
     SrvRecord srvRecord;
     PreloadSrvRecord(inSuite, srvRecord);
 
-    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestOperationalName.Serialized(), srvRecord) == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestOperationalName.Serialized(), 1, srvRecord) == CHIP_NO_ERROR);
 
     // once initialized, parsing should be ready however no IP address is available
     NL_TEST_ASSERT(inSuite, resolver.IsActiveOperationalParse());
@@ -312,6 +313,7 @@ void TestParseOperational(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite,
                    nodeData.operationalData.peerId ==
                        PeerId().SetCompressedFabricId(0x1234567898765432LL).SetNodeId(0xABCDEFEDCBAABCDELL));
+    NL_TEST_ASSERT(inSuite, !nodeData.operationalData.hasZeroTTL);
     NL_TEST_ASSERT(inSuite, nodeData.resolutionData.numIPs == 1);
     NL_TEST_ASSERT(inSuite, nodeData.resolutionData.port == 0x1234);
     NL_TEST_ASSERT(inSuite, !nodeData.resolutionData.supportsTcp);
@@ -333,10 +335,10 @@ void TestParseCommissionable(nlTestSuite * inSuite, void * inContext)
     SrvRecord srvRecord;
     PreloadSrvRecord(inSuite, srvRecord);
 
-    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestCommissionableNode.Serialized(), srvRecord) == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, resolver.InitializeParsing(kTestCommissionableNode.Serialized(), 0, srvRecord) == CHIP_NO_ERROR);
 
     // once initialized, parsing should be ready however no IP address is available
-    NL_TEST_ASSERT(inSuite, resolver.IsActiveBrowseParse());
+    NL_TEST_ASSERT(inSuite, resolver.IsActiveCommissionParse());
     NL_TEST_ASSERT(inSuite,
                    resolver.GetMissingRequiredInformation().HasOnly(IncrementalResolver::RequiredInformationBitFlags::kIpAddress));
     NL_TEST_ASSERT(inSuite, resolver.GetTargetHostName() == kTestHostName.Serialized());

diff --git a/src/lib/shell/commands/Dns.cpp b/src/lib/shell/commands/Dns.cpp
@@ -83,15 +83,25 @@ class DnsShellResolverDelegate : public Dnssd::DiscoverNodeDelegate, public Addr
 
     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: " ChipLogFormatX64 ":" ChipLogFormatX64 "\r\n",
+                        ChipLogValueX64(nodeData.peerId.GetCompressedFabricId()),
+                        ChipLogValueX64(nodeData.peerId.GetNodeId()));
+        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 +247,16 @@ CHIP_ERROR BrowseCommissionerHandler(int argc, char ** argv)
     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 +291,14 @@ void RegisterDnsCommands()
           "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" };