* [PATCH wireless-next v2] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
@ 2026-02-08 8:18 Sai Pratyusha Magam
2026-02-09 11:27 ` Johannes Berg
0 siblings, 1 reply; 5+ messages in thread
From: Sai Pratyusha Magam @ 2026-02-08 8:18 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, sai.magam, quic_drohan
Per IEEE Std 802.11be-2024, 12.5.2.3.3, if the MPDU is an
individually addressed Data frame between an AP MLD and a
non-AP MLD associated with the AP MLD, then A1/A2/A3
will be MLD MAC addresses. Otherwise, Al/A2/A3 will be
over-the-air link MAC addresses.
Currently, during AAD and Nonce computation for software based
encryption/decryption cases, mac80211 directly uses the addresses it
receives in the skb frame header. However, after the first
authentication, management frame addresses for non-AP MLD stations
are translated to MLD addresses from over the air link addresses in
software. This means that the skb header could contain translated MLD
addresses, which when used as is, can lead to incorrect AAD/Nonce
computation.
In the following manner, ensure that the right set of addresses are used:
In the receive path, stash the pre-translated link addresses in
ieee80211_rx_data and use them for the AAD/Nonce computations in the
decrypt path for robust management frames (MFP) and (Re)Association
Request/Response frames that require encryption (EPP)
In the transmit path, offload the encryption of robust
management frames (MFP) and (Re)Association Request/Response
frames that require encryption (EPP) to the hwsim driver that
can then ensure that encryption and hence the AAD/Nonce computations
are performed on the frame containing the link addresses.
To do so, register the set key handler in hwsim driver so mac80211 is
aware that it is the driver that would take care of encrypting the
frame. The key flag IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD selectively
offloads only encryption of management frames, while keeping the
encryption for data frames and MMIE generation for a AES_CMAC or a
AES_GMAC key still at the SW crypto in mac layer
The hybrid way of having both HW/driver based crypto for certain frames
and SW crypto for others require that the fast-xmit path be skipped
when the above mentioned key flag is set, since data frames still rely
on software based crypto and also that sufficient tailroom is always
reserved in the skb for ICV/MIC addition.
Co-developed-by: Rohan Dutta <quic_drohan@quicinc.com>
Signed-off-by: Rohan Dutta <quic_drohan@quicinc.com>
Signed-off-by: Sai Pratyusha Magam <sai.magam@oss.qualcomm.com>
---
v1 -> v2:
Removed the additional address translation to link addresses approach
in the encrypt/decrypt path for management frames and instead the rx
path uses the stashed pre-translated link addresses for decryption and
in the tx path, encryption for mgmt frames is offloaded to hwsim driver
drivers/net/wireless/virtual/mac80211_hwsim.c | 19 ++++++
include/net/mac80211.h | 11 ++++
net/mac80211/cfg.c | 4 +-
net/mac80211/ieee80211_i.h | 15 +++++
net/mac80211/rx.c | 3 +
net/mac80211/tx.c | 60 ++++++++++++++++++-
net/mac80211/wpa.c | 57 ++++++++++++++----
7 files changed, 153 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
index 4d9f5f87e814..9a364c6774a5 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
@@ -1955,6 +1955,16 @@ mac80211_hwsim_select_tx_link(struct mac80211_hwsim_data *data,
return NULL;
}
+static int mac80211_hwsim_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta,
+ struct ieee80211_key_conf *key)
+{
+ key->flags |= IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD |
+ IEEE80211_KEY_FLAG_RESERVE_TAILROOM;
+ return 0;
+}
+
static void mac80211_hwsim_tx(struct ieee80211_hw *hw,
struct ieee80211_tx_control *control,
struct sk_buff *skb)
@@ -2060,6 +2070,13 @@ static void mac80211_hwsim_tx(struct ieee80211_hw *hw,
}
}
+ if (ieee80211_encrypt_tx_skb(skb) < 0) {
+ ieee80211_free_txskb(hw, skb);
+ return;
+ }
+ /* re-assign hdr since skb data may have shifted after encryption */
+ hdr = (void *)skb->data;
+
if (WARN(!channel, "TX w/o channel - queue = %d\n", txi->hw_queue)) {
ieee80211_free_txskb(hw, skb);
return;
@@ -4189,6 +4206,7 @@ static int mac80211_hwsim_change_nan_config(struct ieee80211_hw *hw,
.start_nan = mac80211_hwsim_start_nan, \
.stop_nan = mac80211_hwsim_stop_nan, \
.nan_change_conf = mac80211_hwsim_change_nan_config, \
+ .set_key = mac80211_hwsim_set_key, \
HWSIM_DEBUGFS_OPS
#define HWSIM_NON_MLO_OPS \
@@ -5621,6 +5639,7 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
WIPHY_FLAG_AP_UAPSD |
WIPHY_FLAG_SUPPORTS_5_10_MHZ |
WIPHY_FLAG_HAS_CHANNEL_SWITCH;
+ hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
hw->wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR |
NL80211_FEATURE_AP_MODE_CHAN_WIDTH_CHANGE |
NL80211_FEATURE_STATIC_SMPS |
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 7f9d96939a4e..69530aa913be 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2242,6 +2242,9 @@ static inline bool lockdep_vif_wiphy_mutex_held(struct ieee80211_vif *vif)
* number generation only
* @IEEE80211_KEY_FLAG_SPP_AMSDU: SPP A-MSDUs can be used with this key
* (set by mac80211 from the sta->spp_amsdu flag)
+ * @IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD: This flag should be set by
+ * the driver for a key if encryption of only management frames (MFP)
+ * is offloaded to the driver
*/
enum ieee80211_key_flags {
IEEE80211_KEY_FLAG_GENERATE_IV_MGMT = BIT(0),
@@ -2256,6 +2259,7 @@ enum ieee80211_key_flags {
IEEE80211_KEY_FLAG_NO_AUTO_TX = BIT(9),
IEEE80211_KEY_FLAG_GENERATE_MMIE = BIT(10),
IEEE80211_KEY_FLAG_SPP_AMSDU = BIT(11),
+ IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD = BIT(12),
};
/**
@@ -7962,4 +7966,11 @@ int ieee80211_emulate_switch_vif_chanctx(struct ieee80211_hw *hw,
* Return: %true iff the vif is a NAN interface and NAN is started
*/
bool ieee80211_vif_nan_started(struct ieee80211_vif *vif);
+
+/**
+ * ieee80211_encrypt_tx_skb - Encrypt the transmit skb
+ * @skb: the skb
+ * Return: 0 if success and non-zero on error
+ */
+int ieee80211_encrypt_tx_skb(struct sk_buff *skb);
#endif /* MAC80211_H */
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5d04d7d550b0..d48a53ebb98f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -885,7 +885,9 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct net_device *dev,
offsetof(typeof(kseq), gcmp));
if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE &&
- !(key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
+ !(key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
+ !(key->conf.flags &
+ IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD)) {
drv_get_key_seq(sdata->local, key, &kseq);
memcpy(seq, kseq.ccmp.pn, 6);
} else {
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e60b814dd89e..5fe561fd09f2 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -256,6 +256,8 @@ struct ieee80211_rx_data {
u8 pn[IEEE80211_CCMP_PN_LEN];
} ccm_gcm;
};
+
+ u8 link_addrs[3 * ETH_ALEN];
};
struct ieee80211_csa_settings {
@@ -2404,6 +2406,19 @@ static inline bool ieee80211_require_encrypted_assoc(__le16 fc,
ieee80211_is_assoc_resp(fc) || ieee80211_is_reassoc_resp(fc)));
}
+static inline bool ieee80211_require_sw_tx_enc(__le16 fc, u16 flags,
+ struct sta_info *sta,
+ struct sk_buff *skb)
+{
+ struct ieee80211_hdr *hdr = (void *)skb->data;
+ bool unicast_robust_mgmt = is_unicast_ether_addr(hdr->addr1) &&
+ ieee80211_is_robust_mgmt_frame(skb);
+
+ return ((flags & IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD) &&
+ !unicast_robust_mgmt &&
+ !ieee80211_require_encrypted_assoc(fc, sta));
+}
+
/* sta_out needs to be checked for ERR_PTR() before using */
int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 11d6c56c9d7e..e8eb38cbafc6 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -5127,6 +5127,9 @@ static bool ieee80211_prepare_and_rx_handle(struct ieee80211_rx_data *rx,
hdr = (struct ieee80211_hdr *)rx->skb->data;
}
+ /* Store a copy of the pre-translated link addresses */
+ memcpy(rx->link_addrs, &hdr->addrs, 3 * ETH_ALEN);
+
if (unlikely(rx->sta && rx->sta->sta.mlo) &&
is_unicast_ether_addr(hdr->addr1) &&
!ieee80211_is_probe_resp(hdr->frame_control) &&
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 007f5a368d41..ca756868594d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -632,6 +632,11 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
case WLAN_CIPHER_SUITE_TKIP:
if (!ieee80211_is_data_present(hdr->frame_control))
tx->key = NULL;
+ else
+ skip_hw = ieee80211_require_sw_tx_enc(hdr->frame_control,
+ tx->key->conf.flags,
+ tx->sta,
+ tx->skb);
break;
case WLAN_CIPHER_SUITE_CCMP:
case WLAN_CIPHER_SUITE_CCMP_256:
@@ -645,9 +650,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
tx->sta))
tx->key = NULL;
else
- skip_hw = (tx->key->conf.flags &
- IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
- ieee80211_is_mgmt(hdr->frame_control);
+ skip_hw = ((tx->key->conf.flags &
+ IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
+ ieee80211_is_mgmt(hdr->frame_control)) ||
+ ieee80211_require_sw_tx_enc(hdr->frame_control,
+ tx->key->conf.flags,
+ tx->sta,
+ tx->skb);
break;
case WLAN_CIPHER_SUITE_AES_CMAC:
case WLAN_CIPHER_SUITE_BIP_CMAC_256:
@@ -655,6 +664,11 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
case WLAN_CIPHER_SUITE_BIP_GMAC_256:
if (!ieee80211_is_mgmt(hdr->frame_control))
tx->key = NULL;
+ else
+ skip_hw = ieee80211_require_sw_tx_enc(hdr->frame_control,
+ tx->key->conf.flags,
+ tx->sta,
+ tx->skb);
break;
}
@@ -3205,6 +3219,13 @@ void ieee80211_check_fast_xmit(struct sta_info *sta)
if (!(build.key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE))
goto out;
+ /* Only management frame encryption is offloaded to the driver,
+ * So skip fast-xmit
+ */
+ if (build.key->conf.flags &
+ IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD)
+ goto out;
+
/* Key is being removed */
if (build.key->flags & KEY_FLAG_TAINTED)
goto out;
@@ -5299,6 +5320,7 @@ static int ieee80211_beacon_protect(struct sk_buff *skb,
}
if (!(tx.key->conf.flags & IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
+ !(tx.key->conf.flags & IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD) &&
tx.key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
IEEE80211_SKB_CB(skb)->control.hw_key = &tx.key->conf;
@@ -5316,6 +5338,38 @@ static int ieee80211_beacon_protect(struct sk_buff *skb,
return 0;
}
+int ieee80211_encrypt_tx_skb(struct sk_buff *skb)
+{
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ struct ieee80211_sub_if_data *sdata;
+ struct sk_buff *check_skb;
+ struct ieee80211_tx_data tx;
+ ieee80211_tx_result res;
+
+ if (!info->control.hw_key)
+ return 0;
+
+ memset(&tx, 0, sizeof(tx));
+ tx.key = container_of(info->control.hw_key, struct ieee80211_key, conf);
+ /* NULL it out now so we do full SW crypto */
+ info->control.hw_key = NULL;
+ __skb_queue_head_init(&tx.skbs);
+ __skb_queue_tail(&tx.skbs, skb);
+
+ sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev);
+ tx.sdata = sdata;
+ tx.local = sdata->local;
+ res = ieee80211_tx_h_encrypt(&tx);
+ check_skb = __skb_dequeue(&tx.skbs);
+ /* we may crash after this, but it'd be a bug in crypto */
+ WARN_ON(check_skb != skb);
+ if (WARN_ON_ONCE(res != TX_CONTINUE))
+ return -EINVAL;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ieee80211_encrypt_tx_skb);
+
static void
ieee80211_beacon_get_finish(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index fdf98c21d32c..9d10e82fcf1c 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -315,7 +315,8 @@ ieee80211_crypto_tkip_decrypt(struct ieee80211_rx_data *rx)
* Calculate AAD for CCMP/GCMP, returning qos_tid since we
* need that in CCMP also for b_0.
*/
-static u8 ccmp_gcmp_aad(struct sk_buff *skb, u8 *aad, bool spp_amsdu)
+static u8 ccmp_gcmp_aad(struct sk_buff *skb, u8 *aad, bool spp_amsdu,
+ bool aad_nonce_computed)
{
struct ieee80211_hdr *hdr = (void *)skb->data;
__le16 mask_fc;
@@ -358,7 +359,8 @@ static u8 ccmp_gcmp_aad(struct sk_buff *skb, u8 *aad, bool spp_amsdu)
* FC | A1 | A2 | A3 | SC | [A4] | [QC] */
put_unaligned_be16(len_a, &aad[0]);
put_unaligned(mask_fc, (__le16 *)&aad[2]);
- memcpy(&aad[4], &hdr->addrs, 3 * ETH_ALEN);
+ if (!aad_nonce_computed)
+ memcpy(&aad[4], &hdr->addrs, 3 * ETH_ALEN);
/* Mask Seq#, leave Frag# */
aad[22] = *((u8 *) &hdr->seq_ctrl) & 0x0f;
@@ -377,10 +379,10 @@ static u8 ccmp_gcmp_aad(struct sk_buff *skb, u8 *aad, bool spp_amsdu)
}
static void ccmp_special_blocks(struct sk_buff *skb, u8 *pn, u8 *b_0, u8 *aad,
- bool spp_amsdu)
+ bool spp_amsdu, bool aad_nonce_computed)
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
- u8 qos_tid = ccmp_gcmp_aad(skb, aad, spp_amsdu);
+ u8 qos_tid = ccmp_gcmp_aad(skb, aad, spp_amsdu, aad_nonce_computed);
/* In CCM, the initial vectors (IV) used for CTR mode encryption and CBC
* mode authentication are not allowed to collide, yet both are derived
@@ -395,7 +397,8 @@ static void ccmp_special_blocks(struct sk_buff *skb, u8 *pn, u8 *b_0, u8 *aad,
* Nonce Flags: Priority (b0..b3) | Management (b4) | Reserved (b5..b7)
*/
b_0[1] = qos_tid | (ieee80211_is_mgmt(hdr->frame_control) << 4);
- memcpy(&b_0[2], hdr->addr2, ETH_ALEN);
+ if (!aad_nonce_computed)
+ memcpy(&b_0[2], hdr->addr2, ETH_ALEN);
memcpy(&b_0[8], pn, IEEE80211_CCMP_PN_LEN);
}
@@ -488,7 +491,8 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
pos += IEEE80211_CCMP_HDR_LEN;
ccmp_special_blocks(skb, pn, b_0, aad,
- key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU);
+ key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU,
+ false);
return ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, b_0, aad, pos, len,
skb_put(skb, mic_len));
}
@@ -566,9 +570,23 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
if (!(status->flag & RX_FLAG_DECRYPTED)) {
u8 aad[2 * AES_BLOCK_SIZE];
u8 b_0[AES_BLOCK_SIZE];
+ bool aad_nonce_computed = false;
+
+ if (ieee80211_is_robust_mgmt_frame(skb) ||
+ ieee80211_require_encrypted_assoc(hdr->frame_control,
+ rx->sta)) {
+ /* AAD computation */
+ memcpy(&aad[4], rx->link_addrs, 3 * ETH_ALEN);
+ /* Nonce computation */
+ ether_addr_copy(&b_0[2],
+ &rx->link_addrs[ETH_ALEN]);
+ aad_nonce_computed = true;
+ }
+
/* hardware didn't decrypt/verify MIC */
ccmp_special_blocks(skb, pn, b_0, aad,
- key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU);
+ key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU,
+ aad_nonce_computed);
if (ieee80211_aes_ccm_decrypt(
key->u.ccmp.tfm, b_0, aad,
@@ -593,14 +611,15 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
}
static void gcmp_special_blocks(struct sk_buff *skb, u8 *pn, u8 *j_0, u8 *aad,
- bool spp_amsdu)
+ bool spp_amsdu, bool aad_nonce_computed)
{
struct ieee80211_hdr *hdr = (void *)skb->data;
- memcpy(j_0, hdr->addr2, ETH_ALEN);
+ if (!aad_nonce_computed)
+ memcpy(j_0, hdr->addr2, ETH_ALEN);
memcpy(&j_0[ETH_ALEN], pn, IEEE80211_GCMP_PN_LEN);
- ccmp_gcmp_aad(skb, aad, spp_amsdu);
+ ccmp_gcmp_aad(skb, aad, spp_amsdu, aad_nonce_computed);
}
static inline void gcmp_pn2hdr(u8 *hdr, const u8 *pn, int key_id)
@@ -690,7 +709,8 @@ static int gcmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
pos += IEEE80211_GCMP_HDR_LEN;
gcmp_special_blocks(skb, pn, j_0, aad,
- key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU);
+ key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU,
+ false);
return ieee80211_aes_gcm_encrypt(key->u.gcmp.tfm, j_0, aad, pos, len,
skb_put(skb, IEEE80211_GCMP_MIC_LEN));
}
@@ -763,9 +783,22 @@ ieee80211_crypto_gcmp_decrypt(struct ieee80211_rx_data *rx)
if (!(status->flag & RX_FLAG_DECRYPTED)) {
u8 aad[2 * AES_BLOCK_SIZE];
u8 j_0[AES_BLOCK_SIZE];
+ bool aad_nonce_computed = false;
+
+ if (ieee80211_is_robust_mgmt_frame(skb) ||
+ ieee80211_require_encrypted_assoc(hdr->frame_control,
+ rx->sta)) {
+ /* AAD computation */
+ memcpy(&aad[4], rx->link_addrs, 3 * ETH_ALEN);
+ /* Nonce computation */
+ ether_addr_copy(&j_0[0],
+ &rx->link_addrs[ETH_ALEN]);
+ aad_nonce_computed = true;
+ }
/* hardware didn't decrypt/verify MIC */
gcmp_special_blocks(skb, pn, j_0, aad,
- key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU);
+ key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU,
+ aad_nonce_computed);
if (ieee80211_aes_gcm_decrypt(
key->u.gcmp.tfm, j_0, aad,
base-commit: 333225e1e9ead7b06e5363389403bdac72ba3046
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH wireless-next v2] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-02-08 8:18 [PATCH wireless-next v2] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO Sai Pratyusha Magam
@ 2026-02-09 11:27 ` Johannes Berg
2026-02-09 11:50 ` Johannes Berg
2026-02-10 13:00 ` Sai Pratyusha Magam
0 siblings, 2 replies; 5+ messages in thread
From: Johannes Berg @ 2026-02-09 11:27 UTC (permalink / raw)
To: Sai Pratyusha Magam; +Cc: linux-wireless, quic_drohan
Hi,
So that looks like a huge improvement to me, nice! :-)
On Sun, 2026-02-08 at 13:48 +0530, Sai Pratyusha Magam wrote:
> The hybrid way of having both HW/driver based crypto for certain frames
> and SW crypto for others require that the fast-xmit path be skipped
> when the above mentioned key flag is set, since data frames still rely
> on software based crypto
I can understand this sentence in the sense of "A => B", and it
describes why B follows from A, but ... why do you even need A? I.e. why
do you want to do this for management frames only, it seems the encrypt
function and having hwsim worry about it should work just as well for
data frames?
If I restrict hwsim [1] to only pairwise and CCMP/GCMP (clearly it
doesn't handle TKIP due to Michael MIC, WEP is irrelevant, BIGTK isn't
handled in hwsim), then it mostly works without all of that extra logic
for me for the tests. Maybe need to think about IGTK here though, some
tests are still failing.
[1] https://p.sipsolutions.net/d101bc31582cadcf.txt
But if we do that (even if I don't like it, I'd rather fix the remaining
issues), then you need to document reason for it.
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH wireless-next v2] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-02-09 11:27 ` Johannes Berg
@ 2026-02-09 11:50 ` Johannes Berg
2026-02-10 13:00 ` Sai Pratyusha Magam
1 sibling, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2026-02-09 11:50 UTC (permalink / raw)
To: Sai Pratyusha Magam; +Cc: linux-wireless, quic_drohan
On Mon, 2026-02-09 at 12:27 +0100, Johannes Berg wrote:
>
> If I restrict hwsim [1] to only pairwise and CCMP/GCMP (clearly it
> doesn't handle TKIP due to Michael MIC, WEP is irrelevant, BIGTK isn't
> handled in hwsim), then it mostly works without all of that extra logic
> for me for the tests. Maybe need to think about IGTK here though, some
> tests are still failing.
>
> [1] https://p.sipsolutions.net/d101bc31582cadcf.txt
Oh I guess that should be 'return 1' and hwsim would have to call the
encrypt for data frames before address translation ... But the more we
can push into hwsim, the better IMHO.
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH wireless-next v2] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-02-09 11:27 ` Johannes Berg
2026-02-09 11:50 ` Johannes Berg
@ 2026-02-10 13:00 ` Sai Pratyusha Magam
2026-02-10 13:54 ` Johannes Berg
1 sibling, 1 reply; 5+ messages in thread
From: Sai Pratyusha Magam @ 2026-02-10 13:00 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, quic_drohan
On 2/9/2026 4:57 PM, Johannes Berg wrote:
> Hi,
>
> So that looks like a huge improvement to me, nice! :-)
>
> On Sun, 2026-02-08 at 13:48 +0530, Sai Pratyusha Magam wrote:
>> The hybrid way of having both HW/driver based crypto for certain frames
>> and SW crypto for others require that the fast-xmit path be skipped
>> when the above mentioned key flag is set, since data frames still rely
>> on software based crypto
>
> I can understand this sentence in the sense of "A => B", and it
> describes why B follows from A, but ... why do you even need A? I.e. why
> do you want to do this for management frames only, it seems the encrypt
> function and having hwsim worry about it should work just as well for
> data frames?
>
> If I restrict hwsim [1] to only pairwise and CCMP/GCMP (clearly it
> doesn't handle TKIP due to Michael MIC, WEP is irrelevant, BIGTK isn't
> handled in hwsim), then it mostly works without all of that extra logic
> for me for the tests. Maybe need to think about IGTK here though, some
> tests are still failing.
>
> [1] https://p.sipsolutions.net/d101bc31582cadcf.txt
>
Hi Johannes, Thank you for [1], yes the extra logic I had added
(IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD) to prevent offload for
AES-CMAC/AES-GMAC, i.e, MMIE addition for beacons(BIGTK) and broadcast
robust management frames(IGTK) and for WEP/TKIP cases was not needed and
the correct return value in the set_key ops was all that was needed to
selectively offload encryption for CCMP/GCMP. I missed that.
Today, I tried with [1] , removed the redundant logic around the
additional key flag this patch
introduced(IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD) and data tx
encryption offload, i.e ieee80211_encrypt_tx_skb() before address
translation.
I was able to pass the existing hwsim test cases that include test cases
for beacon protection, broadcast robust mgmt, WEP/TKIP, unicast/bcast
data and encrypted Assoc (EPP)
>
> But if we do that (even if I don't like it, I'd rather fix the remaining
> issues), then you need to document reason for it.
>
sorry, I did not follow this?
> johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH wireless-next v2] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-02-10 13:00 ` Sai Pratyusha Magam
@ 2026-02-10 13:54 ` Johannes Berg
0 siblings, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2026-02-10 13:54 UTC (permalink / raw)
To: Sai Pratyusha Magam; +Cc: linux-wireless, quic_drohan
On Tue, 2026-02-10 at 18:30 +0530, Sai Pratyusha Magam wrote:
>
> > If I restrict hwsim [1] to only pairwise and CCMP/GCMP (clearly it
> > doesn't handle TKIP due to Michael MIC, WEP is irrelevant, BIGTK isn't
> > handled in hwsim), then it mostly works without all of that extra logic
> > for me for the tests. Maybe need to think about IGTK here though, some
> > tests are still failing.
> >
> > [1] https://p.sipsolutions.net/d101bc31582cadcf.txt
> >
> Hi Johannes, Thank you for [1], yes the extra logic I had added
> (IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD) to prevent offload for
> AES-CMAC/AES-GMAC, i.e, MMIE addition for beacons(BIGTK) and broadcast
> robust management frames(IGTK) and for WEP/TKIP cases was not needed and
> the correct return value in the set_key ops was all that was needed to
> selectively offload encryption for CCMP/GCMP. I missed that.
Well, you couldn't do management frames only, so in that sense the flag
IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD was needed - at minimum hwsim
also needs/needed to invoke the encrypt_tx_skb() function before address
translation for data frames.
> Today, I tried with [1] , removed the redundant logic around the
> additional key flag this patch
> introduced(IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD) and data tx
> encryption offload, i.e ieee80211_encrypt_tx_skb() before address
> translation.
>
> I was able to pass the existing hwsim test cases that include test cases
> for beacon protection, broadcast robust mgmt, WEP/TKIP, unicast/bcast
> data and encrypted Assoc (EPP)
OK cool, so maybe I did something wrong in my quick attempt to figure
out if we could get away without IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD.
> > But if we do that (even if I don't like it, I'd rather fix the remaining
> > issues), then you need to document reason for it.
> >
> sorry, I did not follow this?
Sounds like we don't need IEEE80211_KEY_FLAG_MGMT_TX_ENC_OFFLOAD now,
but I meant that if we _do_ need it, the commit message should explain
why exactly.
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-10 13:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-08 8:18 [PATCH wireless-next v2] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO Sai Pratyusha Magam
2026-02-09 11:27 ` Johannes Berg
2026-02-09 11:50 ` Johannes Berg
2026-02-10 13:00 ` Sai Pratyusha Magam
2026-02-10 13:54 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox