Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDNS memory optimization #27612

Merged
merged 20 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
80cbbc7
Reduce number of replies on MinMds query processing.
andreilitvin Jun 9, 2023
15dccc8
Restyled by clang-format
restyled-commits Jun 9, 2023
c3fbd1e
Make test advertiser support multi-admin logic to test packet content
andreilitvin Jun 15, 2023
9dfe146
Make IP addresses sent only once
andreilitvin Jun 15, 2023
853c8ba
Update advertiser to make srv port unique
andreilitvin Jun 15, 2023
2ce5489
Update comment a bit
andreilitvin Jun 15, 2023
8f70f2c
Do not list dnssd service listing at boot time advertisement
andreilitvin Jun 15, 2023
2cdbb20
Clang-format and mark boot time advertisements to have everything as …
andreilitvin Jun 15, 2023
7862219
Add missing break
andreilitvin Jun 15, 2023
e7039d7
Restyled by clang-format
restyled-commits Jun 15, 2023
4e26657
rg InternalBroadcast --files-with-matches | xargs sd 'InternalBroadca…
andreilitvin Jun 16, 2023
9845899
Rename Accept to ShouldSend
andreilitvin Jun 16, 2023
a09abd8
Reduce the boot time advertisement of addresses.
andreilitvin Jun 15, 2023
069b85d
Remove unused method
andreilitvin Jun 15, 2023
6b5b3fe
Update src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
andy31415 Jun 16, 2023
8f32769
Make loopback a method in IPAddress as it seems convenient and does n…
andreilitvin Jun 16, 2023
9ccc202
Clean up Broadcast IP address get now that we cleaned up loopback
andreilitvin Jun 16, 2023
4d68dbe
Restyled by clang-format
restyled-commits Jun 16, 2023
ef2955c
VerifyOrDie if broadcast addresses are wrong
andreilitvin Jun 16, 2023
0e79f95
Fix CI failure for linux arm
Jul 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 73 additions & 37 deletions examples/minimal-mdns/advertiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum class AdvertisingMode
{
kCommissionableNode,
kOperational,
kOperationalMultiAdmin,
kCommissioner,
};

Expand Down Expand Up @@ -95,6 +96,10 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
{
gOptions.advertisingMode = AdvertisingMode::kOperational;
}
else if (strcmp(aValue, "operational-multi-admin") == 0)
{
gOptions.advertisingMode = AdvertisingMode::kOperationalMultiAdmin;
}
else if (strcmp(aValue, "commissionable-node") == 0)
{
gOptions.advertisingMode = AdvertisingMode::kCommissionableNode;
Expand Down Expand Up @@ -190,45 +195,47 @@ OptionDef cmdLineOptionsDef[] = {
{},
};

OptionSet cmdLineOptions = { HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS",
OptionSet cmdLineOptions = {
HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS",
#if INET_CONFIG_ENABLE_IPV4
" -4\n"
" --enable-ip-v4\n"
" enable listening on IPv4\n"
" -4\n"
" --enable-ip-v4\n"
" enable listening on IPv4\n"
#endif
" -m <mode>\n"
" --advertising-mode <mode>\n"
" Advertise in this mode (operational or commissionable-node or commissioner).\n"
" --short-discriminator <value>\n"
" -s <value>\n"
" Commissioning/commissionable short discriminator.\n"
" --long-discriminator <value>\n"
" -l <value>\n"
" Commissioning/commissionable long discriminator.\n"
" --vendor-id <value>\n"
" Commissioning/commissionable vendor id.\n"
" --product-id <value>\n"
" -p <value>\n"
" Commissioning/commissionable product id.\n"
" --commissioning-mode <value>\n"
" Commissioning Mode (0=disabled, 1=basic, 2=enhanced).\n"
" --device-type <value>\n"
" Device type id.\n"
" --device-name <value>\n"
" Name of device.\n"
" --rotating-id <value>\n"
" Rotating Id.\n"
" --pairing-instruction <value>\n"
" Commissionable pairing instruction.\n"
" --pairing-hint <value>\n"
" Commissionable pairing hint.\n"
" --fabric-id <value>\n"
" -f <value>\n"
" Operational fabric id.\n"
" --node-id <value>\n"
" -n <value>\n"
" Operational node id.\n"
"\n" };
" -m <mode>\n"
" --advertising-mode <mode>\n"
" Advertise in this mode (operational, operational-multi-admin, commissionable-node or commissioner).\n"
" --short-discriminator <value>\n"
" -s <value>\n"
" Commissioning/commissionable short discriminator.\n"
" --long-discriminator <value>\n"
" -l <value>\n"
" Commissioning/commissionable long discriminator.\n"
" --vendor-id <value>\n"
" Commissioning/commissionable vendor id.\n"
" --product-id <value>\n"
" -p <value>\n"
" Commissioning/commissionable product id.\n"
" --commissioning-mode <value>\n"
" Commissioning Mode (0=disabled, 1=basic, 2=enhanced).\n"
" --device-type <value>\n"
" Device type id.\n"
" --device-name <value>\n"
" Name of device.\n"
" --rotating-id <value>\n"
" Rotating Id.\n"
" --pairing-instruction <value>\n"
" Commissionable pairing instruction.\n"
" --pairing-hint <value>\n"
" Commissionable pairing hint.\n"
" --fabric-id <value>\n"
" -f <value>\n"
" Operational fabric id.\n"
" --node-id <value>\n"
" -n <value>\n"
" Operational node id.\n"
" -t <dest>\n"
};

HelpOptions helpOptions("advertiser", "Usage: advertiser [options]", "1.0");

Expand Down Expand Up @@ -290,6 +297,35 @@ int main(int argc, char ** args)
.SetMac(chip::ByteSpan(gOptions.mac, 6))
.SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId).SetNodeId(gOptions.nodeId)));
}
else if (gOptions.advertisingMode == AdvertisingMode::kOperationalMultiAdmin)
{
err = chip::Dnssd::ServiceAdvertiser::Instance().Advertise(
chip::Dnssd::OperationalAdvertisingParameters()
.EnableIpV4(gOptions.enableIpV4)
.SetPort(CHIP_PORT)
.SetMac(chip::ByteSpan(gOptions.mac, 6))
.SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId).SetNodeId(gOptions.nodeId)));

if (err == CHIP_NO_ERROR)
{
err = chip::Dnssd::ServiceAdvertiser::Instance().Advertise(
chip::Dnssd::OperationalAdvertisingParameters()
.EnableIpV4(gOptions.enableIpV4)
.SetPort(CHIP_PORT + 1)
.SetMac(chip::ByteSpan(gOptions.mac, 6))
.SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId + 1).SetNodeId(gOptions.nodeId + 1)));
}

if (err == CHIP_NO_ERROR)
{
err = chip::Dnssd::ServiceAdvertiser::Instance().Advertise(
chip::Dnssd::OperationalAdvertisingParameters()
.EnableIpV4(gOptions.enableIpV4)
.SetPort(CHIP_PORT + 2)
.SetMac(chip::ByteSpan(gOptions.mac, 6))
.SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId + 2).SetNodeId(gOptions.nodeId + 2)));
}
}
else if (gOptions.advertisingMode == AdvertisingMode::kCommissioner)
{
printf("Advertise Commissioner\n");
Expand Down
23 changes: 23 additions & 0 deletions src/inet/IPAddress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -508,5 +508,28 @@ IPAddress IPAddress::MakeIPv4Broadcast()
return ipAddr;
}

IPAddress IPAddress::Loopback(IPAddressType type)
{
IPAddress address;
#if INET_CONFIG_ENABLE_IPV4
if (type == IPAddressType::kIPv4)
{
address.Addr[0] = 0;
address.Addr[1] = 0;
address.Addr[2] = htonl(0xFFFF);
address.Addr[3] = htonl(0x7F000001);
}
else
#endif
{
address.Addr[0] = 0;
address.Addr[1] = 0;
address.Addr[2] = 0;
address.Addr[3] = htonl(1);
}

return address;
}

} // namespace Inet
} // namespace chip
8 changes: 8 additions & 0 deletions src/inet/IPAddress.h
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,14 @@ class DLL_EXPORT IPAddress
* not be modified by users of the CHIP Inet Layer.
*/
static IPAddress Any;

/**
* Creates a loopback of the specified type. Type MUST be IPv6/v4.
*
* If type is anything else (or IPv4 is not available) an IPv6
* loopback will be created.
*/
static IPAddress Loopback(IPAddressType type);
};

static_assert(std::is_trivial<IPAddress>::value, "IPAddress is not trivial");
Expand Down
130 changes: 45 additions & 85 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,53 @@ 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))
chip::Inet::IPPacketInfo packetInfo;

packetInfo.Clear();

// 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 Broadcast 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.
packetInfo.SrcAddress = chip::Inet::IPAddress::Loopback(addressType);
packetInfo.DestAddress = BroadcastIpAddresses::Get(addressType);
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.SetIsAnnounceBroadcast(true);

for (auto & it : mOperationalResponders)
{
if (!ShouldAdvertiseOn(interfaceId, ipAddress))
{
continue;
}

chip::Inet::IPPacketInfo packetInfo;

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();
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
6 changes: 3 additions & 3 deletions src/lib/dnssd/minimal_mdns/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class QueryData
/// any broadcast filtering. Intent is for paths such as:
/// - boot time advertisement: advertise all services available
/// - stop-time advertisement: advertise a TTL of 0 as services are removed
bool IsInternalBroadcast() const { return mIsInternalBroadcast; }
void SetIsInternalBroadcast(bool isInternalBroadcast) { mIsInternalBroadcast = isInternalBroadcast; }
bool IsAnnounceBroadcast() const { return mIsAnnounceBroadcast; }
void SetIsAnnounceBroadcast(bool isAnnounceBroadcast) { mIsAnnounceBroadcast = isAnnounceBroadcast; }

SerializedQNameIterator GetName() const { return mNameIterator; }

Expand All @@ -69,7 +69,7 @@ class QueryData

/// Flag as an internal broadcast, controls reply construction (e.g. no
/// filtering applied)
bool mIsInternalBroadcast = false;
bool mIsAnnounceBroadcast = false;
};

class ResourceData
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/minimal_mdns/QueryReplyFilter.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class QueryReplyFilter : public ReplyFilter

bool AcceptablePath(FullQName qname)
{
if (mIgnoreNameMatch || mQueryData.IsInternalBroadcast())
if (mIgnoreNameMatch || mQueryData.IsAnnounceBroadcast())
{
return true;
}
Expand Down
Loading