From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9F259C83F1A for ; Thu, 17 Jul 2025 14:24:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=U+rA0WL2vtAgdbnb0RO7hJmpVlrm4hMeQOrhAQBOlvs=; b=dmGH/CJMQIqffoOOPGk6VAVhDx KTqzVyeovnYDHtd8p3EDAIA/si0M10v5yxMNthfuayAgAgvN91vU0pG/ufDADFn5wsXeEGFLu3Uqo mmPdfkc7Vp16bfti4G0mWa9q0NY7Znx85OXYME0OLXm8WK6Pk1sqUUWjICYfDQBMOpKT8GDpMMYB3 ELUXGxLWtILg0C+xUUNG0E2obb5Svs9+UdTLSvbgy4tA41YIQNJKDzSakflstDl40ZvGDljF4H5l8 6xBZz7TSUJcT41SPqMNf8dDtHAcwGfmACQ59OJarsD6QmokW2eqAAW13QNdoxkkpv/RRzm7Eqbaj6 rwRSHvhQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1ucPXH-0000000AMLG-3gOM; Thu, 17 Jul 2025 14:24:36 +0000 Received: from e3i105.smtp2go.com ([158.120.84.105]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1ucOiw-0000000AGX6-3AcH for linux-mediatek@lists.infradead.org; Thu, 17 Jul 2025 13:32:36 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=smtpservice.net; i=@smtpservice.net; q=dns/txt; s=a1-4; t=1752759097; h=feedback-id : x-smtpcorp-track : date : message-id : to : subject : from : reply-to : sender : list-unsubscribe : list-unsubscribe-post; bh=U+rA0WL2vtAgdbnb0RO7hJmpVlrm4hMeQOrhAQBOlvs=; b=2+0tE3uR1bwPar0+FYoth0GXAznmru7gV0ZweJcfwo4eDfDPO6IZnjbwIrtINKg1nc+kd 8zb6N+lszT3jD09UBel7FseuGsWI3KIZJADrezsR3OPKkd4ygpBeXRMOyOYdjYskOOy1dsV Yd56DO57oIgrptu9ZTBteMtvnIKNlIgk2lTgM9T3F/1sk7sA20p8k3BK3Sd5i59gGM87WMS CSyCKtt4K1dNcsbmYU2bytVWyVo2BMhvU0Ml9i5im7Zkzt8fBOjVoq3+qMMWAwEMJRPbp3e lQU/X7JEOX3jx23JWSksstEr34CZ+vKlHQU/1l5/gJEErhMrQjDJLpgyNMBw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=triplefau.lt; i=@triplefau.lt; q=dns/txt; s=s510616; t=1752759097; h=from : subject : to : message-id : date; bh=U+rA0WL2vtAgdbnb0RO7hJmpVlrm4hMeQOrhAQBOlvs=; b=Bx8kPu92SaOwIdIZfCSX9b1hMfQB7/SHzjpRa7rlMQmdMCvHwbJ1tpo4ng9IKv0zy7zb9 m0MKviWVAksqsSzgVuKkuIQjLm1XYgn9/nrykftIAjbqTXrxTGREsdQuQNjdXmvPHxVPWkp cgXQkTNGzyQQQEnnqyqi0LpNoQSCejoPtlW346w2zqcOrk5192r2knGwcc2gfZbPA0RTWsa Fssv0Szv8UpChY6MCFv+gC8ZQvGKNH7zRRuQj6X89J0dGMk1kH1GqcEql9SGwk6Ktbs2KSr g/ddK91xHzMgVJ5ktl+C16c3Y4JtClt/CUwmFPkYm4GGU8ylugRXOzMZbzZg== Received: from [10.12.239.196] (helo=localhost) by smtpcorp.com with esmtpsa (TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.98.1-S2G) (envelope-from ) id 1ucOhx-4o5NDgrvtl6-ppbp; Thu, 17 Jul 2025 13:31:33 +0000 Date: Thu, 17 Jul 2025 15:21:17 +0200 From: Remi Pommarel To: Bert Karwatzki Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, johannes@sipsolutions.net, linux-mediatek@lists.infradead.org Subject: Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue() Message-ID: References: <06aa507b853ca385ceded81c18b0a6dd0f081bc8.1742833382.git.repk@triplefau.lt> <20250410215527.3001-1-spasswolf@web.de> <1df3a3df19b77e3b8d1f71a3a93c61221ff46a6b.camel@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1df3a3df19b77e3b8d1f71a3a93c61221ff46a6b.camel@web.de> X-Report-Abuse: Please forward a copy of this message, including all headers, to Feedback-ID: 510616m:510616apGKSTK:510616sBGiWowcYh X-smtpcorp-track: sAeunXHnvd5a.E-ROFHZrQ_L3.lfWsVAV8NWv X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250717_063235_272213_B65FC460 X-CRM114-Status: GOOD ( 44.25 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org 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