Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Dnssd changes to browse and resolve using open thread domain along wi…" #32692

Merged
merged 1 commit into from
Mar 22, 2024
Merged

Revert "Dnssd changes to browse and resolve using open thread domain along wi…" #32692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 20 additions & 32 deletions src/platform/Darwin/DnssdContexts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ namespace {

constexpr uint8_t kDnssdKeyMaxSize = 32;
constexpr uint8_t kDnssdTxtRecordMaxEntries = 20;
constexpr char kLocalDot[] = "local.";

bool IsLocalDomain(const char * domain)
{
return strcmp(kLocalDot, domain) == 0;
}

std::string GetHostNameWithoutDomain(const char * hostnameWithDomain)
{
Expand Down Expand Up @@ -246,7 +252,6 @@ void MdnsContexts::Delete(GenericContext * context)
{
DNSServiceRefDeallocate(context->serviceRef);
}

chip::Platform::Delete(context);
}

Expand Down Expand Up @@ -383,6 +388,7 @@ void BrowseContext::OnBrowseAdd(const char * name, const char * type, const char
ChipLogProgress(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, interface: %" PRIu32, __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);

VerifyOrReturn(IsLocalDomain(domain));
auto service = GetService(name, type, protocol, interfaceId);
services.push_back(service);
}
Expand All @@ -393,6 +399,7 @@ void BrowseContext::OnBrowseRemove(const char * name, const char * type, const c
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);

VerifyOrReturn(name != nullptr);
VerifyOrReturn(IsLocalDomain(domain));

services.erase(std::remove_if(services.begin(), services.end(),
[name, type, interfaceId](const DnssdService & service) {
Expand Down Expand Up @@ -436,6 +443,8 @@ void BrowseWithDelegateContext::OnBrowseAdd(const char * name, const char * type
ChipLogProgress(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, interface: %" PRIu32, __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);

VerifyOrReturn(IsLocalDomain(domain));

auto delegate = static_cast<DnssdBrowseDelegate *>(context);
auto service = GetService(name, type, protocol, interfaceId);
delegate->OnBrowseAdd(service);
Expand All @@ -447,6 +456,7 @@ void BrowseWithDelegateContext::OnBrowseRemove(const char * name, const char * t
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);

VerifyOrReturn(name != nullptr);
VerifyOrReturn(IsLocalDomain(domain));

auto delegate = static_cast<DnssdBrowseDelegate *>(context);
auto service = GetService(name, type, protocol, interfaceId);
Expand Down Expand Up @@ -526,17 +536,7 @@ void ResolveContext::DispatchSuccess()

for (auto interfaceIndex : priorityInterfaceIndices)
{
// Try finding interfaces for domains kLocalDot and kOpenThreadDot and delete them.
if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex), std::string(kLocalDot)))
{
if (needDelete)
{
MdnsContexts::GetInstance().Delete(this);
}
return;
}

if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex), std::string(kOpenThreadDot)))
if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex)))
{
if (needDelete)
{
Expand All @@ -548,7 +548,7 @@ void ResolveContext::DispatchSuccess()

for (auto & interface : interfaces)
{
if (TryReportingResultsForInterfaceIndex(interface.first.first, interface.first.second))
if (TryReportingResultsForInterfaceIndex(interface.first))
{
break;
}
Expand All @@ -560,17 +560,16 @@ void ResolveContext::DispatchSuccess()
}
}

bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, std::string domainName)
bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex)
{
if (interfaceIndex == 0)
{
// Not actually an interface we have.
return false;
}

std::pair<uint32_t, std::string> interfaceKey = std::make_pair(interfaceIndex, domainName);
auto & interface = interfaces[interfaceKey];
auto & ips = interface.addresses;
auto & interface = interfaces[interfaceIndex];
auto & ips = interface.addresses;

// Some interface may not have any ips, just ignore them.
if (ips.size() == 0)
Expand All @@ -597,17 +596,15 @@ bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceInde
return true;
}

CHIP_ERROR ResolveContext::OnNewAddress(const std::pair<uint32_t, std::string> interfaceKey, const struct sockaddr * address)
CHIP_ERROR ResolveContext::OnNewAddress(uint32_t interfaceId, const struct sockaddr * address)
{
// If we don't have any information about this interfaceId, just ignore the
// address, since it won't be usable anyway without things like the port.
// This can happen if "local" is set up as a search domain in the DNS setup
// on the system, because the hostnames we are looking up all end in
// ".local". In other words, we can get regular DNS results in here, not
// just DNS-SD ones.
uint32_t interfaceId = interfaceKey.first;

if (interfaces.find(interfaceKey) == interfaces.end())
if (interfaces.find(interfaceId) == interfaces.end())
{
return CHIP_NO_ERROR;
}
Expand All @@ -630,7 +627,7 @@ CHIP_ERROR ResolveContext::OnNewAddress(const std::pair<uint32_t, std::string> i
return CHIP_NO_ERROR;
}

interfaces[interfaceKey].addresses.push_back(ip);
interfaces[interfaceId].addresses.push_back(ip);

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -712,16 +709,7 @@ void ResolveContext::OnNewInterface(uint32_t interfaceId, const char * fullname,
// resolving.
interface.fullyQualifiedDomainName = hostnameWithDomain;

std::string domainFromHostname = GetDomainFromHostName(hostnameWithDomain);
if (domainFromHostname.empty())
{
ChipLogError(Discovery, "Mdns: No domain set in hostname %s", hostnameWithDomain);
return;
}

std::pair<uint32_t, std::string> interfaceKey = std::make_pair(interfaceId, domainFromHostname);

interfaces.insert(std::make_pair(interfaceKey, std::move(interface)));
interfaces.insert(std::make_pair(interfaceId, std::move(interface)));
}

bool ResolveContext::HasInterface()
Expand Down
145 changes: 17 additions & 128 deletions src/platform/Darwin/DnssdImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,15 @@
#include <lib/support/logging/CHIPLogging.h>
#include <platform/CHIPDeviceLayer.h>

using namespace chip;
using namespace chip::Dnssd;
using namespace chip::Dnssd::Internal;

namespace {

// The extra time in milliseconds that we will wait for the resolution on the open thread domain to complete.
constexpr uint16_t kOpenThreadTimeoutInMsec = 250;
constexpr char kLocalDot[] = "local.";

constexpr DNSServiceFlags kRegisterFlags = kDNSServiceFlagsNoAutoRename;
constexpr DNSServiceFlags kBrowseFlags = kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kBrowseFlags = 0;
constexpr DNSServiceFlags kGetAddrInfoFlags = kDNSServiceFlagsTimeout | kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kResolveFlags = kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kReconfirmRecordFlags = 0;
Expand All @@ -51,7 +49,7 @@ uint32_t GetInterfaceId(chip::Inet::InterfaceId interfaceId)
return interfaceId.IsPresent() ? interfaceId.GetPlatformInterface() : kDNSServiceInterfaceIndexAny;
}

std::string GetHostNameWithLocalDomain(const char * hostname)
std::string GetHostNameWithDomain(const char * hostname)
{
return std::string(hostname) + '.' + kLocalDot;
}
Expand Down Expand Up @@ -133,70 +131,10 @@ std::shared_ptr<uint32_t> GetCounterHolder(const char * name)
namespace chip {
namespace Dnssd {

/**
* @brief Returns the domain name from a given hostname with domain.
* The assumption here is that the hostname comprises of "hostnameWithoutDomain.<domain>."
* The domainName returned from this API is "<domain>."
*
* @param[in] hostname The hostname with domain.
*/
std::string GetDomainFromHostName(const char * hostnameWithDomain)
{
std::string hostname = std::string(hostnameWithDomain);

// Find the last occurence of '.'
size_t last_pos = hostname.find_last_of(".");
if (last_pos != std::string::npos)
{
// Get a substring without last '.'
std::string substring = hostname.substr(0, last_pos);

// Find the last occurence of '.' in the substring created above.
size_t pos = substring.find_last_of(".");
if (pos != std::string::npos)
{
// Return the domain name between the last 2 occurences of '.' including the trailing dot'.'.
return std::string(hostname.substr(pos + 1, last_pos));
}
}
return std::string();
}

Global<MdnsContexts> MdnsContexts::sInstance;

namespace {

/**
* @brief Callback that is called when the timeout for resolving on the kOpenThreadDot domain has expired.
*
* @param[in] systemLayer The system layer.
* @param[in] callbackContext The context passed to the timer callback.
*/
void OpenThreadTimerExpiredCallback(System::Layer * systemLayer, void * callbackContext)
{
ChipLogProgress(Discovery, "Mdns: Timer expired for resolve to complete on the open thread domain.");
auto sdCtx = static_cast<ResolveContext *>(callbackContext);
VerifyOrDie(sdCtx != nullptr);

if (sdCtx->hasOpenThreadTimerStarted)
{
sdCtx->Finalize();
}
}

/**
* @brief Starts a timer to wait for the resolution on the kOpenThreadDot domain to happen.
*
* @param[in] timeoutSeconds The timeout in seconds.
* @param[in] ResolveContext The resolve context.
*/
void StartOpenThreadTimer(uint16_t timeoutInMSecs, ResolveContext * ctx)
{
VerifyOrReturn(ctx != nullptr, ChipLogError(Discovery, "Can't schedule open thread timer since context is null"));
DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds16(timeoutInMSecs), OpenThreadTimerExpiredCallback,
reinterpret_cast<void *>(ctx));
}

static void OnRegister(DNSServiceRef sdRef, DNSServiceFlags flags, DNSServiceErrorType err, const char * name, const char * type,
const char * domain, void * context)
{
Expand Down Expand Up @@ -245,24 +183,14 @@ static void OnBrowse(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t interf

CHIP_ERROR Browse(BrowseHandler * sdCtx, uint32_t interfaceId, const char * type)
{
auto err = DNSServiceCreateConnection(&sdCtx->serviceRef);
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));

// We will browse on both the local domain and the open thread domain.
ChipLogProgress(Discovery, "Browsing for: %s on domain %s", StringOrNullMarker(type), kLocalDot);

auto sdRefLocal = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
err = DNSServiceBrowse(&sdRefLocal, kBrowseFlags, interfaceId, type, kLocalDot, OnBrowse, sdCtx);
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));

ChipLogProgress(Discovery, "Browsing for: %s on domain %s", StringOrNullMarker(type), kOpenThreadDot);

DNSServiceRef sdRefOpenThread = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
err = DNSServiceBrowse(&sdRefOpenThread, kBrowseFlags, interfaceId, type, kOpenThreadDot, OnBrowse, sdCtx);
ChipLogProgress(Discovery, "Browsing for: %s", StringOrNullMarker(type));
DNSServiceRef sdRef;
auto err = DNSServiceBrowse(&sdRef, kBrowseFlags, interfaceId, type, kLocalDot, OnBrowse, sdCtx);
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));

return MdnsContexts::GetInstance().Add(sdCtx, sdCtx->serviceRef);
return MdnsContexts::GetInstance().Add(sdCtx, sdRef);
}

CHIP_ERROR Browse(void * context, DnssdBrowseCallback callback, uint32_t interfaceId, const char * type,
DnssdServiceProtocol protocol, intptr_t * browseIdentifier)
{
Expand Down Expand Up @@ -291,52 +219,25 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
ReturnOnFailure(MdnsContexts::GetInstance().Has(sdCtx));
LogOnFailure(__func__, err);

std::string domainName = GetDomainFromHostName(hostname);
if (domainName.empty())
{
ChipLogError(Discovery, "Mdns: Domain name is not set in hostname %s", hostname);
return;
}
if (kDNSServiceErr_NoError == err)
{
std::pair<uint32_t, std::string> key = std::make_pair(interfaceId, domainName);
sdCtx->OnNewAddress(key, address);
sdCtx->OnNewAddress(interfaceId, address);
}

if (!(flags & kDNSServiceFlagsMoreComing))
{
VerifyOrReturn(sdCtx->HasAddress(), sdCtx->Finalize(kDNSServiceErr_BadState));

if (domainName.compare(kOpenThreadDot) == 0)
{
ChipLogProgress(Discovery, "Mdns: Resolve completed on the open thread domain.");
sdCtx->Finalize();
}
else if (domainName.compare(kLocalDot) == 0)
{
ChipLogProgress(
Discovery,
"Mdns: Resolve completed on the local domain. Starting a timer for the open thread resolve to come back");

// Usually the resolution on the local domain is quicker than on the open thread domain. We would like to give the
// resolution on the open thread domain around 250 millisecs more to give it a chance to resolve before finalizing
// the resolution.
if (!sdCtx->hasOpenThreadTimerStarted)
{
// Schedule a timer to allow the resolve on OpenThread domain to complete.
StartOpenThreadTimer(kOpenThreadTimeoutInMsec, sdCtx);
sdCtx->hasOpenThreadTimerStarted = true;
}
}
sdCtx->Finalize();
}
}

static void GetAddrInfo(ResolveContext * sdCtx)
{
auto protocol = sdCtx->protocol;

for (auto & interface : sdCtx->interfaces)
{
auto interfaceId = interface.first.first;
auto interfaceId = interface.first;
auto hostname = interface.second.fullyQualifiedDomainName.c_str();
auto sdRefCopy = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
auto err = DNSServiceGetAddrInfo(&sdRefCopy, kGetAddrInfoFlags, interfaceId, protocol, hostname, OnGetAddrInfo, sdCtx);
Expand All @@ -362,14 +263,7 @@ static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t inter
if (!(flags & kDNSServiceFlagsMoreComing))
{
VerifyOrReturn(sdCtx->HasInterface(), sdCtx->Finalize(kDNSServiceErr_BadState));

// If a resolve was not requested on this context, call GetAddrInfo and set the isResolveRequested flag to true.
if (!sdCtx->isResolveRequested)
{
GetAddrInfo(sdCtx);
sdCtx->isResolveRequested = true;
sdCtx->hasOpenThreadTimerStarted = false;
}
GetAddrInfo(sdCtx);
}
}

Expand All @@ -382,13 +276,8 @@ static CHIP_ERROR Resolve(ResolveContext * sdCtx, uint32_t interfaceId, chip::In
auto err = DNSServiceCreateConnection(&sdCtx->serviceRef);
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));

// Similar to browse, will try to resolve using both the local domain and the open thread domain.
auto sdRefLocal = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
err = DNSServiceResolve(&sdRefLocal, kResolveFlags, interfaceId, name, type, kLocalDot, OnResolve, sdCtx);
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));

auto sdRefOpenThread = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
err = DNSServiceResolve(&sdRefOpenThread, kResolveFlags, interfaceId, name, type, kOpenThreadDot, OnResolve, sdCtx);
auto sdRefCopy = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
err = DNSServiceResolve(&sdRefCopy, kResolveFlags, interfaceId, name, type, kLocalDot, OnResolve, sdCtx);
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));

auto retval = MdnsContexts::GetInstance().Add(sdCtx, sdCtx->serviceRef);
Expand Down Expand Up @@ -450,7 +339,7 @@ CHIP_ERROR ChipDnssdPublishService(const DnssdService * service, DnssdPublishCal

auto regtype = GetFullTypeWithSubTypes(service);
auto interfaceId = GetInterfaceId(service->mInterface);
auto hostname = GetHostNameWithLocalDomain(service->mHostName);
auto hostname = GetHostNameWithDomain(service->mHostName);

return Register(context, callback, interfaceId, regtype.c_str(), service->mName, service->mPort, record, service->mAddressType,
hostname.c_str());
Expand Down Expand Up @@ -596,7 +485,7 @@ CHIP_ERROR ChipDnssdReconfirmRecord(const char * hostname, chip::Inet::IPAddress

auto interfaceId = interface.GetPlatformInterface();
auto rrclass = kDNSServiceClass_IN;
auto fullname = GetHostNameWithLocalDomain(hostname);
auto fullname = GetHostNameWithDomain(hostname);

uint16_t rrtype;
uint16_t rdlen;
Expand Down
Loading
Loading