Re: [RFC PATCH 13/14] wifi: rtl8xxxu: Clean up filter configuration

[Martin Kaistra <martin.kaistra@linutronix.de>]

Am 27.03.23 um 04:06 schrieb Ping-Ke Shih:
> 
> 
>> -----Original Message-----
>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>> Sent: Thursday, March 23, 2023 1:19 AM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
>> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de>
>> Subject: [RFC PATCH 13/14] wifi: rtl8xxxu: Clean up filter configuration
>>
>> In AP mode, RCR_CHECK_BSSID_MATCH should not be set. Rearrange RCR bits
>> to filter flags to match other realtek drivers and don't set
>> RCR_CHECK_BSSID_BEACON and RCR_CHECK_BSSID_MATCH in AP mode.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 ++++++++++---------
>>   1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 82fbe778fc5ec..b6f811ad01333 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6597,23 +6597,24 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
>>           * FIF_PLCPFAIL not supported?
>>           */
>>
>> -       if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
>> -               rcr &= ~RCR_CHECK_BSSID_BEACON;
>> -       else
>> -               rcr |= RCR_CHECK_BSSID_BEACON;
>> +       if (priv->vif->type != NL80211_IFTYPE_AP) {
> 
> I think mac80211 configure filters depends on operating conditions, so it would
> be possible to avoid checking vif->type.

It should be possible to remove the vif->type check from 
FIF_BCN_PRBRESP_PROMISC check, but I would still need it to remove the 
CHECK_BSSID_MATCH bit in the AP mode case. Otherwise I seem to receive 
no data frames.


if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
	rcr &= ~(RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH);
else
	rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;

if (priv->vif && priv->vif->type == NL80211_IFTYPE_AP)
	rcr &= ~RCR_CHECK_BSSID_MATCH;

Another way would be like in the rtw88 driver, where the BIT_CBSSID_DATA 
is not set again in the else case, but I am not sure, that is the right way.

> 
>> +               if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
>> +                       rcr &= ~(RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH);
>> +               else
>> +                       rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
>> +       } else {
>> +               rcr &= ~RCR_CHECK_BSSID_MATCH;
>> +       }
>>
>>          if (*total_flags & FIF_CONTROL)
>>                  rcr |= RCR_ACCEPT_CTRL_FRAME;
>>          else
>>                  rcr &= ~RCR_ACCEPT_CTRL_FRAME;
>>
>> -       if (*total_flags & FIF_OTHER_BSS) {
>> +       if (*total_flags & FIF_OTHER_BSS)
>>                  rcr |= RCR_ACCEPT_AP;
>> -               rcr &= ~RCR_CHECK_BSSID_MATCH;
>> -       } else {
>> +       else
>>                  rcr &= ~RCR_ACCEPT_AP;
>> -               rcr |= RCR_CHECK_BSSID_MATCH;
>> -       }
>>
>>          if (*total_flags & FIF_PSPOLL)
>>                  rcr |= RCR_ACCEPT_PM;
>> --
>> 2.30.2
>