Skip to content

This is a continuation on the topic of adding IPv6 Support to ESP32 Arduino #9016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Jan 15, 2024
Merged

This is a continuation on the topic of adding IPv6 Support to ESP32 Arduino #9016

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7b42a93
IPv6 for Arduino 3.0.0
Jason2866 Nov 21, 2023
12be369
Fix warning in WifiUdp
s-hadinger Nov 21, 2023
5987211
remove comment / formating
Jason2866 Nov 22, 2023
b8cc73c
Add zone to IPAddress and update WiFiUDP and WiFiGeneric
me-no-dev Dec 18, 2023
305d276
Add from ip_addr_t conversion and better toString implementation
me-no-dev Dec 18, 2023
41fb1a1
Use constant for IPAddress offset
me-no-dev Dec 18, 2023
b37ce6c
Combine hostByName to support both IPv6 and IPv4 results
me-no-dev Dec 18, 2023
e333afb
implement logic to use v6 dns only when global v6 address is assigned…
me-no-dev Jan 11, 2024
c7e63e6
Rename softAPenableIPv6
me-no-dev Jan 11, 2024
55aaa6f
Rename mDNS methods
me-no-dev Jan 11, 2024
c4f366c
fix IPAddress method to work with const address
me-no-dev Jan 11, 2024
701d35f
Some cleanup and do not print zone in IPAddress
me-no-dev Jan 11, 2024
a1b3f16
Merge branch 'master' into feature/ipv6_support
me-no-dev Jan 11, 2024
cb381f2
rename WiFiMulti method
me-no-dev Jan 11, 2024
726496c
Fix AP DHCPS not properly working on recent IDF
me-no-dev Jan 11, 2024
9012f64
Add option to print the zone at the end of IPv6
me-no-dev Jan 11, 2024
58520a7
remove log prints from hostByName
me-no-dev Jan 11, 2024
aad1041
Use correct array length for listing IPv6 addresses
me-no-dev Jan 12, 2024
04a2034
Merge branch 'master' into feature/ipv6_support
me-no-dev Jan 12, 2024
a2a6bd8
Implement some Tasmota requirements
me-no-dev Jan 14, 2024
1fb442d
add 'const' to IPAddress::addr_type()
me-no-dev Jan 14, 2024
0df3aaa
Fix WiFiUdp not updating mapped v4 address
me-no-dev Jan 15, 2024
4161873
Update WiFiServer.cpp
me-no-dev Jan 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Combine hostByName to support both IPv6 and IPv4 results
  • Loading branch information
@me-no-dev
me-no-dev committed Dec 18, 2023
commit b37ce6c772a1aeddd998cde7119958c220f8fe65
15 changes: 1 addition & 14 deletions libraries/WiFi/src/WiFiClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,21 +319,8 @@ int WiFiClient::connect(const char *host, uint16_t port)

int WiFiClient::connect(const char *host, uint16_t port, int32_t timeout_ms)
{
if (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT) {
ip_addr_t srv6;
if(!WiFiGenericClass::hostByName6(host, srv6)){
return 0;
}
if (srv6.type == IPADDR_TYPE_V4) {
IPAddress ip(srv6.u_addr.ip4.addr);
return connect(ip, port, timeout_ms);
} else {
IPAddress ip(IPv6, (uint8_t*)&srv6.u_addr.ip6.addr[0]);
return connect(ip, port, timeout_ms);
}
}
IPAddress srv((uint32_t)0);
if(!WiFiGenericClass::hostByName(host, srv)){
if(!WiFiGenericClass::hostByName(host, srv, (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT))){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than change the signature, do the check for (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT) inside WiFiGeneric

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. left there to remind me to ask for discussion

return 0;
}
return connect(srv, port, timeout_ms);
Expand Down
112 changes: 37 additions & 75 deletions libraries/WiFi/src/WiFiGeneric.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1546,10 +1546,12 @@ bool WiFiGenericClass::setDualAntennaConfig(uint8_t gpio_ant1, uint8_t gpio_ant2
// ------------------------------------------------ Generic Network function ---------------------------------------------
// -----------------------------------------------------------------------------------------------------------------------

struct dns_api_msg6 {
ip_addr_t ip_addr;
typedef struct gethostbynameParameters {
const char *hostname;
ip_addr_t addr;
uint8_t addr_type;
int result;
};
} gethostbynameParameters_t;

/**
* DNS callback
Expand All @@ -1559,45 +1561,26 @@ struct dns_api_msg6 {
*/
static void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg)
{
gethostbynameParameters_t *parameters = static_cast<gethostbynameParameters_t *>(callback_arg);
if(ipaddr) {
(*reinterpret_cast<IPAddress*>(callback_arg)) = ipaddr->u_addr.ip4.addr;
if(parameters->result == 0){
memcpy(&(parameters->addr), ipaddr, sizeof(ip_addr_t));
parameters->result = 1;
}
} else {
parameters->result = -1;
}
xEventGroupSetBits(_arduino_event_group, WIFI_DNS_DONE_BIT);
}

typedef struct gethostbynameParameters {
const char *hostname;
ip_addr_t addr;
void *callback_arg;
} gethostbynameParameters_t;

/**
* Callback to execute dns_gethostbyname in lwIP's TCP/IP context
* @param param Parameters for dns_gethostbyname call
*/
static esp_err_t wifi_gethostbyname_tcpip_ctx(void *param)
{
gethostbynameParameters_t *parameters = static_cast<gethostbynameParameters_t *>(param);
return dns_gethostbyname(parameters->hostname, &parameters->addr, &wifi_dns_found_callback, parameters->callback_arg);
}

/**
* IPv6 compatible DNS callback
* @param name
* @param ipaddr
* @param callback_arg
*/
static void wifi_dns6_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg)
{
struct dns_api_msg6 *msg = (struct dns_api_msg6 *)callback_arg;

if(ipaddr && !msg->result) {
msg->ip_addr = *ipaddr;
msg->result = 1;
} else {
msg->result = -1;
}
xEventGroupSetBits(_arduino_event_group, WIFI_DNS_DONE_BIT);
return dns_gethostbyname_addrtype(parameters->hostname, &parameters->addr, &wifi_dns_found_callback, parameters, parameters->addr_type);
}

/**
Expand All @@ -1607,60 +1590,39 @@ static void wifi_dns6_found_callback(const char *name, const ip_addr_t *ipaddr,
* @return 1 if aIPAddrString was successfully converted to an IP address,
* else error code
*/
int WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult)
int WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, bool preferV6)
Copy link
Contributor

@sgryphon sgryphon Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry about changing the signature here. It means that every app that uses this code needs to change. Even if they don't want or aren't ready for IPv6.

Rather than add this parameter, can we just do a check inside this function:

params.addre_type = (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT) ? LWIP_DNS_ADDRTYPE_IPV6_IPV4 : LWIP_DNS_ADDRTYPE_IPV4;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you again looking at the implementation... in the header a default value is given false so no code needs to change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you could make it an optional parameter with some define as default.
Then you could set your default preference a compile-time define or set it explicitly in your code.

The if-no-defined-Arduino-default could then be false here to remain IPv4 compatible.

Or add a 2nd function signature which then uses a static defined bool (which can be set by the user at runtime) to call the function with this bool argument.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TD-er read my comment. nothing needs to be done. default values are defined in headers, not in source files

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, read your post after a reload of the page. (there is another post here where you even 'beat' me to it by about 45 mins which did not show up when I typed my reply)

{
if (!aResult.fromString(aHostname))
{
gethostbynameParameters_t params;
params.hostname = aHostname;
params.callback_arg = &aResult;
aResult = static_cast<uint32_t>(0);
err_t err = ERR_OK;
gethostbynameParameters_t params;

aResult = static_cast<uint32_t>(0);
params.hostname = aHostname;
params.addr_type = preferV6?LWIP_DNS_ADDRTYPE_IPV6_IPV4:LWIP_DNS_ADDRTYPE_IPV4_IPV6;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just make this:

params.addr_type = (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT) ? LWIP_DNS_ADDRTYPE_IPV6_IPV4 : LWIP_DNS_ADDRTYPE_IPV4;

If WIFI_WANT_IP6_BIT is turn off, then we only want IPV4. Do not return IPv6 at all, because it might break backwards clients. If a client turns off the IPv6 bit they should only get IPv4 results (like before), i.e. they are IPv4 only. Note the IPV4 only.

If IPv6 is turned on, the return IPv6 if available (thats why they turned it on!), but with fall back to IPv4.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function only returns one address, so if you are doing something where you want both IPv6 global and link-local (although the later don't usually come from DNS) addresses, it isn't very useful.

You really also need to know the context. e.g. if an address has global IPv6, global IPv4, and link-local IPv6, you probably want the global IPv6 (although it is has a relevant link-local maybe use that).

But maybe if you have only link-local IPv6 and a global IPv4, you actually want to return the IPv4.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT) is probably not sufficient.

You probably want (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT) && haveGlobalIPv6Address.

Just because you want IPv6 doesn't mean you got it. Maybe you are on an IPv4 only network. You might have auto-assigned yourself an IPv6 link-local address, but if there are no other IPv6 nodes that may be useless, as will a DNS result for google.com that returns you the global IPv6, because you have no way to reach it.

Maybe the network stack takes care of this? (i.e. will it only return addresses you can reach, or will it return all addresses).

RFC 6724 talks about address selection, so may be relevant, where the destination addresses need to be filtered by which make sense based on the available source addresses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. it can not be as simple as now, because there are many cases which will cause trouble. I do vote to not bother and change it here much, because after this is merged, I will continue on rewriting the network stack of Arduino and the new things will allow to ask if we have a proper IPv6 address present. What we need to do is define the proper algo to decide what kind of query to issue and how to make sure that results are what they should be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you probably don't even need to check (WiFiGenericClass::getStatusBits() & WIFI_WANT_IP6_BIT) at all ... all you need to check is (the nonexistent) haveGlobalIPv6Address.

You will only get an address if WANT is on, so re-checking WANT is not necessary.

There are a few different scenarios, if WANT is off, then just do the IPv4 behaviour. If WANT is on, then sometimes you will be assigned an global/routable address, and then can use IPv6 lookups. Even if you don't get an address, you can still assign your own link-local, and communicate with other link-local. DNS probably isn't going to return link-local addresses, but mDNS might.

Happy to go with something initial, and then refine.

I did notice the LWIP has a ip6_select_source_address function based on RFC 6724 (given a particular destination, work out which source address to use from a given interface).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going with the current implementation, you should still change to:

 params.addr_type = preferV6?LWIP_DNS_ADDRTYPE_IPV6_IPV4:LWIP_DNS_ADDRTYPE_IPV4;

Using LWIP_DNS_ADDRTYPE_IPV4 instead of LWIP_DNS_ADDRTYPE_IPV4_IPV6 for when IPv6 is off.

Because if IPv6 is off for backwards compatiblity we only want to return IPv4; otherwise we might break apps (e.g. if no IPv4 we should return nothing instead of IPv6).

params.result = 0;
aResult.to_ip_addr_t(&(params.addr));

if (!aResult.fromString(aHostname)) {
waitStatusBits(WIFI_DNS_IDLE_BIT, 16000);
clearStatusBits(WIFI_DNS_IDLE_BIT | WIFI_DNS_DONE_BIT);
err_t err = esp_netif_tcpip_exec(wifi_gethostbyname_tcpip_ctx, &params);
if(err == ERR_OK && params.addr.u_addr.ip4.addr) {
aResult = params.addr.u_addr.ip4.addr;
} else if(err == ERR_INPROGRESS) {

err = esp_netif_tcpip_exec(wifi_gethostbyname_tcpip_ctx, &params);
if (err == ERR_OK) {
aResult.from_ip_addr_t(&(params.addr));
} else if (err == ERR_INPROGRESS) {
waitStatusBits(WIFI_DNS_DONE_BIT, 15000); //real internal timeout in lwip library is 14[s]
clearStatusBits(WIFI_DNS_DONE_BIT);
if (params.result == 1) {
aResult.from_ip_addr_t(&(params.addr));
err = ERR_OK;
}
}
setStatusBits(WIFI_DNS_IDLE_BIT);
if((uint32_t)aResult == 0){
log_e("DNS Failed for %s", aHostname);
}
}
return (uint32_t)aResult != 0;
}

/**
* Resolve the given hostname to an IP6 address.
* @param aHostname Name to be resolved
* @param aResult IPv6Address structure to store the returned IP address
* @return 1 if aHostname was successfully converted to an IP address,
* else error code
*/
int WiFiGenericClass::hostByName6(const char* aHostname, ip_addr_t& aResult)
{
ip_addr_t addr;
struct dns_api_msg6 arg;

memset(&arg, 0x0, sizeof(arg));
waitStatusBits(WIFI_DNS_IDLE_BIT, 16000);
clearStatusBits(WIFI_DNS_IDLE_BIT | WIFI_DNS_DONE_BIT);

err_t err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns6_found_callback,
&arg, LWIP_DNS_ADDRTYPE_IPV6_IPV4);
if(err == ERR_OK) {
aResult = addr;
} else if(err == ERR_INPROGRESS) {
waitStatusBits(WIFI_DNS_DONE_BIT, 15000); //real internal timeout in lwip library is 14[s]
clearStatusBits(WIFI_DNS_DONE_BIT);
if (arg.result == 1) {
aResult = arg.ip_addr;
}
if (err == ERR_OK) {
return 1;
}
setStatusBits(WIFI_DNS_IDLE_BIT);
return (uint32_t)err == ERR_OK || (err == ERR_INPROGRESS && arg.result == 1);
log_e("DNS Failed for '%s' with error '%d' and result '%d'", aHostname, err, params.result);
return err;
}

IPAddress WiFiGenericClass::calculateNetworkID(IPAddress ip, IPAddress subnet) {
Expand Down
3 changes: 1 addition & 2 deletions libraries/WiFi/src/WiFiGeneric.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ class WiFiGenericClass
static const char * getHostname();
static bool setHostname(const char * hostname);
static bool hostname(const String& aHostname) { return setHostname(aHostname.c_str()); }
static int hostByName6(const char *aHostname, ip_addr_t& aResult);

static esp_err_t _eventCallback(arduino_event_t *event);

Expand All @@ -219,7 +218,7 @@ class WiFiGenericClass
static bool _isReconnectableReason(uint8_t reason);

public:
static int hostByName(const char *aHostname, IPAddress &aResult);
static int hostByName(const char *aHostname, IPAddress &aResult, bool preferV6=false);

static IPAddress calculateNetworkID(IPAddress ip, IPAddress subnet);
static IPAddress calculateBroadcast(IPAddress ip, IPAddress subnet);
Expand Down