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

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 26/01/2026 00:19, Lucid Duck wrote:
> rtw89_usb_ops_check_and_reclaim_tx_resource() currently returns a
> hardcoded placeholder value of 42, violating mac80211's TX flow control
> contract. This causes uncontrolled URB accumulation under sustained TX
> load since mac80211 believes resources are always available.
> 
> Fix this by implementing proper TX backpressure:
> 
> - Add per-channel atomic counters (tx_inflight[]) to track URBs between
>   submission and completion
> - Increment counter before usb_submit_urb() with rollback on failure
> - Decrement counter in completion callback
> - Return available slots (max - inflight) to mac80211, or 0 at capacity
> - Exclude firmware command channel (CH12) from flow control
> 
> Tested on D-Link DWA-X1850 (RTL8832AU) with:
> - Sustained high-throughput traffic
> - Module load/unload stress tests
> - Hot-unplug during active transmission
> - 30-minute soak test verifying counters balance at idle
> 
> Signed-off-by: Lucid Duck <lucid_duck@justthetip.ca>
> ---
>  drivers/net/wireless/realtek/rtw89/usb.c | 27 ++++++++++++++++++++++--
>  drivers/net/wireless/realtek/rtw89/usb.h |  6 ++++++
>  2 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c
> index e77561a4d..6fcf32603 100644
> --- a/drivers/net/wireless/realtek/rtw89/usb.c
> +++ b/drivers/net/wireless/realtek/rtw89/usb.c
> @@ -161,16 +161,25 @@ static u32
>  rtw89_usb_ops_check_and_reclaim_tx_resource(struct rtw89_dev *rtwdev,
>  					    u8 txch)
>  {
> +	struct rtw89_usb *rtwusb = rtw89_usb_priv(rtwdev);
> +	int inflight;
> +
> +	/* Firmware command channel is not flow-controlled */
>  	if (txch == RTW89_TXCH_CH12)
>  		return 1;
>  
> -	return 42; /* TODO some kind of calculation? */
> +	inflight = atomic_read(&rtwusb->tx_inflight[txch]);
> +	if (inflight >= RTW89_USB_MAX_TX_URBS_PER_CH)
> +		return 0;
> +
> +	return RTW89_USB_MAX_TX_URBS_PER_CH - inflight;
>  }
>  
>  static void rtw89_usb_write_port_complete(struct urb *urb)
>  {
>  	struct rtw89_usb_tx_ctrl_block *txcb = urb->context;
>  	struct rtw89_dev *rtwdev = txcb->rtwdev;
> +	struct rtw89_usb *rtwusb = rtw89_usb_priv(rtwdev);
>  	struct ieee80211_tx_info *info;
>  	struct rtw89_txwd_body *txdesc;
>  	struct sk_buff *skb;
> @@ -229,6 +238,10 @@ static void rtw89_usb_write_port_complete(struct urb *urb)
>  		break;
>  	}
>  
> +	/* Decrement in-flight counter (skip firmware command channel) */
> +	if (txcb->txch != RTW89_TXCH_CH12)

You don't need to add these checks because there is one in
rtw89_usb_ops_check_and_reclaim_tx_resource().

> +		atomic_dec(&rtwusb->tx_inflight[txcb->txch]);
> +
>  	kfree(txcb);
>  }
>  
> @@ -306,9 +319,17 @@ static void rtw89_usb_ops_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch)
>  
>  		skb_queue_tail(&txcb->tx_ack_queue, skb);
>  
> +		/* Increment BEFORE submit to avoid race with completion */
> +		if (txch != RTW89_TXCH_CH12)
> +			atomic_inc(&rtwusb->tx_inflight[txch]);
> +
>  		ret = rtw89_usb_write_port(rtwdev, txch, skb->data, skb->len,
>  					   txcb);
>  		if (ret) {
> +			/* Rollback increment on failure */
> +			if (txch != RTW89_TXCH_CH12)
> +				atomic_dec(&rtwusb->tx_inflight[txch]);
> +
>  			if (ret != -ENODEV)
>  				rtw89_err(rtwdev, "write port txch %d failed: %d\n",
>  					  txch, ret);
> @@ -666,8 +687,10 @@ static void rtw89_usb_init_tx(struct rtw89_dev *rtwdev)
>  	struct rtw89_usb *rtwusb = rtw89_usb_priv(rtwdev);
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(rtwusb->tx_queue); i++)
> +	for (i = 0; i < ARRAY_SIZE(rtwusb->tx_queue); i++) {
>  		skb_queue_head_init(&rtwusb->tx_queue[i]);
> +		atomic_set(&rtwusb->tx_inflight[i], 0);
> +	}
>  }
>  
>  static void rtw89_usb_deinit_tx(struct rtw89_dev *rtwdev)
> diff --git a/drivers/net/wireless/realtek/rtw89/usb.h b/drivers/net/wireless/realtek/rtw89/usb.h
> index 203ec8e99..f72a8b1b2 100644
> --- a/drivers/net/wireless/realtek/rtw89/usb.h
> +++ b/drivers/net/wireless/realtek/rtw89/usb.h
> @@ -20,6 +20,9 @@
>  #define RTW89_MAX_ENDPOINT_NUM		9
>  #define RTW89_MAX_BULKOUT_NUM		7
>  
> +/* TX flow control: max in-flight URBs per channel */
> +#define RTW89_USB_MAX_TX_URBS_PER_CH	32
> +
>  struct rtw89_usb_info {
>  	u32 usb_host_request_2;
>  	u32 usb_wlan0_1;
> @@ -63,6 +66,9 @@ struct rtw89_usb {
>  	struct usb_anchor tx_submitted;
>  
>  	struct sk_buff_head tx_queue[RTW89_TXCH_NUM];
> +
> +	/* TX flow control: track in-flight URBs per channel */
> +	atomic_t tx_inflight[RTW89_TXCH_NUM];

Is there a reason to add a new counter instead of just using
the length of each tx_queue?

>  };
>  
>  static inline struct rtw89_usb *rtw89_usb_priv(struct rtw89_dev *rtwdev)