* [PATCH wireless v2 0/2] Fix packets processed after vif is stopped
@ 2025-03-24 16:28 Remi Pommarel
2025-03-24 16:28 ` [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue() Remi Pommarel
2025-03-24 16:28 ` [PATCH wireless v2 2/2] wifi: mac80211: Purge vif txq in ieee80211_do_stop() Remi Pommarel
0 siblings, 2 replies; 13+ messages in thread
From: Remi Pommarel @ 2025-03-24 16:28 UTC (permalink / raw)
To: linux-wireless, linux-kernel; +Cc: Johannes Berg, Remi Pommarel
Those are a couple of fixes that prevent crashes due to processing
packets (especially multicast ones) for TX after vif is stopped (either
after a mesh interface left the group or interface is put down).
The first one ensure the key info passed to drivers through ieee80211
skb control block is up to date, even after key removal.
The second one ensure no packets get processed after vif driver private
data is cleared in ieee80211_do_stop().
v2:
- Add Fixes tag
- Clear SKB's control block key before in ieee80211_tx_dequeue()
instead of ieee80211_tx_h_select_key()
- Add wireless tag and fix type in subject
- Because subject changed, v1 can be found at
https://lore.kernel.org/lkml/cover.1741950009.git.repk@triplefau.lt/
Remi Pommarel (2):
wifi: mac80211: Update skb's control block key in
ieee80211_tx_dequeue()
wifi: mac80211: Purge vif txq in ieee80211_do_stop()
net/mac80211/iface.c | 3 +++
net/mac80211/tx.c | 1 +
2 files changed, 4 insertions(+)
--
2.40.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-03-24 16:28 [PATCH wireless v2 0/2] Fix packets processed after vif is stopped Remi Pommarel
@ 2025-03-24 16:28 ` Remi Pommarel
2025-04-10 21:55 ` Bert Karwatzki
2025-03-24 16:28 ` [PATCH wireless v2 2/2] wifi: mac80211: Purge vif txq in ieee80211_do_stop() Remi Pommarel
1 sibling, 1 reply; 13+ messages in thread
From: Remi Pommarel @ 2025-03-24 16:28 UTC (permalink / raw)
To: linux-wireless, linux-kernel; +Cc: Johannes Berg, Remi Pommarel
The ieee80211 skb control block key (set when skb was queued) could have
been removed before ieee80211_tx_dequeue() call. ieee80211_tx_dequeue()
already called ieee80211_tx_h_select_key() to get the current key, but
the latter do not update the key in skb control block in case it is
NULL. Because some drivers actually use this key in their TX callbacks
(e.g. ath1{1,2}k_mac_op_tx()) this could lead to the use after free
below:
BUG: KASAN: slab-use-after-free in ath11k_mac_op_tx+0x590/0x61c
Read of size 4 at addr ffffff803083c248 by task kworker/u16:4/1440
CPU: 3 UID: 0 PID: 1440 Comm: kworker/u16:4 Not tainted 6.13.0-ge128f627f404 #2
Hardware name: HW (DT)
Workqueue: bat_events batadv_send_outstanding_bcast_packet
Call trace:
show_stack+0x14/0x1c (C)
dump_stack_lvl+0x58/0x74
print_report+0x164/0x4c0
kasan_report+0xac/0xe8
__asan_report_load4_noabort+0x1c/0x24
ath11k_mac_op_tx+0x590/0x61c
ieee80211_handle_wake_tx_queue+0x12c/0x1c8
ieee80211_queue_skb+0xdcc/0x1b4c
ieee80211_tx+0x1ec/0x2bc
ieee80211_xmit+0x224/0x324
__ieee80211_subif_start_xmit+0x85c/0xcf8
ieee80211_subif_start_xmit+0xc0/0xec4
dev_hard_start_xmit+0xf4/0x28c
__dev_queue_xmit+0x6ac/0x318c
batadv_send_skb_packet+0x38c/0x4b0
batadv_send_outstanding_bcast_packet+0x110/0x328
process_one_work+0x578/0xc10
worker_thread+0x4bc/0xc7c
kthread+0x2f8/0x380
ret_from_fork+0x10/0x20
Allocated by task 1906:
kasan_save_stack+0x28/0x4c
kasan_save_track+0x1c/0x40
kasan_save_alloc_info+0x3c/0x4c
__kasan_kmalloc+0xac/0xb0
__kmalloc_noprof+0x1b4/0x380
ieee80211_key_alloc+0x3c/0xb64
ieee80211_add_key+0x1b4/0x71c
nl80211_new_key+0x2b4/0x5d8
genl_family_rcv_msg_doit+0x198/0x240
<...>
Freed by task 1494:
kasan_save_stack+0x28/0x4c
kasan_save_track+0x1c/0x40
kasan_save_free_info+0x48/0x94
__kasan_slab_free+0x48/0x60
kfree+0xc8/0x31c
kfree_sensitive+0x70/0x80
ieee80211_key_free_common+0x10c/0x174
ieee80211_free_keys+0x188/0x46c
ieee80211_stop_mesh+0x70/0x2cc
ieee80211_leave_mesh+0x1c/0x60
cfg80211_leave_mesh+0xe0/0x280
cfg80211_leave+0x1e0/0x244
<...>
Reset SKB control block key before calling ieee80211_tx_h_select_key()
to avoid that.
Fixes: bb42f2d13ffc ("mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue")
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
net/mac80211/tx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index a24636bda679..0c6214f12ea3 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3893,6 +3893,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
* The key can be removed while the packet was queued, so need to call
* this here to get the current key.
*/
+ info->control.hw_key = NULL;
r = ieee80211_tx_h_select_key(&tx);
if (r != TX_CONTINUE) {
ieee80211_free_txskb(&local->hw, skb);
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH wireless v2 2/2] wifi: mac80211: Purge vif txq in ieee80211_do_stop()
2025-03-24 16:28 [PATCH wireless v2 0/2] Fix packets processed after vif is stopped Remi Pommarel
2025-03-24 16:28 ` [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue() Remi Pommarel
@ 2025-03-24 16:28 ` Remi Pommarel
1 sibling, 0 replies; 13+ messages in thread
From: Remi Pommarel @ 2025-03-24 16:28 UTC (permalink / raw)
To: linux-wireless, linux-kernel; +Cc: Johannes Berg, Remi Pommarel
After ieee80211_do_stop() SKB from vif's txq could still be processed.
Indeed another concurrent vif schedule_and_wake_txq call could cause
those packets to be dequeued (see ieee80211_handle_wake_tx_queue())
without checking the sdata current state.
Because vif.drv_priv is now cleared in this function, this could lead to
driver crash.
For example in ath12k, ahvif is store in vif.drv_priv. Thus if
ath12k_mac_op_tx() is called after ieee80211_do_stop(), ahvif->ah can be
NULL, leading the ath12k_warn(ahvif->ah,...) call in this function to
trigger the NULL deref below.
Unable to handle kernel paging request at virtual address dfffffc000000001
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
batman_adv: bat0: Interface deactivated: brbh1337
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[dfffffc000000001] address between user and kernel address ranges
Internal error: Oops: 0000000096000004 [#1] SMP
CPU: 1 UID: 0 PID: 978 Comm: lbd Not tainted 6.13.0-g633f875b8f1e #114
Hardware name: HW (DT)
pstate: 10000005 (nzcV daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : ath12k_mac_op_tx+0x6cc/0x29b8 [ath12k]
lr : ath12k_mac_op_tx+0x174/0x29b8 [ath12k]
sp : ffffffc086ace450
x29: ffffffc086ace450 x28: 0000000000000000 x27: 1ffffff810d59ca4
x26: ffffff801d05f7c0 x25: 0000000000000000 x24: 000000004000001e
x23: ffffff8009ce4926 x22: ffffff801f9c0800 x21: ffffff801d05f7f0
x20: ffffff8034a19f40 x19: 0000000000000000 x18: ffffff801f9c0958
x17: ffffff800bc0a504 x16: dfffffc000000000 x15: ffffffc086ace4f8
x14: ffffff801d05f83c x13: 0000000000000000 x12: ffffffb003a0bf03
x11: 0000000000000000 x10: ffffffb003a0bf02 x9 : ffffff8034a19f40
x8 : ffffff801d05f818 x7 : 1ffffff0069433dc x6 : ffffff8034a19ee0
x5 : ffffff801d05f7f0 x4 : 0000000000000000 x3 : 0000000000000001
x2 : 0000000000000000 x1 : dfffffc000000000 x0 : 0000000000000008
Call trace:
ath12k_mac_op_tx+0x6cc/0x29b8 [ath12k] (P)
ieee80211_handle_wake_tx_queue+0x16c/0x260
ieee80211_queue_skb+0xeec/0x1d20
ieee80211_tx+0x200/0x2c8
ieee80211_xmit+0x22c/0x338
__ieee80211_subif_start_xmit+0x7e8/0xc60
ieee80211_subif_start_xmit+0xc4/0xee0
__ieee80211_subif_start_xmit_8023.isra.0+0x854/0x17a0
ieee80211_subif_start_xmit_8023+0x124/0x488
dev_hard_start_xmit+0x160/0x5a8
__dev_queue_xmit+0x6f8/0x3120
br_dev_queue_push_xmit+0x120/0x4a8
__br_forward+0xe4/0x2b0
deliver_clone+0x5c/0xd0
br_flood+0x398/0x580
br_dev_xmit+0x454/0x9f8
dev_hard_start_xmit+0x160/0x5a8
__dev_queue_xmit+0x6f8/0x3120
ip6_finish_output2+0xc28/0x1b60
__ip6_finish_output+0x38c/0x638
ip6_output+0x1b4/0x338
ip6_local_out+0x7c/0xa8
ip6_send_skb+0x7c/0x1b0
ip6_push_pending_frames+0x94/0xd0
rawv6_sendmsg+0x1a98/0x2898
inet_sendmsg+0x94/0xe0
__sys_sendto+0x1e4/0x308
__arm64_sys_sendto+0xc4/0x140
do_el0_svc+0x110/0x280
el0_svc+0x20/0x60
el0t_64_sync_handler+0x104/0x138
el0t_64_sync+0x154/0x158
To avoid that, empty vif's txq at ieee80211_do_stop() so no packet could
be dequeued after ieee80211_do_stop() (new packets cannot be queued
because SDATA_STATE_RUNNING is cleared at this point).
Fixes: ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
net/mac80211/iface.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 738de269e13f..e60c1ffebaea 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -660,6 +660,9 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
ieee80211_txq_remove_vlan(local, sdata);
+ if (sdata->vif.txq)
+ ieee80211_txq_purge(sdata->local, to_txq_info(sdata->vif.txq));
+
sdata->bss = NULL;
if (local->open_count == 0)
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-03-24 16:28 ` [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue() Remi Pommarel
@ 2025-04-10 21:55 ` Bert Karwatzki
2025-04-11 10:06 ` Remi Pommarel
2025-04-11 14:16 ` Johannes Berg
0 siblings, 2 replies; 13+ messages in thread
From: Bert Karwatzki @ 2025-04-10 21:55 UTC (permalink / raw)
To: Remi Pommarel; +Cc: Bert Karwatzki, linux-wireless, linux-kernel, johannes
This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410
my mt7921e Wi-Fi controller is no longer able to connect to a network.
I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control
block key in ieee80211_tx_dequeue()").
Hardware:
04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz
This debugging patch reveals that the change causes key to be NULL in
mt7921_tx_prepare_skb().
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
index 881812ba03ff..3b8552a1055c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
@@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
struct ieee80211_key_conf *key = info->control.hw_key;
+ dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
struct mt76_connac_hw_txp *txp;
struct mt76_txwi_cache *t;
int id, pid;
So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 34f229a6eab0..2510e3533d13 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
case WLAN_CIPHER_SUITE_WEP40:
case WLAN_CIPHER_SUITE_WEP104:
case WLAN_CIPHER_SUITE_TKIP:
- if (!ieee80211_is_data_present(hdr->frame_control))
+ if (!ieee80211_is_data_present(hdr->frame_control)) {
+ printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
tx->key = NULL;
+ }
break;
case WLAN_CIPHER_SUITE_CCMP:
case WLAN_CIPHER_SUITE_CCMP_256:
@@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
if (!ieee80211_is_data_present(hdr->frame_control) &&
!ieee80211_use_mfp(hdr->frame_control, tx->sta,
tx->skb) &&
- !ieee80211_is_group_privacy_action(tx->skb))
+ !ieee80211_is_group_privacy_action(tx->skb)) {
+ printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
tx->key = NULL;
- else
+ } else {
skip_hw = (tx->key->conf.flags &
IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
ieee80211_is_mgmt(hdr->frame_control);
+ }
break;
case WLAN_CIPHER_SUITE_AES_CMAC:
case WLAN_CIPHER_SUITE_BIP_CMAC_256:
case WLAN_CIPHER_SUITE_BIP_GMAC_128:
case WLAN_CIPHER_SUITE_BIP_GMAC_256:
- if (!ieee80211_is_mgmt(hdr->frame_control))
+ if (!ieee80211_is_mgmt(hdr->frame_control)) {
+ printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
tx->key = NULL;
+ }
break;
}
@@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
tx->skb->protocol != tx->sdata->control_port_protocol)
return TX_DROP;
+ printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
+ __func__, skip_hw, tx->key);
if (!skip_hw && tx->key &&
- tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
+ tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
info->control.hw_key = &tx->key->conf;
+ printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
+ }
} else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
return TX_DROP;
@@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
* The key can be removed while the packet was queued, so need to call
* this here to get the current key.
*/
+ printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
+ __func__, info->control.hw_key);
info->control.hw_key = NULL;
r = ieee80211_tx_h_select_key(&tx);
if (r != TX_CONTINUE) {
This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk)
and so the updating of info->contro.hw_key is skipped:
[ 17.411298] [ T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL
[ 17.411300] [ T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
[ 17.411307] [ T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
If I revert commit a104042e2bf6 while keeping the debug patches it shows that
the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e
relies on the key your patch is setting to NULL.
Is this a problem with your patch or with the mt7921e driver that just got
revealed by your patch?
Bert Karwatzki
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-04-10 21:55 ` Bert Karwatzki
@ 2025-04-11 10:06 ` Remi Pommarel
2025-04-11 11:10 ` Bert Karwatzki
2025-04-11 14:16 ` Johannes Berg
1 sibling, 1 reply; 13+ messages in thread
From: Remi Pommarel @ 2025-04-11 10:06 UTC (permalink / raw)
To: Bert Karwatzki; +Cc: linux-wireless, linux-kernel, johannes
Hi Bert,
On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410
> my mt7921e Wi-Fi controller is no longer able to connect to a network.
> I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control
> block key in ieee80211_tx_dequeue()").
>
> Hardware:
> 04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz
>
> This debugging patch reveals that the change causes key to be NULL in
> mt7921_tx_prepare_skb().
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> index 881812ba03ff..3b8552a1055c 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> @@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
> struct ieee80211_key_conf *key = info->control.hw_key;
> + dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
> struct mt76_connac_hw_txp *txp;
> struct mt76_txwi_cache *t;
> int id, pid;
>
>
> So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 34f229a6eab0..2510e3533d13 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> case WLAN_CIPHER_SUITE_WEP40:
> case WLAN_CIPHER_SUITE_WEP104:
> case WLAN_CIPHER_SUITE_TKIP:
> - if (!ieee80211_is_data_present(hdr->frame_control))
> + if (!ieee80211_is_data_present(hdr->frame_control)) {
> + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> tx->key = NULL;
> + }
> break;
> case WLAN_CIPHER_SUITE_CCMP:
> case WLAN_CIPHER_SUITE_CCMP_256:
> @@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> if (!ieee80211_is_data_present(hdr->frame_control) &&
> !ieee80211_use_mfp(hdr->frame_control, tx->sta,
> tx->skb) &&
> - !ieee80211_is_group_privacy_action(tx->skb))
> + !ieee80211_is_group_privacy_action(tx->skb)) {
> + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> tx->key = NULL;
> - else
> + } else {
> skip_hw = (tx->key->conf.flags &
> IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
> ieee80211_is_mgmt(hdr->frame_control);
> + }
> break;
> case WLAN_CIPHER_SUITE_AES_CMAC:
> case WLAN_CIPHER_SUITE_BIP_CMAC_256:
> case WLAN_CIPHER_SUITE_BIP_GMAC_128:
> case WLAN_CIPHER_SUITE_BIP_GMAC_256:
> - if (!ieee80211_is_mgmt(hdr->frame_control))
> + if (!ieee80211_is_mgmt(hdr->frame_control)) {
> + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> tx->key = NULL;
> + }
> break;
> }
>
> @@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> tx->skb->protocol != tx->sdata->control_port_protocol)
> return TX_DROP;
>
> + printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
> + __func__, skip_hw, tx->key);
> if (!skip_hw && tx->key &&
> - tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
> + tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
> info->control.hw_key = &tx->key->conf;
> + printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
> + }
> } else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
> test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
> return TX_DROP;
> @@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> * The key can be removed while the packet was queued, so need to call
> * this here to get the current key.
> */
> + printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
> + __func__, info->control.hw_key);
> info->control.hw_key = NULL;
> r = ieee80211_tx_h_select_key(&tx);
> if (r != TX_CONTINUE) {
>
> This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk)
> and so the updating of info->contro.hw_key is skipped:
>
> [ 17.411298] [ T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL
That means that we are trying to send non management frames using
AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group
management frames ?
> [ 17.411300] [ T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
> [ 17.411307] [ T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
>
> If I revert commit a104042e2bf6 while keeping the debug patches it shows that
> the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e
> relies on the key your patch is setting to NULL.
>
> Is this a problem with your patch or with the mt7921e driver that just got
> revealed by your patch?
Not sure yet, do you happen to know which kind of frame mt7921e is
trying to be sent using this NULL key ?
Thanks,
--
Remi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-04-11 10:06 ` Remi Pommarel
@ 2025-04-11 11:10 ` Bert Karwatzki
2025-04-11 12:15 ` Remi Pommarel
2025-07-17 13:21 ` Remi Pommarel
0 siblings, 2 replies; 13+ messages in thread
From: Bert Karwatzki @ 2025-04-11 11:10 UTC (permalink / raw)
To: Remi Pommarel
Cc: linux-wireless, linux-kernel, johannes, linux-mediatek, spasswolf
Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:
> Hi Bert,
>
> On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> > This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410
> > my mt7921e Wi-Fi controller is no longer able to connect to a network.
> > I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control
> > block key in ieee80211_tx_dequeue()").
> >
> > Hardware:
> > 04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz
> >
> > This debugging patch reveals that the change causes key to be NULL in
> > mt7921_tx_prepare_skb().
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > index 881812ba03ff..3b8552a1055c 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > @@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> > struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
> > struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
> > struct ieee80211_key_conf *key = info->control.hw_key;
> > + dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
> > struct mt76_connac_hw_txp *txp;
> > struct mt76_txwi_cache *t;
> > int id, pid;
> >
> >
> > So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?
> >
> > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > index 34f229a6eab0..2510e3533d13 100644
> > --- a/net/mac80211/tx.c
> > +++ b/net/mac80211/tx.c
> > @@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > case WLAN_CIPHER_SUITE_WEP40:
> > case WLAN_CIPHER_SUITE_WEP104:
> > case WLAN_CIPHER_SUITE_TKIP:
> > - if (!ieee80211_is_data_present(hdr->frame_control))
> > + if (!ieee80211_is_data_present(hdr->frame_control)) {
> > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > tx->key = NULL;
> > + }
> > break;
> > case WLAN_CIPHER_SUITE_CCMP:
> > case WLAN_CIPHER_SUITE_CCMP_256:
> > @@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > if (!ieee80211_is_data_present(hdr->frame_control) &&
> > !ieee80211_use_mfp(hdr->frame_control, tx->sta,
> > tx->skb) &&
> > - !ieee80211_is_group_privacy_action(tx->skb))
> > + !ieee80211_is_group_privacy_action(tx->skb)) {
> > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > tx->key = NULL;
> > - else
> > + } else {
> > skip_hw = (tx->key->conf.flags &
> > IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
> > ieee80211_is_mgmt(hdr->frame_control);
> > + }
> > break;
> > case WLAN_CIPHER_SUITE_AES_CMAC:
> > case WLAN_CIPHER_SUITE_BIP_CMAC_256:
> > case WLAN_CIPHER_SUITE_BIP_GMAC_128:
> > case WLAN_CIPHER_SUITE_BIP_GMAC_256:
> > - if (!ieee80211_is_mgmt(hdr->frame_control))
> > + if (!ieee80211_is_mgmt(hdr->frame_control)) {
> > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > tx->key = NULL;
> > + }
> > break;
> > }
> >
> > @@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > tx->skb->protocol != tx->sdata->control_port_protocol)
> > return TX_DROP;
> >
> > + printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
> > + __func__, skip_hw, tx->key);
> > if (!skip_hw && tx->key &&
> > - tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
> > + tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
> > info->control.hw_key = &tx->key->conf;
> > + printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
> > + }
> > } else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
> > test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
> > return TX_DROP;
> > @@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> > * The key can be removed while the packet was queued, so need to call
> > * this here to get the current key.
> > */
> > + printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
> > + __func__, info->control.hw_key);
> > info->control.hw_key = NULL;
> > r = ieee80211_tx_h_select_key(&tx);
> > if (r != TX_CONTINUE) {
> >
> > This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk)
> > and so the updating of info->contro.hw_key is skipped:
> >
> > [ 17.411298] [ T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL
>
> That means that we are trying to send non management frames using
> AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group
> management frames ?
>
> > [ 17.411300] [ T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
> > [ 17.411307] [ T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
> >
> > If I revert commit a104042e2bf6 while keeping the debug patches it shows that
> > the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e
> > relies on the key your patch is setting to NULL.
> >
> > Is this a problem with your patch or with the mt7921e driver that just got
> > revealed by your patch?
>
> Not sure yet, do you happen to know which kind of frame mt7921e is
> trying to be sent using this NULL key ?
>
> Thanks,
I modified my debugging patch to print mgmt->frame_control, if needed I could
also insert a nore complicated function printing out frame types using the
ieee80211_is_*() functions:
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
index 881812ba03ff..cfbe7e1e4713 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
@@ -13,6 +13,9 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void
*txwi_ptr,
struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
struct ieee80211_key_conf *key = info->control.hw_key;
+ struct ieee80211_mgmt *mgmt = (void *)tx_info->skb->data;
+ __le16 fc = mgmt->frame_control;
+ dev_info(mdev->dev, "%s: key = %px fc = 0x%hx\n", __func__, key, fc);
struct mt76_connac_hw_txp *txp;
struct mt76_txwi_cache *t;
int id, pid;
and get this, while unsuccesfully trying to connect (also note that one time
getting a key worked):
$ dmesg | grep prepare_skb
[ 11.775642] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[ 11.800047] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[ 13.365330] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[ 13.370257] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[ 16.468481] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[ 16.472407] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[ 16.542017] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 16.549581] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 16.597120] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 16.612263] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
Here we actually go a key:
[ 16.614478] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
ffff89c275297230 fc = 0x4188
[ 16.654273] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 16.698286] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 17.735855] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 17.837355] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 17.851029] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 18.613079] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 18.786202] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 21.027478] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 21.150212] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 21.843201] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 25.769981] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 29.776926] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 34.424966] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 44.945880] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 50.670382] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 62.054907] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xc0
[ 65.042457] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[ 65.047387] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[ 65.391881] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 65.405189] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 65.445775] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 65.449289] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 65.479305] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
[ 65.854396] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[ 65.878897] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[ 65.954047] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 65.961191] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 65.966296] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 65.977287] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
[ 66.117317] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 66.626938] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 66.700300] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 66.818440] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 67.347089] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 67.448947] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 67.649713] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 68.535890] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 69.537434] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 69.697331] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 69.866666] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 70.500294] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 71.537535] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 74.615436] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 75.543129] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 77.741585] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 83.309171] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 83.550827] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 91.650611] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 99.564147] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 99.625088] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 111.050345] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xc0
[ 114.346190] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[ 114.370450] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[ 114.447350] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 114.454710] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[ 114.494605] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 114.520269] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
[ 114.522520] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
ffff89c2e7227230 fc = 0x4188
[ 115.138564] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 115.394552] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 116.435866] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 116.494988] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 116.612522] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 116.724047] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 117.459651] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 117.600286] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 118.816266] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 118.961067] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 120.304632] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 121.614000] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 123.281756] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[ 127.930929] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[ 129.617550] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
Bert Karwatzki
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-04-11 11:10 ` Bert Karwatzki
@ 2025-04-11 12:15 ` Remi Pommarel
2025-07-17 13:21 ` Remi Pommarel
1 sibling, 0 replies; 13+ messages in thread
From: Remi Pommarel @ 2025-04-11 12:15 UTC (permalink / raw)
To: Bert Karwatzki; +Cc: linux-wireless, linux-kernel, johannes, linux-mediatek
On Fri, Apr 11, 2025 at 01:10:02PM +0200, Bert Karwatzki wrote:
> Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:
> > Hi Bert,
> >
> > On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> > > This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410
> > > my mt7921e Wi-Fi controller is no longer able to connect to a network.
> > > I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control
> > > block key in ieee80211_tx_dequeue()").
> > >
> > > Hardware:
> > > 04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz
> > >
> > > This debugging patch reveals that the change causes key to be NULL in
> > > mt7921_tx_prepare_skb().
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > index 881812ba03ff..3b8552a1055c 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > @@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> > > struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
> > > struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
> > > struct ieee80211_key_conf *key = info->control.hw_key;
> > > + dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
> > > struct mt76_connac_hw_txp *txp;
> > > struct mt76_txwi_cache *t;
> > > int id, pid;
> > >
> > >
> > > So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?
> > >
> > > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > > index 34f229a6eab0..2510e3533d13 100644
> > > --- a/net/mac80211/tx.c
> > > +++ b/net/mac80211/tx.c
> > > @@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > > case WLAN_CIPHER_SUITE_WEP40:
> > > case WLAN_CIPHER_SUITE_WEP104:
> > > case WLAN_CIPHER_SUITE_TKIP:
> > > - if (!ieee80211_is_data_present(hdr->frame_control))
> > > + if (!ieee80211_is_data_present(hdr->frame_control)) {
> > > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > > tx->key = NULL;
> > > + }
> > > break;
> > > case WLAN_CIPHER_SUITE_CCMP:
> > > case WLAN_CIPHER_SUITE_CCMP_256:
> > > @@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > > if (!ieee80211_is_data_present(hdr->frame_control) &&
> > > !ieee80211_use_mfp(hdr->frame_control, tx->sta,
> > > tx->skb) &&
> > > - !ieee80211_is_group_privacy_action(tx->skb))
> > > + !ieee80211_is_group_privacy_action(tx->skb)) {
> > > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > > tx->key = NULL;
> > > - else
> > > + } else {
> > > skip_hw = (tx->key->conf.flags &
> > > IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
> > > ieee80211_is_mgmt(hdr->frame_control);
> > > + }
> > > break;
> > > case WLAN_CIPHER_SUITE_AES_CMAC:
> > > case WLAN_CIPHER_SUITE_BIP_CMAC_256:
> > > case WLAN_CIPHER_SUITE_BIP_GMAC_128:
> > > case WLAN_CIPHER_SUITE_BIP_GMAC_256:
> > > - if (!ieee80211_is_mgmt(hdr->frame_control))
> > > + if (!ieee80211_is_mgmt(hdr->frame_control)) {
> > > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > > tx->key = NULL;
> > > + }
> > > break;
> > > }
> > >
> > > @@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > > tx->skb->protocol != tx->sdata->control_port_protocol)
> > > return TX_DROP;
> > >
> > > + printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
> > > + __func__, skip_hw, tx->key);
> > > if (!skip_hw && tx->key &&
> > > - tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
> > > + tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
> > > info->control.hw_key = &tx->key->conf;
> > > + printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
> > > + }
> > > } else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
> > > test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
> > > return TX_DROP;
> > > @@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> > > * The key can be removed while the packet was queued, so need to call
> > > * this here to get the current key.
> > > */
> > > + printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
> > > + __func__, info->control.hw_key);
> > > info->control.hw_key = NULL;
> > > r = ieee80211_tx_h_select_key(&tx);
> > > if (r != TX_CONTINUE) {
> > >
> > > This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk)
> > > and so the updating of info->contro.hw_key is skipped:
> > >
> > > [ 17.411298] [ T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL
> >
> > That means that we are trying to send non management frames using
> > AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group
> > management frames ?
Thanks for the debug.
I completely miscalculated the line here sorry. In fact it means here
that mt7921e needs a key for null data frames or for non protected
management frames.
As a lot of your frame control dump does not make sense to me (e.g.
0xffff) maybe you could try to print the fc here at line 647. Just a
wild guess maybe mt7921e needs MT_TXD3_PROTECT_FRAME for NULL data
frames ?
> >
> > > [ 17.411300] [ T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
> > > [ 17.411307] [ T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
> > >
> > > If I revert commit a104042e2bf6 while keeping the debug patches it shows that
> > > the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e
> > > relies on the key your patch is setting to NULL.
> > >
> > > Is this a problem with your patch or with the mt7921e driver that just got
> > > revealed by your patch?
> >
> > Not sure yet, do you happen to know which kind of frame mt7921e is
> > trying to be sent using this NULL key ?
> >
> > Thanks,
>
> I modified my debugging patch to print mgmt->frame_control, if needed I could
> also insert a nore complicated function printing out frame types using the
> ieee80211_is_*() functions:
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> index 881812ba03ff..cfbe7e1e4713 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> @@ -13,6 +13,9 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void
> *txwi_ptr,
> struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
> struct ieee80211_key_conf *key = info->control.hw_key;
> + struct ieee80211_mgmt *mgmt = (void *)tx_info->skb->data;
> + __le16 fc = mgmt->frame_control;
> + dev_info(mdev->dev, "%s: key = %px fc = 0x%hx\n", __func__, key, fc);
> struct mt76_connac_hw_txp *txp;
> struct mt76_txwi_cache *t;
> int id, pid;
>
> and get this, while unsuccesfully trying to connect (also note that one time
> getting a key worked):
>
> $ dmesg | grep prepare_skb
> [ 11.775642] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xb0
> [ 11.800047] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x0
> [ 13.365330] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xb0
> [ 13.370257] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x0
> [ 16.468481] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xb0
> [ 16.472407] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x0
> [ 16.542017] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x188
> [ 16.549581] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x188
> [ 16.597120] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xffff
Thanks again for taking time to debug that.
As said above some of those fc does not make sense to me as I am not
expected first two bits (Protocol version) to be non null.
> [ 16.612263] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xd0
>
> Here we actually go a key:
> [ 16.614478] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> ffff89c275297230 fc = 0x4188
> [...]
--
Remi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-04-10 21:55 ` Bert Karwatzki
2025-04-11 10:06 ` Remi Pommarel
@ 2025-04-11 14:16 ` Johannes Berg
1 sibling, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2025-04-11 14:16 UTC (permalink / raw)
To: Bert Karwatzki, Remi Pommarel; +Cc: linux-wireless, linux-kernel
On Thu, 2025-04-10 at 23:55 +0200, Bert Karwatzki wrote:
> This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410
> my mt7921e Wi-Fi controller is no longer able to connect to a network.
> I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control
> block key in ieee80211_tx_dequeue()").
>
Thanks for the report.
Remi, I see you've been debugging it already - thanks!
Nonetheless I need to get a couple of fixes out, so I've reverted it for
now. We have a couple of weeks to fix it even for this release, and the
bug has been around for years, as far as we know.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-04-11 11:10 ` Bert Karwatzki
2025-04-11 12:15 ` Remi Pommarel
@ 2025-07-17 13:21 ` Remi Pommarel
2025-07-17 13:46 ` Johannes Berg
1 sibling, 1 reply; 13+ messages in thread
From: Remi Pommarel @ 2025-07-17 13:21 UTC (permalink / raw)
To: Bert Karwatzki; +Cc: linux-wireless, linux-kernel, johannes, linux-mediatek
Hello Bert, Johannes
On Fri, Apr 11, 2025 at 01:10:02PM +0200, Bert Karwatzki wrote:
> Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:
> > Hi Bert,
> >
> > On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> > > This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410
> > > my mt7921e Wi-Fi controller is no longer able to connect to a network.
> > > I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control
> > > block key in ieee80211_tx_dequeue()").
> > >
> > > Hardware:
> > > 04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz
> > >
> > > This debugging patch reveals that the change causes key to be NULL in
> > > mt7921_tx_prepare_skb().
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > index 881812ba03ff..3b8552a1055c 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > @@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> > > struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
> > > struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
> > > struct ieee80211_key_conf *key = info->control.hw_key;
> > > + dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
> > > struct mt76_connac_hw_txp *txp;
> > > struct mt76_txwi_cache *t;
> > > int id, pid;
> > >
> > >
> > > So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?
> > >
> > > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > > index 34f229a6eab0..2510e3533d13 100644
> > > --- a/net/mac80211/tx.c
> > > +++ b/net/mac80211/tx.c
> > > @@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > > case WLAN_CIPHER_SUITE_WEP40:
> > > case WLAN_CIPHER_SUITE_WEP104:
> > > case WLAN_CIPHER_SUITE_TKIP:
> > > - if (!ieee80211_is_data_present(hdr->frame_control))
> > > + if (!ieee80211_is_data_present(hdr->frame_control)) {
> > > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > > tx->key = NULL;
> > > + }
> > > break;
> > > case WLAN_CIPHER_SUITE_CCMP:
> > > case WLAN_CIPHER_SUITE_CCMP_256:
> > > @@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > > if (!ieee80211_is_data_present(hdr->frame_control) &&
> > > !ieee80211_use_mfp(hdr->frame_control, tx->sta,
> > > tx->skb) &&
> > > - !ieee80211_is_group_privacy_action(tx->skb))
> > > + !ieee80211_is_group_privacy_action(tx->skb)) {
> > > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > > tx->key = NULL;
> > > - else
> > > + } else {
> > > skip_hw = (tx->key->conf.flags &
> > > IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
> > > ieee80211_is_mgmt(hdr->frame_control);
> > > + }
> > > break;
> > > case WLAN_CIPHER_SUITE_AES_CMAC:
> > > case WLAN_CIPHER_SUITE_BIP_CMAC_256:
> > > case WLAN_CIPHER_SUITE_BIP_GMAC_128:
> > > case WLAN_CIPHER_SUITE_BIP_GMAC_256:
> > > - if (!ieee80211_is_mgmt(hdr->frame_control))
> > > + if (!ieee80211_is_mgmt(hdr->frame_control)) {
> > > + printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
> > > tx->key = NULL;
> > > + }
> > > break;
> > > }
> > >
> > > @@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
> > > tx->skb->protocol != tx->sdata->control_port_protocol)
> > > return TX_DROP;
> > >
> > > + printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
> > > + __func__, skip_hw, tx->key);
> > > if (!skip_hw && tx->key &&
> > > - tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
> > > + tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
> > > info->control.hw_key = &tx->key->conf;
> > > + printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
> > > + }
> > > } else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
> > > test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
> > > return TX_DROP;
> > > @@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> > > * The key can be removed while the packet was queued, so need to call
> > > * this here to get the current key.
> > > */
> > > + printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
> > > + __func__, info->control.hw_key);
> > > info->control.hw_key = NULL;
> > > r = ieee80211_tx_h_select_key(&tx);
> > > if (r != TX_CONTINUE) {
> > >
> > > This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk)
> > > and so the updating of info->contro.hw_key is skipped:
> > >
> > > [ 17.411298] [ T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL
> >
> > That means that we are trying to send non management frames using
> > AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group
> > management frames ?
> >
> > > [ 17.411300] [ T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
> > > [ 17.411307] [ T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
> > >
> > > If I revert commit a104042e2bf6 while keeping the debug patches it shows that
> > > the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e
> > > relies on the key your patch is setting to NULL.
> > >
> > > Is this a problem with your patch or with the mt7921e driver that just got
> > > revealed by your patch?
> >
> > Not sure yet, do you happen to know which kind of frame mt7921e is
> > trying to be sent using this NULL key ?
> >
> > Thanks,
>
> I modified my debugging patch to print mgmt->frame_control, if needed I could
> also insert a nore complicated function printing out frame types using the
> ieee80211_is_*() functions:
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> index 881812ba03ff..cfbe7e1e4713 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> @@ -13,6 +13,9 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void
> *txwi_ptr,
> struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
> struct ieee80211_key_conf *key = info->control.hw_key;
> + struct ieee80211_mgmt *mgmt = (void *)tx_info->skb->data;
> + __le16 fc = mgmt->frame_control;
> + dev_info(mdev->dev, "%s: key = %px fc = 0x%hx\n", __func__, key, fc);
> struct mt76_connac_hw_txp *txp;
> struct mt76_txwi_cache *t;
> int id, pid;
>
> and get this, while unsuccesfully trying to connect (also note that one time
> getting a key worked):
>
> $ dmesg | grep prepare_skb
> [ 11.775642] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xb0
> [ 11.800047] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x0
> [ 13.365330] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xb0
> [ 13.370257] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x0
> [ 16.468481] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xb0
> [ 16.472407] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x0
> [ 16.542017] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x188
> [ 16.549581] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x188
> [ 16.597120] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xffff
> [ 16.612263] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0xd0
>
> Here we actually go a key:
> [ 16.614478] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> ffff89c275297230 fc = 0x4188
>
> [ 16.654273] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x3333
> [ 16.698286] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x3333
> [ 17.735855] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
> 0000000000000000 fc = 0x3333
> [ 17.837355] [ T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
[....]
I have ordered a mt7921 card so I could reproduce this and finally took
time to debug that. The issue comes to the fact that mt7921 uses 802.11
encapsulation offloading and as such we are calling
ieee80211_tx_h_select_key() on a 802.3 frame.
This function casts the skb data as a 802.11 header regardless the skb
is 802.11 or not in order to decide if the info->control.hw_key needs to
be set. So the hw_key is likely to remain NULL in ieee80211_tx_dequeue()
and because mt7921 driver needs this key to be valid data frames are
dropped.
Will send a patch so that ieee80211_tx_h_select_key() does not try to
get info from a ieee80211_hdr mapped on 802.3 packet data (i.e. when
IEEE80211_TX_CTL_HW_80211_ENCAP is set).
Thanks
--
Remi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-07-17 13:21 ` Remi Pommarel
@ 2025-07-17 13:46 ` Johannes Berg
2025-07-17 14:22 ` Remi Pommarel
0 siblings, 1 reply; 13+ messages in thread
From: Johannes Berg @ 2025-07-17 13:46 UTC (permalink / raw)
To: Remi Pommarel, Bert Karwatzki
Cc: linux-wireless, linux-kernel, linux-mediatek
Hi,
On Thu, 2025-07-17 at 15:21 +0200, Remi Pommarel wrote:
> I have ordered a mt7921 card so I could reproduce this and finally took
> time to debug that.
Oh wow, talk about dedication. Thank you!
> The issue comes to the fact that mt7921 uses 802.11
> encapsulation offloading and as such we are calling
> ieee80211_tx_h_select_key() on a 802.3 frame.
Oh... Guess we could've thought about that, sorry. Though I would've
thought ath12k will also do that. Maybe not in the config you were
seeing the issue in?
> This function casts the skb data as a 802.11 header regardless the skb
> is 802.11 or not in order to decide if the info->control.hw_key needs to
> be set. So the hw_key is likely to remain NULL in ieee80211_tx_dequeue()
> and because mt7921 driver needs this key to be valid data frames are
> dropped.
>
> Will send a patch so that ieee80211_tx_h_select_key() does not try to
> get info from a ieee80211_hdr mapped on 802.3 packet data (i.e. when
> IEEE80211_TX_CTL_HW_80211_ENCAP is set).
>
Sounds good!
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-07-17 13:46 ` Johannes Berg
@ 2025-07-17 14:22 ` Remi Pommarel
2025-07-17 15:46 ` Jeff Johnson
0 siblings, 1 reply; 13+ messages in thread
From: Remi Pommarel @ 2025-07-17 14:22 UTC (permalink / raw)
To: Johannes Berg
Cc: Bert Karwatzki, linux-wireless, linux-kernel, linux-mediatek
On Thu, Jul 17, 2025 at 03:46:00PM +0200, Johannes Berg wrote:
> Hi,
>
> On Thu, 2025-07-17 at 15:21 +0200, Remi Pommarel wrote:
> > I have ordered a mt7921 card so I could reproduce this and finally took
> > time to debug that.
>
> Oh wow, talk about dedication. Thank you!
>
> > The issue comes to the fact that mt7921 uses 802.11
> > encapsulation offloading and as such we are calling
> > ieee80211_tx_h_select_key() on a 802.3 frame.
>
> Oh... Guess we could've thought about that, sorry. Though I would've
> thought ath12k will also do that. Maybe not in the config you were
> seeing the issue in?
I've mainly tested on ath*k HW. Either without 802.11 encapsulation HW
offloading or with it but only on ath12k and it seems that this driver
does not need the key in this mode. After a quick look into ath11k and
10k that also seems to be the case for those chips.
For the record, if I am not mistaken, ath12k driver does not seem to
support 802.11 encaps HW offloading mainline yet, I do seem to have some
vendor patches to support it though.
>
> > This function casts the skb data as a 802.11 header regardless the skb
> > is 802.11 or not in order to decide if the info->control.hw_key needs to
> > be set. So the hw_key is likely to remain NULL in ieee80211_tx_dequeue()
> > and because mt7921 driver needs this key to be valid data frames are
> > dropped.
> >
> > Will send a patch so that ieee80211_tx_h_select_key() does not try to
> > get info from a ieee80211_hdr mapped on 802.3 packet data (i.e. when
> > IEEE80211_TX_CTL_HW_80211_ENCAP is set).
> >
>
> Sounds good!
>
> johannes
--
Remi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-07-17 14:22 ` Remi Pommarel
@ 2025-07-17 15:46 ` Jeff Johnson
2025-07-17 19:42 ` Jeff Johnson
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Johnson @ 2025-07-17 15:46 UTC (permalink / raw)
To: Remi Pommarel, Johannes Berg
Cc: Bert Karwatzki, linux-wireless, linux-kernel, linux-mediatek
On 7/17/2025 7:22 AM, Remi Pommarel wrote:
> For the record, if I am not mistaken, ath12k driver does not seem to
> support 802.11 encaps HW offloading mainline yet, I do seem to have some
> vendor patches to support it though.
Upstreaming this functionality is in the internal pipeline but won't make the
v6.17 merge window.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()
2025-07-17 15:46 ` Jeff Johnson
@ 2025-07-17 19:42 ` Jeff Johnson
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Johnson @ 2025-07-17 19:42 UTC (permalink / raw)
To: Remi Pommarel, Johannes Berg
Cc: Bert Karwatzki, linux-wireless, linux-kernel, linux-mediatek
On 7/17/2025 8:46 AM, Jeff Johnson wrote:
> On 7/17/2025 7:22 AM, Remi Pommarel wrote:
>> For the record, if I am not mistaken, ath12k driver does not seem to
>> support 802.11 encaps HW offloading mainline yet, I do seem to have some
>> vendor patches to support it though.
>
> Upstreaming this functionality is in the internal pipeline but won't make the
> v6.17 merge window.
I make a comment like that and all of a sudden internal reviews get expedited.
s/but won't/and should/
/jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-07-17 19:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-24 16:28 [PATCH wireless v2 0/2] Fix packets processed after vif is stopped Remi Pommarel
2025-03-24 16:28 ` [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue() Remi Pommarel
2025-04-10 21:55 ` Bert Karwatzki
2025-04-11 10:06 ` Remi Pommarel
2025-04-11 11:10 ` Bert Karwatzki
2025-04-11 12:15 ` Remi Pommarel
2025-07-17 13:21 ` Remi Pommarel
2025-07-17 13:46 ` Johannes Berg
2025-07-17 14:22 ` Remi Pommarel
2025-07-17 15:46 ` Jeff Johnson
2025-07-17 19:42 ` Jeff Johnson
2025-04-11 14:16 ` Johannes Berg
2025-03-24 16:28 ` [PATCH wireless v2 2/2] wifi: mac80211: Purge vif txq in ieee80211_do_stop() Remi Pommarel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).