ncm: retry link-state notification, fix carrier lost on collision

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;

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;