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

mdns: ethernet link flap doesn't restart mdns if DHCP in use. (IDFGH-12563) #546

Open
3 tasks done
karlp opened this issue Apr 8, 2024 · 2 comments
Open
3 tasks done
Assignees

Comments

@karlp
Copy link

karlp commented Apr 8, 2024

Answers checklist.

  • I have read the documentation for esp-protocols components and the issue is not addressed there.
  • I have updated my esp-protocols 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.

General issue report

When using DHCP on ethernet, If I pull the ethernet cable on an esp32 board, with MDNS advertising enabled[1], I will (correctly) get the event here https://github.com/espressif/esp-protocols/blob/mdns-v1.2.5/components/mdns/mdns.c#L4223-L4225, and the mdns service record will be withdrawn.

However, when I plug the cable back in again, I will (correctly) get an ethernet link up event (ETHERNET_EVENT_CONNECTED) in the mdns code, here: https://github.com/espressif/esp-protocols/blob/mdns-v1.2.5/components/mdns/mdns.c#L4216-L4222 but, because DHCP is not "stopped" the mdns service is never re-enabled.

For my own use cases, simply removing the check on DHCP being stopped "fixes" this issue, and re-stores mdns services when the link is restored. Receiving new DHCP leases still works correctly, as those come in via the IP_EVENT_ETH_GOT_IP event here https://github.com/espressif/esp-protocols/blob/mdns-v1.2.5/components/mdns/mdns.c#L4239-L4241 But that event is not triggered when the link simply flaps.

I'm not aware of any workaround beside restarting my application, or not using DHCP.

While the ethernet interface and the wifi interface both have the same code style here, I believe this is never seen on wifi, as the wifi WIFI_EVENT_STA_CONNECTED event is never seen without getting the IP_EVENT_STA_GOT_IP immediately afterwards. This is not the case on ethernet. The DHCP lease hasn't expired, the link just flapped.

Note, when using a static IP, this link flapping works fine, because DHCP is "stopped"

Solutions?

I'm not sure. This appears to stem from the change in f44c569 However, I'm unconvinced that the solution is correct. IMO both dhcp and static IPs should work, and it's always broken to have this check on enable, but no check on disable?

The original behaviour of only enabling if dhcp was started seems worse, in that it might make dhcp work, but it would prevent static IPs from working at all? I can't track the reasoning for this check any further than ad8c92d in 2017 however. The corresponding commit in esp-idf is espressif/esp-idf@4bddbc0 But there's no guidance there on why this check exists.

Can we just drop these checks? It "works for me" at least :) fixing both DHCP and Static IP configurations :)

[1] As per the examples... ie, something like this... with no specific interface handling, the default predefined ethernet interface is sufficient.

void app_start_mdns(void) {
	char *hostname = "hohostname";
	ESP_ERROR_CHECK(mdns_init());
	ESP_ERROR_CHECK(mdns_hostname_set(hostname));

	// structure with TXT records
	mdns_txt_item_t serviceTxtData[] = {
		{"vendor", "whatever"},
		{"model", "fun-model"},
	};

	ESP_ERROR_CHECK(mdns_service_add(NULL, "_http", "_tcp", 80, NULL, 0));
	ESP_ERROR_CHECK(mdns_service_add(NULL, "_whatever", "_tcp", 2222, serviceTxtData, ARRAY_SIZE(serviceTxtData)));
}
@github-actions github-actions bot changed the title mdns: ethernet link flap doesn't restart mdns if DHCP in use. mdns: ethernet link flap doesn't restart mdns if DHCP in use. (IDFGH-12563) Apr 8, 2024
@karlp
Copy link
Author

karlp commented Apr 8, 2024

To clarify, the follow patch "fixes" it, but a) this makes ethernet and wifi inconsistent and b) fairly obviously, ipv6 probably still has an issue here... (unrelated, but... obvious now)

diff --git a/components/mdns/mdns.c b/components/mdns/mdns.c
index 5dcc40b..948e259 100644
--- a/components/mdns/mdns.c
+++ b/components/mdns/mdns.c
@@ -4229,11 +4229,7 @@ void mdns_preset_if_handle_system_event(void *arg, esp_event_base_t event_base,
         if (event_base == ETH_EVENT) {
             switch (event_id) {
             case ETHERNET_EVENT_CONNECTED:
-                if (!esp_netif_dhcpc_get_status(esp_netif_from_preset_if(MDNS_IF_ETH), &dcst)) {
-                    if (dcst == ESP_NETIF_DHCP_STOPPED) {
-                        post_mdns_enable_pcb(MDNS_IF_ETH, MDNS_IP_PROTOCOL_V4);
-                    }
-                }
+                post_mdns_enable_pcb(MDNS_IF_ETH, MDNS_IP_PROTOCOL_V4);
                 break;
             case ETHERNET_EVENT_DISCONNECTED:
                 post_mdns_disable_pcb(MDNS_IF_ETH, MDNS_IP_PROTOCOL_V4);

@david-cermak
Copy link
Collaborator

Hi @karlp

You can work this around by registering your own netif and configuring advertising actions per your needs (e.g. on link up event)

Here's how you register a netif

/* Demonstration of adding a custom netif to mdns service, but we're adding the default example one,
* so we must disable all predefined interfaces (PREDEF_NETIF_STA, AP and ETH) first
*/
ESP_ERROR_CHECK(mdns_register_netif(EXAMPLE_INTERFACE));

and set up announcing events (typically in your event handlers):

/* It is not enough to just register the interface, we have to enable is manually.
* This is typically performed in "GOT_IP" event handler, but we call it here directly
* since the `EXAMPLE_INTERFACE` netif is connected already, to keep the example simple.
*/
ESP_ERROR_CHECK(mdns_netif_action(EXAMPLE_INTERFACE, MDNS_EVENT_ENABLE_IP4 | MDNS_EVENT_ENABLE_IP6));
ESP_ERROR_CHECK(mdns_netif_action(EXAMPLE_INTERFACE, MDNS_EVENT_ANNOUNCE_IP4 | MDNS_EVENT_ANNOUNCE_IP6));

and sdkconfig:

CONFIG_MDNS_PREDEF_NETIF_STA=n
CONFIG_MDNS_PREDEF_NETIF_AP=n
CONFIG_MDNS_PREDEF_NETIF_ETH=n
CONFIG_MDNS_ADD_CUSTOM_NETIF=y

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

No branches or pull requests

4 participants