Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7e1c9cd8-cb1e-197a-16b4-379647589ff4@linutronix.de>
Date:   Tue, 28 Mar 2023 16:47:05 +0200
MIME-Version: 1.0
Subject: Re: [RFC PATCH 13/14] wifi: rtl8xxxu: Clean up filter configuration
Content-Language: de-DE, en-US
To:     Ping-Ke Shih <pkshih@realtek.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc:     Jes Sorensen <Jes.Sorensen@gmail.com>,
        Kalle Valo <kvalo@kernel.org>,
        Bitterblue Smith <rtl8821cerfe2@gmail.com>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
References: <20230322171905.492855-1-martin.kaistra@linutronix.de>
 <20230322171905.492855-14-martin.kaistra@linutronix.de>
 <ab74ec9f73b741f7b0dd3b8a498d7e32@realtek.com>
From:   Martin Kaistra <martin.kaistra@linutronix.de>
In-Reply-To: <ab74ec9f73b741f7b0dd3b8a498d7e32@realtek.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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
>