ricardgb opened a new pull request, #19470:
URL: https://github.com/apache/nuttx/pull/19470

   ## Summary
   
   `cdcncm` coalesces several outgoing datagrams into the single pre-allocated 
NTB buffer
   `self->wrreq->buf`, which the USB controller transmits directly from. However
   `cdcncm_send()` calls `cdcncm_transmit_format()` to write a new datagram 
into that buffer
   **without first ensuring the previous transfer has completed** — the 
`wrreq_idle` wait was
   only performed later, inside `cdcncm_transmit_work()`, *after* the data had 
already been
   formatted into the buffer.
   
   So when a new NTB batch is started (`self->dgramcount == 0`) while the 
previous NTB is
   still in flight (`EP_SUBMIT`'ed, `cdcncm_wrcomplete` not yet run), the 
in-flight buffer is
   overwritten. The host drops the corrupted NTB, and the transfer can wedge 
permanently
   (`wrreq_idle` is never reposted), stalling **all** subsequent transmit.
   
   This is exercised as soon as transmits become bursty — most notably with TCP 
write buffers
   (`CONFIG_NET_TCP_WRITE_BUFFERS=y`): a single `txavail` poll drains many 
queued segments
   back-to-back through `cdcncm_send()`, so a full NTB is submitted and the 
next batch is
   started while the first is still on the wire. The unbuffered send path 
transmits one
   segment per blocking `send()` and blocks for the ACK, so transfers never 
overlap and the
   race is not hit — which is why the bug is invisible without write buffers.
   
   ## Fix
   
   Acquire the `wrreq_idle` token (wait for the previous transfer to complete) 
in
   `cdcncm_send()` when starting a new NTB batch, *before* 
`cdcncm_transmit_format()` touches
   the buffer. The now-redundant `nxsem_wait(&self->wrreq_idle)` in 
`cdcncm_transmit_work()`
   is removed — ownership is already held by the `cdcncm_send()` that started 
the batch, and a
   second wait on this init-to-1 counting semaphore would deadlock. Semaphore 
accounting stays
   balanced: exactly one acquire per `EP_SUBMIT`, one repost per 
`cdcncm_wrcomplete`.
   
   ## Impact
   
   Without this fix, on a Raspberry Pi Pico 2 W (RP2350) USB-NIC with TCP write 
buffers
   enabled, the TCP handshake completes but no response is ever transmitted — 
even a few-byte
   reply — and the transmit path stays wedged (core idle, no fault). With the 
fix, transmit
   works and write buffers deliver the expected pipelining:
   
   | (write buffers on) | Before | After |
   |---|---|---|
   | HTTP GET of a 257 KB gzip page | hangs (no response) | 0.56 s |
   | Throughput | 0 | 462 KB/s |
   | Small reply (`/pin/presence`) | no response | 200, 40 ms |
   
   Content verified byte-identical (sha256) after the fix.
   
   ## Testing
   
   Validated on hardware (RP2350 / Pico 2 W, CDC-NCM over USB FS to a Linux 
host) with
   `CONFIG_NET_TCP_WRITE_BUFFERS=y`. Not yet exercised on other platforms; the 
race is generic
   to the single-`wrreq` coalescing design and not board-specific.
   
   ---
   
   *Disclosure: this change was prepared with the assistance of an AI agent 
(Anthropic's
   Claude, via Claude Code). The root cause, fix, and on-hardware validation 
were reviewed by
   a human maintainer before submission.*
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to