RE: rtw89: Did a8188bb7644f change the behaviour for MLO?

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

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> a8188bb7644f ("wifi: mac80211: change public RX API to use link stations")
> does this:
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
> index 397ebbfcac09af..454d876aeb8861 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.c
> +++ b/drivers/net/wireless/realtek/rtw89/core.c
> @@ -3073,10 +3073,8 @@ static void rtw89_vif_rx_stats_iter(void *data, u8 *mac,
>         struct rtw89_vif *rtwvif = vif_to_rtwvif(vif);
>         struct rtw89_rx_desc_info *desc_info = iter_data->desc_info;
>         struct sk_buff *skb = iter_data->skb;
> -       struct ieee80211_rx_status *rx_status = IEEE80211_SKB_RXCB(skb);
>         struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>         struct rtw89_rx_phy_ppdu *phy_ppdu = iter_data->phy_ppdu;
> -       bool is_mld = ieee80211_vif_is_mld(vif);
>         struct ieee80211_bss_conf *bss_conf;
>         struct rtw89_vif_link *rtwvif_link;
>         const u8 *bssid = iter_data->bssid;
> @@ -3110,11 +3108,6 @@ static void rtw89_vif_rx_stats_iter(void *data, u8 *mac,
>         if (!ether_addr_equal(target_bssid, bssid))
>                 goto out;
> 
> -       if (is_mld) {
> -               rx_status->link_valid = true;
> -               rx_status->link_id = rtwvif_link->link_id;
> -       }
> -
>         bb = rtw89_get_bb_ctx(rtwdev, rtwvif_link->phy_idx);
>         pkt_stat = &bb->cur_pkt_stat;
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 7bb4618065b672..cba7cdf61ce8a5 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -5389,14 +5389,18 @@ void ieee80211_restart_hw(struct ieee80211_hw *hw);
>   * mixed for a single hardware. Must not run concurrently with
>   * ieee80211_tx_status_skb() or ieee80211_tx_status_ni().
>   *
> + * For data frames, when hardware has done address translation, a link station
> + * has to be provided and the frequency information may be skipped.
> + *
>   * This function must be called with BHs disabled and RCU read lock
>   *
>   * @hw: the hardware this frame came in on
> - * @sta: the station the frame was received from, or %NULL
> + * @link_sta: the link station the data frame was received from, or %NULL
>   * @skb: the buffer to receive, owned by mac80211 after this call
>   * @list: the destination list
>   */
> -void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
> +void ieee80211_rx_list(struct ieee80211_hw *hw,
> +                      struct ieee80211_link_sta *link_sta,
>                        struct sk_buff *skb, struct list_head *list);
> 
>  /**
> @@ -5414,14 +5418,18 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
>   * mixed for a single hardware. Must not run concurrently with
>   * ieee80211_tx_status_skb() or ieee80211_tx_status_ni().
>   *
> + * For data frames, when hardware has done address translation, a link station
> + * has to be provided and the frequency information may be skipped.
> + *
>   * This function must be called with BHs disabled.
>   *
>   * @hw: the hardware this frame came in on
> - * @sta: the station the frame was received from, or %NULL
> + * @link_sta: the link station the data frame was received from, or %NULL
>   * @skb: the buffer to receive, owned by mac80211 after this call
>   * @napi: the NAPI context
>   */
> -void ieee80211_rx_napi(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
> +void ieee80211_rx_napi(struct ieee80211_hw *hw,
> +                      struct ieee80211_link_sta *link_sta,
>                        struct sk_buff *skb, struct napi_struct *napi);
> 
>  /**
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 31dc6f83781b1d..0ddd0b8ea12a05 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -5556,7 +5556,8 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
>   * This is the receive path handler. It is called by a low level driver when an
>   * 802.11 MPDU is received from the hardware.
>   */
> -void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
> +void ieee80211_rx_list(struct ieee80211_hw *hw,
> +                      struct ieee80211_link_sta *link_pubsta,
>                        struct sk_buff *skb, struct list_head *list)
>  {
>         struct ieee80211_local *local = hw_to_local(hw);
> @@ -5564,6 +5565,7 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
>         struct ieee80211_supported_band *sband;
>         struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
>         struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> +       struct ieee80211_sta *pubsta;
> 
>         WARN_ON_ONCE(softirq_count() == 0);
> 
> @@ -5694,8 +5696,15 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
>                 }
>         }
> 
> -       if (WARN_ON_ONCE(status->link_id >= IEEE80211_LINK_UNSPECIFIED))
> -               goto drop;
> +       /* FIXME: Emulate the old driver behaviour for now */
> +       if (link_pubsta) {
> +               status->link_valid = 1;
> +               status->link_id = link_pubsta->link_id;
> +               pubsta = link_pubsta->sta;
> +       } else {
> +               status->link_valid = 0;
> +               pubsta = NULL;
> +       }
> 
>         status->rx_flags = 0;
> 
> @@ -5727,7 +5736,8 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
>  }
>  EXPORT_SYMBOL(ieee80211_rx_list);
> 
> -void ieee80211_rx_napi(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
> +void ieee80211_rx_napi(struct ieee80211_hw *hw,
> +                      struct ieee80211_link_sta *link_pubsta,
>                        struct sk_buff *skb, struct napi_struct *napi)
>  {
>         struct sk_buff *tmp;
> @@ -5740,7 +5750,7 @@ void ieee80211_rx_napi(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
>          * receive processing
>          */
>         rcu_read_lock();
> -       ieee80211_rx_list(hw, pubsta, skb, &list);
> +       ieee80211_rx_list(hw, link_pubsta, skb, &list);
>         rcu_read_unlock();
> 
>         if (!napi) {
> 
> rtw89_core_rx_to_mac80211() calls ieee80211_rx_napi() with link_sta = NULL,
> so it's not getting the emulated old behaviour in ieee80211_rx_list().
> 
> Is this something that needs fixing?

By the comment of link_id field in struct ieee80211_rx_status:

 * @link_id: id of the link used to receive the packet. Set and used by
 *	mac80211 internally, it uses @freq set by the driver to identify the
 *	correct link per vif.

rtw89 can report proper freq to get link_id back. We tested 7.4-rc and
things are expected. Did you encounter problems?