Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 16 May 2023 16:33:08 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Deren Wu <deren.wu@mediatek.com>
Cc:     Felix Fietkau <nbd@nbd.name>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Sean Wang <sean.wang@mediatek.com>,
        Soul Huang <Soul.Huang@mediatek.com>,
        Ming Yen Hsieh <mingyen.hsieh@mediatek.com>,
        Leon Yen <Leon.Yen@mediatek.com>,
        Eric-SY Chang <Eric-SY.Chang@mediatek.com>,
        KM Lin <km.lin@mediatek.com>,
        Robin Chiu <robin.chiu@mediatek.com>,
        CH Yeh <ch.yeh@mediatek.com>, Posh Sun <posh.sun@mediatek.com>,
        Stella Chang <Stella.Chang@mediatek.com>,
        Quan Zhou <quan.zhou@mediatek.com>,
        Ryder Lee <ryder.lee@mediatek.com>,
        Shayne Chen <shayne.chen@mediatek.com>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        linux-mediatek <linux-mediatek@lists.infradead.org>,
        stable@vger.kernel.org
Subject: Re: [PATCH] wifi: mt76: mt7921: fix skb leak by txs missing in AMSDU
Message-ID: <ZGOUJJgXiZLvJ03N@corigine.com>
References: <b45ef06de62ce4f02d35ecef26ba78c7a7e75b51.1684153148.git.deren.wu@mediatek.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b45ef06de62ce4f02d35ecef26ba78c7a7e75b51.1684153148.git.deren.wu@mediatek.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Y0g+bIKU47Fu04atQIKduw18hJ5l2K0BLlpswQKAE5CVQZU3G7s2y2KineOR?=
 =?us-ascii?Q?ZtDQDaQB2pgqbGWjKIILQZiR0UHr7SzfzBQCtqXlzuGApr/MflcRiBliGxHT?=
 =?us-ascii?Q?fYmiHgy4ss9aHT/rcRGk6lANVm0ptwasA31QvBraVD3R37DiH33dPe0b0bxf?=
 =?us-ascii?Q?SE1VkAe6RYbZ2AG/roKFMU3e+8OtK803ng/w6icNYA9Z0FbAO9p/whN3GYxi?=
 =?us-ascii?Q?StkgokxmfGt1+c9ZxpEHpPtlILDzvaxzGaLR1BiouupBrhbyV+GsrxKOQ2zP?=
 =?us-ascii?Q?WTl3QhF5bWR7oZjOE4ARLyOrWwLPkRW95s7UXWttYxFCoM1GPmLApMsex80x?=
 =?us-ascii?Q?Q8W+v23ozGpdAsIBxj2ETP0FYC7D73DhQ+fHKBbINFswRHX7oOhzMQLbfKWb?=
 =?us-ascii?Q?I5WRiJLD15h9nGUe6PLmnhOiWVeZ+p/wXl7EdWD9OCeznh6dr9M2vzE+B2t4?=
 =?us-ascii?Q?s+6RnkZg8WBUt2Cq6BjP6WXWADM8f24MSWBLBPdUxCzhgI0VARQOQra/I3HR?=
 =?us-ascii?Q?ZAimh0ZBIndgLcllJp93QAdRnBwTd/u0sjAF0pzv11HRChkIlnZ8SEdSTLEO?=
 =?us-ascii?Q?s6pz2GpgWNlFk4UW6U4N4Z3hc2GPTLHh0JSzNhQtggS34yig433Z1SPbSrDG?=
 =?us-ascii?Q?ERdhajUL5QHuvMULyKhNW7Ij8vDrjRe6/94aKge080lOZx9K0tla3WLTVz23?=
 =?us-ascii?Q?zmMzJujfZGHgvPa2/OodCZfwzGm3avPfPR7pgAtQaLKlUvlT53FF07B9MtAN?=
 =?us-ascii?Q?9kxFg2qEHWp4Wv3xXkJHsmt4Bh4BaFVkNZz2NdIWRaePJAgsEaZuUQHqByPL?=
 =?us-ascii?Q?lCJtP/rp1UyqFddy0cBOYPAVHZOMQIHH8VUQ8rexWNNIUArGoqUbSBCLjSWV?=
 =?us-ascii?Q?RWPAdYB+RBL8EuDK5bKvtwxCG3AM32jYm6DjTgmm2U2BocX8XT5dNGsMPo3W?=
 =?us-ascii?Q?jFkvVw6GzkfQzL8NToAMk/LCwM6ahA56y05GHzTaNQyIVBDC7db7f4/gh8xv?=
 =?us-ascii?Q?pLM7T5ZRoUjtBpt9It6l0BXGIRsmrROQ7GHbpEw9z7/K814c2rMpw+QkWe16?=
 =?us-ascii?Q?LMKLYjOCm6YuTFFN4HKdjWtr+iPRIfg3G4a376GK79Axr3Kaas+UW/c6QdPw?=
 =?us-ascii?Q?DFQ09ZTQ2lv0JZN9PwMnkmR46dn1FMf7mxdCI++q7ZHGzWIit+pqzVH1UNQC?=
 =?us-ascii?Q?2Ma6GuL/QPbUIR9MQ0S1qJL4wagpDnuovfGUGtu+TDccprSf9tMIRxHD6QK9?=
 =?us-ascii?Q?++lMwGMSz+1DKxjdk/XhRSGz2SPlUiCfkCZWX+U0u5If9qCaajaUoXrXue4z?=
 =?us-ascii?Q?qbFYC/DuhPgQfWlcV+Rfhdix63p9TZBn+R6J1VgygkI93W+9m8Q+zmFC6Plw?=
 =?us-ascii?Q?QCBYYCbp/UIGhTg6xsJ5E/G+IGLzXBJxsTysDHIKN9QeETFZABnifCIYOPcT?=
 =?us-ascii?Q?jJLT+QIti5UJLW+BCOXaAOHxUkaLvwXrQYJGa2el7tyALbWb8RF9yJZ/99i1?=
 =?us-ascii?Q?mZ7jC6Ji88PWW2eAQ3SOcGlOvZvRhbRMDAkBU15sD+KRamQTfn+vGnIT1Jue?=
 =?us-ascii?Q?Bty6YyoikoUL6RU5wJBbcWTlTGprrPRr/mYGasTMow/HKXjqg7gjuAai/SbE?=
 =?us-ascii?Q?xWA8HiJ85T7b1VXvHtAmVqepBPjv/0MtKMMhFeBIhWnZkWx30IiwslWssqPN?=
 =?us-ascii?Q?JRvTZA=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5866a266-cc88-4236-58bb-08db561a7c9a
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2023 14:33:16.7097
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: D41peU9yKRMFYSDm7uIBWx8ZfOQrfKtEE3koveYW3dbJ6gqb2R7IN6zUfHR6qpS/vglbvKAMhX5G0saQTAbUWrObSGPwa9QR1cP4yrkj1R8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR13MB4632
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

On Mon, May 15, 2023 at 10:18:05PM +0800, Deren Wu wrote:
> txs may be dropped if the frame is aggregated in AMSDU. When the problem
> shows up, some SKBs would be hold in driver to cause network stopped
> temporarily. Even if the problem can be recovered by txs timeout handling,
> mt7921 still need to disable txs in AMSDU to avoid this issue.
> 
> Cc: stable@vger.kernel.org
> Fixes: 182071cdd594 ("mt76: connac: move connac2_mac_write_txwi in mt76_connac module")
> Reviewed-by: Shayne Chen <shayne.chen@mediatek.com>
> Signed-off-by: Deren Wu <deren.wu@mediatek.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> index ee0fbfcd07d6..56c42ee1178c 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> @@ -495,6 +495,7 @@ void mt76_connac2_mac_write_txwi(struct mt76_dev *dev, __le32 *txwi,
>  				    BSS_CHANGED_BEACON_ENABLED));
>  	bool inband_disc = !!(changed & (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
>  					 BSS_CHANGED_FILS_DISCOVERY));
> +	bool amsdu_en = wcid->amsdu;
>  
>  	if (vif) {
>  		struct mt76_vif *mvif = (struct mt76_vif *)vif->drv_priv;
> @@ -554,12 +555,14 @@ void mt76_connac2_mac_write_txwi(struct mt76_dev *dev, __le32 *txwi,
>  	txwi[4] = 0;
>  
>  	val = FIELD_PREP(MT_TXD5_PID, pid);
> -	if (pid >= MT_PACKET_ID_FIRST)
> +	if (pid >= MT_PACKET_ID_FIRST) {
>  		val |= MT_TXD5_TX_STATUS_HOST;
> +		amsdu_en &= !is_mt7921(dev);

These are booleans not bitfields,
so perhaps something like this is more appropriate?

		amsdu_en = amsdu_en && !is_mt7921(dev);

> +	}
>  
>  	txwi[5] = cpu_to_le32(val);
>  	txwi[6] = 0;
> -	txwi[7] = wcid->amsdu ? cpu_to_le32(MT_TXD7_HW_AMSDU) : 0;
> +	txwi[7] = amsdu_en ? cpu_to_le32(MT_TXD7_HW_AMSDU) : 0;
>  
>  	if (is_8023)
>  		mt76_connac2_mac_write_txwi_8023(txwi, skb, wcid);
> -- 
> 2.18.0
>