Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@nivi-apple
nivi-apple committed Mar 20, 2024
1 parent 18e3f95 commit e50da9b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 46 deletions.
22 changes: 15 additions & 7 deletions src/platform/Darwin/DnssdContexts.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,6 @@ ResolveContext::ResolveContext(void * cbContext, DnssdResolveCallback cb, chip::
callback = cb;
protocol = GetProtocol(cbAddressType);
instanceName = instanceNameToResolve;
domainName = std::string(kLocalDot);
consumerCounter = std::move(consumerCounterToUse);
}

Expand All @@ -476,7 +475,6 @@ ResolveContext::ResolveContext(CommissioningResolveDelegate * delegate, chip::In
callback = nullptr;
protocol = GetProtocol(cbAddressType);
instanceName = instanceNameToResolve;
domainName = std::string(kLocalDot);
consumerCounter = std::move(consumerCounterToUse);
}

Expand Down Expand Up @@ -528,7 +526,17 @@ void ResolveContext::DispatchSuccess()

for (auto interfaceIndex : priorityInterfaceIndices)
{
if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex)))
// 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 (needDelete)
{
Expand All @@ -540,7 +548,7 @@ void ResolveContext::DispatchSuccess()

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

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

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

Expand Down Expand Up @@ -707,7 +715,7 @@ void ResolveContext::OnNewInterface(uint32_t interfaceId, const char * fullname,
std::string domainFromHostname = GetDomainFromHostName(hostnameWithDomain);
if (domainFromHostname.empty())
{
ChipLogError(Discovery, "Mdns: Domain from hostname is empty");
ChipLogError(Discovery, "Mdns: No domain set in hostname %s", hostnameWithDomain);
return;
}

Expand Down
78 changes: 42 additions & 36 deletions src/platform/Darwin/DnssdImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ 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;

static bool hasOpenThreadTimerStarted = false;

constexpr DNSServiceFlags kRegisterFlags = kDNSServiceFlagsNoAutoRename;
constexpr DNSServiceFlags kBrowseFlags = kDNSServiceFlagsShareConnection;
constexpr DNSServiceFlags kGetAddrInfoFlags = kDNSServiceFlagsTimeout | kDNSServiceFlagsShareConnection;
Expand All @@ -58,22 +56,6 @@ std::string GetHostNameWithLocalDomain(const char * hostname)
return std::string(hostname) + '.' + kLocalDot;
}

bool HostNameHasDomain(const char * hostname, const char * domain)
{
size_t domainLength = strlen(domain);
size_t hostnameLength = strlen(hostname);
if (domainLength > hostnameLength)
{
return false;
}
const char * found = strstr(hostname, domain);
if (found == nullptr)
{
return false;
}
return (strncmp(found, domain, domainLength) == 0);
}

void LogOnFailure(const char * name, DNSServiceErrorType err)
{
if (kDNSServiceErr_NoError != err)
Expand Down Expand Up @@ -151,15 +133,31 @@ std::shared_ptr<uint32_t> GetCounterHolder(const char * name)
namespace chip {
namespace Dnssd {

std::string GetDomainFromHostName(const char * hostname)
/**
* @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)
{
if (HostNameHasDomain(hostname, kLocalDot))
{
return std::string(kLocalDot);
}
else if (HostNameHasDomain(hostname, kOpenThreadDot))
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)
{
return std::string(kOpenThreadDot);
// 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();
}
Expand All @@ -176,11 +174,14 @@ namespace {
*/
void OpenThreadTimerExpiredCallback(System::Layer * systemLayer, void * callbackContext)
{
ChipLogProgress(Discovery, "Mdns: Resolve completed on the open thread domain.");
ChipLogProgress(Discovery, "Mdns: Timer expired for resolve to complete on the open thread domain.");
auto sdCtx = static_cast<ResolveContext *>(callbackContext);
VerifyOrDie(sdCtx != nullptr);
sdCtx->Finalize();
hasOpenThreadTimerStarted = false;

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

/**
Expand Down Expand Up @@ -290,37 +291,41 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
ReturnOnFailure(MdnsContexts::GetInstance().Has(sdCtx));
LogOnFailure(__func__, err);

sdCtx->domainName = GetDomainFromHostName(hostname);
if (sdCtx->domainName.empty())
std::string domainName = GetDomainFromHostName(hostname);
if (domainName.empty())
{
ChipLogError(Discovery, "Mdns: Domain name is not set");
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, sdCtx->domainName);
std::pair<uint32_t, std::string> key = std::make_pair(interfaceId, domainName);
sdCtx->OnNewAddress(key, address);
}

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

if (sdCtx->domainName.compare(kOpenThreadDot) == 0)
if (domainName.compare(kOpenThreadDot) == 0)
{
ChipLogProgress(Discovery, "Mdns: Resolve completed on the open thread domain.");
sdCtx->Finalize();
}
else if (sdCtx->domainName.compare(kLocalDot) == 0)
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");
if (!hasOpenThreadTimerStarted)

// 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);
hasOpenThreadTimerStarted = true;
sdCtx->hasOpenThreadTimerStarted = true;
}
}
}
Expand Down Expand Up @@ -363,6 +368,7 @@ static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t inter
{
GetAddrInfo(sdCtx);
sdCtx->isResolveRequested = true;
sdCtx->hasOpenThreadTimerStarted = false;
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/platform/Darwin/DnssdImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

constexpr char kLocalDot[] = "local.";

constexpr char kOpenThreadDot[] = "openthread.thread.home.arpa";
constexpr char kOpenThreadDot[] = "default.service.arpa.";

namespace chip {
namespace Dnssd {
Expand Down Expand Up @@ -236,10 +236,10 @@ struct ResolveContext : public GenericContext
std::map<std::pair<uint32_t, std::string>, InterfaceInfo> interfaces;
DNSServiceProtocol protocol;
std::string instanceName;
std::string domainName;
bool isResolveRequested = false;
std::shared_ptr<uint32_t> consumerCounter;
BrowseContext * const browseThatCausedResolve; // Can be null
bool hasOpenThreadTimerStarted = false;

// browseCausingResolve can be null.
ResolveContext(void * cbContext, DnssdResolveCallback cb, chip::Inet::IPAddressType cbAddressType,
Expand All @@ -261,12 +261,13 @@ struct ResolveContext : public GenericContext
bool Matches(const char * otherInstanceName) const { return instanceName == otherInstanceName; }

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);
bool TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, std::string domainName);
};

} // namespace Dnssd
Expand Down

0 comments on commit e50da9b

Please sign in to comment.