This is an automated email from the ASF dual-hosted git repository.
xiaoxiang781216 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new bc669f9b139 drivers/usbdev/cdcncm: fix TX corruption/wedge under write
buffers
bc669f9b139 is described below
commit bc669f9b1395a6ea708a152092323720fe91e904
Author: Ricard Rosson <[email protected]>
AuthorDate: Sat Jul 18 13:42:12 2026 +0100
drivers/usbdev/cdcncm: fix TX corruption/wedge under write buffers
Two related defects corrupt CDC-NCM transmit once TCP write buffers make TX
bursty (a single txavail poll drains many queued segments back-to-back
through
cdcncm_send):
1. Buffer-reuse race. cdcncm coalesces datagrams into the single
pre-allocated
wrreq->buf that the USB controller transmits directly from, but
cdcncm_send
formatted a new NTB batch into it (cdcncm_transmit_format) without first
waiting for the previous transfer to complete -- the wrreq_idle wait
happened
only later, in cdcncm_transmit_work. A new batch started while the
previous
NTB was still in flight overwrote the in-flight buffer, so the host
dropped
the corrupted NTB and TX could wedge (wrreq_idle never reposted).
Fix: acquire wrreq_idle in cdcncm_send when starting a new batch
(dgramcount == 0), before formatting; drop the now-redundant wait in
cdcncm_transmit_work (a second wait on the init-to-1 semaphore would
deadlock).
2. Concurrent transmit_work. cdcncm_send runs under the recursive
netdev_lock and
calls cdcncm_transmit_work() synchronously in the buffer-full branch,
while a
scheduled delaywork instance runs cdcncm_transmit_work() on ETHWORK --
two
different threads. Two EP_SUBMITs of the one wrreq corrupt the IN request
queue and leave the IN buffer prepared-but-unarmed (controller idle,
wrreq_idle never reposted).
Fix: wrap cdcncm_transmit_work in netdev_lock (the synchronous caller
already
holds this recursive nxrmutex; a delaywork instance blocks until the
drain
releases it), and add an empty-batch guard (dgramcount == 0 -> return)
so a
delaywork that runs after a synchronous flush emptied the batch does not
seal
an empty NTB and double-submit the in-flight wrreq.
Validated on RP2350 (Pico 2 W) with CONFIG_NET_TCP_WRITE_BUFFERS=y as part
of the
complete fix set: 144 dense/concurrent HTTP downloads, zero wedges, ~486
KB/s
(previously transmit hung within a few requests). On RP2350 full stability
under
maximal TX density additionally requires a memory barrier between the
BUFF_STATUS
clear and the AVAILABLE re-arm in the Cortex-M33 USB device driver (a
separate
change); these cdcncm defects are real and the fixes correct independent of
it.
Signed-off-by: Ricard Rosson <[email protected]>
Assisted-by: Claude (Anthropic Claude Code)
Signed-off-by: Ricard Rosson <[email protected]>
---
drivers/usbdev/cdcncm.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/drivers/usbdev/cdcncm.c b/drivers/usbdev/cdcncm.c
index 5407c8ff72f..d9fa738c600 100644
--- a/drivers/usbdev/cdcncm.c
+++ b/drivers/usbdev/cdcncm.c
@@ -918,14 +918,26 @@ static void cdcncm_transmit_work(FAR void *arg)
int ndpindex;
int totallen;
- /* Wait until the USB device request for Ethernet frame transmissions
- * becomes available.
+ /* Serialise against cdcncm_send() and any other transmit_work: they share
+ * the single wrreq buffer and run under the recursive netdev_lock.
+ * Without it, delay-0 scheduling can submit the same wrreq twice and
+ * wedge TX.
*/
- while (nxsem_wait(&self->wrreq_idle) != OK)
+ netdev_lock(&self->dev.netdev);
+
+ /* Empty batch: a previous flush already submitted it. Don't resubmit. */
+
+ if (self->dgramcount == 0)
{
+ netdev_unlock(&self->dev.netdev);
+ return;
}
+ /* cdcncm_send() already holds the wrreq_idle token for this batch, so we
+ * must not wait for it again here (wrcomplete reposts it after EP_SUBMIT).
+ */
+
ncblen = opts->nthsize;
ndpindex = NCM_ALIGN(ncblen, ndpalign);
@@ -950,6 +962,8 @@ static void cdcncm_transmit_work(FAR void *arg)
self->wrreq->len = totallen;
EP_SUBMIT(self->epbulkin, self->wrreq);
+
+ netdev_unlock(&self->dev.netdev);
}
/****************************************************************************
@@ -1291,6 +1305,21 @@ static int cdcncm_send(FAR struct netdev_lowerhalf_s
*dev, FAR netpkt_t *pkt)
FAR struct cdcncm_driver_s *self;
self = container_of(dev, struct cdcncm_driver_s, dev);
+
+ /* At the start of a new NTB batch, wait for the previous transfer to
+ * finish before reusing wrreq->buf (the USB controller transmits straight
+ * out of it). With TCP write buffers, cdcncm_send() drains many segments
+ * back-to-back, so batches overlap; waiting only just before EP_SUBMIT let
+ * the in-flight buffer be overwritten and wedged TX.
+ */
+
+ if (self->dgramcount == 0)
+ {
+ while (nxsem_wait(&self->wrreq_idle) != OK)
+ {
+ }
+ }
+
cdcncm_transmit_format(self, pkt);
netpkt_free(dev, pkt, NETPKT_TX);
