Do a mDNS advertisement on minmdns server startup (#4884)

* Listen on all IPv4 interfaces (not only NULL interface ID) so that replies to queries can be sent with link local information as well

* Do a startup mDNS broadcast for available services - no filtering by interface/capability yet

* Ensure mdns advertise at boot only happens on interfaces that it is aware of

* Remove FIXME since it was already addressed

* Revise doc comment on boot time advertisement

* Add a constant for maximum interface name length

* Restyle fixes

* Fix LWIP interface name max size

* Increase LWIP max size some more, to support for 32bit interface numbers (even if unlikely)

* Update include file for zephyr interface name constant

* Deduplicate the detection of usable mdns interfaces

* Deduplicate ip addresses for global mdns broadcast

* Fix compilation

* Fix linkage too

* do not use pre-main initialization for mDNS broadcast IP addresses

* Fix typo: Trottle -> Throttle

* Fix typo: Trottle -> Throttle

* Add todo to have better local loopback detection for interfaces

* Log on IP parse error - we seem to crash in esp32 and am not sure why. Better to log instead of dying

* Fix condition inversion

* Fix logic error: IP address returns a bool not CHIP_ERROR

examples/minimal-mdns/AllInterfaceListener.h

@@ -94,7 +94,8 @@ class AllInterfaces : public mdns::Minimal::ListenIterator
         {
             return true; // not a usable interface
         }
-        char name[64];
+
+        char name[chip::Inet::InterfaceIterator::kMaxIfNameLength];
         if (mIterator.GetInterfaceName(name, sizeof(name)) != CHIP_NO_ERROR)
         {
             printf("!!!! FAILED TO GET INTERFACE NAME\n");

src/inet/InetInterface.h

@@ -42,6 +42,8 @@ struct ifaddrs;
 #endif // CHIP_SYSTEM_CONFIG_USE_BSD_IFADDRS
 
 #if CHIP_SYSTEM_CONFIG_USE_ZEPHYR_NET_IF
+#include <device.h>
+
 struct net_if;
 struct net_if_ipv4;
 struct net_if_ipv6;
@@ -152,6 +154,19 @@ class InterfaceIterator
     bool SupportsMulticast();
     bool HasBroadcastAddress();
 
+#if CHIP_SYSTEM_CONFIG_USE_LWIP
+    static constexpr size_t kMaxIfNameLength = 13; // Names are formatted as %c%c%d
+#elif CHIP_SYSTEM_CONFIG_USE_SOCKETS && CHIP_SYSTEM_CONFIG_USE_BSD_IFADDRS
+    static constexpr size_t kMaxIfNameLength = IF_NAMESIZE;
+#elif CHIP_SYSTEM_CONFIG_USE_ZEPHYR_NET_IF
+    static constexpr size_t kMaxIfNameLength = Z_DEVICE_MAX_NAME_LEN;
+#elif defined(IFNAMSIZ)
+    static constexpr size_t kMaxIfNameLength = IFNAMSIZ;
+#else
+    // No constant available here - set some reasonable size
+    static constexpr size_t kMaxIfNameLength = 33;
+#endif
+
 protected:
 #if CHIP_SYSTEM_CONFIG_USE_LWIP
     struct netif * mCurNetif;

src/inet/tests/TestInetCommonPosix.cpp

@@ -177,7 +177,7 @@ void ShutdownSystemLayer()
 #if CHIP_SYSTEM_CONFIG_USE_LWIP
 static void PrintNetworkState()
 {
-    char intfName[10];
+    char intfName[chip::Inet::InterfaceIterator::kMaxIfNameLength];
 
     for (size_t j = 0; j < gNetworkOptions.TapDeviceName.size(); j++)
     {

src/inet/tests/TestInetEndPoint.cpp

@@ -224,7 +224,7 @@ static void TestInetInterface(nlTestSuite * inSuite, void * inContext)
 {
     InterfaceIterator intIterator;
     InterfaceAddressIterator addrIterator;
-    char intName[20];
+    char intName[chip::Inet::InterfaceIterator::kMaxIfNameLength];
     InterfaceId intId;
     IPAddress addr;
     IPPrefix addrWithPrefix;

src/lib/mdns/Advertiser_ImplMinimalMdns.cpp

@@ -86,35 +86,66 @@ void LogQuery(const QueryData & data)
     ChipLogDetail(Discovery, "%s", logString.c_str());
 }
 
+/// Checks if the current interface is powered on
+/// and not local loopback.
+template <typename T>
+bool IsCurrentInterfaceUsable(T & iterator)
+{
+    if (!iterator.IsUp() || !iterator.SupportsMulticast())
+    {
+        return false; // not a usable interface
+    }
+    char name[chip::Inet::InterfaceIterator::kMaxIfNameLength];
+    if (iterator.GetInterfaceName(name, sizeof(name)) != CHIP_NO_ERROR)
+    {
+        ChipLogError(Discovery, "Failed to get interface name.");
+        return false;
+    }
+
+    // TODO: need a better way to ignore local loopback interfaces/addresses
+    // We do not want to listen on local loopback even though they are up and
+    // support multicast
+    //
+    // Some way to detect 'is local looback' that is smarter (e.g. at least
+    // strict string compare on linux instead of substring) would be better.
+    //
+    // This would reject likely valid interfaces like 'lollipop' or 'lostinspace'
+    if (strncmp(name, "lo", 2) == 0)
+    {
+        /// local loopback interface is not usable by MDNS
+        return false;
+    }
+    return true;
+}
+
 class AllInterfaces : public ListenIterator
 {
+private:
 public:
-    AllInterfaces() {}
+    AllInterfaces() { SkipToFirstValidInterface(); }
 
     bool Next(chip::Inet::InterfaceId * id, chip::Inet::IPAddressType * type) override
     {
+        if (!mIterator.HasCurrent())
+        {
+            return false;
+        }
+
 #if INET_CONFIG_ENABLE_IPV4
         if (mState == State::kIpV4)
         {
-            *id    = INET_NULL_INTERFACEID;
+            *id    = mIterator.GetInterfaceId();
             *type  = chip::Inet::kIPAddressType_IPv4;
             mState = State::kIpV6;
-
-            SkipToFirstValidInterface();
             return true;
         }
-#else
-        mState = State::kIpV6;
-        SkipToFirstValidInterface();
 #endif
 
-        if (!mIterator.HasCurrent())
-        {
-            return false;
-        }
-
         *id   = mIterator.GetInterfaceId();
         *type = chip::Inet::kIPAddressType_IPv6;
+#if INET_CONFIG_ENABLE_IPV4
+        mState = State::kIpV4;
+#endif
 
         for (mIterator.Next(); SkipCurrentInterface(); mIterator.Next())
         {
@@ -128,7 +159,11 @@ class AllInterfaces : public ListenIterator
         kIpV4,
         kIpV6,
     };
+#if INET_CONFIG_ENABLE_IPV4
     State mState = State::kIpV4;
+#else
+    State mState = State::kIpV6;
+#endif
     chip::Inet::InterfaceIterator mIterator;
 
     void SkipToFirstValidInterface()
@@ -149,23 +184,7 @@ class AllInterfaces : public ListenIterator
             return false; // nothing to try.
         }
 
-        if (!mIterator.IsUp() || !mIterator.SupportsMulticast())
-        {
-            return true; // not a usable interface
-        }
-        char name[64];
-        if (mIterator.GetInterfaceName(name, sizeof(name)) != CHIP_NO_ERROR)
-        {
-            ChipLogError(Discovery, "Interface iterator failed to get interface name.");
-            return true;
-        }
-
-        if (strncmp(name, "lo", 2) == 0)
-        {
-            ChipLogDetail(Discovery, "Skipping interface '%s' (assume local loopback)", name);
-            return true;
-        }
-        return false;
+        return !IsCurrentInterfaceUsable(mIterator);
     }
 };
 
@@ -209,6 +228,15 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
     /// allocated memory.
     void Clear();
 
+    /// Advertise available records configured within the server
+    ///
+    /// Usable as boot-time advertisement of available SRV records.
+    void AdvertiseRecords();
+
+    /// 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);
+
     QueryResponderSettings AddAllocatedResponder(Responder * responder)
     {
         if (responder == nullptr)
@@ -327,6 +355,9 @@ CHIP_ERROR AdvertiserMinMdns::Start(chip::Inet::InetLayer * inetLayer, uint16_t
     ReturnErrorOnFailure(mServer.Listen(inetLayer, &allInterfaces, port));
 
     ChipLogProgress(Discovery, "CHIP minimal mDNS started advertising.");
+
+    AdvertiseRecords();
+
     return CHIP_NO_ERROR;
 }
 
@@ -569,6 +600,86 @@ FullQName AdvertiserMinMdns::GetCommisioningTextEntries(const CommissionAdvertis
     }
 }
 
+bool AdvertiserMinMdns::ShouldAdvertiseOn(const chip::Inet::InterfaceId id, const chip::Inet::IPAddress & addr)
+{
+    for (unsigned i = 0; i < mServer.GetEndpointCount(); i++)
+    {
+        const ServerBase::EndpointInfo & info = mServer.GetEndpoints()[i];
+
+        if (info.udp == nullptr)
+        {
+            continue;
+        }
+
+        if (info.interfaceId != id)
+        {
+            continue;
+        }
+
+        if (info.addressType != addr.Type())
+        {
+            continue;
+        }
+
+        return true;
+    }
+
+    return false;
+}
+
+void AdvertiserMinMdns::AdvertiseRecords()
+{
+    chip::Inet::InterfaceAddressIterator interfaceAddress;
+
+    if (!interfaceAddress.Next())
+    {
+        return;
+    }
+
+    for (; interfaceAddress.HasCurrent(); interfaceAddress.Next())
+    {
+        if (!IsCurrentInterfaceUsable(interfaceAddress))
+        {
+            continue;
+        }
+
+        if (!ShouldAdvertiseOn(interfaceAddress.GetInterfaceId(), interfaceAddress.GetAddress()))
+        {
+            continue;
+        }
+
+        chip::Inet::IPPacketInfo packetInfo;
+
+        packetInfo.Clear();
+        packetInfo.SrcAddress = interfaceAddress.GetAddress();
+        if (interfaceAddress.GetAddress().IsIPv4())
+        {
+            BroadcastIpAddresses::GetIpv4Into(packetInfo.DestAddress);
+        }
+        else
+        {
+            BroadcastIpAddresses::GetIpv6Into(packetInfo.DestAddress);
+        }
+        packetInfo.SrcPort   = kMdnsPort;
+        packetInfo.DestPort  = kMdnsPort;
+        packetInfo.Interface = interfaceAddress.GetInterfaceId();
+
+        QueryData queryData(QType::PTR, QClass::IN, false /* unicast */);
+        queryData.SetIsBootAdvertising(true);
+
+        mQueryResponder.ClearBroadcastThrottle();
+
+        CHIP_ERROR err = mResponseSender.Respond(0, queryData, &packetInfo);
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(Discovery, "Failed to advertise records: %s", ErrorStr(err));
+        }
+    }
+
+    // Once all automatic broadcasts are done, allow immediate replies once.
+    mQueryResponder.ClearBroadcastThrottle();
+}
+
 AdvertiserMinMdns gAdvertiser;
 } // namespace
 

src/lib/mdns/minimal/Parser.h

@@ -31,6 +31,8 @@ class QueryData
     QueryData(const QueryData &) = default;
     QueryData & operator=(const QueryData &) = default;
 
+    QueryData(QType type, QClass klass, bool unicast) : mType(type), mClass(klass), mAnswerViaUnicast(unicast) {}
+
     QueryData(QType type, QClass klass, bool unicast, const uint8_t * nameStart, const BytesRange & validData) :
         mType(type), mClass(klass), mAnswerViaUnicast(unicast), mNameIterator(validData, nameStart)
     {}
@@ -39,6 +41,11 @@ class QueryData
     QClass GetClass() const { return mClass; }
     bool RequestedUnicastAnswer() const { return mAnswerViaUnicast; }
 
+    /// Boot advertisement is an internal query meant to advertise all available
+    /// services at device startup time.
+    bool IsBootAdvertising() const { return mIsBootAdvertising; }
+    void SetIsBootAdvertising(bool isBootAdvertising) { mIsBootAdvertising = isBootAdvertising; }
+
     SerializedQNameIterator GetName() const { return mNameIterator; }
 
     /// Parses a query structure
@@ -55,7 +62,11 @@ class QueryData
     QType mType            = QType::ANY;
     QClass mClass          = QClass::ANY;
     bool mAnswerViaUnicast = false;
-    SerializedQNameIterator mNameIterator; // const since we reuse it
+    SerializedQNameIterator mNameIterator;
+
+    /// Flag as a boot-time internal query. This allows query replies
+    /// to be built accordingly.
+    bool mIsBootAdvertising = false;
 };
 
 class ResourceData

src/lib/mdns/minimal/QueryReplyFilter.h

@@ -33,19 +33,17 @@ class QueryReplyFilter : public ReplyFilter
 
     bool Accept(QType qType, QClass qClass, FullQName qname) override
     {
-        // resource type is ignored
-        if ((mQueryData.GetType() != QType::ANY) && (mQueryData.GetType() != qType))
+        if (!AcceptableQueryType(qType))
         {
             return false;
         }
 
-        if ((mQueryData.GetClass() != QClass::ANY) && (mQueryData.GetClass() != qClass))
+        if (!AcceptableQueryClass(qClass))
         {
             return false;
         }
 
-        // path must match
-        return mIgnoreNameMatch || (mQueryData.GetName() == qname);
+        return AcceptablePath(qname);
     }
 
     /// Ignore qname matches during Accept calls (if set to true, only qtype and qclass are matched).
@@ -59,9 +57,41 @@ class QueryReplyFilter : public ReplyFilter
         return *this;
     }
 
+    QueryReplyFilter & SetSendingAdditionalItems(bool additional)
+    {
+        mSendingAdditionalItems = additional;
+        return *this;
+    }
+
 private:
+    bool AcceptableQueryType(QType qType)
+    {
+        if (mSendingAdditionalItems && mQueryData.IsBootAdvertising())
+        {
+            return true;
+        }
+
+        return ((mQueryData.GetType() == QType::ANY) || (mQueryData.GetType() == qType));
+    }
+
+    bool AcceptableQueryClass(QClass qClass)
+    {
+        return ((mQueryData.GetClass() == QClass::ANY) || (mQueryData.GetClass() == qClass));
+    }
+
+    bool AcceptablePath(FullQName qname)
+    {
+        if (mIgnoreNameMatch || mQueryData.IsBootAdvertising())
+        {
+            return true;
+        }
+
+        return (mQueryData.GetName() == qname);
+    }
+
     const QueryData & mQueryData;
-    bool mIgnoreNameMatch = false;
+    bool mIgnoreNameMatch        = false;
+    bool mSendingAdditionalItems = false;
 };
 
 } // namespace Minimal