Re: [PATCH net v2 2/2] net: stmmac: Stop Tx queue when (en|dis)abling XSK pools

[Kurt Kanzenbach <kurt@linutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2744 bytes --]

Hi Maciej,

On Thu Oct 08 2026, Maciej Fijalkowski wrote:
> On Mon, Oct 05, 2026 at 09:09:33AM +0200, Kurt Kanzenbach wrote:
>> When enabling or disabling XSK pools in parallel to Tx traffic, kernel
>> crashes occur. For VLAN tagged frames that happens in stmmac_xmit() ->
>> dwmac4_set_vlan_tag() and for normal frames in stmmac_xmit() ->
>> dwmac4_set_addr(). Both of these functions access the Tx DMA descriptors.
>> 
>> The XDP pool (en|dis)ablement frees and reallocates the Tx DMA resources:
>> 
>>  stmmac_disable_tx_queue:
>>    __free_dma_tx_desc_resources
>> 
>>  stmmac_enable_tx_queue:
>>    __alloc_dma_tx_desc_resources
>>    __init_dma_tx_desc_rings
>> 
>> NAPI is disabled during that allocation window, but the Tx queue is not
>> stopped. Therefore, add the stopping of the Tx queue during the enabling
>> and disabling of XSK pools. Update trans_start when stopping the queue
>> to avoid spurious watchdog timeouts.
>> 
>> The issue can be easily reproduced by:
>> 
>>  1. Run iperf
>>  2. Run application which opens an AF_XDP/ZC socket
>> 
>> Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")
>> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
>> index d7e4db7224b0..883bd3fe8089 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
>> @@ -6,6 +6,16 @@
>>  #include "stmmac.h"
>>  #include "stmmac_xdp.h"
>>  
>> +static void stmmac_xdp_stop_tx_queue(struct stmmac_priv *priv, u16 queue)
>> +{
>> +	struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
>> +
>> +	__netif_tx_lock_bh(nq);
>> +	txq_trans_cond_update(nq);
>> +	netif_tx_stop_queue(nq);
>> +	__netif_tx_unlock_bh(nq);
>> +}
>> +
>>  static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
>>  				  struct xsk_buff_pool *pool, u16 queue)
>>  {
>> @@ -36,6 +46,7 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
>>  	if (need_update) {
>>  		napi_disable(&ch->rx_napi);
>>  		napi_disable(&ch->tx_napi);
>> +		stmmac_xdp_stop_tx_queue(priv, queue);
>
> FWIW you can look at what I did at ice driver (ice_qp_dis()) where I used
> a bigger hammer here; I think updating trans_start is kinda a workaround.
>
> https://lore.kernel.org/netdev/20240708221416.625850-1-anthony.l.nguyen@intel.com/
>

Actually, trans_start is already updated by netif_tx_stop_queue(). I'd
just remove txq_trans_cond_update() for next version.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]