[PATCH v1] net: stmmac: Disable queues before tx_disable

[PATCH v1] net: stmmac: Disable queues before tx_disable

[muhammad.nazim.amirul.nazle.asmade]

From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

Disable all queues and disconnect before tx_disable in stmmac_xdp_release
to prevent a corner case where packet may be still queued at the same time
tx_disable is called resulting in kernel panic if some packet still has to
be processed.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index da206b24aaed..7f5a6f30c231 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6967,15 +6967,15 @@ void stmmac_xdp_release(struct net_device *dev)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	u32 chan;
 
-	/* Ensure tx function is not running */
-	netif_tx_disable(dev);
-
 	/* Disable NAPI process */
 	stmmac_disable_all_queues(priv);
 
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
 		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
 
+	/* Ensure tx function is not running */
+	netif_tx_disable(dev);
+
 	/* Free the IRQ lines */
 	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
 
-- 
2.43.7

Re: [PATCH v1] net: stmmac: Disable queues before tx_disable

[Jacob Keller]

On 2/3/2026 6:15 PM, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> 
> Disable all queues and disconnect before tx_disable in stmmac_xdp_release
> to prevent a corner case where packet may be still queued at the same time
> tx_disable is called resulting in kernel panic if some packet still has to
> be processed.
> 

It is helpful to include a (trimmed) copy of the panic if you have one.

> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

This is a net fix which should have the net tag in the subject and a 
Fixes commit. If it is a regression you should tag as the commit which 
introduced it. Otherwise, indicate the first commit which added 
stmmac_xdp_release().

I saw that there is also __sttmac_release which calls 
stmmac_disable_all_queues() first before calling netif_tx_disable(), so 
this aligns the XDP case with the non-XDP case, so this seems like the 
right approach.

The stmmac_disable_all_queue essentially just calls napi_disable on all 
of the appropriate queues. The netif_tx_disable() calls 
netif_tx_stop_queue() on each queue....


I don't actually know the proper sequencing here. I see several other 
drivers seem to have their napi_disable happening after the call to 
netif_tx_disable. It seems to me like the existing code should be safe, 
at least based on many other driver examples.

I think we need to see the supposed panic to understand the problem and 
whether this fix is correct.

> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index da206b24aaed..7f5a6f30c231 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6967,15 +6967,15 @@ void stmmac_xdp_release(struct net_device *dev)
>   	struct stmmac_priv *priv = netdev_priv(dev);
>   	u32 chan;
>   
> -	/* Ensure tx function is not running */
> -	netif_tx_disable(dev);
> -
>   	/* Disable NAPI process */
>   	stmmac_disable_all_queues(priv);
>   
>   	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
>   		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
>   

The only other action here is to cancel some tx_queue hrtimer? Could 
this have an impact?

> +	/* Ensure tx function is not running */
> +	netif_tx_disable(dev);
> +
>   	/* Free the IRQ lines */
>   	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
>