[PATCH ath-next] wifi: ath12k: allow beacon protection keys to be installed in hardware

[Karthikeyan Kathirvel]

On 5/9/2025 9:37 PM, Jeff Johnson wrote:
> On 4/29/2025 11:05 PM, Karthikeyan Kathirvel wrote:
>>
>> On 4/29/2025 9:17 PM, Nicolas Escande wrote:
>>> On Mon Apr 21, 2025 at 1:47 PM CEST, Karthikeyan Kathirvel wrote:
>>>> Install beacon protection keys in hardware for AP modes only if hardware
>>>> supports it, as indicated by the WMI service bit
>>>> WMI_TLV_SERVICE_BEACON_PROTECTION_SUPPORT. Allow keyidx up to 7, since
>>>> beacon protection uses keyidx 6 and 7.
>>>>
>>>> Control this feature by setting bit 0 of feature_enable_bitmap when sending
>>>> the WMI_BCN_TMPL_CMDID command to firmware.
>>>>
>>>> Check for the beacon protection enabled bit in both tx and non-tx profiles
>>>> for MBSSID cases. If set in either profile, enable the beacon protection
>>>> feature in firmware for transmitted vif.
>>>>
>>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.4.1-00199-QCAHKSWPL_SILICONZ-1
>>>>
>>>> Signed-off-by: Karthikeyan Kathirvel <karthikeyan.kathirvel at oss.qualcomm.com>
>>> [...]
>>>> @@ -4964,14 +4994,6 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>>    
>>>>    	lockdep_assert_wiphy(hw->wiphy);
>>>>    
>>>> -	/* BIP needs to be done in software */
>>>> -	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
>>>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
>>>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
>>>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256) {
>>>> -		return 1;
>>>> -	}
>>>> -
>>>>    	if (key->keyidx > WMI_MAX_KEY_INDEX)
>>>>    		return -ENOSPC;
>>>>    
>>> This hunk seems to break station mode on QCN9274. Maybe on WCN7850 too ? I see
>>> that it was not tested against that HW.
>>>
>>> With that hunk I cannot receive broadcast trafic sent by the ap anymore.
>>> Generated by a simple "arping -b X.X.X.X -I br0" in my case.
>>>
>>> Replacing that hunk with something similar as what is done in CLO [0] seems to
>>> fix the issue:
>>>
>>> @@ -5575,13 +5605,9 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>    
>>>    	lockdep_assert_wiphy(hw->wiphy);
>>>    
>>> -	/* BIP needs to be done in software */
>>> -	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
>>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
>>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
>>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256) {
>>> +	/* IGTK needs to be done in host software */
>>> +	if (key->keyidx == 4 || key->keyidx == 5)
>>>    		return 1;
>>> -	}
>>>    
>>>    	if (key->keyidx > WMI_MAX_KEY_INDEX)
>>>    		return -ENOSPC;
>>>
>>>
>>> PS: I tested that with firmware PCI WLAN.WBE.1.3.1-00218-QCAHKSWPL_SILICONZ-1
>>>
>>> [0] https://git.codelinaro.org/clo/qsdk/oss/system/feeds/wlan-open/-/blob/win.wlan_host_opensource.3.0/patches/ath12k/726-ath12k-add-beacon-protection-support-for-ath12k.patch?ref_type=heads
>>
>> Thanks for catching this Nicolas, will check and get back on this
> 
> Will you be spinning a v2? Note the dependent mac80211 change has merged.
Yes Jeff, we've identified and confirmed a firmware bug that gets 
triggered during IGTK key installation, which leads to corruption of the 
GTK keys in firmware. To work around this, I’ll send a new version where 
IGTK uses software crypto. Once the firmware bug is resolved upstream, 
we can re-enable hardware offload for IGTK.
> 
> Also there is an indentation issue in the blob:
Will address this in next version
>> -static void ath12k_mac_set_arvif_ies(struct ath12k_link_vif *arvif, struct sk_buff *bcn,
>> +static void ath12k_mac_set_arvif_ies(struct ath12k_link_vif *arvif,
>> +				     struct ath12k_link_vif *tx_arvif,
>> +					 struct sk_buff *bcn,
>>   				     u8 bssid_index, bool *nontx_profile_found)
> 
> struct sk_buff *bcn is not aligned on the (
> 
> /jeff