Report mdns results from all interfaces instead of the highest priori… (#35597)

* [chip-tool] Add discover find-commissionable-by-instance-name command

* Report mdns results from all interfaces instead of the highest priority interface

* Update DnssdContexts.cpp

Author: vivien-apple

examples/chip-tool/commands/discover/Commands.h

@@ -84,6 +84,7 @@ void registerCommandsDiscover(Commands & commands, CredentialIssuerCommands * cr
         make_unique<DiscoverCommissionableByCommissioningModeCommand>(credsIssuerConfig),
         make_unique<DiscoverCommissionableByVendorIdCommand>(credsIssuerConfig),
         make_unique<DiscoverCommissionableByDeviceTypeCommand>(credsIssuerConfig),
+        make_unique<DiscoverCommissionableByInstanceNameCommand>(credsIssuerConfig),
         make_unique<DiscoverCommissionersCommand>(credsIssuerConfig),
     };
 

examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp

@@ -122,3 +122,11 @@ CHIP_ERROR DiscoverCommissionableByDeviceTypeCommand::RunCommand()
     chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kDeviceType, mDeviceType);
     return mCommissioner->DiscoverCommissionableNodes(filter);
 }
+
+CHIP_ERROR DiscoverCommissionableByInstanceNameCommand::RunCommand()
+{
+    mCommissioner = &CurrentCommissioner();
+    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
+    chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kInstanceName, mInstanceName);
+    return mCommissioner->DiscoverCommissionableNodes(filter);
+}

examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h

@@ -159,3 +159,19 @@ class DiscoverCommissionableByDeviceTypeCommand : public DiscoverCommissionables
     // TODO: possibly 32-bit - see spec issue #3226
     uint16_t mDeviceType;
 };
+
+class DiscoverCommissionableByInstanceNameCommand : public DiscoverCommissionablesCommandBase
+{
+public:
+    DiscoverCommissionableByInstanceNameCommand(CredentialIssuerCommands * credsIssuerConfig) :
+        DiscoverCommissionablesCommandBase("find-commissionable-by-instance-name", credsIssuerConfig)
+    {
+        AddArgument("value", &mInstanceName);
+    }
+
+    /////////// CHIPCommand Interface /////////
+    CHIP_ERROR RunCommand() override;
+
+private:
+    char * mInstanceName;
+};

src/lib/dnssd/Discovery_ImplPlatform.cpp

@@ -43,7 +43,7 @@ static void HandleNodeResolve(void * context, DnssdService * result, const Span<
 {
     DiscoveryContext * discoveryContext = static_cast<DiscoveryContext *>(context);
 
-    if (error != CHIP_NO_ERROR)
+    if (error != CHIP_NO_ERROR && error != CHIP_ERROR_IN_PROGRESS)
     {
         discoveryContext->Release();
         return;
@@ -55,7 +55,13 @@ static void HandleNodeResolve(void * context, DnssdService * result, const Span<
 
     nodeData.Get<CommissionNodeData>().LogDetail();
     discoveryContext->OnNodeDiscovered(nodeData);
-    discoveryContext->Release();
+
+    // CHIP_ERROR_IN_PROGRESS indicates that more results are coming, so don't release
+    // the context yet.
+    if (error == CHIP_NO_ERROR)
+    {
+        discoveryContext->Release();
+    }
 }
 
 static void HandleNodeOperationalBrowse(void * context, DnssdService * result, CHIP_ERROR error)

src/platform/Darwin/DnssdContexts.cpp

@@ -564,88 +564,78 @@ void ResolveContext::DispatchSuccess()
     };
 #endif // TARGET_OS_TV
 
-    for (auto interfaceIndex : priorityInterfaceIndices)
+    std::vector<InterfaceKey> interfacesOrder;
+    for (auto priorityInterfaceIndex : priorityInterfaceIndices)
     {
-        if (interfaceIndex == 0)
+        if (priorityInterfaceIndex == 0)
         {
             // Not actually an interface we have, since if_nametoindex
             // returned 0.
             continue;
         }
 
-        if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex)))
+        for (auto & interface : interfaces)
         {
-            return;
+            if (interface.second.HasAddresses() && priorityInterfaceIndex == interface.first.interfaceId)
+            {
+                interfacesOrder.push_back(interface.first);
+            }
         }
     }
 
     for (auto & interface : interfaces)
     {
-        if (TryReportingResultsForInterfaceIndex(interface.first.interfaceId, interface.first.hostname,
-                                                 interface.first.isSRPResult))
+        // Skip interfaces that have already been prioritized to avoid duplicate results
+        auto interfaceKey = std::find(std::begin(interfacesOrder), std::end(interfacesOrder), interface.first);
+        if (interfaceKey != std::end(interfacesOrder))
         {
-            return;
+            continue;
         }
-    }
 
-    ChipLogError(Discovery, "Successfully finalizing resolve for %s without finding any actual IP addresses.",
-                 instanceName.c_str());
-}
-
-bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, const std::string & hostname, bool isSRPResult)
-{
-    InterfaceKey interfaceKey = { interfaceIndex, hostname, isSRPResult };
-    auto & interface          = interfaces[interfaceKey];
-    auto & ips                = interface.addresses;
+        // Some interface may not have any ips, just ignore them.
+        if (!interface.second.HasAddresses())
+        {
+            continue;
+        }
 
-    // Some interface may not have any ips, just ignore them.
-    if (ips.size() == 0)
-    {
-        return false;
+        interfacesOrder.push_back(interface.first);
     }
 
-    ChipLogProgress(Discovery, "Mdns: Resolve success on interface %" PRIu32, interfaceIndex);
-
-    auto & service = interface.service;
-    auto addresses = Span<Inet::IPAddress>(ips.data(), ips.size());
-    if (nullptr == callback)
+    for (auto & interfaceKey : interfacesOrder)
     {
-        auto delegate = static_cast<DiscoverNodeDelegate *>(context);
-        DiscoveredNodeData nodeData;
+        auto & interfaceInfo = interfaces[interfaceKey];
+        auto & service       = interfaceInfo.service;
+        auto & ips           = interfaceInfo.addresses;
+        auto addresses       = Span<Inet::IPAddress>(ips.data(), ips.size());
 
-        // Check whether mType (service name) exactly matches with operational service name
-        if (strcmp(service.mType, kOperationalServiceName) == 0)
+        ChipLogProgress(Discovery, "Mdns: Resolve success on interface %" PRIu32, interfaceKey.interfaceId);
+
+        if (nullptr == callback)
         {
-            service.ToDiscoveredOperationalNodeBrowseData(nodeData);
+            auto delegate = static_cast<DiscoverNodeDelegate *>(context);
+            DiscoveredNodeData nodeData;
+
+            // Check whether mType (service name) exactly matches with operational service name
+            if (strcmp(service.mType, kOperationalServiceName) == 0)
+            {
+                service.ToDiscoveredOperationalNodeBrowseData(nodeData);
+            }
+            else
+            {
+                service.ToDiscoveredCommissionNodeData(addresses, nodeData);
+            }
+            delegate->OnNodeDiscovered(nodeData);
         }
         else
         {
-            service.ToDiscoveredCommissionNodeData(addresses, nodeData);
+            CHIP_ERROR error = &interfaceKey == &interfacesOrder.back() ? CHIP_NO_ERROR : CHIP_ERROR_IN_PROGRESS;
+            callback(context, &service, addresses, error);
         }
-        delegate->OnNodeDiscovered(nodeData);
     }
-    else
-    {
-        callback(context, &service, addresses, CHIP_NO_ERROR);
-    }
-
-    return true;
-}
 
-bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex)
-{
-    for (auto & interface : interfaces)
-    {
-        if (interface.first.interfaceId == interfaceIndex)
-        {
-            if (TryReportingResultsForInterfaceIndex(interface.first.interfaceId, interface.first.hostname,
-                                                     interface.first.isSRPResult))
-            {
-                return true;
-            }
-        }
-    }
-    return false;
+    VerifyOrDo(interfacesOrder.size(),
+               ChipLogError(Discovery, "Successfully finalizing resolve for %s without finding any actual IP addresses.",
+                            instanceName.c_str()));
 }
 
 void ResolveContext::SRPTimerExpiredCallback(chip::System::Layer * systemLayer, void * callbackContext)

src/platform/Darwin/DnssdImpl.h

@@ -233,6 +233,7 @@ struct InterfaceInfo
     std::vector<Inet::IPAddress> addresses;
     std::string fullyQualifiedDomainName;
     bool isDNSLookUpRequested = false;
+    bool HasAddresses() const { return addresses.size() != 0; };
 };
 
 struct InterfaceKey
@@ -247,6 +248,11 @@ struct InterfaceKey
              (this->isSRPResult < other.isSRPResult));
     }
 
+    inline bool operator==(const InterfaceKey & other) const
+    {
+        return this->interfaceId == other.interfaceId && this->hostname == other.hostname && this->isSRPResult == other.isSRPResult;
+    }
+
     uint32_t interfaceId;
     std::string hostname;
     bool isSRPResult = false;
@@ -310,16 +316,6 @@ struct ResolveContext : public GenericContext
      *
      */
     void CancelSRPTimerIfRunning();
-
-private:
-    /**
-     * Try reporting the results we got on the provided interface index.
-     * Returns true if information was reported, false if not (e.g. if there
-     * were no IP addresses, etc).
-     */
-    bool TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, const std::string & hostname, bool isSRPResult);
-
-    bool TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex);
 };
 
 } // namespace Dnssd