diff mbox series

wifi: mt76: mt7921: fix skb leak by txs missing in AMSDU

Message ID b45ef06de62ce4f02d35ecef26ba78c7a7e75b51.1684153148.git.deren.wu@mediatek.com
State Superseded
Headers show
Series wifi: mt76: mt7921: fix skb leak by txs missing in AMSDU | expand

Commit Message

Deren Wu May 15, 2023, 2:18 p.m. UTC
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(-)

Comments

Simon Horman May 16, 2023, 2:33 p.m. UTC | #1
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
>
Sean Wang May 16, 2023, 9:12 p.m. UTC | #2
Hi, Deren

On Mon, May 15, 2023 at 7:21 AM Deren Wu <deren.wu@mediatek.com> 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")

The patch with the identifier "182071cdd594 ("mt76: connac: move
connac2_mac_write_txwi in mt76_connac module") was merely a
refactoring patch and not the root cause of the issue. Therefore, I
believe it would be more appropriate to address and rectify the patch
"163f4d22c118 ("mt76: mt7921: add MAC support") instead. This will
ensure downstream branches are better informed about the necessary fix
and can accurately backport the patch.

> 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);
> +       }
>
>         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
>
diff mbox series

Patch

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);
+	}
 
 	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);