Skip to content

Commit

Permalink
Fix Darwin dns-sd handling of results on interface index 0. (project-…
Browse files Browse the repository at this point in the history
…chip#32872)

project-chip#32513 introduced a bug, in
that we would ignore all results associated with interface index 0.

But that's perfectly valid for dns-sd results in general.  What we want to
ignore are cases where if_nametoindex returned 0, since those correspond to "no
such interface name".

The fix is to move "interfaceIndex == 0" check to the right spot.

The rest of the changes are just there to improve logging.  Specifically, log
what's going on with our SRP resolve timer, and include the instance name being
resolved in various logs so we can tell which logs have to do with which node
being resolved.
  • Loading branch information
bzbarsky-apple authored Apr 5, 2024
1 parent 739923c commit f46cdf3
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 18 deletions.
52 changes: 36 additions & 16 deletions src/platform/Darwin/DnssdContexts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,26 @@ void ResolveContext::DispatchSuccess()
// ChipDnssdResolveNoLongerNeeded don't find us and try to also remove us.
bool needDelete = MdnsContexts::GetInstance().RemoveWithoutDeleting(this);

class AutoSelfDeleter
{
public:
AutoSelfDeleter(bool needDelete, ResolveContext * self) : mNeedDelete(needDelete), mSelf(self) {}

~AutoSelfDeleter()
{
if (mNeedDelete)
{
MdnsContexts::GetInstance().Delete(mSelf);
}
}

private:
bool mNeedDelete;
ResolveContext * mSelf;
};

AutoSelfDeleter selfDeleter(needDelete, this);

#if TARGET_OS_TV
// On tvOS, prioritize results from en0, en1, ir0 in that order, if those
// interfaces are present, since those will generally have more up-to-date
Expand All @@ -544,12 +564,15 @@ void ResolveContext::DispatchSuccess()

for (auto interfaceIndex : priorityInterfaceIndices)
{
if (interfaceIndex == 0)
{
// Not actually an interface we have, since if_nametoindex
// returned 0.
continue;
}

if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex)))
{
if (needDelete)
{
MdnsContexts::GetInstance().Delete(this);
}
return;
}
}
Expand All @@ -559,24 +582,16 @@ void ResolveContext::DispatchSuccess()
if (TryReportingResultsForInterfaceIndex(interface.first.interfaceId, interface.first.hostname,
interface.first.isSRPResult))
{
break;
return;
}
}

if (needDelete)
{
MdnsContexts::GetInstance().Delete(this);
}
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)
{
if (interfaceIndex == 0)
{
// Not actually an interface we have.
return false;
}

InterfaceKey interfaceKey = { interfaceIndex, hostname, isSRPResult };
auto & interface = interfaces[interfaceKey];
auto & ips = interface.addresses;
Expand Down Expand Up @@ -626,12 +641,16 @@ void ResolveContext::SRPTimerExpiredCallback(chip::System::Layer * systemLayer,
{
auto sdCtx = static_cast<ResolveContext *>(callbackContext);
VerifyOrDie(sdCtx != nullptr);
sdCtx->isSRPTimerRunning = false;

ChipLogProgress(Discovery, "SRP resolve timer for %s expired; completing resolve", sdCtx->instanceName.c_str());
sdCtx->Finalize();
}

void ResolveContext::CancelSRPTimer()
{
DeviceLayer::SystemLayer().CancelTimer(SRPTimerExpiredCallback, static_cast<void *>(this));
ChipLogProgress(Discovery, "SRP resolve timer for %s cancelled; resolve timed out", instanceName.c_str());
}

CHIP_ERROR ResolveContext::OnNewAddress(const InterfaceKey & interfaceKey, const struct sockaddr * address)
Expand All @@ -655,7 +674,8 @@ CHIP_ERROR ResolveContext::OnNewAddress(const InterfaceKey & interfaceKey, const
#ifdef CHIP_PROGRESS_LOGGING
char addrStr[INET6_ADDRSTRLEN];
ip.ToString(addrStr, sizeof(addrStr));
ChipLogProgress(Discovery, "Mdns: %s interface: %" PRIu32 " ip:%s", __func__, interfaceId, addrStr);
ChipLogProgress(Discovery, "Mdns: %s instance: %s interface: %" PRIu32 " ip: %s", __func__, instanceName.c_str(), interfaceId,
addrStr);
#endif // CHIP_PROGRESS_LOGGING

if (ip.IsIPv6LinkLocal() && interfaceId == kDNSServiceInterfaceIndexLocalOnly)
Expand Down
6 changes: 4 additions & 2 deletions src/platform/Darwin/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ void LogOnFailure(const char * name, DNSServiceErrorType err)
CHIP_ERROR StartSRPTimer(uint16_t timeoutInMSecs, ResolveContext * ctx)
{
VerifyOrReturnValue(ctx != nullptr, CHIP_ERROR_INCORRECT_STATE);
ChipLogProgress(Discovery, "Starting timer to wait for possible SRP resolve results for %s", ctx->instanceName.c_str());
return chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Milliseconds16(timeoutInMSecs),
ResolveContext::SRPTimerExpiredCallback, static_cast<void *>(ctx));
}
Expand Down Expand Up @@ -280,6 +281,7 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
// we are done. Otherwise start the timer to give the resolve on SRP domain some extra time to complete.
if (!sdCtx->shouldStartSRPTimerForResolve)
{
ChipLogDetail(Discovery, "No need to start SRP resolve timer for %s; completing resolve", sdCtx->instanceName.c_str());
sdCtx->Finalize();
}
else
Expand Down Expand Up @@ -363,8 +365,8 @@ static CHIP_ERROR ResolveWithContext(ResolveContext * sdCtx, uint32_t interfaceI
static CHIP_ERROR Resolve(ResolveContext * sdCtx, uint32_t interfaceId, chip::Inet::IPAddressType addressType, const char * type,
const char * name, const char * domain)
{
ChipLogProgress(Discovery, "Resolve type=%s name=%s interface=%" PRIu32, StringOrNullMarker(type), StringOrNullMarker(name),
interfaceId);
ChipLogProgress(Discovery, "Resolve type=%s name=%s domain=%s interface=%" PRIu32, StringOrNullMarker(type),
StringOrNullMarker(name), StringOrNullMarker(domain), interfaceId);

auto err = DNSServiceCreateConnection(&sdCtx->serviceRef);
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
Expand Down

0 comments on commit f46cdf3

Please sign in to comment.