Support LWIP_HOOK_NETCONN_EXTERNAL_RESOLVE / LWIP_HOOK_FILENAME in dns_gethostbyname_addrtype (IDFGH-6101)

[mo22]

Using the following patch the external resolve hook works for all API types, not just for the gethostbyname() api.

This makes it possible to use a custom resolve hook for esp based project to get proper mdns resolution.

diff --git a/src/core/dns.c b/src/core/dns.c
index e758aca5..1429b423 100644
--- a/src/core/dns.c
+++ b/src/core/dns.c
@@ -95,6 +95,10 @@
 #include "lwip/dns.h"
 #include "lwip/prot/dns.h"

+#ifdef LWIP_HOOK_FILENAME
+#include LWIP_HOOK_FILENAME
+#endif
+
 #include <string.h>

 /** Random generator function to create random TXIDs and source ports for queries */
@@ -1620,6 +1624,14 @@ dns_gethostbyname_addrtype(const char *hostname, ip_addr_t *addr, dns_found_call
     return ERR_ARG;
   }

+#ifdef LWIP_HOOK_NETCONN_EXTERNAL_RESOLVE
+  {
+    err_t err = ERR_OK;
+    if (LWIP_HOOK_NETCONN_EXTERNAL_RESOLVE(hostname, addr, dns_addrtype, &err)) {
+      return err;
+    }
+  }
+#endif /* LWIP_HOOK_NETCONN_EXTERNAL_RESOLVE */

 #if LWIP_HAVE_LOOPIF
   if (strcmp(hostname, "localhost") == 0) {

[david-cermak]

@mo22 We should not allow for this kind of hook, since the dns_gethostbyname_addrtype() is supposed to be called from lwip context. The idea behind the api_msg.c is that we execute a DNS query from user tasks where is the right place for custom hooks.

[david-cermak]

Closing this for the reasons mentioned above.

Please note that if you wanted to insert such hooks you're effectively stalling the TCPIP thread, and adding a blocking mdns resolution (as you suggested in the referenced issue) would not only suspend the lwip task for seconds, but might also cause potential deadlocks as mdns internals rely on messaging to lwip task.

[mo22]

@david-cermak so dns_gethostbyname_addrtype() is supposed to be used internally only? I found that the ESP32 Arduino Core uses that function for resolving HTTP lookups, and for some reason I have not yet found out the lwip mdns resolver does not correctly resolve other esp32s, whereas the esp32 mdns library does.

If dns_gethostbyname_addrtype() is internal only I would submit an issue to the Arduino lib to change it to the non-internal API, if it is public it might be the better place for the dns hook, as it is called by the other functions. The hook could be extended to allow for asynchronous resolution.

[david-cermak]

dns_gethostbyname_addrtype() is supposed to be used internally only?

No, It's not internal only, but should be called either from TCPIP task or with core-locking.