From f06710eb6e1559b8a0035d02c0c0fc0c860b821f Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Thu, 5 Jan 2023 16:29:14 +0300 Subject: [PATCH] hostByName timeout fixes (#8787) * single impl --- cores/esp8266/IPAddress.cpp | 5 + cores/esp8266/IPAddress.h | 4 +- .../ESP8266WiFi/src/ESP8266WiFiGeneric.cpp | 185 +++++++++--------- .../ESP8266WiFi/src/ESP8266WiFiGeneric.h | 15 +- tests/host/common/ClientContextTools.cpp | 21 +- 5 files changed, 126 insertions(+), 104 deletions(-) diff --git a/cores/esp8266/IPAddress.cpp b/cores/esp8266/IPAddress.cpp index c269d0050..25660ec0b 100644 --- a/cores/esp8266/IPAddress.cpp +++ b/cores/esp8266/IPAddress.cpp @@ -27,6 +27,11 @@ IPAddress::IPAddress(const IPAddress& from) ip_addr_copy(_ip, from._ip); } +IPAddress::IPAddress(IPAddress&& from) +{ + ip_addr_copy(_ip, from._ip); +} + IPAddress::IPAddress() { _ip = *IP_ANY_TYPE; // lwIP's v4-or-v6 generic address } diff --git a/cores/esp8266/IPAddress.h b/cores/esp8266/IPAddress.h index 894d431c6..3c4d12965 100644 --- a/cores/esp8266/IPAddress.h +++ b/cores/esp8266/IPAddress.h @@ -66,7 +66,9 @@ class IPAddress: public Printable { public: // Constructors IPAddress(); - IPAddress(const IPAddress& from); + IPAddress(const IPAddress&); + IPAddress(IPAddress&&); + IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet); IPAddress(uint32_t address) { ctor32(address); } IPAddress(unsigned long address) { ctor32(address); } diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp index 9b7fccc9a..be482737a 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp @@ -22,8 +22,11 @@ */ +#include #include -#include +#include +#include + #include #include #include "ESP8266WiFi.h" @@ -595,9 +598,83 @@ bool ESP8266WiFiGenericClass::isSleepLevelMax () { // ------------------------------------------------ Generic Network function --------------------------------------------- // ----------------------------------------------------------------------------------------------------------------------- -void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg); +namespace { -static bool _dns_lookup_pending = false; +struct _dns_found_result { + IPAddress addr; + bool done; +}; + +} + +static void _dns_found_callback(const char *, const ip_addr_t *, void *); + +static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) { + if (aResult.fromString(aHostname)) { + DEBUG_WIFI_GENERIC("[hostByName] Host: %s is IP!\n", aHostname); + return 1; + } + + static_assert(std::is_same_v>, ""); + DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname); + + ip_addr_t addr; + auto pending = std::make_unique<_dns_found_result>( + _dns_found_result{ + .addr = IPADDR_NONE, + .done = false, + }); + + err_t err = dns_gethostbyname_addrtype(aHostname, + &addr, &_dns_found_callback, pending.get(), + static_cast(resolveType)); + + switch (err) { + // Address already known + case ERR_OK: + aResult = addr; + break; + + // We are no longer able to issue requests + case ERR_MEM: + break; + + // We need to wait for c/b to fire *or* we exit on our own timeout + // (which also requires us to notify the c/b that it is supposed to delete the pending obj) + case ERR_INPROGRESS: + // Re-check every 10ms, we expect this to happen fast + esp_delay(timeout_ms, + [&]() { + return !pending->done; + }, 10); + + if (pending->done) { + if ((pending->addr).isSet()) { + aResult = pending->addr; + err = ERR_OK; + } + } else { + pending->done = true; + pending.release(); + err = ERR_TIMEOUT; + } + + break; + } + + if (err == ERR_OK) { + DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str()); + return 1; + } + + DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %s (%d)!\n", + aHostname, + (err == ERR_TIMEOUT) ? "Timeout" : + (err == ERR_INPROGRESS) ? "No response" : + "Unknown", static_cast(err)); + + return 0; +} /** * Resolve the given hostname to an IP address. @@ -608,99 +685,18 @@ static bool _dns_lookup_pending = false; */ int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult) { - return hostByName(aHostname, aResult, 10000); + return hostByNameImpl(aHostname, aResult, DNSDefaultTimeoutMs, DNSResolveTypeDefault); } - int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms) { - ip_addr_t addr; - aResult = static_cast(INADDR_NONE); - - if(aResult.fromString(aHostname)) { - // Host name is a IP address use it! - DEBUG_WIFI_GENERIC("[hostByName] Host: %s is a IP!\n", aHostname); - return 1; - } - - DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname); -#if LWIP_IPV4 && LWIP_IPV6 - err_t err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult,LWIP_DNS_ADDRTYPE_DEFAULT); -#else - err_t err = dns_gethostbyname(aHostname, &addr, &wifi_dns_found_callback, &aResult); -#endif - if(err == ERR_OK) { - aResult = IPAddress(&addr); - } else if(err == ERR_INPROGRESS) { - _dns_lookup_pending = true; - // Will resume on timeout or when wifi_dns_found_callback fires. - // The final argument, intvl_ms, to esp_delay influences how frequently - // the scheduled recurrent functions (Schedule.h) are probed; here, to allow - // the ethernet driver perform work. - esp_delay(timeout_ms, []() { return _dns_lookup_pending; }, 1); - _dns_lookup_pending = false; - if(aResult.isSet()) { - err = ERR_OK; - } - } - - if(err == ERR_OK) { - DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str()); - return 1; - } - - DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %s (%d)!\n", aHostname, lwip_strerr(err), (int)err); - return 0; + return hostByNameImpl(aHostname, aResult, timeout_ms, DNSResolveTypeDefault); } #if LWIP_IPV4 && LWIP_IPV6 int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) { - ip_addr_t addr; - err_t err; - aResult = static_cast(INADDR_NONE); - - if(aResult.fromString(aHostname)) { - // Host name is a IP address use it! - DEBUG_WIFI_GENERIC("[hostByName] Host: %s is a IP!\n", aHostname); - return 1; - } - - DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname); - switch(resolveType) - { - // Use selected addrtype - case DNSResolveType::DNS_AddrType_IPv4: - case DNSResolveType::DNS_AddrType_IPv6: - case DNSResolveType::DNS_AddrType_IPv4_IPv6: - case DNSResolveType::DNS_AddrType_IPv6_IPv4: - err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult, (uint8_t) resolveType); - break; - default: - err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult, LWIP_DNS_ADDRTYPE_DEFAULT); // If illegal type, use default. - break; - } - - if(err == ERR_OK) { - aResult = IPAddress(&addr); - } else if(err == ERR_INPROGRESS) { - _dns_lookup_pending = true; - // will resume on timeout or when wifi_dns_found_callback fires - esp_delay(timeout_ms, []() { return _dns_lookup_pending; }); - _dns_lookup_pending = false; - // will return here when dns_found_callback fires - if(aResult.isSet()) { - err = ERR_OK; - } - } - - if(err == ERR_OK) { - DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str()); - return 1; - } - - DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %d!\n", aHostname, (int)err); - return 0; + return hostByNameImpl(aHostname, aResult, timeout_ms, resolveType); } #endif @@ -710,16 +706,19 @@ int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResul * @param ipaddr * @param callback_arg */ -void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg) +static void _dns_found_callback(const char*, const ip_addr_t* ipaddr, void* arg) { - (void) name; - if (!_dns_lookup_pending) { + auto result = reinterpret_cast<_dns_found_result*>(arg); + if (result->done) { + delete result; return; } - if(ipaddr) { - (*reinterpret_cast(callback_arg)) = IPAddress(ipaddr); + + if (ipaddr) { + result->addr = IPAddress(ipaddr); } - _dns_lookup_pending = false; // resume hostByName + + result->done = true; esp_schedule(); } diff --git a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h index 3d3ceb326..d0525176b 100644 --- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h +++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h @@ -24,6 +24,10 @@ #define ESP8266WIFIGENERIC_H_ #include "ESP8266WiFiType.h" + +#include +#include + #include #include @@ -44,12 +48,15 @@ typedef void (*WiFiEventCb)(WiFiEvent_t); enum class DNSResolveType: uint8_t { - DNS_AddrType_IPv4 = 0, // LWIP_DNS_ADDRTYPE_IPV4 = 0 - DNS_AddrType_IPv6, // LWIP_DNS_ADDRTYPE_IPV6 = 1 - DNS_AddrType_IPv4_IPv6, // LWIP_DNS_ADDRTYPE_IPV4_IPV6 = 2 - DNS_AddrType_IPv6_IPv4 // LWIP_DNS_ADDRTYPE_IPV6_IPV4 = 3 + DNS_AddrType_IPv4 = LWIP_DNS_ADDRTYPE_IPV4, + DNS_AddrType_IPv6 = LWIP_DNS_ADDRTYPE_IPV6, + DNS_AddrType_IPv4_IPv6 = LWIP_DNS_ADDRTYPE_IPV4_IPV6, + DNS_AddrType_IPv6_IPv4 = LWIP_DNS_ADDRTYPE_IPV6_IPV4, }; +inline constexpr auto DNSDefaultTimeoutMs = 10000; +inline constexpr auto DNSResolveTypeDefault = static_cast(LWIP_DNS_ADDRTYPE_DEFAULT); + struct WiFiState; class ESP8266WiFiGenericClass { diff --git a/tests/host/common/ClientContextTools.cpp b/tests/host/common/ClientContextTools.cpp index cf9987fea..74f66fd6b 100644 --- a/tests/host/common/ClientContextTools.cpp +++ b/tests/host/common/ClientContextTools.cpp @@ -37,18 +37,27 @@ #include // gethostbyname -err_t dns_gethostbyname(const char* hostname, ip_addr_t* addr, dns_found_callback found, - void* callback_arg) +err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback, void*, + u8 type) { - (void)callback_arg; - (void)found; - struct hostent* hbn = gethostbyname(hostname); + auto* hbn = gethostbyname(hostname); if (!hbn) return ERR_TIMEOUT; - addr->addr = *(uint32_t*)hbn->h_addr_list[0]; + + uint32_t tmp; + std::memcpy(&tmp, hbn->h_addr_list[0], sizeof(tmp)); + addr->addr = tmp; + return ERR_OK; } +err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback found, + void* callback_arg) +{ + return dns_gethostbyname_addrtype(hostname, addr, found, callback_arg, + LWIP_DNS_ADDRTYPE_DEFAULT); +} + static struct tcp_pcb mock_tcp_pcb; tcp_pcb* tcp_new(void) {