Generic netif status callback and mDNS (esp8266#8705)

* sprinkle IPAddress(...).isSet() across our loops to avoid polling on a stopped interface. status callback and netif_is_up **does not guarantee and we could use the interface**! * register *one* status callback per instance, e.g. when begin() is called multiple times (also notice a subtle issue with schedule function when instance is delete'ed) * consistent LwipIntf callback signature. no need for rvalue, just pass stdfunc as-is and let the compiler figure it out
mdogancay · Nov 1, 2022 · 04494f0 · 04494f0
1 parent 78444a5
commit 04494f0
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 41 deletions.
diff --git a/cores/esp8266/LwipIntf.h b/cores/esp8266/LwipIntf.h
@@ -34,8 +34,6 @@ class LwipIntf
 public:
     using CBType = std::function<void(netif*)>;
 
-    static bool stateUpCB(LwipIntf::CBType&& cb);
-
     // reorder WiFi.config() parameters for a esp8266/official Arduino dual-compatibility API
     // args     | esp order  arduino order
     // ----     + ---------  -------------
@@ -67,8 +65,11 @@ class LwipIntf
     // ESP32 API compatibility
     const char* getHostname();
 
-protected:
-    static bool stateChangeSysCB(LwipIntf::CBType&& cb);
+    // whenever netif status callback is called
+    static bool statusChangeCB(LwipIntf::CBType);
+
+    static bool stateUpCB(LwipIntf::CBType);
+    static bool stateDownCB(LwipIntf::CBType);
 };
 
 #endif  // _LWIPINTF_H
diff --git a/cores/esp8266/LwipIntfCB.cpp b/cores/esp8266/LwipIntfCB.cpp
@@ -25,44 +25,54 @@
 #include <Schedule.h>
 #include <debug.h>
 
-#define NETIF_STATUS_CB_SIZE 3
+static constexpr size_t LwipIntfCallbacks = 3;
 
-static int       netifStatusChangeListLength = 0;
-LwipIntf::CBType netifStatusChangeList[NETIF_STATUS_CB_SIZE];
+static LwipIntf::CBType callbacks[LwipIntfCallbacks];
+static size_t           size = 0;
 
+// override empty weak function from glue-lwip
 extern "C" void netif_status_changed(struct netif* netif)
 {
-    // override the default empty weak function
-    for (int i = 0; i < netifStatusChangeListLength; i++)
+    for (size_t index = 0; index < size; ++index)
     {
-        netifStatusChangeList[i](netif);
+        callbacks[index](netif);
     }
 }
 
-bool LwipIntf::stateChangeSysCB(LwipIntf::CBType&& cb)
+bool LwipIntf::statusChangeCB(LwipIntf::CBType cb)
 {
-    if (netifStatusChangeListLength >= NETIF_STATUS_CB_SIZE)
+    if (size < LwipIntfCallbacks)
     {
+        callbacks[size++] = std::move(cb);
+        return true;
+    }
 #if defined(DEBUG_ESP_CORE)
-        DEBUGV("NETIF_STATUS_CB_SIZE is too low\n");
+    DEBUGV("LwipIntf::CB %zu/%zu, cannot add more!\n", size, size);
 #endif
-        return false;
-    }
 
-    netifStatusChangeList[netifStatusChangeListLength++] = cb;
-    return true;
+    return false;
+}
+
+bool LwipIntf::stateUpCB(LwipIntf::CBType cb)
+{
+    return statusChangeCB(
+        [cb](netif* interface)
+        {
+            if (netif_is_up(interface))
+            {
+                cb(interface);
+            }
+        });
 }
 
-bool LwipIntf::stateUpCB(LwipIntf::CBType&& cb)
+bool LwipIntf::stateDownCB(LwipIntf::CBType cb)
 {
-    return stateChangeSysCB(
-        [cb](netif* nif)
+    return statusChangeCB(
+        [cb](netif* interface)
         {
-            if (netif_is_up(nif))
-                schedule_function(
-                    [cb, nif]()
-                    {
-                        cb(nif);
-                    });
+            if (!netif_is_up(interface))
+            {
+                cb(interface);
+            }
         });
 }
diff --git a/libraries/ESP8266mDNS/src/LEAmDNS.cpp b/libraries/ESP8266mDNS/src/LEAmDNS.cpp
@@ -27,7 +27,7 @@
 
 #include "ESP8266mDNS.h"
 #include "LEAmDNS_Priv.h"
-#include <LwipIntf.h>  // LwipIntf::stateUpCB()
+#include <LwipIntf.h>
 #include <lwip/igmp.h>
 #include <lwip/prot/dns.h>
 
@@ -54,7 +54,7 @@ namespace MDNSImplementation
     */
     MDNSResponder::MDNSResponder(void) :
         m_pServices(0), m_pUDPContext(0), m_pcHostname(0), m_pServiceQueries(0),
-        m_fnServiceTxtCallback(0)
+        m_fnServiceTxtCallback(0), m_bLwipCb(false), m_bRestarting(false)
     {
     }
 
@@ -89,17 +89,34 @@ namespace MDNSImplementation
             bResult = _restart();
         }
 
-        LwipIntf::stateUpCB(
-            [this](netif* intf)
-            {
-                if (IPAddress(intf->ip_addr).isSet())
+        if (bResult && !m_bLwipCb)
+        {
+            bool bCallback = LwipIntf::statusChangeCB(
+                [this](netif*)
                 {
-                    DEBUG_EX_INFO(DEBUG_OUTPUT.printf_P(
-                        PSTR("[MDNSResponder] new Interface '%c%c' is UP! restarting\n"),
-                        intf->name[0], intf->name[1]));
-                    _restart();
-                }
+                    if (m_bRestarting)
+                    {
+                        return;
+                    }
+
+                    m_bRestarting = true;
+                    schedule_function(
+                        [this]()
+                        {
+                            DEBUG_EX_INFO(
+                                DEBUG_OUTPUT.printf_P(PSTR("[MDNSResponder] begin: restarting "
+                                                           "after interface status changed\n")););
+                            _restart();
+                            m_bRestarting = false;
+                        });
+                });
+            DEBUG_EX_ERR(if (!bCallback) {
+                DEBUG_OUTPUT.printf_P(
+                    PSTR("[MDNSResponder] begin: FAILED LwipIntf::statusChangeCB!\n"));
             });
+            m_bLwipCb = bCallback;
+        }
+
         DEBUG_EX_ERR(if (!bResult) {
             DEBUG_OUTPUT.printf_P(PSTR("[MDNSResponder] begin: FAILED for '%s'!\n"),
                                   (p_pcHostname ?: "-"));
@@ -1281,7 +1298,7 @@ namespace MDNSImplementation
         // Join multicast group(s)
         for (netif* pNetIf = netif_list; pNetIf; pNetIf = pNetIf->next)
         {
-            if (netif_is_up(pNetIf))
+            if (netif_is_up(pNetIf) && IPAddress(pNetIf->ip_addr).isSet())
             {
 #ifdef MDNS_IP4_SUPPORT
                 ip_addr_t multicast_addr_V4 = DNS_MQUERY_IPV4_GROUP_INIT;
@@ -1337,7 +1354,7 @@ namespace MDNSImplementation
 
         for (netif* pNetIf = netif_list; pNetIf; pNetIf = pNetIf->next)
         {
-            if (netif_is_up(pNetIf))
+            if (netif_is_up(pNetIf) && IPAddress(pNetIf->ip_addr).isSet())
             {
                 bResult = true;
 

diff --git a/libraries/ESP8266mDNS/src/LEAmDNS.h b/libraries/ESP8266mDNS/src/LEAmDNS.h
@@ -1174,6 +1174,8 @@ namespace MDNSImplementation
         stcMDNSServiceQuery*              m_pServiceQueries;
         MDNSDynamicServiceTxtCallbackFunc m_fnServiceTxtCallback;
         stcProbeInformation               m_HostProbeInformation;
+        bool                              m_bLwipCb;
+        bool                              m_bRestarting;
 
         /** CONTROL **/
         /* MAINTENANCE */

diff --git a/libraries/ESP8266mDNS/src/LEAmDNS_Control.cpp b/libraries/ESP8266mDNS/src/LEAmDNS_Control.cpp
@@ -2228,7 +2228,7 @@ namespace MDNSImplementation
                 stcMDNS_RRDomain reverseIP4Domain;
                 for (netif* pNetIf = netif_list; pNetIf; pNetIf = pNetIf->next)
                 {
-                    if (netif_is_up(pNetIf))
+                    if (netif_is_up(pNetIf) && IPAddress(pNetIf->ip_addr).isSet())
                     {
                         if ((_buildDomainForReverseIP4(pNetIf->ip_addr, reverseIP4Domain))
                             && (p_RRHeader.m_Domain == reverseIP4Domain))

diff --git a/libraries/ESP8266mDNS/src/LEAmDNS_Transfer.cpp b/libraries/ESP8266mDNS/src/LEAmDNS_Transfer.cpp
@@ -122,7 +122,7 @@ namespace MDNSImplementation
 
         for (netif* pNetIf = netif_list; pNetIf; pNetIf = pNetIf->next)
         {
-            if (netif_is_up(pNetIf))
+            if (netif_is_up(pNetIf) && IPAddress(pNetIf->ip_addr).isSet())
             {
                 IPAddress fromIPAddress;
                 // fromIPAddress = _getResponseMulticastInterface();