[PATCH] net: stmmac: Modify the judgment condition of "tx_avail" from 1 to 2

[PATCH] net: stmmac: Modify the judgment condition of "tx_avail" from 1 to 2

[2694439648]

From: "hailong.fan" <hailong.fan@siengine.com>

    Under certain conditions, a WARN_ON will be triggered
    if avail equals 1.

    For example, when a VLAN packet is to send,
    stmmac_vlan_insert consumes one unit of space,
    and the data itself consumes another.
    actually requiring 2 units of space in total.

Signed-off-by: hailong.fan <hailong.fan@siengine.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7b90ecd3a..b575384cd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4529,7 +4529,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 
-	if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) {
+	if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 2)) {
 		if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, queue))) {
 			netif_tx_stop_queue(netdev_get_tx_queue(priv->dev,
 								queue));
-- 
2.34.1

Re: [PATCH] net: stmmac: Modify the judgment condition of "tx_avail" from 1 to 2

[Simon Horman]

On Mon, Dec 01, 2025 at 10:57:01AM +0800, 2694439648@qq.com wrote:
> From: "hailong.fan" <hailong.fan@siengine.com>
> 
>     Under certain conditions, a WARN_ON will be triggered
>     if avail equals 1.
> 
>     For example, when a VLAN packet is to send,
>     stmmac_vlan_insert consumes one unit of space,
>     and the data itself consumes another.
>     actually requiring 2 units of space in total.

Hi,

I am wondering if there are other cases where an extra
descriptor is needed. And if so, can multiple such conditions
occur at the same time?

I am also wondering if the VLAN condition can be detected,
so a descriptor is only reserved for VLAN use if it will
actually be used for a VLAN.

And I think it would be worth noting how this problem was discovered
e.g. by inspection, using tooling (static analysis, AI, ...).
And how it has been tested e.g. On real HW, compile tested only.


As this is a fix for Networking code present in the net tree
it should be based on that tree. And targeted at that tree like this:

Subject: [PATCH net] ...

Also, as a fix for net, it should have a fixes tag.
Generally, this should denote the first patch where the problem would
manifest. In this case this seems to be a likely candidate:

Fixes: 30d932279dc2 ("net: stmmac: Add support for VLAN Insertion Offload")

The tag should go immediately above other tags, in this case your
Signed-off-by line, without any blank lines in between. And, like other
tags, it should not be line-wrapped.

For more information on the workflow for Networking changes please see:
https://docs.kernel.org/process/maintainer-netdev.html

> 
> Signed-off-by: hailong.fan <hailong.fan@siengine.com>

Re: [PATCH] net: stmmac: Modify the judgment condition of "tx_avail" from 1 to 2

[Eric Dumazet]

On Sun, Nov 30, 2025 at 6:57 PM <2694439648@qq.com> wrote:
>
> From: "hailong.fan" <hailong.fan@siengine.com>
>
>     Under certain conditions, a WARN_ON will be triggered
>     if avail equals 1.
>
>     For example, when a VLAN packet is to send,
>     stmmac_vlan_insert consumes one unit of space,
>     and the data itself consumes another.
>     actually requiring 2 units of space in total.
>
> Signed-off-by: hailong.fan <hailong.fan@siengine.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 7b90ecd3a..b575384cd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4529,7 +4529,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>                 }
>         }
>
> -       if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) {
> +       if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 2)) {
>                 if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, queue))) {
>                         netif_tx_stop_queue(netdev_get_tx_queue(priv->dev,
>                                                                 queue));

Drivers should stop their queues earlier.

NETDEV_TX_BUSY is almost always wrong.

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7b90ecd3a55e600458b0c87d6125831626f4683d..6dcc7b84a8759763b6471a48a6c80b1f17cd937c
100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4675,7 +4675,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff
*skb, struct net_device *dev)
                print_pkt(skb->data, skb->len);
        }

-       if (unlikely(stmmac_tx_avail(priv, queue) <= (MAX_SKB_FRAGS + 1))) {
+       if (unlikely(stmmac_tx_avail(priv, queue) <= (MAX_SKB_FRAGS + 2))) {
                netif_dbg(priv, hw, priv->dev, "%s: stop transmitted packets\n",
                          __func__);
                netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, queue));

回复: [PATCH] net: stmmac: Modify the judgment condition of "tx_avail" from 1 to 2

[Fan Hailong/范海龙]

Hi Eric

You are right, I will follow your advice to modify the relevant code.

Regards
Hailong


-----邮件原件-----
发件人: Eric Dumazet <edumazet@google.com> 
发送时间: 2025年12月1日 20:21
收件人: 2694439648@qq.com
抄送: andrew+netdev@lunn.ch; davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com; mcoquelin.stm32@gmail.com; alexandre.torgue@foss.st.com; Fan Hailong/范海龙 <hailong.fan@siengine.com>; netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; inux-kernel@vger.kernel.org
主题: Re: [PATCH] net: stmmac: Modify the judgment condition of "tx_avail" from 1 to 2

On Sun, Nov 30, 2025 at 6:57 PM <2694439648@qq.com> wrote:
>
> From: "hailong.fan" <hailong.fan@siengine.com>
>
>     Under certain conditions, a WARN_ON will be triggered
>     if avail equals 1.
>
>     For example, when a VLAN packet is to send,
>     stmmac_vlan_insert consumes one unit of space,
>     and the data itself consumes another.
>     actually requiring 2 units of space in total.
>
> Signed-off-by: hailong.fan <hailong.fan@siengine.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 7b90ecd3a..b575384cd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4529,7 +4529,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>                 }
>         }
>
> -       if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) {
> +       if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 2)) {
>                 if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, queue))) {
>                         netif_tx_stop_queue(netdev_get_tx_queue(priv->dev,
>                                                                 
> queue));

Drivers should stop their queues earlier.

NETDEV_TX_BUSY is almost always wrong.

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7b90ecd3a55e600458b0c87d6125831626f4683d..6dcc7b84a8759763b6471a48a6c80b1f17cd937c
100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4675,7 +4675,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
                print_pkt(skb->data, skb->len);
        }

-       if (unlikely(stmmac_tx_avail(priv, queue) <= (MAX_SKB_FRAGS + 1))) {
+       if (unlikely(stmmac_tx_avail(priv, queue) <= (MAX_SKB_FRAGS + 
+ 2))) {
                netif_dbg(priv, hw, priv->dev, "%s: stop transmitted packets\n",
                          __func__);
                netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, queue));