Skip to content

Commit

Permalink
MinMDNS: reduce amount of data in the mdns advertising packets (and m…
Browse files Browse the repository at this point in the history 
…ake it more RFC compliant) (#27274)

* Make test advertiser support multi-admin logic to test packet content

* Make IP addresses sent only once

* Update advertiser to make srv port unique

* Update comment a bit

* Do not list dnssd service listing at boot time advertisement

* Clang-format and mark boot time advertisements to have everything as answers instead of additional records

* Add missing break

* Restyled by clang-format

* rg InternalBroadcast --files-with-matches | xargs sd 'InternalBroadcast' 'AnnounceBroadcast'

* Rename Accept to ShouldSend

---------

Co-authored-by: Andrei Litvin <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
@andy31415 @andreilitvin
@restyled-commits @pull
3 people authored and pull[bot] committed Aug 22, 2023
1 parent feba758 commit 1401120
Show file tree
Hide file tree
Showing 12 changed files with 220 additions and 56 deletions.
116 changes: 76 additions & 40 deletions examples/minimal-mdns/advertiser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ enum class AdvertisingMode
{
kCommissionableNode,
kOperational,
kOperationalMultiAdmin,
kCommissioner,
};

Expand Down Expand Up @@ -100,6 +101,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 @@ -196,48 +201,50 @@ 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"
" -t <dest>\n"
" --trace-to <dest>\n"
" trace to the given destination (supported: " SUPPORTED_COMMAND_LINE_TRACING_TARGETS ").\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"
" --trace-to <dest>\n"
" trace to the given destination (supported: " SUPPORTED_COMMAND_LINE_TRACING_TARGETS ").\n"
"\n"
};

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

Expand Down Expand Up @@ -304,6 +311,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
3 changes: 2 additions & 1 deletion src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ void AdvertiserMinMdns::AdvertiseRecords(BroadcastAdvertiseType type)
// 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);
queryData.SetIsAnnounceBroadcast(true);

for (auto & it : mOperationalResponders)
{
Expand All @@ -916,6 +916,7 @@ void AdvertiserMinMdns::AdvertiseRecords(BroadcastAdvertiseType type)
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());
Expand Down
6 changes: 3 additions & 3 deletions src/lib/dnssd/minimal_mdns/Parser.h
View file
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
View file
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
54 changes: 53 additions & 1 deletion src/lib/dnssd/minimal_mdns/ResponseSender.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ namespace Minimal {

namespace {

using namespace mdns::Minimal::Internal;

constexpr uint16_t kMdnsStandardPort = 5353;

// Restriction for UDP packets: https://tools.ietf.org/html/rfc1035#section-4.2.1
Expand Down Expand Up @@ -105,6 +107,12 @@ CHIP_ERROR ResponseSender::Respond(uint16_t messageId, const QueryData & query,
{
mSendState.Reset(messageId, query, querySource);

if (query.IsAnnounceBroadcast())
{
// Deny listing large amount of data
mSendState.MarkWasSent(ResponseItemsSent::kServiceListingData);
}

// Responder has a stateful 'additional replies required' that is used within the response
// loop. 'no additionals required' is set at the start and additionals are marked as the query
// reply is built.
Expand Down Expand Up @@ -158,7 +166,12 @@ CHIP_ERROR ResponseSender::Respond(uint16_t messageId, const QueryData & query,

// send all 'Additional' replies
{
mSendState.SetResourceType(ResourceType::kAdditional);
if (!query.IsAnnounceBroadcast())
{
// Initial service broadcast should keep adding data as 'Answers' rather
// than addtional data (https://datatracker.ietf.org/doc/html/rfc6762#section-8.3)
mSendState.SetResourceType(ResourceType::kAdditional);
}

QueryReplyFilter queryReplyFilter(query);

Expand Down Expand Up @@ -232,6 +245,45 @@ CHIP_ERROR ResponseSender::PrepareNewReplyPacket()
return CHIP_NO_ERROR;
}

bool ResponseSender::ShouldSend(const Responder & responder) const
{
switch (responder.GetQType())
{
case QType::A:
return !mSendState.GetWasSent(ResponseItemsSent::kIPv4Addresses);
case QType::AAAA:
return !mSendState.GetWasSent(ResponseItemsSent::kIPv6Addresses);
case QType::PTR: {
static const QNamePart kDnsSdQueryPath[] = { "_services", "_dns-sd", "_udp", "local" };

if (responder.GetQName() == FullQName(kDnsSdQueryPath))
{
return !mSendState.GetWasSent(ResponseItemsSent::kServiceListingData);
}
break;
}
default:
break;
}

return true;
}

void ResponseSender::ResponsesAdded(const Responder & responder)
{
switch (responder.GetQType())
{
case QType::A:
mSendState.MarkWasSent(ResponseItemsSent::kIPv4Addresses);
break;
case QType::AAAA:
mSendState.MarkWasSent(ResponseItemsSent::kIPv6Addresses);
break;
default:
break;
}
}

void ResponseSender::AddResponse(const ResourceRecord & record)
{
ReturnOnFailure(mSendState.GetError());
Expand Down
25 changes: 25 additions & 0 deletions src/lib/dnssd/minimal_mdns/ResponseSender.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ namespace Minimal {

namespace Internal {

// Flags for keeping track of items having been sent as DNSSD responses
//
// We rely on knowing Matter DNSSD only sends the same set of data
// for some instances like A/AAAA always being the same.
//
enum class ResponseItemsSent : uint8_t
{
// DNSSD may have different servers referenced by IP addresses,
// however we know the matter dnssd server name is fixed and
// the same even across SRV records.
kIPv4Addresses = 0x01,
kIPv6Addresses = 0x02,

// Boot time advertisement filters. We allow multiple of these
// however we also allow filtering them out at response start
kServiceListingData = 0x04,
};

/// Represents the internal state for sending a currently active request
class ResponseSendingState
{
Expand All @@ -60,6 +78,7 @@ class ResponseSendingState
mSource = packet;
mSendError = CHIP_NO_ERROR;
mResourceType = ResourceType::kAnswer;
mSentItems.ClearAll();
}

void SetResourceType(ResourceType resourceType) { mResourceType = resourceType; }
Expand Down Expand Up @@ -88,12 +107,16 @@ class ResponseSendingState
const chip::Inet::IPAddress & GetSourceAddress() const { return mSource->SrcAddress; }
chip::Inet::InterfaceId GetSourceInterfaceId() const { return mSource->Interface; }

bool GetWasSent(ResponseItemsSent item) const { return mSentItems.Has(item); }
void MarkWasSent(ResponseItemsSent item) { mSentItems.Set(item); }

private:
const QueryData * mQuery = nullptr; // query being replied to
const chip::Inet::IPPacketInfo * mSource = nullptr; // Where to send the reply (if unicast)
uint16_t mMessageId = 0; // message id for the reply
ResourceType mResourceType = ResourceType::kAnswer; // what is being sent right now
CHIP_ERROR mSendError = CHIP_NO_ERROR;
chip::BitFlags<ResponseItemsSent> mSentItems;
};

} // namespace Internal
Expand All @@ -117,6 +140,8 @@ class ResponseSender : public ResponderDelegate

// Implementation of ResponderDelegate
void AddResponse(const ResourceRecord & record) override;
bool ShouldSend(const Responder &) const override;
void ResponsesAdded(const Responder &) override;

void SetServer(ServerBase * server) { mServer = server; }

Expand Down
12 changes: 12 additions & 0 deletions src/lib/dnssd/minimal_mdns/responders/IP.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res
const ResponseConfiguration & configuration)
{
#if INET_CONFIG_ENABLE_IPV4
if (!delegate->ShouldSend(*this))
{
return;
}

chip::Inet::IPAddress addr;
UniquePtr<IpAddressIterator> ips =
GetAddressPolicy()->GetIpAddressesForEndpoint(source->Interface, chip::Inet::IPAddressType::kIPv4);
Expand All @@ -46,12 +51,18 @@ void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res
configuration.Adjust(record);
delegate->AddResponse(record);
}
delegate->ResponsesAdded(*this);
#endif
}

void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate,
const ResponseConfiguration & configuration)
{
if (!delegate->ShouldSend(*this))
{
return;
}

chip::Inet::IPAddress addr;
UniquePtr<IpAddressIterator> ips =
GetAddressPolicy()->GetIpAddressesForEndpoint(source->Interface, chip::Inet::IPAddressType::kIPv6);
Expand All @@ -68,6 +79,7 @@ void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res
configuration.Adjust(record);
delegate->AddResponse(record);
}
delegate->ResponsesAdded(*this);
}

} // namespace Minimal
Expand Down
6 changes: 6 additions & 0 deletions src/lib/dnssd/minimal_mdns/responders/Ptr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,15 @@ class PtrResponder : public RecordResponder
void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate,
const ResponseConfiguration & configuration) override
{
if (!delegate->ShouldSend(*this))
{
return;
}

PtrResourceRecord record(GetQName(), mTarget);
configuration.Adjust(record);
delegate->AddResponse(record);
delegate->ResponsesAdded(*this);
}

private:
Expand Down
Loading

0 comments on commit 1401120

Please sign in to comment.