ncm: fix carrier lost on link-state notify collision

tud_network_link_state() delivered the NETWORK_CONNECTION notification
edge-triggered and fire-once: if a previous notification was still in
flight, notification_xmit() returned early and the notification for the
new link state was never queued. Because link_is_up is committed before
the send, the host could be left reporting a stale carrier state - e.g.
a permanent NO-CARRIER after a link up. The notification state was also
mutated from both the caller and the notify xfer-completion callback
with no serialisation, so on RTOS ports where tud_network_link_state()
runs in a task other than tud_task() the two could race.

Defer the whole link-state update onto the usbd task, so it can no
longer race the completion callback. A collision with an in-flight
notification is resolved by re-arming notification_xmit_state and
letting the existing completion callback drive it forward on the next
xfer completion, rather than adding a separate pending/retry flag.

A link toggle does not change the link speed, so strictly only the
NETWORK_CONNECTION notification needs (re)sending, but reusing the
existing speed-then-connection state machine keeps the fix on a single,
already-serialised code path.

Closes #3760

1 files changed, 36 insertions, 11 deletions

diff --git a/src/class/net/ncm_device.c b/src/class/net/ncm_device.c
index 84a524f49..72b592787 100644
--- a/src/class/net/ncm_device.c
+++ b/src/class/net/ncm_device.c
@@ -800,31 +800,56 @@ static void tud_network_recv_renew_r(uint8_t rhport) {
 } // tud_network_recv_renew
 
 /**
- * Set the link state and send notification to host
+ * usbd-task trampoline for tud_network_link_state(), packing rhport and is_up
+ * into a single pointer-sized argument.
+ *
+ * Runs entirely in the usbd task context, so it cannot race the notify
+ * xfer-completion callback over the notification state machine. Re-arming
+ * notification_xmit_state and kicking notification_xmit() (rather than
+ * sending NETWORK_CONNECTION directly) means a state change that collides
+ * with an in-flight notification is picked up by the existing completion
+ * callback instead of being silently dropped - which would otherwise leave
+ * the host stuck at NO-CARRIER after a link-state change.
  */
-void tud_network_link_state(uint8_t rhport, bool is_up) {
-  TU_LOG_DRV("tud_network_link_state(%d, %d)\n", rhport, is_up);
+static void ncm_link_state_task(void *param) {
+  uintptr_t const arg = (uintptr_t) param;
+  uint8_t const rhport = (uint8_t) (arg >> 1);
+  bool const is_up = (arg & 1u) != 0;
 
   if (ncm_interface.link_is_up == is_up) {
-    // No change in link state
-    return;
+    return; // no change in link state
   }
 
   ncm_interface.link_is_up = is_up;
 
-  // Only send notification if we have an active data interface
   if (ncm_interface.itf_data_alt != 1) {
-    TU_LOG_DRV("  link state notification skipped (interface not active)\n");
-    return;
+    TU_LOG_DRV("  link state notification deferred (interface not active)\n");
+    return; // data interface not active yet; SET_INTERFACE(alt=1) will notify
   }
 
-  // Reset notification state to send speed change notification first, then link state notification
+  // A link toggle does not change the link speed, so strictly only the
+  // NETWORK_CONNECTION notification would need (re)sending. Re-running the
+  // speed-then-connection sequence keeps this on the same state machine the
+  // completion callback already drives, at the cost of a redundant speed
+  // notification on every toggle.
   ncm_interface.notification_xmit_state = NOTIFICATION_SPEED;
-
-  // Trigger notification transmission
   notification_xmit(rhport, false);
 }
 
+/**
+ * Set the link state and notify the host.
+ *
+ * Defers onto the usbd task so a caller running in a different task than
+ * tud_task() cannot race the notification state machine against the notify
+ * xfer-completion callback.
+ */
+void tud_network_link_state(uint8_t rhport, bool is_up) {
+  TU_LOG_DRV("tud_network_link_state(%d, %d)\n", rhport, is_up);
+
+  uintptr_t const arg = ((uintptr_t) rhport << 1) | (is_up ? 1u : 0u);
+  usbd_defer_func(ncm_link_state_task, (void *) arg, false);
+}
+
 //-----------------------------------------------------------------------------
 //
 // all the netd_*() stuff (interface TinyUSB -> driver)