Skip to content

Commit db74b20

Browse files
woody-applewoody-apple
authored
Revert "Dnssd changes to browse and resolve using open thread domain along wi…" (project-chip#32692)
This reverts commit cc576dc.
1 parent 35fb178 commit db74b20

File tree

3 files changed

+40
-171
lines changed

3 files changed

+40
-171
lines changed

‎src/platform/Darwin/DnssdContexts.cpp

+20-32
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ namespace {
3030

3131
constexpr uint8_t kDnssdKeyMaxSize = 32;
3232
constexpr uint8_t kDnssdTxtRecordMaxEntries = 20;
33+
constexpr char kLocalDot[] = "local.";
34+
35+
bool IsLocalDomain(const char * domain)
36+
{
37+
return strcmp(kLocalDot, domain) == 0;
38+
}
3339

3440
std::string GetHostNameWithoutDomain(const char * hostnameWithDomain)
3541
{
@@ -246,7 +252,6 @@ void MdnsContexts::Delete(GenericContext * context)
246252
{
247253
DNSServiceRefDeallocate(context->serviceRef);
248254
}
249-
250255
chip::Platform::Delete(context);
251256
}
252257

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

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

395401
VerifyOrReturn(name != nullptr);
402+
VerifyOrReturn(IsLocalDomain(domain));
396403

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

446+
VerifyOrReturn(IsLocalDomain(domain));
447+
439448
auto delegate = static_cast<DnssdBrowseDelegate *>(context);
440449
auto service = GetService(name, type, protocol, interfaceId);
441450
delegate->OnBrowseAdd(service);
@@ -447,6 +456,7 @@ void BrowseWithDelegateContext::OnBrowseRemove(const char * name, const char * t
447456
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);
448457

449458
VerifyOrReturn(name != nullptr);
459+
VerifyOrReturn(IsLocalDomain(domain));
450460

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

527537
for (auto interfaceIndex : priorityInterfaceIndices)
528538
{
529-
// Try finding interfaces for domains kLocalDot and kOpenThreadDot and delete them.
530-
if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex), std::string(kLocalDot)))
531-
{
532-
if (needDelete)
533-
{
534-
MdnsContexts::GetInstance().Delete(this);
535-
}
536-
return;
537-
}
538-
539-
if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex), std::string(kOpenThreadDot)))
539+
if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex)))
540540
{
541541
if (needDelete)
542542
{
@@ -548,7 +548,7 @@ void ResolveContext::DispatchSuccess()
548548

549549
for (auto & interface : interfaces)
550550
{
551-
if (TryReportingResultsForInterfaceIndex(interface.first.first, interface.first.second))
551+
if (TryReportingResultsForInterfaceIndex(interface.first))
552552
{
553553
break;
554554
}
@@ -560,17 +560,16 @@ void ResolveContext::DispatchSuccess()
560560
}
561561
}
562562

563-
bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, std::string domainName)
563+
bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex)
564564
{
565565
if (interfaceIndex == 0)
566566
{
567567
// Not actually an interface we have.
568568
return false;
569569
}
570570

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

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

600-
CHIP_ERROR ResolveContext::OnNewAddress(const std::pair<uint32_t, std::string> interfaceKey, const struct sockaddr * address)
599+
CHIP_ERROR ResolveContext::OnNewAddress(uint32_t interfaceId, const struct sockaddr * address)
601600
{
602601
// If we don't have any information about this interfaceId, just ignore the
603602
// address, since it won't be usable anyway without things like the port.
604603
// This can happen if "local" is set up as a search domain in the DNS setup
605604
// on the system, because the hostnames we are looking up all end in
606605
// ".local". In other words, we can get regular DNS results in here, not
607606
// just DNS-SD ones.
608-
uint32_t interfaceId = interfaceKey.first;
609-
610-
if (interfaces.find(interfaceKey) == interfaces.end())
607+
if (interfaces.find(interfaceId) == interfaces.end())
611608
{
612609
return CHIP_NO_ERROR;
613610
}
@@ -630,7 +627,7 @@ CHIP_ERROR ResolveContext::OnNewAddress(const std::pair<uint32_t, std::string> i
630627
return CHIP_NO_ERROR;
631628
}
632629

633-
interfaces[interfaceKey].addresses.push_back(ip);
630+
interfaces[interfaceId].addresses.push_back(ip);
634631

635632
return CHIP_NO_ERROR;
636633
}
@@ -712,16 +709,7 @@ void ResolveContext::OnNewInterface(uint32_t interfaceId, const char * fullname,
712709
// resolving.
713710
interface.fullyQualifiedDomainName = hostnameWithDomain;
714711

715-
std::string domainFromHostname = GetDomainFromHostName(hostnameWithDomain);
716-
if (domainFromHostname.empty())
717-
{
718-
ChipLogError(Discovery, "Mdns: No domain set in hostname %s", hostnameWithDomain);
719-
return;
720-
}
721-
722-
std::pair<uint32_t, std::string> interfaceKey = std::make_pair(interfaceId, domainFromHostname);
723-
724-
interfaces.insert(std::make_pair(interfaceKey, std::move(interface)));
712+
interfaces.insert(std::make_pair(interfaceId, std::move(interface)));
725713
}
726714

727715
bool ResolveContext::HasInterface()

‎src/platform/Darwin/DnssdImpl.cpp

+17-128
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,15 @@
2626
#include <lib/support/logging/CHIPLogging.h>
2727
#include <platform/CHIPDeviceLayer.h>
2828

29-
using namespace chip;
3029
using namespace chip::Dnssd;
3130
using namespace chip::Dnssd::Internal;
3231

3332
namespace {
3433

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

3836
constexpr DNSServiceFlags kRegisterFlags = kDNSServiceFlagsNoAutoRename;
39-
constexpr DNSServiceFlags kBrowseFlags = kDNSServiceFlagsShareConnection;
37+
constexpr DNSServiceFlags kBrowseFlags = 0;
4038
constexpr DNSServiceFlags kGetAddrInfoFlags = kDNSServiceFlagsTimeout | kDNSServiceFlagsShareConnection;
4139
constexpr DNSServiceFlags kResolveFlags = kDNSServiceFlagsShareConnection;
4240
constexpr DNSServiceFlags kReconfirmRecordFlags = 0;
@@ -51,7 +49,7 @@ uint32_t GetInterfaceId(chip::Inet::InterfaceId interfaceId)
5149
return interfaceId.IsPresent() ? interfaceId.GetPlatformInterface() : kDNSServiceInterfaceIndexAny;
5250
}
5351

54-
std::string GetHostNameWithLocalDomain(const char * hostname)
52+
std::string GetHostNameWithDomain(const char * hostname)
5553
{
5654
return std::string(hostname) + '.' + kLocalDot;
5755
}
@@ -133,70 +131,10 @@ std::shared_ptr<uint32_t> GetCounterHolder(const char * name)
133131
namespace chip {
134132
namespace Dnssd {
135133

136-
/**
137-
* @brief Returns the domain name from a given hostname with domain.
138-
* The assumption here is that the hostname comprises of "hostnameWithoutDomain.<domain>."
139-
* The domainName returned from this API is "<domain>."
140-
*
141-
* @param[in] hostname The hostname with domain.
142-
*/
143-
std::string GetDomainFromHostName(const char * hostnameWithDomain)
144-
{
145-
std::string hostname = std::string(hostnameWithDomain);
146-
147-
// Find the last occurence of '.'
148-
size_t last_pos = hostname.find_last_of(".");
149-
if (last_pos != std::string::npos)
150-
{
151-
// Get a substring without last '.'
152-
std::string substring = hostname.substr(0, last_pos);
153-
154-
// Find the last occurence of '.' in the substring created above.
155-
size_t pos = substring.find_last_of(".");
156-
if (pos != std::string::npos)
157-
{
158-
// Return the domain name between the last 2 occurences of '.' including the trailing dot'.'.
159-
return std::string(hostname.substr(pos + 1, last_pos));
160-
}
161-
}
162-
return std::string();
163-
}
164-
165134
Global<MdnsContexts> MdnsContexts::sInstance;
166135

167136
namespace {
168137

169-
/**
170-
* @brief Callback that is called when the timeout for resolving on the kOpenThreadDot domain has expired.
171-
*
172-
* @param[in] systemLayer The system layer.
173-
* @param[in] callbackContext The context passed to the timer callback.
174-
*/
175-
void OpenThreadTimerExpiredCallback(System::Layer * systemLayer, void * callbackContext)
176-
{
177-
ChipLogProgress(Discovery, "Mdns: Timer expired for resolve to complete on the open thread domain.");
178-
auto sdCtx = static_cast<ResolveContext *>(callbackContext);
179-
VerifyOrDie(sdCtx != nullptr);
180-
181-
if (sdCtx->hasOpenThreadTimerStarted)
182-
{
183-
sdCtx->Finalize();
184-
}
185-
}
186-
187-
/**
188-
* @brief Starts a timer to wait for the resolution on the kOpenThreadDot domain to happen.
189-
*
190-
* @param[in] timeoutSeconds The timeout in seconds.
191-
* @param[in] ResolveContext The resolve context.
192-
*/
193-
void StartOpenThreadTimer(uint16_t timeoutInMSecs, ResolveContext * ctx)
194-
{
195-
VerifyOrReturn(ctx != nullptr, ChipLogError(Discovery, "Can't schedule open thread timer since context is null"));
196-
DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds16(timeoutInMSecs), OpenThreadTimerExpiredCallback,
197-
reinterpret_cast<void *>(ctx));
198-
}
199-
200138
static void OnRegister(DNSServiceRef sdRef, DNSServiceFlags flags, DNSServiceErrorType err, const char * name, const char * type,
201139
const char * domain, void * context)
202140
{
@@ -245,24 +183,14 @@ static void OnBrowse(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t interf
245183

246184
CHIP_ERROR Browse(BrowseHandler * sdCtx, uint32_t interfaceId, const char * type)
247185
{
248-
auto err = DNSServiceCreateConnection(&sdCtx->serviceRef);
249-
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
250-
251-
// We will browse on both the local domain and the open thread domain.
252-
ChipLogProgress(Discovery, "Browsing for: %s on domain %s", StringOrNullMarker(type), kLocalDot);
253-
254-
auto sdRefLocal = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
255-
err = DNSServiceBrowse(&sdRefLocal, kBrowseFlags, interfaceId, type, kLocalDot, OnBrowse, sdCtx);
256-
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
257-
258-
ChipLogProgress(Discovery, "Browsing for: %s on domain %s", StringOrNullMarker(type), kOpenThreadDot);
259-
260-
DNSServiceRef sdRefOpenThread = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
261-
err = DNSServiceBrowse(&sdRefOpenThread, kBrowseFlags, interfaceId, type, kOpenThreadDot, OnBrowse, sdCtx);
186+
ChipLogProgress(Discovery, "Browsing for: %s", StringOrNullMarker(type));
187+
DNSServiceRef sdRef;
188+
auto err = DNSServiceBrowse(&sdRef, kBrowseFlags, interfaceId, type, kLocalDot, OnBrowse, sdCtx);
262189
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
263190

264-
return MdnsContexts::GetInstance().Add(sdCtx, sdCtx->serviceRef);
191+
return MdnsContexts::GetInstance().Add(sdCtx, sdRef);
265192
}
193+
266194
CHIP_ERROR Browse(void * context, DnssdBrowseCallback callback, uint32_t interfaceId, const char * type,
267195
DnssdServiceProtocol protocol, intptr_t * browseIdentifier)
268196
{
@@ -291,52 +219,25 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
291219
ReturnOnFailure(MdnsContexts::GetInstance().Has(sdCtx));
292220
LogOnFailure(__func__, err);
293221

294-
std::string domainName = GetDomainFromHostName(hostname);
295-
if (domainName.empty())
296-
{
297-
ChipLogError(Discovery, "Mdns: Domain name is not set in hostname %s", hostname);
298-
return;
299-
}
300222
if (kDNSServiceErr_NoError == err)
301223
{
302-
std::pair<uint32_t, std::string> key = std::make_pair(interfaceId, domainName);
303-
sdCtx->OnNewAddress(key, address);
224+
sdCtx->OnNewAddress(interfaceId, address);
304225
}
305226

306227
if (!(flags & kDNSServiceFlagsMoreComing))
307228
{
308229
VerifyOrReturn(sdCtx->HasAddress(), sdCtx->Finalize(kDNSServiceErr_BadState));
309-
310-
if (domainName.compare(kOpenThreadDot) == 0)
311-
{
312-
ChipLogProgress(Discovery, "Mdns: Resolve completed on the open thread domain.");
313-
sdCtx->Finalize();
314-
}
315-
else if (domainName.compare(kLocalDot) == 0)
316-
{
317-
ChipLogProgress(
318-
Discovery,
319-
"Mdns: Resolve completed on the local domain. Starting a timer for the open thread resolve to come back");
320-
321-
// Usually the resolution on the local domain is quicker than on the open thread domain. We would like to give the
322-
// resolution on the open thread domain around 250 millisecs more to give it a chance to resolve before finalizing
323-
// the resolution.
324-
if (!sdCtx->hasOpenThreadTimerStarted)
325-
{
326-
// Schedule a timer to allow the resolve on OpenThread domain to complete.
327-
StartOpenThreadTimer(kOpenThreadTimeoutInMsec, sdCtx);
328-
sdCtx->hasOpenThreadTimerStarted = true;
329-
}
330-
}
230+
sdCtx->Finalize();
331231
}
332232
}
333233

334234
static void GetAddrInfo(ResolveContext * sdCtx)
335235
{
336236
auto protocol = sdCtx->protocol;
237+
337238
for (auto & interface : sdCtx->interfaces)
338239
{
339-
auto interfaceId = interface.first.first;
240+
auto interfaceId = interface.first;
340241
auto hostname = interface.second.fullyQualifiedDomainName.c_str();
341242
auto sdRefCopy = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
342243
auto err = DNSServiceGetAddrInfo(&sdRefCopy, kGetAddrInfoFlags, interfaceId, protocol, hostname, OnGetAddrInfo, sdCtx);
@@ -362,14 +263,7 @@ static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t inter
362263
if (!(flags & kDNSServiceFlagsMoreComing))
363264
{
364265
VerifyOrReturn(sdCtx->HasInterface(), sdCtx->Finalize(kDNSServiceErr_BadState));
365-
366-
// If a resolve was not requested on this context, call GetAddrInfo and set the isResolveRequested flag to true.
367-
if (!sdCtx->isResolveRequested)
368-
{
369-
GetAddrInfo(sdCtx);
370-
sdCtx->isResolveRequested = true;
371-
sdCtx->hasOpenThreadTimerStarted = false;
372-
}
266+
GetAddrInfo(sdCtx);
373267
}
374268
}
375269

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

385-
// Similar to browse, will try to resolve using both the local domain and the open thread domain.
386-
auto sdRefLocal = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
387-
err = DNSServiceResolve(&sdRefLocal, kResolveFlags, interfaceId, name, type, kLocalDot, OnResolve, sdCtx);
388-
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
389-
390-
auto sdRefOpenThread = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
391-
err = DNSServiceResolve(&sdRefOpenThread, kResolveFlags, interfaceId, name, type, kOpenThreadDot, OnResolve, sdCtx);
279+
auto sdRefCopy = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
280+
err = DNSServiceResolve(&sdRefCopy, kResolveFlags, interfaceId, name, type, kLocalDot, OnResolve, sdCtx);
392281
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
393282

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

451340
auto regtype = GetFullTypeWithSubTypes(service);
452341
auto interfaceId = GetInterfaceId(service->mInterface);
453-
auto hostname = GetHostNameWithLocalDomain(service->mHostName);
342+
auto hostname = GetHostNameWithDomain(service->mHostName);
454343

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

597486
auto interfaceId = interface.GetPlatformInterface();
598487
auto rrclass = kDNSServiceClass_IN;
599-
auto fullname = GetHostNameWithLocalDomain(hostname);
488+
auto fullname = GetHostNameWithDomain(hostname);
600489

601490
uint16_t rrtype;
602491
uint16_t rdlen;

0 commit comments

Comments
 (0)