[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>

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();

src/lib/dnssd/IncrementalResolve.h

@@ -100,7 +100,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; }
@@ -115,7 +115,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
@@ -150,7 +151,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.

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);

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:

src/lib/dnssd/Resolver_ImplMinimalMdns.cpp

@@ -213,7 +213,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:
@@ -363,14 +363,15 @@ void MinMdnsResolver::AdvancePendingResolverStates()
     {
         if (!resolver->IsActive())
         {
+            ChipLogError(Discovery, "resolver inactive, continue to next");
             continue;
         }
 
         IncrementalResolver::RequiredInformationFlags missing = resolver->GetMissingRequiredInformation();
 
         if (missing.Has(IncrementalResolver::RequiredInformationBitFlags::kIpAddress))
         {
-            if (resolver->IsActiveBrowseParse())
+            if (resolver->IsActiveCommissionParse())
             {
                 // Browse wants IP addresses
                 ScheduleIpAddressResolve(resolver->GetTargetHostName());
@@ -399,7 +400,7 @@ void MinMdnsResolver::AdvancePendingResolverStates()
         }
 
         // SUCCESS. Call the delegates
-        if (resolver->IsActiveBrowseParse())
+        if (resolver->IsActiveCommissionParse())
         {
             MATTER_TRACE_SCOPE("Active commissioning delegate call", "MinMdnsResolver");
             DiscoveredNodeData nodeData;
@@ -452,19 +453,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;
             }
 
-            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
             {

src/lib/dnssd/Types.h

@@ -197,10 +197,21 @@ 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: " 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 @@ 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:

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 @@ TEST(TestIncrementalResolve, TestCreation)
     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 @@ TEST(TestIncrementalResolve, TestInactiveResetOnInitError)
     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 @@ TEST(TestIncrementalResolve, TestStartOperational)
     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 @@ TEST(TestIncrementalResolve, TestStartCommissionable)
     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 @@ TEST(TestIncrementalResolve, TestStartCommissioner)
     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 @@ TEST(TestIncrementalResolve, TestParseOperational)
     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 @@ TEST(TestIncrementalResolve, TestParseOperational)
     // 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 @@ TEST(TestIncrementalResolve, TestParseCommissionable)
     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());
 

src/lib/shell/commands/Dns.cpp

@@ -83,15 +83,23 @@ 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: " 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 @@ 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 +289,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" };