RE: [PATCH v2 10/11] wifi: rtw88: record beacons from the target BSSID before authenticating

[Ping-Ke Shih]

luka.gejak@xxxxxxxxx <luka.gejak@xxxxxxxxx> wrote:
> From: Luka Gejak <luka.gejak@xxxxxxxxx>
> 
> The vendor start_clnt_join() does not begin authentication until it has
> seen a beacon or probe response from the target BSSID, and the RTL8723BS
> firmware depends on that ordering: authenticating earlier leaves the
> firmware without the BSS parameters it needs and the exchange times out.

I know vendor driver does wait for a beacon, but I don't think this can
affect connection. Could you confirm this again?

> 
> Record such frames from the receive path so the join sequence can wait
> for one. The recording is gated on the chip and only active during the
> pre-authentication window, so it costs other chips a single test per
> frame.
> 
> Signed-off-by: Luka Gejak <luka.gejak@xxxxxxxxx>
> ---
>  drivers/net/wireless/realtek/rtw88/mac80211.c | 31 +++++++++++++++++++
>  drivers/net/wireless/realtek/rtw88/main.c     |  2 ++
>  drivers/net/wireless/realtek/rtw88/main.h     | 20 ++++++++++++
>  drivers/net/wireless/realtek/rtw88/sdio.c     |  5 +++
>  4 files changed, 58 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c
> b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index b01b98d24b0a..ff0f4f0bdc7b 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
> @@ -15,6 +15,37 @@
>  #include "wow.h"
>  #include "sar.h"
> 
> +/* 8723BS SDIO: record a beacon/probe-resp seen from the target BSSID during
> + * the pre-auth window so the join sequence (mgd_prepare_tx) can wait for it,
> + * mirroring the vendor start_clnt_join(). Called from the SDIO RX path.
> + */

comment style.

> +void rtw8723bs_auth_sync_rx(struct rtw_dev *rtwdev,
> +                           const struct ieee80211_hdr *hdr, u32 len,
> +                           const struct rtw_rx_pkt_stat *pkt_stat,
> +                           const struct ieee80211_rx_status *rx_status)
> +{
> +       struct rtw_auth_sync *sync = &rtwdev->auth_sync;
> +       unsigned long flags;
> +       __le16 fc = hdr->frame_control;

reverse X'mas tree 

> +
> +       if (!rtw_is_8723bs(rtwdev) ||

Might caller ensure this?

> +           test_bit(RTW_FLAG_SCANNING, rtwdev->flags) ||
> +           pkt_stat->crc_err || pkt_stat->icv_err)
> +               return;
> +
> +       if (!ieee80211_is_beacon(fc) && !ieee80211_is_probe_resp(fc))
> +               return;
> +
> +       spin_lock_irqsave(&sync->lock, flags);
> +       if (sync->active && ether_addr_equal(hdr->addr3, sync->bssid)) {
> +               sync->seen = true;
> +               sync->seen_count++;
> +               wake_up(&sync->wait);
> +       }
> +       spin_unlock_irqrestore(&sync->lock, flags);
> +}
> +EXPORT_SYMBOL(rtw8723bs_auth_sync_rx);
> +
>  static void rtw_ops_tx(struct ieee80211_hw *hw,
>                        struct ieee80211_tx_control *control,
>                        struct sk_buff *skb)
> diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> index cd9254370fcc..16228cae252e 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c
> @@ -2174,11 +2174,13 @@ int rtw_core_init(struct rtw_dev *rtwdev)
> 
>         spin_lock_init(&rtwdev->txq_lock);
>         spin_lock_init(&rtwdev->tx_report.q_lock);
> +       spin_lock_init(&rtwdev->auth_sync.lock);
> 
>         mutex_init(&rtwdev->mutex);
>         mutex_init(&rtwdev->hal.tx_power_mutex);
> 
>         init_waitqueue_head(&rtwdev->coex.wait);
> +       init_waitqueue_head(&rtwdev->auth_sync.wait);
>         init_completion(&rtwdev->lps_leave_check);
>         init_completion(&rtwdev->fw_scan_density);
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
> index 8f86f7c12de5..4d3d1d99e305 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.h
> +++ b/drivers/net/wireless/realtek/rtw88/main.h
> @@ -2058,6 +2058,20 @@ struct rtw_hw_scan_info {
>         u8 op_bw;
>  };
> 
> +/*
> + * Synchronises the pre-auth wait on a beacon or probe response from the
> + * target BSSID before the join sequence continues.
> + */
> +struct rtw_auth_sync {
> +       wait_queue_head_t wait;
> +       /* Protects the fields below. */
> +       spinlock_t lock;

I think you can use wait/completion. Then no need lock. 

> +       u8 bssid[ETH_ALEN];
> +       bool active;
> +       bool seen;
> +       u32 seen_count;
> +};
> +
>  struct rtw_dev {
>         struct ieee80211_hw *hw;
>         struct device *dev;
> @@ -2134,6 +2148,8 @@ struct rtw_dev {
>         struct completion fw_scan_density;
>         bool ap_active;
> 
> +       struct rtw_auth_sync auth_sync;
> +
>         bool led_registered;
>         char led_name[32];
>         struct led_classdev led_cdev;
> @@ -2291,4 +2307,8 @@ bool rtw_core_check_sta_active(struct rtw_dev *rtwdev);
>  void rtw_core_enable_beacon(struct rtw_dev *rtwdev, bool enable);
>  void rtw_set_ampdu_factor(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
>                           struct ieee80211_bss_conf *bss_conf);
> +void rtw8723bs_auth_sync_rx(struct rtw_dev *rtwdev,
> +                           const struct ieee80211_hdr *hdr, u32 len,
> +                           const struct rtw_rx_pkt_stat *pkt_stat,
> +                           const struct ieee80211_rx_status *rx_status);
>  #endif
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index 572b791249ed..b294bcc4d828 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -1203,6 +1203,11 @@ static void rtw_sdio_rx_skb(struct rtw_dev *rtwdev, struct sk_buff *skb,
>         rtw_update_rx_freq_for_invalid(rtwdev, skb, rx_status, pkt_stat);
>         rtw_rx_stats(rtwdev, pkt_stat->vif, skb);
> 
> +       if (skb->len >= sizeof(struct ieee80211_hdr_3addr))
> +               rtw8723bs_auth_sync_rx(rtwdev,
> +                                      (struct ieee80211_hdr *)skb->data,
> +                                      skb->len, pkt_stat, rx_status);
> +
>         ieee80211_rx_irqsafe(rtwdev->hw, skb);
>  }
> 
> --
> 2.55.0