mirror/tinyusb
3
0
Fork 0
mirror of https://github.com/hathach/tinyusb.git synced 2026-08-18 02:53:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

ncm: fix carrier lost on link-state notify collision

Browse Source 
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
This commit is contained in:
Cedric Van den Bergh
2026-07-08 12:47:03 +01:00
parent fa750d6bf0
commit cf055c237a
1 changed files with 36 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
src/class/net/ncm_device.c
View File

@ -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)