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.
