On 29/08/2025 00:11, Fedor Pchelkin wrote:
> There is no completion signaling for tx_wait skbs on USB part. This means
> rtw89_core_tx_kick_off_and_wait() always returns with a timeout.
>
> Moreover, recent rework of tx_wait objects lifecycle handling made the
> driver be responsible for freeing the associated skbs, not the core
> ieee80211 stack. Lack of completion signaling would cause those objects
> being kept in driver internal tx_waits queue until rtw89_hci_reset()
> occurs, and then a double free would happen.
>
> Extract TX status handling into a separate function, like its
> rtw89_pci_tx_status() counterpart. Signal completion from there.
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Fixes: 2135c28be6a8 ("wifi: rtw89: Add usb.{c,h}")
> Signed-off-by: Fedor Pchelkin <pchelkin@xxxxxxxxx>
> ---
>
> New series iteration -> new nuances found.
>
> It seems the two previous patches from the series would not be too great
> in USB case because there is no completion signaling for tx_wait skbs
> there.
>
> I don't have this hardware so *the patch is compile tested only*. It'd
> be nice if someone gave it a run on top of two previous patches of the
> series, thanks!
>
I tested your first three patches with RTL8851BU for a few hours. It's
looking good, no explosion yet.
The USB side doesn't have real TX ACK status reporting yet. I only
learned recently how to do that. It looks like it will work about the
same as in rtw88.
> drivers/net/wireless/realtek/rtw89/usb.c | 36 +++++++++++++++---------
> 1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c
> index 6cf89aee252e..10fe19bd5166 100644
> --- a/drivers/net/wireless/realtek/rtw89/usb.c
> +++ b/drivers/net/wireless/realtek/rtw89/usb.c
> @@ -188,11 +188,32 @@ static u8 rtw89_usb_get_bulkout_id(u8 ch_dma)
> }
> }
>
> +static void rtw89_usb_tx_status(struct rtw89_dev *rtwdev, struct sk_buff *skb,
> + int status)
> +{
> + struct rtw89_tx_skb_data *skb_data = RTW89_TX_SKB_CB(skb);
> + struct ieee80211_tx_info *info;
> +
> + if (rtw89_core_tx_wait_complete(rtwdev, skb_data, status == 0))
> + return;
> +
> + info = IEEE80211_SKB_CB(skb);
> + ieee80211_tx_info_clear_status(info);
> +
> + if (status == 0) {
> + if (info->flags & IEEE80211_TX_CTL_NO_ACK)
> + info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
> + else
> + info->flags |= IEEE80211_TX_STAT_ACK;
> + }
> +
> + ieee80211_tx_status_irqsafe(rtwdev->hw, skb);
> +}
> +
> 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 ieee80211_tx_info *info;
> struct rtw89_txwd_body *txdesc;
> struct sk_buff *skb;
> u32 txdesc_size;
> @@ -214,18 +235,7 @@ static void rtw89_usb_write_port_complete(struct urb *urb)
> txdesc_size += rtwdev->chip->txwd_info_size;
>
> skb_pull(skb, txdesc_size);
> -
> - info = IEEE80211_SKB_CB(skb);
> - ieee80211_tx_info_clear_status(info);
> -
> - if (urb->status == 0) {
> - if (info->flags & IEEE80211_TX_CTL_NO_ACK)
> - info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
> - else
> - info->flags |= IEEE80211_TX_STAT_ACK;
> - }
> -
> - ieee80211_tx_status_irqsafe(rtwdev->hw, skb);
> + rtw89_usb_tx_status(rtwdev, skb, urb->status);
> }
>
> switch (urb->status) {
