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

esp_netif_set_ip_info doesn't always trigger an IP_CHANGED event. (IDFGH-11836) #12927

Closed
3 tasks done
karlp opened this issue Jan 4, 2024 · 1 comment
Closed
3 tasks done
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@karlp
Copy link
Contributor

karlp commented Jan 4, 2024

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.1.2-2-g11f9b315f7 (2 commits are unrelated phy changes, separate PR)

Espressif SoC revision.

ESP32

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

None

Development Kit.

custom

Power Supply used.

External 3.3V

What is the expected behavior?

When I use esp_netif_set_ip_info() to change to a different static IP address, I expect the IP_EVENT_ETH_GOT_IP (as appropriate for interface) to be published. This is required so that (for instance) mDNS can update automatically.

What is the actual behavior?

Instead, the event is not published. Note, this only fails when the gateway portion of a static IP is null.

Steps to reproduce.

	esp_netif_ip_info_t ip;
        ip.ip.addr = ipaddr_addr("10.1.1.1");
        ip.netmask.addr = ipaddr_addr("255.255.0.0");
        ip.gw.addr = ipaddr_addr("0.0.0.0");
        esp_netif_set_ip_info(netif, ip))
  1. No event will be produced.

Debug Logs.

No response

More Information.

The problem is here: https://github.com/espressif/esp-idf/blob/master/components/esp_netif/lwip/esp_netif_lwip.c#L1836:

The new IP address is checked, and if any of ip/mask/gw are null, the actual check on whether the IP has changed is just skipped entirely, regardless of what changes have taken place. Given that a null gateway is perfectly legitimate, I propose at least removing the ip4_addr_isany_val(ip_info->gw) check. This does fix the problem for me, and makes the system behave much more predictably.

Arguably, the entire triple check should be dropped though. The memcmp check below, that actually checks whether the addresses have changed at all should already completely cover this.

@karlp karlp added the Type: Bug bugs in IDF label Jan 4, 2024
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jan 4, 2024
@github-actions github-actions bot changed the title esp_netif_set_ip_info doesn't always trigger an IP_CHANGED event. esp_netif_set_ip_info doesn't always trigger an IP_CHANGED event. (IDFGH-11836) Jan 4, 2024
@david-cermak
Copy link
Collaborator

Hi @karlp

Thanks for reporting this problem. I agree that we don't have to check the default gateway nor the mask. I would however leave the IP address condition there, due to historic reasons (setting an IP to IP4_ANY meant loosing the IP) and to be in line with other parts of the code, e.g. here (called directly from lwip):

if ( !ip4_addr_cmp(ip_2_ip4(&netif->ip_addr), IP4_ADDR_ANY4) ) {

I'll replace this line:

if (!(ip4_addr_isany_val(ip_info->ip) || ip4_addr_isany_val(ip_info->netmask) || ip4_addr_isany_val(ip_info->gw))) {

with checking ip4_addr_isany_val(ip_info->ip) only. Please keep this open, we'll close it after it's fixed in IDF.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Mar 19, 2024
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress labels Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

3 participants