Re: [PATCH net-next V5 2/2] veth: apply qdisc backpressure on full ptr_ring to reduce TX drops

[Toshiaki Makita <toshiaki.makita1@gmail.com>]

On 2025/04/17 22:55, Jesper Dangaard Brouer wrote:
...
> +	case NETDEV_TX_BUSY:
> +		/* If a qdisc is attached to our virtual device, returning
> +		 * NETDEV_TX_BUSY is allowed.
> +		 */
> +		txq = netdev_get_tx_queue(dev, rxq);
> +
> +		if (qdisc_txq_has_no_queue(txq)) {
> +			dev_kfree_skb_any(skb);
> +			goto drop;
> +		}
> +		netif_tx_stop_queue(txq);
> +		/* Restore Eth hdr pulled by dev_forward_skb/eth_type_trans */
> +		__skb_push(skb, ETH_HLEN);
> +		if (use_napi)
> +			__veth_xdp_flush(rq);
> +		/* Cancel TXQ stop for very unlikely race */
> +		if (unlikely(__ptr_ring_empty(&rq->xdp_ring)))
> +			netif_tx_wake_queue(txq);

xdp_ring is only initialized when use_napi is not NULL.
Should add "if (use_napi)" ?

BTW, you added a check for the ring_empty here. so

if empty:
   this function starts the queue by itself
else:
   it is guaranteed that veth_xdp_rcv() consumes the ring after this point.
   so the rcv side definitely starts the queue.

With that, __veth_xdp_flush invocation seems to be unnecessary,
if your concern is starting the queue.

Toshiaki Makita