* [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
@ 2025-12-11 12:36 Sai Pratyusha Magam
2026-01-08 12:52 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Sai Pratyusha Magam @ 2025-12-11 12:36 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, Sai Pratyusha Magam, Rohan Dutta
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.
For management frames, ensure that the AAD/Nonce computation is solely
using the cached link addresses, avoiding dependence on potentially
translated headers. To achieve this, add an additional layer of address
translation to link MAC addresses to be used in both encrypt/decrypt paths.
For data frames, the existing behavior remains unchanged.
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>
---
net/mac80211/wpa.c | 167 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 155 insertions(+), 12 deletions(-)
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 4a858112e4ef..3f73c6286f03 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -311,11 +311,110 @@ ieee80211_crypto_tkip_decrypt(struct ieee80211_rx_data *rx)
return RX_CONTINUE;
}
+static bool ccmp_gcmp_aad_nonce_addr_translate_rx(struct ieee80211_rx_data *rx,
+ u8 *aad, u8 *b_0, u32 cipher,
+ __le16 fc)
+{
+ struct ieee80211_link_data *link = rx->link;
+ struct ieee80211_bss_conf *bss_conf = link->conf;
+ struct link_sta_info *link_sta = rx->link_sta;
+ struct ieee80211_vif *vif = &rx->sdata->vif;
+
+ if (!rx->sta || !rx->sta->sta.mlo ||
+ (vif->type != NL80211_IFTYPE_AP &&
+ vif->type != NL80211_IFTYPE_STATION))
+ return false;
+
+ /* Address Translation for AAD computation */
+ ether_addr_copy(&aad[4], bss_conf->addr);
+ ether_addr_copy(&aad[4] + ETH_ALEN, link_sta->addr);
+
+ if (!ieee80211_has_tods(fc) && !ieee80211_has_fromds(fc)) {
+ if (vif->type == NL80211_IFTYPE_STATION && bss_conf->bssid)
+ ether_addr_copy(&aad[4] + 2 * ETH_ALEN,
+ bss_conf->bssid);
+ else if (vif->type == NL80211_IFTYPE_AP)
+ ether_addr_copy(&aad[4] + 2 * ETH_ALEN,
+ bss_conf->addr);
+ }
+
+ /* Address Translation for Nonce computation */
+ if (cipher == WLAN_CIPHER_SUITE_CCMP ||
+ cipher == WLAN_CIPHER_SUITE_CCMP_256)
+ ether_addr_copy(&b_0[2], link_sta->addr);
+ if (cipher == WLAN_CIPHER_SUITE_GCMP ||
+ cipher == WLAN_CIPHER_SUITE_GCMP_256)
+ ether_addr_copy(&b_0[0], link_sta->addr);
+ return true;
+}
+
+/* This function is called with the caller held under RCU Read Lock */
+static bool ccmp_gcmp_aad_nonce_addr_translate_tx(struct ieee80211_tx_data *tx,
+ struct ieee80211_tx_info *info,
+ u8 *aad, u8 *b_0,
+ u32 cipher, __le16 fc)
+{
+ struct ieee80211_vif *vif = info->control.vif;
+ struct sta_info *sta = tx->sta;
+ u8 link_id = u32_get_bits(info->control.flags,
+ IEEE80211_TX_CTRL_MLO_LINK);
+ struct ieee80211_bss_conf *bss_conf;
+ struct link_sta_info *link_sta;
+
+ if (!sta || !vif || !sta->sta.mlo ||
+ (vif->type != NL80211_IFTYPE_AP &&
+ vif->type != NL80211_IFTYPE_STATION))
+ return false;
+
+ /*
+ * When link_id is IEEE80211_LINK_UNSPECIFIED, use default link
+ * for AAD computations and update the control flags to encode
+ * the default link_id. This is to make sure that the driver
+ * also uses the same link to transmit the frame
+ */
+ if (link_id == IEEE80211_LINK_UNSPECIFIED) {
+ link_id = sta->deflink.link_id;
+ info->control.flags &= ~IEEE80211_TX_CTRL_MLO_LINK;
+ info->control.flags |=
+ u32_encode_bits(link_id, IEEE80211_TX_CTRL_MLO_LINK);
+ }
+
+ bss_conf = rcu_dereference(vif->link_conf[link_id]);
+ link_sta = rcu_dereference(sta->link[link_id]);
+
+ if (!bss_conf || !link_sta)
+ return false;
+
+ /* Address Translation for AAD computation */
+ ether_addr_copy(&aad[4], link_sta->addr);
+ ether_addr_copy(&aad[4] + ETH_ALEN, bss_conf->addr);
+
+ if (!ieee80211_has_tods(fc) && !ieee80211_has_fromds(fc)) {
+ if (vif->type == NL80211_IFTYPE_STATION && bss_conf->bssid)
+ ether_addr_copy(&aad[4] + 2 * ETH_ALEN,
+ bss_conf->bssid);
+ else if (vif->type == NL80211_IFTYPE_AP)
+ ether_addr_copy(&aad[4] + 2 * ETH_ALEN,
+ bss_conf->addr);
+ }
+
+ /* Address Translation for Nonce computation */
+ if (cipher == WLAN_CIPHER_SUITE_CCMP ||
+ cipher == WLAN_CIPHER_SUITE_CCMP_256)
+ ether_addr_copy(&b_0[2], bss_conf->addr);
+ if (cipher == WLAN_CIPHER_SUITE_GCMP ||
+ cipher == WLAN_CIPHER_SUITE_GCMP_256)
+ ether_addr_copy(&b_0[0], bss_conf->addr);
+
+ return true;
+}
+
/*
* 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 is_translated)
{
struct ieee80211_hdr *hdr = (void *)skb->data;
__le16 mask_fc;
@@ -358,7 +457,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 (!is_translated)
+ memcpy(&aad[4], &hdr->addrs, 3 * ETH_ALEN);
/* Mask Seq#, leave Frag# */
aad[22] = *((u8 *) &hdr->seq_ctrl) & 0x0f;
@@ -377,10 +477,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 is_translated)
{
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, is_translated);
/* 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 +495,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 (!is_translated)
+ memcpy(&b_0[2], hdr->addr2, ETH_ALEN);
memcpy(&b_0[8], pn, IEEE80211_CCMP_PN_LEN);
}
@@ -435,6 +536,8 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
u64 pn64;
u8 aad[CCM_AAD_LEN];
u8 b_0[AES_BLOCK_SIZE];
+ __le16 fc = hdr->frame_control;
+ bool is_translated = false;
if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
@@ -487,8 +590,15 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
return 0;
pos += IEEE80211_CCMP_HDR_LEN;
+
+ if (ieee80211_is_mgmt(fc))
+ is_translated = ccmp_gcmp_aad_nonce_addr_translate_tx(tx, info,
+ aad, b_0,
+ key->conf.cipher,
+ fc);
ccmp_special_blocks(skb, pn, b_0, aad,
- key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU);
+ key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU,
+ is_translated);
return ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, b_0, aad, pos, len,
skb_put(skb, mic_len));
}
@@ -565,9 +675,21 @@ 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 is_translated = false;
+ __le16 fc = hdr->frame_control;
+
+ if (ieee80211_is_mgmt(fc))
+ is_translated =
+ ccmp_gcmp_aad_nonce_addr_translate_rx(rx,
+ aad,
+ b_0,
+ key->conf.cipher,
+ fc);
+
/* 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,
+ is_translated);
if (ieee80211_aes_ccm_decrypt(
key->u.ccmp.tfm, b_0, aad,
@@ -592,14 +714,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 is_translated)
{
struct ieee80211_hdr *hdr = (void *)skb->data;
- memcpy(j_0, hdr->addr2, ETH_ALEN);
+ if (!is_translated)
+ 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, is_translated);
}
static inline void gcmp_pn2hdr(u8 *hdr, const u8 *pn, int key_id)
@@ -635,6 +758,8 @@ static int gcmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
u64 pn64;
u8 aad[GCM_AAD_LEN];
u8 j_0[AES_BLOCK_SIZE];
+ __le16 fc = hdr->frame_control;
+ bool is_translated = false;
if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
@@ -688,8 +813,15 @@ static int gcmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
return 0;
pos += IEEE80211_GCMP_HDR_LEN;
+
+ if (ieee80211_is_mgmt(fc))
+ is_translated = ccmp_gcmp_aad_nonce_addr_translate_tx(tx, info,
+ aad, j_0,
+ key->conf.cipher,
+ fc);
gcmp_special_blocks(skb, pn, j_0, aad,
- key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU);
+ key->conf.flags & IEEE80211_KEY_FLAG_SPP_AMSDU,
+ is_translated);
return ieee80211_aes_gcm_encrypt(key->u.gcmp.tfm, j_0, aad, pos, len,
skb_put(skb, IEEE80211_GCMP_MIC_LEN));
}
@@ -761,9 +893,20 @@ 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 is_translated = false;
+ __le16 fc = hdr->frame_control;
+
+ if (ieee80211_is_mgmt(fc))
+ is_translated =
+ ccmp_gcmp_aad_nonce_addr_translate_rx(rx,
+ aad,
+ j_0,
+ key->conf.cipher,
+ fc);
/* 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,
+ is_translated);
if (ieee80211_aes_gcm_decrypt(
key->u.gcmp.tfm, j_0, aad,
base-commit: f9e788c5fd3a23edecd808ebb354e2cb1aef87c3
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2025-12-11 12:36 [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO Sai Pratyusha Magam
@ 2026-01-08 12:52 ` Johannes Berg
2026-01-09 10:03 ` Sai Pratyusha Magam
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2026-01-08 12:52 UTC (permalink / raw)
To: Sai Pratyusha Magam; +Cc: linux-wireless, Rohan Dutta
On Thu, 2025-12-11 at 18:06 +0530, Sai Pratyusha Magam wrote:
> 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.
Make sense ... we translate them internally because of all the matching
to interfaces etc.
> net/mac80211/wpa.c | 167 +++++++++++++++++++++++++++++++++++++++++----
This seems rather complex.
I believe the translation generally happens in mac80211 itself, in
ieee80211_prepare_and_rx_handle, couldn't we stash away the 18 bytes of
the three addresses in struct ieee80211_rx_data and then just
*unconditionally* use those, instead of all this conditional logic?
I can see the value in putting something only into the SW crypto so it
doesn't affect anything else, but also the address rewriting that
happens is pretty rare, so maybe that means we also have a flag there if
using the original 18 bytes is needed or not, but that should all still
be far less complex?
johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-01-08 12:52 ` Johannes Berg
@ 2026-01-09 10:03 ` Sai Pratyusha Magam
2026-01-14 17:04 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Sai Pratyusha Magam @ 2026-01-09 10:03 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Rohan Dutta
On 1/8/2026 6:22 PM, Johannes Berg wrote:
> On Thu, 2025-12-11 at 18:06 +0530, Sai Pratyusha Magam wrote:
>> 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.
>
> Make sense ... we translate them internally because of all the matching
> to interfaces etc.
>
>> net/mac80211/wpa.c | 167 +++++++++++++++++++++++++++++++++++++++++----
>
> This seems rather complex.
>
> I believe the translation generally happens in mac80211 itself, in
> ieee80211_prepare_and_rx_handle, couldn't we stash away the 18 bytes of
> the three addresses in struct ieee80211_rx_data and then just
> *unconditionally* use those, instead of all this conditional logic?
>
Hi Johannes, I agree that by maintaining a local storage of the A1/A2/A3
link addresses before mac80211 translates them to the MLD addresses
would make things easy, i.e, they can directly be used for the
computations in the SW crypto. While this works well for the receive
path, on the Transmit path, mac80211 would still receive management
frames from hostapd with the MLD addresses, which again cannot be used
directly for the AAD/Nonce computations.
> I can see the value in putting something only into the SW crypto so it
> doesn't affect anything else, but also the address rewriting that
> happens is pretty rare, so maybe that means we also have a flag there if
> using the original 18 bytes is needed or not, but that should all still
> be far less complex?
>
> johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-01-09 10:03 ` Sai Pratyusha Magam
@ 2026-01-14 17:04 ` Johannes Berg
2026-01-23 11:38 ` Sai Pratyusha Magam
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2026-01-14 17:04 UTC (permalink / raw)
To: Sai Pratyusha Magam; +Cc: linux-wireless, Rohan Dutta
On Fri, 2026-01-09 at 15:33 +0530, Sai Pratyusha Magam wrote:
>
> Hi Johannes, I agree that by maintaining a local storage of the A1/A2/A3
> link addresses before mac80211 translates them to the MLD addresses
> would make things easy, i.e, they can directly be used for the
> computations in the SW crypto. While this works well for the receive
> path, on the Transmit path, mac80211 would still receive management
> frames from hostapd with the MLD addresses, which again cannot be used
> directly for the AAD/Nonce computations.
Fair point.
Thinking out loud: First, I think we can afford to separate TX and RX
discussions.
For RX, I think we agree that it could be done much simpler by
(conditionally, when we do translation in mac80211) keeping the pre-
translation addresses, and passing them into the SW crypto. If not set,
use the frame itself.
Secondly, this all seems only relevant to hwsim. Do you think otherwise?
Few drivers seem to use IEEE80211_KEY_FLAG_SW_MGMT_TX, so I think
especially with MLO we can say that you simply _have_ to support TX
encryption offload for EPPKE [1].
Obviously we still want to have hwsim, but if this really is only for
that then I feel we can still fairly easily implement TX encryption
"offload"? After all, in RX we make decryption optional for every single
frame - so if the device/driver didn't decrypt/validate the frame then
mac80211 will. We also pass the key to the driver for each individual
packet (struct ieee80211_tx_info::control::key). So doing the encryption
in hwsim would be really simple if we export a function akin to
ieee80211_tx_h_encrypt() that works on a single skb (which has the key
pointer), sets up a single-frame struct ieee80211_tx_data and returns
the skb from that [2].
While this may sound like a bit more work overall, I'm not even
convinced that it's _that_ much more, and I it would also align hwsim
more with how modern hardware works today anyway.
Any thoughts?
johannes
[1] and, it seems, correct unicast action frame encryption?
[2] https://p.sipsolutions.net/154c5c86af7765fd.txt
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-01-14 17:04 ` Johannes Berg
@ 2026-01-23 11:38 ` Sai Pratyusha Magam
2026-01-23 12:34 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Sai Pratyusha Magam @ 2026-01-23 11:38 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Rohan Dutta
On 1/14/2026 10:34 PM, Johannes Berg wrote:
> On Fri, 2026-01-09 at 15:33 +0530, Sai Pratyusha Magam wrote:
>>
>> Hi Johannes, I agree that by maintaining a local storage of the A1/A2/A3
>> link addresses before mac80211 translates them to the MLD addresses
>> would make things easy, i.e, they can directly be used for the
>> computations in the SW crypto. While this works well for the receive
>> path, on the Transmit path, mac80211 would still receive management
>> frames from hostapd with the MLD addresses, which again cannot be used
>> directly for the AAD/Nonce computations.
>
> Fair point.
>
> Thinking out loud: First, I think we can afford to separate TX and RX
> discussions.
>
> For RX, I think we agree that it could be done much simpler by
> (conditionally, when we do translation in mac80211) keeping the pre-
> translation addresses, and passing them into the SW crypto. If not set,
> use the frame itself.
Agree that on the RX, crypto computations can use the copy of
untranslated storage of addresses
>
> Secondly, this all seems only relevant to hwsim. Do you think otherwise?
> Few drivers seem to use IEEE80211_KEY_FLAG_SW_MGMT_TX, so I think
> especially with MLO we can say that you simply _have_ to support TX
> encryption offload for EPPKE [1].
>
True, all of this is for the hwsim use case that uses the mac80211 based
software crypto.
> Obviously we still want to have hwsim, but if this really is only for
> that then I feel we can still fairly easily implement TX encryption
> "offload"? After all, in RX we make decryption optional for every single
> frame - so if the device/driver didn't decrypt/validate the frame then
> mac80211 will. We also pass the key to the driver for each individual
> packet (struct ieee80211_tx_info::control::key). So doing the encryption
> in hwsim would be really simple if we export a function akin to
> ieee80211_tx_h_encrypt() that works on a single skb (which has the key
> pointer), sets up a single-frame struct ieee80211_tx_data and returns
> the skb from that [2].
>
"Implement TX encryption offload" - As I understand, your guidance is
towards implementing the Tx encryption in the mac80211_hwsim driver
Can you please kindly clarify if the understanding is correct?
In order to let mac80211_hwsim driver do the Tx encryption:
(1)Add support for key install to hwsim driver, i.e,
mac80211_hwsim_ops::add_key, so mac80211 knows that the driver is going
to do the encryption.
(2)A function similar to ieee80211_tx_h_encrypt() in the hwsim driver
that can do the actual encryption. Since mgmt frames need to use link
addresses for crypto computations, possibly I could call
ieee80211_encrypt_tx_skb() [2] after address translation to link
addresses in mac80211_hwsim_tx() and since data frames need to use the
MLD addresses, ieee80211_encrypt_tx_skb() can be called before the
address translation to link addresses.
> While this may sound like a bit more work overall, I'm not even
> convinced that it's _that_ much more, and I it would also align hwsim
> more with how modern hardware works today anyway.
>
> Any thoughts?
>
> johannes
>
> [1] and, it seems, correct unicast action frame encryption?
> [2] https://p.sipsolutions.net/154c5c86af7765fd.txt
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-01-23 11:38 ` Sai Pratyusha Magam
@ 2026-01-23 12:34 ` Johannes Berg
2026-02-02 5:40 ` Sai Pratyusha Magam
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2026-01-23 12:34 UTC (permalink / raw)
To: Sai Pratyusha Magam; +Cc: linux-wireless, Rohan Dutta
Hi Sai,
So I'll preface this saying (maybe again) I'm mostly thinking out loud,
because adding 150+ lines of complex address munging code that reaches
into a lot of different data structures for what's effectively a hwsim-
only special case doesn't seem like a good trade-off right now... If we
can avoid this in mac80211 and push something into hwsim, it seems like
a win even if it were more complex there.
> > Obviously we still want to have hwsim, but if this really is only for
> > that then I feel we can still fairly easily implement TX encryption
> > "offload"? After all, in RX we make decryption optional for every single
> > frame - so if the device/driver didn't decrypt/validate the frame then
> > mac80211 will. We also pass the key to the driver for each individual
> > packet (struct ieee80211_tx_info::control::key). So doing the encryption
> > in hwsim would be really simple if we export a function akin to
> > ieee80211_tx_h_encrypt() that works on a single skb (which has the key
> > pointer), sets up a single-frame struct ieee80211_tx_data and returns
> > the skb from that [2].
> >
> "Implement TX encryption offload" - As I understand, your guidance is
> towards implementing the Tx encryption in the mac80211_hwsim driver
> Can you please kindly clarify if the understanding is correct?
Not sure I'd say "guidance", but yes, that's what I was thinking of.
Note that this is fairly limited, I'll elaborate below.
> In order to let mac80211_hwsim driver do the Tx encryption:
> (1)Add support for key install to hwsim driver, i.e,
> mac80211_hwsim_ops::add_key, so mac80211 knows that the driver is going
> to do the encryption.
Yes. Note that due to the mac80211 SW crypto design, it doesn't really
need to do _anything_, because:
- on TX, mac80211 gives the key in the tx_info of every skb
- on RX, mac80211 will happily do SW decryption if RX_FLAG_DECRYPTED
isn't set on the skb
> (2)A function similar to ieee80211_tx_h_encrypt() in the hwsim driver
> that can do the actual encryption.
Actually, the function I posted in my other mail was going to live in
mac80211, but exported to be (only) called by hwsim. That way we can
reuse all the existing mac80211 code without making it more complex.
(We could even put it under a hwsim ifdef or hwsim-only export
namespace, but not sure that has much value.)
> Since mgmt frames need to use link
> addresses for crypto computations, possibly I could call
> ieee80211_encrypt_tx_skb() [2] after address translation to link
> addresses in mac80211_hwsim_tx() and since data frames need to use the
> MLD addresses, ieee80211_encrypt_tx_skb() can be called before the
> address translation to link addresses.
Right.
Given the function I posted wasn't even 30 lines, and the key install
function in hwsim can be basically "return 0", the RX part likely won't
be _that_ much either, I'd tend to think that overall it could end up
with even fewer lines of code than this solution, never mind the fact
that it won't have to reach into all the vif/link/sta/link_sta data
structures.
Now I haven't actually written it, so I don't know for sure it'll be
that simple, but at this point I'm fairly confident it should be? Do you
see any holes in this?
johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO
2026-01-23 12:34 ` Johannes Berg
@ 2026-02-02 5:40 ` Sai Pratyusha Magam
0 siblings, 0 replies; 7+ messages in thread
From: Sai Pratyusha Magam @ 2026-02-02 5:40 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Rohan Dutta
Hi Johannes, Thanks for the elaborate response. I have been working
on your suggestions and will send out the next version with the changes
incorporated soon.
On 1/23/2026 6:04 PM, Johannes Berg wrote:
> Hi Sai,
>
> So I'll preface this saying (maybe again) I'm mostly thinking out loud,
> because adding 150+ lines of complex address munging code that reaches
> into a lot of different data structures for what's effectively a hwsim-
> only special case doesn't seem like a good trade-off right now... If we
> can avoid this in mac80211 and push something into hwsim, it seems like
> a win even if it were more complex there.
>
>>> Obviously we still want to have hwsim, but if this really is only for
>>> that then I feel we can still fairly easily implement TX encryption
>>> "offload"? After all, in RX we make decryption optional for every single
>>> frame - so if the device/driver didn't decrypt/validate the frame then
>>> mac80211 will. We also pass the key to the driver for each individual
>>> packet (struct ieee80211_tx_info::control::key). So doing the encryption
>>> in hwsim would be really simple if we export a function akin to
>>> ieee80211_tx_h_encrypt() that works on a single skb (which has the key
>>> pointer), sets up a single-frame struct ieee80211_tx_data and returns
>>> the skb from that [2].
>>>
>> "Implement TX encryption offload" - As I understand, your guidance is
>> towards implementing the Tx encryption in the mac80211_hwsim driver
>> Can you please kindly clarify if the understanding is correct?
>
> Not sure I'd say "guidance", but yes, that's what I was thinking of.
> Note that this is fairly limited, I'll elaborate below.
>
>> In order to let mac80211_hwsim driver do the Tx encryption:
>> (1)Add support for key install to hwsim driver, i.e,
>> mac80211_hwsim_ops::add_key, so mac80211 knows that the driver is going
>> to do the encryption.
>
> Yes. Note that due to the mac80211 SW crypto design, it doesn't really
> need to do _anything_, because:
> - on TX, mac80211 gives the key in the tx_info of every skb
> - on RX, mac80211 will happily do SW decryption if RX_FLAG_DECRYPTED
> isn't set on the skb
>
>> (2)A function similar to ieee80211_tx_h_encrypt() in the hwsim driver
>> that can do the actual encryption.
>
> Actually, the function I posted in my other mail was going to live in
> mac80211, but exported to be (only) called by hwsim. That way we can
> reuse all the existing mac80211 code without making it more complex.
> (We could even put it under a hwsim ifdef or hwsim-only export
> namespace, but not sure that has much value.)
>
>> Since mgmt frames need to use link
>> addresses for crypto computations, possibly I could call
>> ieee80211_encrypt_tx_skb() [2] after address translation to link
>> addresses in mac80211_hwsim_tx() and since data frames need to use the
>> MLD addresses, ieee80211_encrypt_tx_skb() can be called before the
>> address translation to link addresses.
>
> Right.
>
> Given the function I posted wasn't even 30 lines, and the key install
> function in hwsim can be basically "return 0", the RX part likely won't
> be _that_ much either, I'd tend to think that overall it could end up
> with even fewer lines of code than this solution, never mind the fact
> that it won't have to reach into all the vif/link/sta/link_sta data
> structures.
>
> Now I haven't actually written it, so I don't know for sure it'll be
> that simple, but at this point I'm fairly confident it should be? Do you
> see any holes in this?
>
> johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-02-02 5:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-11 12:36 [PATCH wireless-next] wifi: mac80211: Fix AAD/Nonce computation for management frames with MLO Sai Pratyusha Magam
2026-01-08 12:52 ` Johannes Berg
2026-01-09 10:03 ` Sai Pratyusha Magam
2026-01-14 17:04 ` Johannes Berg
2026-01-23 11:38 ` Sai Pratyusha Magam
2026-01-23 12:34 ` Johannes Berg
2026-02-02 5:40 ` Sai Pratyusha Magam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox