Skip to content

drivers/usbdev/cdcncm: send TX immediately instead of after a tick-quantized delay - #19469

Merged
jerpelea merged 1 commit into
apache:masterfrom
ricardgb:cdcncm-tx-latency
Jul 28, 2026
Merged

drivers/usbdev/cdcncm: send TX immediately instead of after a tick-quantized delay#19469
jerpelea merged 1 commit into
apache:masterfrom
ricardgb:cdcncm-tx-latency

Conversation

@ricardgb

Copy link
Copy Markdown
Contributor

Summary

cdcncm_send() defers each transmit to the work queue with a coalescing delay of
MSEC2TICK(CDCNCM_DGRAM_COMBINE_PERIOD), where CDCNCM_DGRAM_COMBINE_PERIOD is 1 (ms).
MSEC2TICK() quantizes to the system tick and rounds up, so on the default 100 Hz
tick (CONFIG_USEC_PER_TICK=10000) a "1 ms" delay becomes a full 10 ms tick — 10–20 ms
of real dead time once tick phase is included. As a result the reply to every
single-datagram exchange (ICMP echo, TCP ACK, a one-MSS HTTP segment) is parked in the
work queue for ~10–20 ms before EP_SUBMIT, which dominates the CDC-NCM round-trip time.

Because the coalescing window only ever batches datagrams that the network stack appends
within the same synchronous TX burst (which are already queued before the worker runs),
a non-zero inter-burst delay adds latency without providing any batching benefit in the
common one-datagram-at-a-time case. This change fires the transmit worker immediately
(delay 0); within-burst coalescing is preserved.

Impact

Measured on a Raspberry Pi Pico 2 W (RP2350) acting as a USB-NIC (CDC-NCM), default
100 Hz tick:

Before After
Ping RTT (avg) 21.7 ms 2.8 ms
HTTP GET of a 257 KB gzip page 5.79 s 0.92 s
Throughput 44.5 KB/s 279 KB/s

The RTT drop is the direct effect; the throughput gain follows because, without TCP write
buffers, the stack sends one segment per round trip, so throughput is ~MSS / RTT.

Testing

Validated on hardware (RP2350 / Pico 2 W, CDC-NCM over USB FS to a Linux host): ping RTT
and HTTP download measured before/after as above; served content verified byte-identical
(sha256) after the change. Not yet exercised on other platforms; the reasoning is
tick-rate-general (any board at the default 100 Hz tick is affected).


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 measurements were
reviewed by a human maintainer before submission.

@ricardgb
ricardgb requested a review from Donny9 as a code owner July 18, 2026 12:44
@github-actions github-actions Bot added Size: S The size of the change in this PR is small Area: USB labels Jul 18, 2026
@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

@ricardgb
ricardgb force-pushed the cdcncm-tx-latency branch from d18c44e to d1e7f27 Compare July 18, 2026 13:10
@linguini1

Copy link
Copy Markdown
Contributor

Please include the test logs.

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

@ricardgb please fix the ci error:

d1e7f2796b drivers/usbdev/cdcncm: send TX immediately instead of after a tick-quantized delay
../nuttx/tools/checkpatch.sh -c -u -m -g  5b0dd20f3adc39ba1e73f3cb217b608c0168c524..HEAD
❌ Commit subject too long > 80
Used config files:
    1: .codespellrc

@ricardgb
ricardgb force-pushed the cdcncm-tx-latency branch from d1e7f27 to 060ac41 Compare July 23, 2026 15:13
@ricardgb

Copy link
Copy Markdown
Contributor Author

@xiaoxiang781216 fixed — the check failure was the commit subject exceeding 80 chars; reworded to "drivers/usbdev/cdcncm: send TX immediately, not after a tick-quantized delay" (77 chars) and rebased onto the latest master while at it. checkpatch passes locally. Test logs for @linguini1 to follow in a separate comment.

@ricardgb

Copy link
Copy Markdown
Contributor Author

@linguini1 on-hardware before/after from a Raspberry Pi Pico 2 W (RP2350), NuttX 13.0.0-RC0, USB CDC-NCM link (board eth0 192.168.7.1 ⇆ host usb0 192.168.7.2), system tick 100 Hz. Same board, same config; only cdcncm_send()'s else-branch differs.

  • BEFORE = work_queue(..., MSEC2TICK(CDCNCM_DGRAM_COMBINE_PERIOD)) (1 ms → rounds up to a full 10 ms tick)
  • AFTER = work_queue(..., 0) (this PR)

BEFORE — host → board, 20 pings (host µs clock):

20 packets transmitted, 20 received, 0% packet loss
rtt min/avg/max/mdev = 12.668/16.858/21.662/3.009 ms

BEFORE — board → host, 10 pings (board 10 ms tick clock):

rtt min/avg/max/mdev = 20.000/20.000/20.000/0.000 ms   # flat 20 ms = 2 ticks

AFTER — host → board, 30 pings:

30 packets transmitted, 30 received, 0% packet loss
rtt min/avg/max/mdev = 2.169/2.282/2.405/0.077 ms

AFTER — board → host, 10 pings:

icmp_seq=0 time=10.0 ms
icmp_seq=1..9 time=0.0 ms                              # sub-tick
rtt min/avg/max/mdev = 0.000/1.000/10.000/3.000 ms

Round-trip drops from 16.9 ms → 2.28 ms (host clock); the board-clock figure going from a flat 20 ms to sub-tick shows the tick-quantization the patch removes. Consistent with the 21.7 → 2.8 ms in the commit message.

Disclosure: these tests were run and captured with the help of an AI agent (Claude Code, Anthropic); results are from real hardware and human-reviewed.

Comment thread drivers/usbdev/cdcncm.c Outdated
…d delay

cdcncm_send() defers each transmit with MSEC2TICK(CDCNCM_DGRAM_COMBINE_PERIOD)
(1 ms). MSEC2TICK() rounds up to the system tick, so at the default 100 Hz tick
the "1 ms" coalescing window becomes a full 10 ms tick (10-20 ms with phase),
adding that latency to every single-datagram reply (ICMP echo, TCP ACK, one-MSS
HTTP segment) and dominating the CDC-NCM round-trip time.

The window only coalesces datagrams appended within the same synchronous TX
burst (already queued before the worker runs), so an inter-burst delay adds
latency without batching benefit in the common case. Fire the transmit worker
immediately (delay 0); within-burst coalescing is preserved.

On RP2350 (Pico 2 W) USB-NIC at 100 Hz tick: ping RTT 21.7 -> 2.8 ms, a 257 KB
HTTP download 5.79 -> 0.92 s (44.5 -> 279 KB/s).

Signed-off-by: Ricard Rosson <ricard@groundbits.com>
Assisted-by: Claude (Anthropic Claude Code)
Signed-off-by: Ricard Rosson <ricard@groundbits.com>
@ricardgb
ricardgb force-pushed the cdcncm-tx-latency branch from 060ac41 to 165dd88 Compare July 27, 2026 11:04
@github-actions github-actions Bot added Size: XS The size of the change in this PR is very small and removed Size: S The size of the change in this PR is small labels Jul 27, 2026
@jerpelea
jerpelea merged commit 6555e3e into apache:master Jul 28, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: USB Size: XS The size of the change in this PR is very small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rp2040/rp23xx USB: missing DMB between BUFF_STATUS clear and AVAILABLE re-arm loses IN completions on RP2350 (Cortex-M33)

5 participants