Skip to content

Commit

Permalink
Reduce the boot time advertisement of addresses.
Browse files Browse the repository at this point in the history
MinMDNS advertises once for every IP address, but this makes no
sense as a multicast is per interface not per external IP.

Ensure we only broadcast once per interface.
  • Loading branch information
andreilitvin committed Jun 15, 2023
1 parent 5efdd31 commit 5a8ad4f
Showing 1 changed file with 56 additions and 84 deletions.
140 changes: 56 additions & 84 deletions src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,6 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
/// removes all records by advertising a 0 TTL)
void AdvertiseRecords(BroadcastAdvertiseType type);

/// Determine if advertisement on the specified interface/address is ok given the
/// interfaces on which the mDNS server is listening
bool ShouldAdvertiseOn(const chip::Inet::InterfaceId id, const chip::Inet::IPAddress & addr);

FullQName GetCommissioningTxtEntries(const CommissionAdvertisingParameters & params);
FullQName GetOperationalTxtEntries(OperationalQueryAllocator::Allocator * allocator,
const OperationalAdvertisingParameters & params);
Expand Down Expand Up @@ -856,35 +852,6 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis
return allocator->AllocateQNameFromArray(txtFields, numTxtFields);
}

bool AdvertiserMinMdns::ShouldAdvertiseOn(const chip::Inet::InterfaceId id, const chip::Inet::IPAddress & addr)
{
auto & server = GlobalMinimalMdnsServer::Server();

bool result = false;

server.ForEachEndPoints([&](auto * info) {
if (info->mListenUdp == nullptr)
{
return chip::Loop::Continue;
}

if (info->mInterfaceId != id)
{
return chip::Loop::Continue;
}

if (info->mAddressType != addr.Type())
{
return chip::Loop::Continue;
}

result = true;
return chip::Loop::Break;
});

return result;
}

void AdvertiserMinMdns::AdvertiseRecords(BroadcastAdvertiseType type)
{
ResponseConfiguration responseConfiguration;
Expand All @@ -905,60 +872,65 @@ void AdvertiserMinMdns::AdvertiseRecords(BroadcastAdvertiseType type)
UniquePtr<IpAddressIterator> allIps = GetAddressPolicy()->GetIpAddressesForEndpoint(interfaceId, addressType);
VerifyOrDieWithMsg(allIps != nullptr, Discovery, "Failed to allocate memory for ip addresses.");

Inet::IPAddress ipAddress;
while (allIps->Next(ipAddress))
{
if (!ShouldAdvertiseOn(interfaceId, ipAddress))
{
continue;
}
chip::Inet::IPPacketInfo packetInfo;

chip::Inet::IPPacketInfo packetInfo;
packetInfo.Clear();

packetInfo.Clear();
packetInfo.SrcAddress = ipAddress;
if (ipAddress.IsIPv4())
{
BroadcastIpAddresses::GetIpv4Into(packetInfo.DestAddress);
}
else
{
BroadcastIpAddresses::GetIpv6Into(packetInfo.DestAddress);
}
packetInfo.SrcPort = kMdnsPort;
packetInfo.DestPort = kMdnsPort;
packetInfo.Interface = interfaceId;

// Advertise all records
//
// TODO: Consider advertising delta changes.
//
// Current advertisement does not have a concept of "delta" to only
// advertise changes. Current implementation is to always
// 1. advertise TTL=0 (clear all caches)
// 2. advertise available records (with longer TTL)
//
// It would be nice if we could selectively advertise what changes, like
// send TTL=0 for anything removed/about to be removed (and only those),
// then only advertise new items added.
//
// This optimization likely will take more logic and state storage, so
// for now it is not done.
QueryData queryData(QType::PTR, QClass::IN, false /* unicast */);
queryData.SetIsInternalBroadcast(true);

for (auto & it : mOperationalResponders)
{
it.GetAllocator()->GetQueryResponder()->ClearBroadcastThrottle();
}
mQueryResponderAllocatorCommissionable.GetQueryResponder()->ClearBroadcastThrottle();
mQueryResponderAllocatorCommissioner.GetQueryResponder()->ClearBroadcastThrottle();
// advertising on every interface requires a valid IP address
// since we use "BROADCAST" (unicast is false), we do not actually care about
// the source IP address value, just that it has the right "type"
//
// NOTE: cannot use Boradcast address as the source as they have the type kAny.
//
// TODO: ideally we may want to have a destination that is explicit as "unicast/destIp"
// vs "multicast/addressType". Such a change requires larger code updates.
#if INET_CONFIG_ENABLE_IPV4
if (addressType == chip::Inet::IPAddressType::kIPv4)
{
VerifyOrDie(chip::Inet::IPAddress::FromString("127.0.0.1", packetInfo.SrcAddress));
VerifyOrDie(packetInfo.SrcAddress.Type() == chip::Inet::IPAddressType::kIPv4);
BroadcastIpAddresses::GetIpv4Into(packetInfo.DestAddress);
}
else
#endif
{
VerifyOrDie(chip::Inet::IPAddress::FromString("::1", packetInfo.SrcAddress));
VerifyOrDie(packetInfo.SrcAddress.Type() == chip::Inet::IPAddressType::kIPv6);
BroadcastIpAddresses::GetIpv6Into(packetInfo.DestAddress);
}
packetInfo.SrcPort = kMdnsPort;
packetInfo.DestPort = kMdnsPort;
packetInfo.Interface = interfaceId;

// Advertise all records
//
// TODO: Consider advertising delta changes.
//
// Current advertisement does not have a concept of "delta" to only
// advertise changes. Current implementation is to always
// 1. advertise TTL=0 (clear all caches)
// 2. advertise available records (with longer TTL)
//
// It would be nice if we could selectively advertise what changes, like
// send TTL=0 for anything removed/about to be removed (and only those),
// then only advertise new items added.
//
// This optimization likely will take more logic and state storage, so
// for now it is not done.
QueryData queryData(QType::PTR, QClass::IN, false /* unicast */);
queryData.SetIsInternalBroadcast(true);

for (auto & it : mOperationalResponders)
{
it.GetAllocator()->GetQueryResponder()->ClearBroadcastThrottle();
}
mQueryResponderAllocatorCommissionable.GetQueryResponder()->ClearBroadcastThrottle();
mQueryResponderAllocatorCommissioner.GetQueryResponder()->ClearBroadcastThrottle();

CHIP_ERROR err = mResponseSender.Respond(0, queryData, &packetInfo, responseConfiguration);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Discovery, "Failed to advertise records: %" CHIP_ERROR_FORMAT, err.Format());
}
CHIP_ERROR err = mResponseSender.Respond(0, queryData, &packetInfo, responseConfiguration);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Discovery, "Failed to advertise records: %" CHIP_ERROR_FORMAT, err.Format());
}
}

Expand Down

0 comments on commit 5a8ad4f

Please sign in to comment.