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

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 24 Apr 2025 14:56:49 +0200 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);
> +		/* Depend on prior success packets started NAPI consumer via
> +		 * __veth_xdp_flush(). Cancel TXQ stop if consumer stopped,
> +		 * paired with empty check in veth_poll().
> +		 */
> +		if (unlikely(__ptr_ring_empty(&rq->xdp_ring)))
> +			netif_tx_wake_queue(txq);

Looks like I wrote a reply to v5 but didn't hit send. But I may have
set v5 to Changes Requested because of it :S Here is my comment:

 I think this is missing a memory barrier. When drivers do this dance
 there's usually a barrier between stop and recheck, to make sure the
 stop is visible before we check. And vice versa veth_xdp_rcv() needs
 to make sure other side sees the "empty" indication before it checks 
 if the queue is stopped.