RE: [PATCH rtw-next v5 03/10] wifi: rtw89: fix RX filter configuration for monitor mode

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

Enver Ikromov <owner@ph4ntom-rev.uz> wrote:
> When entering monitor mode or enabling FIF_OTHER_BSS,
> rtw89_ops_configure_filter() updates B_AX_SNIFFER_MODE or clears
> B_AX_A_A1_MATCH in hal.rx_fltr, but leaves hardware target-address,
> multicast, address CAM, beacon check, and UID drop masks active unless
> specific FIF_* flags happen to toggle:

Please describe your purpose. For example, describe which kind of 
packets you want to capture, but current settings misbehavior. Then,
I can ask internally to know if you patch is necessary or not.

> 
> - B_AX_A_UC_CAM_MATCH and B_AX_A_BC_CAM_MATCH are only cleared when
>   FIF_PROBE_REQ changes, dropping unicast and broadcast frames from
>   arbitrary BSSs without a CAM entry in hardware.
> - B_AX_A_MC is only cleared when FIF_ALLMULTI is set, and
>   B_AX_A_MC_LIST_CAM_MATCH is never cleared, dropping multicast frames
>   when a monitor socket is opened without IFF_PROMISC.
> - B_AX_UID_FILTER_MASK is initialized in DEFAULT_AX_RX_FLTR and
>   DEFAULT_BE_RX_FLTR via mac->default_rx_fltr and is never cleared by
>   any FIF_* flag.
> - B_AX_A_A1_MATCH, B_AX_A_BC, and B_AX_A_BCN_CHK_EN must stay cleared
>   whenever B_AX_SNIFFER_MODE or FIF_OTHER_BSS is active, including
>   after rtw89_hw_scan_complete_cb(), rtw89_roc_end(), or MAC reinit
>   writes rtwdev->hal.rx_fltr back to R_AX_RX_FLTR_OPT /
>   R_BE_RX_FLTR_OPT.
> - FIF_CONTROL and FIF_PSPOLL are masked out of *new_flags even though
>   rx_fltr_init_ax() and rx_fltr_init_be() already forward control
>   frames to the host via R_AX_CTRL_FLTR / R_BE_CTRL_FLTR.
> 
> Retain FIF_CONTROL and FIF_PSPOLL in *new_flags, record FIF_OTHER_BSS
> in hal.rx_fltr_other_bss so active monitor interfaces with
> MONITOR_FLAG_OTHER_BSS (and concurrent passive monitors) receive
> arbitrary-BSS frames without setting B_AX_SNIFFER_MODE, and add
> rtw89_mac_refine_rx_fltr() to clear the drop masks without mutating
> baseline rtwdev->hal.rx_fltr across mode transitions. The bit positions
> in R_AX_RX_FLTR_OPT and R_BE_RX_FLTR_OPT are identical.

Can you reword commit message generated by LLM? Just point the key
point. Reading lots of stuff spends my time.

> 
> Signed-off-by: Enver Ikromov <owner@ph4ntom-rev.uz>
> ---
>  drivers/net/wireless/realtek/rtw89/core.h     |  1 +
>  drivers/net/wireless/realtek/rtw89/mac.c      |  3 ++-
>  drivers/net/wireless/realtek/rtw89/mac.h      | 16 ++++++++++++++++
>  drivers/net/wireless/realtek/rtw89/mac80211.c |  5 ++++-
>  drivers/net/wireless/realtek/rtw89/mac_be.c   |  3 ++-
>  5 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
> index 7f84ee5..7be8810 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.h
> +++ b/drivers/net/wireless/realtek/rtw89/core.h
> @@ -6499,6 +6499,7 @@ struct rtw89_hal {
>         u8 cv;
>         u8 cid; /* enum rtw89_core_chip_cid */
>         u8 acv;
> +       bool rx_fltr_other_bss;
>         u16 aid; /* enum rtw89_core_chip_aid */
>         u32 antenna_tx;
>         u32 antenna_rx;
> diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
> index 41796bf..06fb263 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac.c
> +++ b/drivers/net/wireless/realtek/rtw89/mac.c
> @@ -2681,6 +2681,7 @@ void rtw89_mac_set_rx_fltr(struct rtw89_dev *rtwdev, u8 mac_idx, u32 rx_fltr)
>         u32 reg;
>         u32 val;
> 
> +       rx_fltr = rtw89_mac_refine_rx_fltr(rtwdev, rx_fltr);
>         reg = rtw89_mac_reg_by_idx(rtwdev, mac->rx_fltr, mac_idx);
> 
>         val = rtw89_read32(rtwdev, reg);
> @@ -2706,7 +2707,7 @@ static int rx_fltr_init_ax(struct rtw89_dev *rtwdev, u8 mac_idx)
>                 if (ret)
>                         return ret;
>         }
> -       mac_ftlr = rtwdev->hal.rx_fltr;
> +       mac_ftlr = rtw89_mac_refine_rx_fltr(rtwdev, rtwdev->hal.rx_fltr);
>         plcp_ftlr = B_AX_CCK_CRC_CHK | B_AX_CCK_SIG_CHK |
>                     B_AX_LSIG_PARITY_CHK_EN | B_AX_SIGA_CRC_CHK |
>                     B_AX_VHT_SU_SIGB_CRC_CHK | B_AX_VHT_MU_SIGB_CRC_CHK |
> diff --git a/drivers/net/wireless/realtek/rtw89/mac.h b/drivers/net/wireless/realtek/rtw89/mac.h
> index db42e33..b4a7cb7 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac.h
> +++ b/drivers/net/wireless/realtek/rtw89/mac.h
> @@ -1494,6 +1494,22 @@ void rtw89_mac_set_edcca_mode_bands(struct rtw89_dev *rtwdev, bool normal)
>         rtw89_mac_set_edcca_mode(rtwdev, RTW89_MAC_1, normal);
>  }
> 
> +static inline u32 rtw89_mac_refine_rx_fltr(struct rtw89_dev *rtwdev,
> +                                          u32 rx_fltr)
> +{
> +       if ((rx_fltr & B_AX_SNIFFER_MODE) || rtwdev->hal.rx_fltr_other_bss)

Can we just do additional in rtw89_ops_configure_filter() under the
condition
   rtwdev->hw->conf.flags & IEEE80211_CONF_MONITOR
?

> +               rx_fltr &= ~(B_AX_A_A1_MATCH |
> +                            B_AX_A_BC |
> +                            B_AX_A_MC |
> +                            B_AX_A_UC_CAM_MATCH |
> +                            B_AX_A_BC_CAM_MATCH |
> +                            B_AX_A_MC_LIST_CAM_MATCH |
> +                            B_AX_A_BCN_CHK_EN |
> +                            B_AX_UID_FILTER_MASK);
> +
> +       return rx_fltr;
> +}
> +
>  void rtw89_mac_set_rx_fltr(struct rtw89_dev *rtwdev, u8 mac_idx, u32 rx_fltr);
>  void rtw89_mac_update_rts_threshold(struct rtw89_dev *rtwdev);
>  void rtw89_mac_flush_txq(struct rtw89_dev *rtwdev, u32 queues, bool drop);
> diff --git a/drivers/net/wireless/realtek/rtw89/mac80211.c
> b/drivers/net/wireless/realtek/rtw89/mac80211.c
> index 5a12639..2a3adf0 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw89/mac80211.c
> @@ -335,7 +335,10 @@ static void rtw89_ops_configure_filter(struct ieee80211_hw *hw,
>         rtw89_leave_ps_mode(rtwdev);
> 
>         *new_flags &= FIF_ALLMULTI | FIF_OTHER_BSS | FIF_FCSFAIL |
> -                     FIF_BCN_PRBRESP_PROMISC | FIF_PROBE_REQ;
> +                     FIF_BCN_PRBRESP_PROMISC | FIF_PROBE_REQ |
> +                     FIF_CONTROL | FIF_PSPOLL;
> +
> +       rtwdev->hal.rx_fltr_other_bss = !!(*new_flags & FIF_OTHER_BSS);
> 
>         if (changed_flags & FIF_ALLMULTI) {
>                 if (*new_flags & FIF_ALLMULTI)
> diff --git a/drivers/net/wireless/realtek/rtw89/mac_be.c
> b/drivers/net/wireless/realtek/rtw89/mac_be.c
> index fa35caf..7147679 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac_be.c
> +++ b/drivers/net/wireless/realtek/rtw89/mac_be.c
> @@ -1328,7 +1328,8 @@ static int rx_fltr_init_be(struct rtw89_dev *rtwdev, u8 mac_idx)
>         rtw89_mac_typ_fltr_opt_be(rtwdev, RTW89_DATA, RTW89_FWD_TO_HOST, mac_idx);
> 
>         reg = rtw89_mac_reg_by_idx(rtwdev, R_BE_RX_FLTR_OPT, mac_idx);
> -       rtw89_write32(rtwdev, reg, rtwdev->hal.rx_fltr);
> +       rtw89_write32(rtwdev, reg,
> +                     rtw89_mac_refine_rx_fltr(rtwdev, rtwdev->hal.rx_fltr));

This is just initial state. I suppose rtw89_ops_configure_filter() can change
to proper setting by *new_flags passed from mac80211.

> 
>         reg = rtw89_mac_reg_by_idx(rtwdev, R_BE_PLCP_HDR_FLTR, mac_idx);
>         val = B_BE_HE_SIGB_CRC_CHK | B_BE_VHT_MU_SIGB_CRC_CHK |
> --
> 2.56.0.windows.4
>