RE: [PATCH rtw-next v2 7/7] wifi: rtw89: process TX wait skbs for USB via C2H handler

[Zong-Zhe Yang <kevin_yang@realtek.com>]

Fedor Pchelkin <pchelkin@ispras.ru> wrote:
> 
> TX wait skbs need to be completed when they are done.  PCIe part does this inside
> rtw89_pci_tx_status() during RPP processing.  Other HCIs use a mechanism based on C2H
> firmware messages.
> 
> Store TX wait skbs inside TX report queue so that it'll be possible to identify completed items
> inside C2H handler via private driver data of skb.
> 
> Found by Linux Verification Center (linuxtesting.org).
> 
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> ---
> 
> v2: store TX wait skbs in tx_rpt_queue (Ping-Ke)
> 
>  drivers/net/wireless/realtek/rtw89/core.c |  6 ++++--
> drivers/net/wireless/realtek/rtw89/core.h | 15 +++++++++++++++
> drivers/net/wireless/realtek/rtw89/mac.c  |  3 ++-
> drivers/net/wireless/realtek/rtw89/mac.h  | 15 +++++++++++++--
> drivers/net/wireless/realtek/rtw89/usb.c  |  3 ++-
>  5 files changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/core.c
> b/drivers/net/wireless/realtek/rtw89/core.c
> index 214924f8bee0..1457a5fe7320 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.c
> +++ b/drivers/net/wireless/realtek/rtw89/core.c
> @@ -1108,7 +1108,7 @@ rtw89_core_tx_update_desc_info(struct rtw89_dev *rtwdev,
>                 if (addr_cam->valid && desc_info->mlo)
>                         upd_wlan_hdr = true;
> 
> -               if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)
> +               if (tx_req->wait || (info->flags &
> + IEEE80211_TX_CTL_REQ_TX_STATUS))
>                         rtw89_tx_rpt_enable(rtwdev, tx_req);
>         }
>         is_bmc = (is_broadcast_ether_addr(hdr->addr1) || @@ -1173,7 +1173,8 @@ int
> rtw89_core_tx_kick_off_and_wait(struct rtw89_dev *rtwdev, struct sk_buff *sk
> 
>         if (time_left == 0) {
>                 ret = -ETIMEDOUT;
> -               list_add_tail(&wait->list, &rtwdev->tx_waits);
> +               if (!rtwdev->hci.tx_rpt_enable)
> +                       list_add_tail(&wait->list, &rtwdev->tx_waits);
>                 wiphy_delayed_work_queue(rtwdev->hw->wiphy,
> &rtwdev->tx_wait_work,
>                                          RTW89_TX_WAIT_WORK_TIMEOUT);
>         } else {
> @@ -1242,6 +1243,7 @@ static int rtw89_core_tx_write_link(struct rtw89_dev *rtwdev,
>         tx_req.skb = skb;
>         tx_req.vif = vif;
>         tx_req.sta = sta;
> +       tx_req.wait = wait;
>         tx_req.rtwvif_link = rtwvif_link;
>         tx_req.rtwsta_link = rtwsta_link;
>         tx_req.desc_info.sw_mld = sw_mld; diff --git
> a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
> index 3940e54353d3..c13465e2730a 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.h
> +++ b/drivers/net/wireless/realtek/rtw89/core.h
> @@ -1201,6 +1201,7 @@ struct rtw89_core_tx_request {
>         struct sk_buff *skb;
>         struct ieee80211_vif *vif;
>         struct ieee80211_sta *sta;
> +       struct rtw89_tx_wait_info *wait;
>         struct rtw89_vif_link *rtwvif_link;
>         struct rtw89_sta_link *rtwsta_link;
>         struct rtw89_tx_desc_info desc_info; @@ -7387,6 +7388,20 @@ static inline struct
> sk_buff *rtw89_alloc_skb_for_rx(struct rtw89_dev *rtwdev,
>         return dev_alloc_skb(length);
>  }
> 
> +static inline bool rtw89_core_is_tx_wait(struct rtw89_dev *rtwdev,
> +                                        struct rtw89_tx_skb_data
> +*skb_data) {
> +       struct rtw89_tx_wait_info *wait;
> +
> +       guard(rcu)();
> +
> +       wait = rcu_dereference(skb_data->wait);
> +       if (!wait)
> +               return false;
> +
> +       return true;
> +}
> +

Look like using rcu_access_pointer() is enough for here.

>  static inline bool rtw89_core_tx_wait_complete(struct rtw89_dev *rtwdev,
>                                                struct rtw89_tx_skb_data
> *skb_data,
>                                                u8 tx_status) diff --git
> a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
> index 75d9efac452b..3406c8b01eb8 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac.c
> +++ b/drivers/net/wireless/realtek/rtw89/mac.c
> @@ -5484,7 +5484,8 @@ rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff
> *c2h, u32 len)
>                         continue;
> 
>                 __skb_unlink(cur, &rtwdev->tx_rpt_queue);
> -               rtw89_tx_rpt_tx_status(rtwdev, cur, tx_status);
> +               if (!rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status))
> +                       rtw89_tx_rpt_tx_status(rtwdev, cur, tx_status);
>                 break;
>         }
>         spin_unlock_irqrestore(&rtwdev->tx_rpt_queue.lock, flags); diff --git
> a/drivers/net/wireless/realtek/rtw89/mac.h b/drivers/net/wireless/realtek/rtw89/mac.h
> index 1f7d3734d15f..2d647d3b0852 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac.h
> +++ b/drivers/net/wireless/realtek/rtw89/mac.h
> @@ -1647,16 +1647,27 @@ void rtw89_tx_rpt_tx_status(struct rtw89_dev *rtwdev, struct
> sk_buff *skb, u8 tx  static inline  void rtw89_tx_rpt_queue_purge(struct rtw89_dev *rtwdev)
> {
> +       struct rtw89_tx_skb_data *skb_data;
> +       struct rtw89_tx_wait_info *wait;
>         struct sk_buff_head q;
>         struct sk_buff *skb;
>         unsigned long flags;
> 
> +       lockdep_assert_wiphy(rtwdev->hw->wiphy);
> +
>         __skb_queue_head_init(&q);
>         spin_lock_irqsave(&rtwdev->tx_rpt_queue.lock, flags);
>         skb_queue_splice_init(&rtwdev->tx_rpt_queue, &q);
>         spin_unlock_irqrestore(&rtwdev->tx_rpt_queue.lock, flags);
> 
> -       while ((skb = __skb_dequeue(&q)))
> -               rtw89_tx_rpt_tx_status(rtwdev, skb, RTW89_TX_MACID_DROP);
> +       while ((skb = __skb_dequeue(&q))) {
> +               skb_data = RTW89_TX_SKB_CB(skb);
> +               wait = wiphy_dereference(rtwdev->hw->wiphy,
> + skb_data->wait);
> +
> +               if (wait)
> +                       rtw89_tx_wait_release(wait);
> +               else
> +                       rtw89_tx_rpt_tx_status(rtwdev, skb, RTW89_TX_MACID_DROP);
> +       }
>  }
>  #endif
> diff --git a/drivers/net/wireless/realtek/rtw89/usb.c
> b/drivers/net/wireless/realtek/rtw89/usb.c
> index f53ab676e9a8..adbadb2783f0 100644
> --- a/drivers/net/wireless/realtek/rtw89/usb.c
> +++ b/drivers/net/wireless/realtek/rtw89/usb.c
> @@ -216,7 +216,8 @@ static void rtw89_usb_write_port_complete(struct urb *urb)
>                 skb_pull(skb, txdesc_size);
> 
>                 info = IEEE80211_SKB_CB(skb);
> -               if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) {
> +               if (rtw89_core_is_tx_wait(rtwdev, RTW89_TX_SKB_CB(skb)) ||
> +                   (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)) {
>                         /* sn is passed to rtw89_mac_c2h_tx_rpt() via driver data */
>                         skb_queue_tail(&rtwdev->tx_rpt_queue, skb);
>                         wiphy_delayed_work_queue(rtwdev->hw->wiphy,
> --
> 2.51.0
>