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: 3612 bytes --]

Hi Nicolai,

On Thu Oct 08 2026, Nicolai Buchwitz wrote:
> On 5.10.2026 09:09, 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);
>
> Unfortunately XDP_TX and ndo_xdp_xmit() ignore the stopped queue and 
> still
> hit the freed ring. I can reproduce this on STM32MP215 with a veth 
> redirect
> into the port while toggling the pool:
>
>    pc : dwmac4_set_addr+0x8/0x18
>    lr : stmmac_xdp_xmit_xdpf+0x1d0/0x3f0
>     stmmac_xdp_xmit+0xe4/0x1a8
>     bq_xmit_all+0xa0/0x208
>     __dev_flush+0x60/0xc0
>     xdp_do_flush+0x134/0x198
>     veth_poll+0x258/0x340
>
> Both go through stmmac_xdp_xmit_xdpf() with the queue lock held, so this
> fixes it for me:
>
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
>        dma_addr_t dma_addr;
>        bool set_ic;
>
> +     /* Ring may be torn down for an XSK pool switch */
> +     if (netif_tx_queue_stopped(netdev_get_tx_queue(priv->dev, queue)))
> +             return STMMAC_XDP_CONSUMED;
> +
>        if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
>                return STMMAC_XDP_CONSUMED;
>
> This is older than your patch, but could you fold it in / add a oatch?

Thanks a lot for testing! I'll fold it in for next version.

Thanks,
Kurt

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