From 829d8bdf24d4b06cbfb8025ebcc81aa2ac9d4bde Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 15 Jun 2023 11:06:02 -0400 Subject: [PATCH 01/10] Make test advertiser support multi-admin logic to test packet content --- examples/minimal-mdns/advertiser.cpp | 116 ++++++++++++++++++--------- 1 file changed, 76 insertions(+), 40 deletions(-) diff --git a/examples/minimal-mdns/advertiser.cpp b/examples/minimal-mdns/advertiser.cpp index 0194a3d7fb4c9a..b4fa99ce440162 100644 --- a/examples/minimal-mdns/advertiser.cpp +++ b/examples/minimal-mdns/advertiser.cpp @@ -35,6 +35,7 @@ enum class AdvertisingMode { kCommissionableNode, kOperational, + kOperationalMultiAdmin, kCommissioner, }; @@ -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; @@ -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 \n" - " --advertising-mode \n" - " Advertise in this mode (operational or commissionable-node or commissioner).\n" - " --short-discriminator \n" - " -s \n" - " Commissioning/commissionable short discriminator.\n" - " --long-discriminator \n" - " -l \n" - " Commissioning/commissionable long discriminator.\n" - " --vendor-id \n" - " Commissioning/commissionable vendor id.\n" - " --product-id \n" - " -p \n" - " Commissioning/commissionable product id.\n" - " --commissioning-mode \n" - " Commissioning Mode (0=disabled, 1=basic, 2=enhanced).\n" - " --device-type \n" - " Device type id.\n" - " --device-name \n" - " Name of device.\n" - " --rotating-id \n" - " Rotating Id.\n" - " --pairing-instruction \n" - " Commissionable pairing instruction.\n" - " --pairing-hint \n" - " Commissionable pairing hint.\n" - " --fabric-id \n" - " -f \n" - " Operational fabric id.\n" - " --node-id \n" - " -n \n" - " Operational node id.\n" - " -t \n" - " --trace-to \n" - " trace to the given destination (supported: " SUPPORTED_COMMAND_LINE_TRACING_TARGETS ").\n" - "\n" }; + " -m \n" + " --advertising-mode \n" + " Advertise in this mode (operational, operational-multi-admin, commissionable-node or commissioner).\n" + " --short-discriminator \n" + " -s \n" + " Commissioning/commissionable short discriminator.\n" + " --long-discriminator \n" + " -l \n" + " Commissioning/commissionable long discriminator.\n" + " --vendor-id \n" + " Commissioning/commissionable vendor id.\n" + " --product-id \n" + " -p \n" + " Commissioning/commissionable product id.\n" + " --commissioning-mode \n" + " Commissioning Mode (0=disabled, 1=basic, 2=enhanced).\n" + " --device-type \n" + " Device type id.\n" + " --device-name \n" + " Name of device.\n" + " --rotating-id \n" + " Rotating Id.\n" + " --pairing-instruction \n" + " Commissionable pairing instruction.\n" + " --pairing-hint \n" + " Commissionable pairing hint.\n" + " --fabric-id \n" + " -f \n" + " Operational fabric id.\n" + " --node-id \n" + " -n \n" + " Operational node id.\n" + " -t \n" + " --trace-to \n" + " trace to the given destination (supported: " SUPPORTED_COMMAND_LINE_TRACING_TARGETS ").\n" + "\n" +}; HelpOptions helpOptions("advertiser", "Usage: advertiser [options]", "1.0"); @@ -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) + .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) + .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"); From 88ded818e2536809b9f8f4092ce6a8889222e1f7 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 15 Jun 2023 12:32:06 -0400 Subject: [PATCH 02/10] Make IP addresses sent only once --- src/lib/dnssd/minimal_mdns/ResponseSender.cpp | 29 ++++++++++++++++ src/lib/dnssd/minimal_mdns/ResponseSender.h | 26 ++++++++++++--- src/lib/dnssd/minimal_mdns/responders/IP.cpp | 12 +++++++ src/lib/dnssd/minimal_mdns/responders/Ptr.h | 6 ++++ .../responders/QueryResponder.cpp | 7 ++++ .../dnssd/minimal_mdns/responders/Responder.h | 33 +++++++++++++------ src/lib/dnssd/minimal_mdns/responders/Srv.h | 6 ++++ src/lib/dnssd/minimal_mdns/responders/Txt.h | 6 ++++ 8 files changed, 110 insertions(+), 15 deletions(-) diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp index 722ca973902fb3..2cc27dc93268d5 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp @@ -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 @@ -232,6 +234,33 @@ CHIP_ERROR ResponseSender::PrepareNewReplyPacket() return CHIP_NO_ERROR; } +bool ResponseSender::Accept(const Responder &responder) const +{ + switch (responder.GetQType()) { + case QType::A: + return !mSendState.GetWasSent(ResponseItemsSent::kIPv4Addresses); + case QType::AAAA: + return !mSendState.GetWasSent(ResponseItemsSent::kIPv6Addresses); + 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()); diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.h b/src/lib/dnssd/minimal_mdns/ResponseSender.h index 636acaaf7acf14..34184b3ea8873d 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.h +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.h @@ -47,6 +47,15 @@ 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 { + kIPv4Addresses = 0x01, + kIPv6Addresses = 0x02, +}; + /// Represents the internal state for sending a currently active request class ResponseSendingState { @@ -55,11 +64,12 @@ class ResponseSendingState void Reset(uint16_t messageId, const QueryData & query, const chip::Inet::IPPacketInfo * packet) { - mMessageId = messageId; - mQuery = &query; - mSource = packet; - mSendError = CHIP_NO_ERROR; - mResourceType = ResourceType::kAnswer; + mMessageId = messageId; + mQuery = &query; + mSource = packet; + mSendError = CHIP_NO_ERROR; + mResourceType = ResourceType::kAnswer; + mSentItems.ClearAll(); } void SetResourceType(ResourceType resourceType) { mResourceType = resourceType; } @@ -88,12 +98,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 mSentItems; }; } // namespace Internal @@ -117,6 +131,8 @@ class ResponseSender : public ResponderDelegate // Implementation of ResponderDelegate void AddResponse(const ResourceRecord & record) override; + bool Accept(const Responder &) const override; + void ResponsesAdded(const Responder &) override; void SetServer(ServerBase * server) { mServer = server; } diff --git a/src/lib/dnssd/minimal_mdns/responders/IP.cpp b/src/lib/dnssd/minimal_mdns/responders/IP.cpp index 8290db94e0aa40..ee9d508dcb572b 100644 --- a/src/lib/dnssd/minimal_mdns/responders/IP.cpp +++ b/src/lib/dnssd/minimal_mdns/responders/IP.cpp @@ -30,6 +30,11 @@ void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res const ResponseConfiguration & configuration) { #if INET_CONFIG_ENABLE_IPV4 + if (!delegate->Accept(*this)) + { + return; + } + chip::Inet::IPAddress addr; UniquePtr ips = GetAddressPolicy()->GetIpAddressesForEndpoint(source->Interface, chip::Inet::IPAddressType::kIPv4); @@ -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->Accept(*this)) + { + return; + } + chip::Inet::IPAddress addr; UniquePtr ips = GetAddressPolicy()->GetIpAddressesForEndpoint(source->Interface, chip::Inet::IPAddressType::kIPv6); @@ -68,6 +79,7 @@ void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res configuration.Adjust(record); delegate->AddResponse(record); } + delegate->ResponsesAdded(*this); } } // namespace Minimal diff --git a/src/lib/dnssd/minimal_mdns/responders/Ptr.h b/src/lib/dnssd/minimal_mdns/responders/Ptr.h index 1fd50a0b5e142c..40c11134f22d0e 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Ptr.h +++ b/src/lib/dnssd/minimal_mdns/responders/Ptr.h @@ -31,9 +31,15 @@ class PtrResponder : public RecordResponder void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) override { + if (!delegate->Accept(*this)) + { + return; + } + PtrResourceRecord record(GetQName(), mTarget); configuration.Adjust(record); delegate->AddResponse(record); + delegate->ResponsesAdded(*this); } private: diff --git a/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp b/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp index 47731c7a42ddfd..063878fd952d4b 100644 --- a/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp +++ b/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp @@ -142,6 +142,11 @@ void QueryResponderBase::MarkAdditionalRepliesFor(QueryResponderIterator it) void QueryResponderBase::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) { + if (!delegate->Accept(*this)) + { + return; + } + // reply to dns-sd service list request for (size_t i = 0; i < mResponderInfoSize; i++) { @@ -159,6 +164,8 @@ void QueryResponderBase::AddAllResponses(const chip::Inet::IPPacketInfo * source configuration.Adjust(record); delegate->AddResponse(record); } + + delegate->ResponsesAdded(*this); } void QueryResponderBase::ClearBroadcastThrottle() diff --git a/src/lib/dnssd/minimal_mdns/responders/Responder.h b/src/lib/dnssd/minimal_mdns/responders/Responder.h index 4a1b4e015dbe59..1f227969694db3 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Responder.h +++ b/src/lib/dnssd/minimal_mdns/responders/Responder.h @@ -26,15 +26,6 @@ namespace mdns { namespace Minimal { -class ResponderDelegate -{ -public: - virtual ~ResponderDelegate() {} - - /// Add the specified resource record to the response - virtual void AddResponse(const ResourceRecord & record) = 0; -}; - /// Controls specific options for responding to mDNS queries /// class ResponseConfiguration @@ -67,6 +58,9 @@ class ResponseConfiguration chip::Optional mTtlSecondsOverride; }; +// Delegates that responders can write themselves to +class ResponderDelegate; + /// Adds ability to respond with specific types of data class Responder { @@ -81,7 +75,7 @@ class Responder /// Domain name is generally just 'local' FullQName GetQName() const { return mQName; } - /// Report all reponses maintained by this responder + /// Report all responses maintained by this responder /// /// Responses are associated with the objects type/class/qname. virtual void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, @@ -92,5 +86,24 @@ class Responder const FullQName mQName; }; +class ResponderDelegate +{ +public: + virtual ~ResponderDelegate() {} + + /// Add the specified resource record to the response + virtual void AddResponse(const ResourceRecord & record) = 0; + + /// Accept to add responses for the particular responder. + /// + /// This will be called before responders serialize their records. + virtual bool Accept(const Responder &) const { return true; } + + /// Called when all responses were added for a particular responder + /// + /// Only called if a previous accept returned true. + virtual void ResponsesAdded(const Responder &) { } +}; + } // namespace Minimal } // namespace mdns diff --git a/src/lib/dnssd/minimal_mdns/responders/Srv.h b/src/lib/dnssd/minimal_mdns/responders/Srv.h index d23226092b3e09..f5ee3f898fa0e1 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Srv.h +++ b/src/lib/dnssd/minimal_mdns/responders/Srv.h @@ -31,9 +31,15 @@ class SrvResponder : public RecordResponder void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) override { + if (!delegate->Accept(*this)) + { + return; + } + SrvResourceRecord record = mRecord; configuration.Adjust(record); delegate->AddResponse(record); + delegate->ResponsesAdded(*this); } private: diff --git a/src/lib/dnssd/minimal_mdns/responders/Txt.h b/src/lib/dnssd/minimal_mdns/responders/Txt.h index 5b607265b8c55c..a92a68977e0147 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Txt.h +++ b/src/lib/dnssd/minimal_mdns/responders/Txt.h @@ -31,9 +31,15 @@ class TxtResponder : public RecordResponder void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) override { + if (!delegate->Accept(*this)) + { + return; + } + TxtResourceRecord record = mRecord; configuration.Adjust(record); delegate->AddResponse(record); + delegate->ResponsesAdded(*this); } private: From 1cc232c9357293fa783f45552d0864a41c943365 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 15 Jun 2023 12:32:54 -0400 Subject: [PATCH 03/10] Update advertiser to make srv port unique --- examples/minimal-mdns/advertiser.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/minimal-mdns/advertiser.cpp b/examples/minimal-mdns/advertiser.cpp index b4fa99ce440162..444a4ab33e5bdf 100644 --- a/examples/minimal-mdns/advertiser.cpp +++ b/examples/minimal-mdns/advertiser.cpp @@ -325,7 +325,7 @@ int main(int argc, char ** args) err = chip::Dnssd::ServiceAdvertiser::Instance().Advertise( chip::Dnssd::OperationalAdvertisingParameters() .EnableIpV4(gOptions.enableIpV4) - .SetPort(CHIP_PORT) + .SetPort(CHIP_PORT+1) .SetMac(chip::ByteSpan(gOptions.mac, 6)) .SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId + 1).SetNodeId(gOptions.nodeId + 1))); } @@ -335,7 +335,7 @@ int main(int argc, char ** args) err = chip::Dnssd::ServiceAdvertiser::Instance().Advertise( chip::Dnssd::OperationalAdvertisingParameters() .EnableIpV4(gOptions.enableIpV4) - .SetPort(CHIP_PORT) + .SetPort(CHIP_PORT+2) .SetMac(chip::ByteSpan(gOptions.mac, 6)) .SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId + 2).SetNodeId(gOptions.nodeId + 2))); } From 0e5a83f7f2e10def676fe3a8ad8fc3d35204962d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 15 Jun 2023 12:34:32 -0400 Subject: [PATCH 04/10] Update comment a bit --- src/lib/dnssd/minimal_mdns/ResponseSender.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.h b/src/lib/dnssd/minimal_mdns/ResponseSender.h index 34184b3ea8873d..b8f7d115d3901f 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.h +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.h @@ -51,7 +51,11 @@ namespace Internal { // // 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, }; From 12364ed15af3ed1c0662e4187b05e848d2e40bac Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 15 Jun 2023 12:43:08 -0400 Subject: [PATCH 05/10] Do not list dnssd service listing at boot time advertisement --- src/lib/dnssd/minimal_mdns/ResponseSender.cpp | 12 ++++++++++++ src/lib/dnssd/minimal_mdns/ResponseSender.h | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp index 2cc27dc93268d5..d5df667ac0180d 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp @@ -107,6 +107,11 @@ CHIP_ERROR ResponseSender::Respond(uint16_t messageId, const QueryData & query, { mSendState.Reset(messageId, query, querySource); + if (query.IsInternalBroadcast()) { + // 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. @@ -241,6 +246,13 @@ bool ResponseSender::Accept(const Responder &responder) const 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); + } + }; default: break; } diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.h b/src/lib/dnssd/minimal_mdns/ResponseSender.h index b8f7d115d3901f..b1d1459402b872 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.h +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.h @@ -58,6 +58,10 @@ enum class ResponseItemsSent : uint8_t { // 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 From 113fa712f79ab676eb0181e0d829ee56f893b4a3 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 15 Jun 2023 12:46:44 -0400 Subject: [PATCH 06/10] Clang-format and mark boot time advertisements to have everything as answers instead of additional records --- src/lib/dnssd/minimal_mdns/ResponseSender.cpp | 64 +++++++++++-------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp index d5df667ac0180d..a774e908804164 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp @@ -107,7 +107,8 @@ CHIP_ERROR ResponseSender::Respond(uint16_t messageId, const QueryData & query, { mSendState.Reset(messageId, query, querySource); - if (query.IsInternalBroadcast()) { + if (query.IsInternalBroadcast()) + { // Deny listing large amount of data mSendState.MarkWasSent(ResponseItemsSent::kServiceListingData); } @@ -165,7 +166,12 @@ CHIP_ERROR ResponseSender::Respond(uint16_t messageId, const QueryData & query, // send all 'Additional' replies { - mSendState.SetResourceType(ResourceType::kAdditional); + if (!query.IsInternalBroadcast()) + { + // 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); @@ -239,37 +245,41 @@ CHIP_ERROR ResponseSender::PrepareNewReplyPacket() return CHIP_NO_ERROR; } -bool ResponseSender::Accept(const Responder &responder) const +bool ResponseSender::Accept(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); - } - }; - default: - break; + 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); + } + }; + 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::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; } } From ea2b01c97e3b1e7350739e5db23c4c921c454262 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 15 Jun 2023 12:54:48 -0400 Subject: [PATCH 07/10] Add missing break --- src/lib/dnssd/minimal_mdns/ResponseSender.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp index a774e908804164..6c5369ebce5756 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp @@ -260,7 +260,8 @@ bool ResponseSender::Accept(const Responder & responder) const { return !mSendState.GetWasSent(ResponseItemsSent::kServiceListingData); } - }; + break; + } default: break; } From 8a12b033d6273d2f9ae82c9916b1dd6c516a9ddb Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 15 Jun 2023 17:08:17 +0000 Subject: [PATCH 08/10] Restyled by clang-format --- examples/minimal-mdns/advertiser.cpp | 4 ++-- src/lib/dnssd/minimal_mdns/ResponseSender.h | 13 +++++++------ src/lib/dnssd/minimal_mdns/responders/Responder.h | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/examples/minimal-mdns/advertiser.cpp b/examples/minimal-mdns/advertiser.cpp index 444a4ab33e5bdf..e9cc880cae81ed 100644 --- a/examples/minimal-mdns/advertiser.cpp +++ b/examples/minimal-mdns/advertiser.cpp @@ -325,7 +325,7 @@ int main(int argc, char ** args) err = chip::Dnssd::ServiceAdvertiser::Instance().Advertise( chip::Dnssd::OperationalAdvertisingParameters() .EnableIpV4(gOptions.enableIpV4) - .SetPort(CHIP_PORT+1) + .SetPort(CHIP_PORT + 1) .SetMac(chip::ByteSpan(gOptions.mac, 6)) .SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId + 1).SetNodeId(gOptions.nodeId + 1))); } @@ -335,7 +335,7 @@ int main(int argc, char ** args) err = chip::Dnssd::ServiceAdvertiser::Instance().Advertise( chip::Dnssd::OperationalAdvertisingParameters() .EnableIpV4(gOptions.enableIpV4) - .SetPort(CHIP_PORT+2) + .SetPort(CHIP_PORT + 2) .SetMac(chip::ByteSpan(gOptions.mac, 6)) .SetPeerId(PeerId().SetCompressedFabricId(gOptions.fabricId + 2).SetNodeId(gOptions.nodeId + 2))); } diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.h b/src/lib/dnssd/minimal_mdns/ResponseSender.h index b1d1459402b872..8d3d407348994b 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.h +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.h @@ -52,7 +52,8 @@ namespace Internal { // 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 { +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. @@ -72,11 +73,11 @@ class ResponseSendingState void Reset(uint16_t messageId, const QueryData & query, const chip::Inet::IPPacketInfo * packet) { - mMessageId = messageId; - mQuery = &query; - mSource = packet; - mSendError = CHIP_NO_ERROR; - mResourceType = ResourceType::kAnswer; + mMessageId = messageId; + mQuery = &query; + mSource = packet; + mSendError = CHIP_NO_ERROR; + mResourceType = ResourceType::kAnswer; mSentItems.ClearAll(); } diff --git a/src/lib/dnssd/minimal_mdns/responders/Responder.h b/src/lib/dnssd/minimal_mdns/responders/Responder.h index 1f227969694db3..328c5436e69501 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Responder.h +++ b/src/lib/dnssd/minimal_mdns/responders/Responder.h @@ -102,7 +102,7 @@ class ResponderDelegate /// Called when all responses were added for a particular responder /// /// Only called if a previous accept returned true. - virtual void ResponsesAdded(const Responder &) { } + virtual void ResponsesAdded(const Responder &) {} }; } // namespace Minimal From 673b8b002aca0cc8ec51d8e54cb6f977f69bfd82 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 16 Jun 2023 09:42:26 -0400 Subject: [PATCH 09/10] rg InternalBroadcast --files-with-matches | xargs sd 'InternalBroadcast' 'AnnounceBroadcast' --- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 2 +- src/lib/dnssd/minimal_mdns/Parser.h | 6 +++--- src/lib/dnssd/minimal_mdns/QueryReplyFilter.h | 2 +- src/lib/dnssd/minimal_mdns/ResponseSender.cpp | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 2c9c021906cecf..b2f1a0b98f69b5 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -945,7 +945,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) { diff --git a/src/lib/dnssd/minimal_mdns/Parser.h b/src/lib/dnssd/minimal_mdns/Parser.h index 509633b9e192fa..879c6858a2912f 100644 --- a/src/lib/dnssd/minimal_mdns/Parser.h +++ b/src/lib/dnssd/minimal_mdns/Parser.h @@ -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; } @@ -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 diff --git a/src/lib/dnssd/minimal_mdns/QueryReplyFilter.h b/src/lib/dnssd/minimal_mdns/QueryReplyFilter.h index c7eb0a98af0151..a9cf13842d3527 100644 --- a/src/lib/dnssd/minimal_mdns/QueryReplyFilter.h +++ b/src/lib/dnssd/minimal_mdns/QueryReplyFilter.h @@ -81,7 +81,7 @@ class QueryReplyFilter : public ReplyFilter bool AcceptablePath(FullQName qname) { - if (mIgnoreNameMatch || mQueryData.IsInternalBroadcast()) + if (mIgnoreNameMatch || mQueryData.IsAnnounceBroadcast()) { return true; } diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp index 6c5369ebce5756..fb6c16981c0569 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp @@ -107,7 +107,7 @@ CHIP_ERROR ResponseSender::Respond(uint16_t messageId, const QueryData & query, { mSendState.Reset(messageId, query, querySource); - if (query.IsInternalBroadcast()) + if (query.IsAnnounceBroadcast()) { // Deny listing large amount of data mSendState.MarkWasSent(ResponseItemsSent::kServiceListingData); @@ -166,7 +166,7 @@ CHIP_ERROR ResponseSender::Respond(uint16_t messageId, const QueryData & query, // send all 'Additional' replies { - if (!query.IsInternalBroadcast()) + 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) From 336591242eb76c7b5520065919d327fa08a548c0 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 16 Jun 2023 09:45:39 -0400 Subject: [PATCH 10/10] Rename Accept to ShouldSend --- src/lib/dnssd/minimal_mdns/ResponseSender.cpp | 2 +- src/lib/dnssd/minimal_mdns/ResponseSender.h | 2 +- src/lib/dnssd/minimal_mdns/responders/IP.cpp | 4 ++-- src/lib/dnssd/minimal_mdns/responders/Ptr.h | 2 +- src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp | 2 +- src/lib/dnssd/minimal_mdns/responders/Responder.h | 2 +- src/lib/dnssd/minimal_mdns/responders/Srv.h | 2 +- src/lib/dnssd/minimal_mdns/responders/Txt.h | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp index fb6c16981c0569..7aec6142f0b555 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.cpp +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.cpp @@ -245,7 +245,7 @@ CHIP_ERROR ResponseSender::PrepareNewReplyPacket() return CHIP_NO_ERROR; } -bool ResponseSender::Accept(const Responder & responder) const +bool ResponseSender::ShouldSend(const Responder & responder) const { switch (responder.GetQType()) { diff --git a/src/lib/dnssd/minimal_mdns/ResponseSender.h b/src/lib/dnssd/minimal_mdns/ResponseSender.h index 8d3d407348994b..487b3b5b104f81 100644 --- a/src/lib/dnssd/minimal_mdns/ResponseSender.h +++ b/src/lib/dnssd/minimal_mdns/ResponseSender.h @@ -140,7 +140,7 @@ class ResponseSender : public ResponderDelegate // Implementation of ResponderDelegate void AddResponse(const ResourceRecord & record) override; - bool Accept(const Responder &) const override; + bool ShouldSend(const Responder &) const override; void ResponsesAdded(const Responder &) override; void SetServer(ServerBase * server) { mServer = server; } diff --git a/src/lib/dnssd/minimal_mdns/responders/IP.cpp b/src/lib/dnssd/minimal_mdns/responders/IP.cpp index ee9d508dcb572b..80e57e15223737 100644 --- a/src/lib/dnssd/minimal_mdns/responders/IP.cpp +++ b/src/lib/dnssd/minimal_mdns/responders/IP.cpp @@ -30,7 +30,7 @@ void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res const ResponseConfiguration & configuration) { #if INET_CONFIG_ENABLE_IPV4 - if (!delegate->Accept(*this)) + if (!delegate->ShouldSend(*this)) { return; } @@ -58,7 +58,7 @@ void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) { - if (!delegate->Accept(*this)) + if (!delegate->ShouldSend(*this)) { return; } diff --git a/src/lib/dnssd/minimal_mdns/responders/Ptr.h b/src/lib/dnssd/minimal_mdns/responders/Ptr.h index 40c11134f22d0e..80e91e2349b16e 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Ptr.h +++ b/src/lib/dnssd/minimal_mdns/responders/Ptr.h @@ -31,7 +31,7 @@ class PtrResponder : public RecordResponder void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) override { - if (!delegate->Accept(*this)) + if (!delegate->ShouldSend(*this)) { return; } diff --git a/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp b/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp index 063878fd952d4b..eb0ed5ac065118 100644 --- a/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp +++ b/src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp @@ -142,7 +142,7 @@ void QueryResponderBase::MarkAdditionalRepliesFor(QueryResponderIterator it) void QueryResponderBase::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) { - if (!delegate->Accept(*this)) + if (!delegate->ShouldSend(*this)) { return; } diff --git a/src/lib/dnssd/minimal_mdns/responders/Responder.h b/src/lib/dnssd/minimal_mdns/responders/Responder.h index 328c5436e69501..534b661df64535 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Responder.h +++ b/src/lib/dnssd/minimal_mdns/responders/Responder.h @@ -97,7 +97,7 @@ class ResponderDelegate /// Accept to add responses for the particular responder. /// /// This will be called before responders serialize their records. - virtual bool Accept(const Responder &) const { return true; } + virtual bool ShouldSend(const Responder &) const { return true; } /// Called when all responses were added for a particular responder /// diff --git a/src/lib/dnssd/minimal_mdns/responders/Srv.h b/src/lib/dnssd/minimal_mdns/responders/Srv.h index f5ee3f898fa0e1..868bf878302aaf 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Srv.h +++ b/src/lib/dnssd/minimal_mdns/responders/Srv.h @@ -31,7 +31,7 @@ class SrvResponder : public RecordResponder void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) override { - if (!delegate->Accept(*this)) + if (!delegate->ShouldSend(*this)) { return; } diff --git a/src/lib/dnssd/minimal_mdns/responders/Txt.h b/src/lib/dnssd/minimal_mdns/responders/Txt.h index a92a68977e0147..b2a920ab96c0fd 100644 --- a/src/lib/dnssd/minimal_mdns/responders/Txt.h +++ b/src/lib/dnssd/minimal_mdns/responders/Txt.h @@ -31,7 +31,7 @@ class TxtResponder : public RecordResponder void AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate, const ResponseConfiguration & configuration) override { - if (!delegate->Accept(*this)) + if (!delegate->ShouldSend(*this)) { return; }