[PATCH] net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog

[PATCH] net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog

[Daniel Thompson]

Currently stmmac will run a full cycle of IRQ tear down and setup when
setting up a new XDP program. This makes tuning TSN systems difficult
because whenever a new XDP program is installed then the irq threads will
be stopped and restarted which will undo any thread tuning.

The problem is avoided by removing stmmac_free_irq()/stmmac_request_irq()
from stmmac_xdp_release()/stmmac_xdp_open().

stmmac_free_irq() implicitly synchronizes interrupts and, with that
removed, I was unable to prove that later actions in
stmmac_xdp_release() are safe when there are concurrent interrupts. To
avoid problems let's also move the code to disable DMA interrupts earlier
in the sequence and explicitly sync the interrupts handler(s).

Signed-off-by: Daniel Thompson <daniel@riscstar.com>
---
This patch was developed (and tested) as part of the TC956x work that
Alex Elder and I have been working on. However the change should work with
any driver that uses the stmmac core so it makes sense to submit it as
a separate patch.
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 61 ++++++++++++++++++-----
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2a0d7eff88d3ff1ffa57e224c25f9e77bc28ed10..acd145f1f3069fde6043a9118c793758c5c8f4c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2531,6 +2531,23 @@ static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
 	}
 }
 
+static void stmmac_disable_all_dma_irq(struct stmmac_priv *priv)
+{
+	u8 rx_channels_count = priv->plat->rx_queues_to_use;
+	u8 tx_channels_count = priv->plat->tx_queues_to_use;
+	u8 dma_csr_ch = max(rx_channels_count, tx_channels_count);
+	u8 chan;
+
+	for (chan = 0; chan < dma_csr_ch; chan++) {
+		struct stmmac_channel *ch = &priv->channel[chan];
+		unsigned long flags;
+
+		spin_lock_irqsave(&ch->lock, flags);
+		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
+		spin_unlock_irqrestore(&ch->lock, flags);
+	}
+}
+
 /**
  * stmmac_start_all_dma - start all RX and TX DMA channels
  * @priv: driver private structure
@@ -3814,6 +3831,33 @@ static void stmmac_free_irq(struct net_device *dev,
 	}
 }
 
+static void stmmac_synchronize_irq(struct net_device *dev)
+{
+	struct stmmac_priv *priv = netdev_priv(dev);
+	struct stmmac_msi *msi = priv->msi;
+	int j;
+
+	for (j = priv->plat->tx_queues_to_use - 1; msi && j >= 0; j--) {
+		if (msi->tx_irq[j] > 0)
+			synchronize_irq(msi->tx_irq[j]);
+	}
+
+	for (j = priv->plat->rx_queues_to_use - 1; msi && j >= 0; j--) {
+		if (msi->rx_irq[j] > 0)
+			synchronize_irq(msi->rx_irq[j]);
+	}
+
+	if (msi && msi->sfty_ue_irq > 0 && msi->sfty_ue_irq != dev->irq)
+		synchronize_irq(msi->sfty_ue_irq);
+	if (msi && msi->sfty_ce_irq > 0 && msi->sfty_ce_irq != dev->irq)
+		synchronize_irq(msi->sfty_ce_irq);
+	if (priv->wol_irq > 0 && priv->wol_irq != dev->irq)
+		synchronize_irq(priv->wol_irq);
+	if (priv->sfty_irq > 0 && priv->sfty_irq != dev->irq)
+		synchronize_irq(priv->sfty_irq);
+	synchronize_irq(dev->irq);
+}
+
 static int stmmac_msi_init(struct stmmac_priv *priv,
 			   struct stmmac_resources *res)
 {
@@ -7108,8 +7152,9 @@ void stmmac_xdp_release(struct net_device *dev)
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
 		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
 
-	/* Free the IRQ lines */
-	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
+	/* Silence DMA interrupts */
+	stmmac_disable_all_dma_irq(priv);
+	stmmac_synchronize_irq(dev);
 
 	/* Stop TX/RX DMA channels */
 	stmmac_stop_all_dma(priv);
@@ -7156,10 +7201,8 @@ int stmmac_xdp_open(struct net_device *dev)
 	stmmac_reset_queues_param(priv);
 
 	/* DMA CSR Channel configuration */
-	for (chan = 0; chan < dma_csr_ch; chan++) {
+	for (chan = 0; chan < dma_csr_ch; chan++)
 		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
-		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
-	}
 
 	/* Adjust Split header */
 	sph_en = (priv->hw->rx_csum > 0) && priv->sph_active;
@@ -7197,10 +7240,6 @@ int stmmac_xdp_open(struct net_device *dev)
 	/* Start Rx & Tx DMA Channels */
 	stmmac_start_all_dma(priv);
 
-	ret = stmmac_request_irq(dev);
-	if (ret)
-		goto irq_error;
-
 	/* Enable NAPI process*/
 	stmmac_enable_all_queues(priv);
 	netif_carrier_on(dev);
@@ -7209,10 +7248,6 @@ int stmmac_xdp_open(struct net_device *dev)
 
 	return 0;
 
-irq_error:
-	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
-		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
-
 init_error:
 	free_dma_desc_resources(priv, &priv->dma_conf);
 dma_desc_error:

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260706-tc956x-stmmac-no_irq_teardown-8a7592b9f8c1

Best regards,
-- 
Daniel Thompson <daniel@riscstar.com>

Re: [PATCH] net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog

[Daniel Thompson]

On Mon, Jul 06, 2026 at 05:17:58PM +0100, Daniel Thompson wrote:
> Currently stmmac will run a full cycle of IRQ tear down and setup when
> setting up a new XDP program. This makes tuning TSN systems difficult
> because whenever a new XDP program is installed then the irq threads will
> be stopped and restarted which will undo any thread tuning.
>
> The problem is avoided by removing stmmac_free_irq()/stmmac_request_irq()
> from stmmac_xdp_release()/stmmac_xdp_open().
>
> stmmac_free_irq() implicitly synchronizes interrupts and, with that
> removed, I was unable to prove that later actions in
> stmmac_xdp_release() are safe when there are concurrent interrupts. To
> avoid problems let's also move the code to disable DMA interrupts earlier
> in the sequence and explicitly sync the interrupts handler(s).
>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> <snip>
> @@ -7156,10 +7201,8 @@ int stmmac_xdp_open(struct net_device *dev)
>  	stmmac_reset_queues_param(priv);
>
>  	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++) {
> +	for (chan = 0; chan < dma_csr_ch; chan++)
>  		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> -		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> -	}

Sashiko picked up that stmmac_init_chan() has the effect of enabling DMA
irqs, making this code unsafe.

Given stmmac_xdp_open() can only be called on a running interface (and that
stmmac_xdp_open() explicitly sets sph) then re-initializing with stmmac_init_chan()
should have no effect and we can drop that as well.

I'll double check with a code review across all the dwmac versions
before pushing out a v2!


Daniel.

Re: [PATCH] net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog

[Andrew Lunn]

On Tue, Jul 07, 2026 at 10:48:50AM +0100, Daniel Thompson wrote:
> On Mon, Jul 06, 2026 at 05:17:58PM +0100, Daniel Thompson wrote:
> > Currently stmmac will run a full cycle of IRQ tear down and setup when
> > setting up a new XDP program. This makes tuning TSN systems difficult
> > because whenever a new XDP program is installed then the irq threads will
> > be stopped and restarted which will undo any thread tuning.
> >
> > The problem is avoided by removing stmmac_free_irq()/stmmac_request_irq()
> > from stmmac_xdp_release()/stmmac_xdp_open().
> >
> > stmmac_free_irq() implicitly synchronizes interrupts and, with that
> > removed, I was unable to prove that later actions in
> > stmmac_xdp_release() are safe when there are concurrent interrupts. To
> > avoid problems let's also move the code to disable DMA interrupts earlier
> > in the sequence and explicitly sync the interrupts handler(s).
> >
> > Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> > <snip>
> > @@ -7156,10 +7201,8 @@ int stmmac_xdp_open(struct net_device *dev)
> >  	stmmac_reset_queues_param(priv);
> >
> >  	/* DMA CSR Channel configuration */
> > -	for (chan = 0; chan < dma_csr_ch; chan++) {
> > +	for (chan = 0; chan < dma_csr_ch; chan++)
> >  		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> > -		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> > -	}
> 
> Sashiko picked up that stmmac_init_chan() has the effect of enabling DMA
> irqs, making this code unsafe.
> 
> Given stmmac_xdp_open() can only be called on a running interface (and that
> stmmac_xdp_open() explicitly sets sph) then re-initializing with stmmac_init_chan()
> should have no effect and we can drop that as well.

Something i like to see is symmetry. In theory, stmmac_xdp_open() and
stmmac_xdp_release() should be mirrors of each other. But i don't see
anything like the opposite of stmmac_init_chan() in
stmmac_xdp_release(). So i can well understand that something is
wrong.

Maybe rather than just fixing this IRQ issue, you can look at both
functions and see about making them symmetric? I've no idea what that
actually entails, it could be a can of worms, which is not so uncommon
with stmmmac.

     Andrew

Re: [PATCH] net: stmmac: Avoid freeing and re-requesting IRQ during XDP set prog

[Maciej Fijalkowski]

On Mon, Jul 06, 2026 at 05:17:58PM +0100, Daniel Thompson wrote:
> Currently stmmac will run a full cycle of IRQ tear down and setup when
> setting up a new XDP program. This makes tuning TSN systems difficult
> because whenever a new XDP program is installed then the irq threads will
> be stopped and restarted which will undo any thread tuning.
> 
> The problem is avoided by removing stmmac_free_irq()/stmmac_request_irq()
> from stmmac_xdp_release()/stmmac_xdp_open().
> 
> stmmac_free_irq() implicitly synchronizes interrupts and, with that
> removed, I was unable to prove that later actions in
> stmmac_xdp_release() are safe when there are concurrent interrupts. To
> avoid problems let's also move the code to disable DMA interrupts earlier
> in the sequence and explicitly sync the interrupts handler(s).
> 
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>

Is that a fix or a new feature? You need to specify the tree you're
targetting this change (net/net-next).

You've also got Sashiko review so please resolve reported issues there.

However the scope reduction of toggling interface state when loading XDP
prog seems correct approach. For intel drivers we started with resetting
whole PF and then realized it's pretty much an overkill and we can achieve
prog load via only down/up of an interface.

> ---
> This patch was developed (and tested) as part of the TC956x work that
> Alex Elder and I have been working on. However the change should work with
> any driver that uses the stmmac core so it makes sense to submit it as
> a separate patch.
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 61 ++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 2a0d7eff88d3ff1ffa57e224c25f9e77bc28ed10..acd145f1f3069fde6043a9118c793758c5c8f4c0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2531,6 +2531,23 @@ static void stmmac_enable_all_dma_irq(struct stmmac_priv *priv)
>  	}
>  }
>  
> +static void stmmac_disable_all_dma_irq(struct stmmac_priv *priv)
> +{
> +	u8 rx_channels_count = priv->plat->rx_queues_to_use;
> +	u8 tx_channels_count = priv->plat->tx_queues_to_use;
> +	u8 dma_csr_ch = max(rx_channels_count, tx_channels_count);
> +	u8 chan;
> +
> +	for (chan = 0; chan < dma_csr_ch; chan++) {
> +		struct stmmac_channel *ch = &priv->channel[chan];
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&ch->lock, flags);
> +		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> +		spin_unlock_irqrestore(&ch->lock, flags);
> +	}
> +}
> +
>  /**
>   * stmmac_start_all_dma - start all RX and TX DMA channels
>   * @priv: driver private structure
> @@ -3814,6 +3831,33 @@ static void stmmac_free_irq(struct net_device *dev,
>  	}
>  }
>  
> +static void stmmac_synchronize_irq(struct net_device *dev)
> +{
> +	struct stmmac_priv *priv = netdev_priv(dev);
> +	struct stmmac_msi *msi = priv->msi;
> +	int j;
> +
> +	for (j = priv->plat->tx_queues_to_use - 1; msi && j >= 0; j--) {
> +		if (msi->tx_irq[j] > 0)
> +			synchronize_irq(msi->tx_irq[j]);
> +	}
> +
> +	for (j = priv->plat->rx_queues_to_use - 1; msi && j >= 0; j--) {
> +		if (msi->rx_irq[j] > 0)
> +			synchronize_irq(msi->rx_irq[j]);
> +	}
> +
> +	if (msi && msi->sfty_ue_irq > 0 && msi->sfty_ue_irq != dev->irq)
> +		synchronize_irq(msi->sfty_ue_irq);
> +	if (msi && msi->sfty_ce_irq > 0 && msi->sfty_ce_irq != dev->irq)
> +		synchronize_irq(msi->sfty_ce_irq);
> +	if (priv->wol_irq > 0 && priv->wol_irq != dev->irq)
> +		synchronize_irq(priv->wol_irq);
> +	if (priv->sfty_irq > 0 && priv->sfty_irq != dev->irq)
> +		synchronize_irq(priv->sfty_irq);
> +	synchronize_irq(dev->irq);
> +}
> +
>  static int stmmac_msi_init(struct stmmac_priv *priv,
>  			   struct stmmac_resources *res)
>  {
> @@ -7108,8 +7152,9 @@ void stmmac_xdp_release(struct net_device *dev)
>  	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
>  		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
>  
> -	/* Free the IRQ lines */
> -	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
> +	/* Silence DMA interrupts */
> +	stmmac_disable_all_dma_irq(priv);
> +	stmmac_synchronize_irq(dev);
>  
>  	/* Stop TX/RX DMA channels */
>  	stmmac_stop_all_dma(priv);
> @@ -7156,10 +7201,8 @@ int stmmac_xdp_open(struct net_device *dev)
>  	stmmac_reset_queues_param(priv);
>  
>  	/* DMA CSR Channel configuration */
> -	for (chan = 0; chan < dma_csr_ch; chan++) {
> +	for (chan = 0; chan < dma_csr_ch; chan++)
>  		stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
> -		stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 1);
> -	}
>  
>  	/* Adjust Split header */
>  	sph_en = (priv->hw->rx_csum > 0) && priv->sph_active;
> @@ -7197,10 +7240,6 @@ int stmmac_xdp_open(struct net_device *dev)
>  	/* Start Rx & Tx DMA Channels */
>  	stmmac_start_all_dma(priv);
>  
> -	ret = stmmac_request_irq(dev);
> -	if (ret)
> -		goto irq_error;
> -
>  	/* Enable NAPI process*/
>  	stmmac_enable_all_queues(priv);
>  	netif_carrier_on(dev);
> @@ -7209,10 +7248,6 @@ int stmmac_xdp_open(struct net_device *dev)
>  
>  	return 0;
>  
> -irq_error:
> -	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> -		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
> -
>  init_error:
>  	free_dma_desc_resources(priv, &priv->dma_conf);
>  dma_desc_error:
> 
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260706-tc956x-stmmac-no_irq_teardown-8a7592b9f8c1
> 
> Best regards,
> -- 
> Daniel Thompson <daniel@riscstar.com>
> 
>