Re: [PATCH net 1/2] net: stmmac: Track the TX DMA threshold per queue

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi,

On 10/13/26 13:45, Ovidiu Panait wrote:
> Commit 3a6c12a0c6c3 ("net: stmmac: bump tc when get underflow error from
> DMA descriptor") made dwmac4 raise the DMA threshold of the queues that
> report TX underflows. This is done by incrementing a module global
> parameter 'tc' by 64 on each underflow.
> 
> This leads to inconsistent TX DMA thresholds between queues and causes
> issues on platforms with multiple stmmac controllers, as 'tc' is shared
> by all of them. Once 'tc' goes over 256 the bumping stops, and queues
> that did not underflow before are left at the initial threshold.
> 
> On the Renesas RZ/V2H platform, which has two dwmac4 controllers, we
> observed sporadic iperf3 TCP TX failures:
> 
>   iperf3 -c <server> -t5 --bind-dev end1
>   ...
>   [  5]   0.00-5.00   sec  1.00 MBytes  1.68 Mbits/sec   87       sender
>   [  5]   0.00-5.00   sec   768 KBytes  1.26 Mbits/sec            receiver
> 
> This happens because the TX queues of end1 were underflowing on a large
> fraction of the packets, but the DMA TX threshold was never raised from
> the default of 64 bytes. The threshold was not raised because end0
> had underflowed first and had already pushed 'tc' past 256. Which
> controller is affected depends on which one underflows first.
> 
> To fix this, make the TX DMA threshold per queue instead of global.
> Each time an underflow happens on a queue, its TX DMA threshold gets
> incremented, without affecting other queues or controllers. With this
> change, 'tc' is only used as the initial value, and the RX threshold
> is no longer bumped together with the TX one, as TX underflows have
> nothing to do with the RX side.
> 
> Fixes: 3a6c12a0c6c3 ("net: stmmac: bump tc when get underflow error from DMA descriptor")
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  1 +
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 33 +++++++++++--------
>  2 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index 9278378407e8..85d63450693e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -270,6 +270,7 @@ struct stmmac_priv {
>  	bool sph_capable;
>  	u32 sarc_type;
>  	u32 rx_riwt[MTL_MAX_RX_QUEUES];
> +	u32 tx_threshold[MTL_MAX_TX_QUEUES];
>  	int hwts_rx_en;
>  	bool tsfupdt_coarse;
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b8b4de4c5b1a..b5ee0f297df6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2622,7 +2622,6 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>  	}
>  
>  	if (priv->plat->force_thresh_dma_mode) {
> -		txmode = tc;
>  		rxmode = tc;
>  	} else if (priv->plat->force_sf_dma_mode || priv->plat->tx_coe) {
>  		/*
> @@ -2636,7 +2635,6 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>  		rxmode = SF_DMA_MODE;
>  		priv->xstats.threshold = SF_DMA_MODE;
>  	} else {
> -		txmode = tc;
>  		rxmode = SF_DMA_MODE;
>  	}
>  
> @@ -2655,8 +2653,11 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>  	for (chan = 0; chan < tx_channels_count; chan++) {
>  		qmode = priv->plat->tx_queues_cfg[chan].mode_to_use;
>  
> -		stmmac_dma_tx_mode(priv, priv->ioaddr, txmode, chan,
> -				txfifosz, qmode);
> +		if (txmode == SF_DMA_MODE)
> +			priv->tx_threshold[chan] = SF_DMA_MODE;
> +
> +		stmmac_dma_tx_mode(priv, priv->ioaddr, priv->tx_threshold[chan],
> +				   chan, txfifosz, qmode);
>  	}
>  }
>  
> @@ -2822,17 +2823,20 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
>  
>  static void stmmac_bump_dma_threshold(struct stmmac_priv *priv, u32 chan)
>  {
> -	if (unlikely(priv->xstats.threshold != SF_DMA_MODE) && tc <= 256) {
> -		tc += 64;
> +	u32 thresh = priv->tx_threshold[chan];
>  
> -		if (priv->plat->force_thresh_dma_mode)
> -			stmmac_set_dma_operation_mode(priv, tc, tc, chan);
> -		else
> -			stmmac_set_dma_operation_mode(priv, tc, SF_DMA_MODE,
> -						      chan);
> +	if (unlikely(thresh == SF_DMA_MODE) || thresh > 256)
> +		return;
>  
> -		priv->xstats.threshold = tc;
> -	}
> +	thresh += 64;
> +
> +	if (priv->plat->force_thresh_dma_mode)
> +		stmmac_set_dma_operation_mode(priv, thresh, tc, chan);
> +	else
> +		stmmac_set_dma_operation_mode(priv, thresh, SF_DMA_MODE, chan);
> +
> +	priv->tx_threshold[chan] = thresh;
> +	priv->xstats.threshold = thresh;
>  }
>  
>  /**
> @@ -8133,6 +8137,9 @@ static int __stmmac_dvr_probe(struct device *device,
>  
>  	priv->xstats.threshold = tc;
>  
> +	for (i = 0; i < MTL_MAX_TX_QUEUES; i++)
> +		priv->tx_threshold[i] = tc;
> +
>  	/* Initialize RSS */
>  	rxq = priv->plat->rx_queues_to_use;
>  	netdev_rss_key_fill(priv->rss.key, sizeof(priv->rss.key));