diff --git a/src/class/net/ncm_device.c b/src/class/net/ncm_device.c index 84a524f492..f1865965f9 100644 --- a/src/class/net/ncm_device.c +++ b/src/class/net/ncm_device.c @@ -99,6 +99,7 @@ typedef struct { } notification_xmit_state; // state of notification transmission bool notification_xmit_is_running; // notification is currently transmitted bool link_is_up; // current link state + bool link_notify_pending; // host not yet (re)told the current link state // host-configured transmit limits uint8_t bm_capabilities; @@ -237,6 +238,7 @@ static void notification_xmit(uint8_t rhport, bool force_next) { uint16_t notif_len = sizeof(notify_connected.header) + notify_connected.header.wLength; ncm_epbuf.epnotif = notify_connected; usbd_edpt_xfer(rhport, ncm_interface.ep_notif, (uint8_t *) &ncm_epbuf.epnotif, notif_len, false); + ncm_interface.link_notify_pending = false; // current link state delivered to host ncm_interface.notification_xmit_state = NOTIFICATION_DONE; ncm_interface.notification_xmit_is_running = true; @@ -800,7 +802,48 @@ static void tud_network_recv_renew_r(uint8_t rhport) { } // tud_network_recv_renew /** - * Set the link state and send notification to host + * Flush a pending link-state notification once the notification channel is free. + * + * Runs only in the usbd task context (the deferred link-state setter and the + * notify xfer-completion callback), so all accesses to the notification state + * machine are serialised. It (re)sends a NETWORK_CONNECTION carrying the current + * link state until the host has been told the latest state, so a notification + * that collides with an in-flight one is retried on the next completion instead + * of being silently dropped (which would otherwise leave the host stuck at + * NO-CARRIER after a link-state change). + */ +static void ncm_flush_link_notification(uint8_t rhport) { + if (!ncm_interface.link_notify_pending) { + return; + } + if (ncm_interface.itf_data_alt != 1) { + return; // data interface not active yet; SET_INTERFACE(alt=1) will flush it + } + if (ncm_interface.notification_xmit_is_running) { + return; // channel busy; the in-flight xfer's completion will flush it + } + + // A link toggle does not change the link speed, so only the NETWORK_CONNECTION + // notification needs (re)sending here. + ncm_interface.notification_xmit_state = NOTIFICATION_CONNECTED; + notification_xmit(rhport, true); +} + +/** + * usbd-task trampoline for tud_network_link_state(). + */ +static void ncm_link_notify_task(void *param) { + ncm_flush_link_notification((uint8_t) (uintptr_t) param); +} + +/** + * Set the link state and notify the host. + * + * Records the desired link state as pending and defers the send onto the usbd + * task, so a caller running in a different task than tud_task() cannot race the + * notification state machine against the xfer-completion callback. The pending + * flag guarantees the NETWORK_CONNECTION for the new link state is retried until + * delivered rather than dropped if a previous notification is still in flight. */ void tud_network_link_state(uint8_t rhport, bool is_up) { TU_LOG_DRV("tud_network_link_state(%d, %d)\n", rhport, is_up); @@ -811,18 +854,15 @@ void tud_network_link_state(uint8_t rhport, bool is_up) { } ncm_interface.link_is_up = is_up; + ncm_interface.link_notify_pending = true; - // Only send notification if we have an active data interface + // Only notify if the data interface is active; SET_INTERFACE flushes otherwise. if (ncm_interface.itf_data_alt != 1) { - TU_LOG_DRV(" link state notification skipped (interface not active)\n"); + TU_LOG_DRV(" link state notification deferred (interface not active)\n"); return; } - // Reset notification state to send speed change notification first, then link state notification - ncm_interface.notification_xmit_state = NOTIFICATION_SPEED; - - // Trigger notification transmission - notification_xmit(rhport, false); + usbd_defer_func(ncm_link_notify_task, (void *) (uintptr_t) rhport, false); } //----------------------------------------------------------------------------- @@ -957,6 +997,8 @@ bool netd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_ } else if (ep_addr == ncm_interface.ep_notif) { // next transfer on notification channel notification_xmit(rhport, true); + // Channel just freed: (re)send a deferred/collided link-state notification. + ncm_flush_link_notification(rhport); } return true; diff --git a/test/fuzz/device/net_ncm/fuzz.c b/test/fuzz/device/net_ncm/fuzz.c index a93c144f7e..3052636d8b 100644 --- a/test/fuzz/device/net_ncm/fuzz.c +++ b/test/fuzz/device/net_ncm/fuzz.c @@ -47,6 +47,9 @@ bool usbd_open_edpt_pair(uint8_t rhport, uint8_t const *p_desc, uint8_t ep_count (void) rhport; (void) p_desc; (void) ep_count; (void) xfer_type; (void) ep_out; (void) ep_in; return true; } +void usbd_defer_func(osal_task_func_t func, void *param, bool in_isr) { + (void) func; (void) param; (void) in_isr; +} bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const *request, void *buffer, uint16_t len) { (void) rhport; (void) request; (void) buffer; (void) len; return true;