RE: [PATCH v2] wifi: rtw89: usb: fix TX flow control by tracking in-flight URBs

[Ping-Ke Shih <pkshih@realtek.com>]

Lucid Duck <lucid_duck@justthetip.ca> wrote:
> Hi Ping-Ke,
> 
> Thank you for the review. Answers to each point below.
> 
> Mailer: I use a direct SMTP script -- git send-email had a hang
> issue on Fedora 43 that has since been resolved. v3 is sent via
> git send-email. The patch applies cleanly with git am.
> 
> Uplink data (USB3 5GHz, 10 runs each, DWA-X1850 RTL8832AU,
> kernel 6.19.8):
> 
>                      Unpatched    Patched-32    Patched-64
>   UL avg:            844 Mbps     763 Mbps      840 Mbps
>   UL retransmits:    3            0             0

If you have RTL8832CU, can you have additional test on 160 MHz bandwidth?

> 
> 32 URBs shows a ~10% upload regression on USB3 5GHz. 64 URBs
> recovers to stock with zero retransmits.
> 
> > Can increasing 32 get better performance? The stress test with
> > small packets might yield low throughput?
> 
> Yes. I tested 32, 64, and 128 URBs per channel. The difference
> is most visible under parallel streams (USB3 5GHz upload):
> 
>                      Stock    32 URBs    64 URBs    128 URBs
>   4-stream:          858      556        837        849
>   8-stream:          872      565        830        833

Not sure if people want 128 URBs to have a few better performance.
For me, if there is no objection or restriction, I'd take 128
because this driver can support RTL8832CU working on 160MHz
bandwidth, which maximum throughput is twice of RTL8852AU.

> 
> 32 URBs drops 35% under multi-stream load. 64 URBs recovers
> fully. 128 URBs shows no further gain -- 64 is the sweet spot
> for USB3.
> 
> On USB2, URB count does not matter -- the bus is the bottleneck:
> 
>                      Stock    32 URBs    64 URBs    128 URBs
>   UL avg (USB2):     250      252        248        253
> 
> Small packets (USB3 5GHz upload, 3 runs each avg Mbps):
> 
>                      Stock    32 URBs    64 URBs
>   64 bytes:          139      128        126
>   256 bytes:         441      444        442
>   1024 bytes:        845      786        846
> 
> Small packets are CPU/USB-framing limited, not URB-count limited.
> No throughput difference at 64 or 256 bytes. At 1024 bytes, 32
> URBs constrains throughput; 64 recovers.
> 
> > Out of curiosity. Is it possible inflight >
> > RTW89_USB_MAX_TX_URBS_PER_CH?
> 
> No. check_and_reclaim is called before tx_kick_off, and each call
> submits at most one URB. The >= is defensive only.
> 
> > I think the code self-explain this already.
> > I'd not prefer this comment neither.
> 
> Both comments removed in v3.

Thanks. Can you additionally remove the comments of CH12, which
looks like not preferred AI-like comments? I will note this in v3.

> 
> v3 follows with MAX_TX_URBS_PER_CH increased from 32 to 64.
> 
> Additional validation at 64 URBs: UDP flood (0% loss across 4M
> datagrams at 930 Mbps), bidirectional (zero retransmits), and
> 60-second soak (844 Mbps sustained, zero degradation).
> 
> Full test data:
> https://github.com/Lucid-Duck/tx-resources-flow-control/blob/main/test-resul
> ts/2026-03-23-ping-ke-v2-review.md

Thanks for your experiments in detail. :)

Ping-Ke