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

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

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>
> 
>