Skip to content
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.

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

feat(dns): Check type of IP addresses and clear DNS cache if they changed #9476

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

sgryphon
Copy link
Contributor

@sgryphon sgryphon commented Apr 9, 2024

Description of Change

Check if the device has a global IPv6 address, and if the status has changed (changing from no address to having an address, or vice-versa) clear the DNS cache.

For consistency, the code also checks if you have an IPv4 address, and handles that similarly.

Tests scenarios

Tested with M5Stack Core 2, using the following PlatformIO configuration and the test app from https://github.com/sgryphon/iot-demo-build/tree/main/m5stack/m5unified_wifi_https

PlatformIO configuration:

platform = https://github.com/sgryphon/platform-espressif32.git#sgryphon/add-esp32-arduino-libs
platform_packages = 
  platformio/framework-arduinoespressif32-libs @ https://github.com/sgryphon/esp32-arduino-libs.git#sgryphon/test-fix-ipv6-config
  platformio/framework-arduinoespressif32 @ https://github.com/sgryphon/arduino-esp32.git#sgryphon/dns-has-ipv6-check

Results after change:

Network Dual-Stack IPv6 IPv4
Dual-stack+NAT64 (Astral) IPv4 Yes IPv4
IPv6+NAT64 (Wildspace) fail -- uses IPv4 Yes fail -- uses IPv4

These are the same as before the change.

The failure is because the device is on an IPv6-only network, but DNS is hard coded to return IPv4 first, so a dual-stack destination does not work (it gets the unusable IPv4), but an IPv6 only destination is fine. An IPv4-only destination also fails, even though NAT64 is active, because it uses the IPv4 address instead of the DNS64 address.

Results for other network types all work, although dual-stack to dual-stack, where both are available, uses IPv4.

Failing example connecting to a dual-stack destination. Note the DNS cache is cleared as the flag changes from the default of false to true. This is also the first DNS request for the app. It fails because IPv4 is returned for the dual-stack destination, even though we can't use.

[  3538][D][STA.cpp:133] _onStaArduinoEvent(): Arduino STA Event: 17 - STA_GOT_IP6
[  3546][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: WF STA IPv6 fe80:0000:0000:0000:0a3a:f2ff:fe65:db28
[  3561][D][WiFiNetworkService.cpp:47] wifiOnEvent(): [Network] IPv6 address type 2
[  4538][D][STA.cpp:133] _onStaArduinoEvent(): Arduino STA Event: 17 - STA_GOT_IP6
[  4546][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: WF STA IPv6 2407:8800:bc61:1300:0a3a:f2ff:fe65:db28
[  4560][I][Core2Logger.cpp:176] success(): [Core2Logger] Success
[  4566][D][WiFiNetworkService.cpp:47] wifiOnEvent(): [Network] IPv6 address type 1
[  4574][D][STA.cpp:133] _onStaArduinoEvent(): Arduino STA Event: 17 - STA_GOT_IP6
[  4581][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: WF STA IPv6 fd7c:e25e:67e8:0000:0a3a:f2ff:fe65:db28
[  4596][I][Core2Logger.cpp:176] success(): [Core2Logger] Success
[  4602][D][WiFiNetworkService.cpp:47] wifiOnEvent(): [Network] IPv6 address type 4
[ 14202][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: Button 1, scenario 0, v0.1.0-160-g02b6d90-dev
[ 14215][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: Global IPv6 2407:8800:bc61:1300:a3a:f2ff:fe65:db28
[ 14229][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: IPv4 0.0.0.0
*sta: <UP,Wildspace,CH:1,RSSI:-58,N,WPA2_PSK> (DHCPC,GARP,IP_MOD,V6_REP)
      ether 08:3A:F2:65:DB:28
      inet 0.0.0.0 netmask 0.0.0.0 broadcast 255.255.255.255
      gateway 0.0.0.0 dns fd7c:e25e:67e8::1
      inet6 fe80::a3a:f2ff:fe65:db28%st1 type LINK_LOCAL
      inet6 2407:8800:bc61:1300:a3a:f2ff:fe65:db28 type GLOBAL
      inet6 fd7c:e25e:67e8:0:a3a:f2ff:fe65:db28 type UNIQUE_LOCAL
[ 14262][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: DNS0 fd7c:e25e:67e8::1
[ 14281][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: DNS1 0.0.0.0
[ 14289][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: URL: http://v4v6.ipv6-test.com/api/myip.php
[ 14302][D][HTTPClient.cpp:303] beginInternal(): protocol: http, host: v4v6.ipv6-test.com port: 80 url: /api/myip.php
[ 14313][D][HTTPClient.cpp:598] sendRequest(): request type: 'GET' redirCount: 0
[ 14320][D][NetworkManager.cpp:86] hostByName(): Clearing DNS cache
[ 14357][D][NetworkManager.cpp:109] hostByName(): DNS found IPv4 51.75.78.103
[ 14365][E][NetworkClient.cpp:258] connect(): connect on fd 48, errno: 118, "Host is unreachable"
[ 14374][D][HTTPClient.cpp:1163] connect(): failed connect to v4v6.ipv6-test.com:80
[ 14381][W][HTTPClient.cpp:1486] returnError(): error(-1): connection refused
[ 14388][E][Core2Logger.cpp:192] log(): [Core2Logger] CORE2: HTTP GET error -1: connection refused

Log connecting to an IPv6-only destination. No need to clear the cache, as then network has not changed. As the destination is also IPv6-only, we get the IPv6 result, which works.

[ 34067][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: Button 2, scenario 1, v0.1.0-160-g02b6d90-dev
[ 34080][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: Global IPv6 2407:8800:bc61:1300:a3a:f2ff:fe65:db28
[ 34094][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: IPv4 0.0.0.0
*sta: <UP,Wildspace,CH:1,RSSI:-57,N,WPA2_PSK> (DHCPC,GARP,IP_MOD,V6_REP)
      ether 08:3A:F2:65:DB:28
      inet 0.0.0.0 netmask 0.0.0.0 broadcast 255.255.255.255
      gateway 0.0.0.0 dns fd7c:e25e:67e8::1
      inet6 fe80::a3a:f2ff:fe65:db28%st1 type LINK_LOCAL
      inet6 2407:8800:bc61:1300:a3a:f2ff:fe65:db28 type GLOBAL
      inet6 fd7c:e25e:67e8:0:a3a:f2ff:fe65:db28 type UNIQUE_LOCAL
[ 34127][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: DNS0 fd7c:e25e:67e8::1
[ 34146][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: DNS1 0.0.0.0
[ 34154][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: URL: http://v6.ipv6-test.com/api/myip.php
[ 34167][D][HTTPClient.cpp:303] beginInternal(): protocol: http, host: v6.ipv6-test.com port: 80 url: /api/myip.php
[ 34177][D][HTTPClient.cpp:598] sendRequest(): request type: 'GET' redirCount: 0
[ 34232][D][NetworkManager.cpp:102] hostByName(): DNS found IPv6 2001:41d0:701:1100::29c8
[ 34671][D][HTTPClient.cpp:1170] connect():  connected to v6.ipv6-test.com:80
[ 35293][D][HTTPClient.cpp:1321] handleHeaderResponse(): code: 200
[ 35299][D][HTTPClient.cpp:1328] handleHeaderResponse(): Transfer-Encoding: chunked
[ 35307][D][HTTPClient.cpp:642] sendRequest(): sendRequest code=200
[ 35313][D][HTTPClient.cpp:388] disconnect(): still data in buffer (2), clean up.
[ 35321][D][HTTPClient.cpp:393] disconnect(): tcp keep open for reuse
[ 35327][I][Core2Logger.cpp:198] log(): [Core2Logger] CORE2: response=<2407:8800:bc61:1300:a3a:f2ff:fe65:db28>
[ 35342][I][Core2Logger.cpp:176] success(): [Core2Logger] Success

Related links

Requested to split the IPv6 DNS fix into separate PRs that implement the address type check separately, and then the bug fix workaround to dynamically check DNS based on the available address types.

Note that the bug will also be fixed if the LWIP change, to implement RFC6724 destination address selection, is merged.

Copy link
Contributor

github-actions bot commented Apr 9, 2024

Messages
📖 🎉 Good Job! All checks are passing!

👋 Hello sgryphon, we appreciate your contribution to this project!


Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 1986126

@me-no-dev me-no-dev merged commit 858b107 into espressif:master Apr 10, 2024
42 checks passed
P-R-O-C-H-Y pushed a commit to P-R-O-C-H-Y/arduino-esp32 that referenced this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants