public inbox for ath12k@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH ath-next 0/2] wifi: ath12k: set proper key flags and MIC space for CCMP256 and GCMP ciphers
@ 2025-04-06  9:22 Rameshkumar Sundaram
  2025-04-06  9:22 ` [PATCH ath-next 1/2] wifi: ath12k: fix wrong handling of " Rameshkumar Sundaram
  2025-04-06  9:22 ` [PATCH ath-next 2/2] wifi: ath12k: avoid multiple skb_cb fetch in ath12k_mac_mgmt_tx_wmi() Rameshkumar Sundaram
  0 siblings, 2 replies; 5+ messages in thread
From: Rameshkumar Sundaram @ 2025-04-06  9:22 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Rameshkumar Sundaram

Currently for CCMP256, GCMP128 and GCMP256 ciphers, Management key flags
are not set properly and MIC space is not added based on the cipher used.

This results in unexpected drop of protected management frames in case
either of above 3 ciphers is used.
Fix this by setting IEEE80211_KEY_FLAG_GENERATE_IV_MGMT flag for above
ciphers and by reserving proper MIC length for those ciphers.

Rameshkumar Sundaram (2):
  wifi: ath12k: fix wrong handling of CCMP256 and GCMP ciphers
  wifi: ath12k: avoid multiple skb_cb fetch in ath12k_mac_mgmt_tx_wmi()

 drivers/net/wireless/ath/ath12k/dp_rx.c |  3 +--
 drivers/net/wireless/ath/ath12k/dp_rx.h |  2 ++
 drivers/net/wireless/ath/ath12k/mac.c   | 22 +++++++++++++---------
 3 files changed, 16 insertions(+), 11 deletions(-)

base-commit: 12b93f7c6d101d22e0ea3bf4a240699746c79117
-- 
2.34.1



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

* [PATCH ath-next 1/2] wifi: ath12k: fix wrong handling of CCMP256 and GCMP ciphers
  2025-04-06  9:22 [PATCH ath-next 0/2] wifi: ath12k: set proper key flags and MIC space for CCMP256 and GCMP ciphers Rameshkumar Sundaram
@ 2025-04-06  9:22 ` Rameshkumar Sundaram
  2025-04-09  6:49   ` Vasanthakumar Thiagarajan
  2025-04-06  9:22 ` [PATCH ath-next 2/2] wifi: ath12k: avoid multiple skb_cb fetch in ath12k_mac_mgmt_tx_wmi() Rameshkumar Sundaram
  1 sibling, 1 reply; 5+ messages in thread
From: Rameshkumar Sundaram @ 2025-04-06  9:22 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Rameshkumar Sundaram

Currently for CCMP256, GCMP128 and GCMP256 ciphers, in
ath12k_install_key() IEEE80211_KEY_FLAG_GENERATE_IV_MGMT is not set and
in ath12k_mac_mgmt_tx_wmi() a length of IEEE80211_CCMP_MIC_LEN is reserved
for all ciphers.

This results in unexpected drop of protected management frames in case
either of above 3 ciphers is used. The reason is, without
IEEE80211_KEY_FLAG_GENERATE_IV_MGMT set, mac80211 will not generate
CCMP/GCMP headers in TX frame for ath12k.
Also MIC length reserved is wrong and such frames are dropped by hardware.

Fix this by setting IEEE80211_KEY_FLAG_GENERATE_IV_MGMT flag for above
ciphers and by reserving proper MIC length for those ciphers.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/dp_rx.c |  3 +--
 drivers/net/wireless/ath/ath12k/dp_rx.h |  2 ++
 drivers/net/wireless/ath/ath12k/mac.c   | 16 ++++++++++------
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
index 48d907a400b3..db477f047c32 100644
--- a/drivers/net/wireless/ath/ath12k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
@@ -1928,8 +1928,7 @@ static void ath12k_dp_rx_h_csum_offload(struct ath12k *ar, struct sk_buff *msdu)
 			  CHECKSUM_NONE : CHECKSUM_UNNECESSARY;
 }
 
-static int ath12k_dp_rx_crypto_mic_len(struct ath12k *ar,
-				       enum hal_encrypt_type enctype)
+int ath12k_dp_rx_crypto_mic_len(struct ath12k *ar, enum hal_encrypt_type enctype)
 {
 	switch (enctype) {
 	case HAL_ENCRYPT_TYPE_OPEN:
diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.h b/drivers/net/wireless/ath/ath12k/dp_rx.h
index 88e42365a9d8..78c0800e06aa 100644
--- a/drivers/net/wireless/ath/ath12k/dp_rx.h
+++ b/drivers/net/wireless/ath/ath12k/dp_rx.h
@@ -145,4 +145,6 @@ int ath12k_dp_htt_tlv_iter(struct ath12k_base *ab, const void *ptr, size_t len,
 			   int (*iter)(struct ath12k_base *ar, u16 tag, u16 len,
 				       const void *ptr, void *data),
 			   void *data);
+int ath12k_dp_rx_crypto_mic_len(struct ath12k *ar, enum hal_encrypt_type enctype);
+
 #endif /* ATH12K_DP_RX_H */
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index b19e30d95560..a1e5d94b400d 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -4677,8 +4677,8 @@ static int ath12k_install_key(struct ath12k_link_vif *arvif,
 
 	switch (key->cipher) {
 	case WLAN_CIPHER_SUITE_CCMP:
+	case WLAN_CIPHER_SUITE_CCMP_256:
 		arg.key_cipher = WMI_CIPHER_AES_CCM;
-		/* TODO: Re-check if flag is valid */
 		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV_MGMT;
 		break;
 	case WLAN_CIPHER_SUITE_TKIP:
@@ -4686,12 +4686,10 @@ static int ath12k_install_key(struct ath12k_link_vif *arvif,
 		arg.key_txmic_len = 8;
 		arg.key_rxmic_len = 8;
 		break;
-	case WLAN_CIPHER_SUITE_CCMP_256:
-		arg.key_cipher = WMI_CIPHER_AES_CCM;
-		break;
 	case WLAN_CIPHER_SUITE_GCMP:
 	case WLAN_CIPHER_SUITE_GCMP_256:
 		arg.key_cipher = WMI_CIPHER_AES_GCM;
+		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV_MGMT;
 		break;
 	default:
 		ath12k_warn(ar->ab, "cipher %d is not supported\n", key->cipher);
@@ -7098,6 +7096,8 @@ static int ath12k_mac_mgmt_tx_wmi(struct ath12k *ar, struct ath12k_link_vif *arv
 	struct ath12k_base *ab = ar->ab;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_tx_info *info;
+	enum hal_encrypt_type enctype;
+	unsigned int mic_len;
 	dma_addr_t paddr;
 	int buf_id;
 	int ret;
@@ -7113,12 +7113,16 @@ static int ath12k_mac_mgmt_tx_wmi(struct ath12k *ar, struct ath12k_link_vif *arv
 		return -ENOSPC;
 
 	info = IEEE80211_SKB_CB(skb);
-	if (!(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP)) {
+	if ((ATH12K_SKB_CB(skb)->flags & ATH12K_SKB_CIPHER_SET) &&
+	    !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP)) {
 		if ((ieee80211_is_action(hdr->frame_control) ||
 		     ieee80211_is_deauth(hdr->frame_control) ||
 		     ieee80211_is_disassoc(hdr->frame_control)) &&
 		     ieee80211_has_protected(hdr->frame_control)) {
-			skb_put(skb, IEEE80211_CCMP_MIC_LEN);
+			enctype =
+			    ath12k_dp_tx_get_encrypt_type(ATH12K_SKB_CB(skb)->cipher);
+			mic_len = ath12k_dp_rx_crypto_mic_len(ar, enctype);
+			skb_put(skb, mic_len);
 		}
 	}
 
-- 
2.34.1



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

* [PATCH ath-next 2/2] wifi: ath12k: avoid multiple skb_cb fetch in ath12k_mac_mgmt_tx_wmi()
  2025-04-06  9:22 [PATCH ath-next 0/2] wifi: ath12k: set proper key flags and MIC space for CCMP256 and GCMP ciphers Rameshkumar Sundaram
  2025-04-06  9:22 ` [PATCH ath-next 1/2] wifi: ath12k: fix wrong handling of " Rameshkumar Sundaram
@ 2025-04-06  9:22 ` Rameshkumar Sundaram
  2025-04-09  6:49   ` Vasanthakumar Thiagarajan
  1 sibling, 1 reply; 5+ messages in thread
From: Rameshkumar Sundaram @ 2025-04-06  9:22 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Rameshkumar Sundaram

ath12k_mac_mgmt_tx_wmi() fetches ath12k's skb_cb space multiple times from
TX skb which is redundant operation. Save the skb_cb in a local pointer
and use the same instead.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index a1e5d94b400d..0fa5c9df19b2 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -7095,6 +7095,7 @@ static int ath12k_mac_mgmt_tx_wmi(struct ath12k *ar, struct ath12k_link_vif *arv
 {
 	struct ath12k_base *ab = ar->ab;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	struct ath12k_skb_cb *skb_cb = ATH12K_SKB_CB(skb);
 	struct ieee80211_tx_info *info;
 	enum hal_encrypt_type enctype;
 	unsigned int mic_len;
@@ -7104,7 +7105,7 @@ static int ath12k_mac_mgmt_tx_wmi(struct ath12k *ar, struct ath12k_link_vif *arv
 
 	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
-	ATH12K_SKB_CB(skb)->ar = ar;
+	skb_cb->ar = ar;
 	spin_lock_bh(&ar->txmgmt_idr_lock);
 	buf_id = idr_alloc(&ar->txmgmt_idr, skb, 0,
 			   ATH12K_TX_MGMT_NUM_PENDING_MAX, GFP_ATOMIC);
@@ -7113,14 +7114,13 @@ static int ath12k_mac_mgmt_tx_wmi(struct ath12k *ar, struct ath12k_link_vif *arv
 		return -ENOSPC;
 
 	info = IEEE80211_SKB_CB(skb);
-	if ((ATH12K_SKB_CB(skb)->flags & ATH12K_SKB_CIPHER_SET) &&
+	if ((skb_cb->flags & ATH12K_SKB_CIPHER_SET) &&
 	    !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP)) {
 		if ((ieee80211_is_action(hdr->frame_control) ||
 		     ieee80211_is_deauth(hdr->frame_control) ||
 		     ieee80211_is_disassoc(hdr->frame_control)) &&
 		     ieee80211_has_protected(hdr->frame_control)) {
-			enctype =
-			    ath12k_dp_tx_get_encrypt_type(ATH12K_SKB_CB(skb)->cipher);
+			enctype = ath12k_dp_tx_get_encrypt_type(skb_cb->cipher);
 			mic_len = ath12k_dp_rx_crypto_mic_len(ar, enctype);
 			skb_put(skb, mic_len);
 		}
@@ -7133,7 +7133,7 @@ static int ath12k_mac_mgmt_tx_wmi(struct ath12k *ar, struct ath12k_link_vif *arv
 		goto err_free_idr;
 	}
 
-	ATH12K_SKB_CB(skb)->paddr = paddr;
+	skb_cb->paddr = paddr;
 
 	ret = ath12k_wmi_mgmt_send(ar, arvif->vdev_id, buf_id, skb);
 	if (ret) {
@@ -7144,7 +7144,7 @@ static int ath12k_mac_mgmt_tx_wmi(struct ath12k *ar, struct ath12k_link_vif *arv
 	return 0;
 
 err_unmap_buf:
-	dma_unmap_single(ab->dev, ATH12K_SKB_CB(skb)->paddr,
+	dma_unmap_single(ab->dev, skb_cb->paddr,
 			 skb->len, DMA_TO_DEVICE);
 err_free_idr:
 	spin_lock_bh(&ar->txmgmt_idr_lock);
-- 
2.34.1



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

* Re: [PATCH ath-next 1/2] wifi: ath12k: fix wrong handling of CCMP256 and GCMP ciphers
  2025-04-06  9:22 ` [PATCH ath-next 1/2] wifi: ath12k: fix wrong handling of " Rameshkumar Sundaram
@ 2025-04-09  6:49   ` Vasanthakumar Thiagarajan
  0 siblings, 0 replies; 5+ messages in thread
From: Vasanthakumar Thiagarajan @ 2025-04-09  6:49 UTC (permalink / raw)
  To: Rameshkumar Sundaram, ath12k; +Cc: linux-wireless



On 4/6/2025 2:52 PM, Rameshkumar Sundaram wrote:
> Currently for CCMP256, GCMP128 and GCMP256 ciphers, in
> ath12k_install_key() IEEE80211_KEY_FLAG_GENERATE_IV_MGMT is not set and
> in ath12k_mac_mgmt_tx_wmi() a length of IEEE80211_CCMP_MIC_LEN is reserved
> for all ciphers.
> 
> This results in unexpected drop of protected management frames in case
> either of above 3 ciphers is used. The reason is, without
> IEEE80211_KEY_FLAG_GENERATE_IV_MGMT set, mac80211 will not generate
> CCMP/GCMP headers in TX frame for ath12k.
> Also MIC length reserved is wrong and such frames are dropped by hardware.
> 
> Fix this by setting IEEE80211_KEY_FLAG_GENERATE_IV_MGMT flag for above
> ciphers and by reserving proper MIC length for those ciphers.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>

Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>


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

* Re: [PATCH ath-next 2/2] wifi: ath12k: avoid multiple skb_cb fetch in ath12k_mac_mgmt_tx_wmi()
  2025-04-06  9:22 ` [PATCH ath-next 2/2] wifi: ath12k: avoid multiple skb_cb fetch in ath12k_mac_mgmt_tx_wmi() Rameshkumar Sundaram
@ 2025-04-09  6:49   ` Vasanthakumar Thiagarajan
  0 siblings, 0 replies; 5+ messages in thread
From: Vasanthakumar Thiagarajan @ 2025-04-09  6:49 UTC (permalink / raw)
  To: Rameshkumar Sundaram, ath12k; +Cc: linux-wireless



On 4/6/2025 2:52 PM, Rameshkumar Sundaram wrote:
> ath12k_mac_mgmt_tx_wmi() fetches ath12k's skb_cb space multiple times from
> TX skb which is redundant operation. Save the skb_cb in a local pointer
> and use the same instead.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>

Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>


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

end of thread, other threads:[~2025-04-09  6:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-06  9:22 [PATCH ath-next 0/2] wifi: ath12k: set proper key flags and MIC space for CCMP256 and GCMP ciphers Rameshkumar Sundaram
2025-04-06  9:22 ` [PATCH ath-next 1/2] wifi: ath12k: fix wrong handling of " Rameshkumar Sundaram
2025-04-09  6:49   ` Vasanthakumar Thiagarajan
2025-04-06  9:22 ` [PATCH ath-next 2/2] wifi: ath12k: avoid multiple skb_cb fetch in ath12k_mac_mgmt_tx_wmi() Rameshkumar Sundaram
2025-04-09  6:49   ` Vasanthakumar Thiagarajan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox