Re: [PATCH] wifi: rtw88: usb: Fix disconnection after beacon loss

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 30/04/2024 03:50, Ping-Ke Shih wrote:
> +Cc: Martin for rtw88 SDIO that seems have the same problem 
> 
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> When there is beacon loss, for example due to unrelated Bluetooth
>> devices transmitting music nearby, the wifi connection dies soon
>> after the first beacon loss message:
>>
>> Apr 28 20:47:14 ideapad2 wpa_supplicant[1161]: wlp3s0f3u4:
>>  CTRL-EVENT-BEACON-LOSS
>> Apr 28 20:47:15 ideapad2 wpa_supplicant[1161]: wlp3s0f3u4:
>>  CTRL-EVENT-DISCONNECTED bssid=... reason=4 locally_generated=1
>>
>> Apr 28 20:47:24 ideapad2 wpa_supplicant[1161]: wlp3s0f3u4:
>>  CTRL-EVENT-BEACON-LOSS
>> Apr 28 20:47:25 ideapad2 wpa_supplicant[1161]: wlp3s0f3u4:
>>  CTRL-EVENT-DISCONNECTED bssid=... reason=4 locally_generated=1
>>
>> Apr 28 20:47:34 ideapad2 wpa_supplicant[1161]: wlp3s0f3u4:
>>  CTRL-EVENT-BEACON-LOSS
>> Apr 28 20:47:35 ideapad2 wpa_supplicant[1161]: wlp3s0f3u4:
>>  CTRL-EVENT-DISCONNECTED bssid=... reason=4 locally_generated=1
>>
>> When the beacon loss happens, mac80211 makes rtw88 transmit a QOS
>> NULL frame and asks to confirm the ACK status. Even though rtw88
>> confirms to mac80211 that the QOS NULL was transmitted successfully,
>> the connection still dies. This is because rtw88 is handing the QOS
>> NULL back to mac80211 with skb->data pointing to the headroom (the
>> TX descriptor) instead of ieee80211_hdr.
>>
>> Fix the disconnection by moving skb->data to the correct position
>> before ieee80211_tx_status_irqsafe().
>>
>> The problem was observed with RTL8811AU (TP-Link Archer T2U Nano)
>> and the potential future rtw88_8821au driver. Also tested with
>> RTL8811CU (Tenda U9).
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>>  drivers/net/wireless/realtek/rtw88/usb.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
>> index 3ba7b81c6080..1dfe7c6ae4ba 100644
>> --- a/drivers/net/wireless/realtek/rtw88/usb.c
>> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
>> @@ -278,6 +278,8 @@ static void rtw_usb_write_port_tx_complete(struct urb *urb)
>>                 info = IEEE80211_SKB_CB(skb);
>>                 tx_data = rtw_usb_get_tx_data(skb);
>>
>> +               skb_pull(skb, rtwdev->chip->tx_pkt_desc_sz);
>> +
> 
> There are two cases of arguments of skb_push() for USB: 
> 1. skb_push(skb, chip->tx_pkt_desc_sz);
> 2. skb_push(skb, headsize);
>    headsize = pkt_info->offset ? pkt_info->offset : desclen;
>       pkt_info->offset = chip->tx_pkt_desc_sz;
>       desclen = chip->tx_pkt_desc_sz;
> 
> Eventually all are chip->tx_pkt_desc_sz, but spend a little time to ensure this.
> Could you test and have another patch to change above case (2) to (1)?
> 

I see what you mean. I will make it another patch, because
the skb from case 2 is not sent to mac80211.

>>                 /* enqueue to wait for tx report */
>>                 if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) {
>>                         rtw_tx_report_enqueue(rtwdev, skb, tx_data->sn);
>> --
>> 2.44.0
>