From c6ad5b160b64c8b7d76a16d1dde169ed2c7ca175 Mon Sep 17 00:00:00 2001 From: Vivien Nicolas Date: Mon, 30 Sep 2024 18:39:16 +0200 Subject: [PATCH] =?UTF-8?q?Report=20mdns=20results=20from=20all=20interfac?= =?UTF-8?q?es=20instead=20of=20the=20highest=20priori=E2=80=A6=20(#35597)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [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 --- .../chip-tool/commands/discover/Commands.h | 1 + .../DiscoverCommissionablesCommand.cpp | 8 ++ .../discover/DiscoverCommissionablesCommand.h | 16 +++ src/lib/dnssd/Discovery_ImplPlatform.cpp | 10 +- src/platform/Darwin/DnssdContexts.cpp | 98 +++++++++---------- src/platform/Darwin/DnssdImpl.h | 16 ++- 6 files changed, 83 insertions(+), 66 deletions(-) diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h index d308d4ab75b430..d70191f8bc8245 100644 --- a/examples/chip-tool/commands/discover/Commands.h +++ b/examples/chip-tool/commands/discover/Commands.h @@ -84,6 +84,7 @@ void registerCommandsDiscover(Commands & commands, CredentialIssuerCommands * cr make_unique(credsIssuerConfig), make_unique(credsIssuerConfig), make_unique(credsIssuerConfig), + make_unique(credsIssuerConfig), make_unique(credsIssuerConfig), }; diff --git a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp b/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp index ad5e7feaf7f948..35d833e5ad678a 100644 --- a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp +++ b/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); +} diff --git a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h b/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h index d1fec307eac19e..2ba81b6404efc8 100644 --- a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h +++ b/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; +}; diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 54947cd617bf0e..6f98ac71b34a38 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -43,7 +43,7 @@ static void HandleNodeResolve(void * context, DnssdService * result, const Span< { DiscoveryContext * discoveryContext = static_cast(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().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) diff --git a/src/platform/Darwin/DnssdContexts.cpp b/src/platform/Darwin/DnssdContexts.cpp index 9d293c089665e2..e6b590d09eb6ec 100644 --- a/src/platform/Darwin/DnssdContexts.cpp +++ b/src/platform/Darwin/DnssdContexts.cpp @@ -564,88 +564,78 @@ void ResolveContext::DispatchSuccess() }; #endif // TARGET_OS_TV - for (auto interfaceIndex : priorityInterfaceIndices) + std::vector 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(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(ips.data(), ips.size()); - if (nullptr == callback) + for (auto & interfaceKey : interfacesOrder) { - auto delegate = static_cast(context); - DiscoveredNodeData nodeData; + auto & interfaceInfo = interfaces[interfaceKey]; + auto & service = interfaceInfo.service; + auto & ips = interfaceInfo.addresses; + auto addresses = Span(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(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) diff --git a/src/platform/Darwin/DnssdImpl.h b/src/platform/Darwin/DnssdImpl.h index d4cd05fecfe20b..9d10b4b3a13444 100644 --- a/src/platform/Darwin/DnssdImpl.h +++ b/src/platform/Darwin/DnssdImpl.h @@ -233,6 +233,7 @@ struct InterfaceInfo std::vector 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