All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wifi: rtl8xxxu: enable MFP support
@ 2024-03-14 16:48 Martin Kaistra
  2024-03-15  1:22 ` Ping-Ke Shih
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Martin Kaistra @ 2024-03-14 16:48 UTC (permalink / raw)
  To: linux-wireless
  Cc: Jes Sorensen, Kalle Valo, Ping-Ke Shih, Bitterblue Smith,
	Sebastian Andrzej Siewior

In order to connect to networks which require 802.11w, add the
MFP_CAPABLE flag and let mac80211 do the actual crypto in software.

When a robust management frames is received, rx_dec->swdec is not set,
even though the HW did not decrypt it. Extend the check and don't set
RX_FLAG_DECRYPTED for these frames in order to use SW decryption.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 4a49f8f9d80f2..870bd952f5902 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6473,7 +6473,9 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
 			rx_status->mactime = rx_desc->tsfl;
 			rx_status->flag |= RX_FLAG_MACTIME_START;
 
-			if (!rx_desc->swdec)
+			if (!rx_desc->swdec &&
+			    !(_ieee80211_is_robust_mgmt_frame(hdr) &&
+			      ieee80211_has_protected(hdr->frame_control)))
 				rx_status->flag |= RX_FLAG_DECRYPTED;
 			if (rx_desc->crc32)
 				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
@@ -6578,7 +6580,9 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
 			rx_status->mactime = rx_desc->tsfl;
 			rx_status->flag |= RX_FLAG_MACTIME_START;
 
-			if (!rx_desc->swdec)
+			if (!rx_desc->swdec &&
+			    !(_ieee80211_is_robust_mgmt_frame(hdr) &&
+			      ieee80211_has_protected(hdr->frame_control)))
 				rx_status->flag |= RX_FLAG_DECRYPTED;
 			if (rx_desc->crc32)
 				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
@@ -7998,6 +8002,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 	ieee80211_hw_set(hw, HAS_RATE_CONTROL);
 	ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
 	ieee80211_hw_set(hw, AMPDU_AGGREGATION);
+	ieee80211_hw_set(hw, MFP_CAPABLE);
 
 	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-03-14 16:48 [PATCH] wifi: rtl8xxxu: enable MFP support Martin Kaistra
@ 2024-03-15  1:22 ` Ping-Ke Shih
  2024-03-20  2:03 ` Ping-Ke Shih
  2024-04-14 11:32 ` Bitterblue Smith
  2 siblings, 0 replies; 11+ messages in thread
From: Ping-Ke Shih @ 2024-03-15  1:22 UTC (permalink / raw)
  To: Martin Kaistra, linux-wireless
  Cc: Jes Sorensen, Kalle Valo, Bitterblue Smith, Sebastian Andrzej Siewior



> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Friday, March 15, 2024 12:49 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: [PATCH] wifi: rtl8xxxu: enable MFP support
> 
> In order to connect to networks which require 802.11w, add the
> MFP_CAPABLE flag and let mac80211 do the actual crypto in software.
> 
> When a robust management frames is received, rx_dec->swdec is not set,
> even though the HW did not decrypt it. Extend the check and don't set
> RX_FLAG_DECRYPTED for these frames in order to use SW decryption.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-03-14 16:48 [PATCH] wifi: rtl8xxxu: enable MFP support Martin Kaistra
  2024-03-15  1:22 ` Ping-Ke Shih
@ 2024-03-20  2:03 ` Ping-Ke Shih
  2024-04-14 11:32 ` Bitterblue Smith
  2 siblings, 0 replies; 11+ messages in thread
From: Ping-Ke Shih @ 2024-03-20  2:03 UTC (permalink / raw)
  To: Martin Kaistra, linux-wireless; +Cc: Jes Sorensen

Martin Kaistra <martin.kaistra@linutronix.de> wrote:

> In order to connect to networks which require 802.11w, add the
> MFP_CAPABLE flag and let mac80211 do the actual crypto in software.
> 
> When a robust management frames is received, rx_dec->swdec is not set,
> even though the HW did not decrypt it. Extend the check and don't set
> RX_FLAG_DECRYPTED for these frames in order to use SW decryption.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

1 patch(es) applied to rtw-next branch of rtw.git, thanks.

77f5924fc41c wifi: rtl8xxxu: enable MFP support

---
https://github.com/pkshih/rtw.git


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-03-14 16:48 [PATCH] wifi: rtl8xxxu: enable MFP support Martin Kaistra
  2024-03-15  1:22 ` Ping-Ke Shih
  2024-03-20  2:03 ` Ping-Ke Shih
@ 2024-04-14 11:32 ` Bitterblue Smith
  2024-04-15  0:57   ` Ping-Ke Shih
  2024-04-15  6:49   ` Martin Kaistra
  2 siblings, 2 replies; 11+ messages in thread
From: Bitterblue Smith @ 2024-04-14 11:32 UTC (permalink / raw)
  To: Martin Kaistra, linux-wireless
  Cc: Jes Sorensen, Kalle Valo, Ping-Ke Shih, Sebastian Andrzej Siewior

On 14/03/2024 18:48, Martin Kaistra wrote:
> In order to connect to networks which require 802.11w, add the
> MFP_CAPABLE flag and let mac80211 do the actual crypto in software.
> 
> When a robust management frames is received, rx_dec->swdec is not set,
> even though the HW did not decrypt it. Extend the check and don't set
> RX_FLAG_DECRYPTED for these frames in order to use SW decryption.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 4a49f8f9d80f2..870bd952f5902 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6473,7 +6473,9 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>  			rx_status->mactime = rx_desc->tsfl;
>  			rx_status->flag |= RX_FLAG_MACTIME_START;
>  
> -			if (!rx_desc->swdec)
> +			if (!rx_desc->swdec &&
> +			    !(_ieee80211_is_robust_mgmt_frame(hdr) &&
> +			      ieee80211_has_protected(hdr->frame_control)))
>  				rx_status->flag |= RX_FLAG_DECRYPTED;
>  			if (rx_desc->crc32)
>  				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
> @@ -6578,7 +6580,9 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>  			rx_status->mactime = rx_desc->tsfl;
>  			rx_status->flag |= RX_FLAG_MACTIME_START;
>  
> -			if (!rx_desc->swdec)
> +			if (!rx_desc->swdec &&
> +			    !(_ieee80211_is_robust_mgmt_frame(hdr) &&
> +			      ieee80211_has_protected(hdr->frame_control)))
>  				rx_status->flag |= RX_FLAG_DECRYPTED;
>  			if (rx_desc->crc32)
>  				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
> @@ -7998,6 +8002,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>  	ieee80211_hw_set(hw, HAS_RATE_CONTROL);
>  	ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
>  	ieee80211_hw_set(hw, AMPDU_AGGREGATION);
> +	ieee80211_hw_set(hw, MFP_CAPABLE);
>  
>  	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>  

I ran into this problem recently with rtl8192du:
https://lore.kernel.org/linux-wireless/ed12ec17-ae6e-45fa-a72f-23e0a34654da@gmail.com/

Does the same fix work for you in rtl8xxxu? Checking the "security"
field of the RX descriptor is simpler than calling two functions.
Sorry to bother you when the patch is already applied.

Also, won't you send the patch to the stable tree?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-04-14 11:32 ` Bitterblue Smith
@ 2024-04-15  0:57   ` Ping-Ke Shih
  2024-04-15  6:49   ` Martin Kaistra
  1 sibling, 0 replies; 11+ messages in thread
From: Ping-Ke Shih @ 2024-04-15  0:57 UTC (permalink / raw)
  To: Bitterblue Smith, Martin Kaistra, linux-wireless
  Cc: Jes Sorensen, Kalle Valo, Sebastian Andrzej Siewior

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:

> > @@ -7998,6 +8002,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> >       ieee80211_hw_set(hw, HAS_RATE_CONTROL);
> >       ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
> >       ieee80211_hw_set(hw, AMPDU_AGGREGATION);
> > +     ieee80211_hw_set(hw, MFP_CAPABLE);
> >
> >       wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
> >
> 
> I ran into this problem recently with rtl8192du:
> https://lore.kernel.org/linux-wireless/ed12ec17-ae6e-45fa-a72f-23e0a34654da@gmail.com/
> 
> Does the same fix work for you in rtl8xxxu? Checking the "security"
> field of the RX descriptor is simpler than calling two functions.
> Sorry to bother you when the patch is already applied.
> 
> Also, won't you send the patch to the stable tree?

I'm not aware of two patches did the similar thing. Thanks for pointing this. 

Martin, please check if you met the same problem as Bitterblue's one. If you
want to add this fix to stable tree, please provide two patches -- one is to
revert this patch, and an new one to add Bitterblue's method with Cc stable.

Ping-Ke 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-04-14 11:32 ` Bitterblue Smith
  2024-04-15  0:57   ` Ping-Ke Shih
@ 2024-04-15  6:49   ` Martin Kaistra
  2024-04-15 19:14     ` Bitterblue Smith
  1 sibling, 1 reply; 11+ messages in thread
From: Martin Kaistra @ 2024-04-15  6:49 UTC (permalink / raw)
  To: Bitterblue Smith, linux-wireless
  Cc: Jes Sorensen, Kalle Valo, Ping-Ke Shih, Sebastian Andrzej Siewior

Am 14.04.24 um 13:32 schrieb Bitterblue Smith:
> On 14/03/2024 18:48, Martin Kaistra wrote:
>> In order to connect to networks which require 802.11w, add the
>> MFP_CAPABLE flag and let mac80211 do the actual crypto in software.
>>
>> When a robust management frames is received, rx_dec->swdec is not set,
>> even though the HW did not decrypt it. Extend the check and don't set
>> RX_FLAG_DECRYPTED for these frames in order to use SW decryption.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 4a49f8f9d80f2..870bd952f5902 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6473,7 +6473,9 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>   			rx_status->mactime = rx_desc->tsfl;
>>   			rx_status->flag |= RX_FLAG_MACTIME_START;
>>   
>> -			if (!rx_desc->swdec)
>> +			if (!rx_desc->swdec &&
>> +			    !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>> +			      ieee80211_has_protected(hdr->frame_control)))
>>   				rx_status->flag |= RX_FLAG_DECRYPTED;
>>   			if (rx_desc->crc32)
>>   				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>> @@ -6578,7 +6580,9 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>   			rx_status->mactime = rx_desc->tsfl;
>>   			rx_status->flag |= RX_FLAG_MACTIME_START;
>>   
>> -			if (!rx_desc->swdec)
>> +			if (!rx_desc->swdec &&
>> +			    !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>> +			      ieee80211_has_protected(hdr->frame_control)))
>>   				rx_status->flag |= RX_FLAG_DECRYPTED;
>>   			if (rx_desc->crc32)
>>   				rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>> @@ -7998,6 +8002,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>>   	ieee80211_hw_set(hw, HAS_RATE_CONTROL);
>>   	ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
>>   	ieee80211_hw_set(hw, AMPDU_AGGREGATION);
>> +	ieee80211_hw_set(hw, MFP_CAPABLE);
>>   
>>   	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>>   
> 
> I ran into this problem recently with rtl8192du:
> https://lore.kernel.org/linux-wireless/ed12ec17-ae6e-45fa-a72f-23e0a34654da@gmail.com/
> 
> Does the same fix work for you in rtl8xxxu? Checking the "security"
> field of the RX descriptor is simpler than calling two functions.
> Sorry to bother you when the patch is already applied.

Thanks for the hint. I tried to do something similar to what has been done in 
other rtlwifi drivers and missed the solution in rtw88, which is probably better:

rtlwifi/rtl8188ee/trx.c
rtlwifi/rtl8192ce/trx.c
rtlwifi/rtl8192ee/trx.c
rtlwifi/rtl8192se/trx.c
rtlwifi/rtl8723ae/trx.c
rtlwifi/rtl8723be/trx.c
rtlwifi/rtl8821ae/trx.c

Shouldn't it be changed in these locations as well?

I will do a test for rtl8xxxu and if it is successful send a new patch.

> 
> Also, won't you send the patch to the stable tree?

The rtl8xxxu driver previously did not have the MFP_CAPABLE flag set. As I am 
adding new functionality (support for WPA3), I don't think this should go to stable.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-04-15  6:49   ` Martin Kaistra
@ 2024-04-15 19:14     ` Bitterblue Smith
  2024-04-17  6:43       ` Martin Kaistra
  0 siblings, 1 reply; 11+ messages in thread
From: Bitterblue Smith @ 2024-04-15 19:14 UTC (permalink / raw)
  To: Martin Kaistra, linux-wireless
  Cc: Jes Sorensen, Kalle Valo, Ping-Ke Shih, Sebastian Andrzej Siewior

On 15/04/2024 09:49, Martin Kaistra wrote:
> Am 14.04.24 um 13:32 schrieb Bitterblue Smith:
>> On 14/03/2024 18:48, Martin Kaistra wrote:
>>> In order to connect to networks which require 802.11w, add the
>>> MFP_CAPABLE flag and let mac80211 do the actual crypto in software.
>>>
>>> When a robust management frames is received, rx_dec->swdec is not set,
>>> even though the HW did not decrypt it. Extend the check and don't set
>>> RX_FLAG_DECRYPTED for these frames in order to use SW decryption.
>>>
>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>> ---
>>>   drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++++++--
>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index 4a49f8f9d80f2..870bd952f5902 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -6473,7 +6473,9 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>>               rx_status->mactime = rx_desc->tsfl;
>>>               rx_status->flag |= RX_FLAG_MACTIME_START;
>>>   -            if (!rx_desc->swdec)
>>> +            if (!rx_desc->swdec &&
>>> +                !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>>> +                  ieee80211_has_protected(hdr->frame_control)))
>>>                   rx_status->flag |= RX_FLAG_DECRYPTED;
>>>               if (rx_desc->crc32)
>>>                   rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>>> @@ -6578,7 +6580,9 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>>               rx_status->mactime = rx_desc->tsfl;
>>>               rx_status->flag |= RX_FLAG_MACTIME_START;
>>>   -            if (!rx_desc->swdec)
>>> +            if (!rx_desc->swdec &&
>>> +                !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>>> +                  ieee80211_has_protected(hdr->frame_control)))
>>>                   rx_status->flag |= RX_FLAG_DECRYPTED;
>>>               if (rx_desc->crc32)
>>>                   rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>>> @@ -7998,6 +8002,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>>>       ieee80211_hw_set(hw, HAS_RATE_CONTROL);
>>>       ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
>>>       ieee80211_hw_set(hw, AMPDU_AGGREGATION);
>>> +    ieee80211_hw_set(hw, MFP_CAPABLE);
>>>         wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>>>   
>>
>> I ran into this problem recently with rtl8192du:
>> https://lore.kernel.org/linux-wireless/ed12ec17-ae6e-45fa-a72f-23e0a34654da@gmail.com/
>>
>> Does the same fix work for you in rtl8xxxu? Checking the "security"
>> field of the RX descriptor is simpler than calling two functions.
>> Sorry to bother you when the patch is already applied.
> 
> Thanks for the hint. I tried to do something similar to what has been done in other rtlwifi drivers and missed the solution in rtw88, which is probably better:
> 
> rtlwifi/rtl8188ee/trx.c
> rtlwifi/rtl8192ce/trx.c
> rtlwifi/rtl8192ee/trx.c
> rtlwifi/rtl8192se/trx.c
> rtlwifi/rtl8723ae/trx.c
> rtlwifi/rtl8723be/trx.c
> rtlwifi/rtl8821ae/trx.c
> 
> Shouldn't it be changed in these locations as well?
> 
> I will do a test for rtl8xxxu and if it is successful send a new patch.
> 
>>
>> Also, won't you send the patch to the stable tree?
> 
> The rtl8xxxu driver previously did not have the MFP_CAPABLE flag set. As I am adding new functionality (support for WPA3), I don't think this should go to stable.

Without your patch I can't connect to my phone's hotspot
when it uses WPA3:

Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: nl80211: kernel reports: key setting validation failed
Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: WPA: Failed to configure IGTK to the driver
Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: RSN: Failed to configure IGTK

It doesn't say anything about WPA3 or management frame
protection, just prints those unhelpful errors and tries
to connect over and over again. To me that looks more like
fixing a bug than adding new functionality. It's just sad
that people need to install kernel 6.10+ in order to support
WPA3, when the patch is so small.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-04-15 19:14     ` Bitterblue Smith
@ 2024-04-17  6:43       ` Martin Kaistra
  2024-04-17  7:23         ` Ping-Ke Shih
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Kaistra @ 2024-04-17  6:43 UTC (permalink / raw)
  To: linux-wireless, Ping-Ke Shih
  Cc: Jes Sorensen, Kalle Valo, Sebastian Andrzej Siewior, Bitterblue Smith

Hi Ping-Ke,

Am 15.04.24 um 21:14 schrieb Bitterblue Smith:
> On 15/04/2024 09:49, Martin Kaistra wrote:
>> Am 14.04.24 um 13:32 schrieb Bitterblue Smith:
>>> On 14/03/2024 18:48, Martin Kaistra wrote:
>>>> In order to connect to networks which require 802.11w, add the
>>>> MFP_CAPABLE flag and let mac80211 do the actual crypto in software.
>>>>
>>>> When a robust management frames is received, rx_dec->swdec is not set,
>>>> even though the HW did not decrypt it. Extend the check and don't set
>>>> RX_FLAG_DECRYPTED for these frames in order to use SW decryption.
>>>>
>>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>>> ---
>>>>    drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++++++--
>>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> index 4a49f8f9d80f2..870bd952f5902 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> @@ -6473,7 +6473,9 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>>>                rx_status->mactime = rx_desc->tsfl;
>>>>                rx_status->flag |= RX_FLAG_MACTIME_START;
>>>>    -            if (!rx_desc->swdec)
>>>> +            if (!rx_desc->swdec &&
>>>> +                !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>>>> +                  ieee80211_has_protected(hdr->frame_control)))
>>>>                    rx_status->flag |= RX_FLAG_DECRYPTED;
>>>>                if (rx_desc->crc32)
>>>>                    rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>>>> @@ -6578,7 +6580,9 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>>>                rx_status->mactime = rx_desc->tsfl;
>>>>                rx_status->flag |= RX_FLAG_MACTIME_START;
>>>>    -            if (!rx_desc->swdec)
>>>> +            if (!rx_desc->swdec &&
>>>> +                !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>>>> +                  ieee80211_has_protected(hdr->frame_control)))
>>>>                    rx_status->flag |= RX_FLAG_DECRYPTED;
>>>>                if (rx_desc->crc32)
>>>>                    rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>>>> @@ -7998,6 +8002,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>>>>        ieee80211_hw_set(hw, HAS_RATE_CONTROL);
>>>>        ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
>>>>        ieee80211_hw_set(hw, AMPDU_AGGREGATION);
>>>> +    ieee80211_hw_set(hw, MFP_CAPABLE);
>>>>          wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>>>>    
>>>
>>> I ran into this problem recently with rtl8192du:
>>> https://lore.kernel.org/linux-wireless/ed12ec17-ae6e-45fa-a72f-23e0a34654da@gmail.com/
>>>
>>> Does the same fix work for you in rtl8xxxu? Checking the "security"
>>> field of the RX descriptor is simpler than calling two functions.
>>> Sorry to bother you when the patch is already applied.
>>
>> Thanks for the hint. I tried to do something similar to what has been done in other rtlwifi drivers and missed the solution in rtw88, which is probably better:
>>
>> rtlwifi/rtl8188ee/trx.c
>> rtlwifi/rtl8192ce/trx.c
>> rtlwifi/rtl8192ee/trx.c
>> rtlwifi/rtl8192se/trx.c
>> rtlwifi/rtl8723ae/trx.c
>> rtlwifi/rtl8723be/trx.c
>> rtlwifi/rtl8821ae/trx.c
>>
>> Shouldn't it be changed in these locations as well?
>>
>> I will do a test for rtl8xxxu and if it is successful send a new patch.
>>
>>>
>>> Also, won't you send the patch to the stable tree?
>>
>> The rtl8xxxu driver previously did not have the MFP_CAPABLE flag set. As I am adding new functionality (support for WPA3), I don't think this should go to stable.
> 
> Without your patch I can't connect to my phone's hotspot
> when it uses WPA3:
> 
> Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: nl80211: kernel reports: key setting validation failed
> Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: WPA: Failed to configure IGTK to the driver
> Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: RSN: Failed to configure IGTK
> 
> It doesn't say anything about WPA3 or management frame
> protection, just prints those unhelpful errors and tries
> to connect over and over again. To me that looks more like
> fixing a bug than adding new functionality. It's just sad
> that people need to install kernel 6.10+ in order to support
> WPA3, when the patch is so small.

I would like to know your opinion on this. imho this patch should not go to 
stable and I would therefore propose to just send a patch to improve the checks.
If you as a maintainer however say, you would like to see this in stable, then I 
will send a revert and a new patch.

Martin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-04-17  6:43       ` Martin Kaistra
@ 2024-04-17  7:23         ` Ping-Ke Shih
  2024-04-17  8:28           ` Kalle Valo
  0 siblings, 1 reply; 11+ messages in thread
From: Ping-Ke Shih @ 2024-04-17  7:23 UTC (permalink / raw)
  To: Kalle Valo, Martin Kaistra, linux-wireless
  Cc: Jes Sorensen, Sebastian Andrzej Siewior, Bitterblue Smith

Hi Kalle,

Martin Kaistra <martin.kaistra@linutronix.de> wrote:

> Hi Ping-Ke,
> 
> Am 15.04.24 um 21:14 schrieb Bitterblue Smith:
> > On 15/04/2024 09:49, Martin Kaistra wrote:
> >> Am 14.04.24 um 13:32 schrieb Bitterblue Smith:
> >>> On 14/03/2024 18:48, Martin Kaistra wrote:
> >>>
> >>> Also, won't you send the patch to the stable tree?
> >>
> >> The rtl8xxxu driver previously did not have the MFP_CAPABLE flag set. As I am adding new functionality
> (support for WPA3), I don't think this should go to stable.
> >
> > Without your patch I can't connect to my phone's hotspot
> > when it uses WPA3:
> >
> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: nl80211: kernel reports: key setting validation failed
> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: WPA: Failed to configure IGTK to the driver
> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: RSN: Failed to configure IGTK
> >
> > It doesn't say anything about WPA3 or management frame
> > protection, just prints those unhelpful errors and tries
> > to connect over and over again. To me that looks more like
> > fixing a bug than adding new functionality. It's just sad
> > that people need to install kernel 6.10+ in order to support
> > WPA3, when the patch is so small.
> 
> I would like to know your opinion on this. imho this patch should not go to
> stable and I would therefore propose to just send a patch to improve the checks.
> If you as a maintainer however say, you would like to see this in stable, then I
> will send a revert and a new patch.
> 

We have a question about MFP support to stable tree. rtl8xxxu never supports MFP,
and recently we have a small patch (10+ LOC) to enable that. I feel we should
treat it as new feature, not add to stable tree. But as Bitterblue mentioned,
it looks like a bug. Need your opinion.

Ping-Ke


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-04-17  7:23         ` Ping-Ke Shih
@ 2024-04-17  8:28           ` Kalle Valo
  2024-04-17  8:47             ` Ping-Ke Shih
  0 siblings, 1 reply; 11+ messages in thread
From: Kalle Valo @ 2024-04-17  8:28 UTC (permalink / raw)
  To: Ping-Ke Shih
  Cc: Martin Kaistra, linux-wireless, Jes Sorensen,
	Sebastian Andrzej Siewior, Bitterblue Smith

Ping-Ke Shih <pkshih@realtek.com> writes:

> Hi Kalle,
>
> Martin Kaistra <martin.kaistra@linutronix.de> wrote:
>
>> Hi Ping-Ke,
>> 
>> Am 15.04.24 um 21:14 schrieb Bitterblue Smith:
>> > On 15/04/2024 09:49, Martin Kaistra wrote:
>> >> Am 14.04.24 um 13:32 schrieb Bitterblue Smith:
>> >>> On 14/03/2024 18:48, Martin Kaistra wrote:
>> >>>
>> >>> Also, won't you send the patch to the stable tree?
>> >>
>> >> The rtl8xxxu driver previously did not have the MFP_CAPABLE flag
>> >> set. As I am adding new functionality
>> (support for WPA3), I don't think this should go to stable.
>> >
>> > Without your patch I can't connect to my phone's hotspot
>> > when it uses WPA3:
>> >
>> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: nl80211: kernel
>> > reports: key setting validation failed
>> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: WPA:
>> > Failed to configure IGTK to the driver
>> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: RSN:
>> > Failed to configure IGTK
>> >
>> > It doesn't say anything about WPA3 or management frame
>> > protection, just prints those unhelpful errors and tries
>> > to connect over and over again. To me that looks more like
>> > fixing a bug than adding new functionality. It's just sad
>> > that people need to install kernel 6.10+ in order to support
>> > WPA3, when the patch is so small.
>> 
>> I would like to know your opinion on this. imho this patch should not go to
>> stable and I would therefore propose to just send a patch to improve the checks.
>> If you as a maintainer however say, you would like to see this in stable, then I
>> will send a revert and a new patch.
>> 
>
> We have a question about MFP support to stable tree. rtl8xxxu never supports MFP,
> and recently we have a small patch (10+ LOC) to enable that. I feel we should
> treat it as new feature, not add to stable tree. But as Bitterblue mentioned,
> it looks like a bug. Need your opinion.

I assume you are talking about this patch:

https://lore.kernel.org/all/20240314164850.86432-1-martin.kaistra@linutronix.de/

I have no strong opinion here. To me it also looks like a new feature
but it might fall within "Serious issues as reported by a user of a
distribution kernel may also be considered if they fix a notable
performance or interactivity issue" stable kernel rule:

https://docs.kernel.org/process/stable-kernel-rules.html

Maybe go with 'Option 2'? That is, wait for the commit to go to Linus'
tree, send a mail to stable maintainers explaining the situation and let
them decide. But I would be fine with 'Option 1' as well, your call :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] wifi: rtl8xxxu: enable MFP support
  2024-04-17  8:28           ` Kalle Valo
@ 2024-04-17  8:47             ` Ping-Ke Shih
  0 siblings, 0 replies; 11+ messages in thread
From: Ping-Ke Shih @ 2024-04-17  8:47 UTC (permalink / raw)
  To: Kalle Valo, Martin Kaistra
  Cc: linux-wireless, Jes Sorensen, Sebastian Andrzej Siewior,
	Bitterblue Smith

Kalle Valo <kvalo@kernel.org> wrote:

> 
> Ping-Ke Shih <pkshih@realtek.com> writes:
> 
> > Hi Kalle,
> >
> > Martin Kaistra <martin.kaistra@linutronix.de> wrote:
> >
> >> Hi Ping-Ke,
> >>
> >> Am 15.04.24 um 21:14 schrieb Bitterblue Smith:
> >> > On 15/04/2024 09:49, Martin Kaistra wrote:
> >> >> Am 14.04.24 um 13:32 schrieb Bitterblue Smith:
> >> >>> On 14/03/2024 18:48, Martin Kaistra wrote:
> >> >>>
> >> >>> Also, won't you send the patch to the stable tree?
> >> >>
> >> >> The rtl8xxxu driver previously did not have the MFP_CAPABLE flag
> >> >> set. As I am adding new functionality
> >> (support for WPA3), I don't think this should go to stable.
> >> >
> >> > Without your patch I can't connect to my phone's hotspot
> >> > when it uses WPA3:
> >> >
> >> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: nl80211: kernel
> >> > reports: key setting validation failed
> >> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: WPA:
> >> > Failed to configure IGTK to the driver
> >> > Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: RSN:
> >> > Failed to configure IGTK
> >> >
> >> > It doesn't say anything about WPA3 or management frame
> >> > protection, just prints those unhelpful errors and tries
> >> > to connect over and over again. To me that looks more like
> >> > fixing a bug than adding new functionality. It's just sad
> >> > that people need to install kernel 6.10+ in order to support
> >> > WPA3, when the patch is so small.
> >>
> >> I would like to know your opinion on this. imho this patch should not go to
> >> stable and I would therefore propose to just send a patch to improve the checks.
> >> If you as a maintainer however say, you would like to see this in stable, then I
> >> will send a revert and a new patch.
> >>
> >
> > We have a question about MFP support to stable tree. rtl8xxxu never supports MFP,
> > and recently we have a small patch (10+ LOC) to enable that. I feel we should
> > treat it as new feature, not add to stable tree. But as Bitterblue mentioned,
> > it looks like a bug. Need your opinion.
> 
> I assume you are talking about this patch:
> 
> https://lore.kernel.org/all/20240314164850.86432-1-martin.kaistra@linutronix.de/
> 
> I have no strong opinion here. To me it also looks like a new feature
> but it might fall within "Serious issues as reported by a user of a
> distribution kernel may also be considered if they fix a notable
> performance or interactivity issue" stable kernel rule:
> 
> https://docs.kernel.org/process/stable-kernel-rules.html
> 
> Maybe go with 'Option 2'? That is, wait for the commit to go to Linus'
> tree, send a mail to stable maintainers explaining the situation and let
> them decide. But I would be fine with 'Option 1' as well, your call :)
> 

Thanks for your inputs, Kalle. :)

Hi Martin,

Bitterblue had a simple way [1] to support MFP, if that also works to you, I
think we can revert your original patch [2], and add an new one with Cc stable.
If that doesn't work to you, let's keep your patch and go via 'Option 2'.

[1] https://lore.kernel.org/linux-wireless/ed12ec17-ae6e-45fa-a72f-23e0a34654da@gmail.com/
[2] https://lore.kernel.org/all/20240314164850.86432-1-martin.kaistra@linutronix.de/



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-04-17  8:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-14 16:48 [PATCH] wifi: rtl8xxxu: enable MFP support Martin Kaistra
2024-03-15  1:22 ` Ping-Ke Shih
2024-03-20  2:03 ` Ping-Ke Shih
2024-04-14 11:32 ` Bitterblue Smith
2024-04-15  0:57   ` Ping-Ke Shih
2024-04-15  6:49   ` Martin Kaistra
2024-04-15 19:14     ` Bitterblue Smith
2024-04-17  6:43       ` Martin Kaistra
2024-04-17  7:23         ` Ping-Ke Shih
2024-04-17  8:28           ` Kalle Valo
2024-04-17  8:47             ` Ping-Ke Shih

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.