Skip to content

Commit

Permalink
DNS-SD: Implement browse retries for avahi (#28720)
Browse files Browse the repository at this point in the history 
* check

* check

* Implement retries for Avahi

The current avahi implementation uses a one-shot browse that completes
in approximately one second. This is insufficient for thread devices
that need time to propagage SRP messages through the BR. The majority
of the dnssd timeout is unused. This implements a browse retry with
backoff and also implements the StopBrowse function.

Test: chip-tool pairing started before app, now works. StopBrowse
      is checked in the TestDnssd platform tests.

* Restyled by clang-format

* Actually hook up the stop

* Fix the deletion on stop browse

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
@cecille @restyled-commits @pull
2 people authored and pull[bot] committed Oct 31, 2023
1 parent f7514b4 commit 4417412
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 9 deletions.
77 changes: 69 additions & 8 deletions src/platform/Linux/DnssdImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,8 @@ CHIP_ERROR MdnsAvahi::StopPublish()
}

CHIP_ERROR MdnsAvahi::Browse(const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context)
chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
intptr_t * browseIdentifier)
{
AvahiServiceBrowser * browser;
BrowseContext * browseContext = chip::Platform::New<BrowseContext>();
Expand All @@ -609,18 +610,40 @@ CHIP_ERROR MdnsAvahi::Browse(const char * type, DnssdServiceProtocol protocol, c
{
avahiInterface = AVAHI_IF_UNSPEC;
}
browseContext->mInterface = avahiInterface;
browseContext->mProtocol = GetFullType(type, protocol);
browseContext->mBrowseRetries = 0;
browseContext->mStopped.store(false);

browser = avahi_service_browser_new(mClient, avahiInterface, AVAHI_PROTO_UNSPEC, GetFullType(type, protocol).c_str(), nullptr,
browser = avahi_service_browser_new(mClient, avahiInterface, AVAHI_PROTO_UNSPEC, browseContext->mProtocol.c_str(), nullptr,
static_cast<AvahiLookupFlags>(0), HandleBrowse, browseContext);
// Otherwise the browser will be freed in the callback
if (browser == nullptr)
{
chip::Platform::Delete(browseContext);
*browseIdentifier = reinterpret_cast<intptr_t>(nullptr);
}
else
{
*browseIdentifier = reinterpret_cast<intptr_t>(browseContext);
}

return browser == nullptr ? CHIP_ERROR_INTERNAL : CHIP_NO_ERROR;
}

CHIP_ERROR MdnsAvahi::StopBrowse(intptr_t browseIdentifier)
{
BrowseContext * browseContext = reinterpret_cast<BrowseContext *>(browseIdentifier);
if (browseContext == nullptr)
{
return CHIP_ERROR_NOT_FOUND;
}
// Any running timers here will check mStopped before rescheduling. Leave the timer running
// so we don't race on deletion of the browse context.
browseContext->mStopped.store(true);
return CHIP_NO_ERROR;
}

DnssdServiceProtocol GetProtocolInType(const char * type)
{
const char * deliminator = strrchr(type, '.');
Expand Down Expand Up @@ -662,6 +685,27 @@ void CopyTypeWithoutProtocol(char (&dest)[N], const char * typeAndProtocol)
}
}

void MdnsAvahi::BrowseRetryCallback(chip::System::Layer * aLayer, void * appState)
{
BrowseContext * context = static_cast<BrowseContext *>(appState);
// Don't schedule anything new if we've stopped.
if (context->mStopped.load())
{
chip::Platform::Delete(context);
return;
}
AvahiServiceBrowser * newBrowser =
avahi_service_browser_new(context->mInstance->mClient, context->mInterface, AVAHI_PROTO_UNSPEC, context->mProtocol.c_str(),
nullptr, static_cast<AvahiLookupFlags>(0), HandleBrowse, context);
if (newBrowser == nullptr)
{
// If we failed to create the browser, this browse context is effectively done. We need to call the final callback and
// delete the context.
context->mCallback(context->mContext, context->mServices.data(), context->mServices.size(), true, CHIP_NO_ERROR);
chip::Platform::Delete(context);
}
}

void MdnsAvahi::HandleBrowse(AvahiServiceBrowser * browser, AvahiIfIndex interface, AvahiProtocol protocol, AvahiBrowserEvent event,
const char * name, const char * type, const char * domain, AvahiLookupResultFlags /*flags*/,
void * userdata)
Expand Down Expand Up @@ -695,12 +739,30 @@ void MdnsAvahi::HandleBrowse(AvahiServiceBrowser * browser, AvahiIfIndex interfa
context->mServices.push_back(service);
}
break;
case AVAHI_BROWSER_ALL_FOR_NOW:
case AVAHI_BROWSER_ALL_FOR_NOW: {
ChipLogProgress(DeviceLayer, "Avahi browse: all for now");
context->mCallback(context->mContext, context->mServices.data(), context->mServices.size(), true, CHIP_NO_ERROR);
bool needRetries = context->mBrowseRetries++ < kMaxBrowseRetries && !context->mStopped.load();
// If we were already asked to stop, no need to send a callback - no one is listening.
if (!context->mStopped.load())
{
context->mCallback(context->mContext, context->mServices.data(), context->mServices.size(), !needRetries,
CHIP_NO_ERROR);
}
avahi_service_browser_free(browser);
chip::Platform::Delete(context);
if (needRetries)
{
context->mNextRetryDelay *= 2;
// Hand the ownership of the context over to the timer. It will either schedule a new browse on the context,
// triggering this function, or it will delete and not reschedule (if stopped).
DeviceLayer::SystemLayer().StartTimer(context->mNextRetryDelay / 2, BrowseRetryCallback, context);
}
else
{
// We didn't schedule a timer, so we're responsible for deleting the context
chip::Platform::Delete(context);
}
break;
}
case AVAHI_BROWSER_REMOVE:
ChipLogProgress(DeviceLayer, "Avahi browse: remove");
if (strcmp("local", domain) == 0)
Expand Down Expand Up @@ -908,13 +970,12 @@ CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chi
chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
intptr_t * browseIdentifier)
{
*browseIdentifier = reinterpret_cast<intptr_t>(nullptr);
return MdnsAvahi::GetInstance().Browse(type, protocol, addressType, interface, callback, context);
return MdnsAvahi::GetInstance().Browse(type, protocol, addressType, interface, callback, context, browseIdentifier);
}

CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
return MdnsAvahi::GetInstance().StopBrowse(browseIdentifier);
}

CHIP_ERROR ChipDnssdResolve(DnssdService * browseResult, chip::Inet::InterfaceId interface, DnssdResolveCallback callback,
Expand Down
11 changes: 10 additions & 1 deletion src/platform/Linux/DnssdImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <sys/select.h>
#include <unistd.h>

#include <atomic>
#include <chrono>
#include <map>
#include <memory>
Expand Down Expand Up @@ -109,7 +110,8 @@ class MdnsAvahi
CHIP_ERROR PublishService(const DnssdService & service, DnssdPublishCallback callback, void * context);
CHIP_ERROR StopPublish();
CHIP_ERROR Browse(const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context);
chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context, intptr_t * browseIdentifier);
CHIP_ERROR StopBrowse(intptr_t browseIdentifier);
CHIP_ERROR Resolve(const char * name, const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
chip::Inet::IPAddressType transportType, chip::Inet::InterfaceId interface, DnssdResolveCallback callback,
void * context);
Expand All @@ -126,6 +128,11 @@ class MdnsAvahi
void * mContext;
Inet::IPAddressType mAddressType;
std::vector<DnssdService> mServices;
size_t mBrowseRetries;
AvahiIfIndex mInterface;
std::string mProtocol;
chip::System::Clock::Timeout mNextRetryDelay = chip::System::Clock::Seconds16(1);
std::atomic_bool mStopped{ false };
};

struct ResolveContext
Expand Down Expand Up @@ -153,6 +160,7 @@ class MdnsAvahi
static void HandleBrowse(AvahiServiceBrowser * broswer, AvahiIfIndex interface, AvahiProtocol protocol, AvahiBrowserEvent event,
const char * name, const char * type, const char * domain, AvahiLookupResultFlags flags,
void * userdata);
static void BrowseRetryCallback(chip::System::Layer * aLayer, void * appState);
static void HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex interface, AvahiProtocol protocol,
AvahiResolverEvent event, const char * name, const char * type, const char * domain,
const char * host_name, const AvahiAddress * address, uint16_t port, AvahiStringList * txt,
Expand All @@ -165,6 +173,7 @@ class MdnsAvahi
AvahiClient * mClient;
std::map<std::string, AvahiEntryGroup *> mPublishedGroups;
Poller mPoller;
static constexpr size_t kMaxBrowseRetries = 4;
};

} // namespace Dnssd
Expand Down

0 comments on commit 4417412

Please sign in to comment.