Subject: Re: [RFC PATCH net-next 2/6] veth: implement Byte Queue Limits (BQL) for latency reduction

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Thu, 19 Mar 2026 11:04:02 +0100

Jesper Dangaard Brouer <[email-masked]> writes:

> On 18/03/2026 15.28, Toke Høiland-Jørgensen wrote:
>> [email-masked] writes:
>> 
>>> From: Jesper Dangaard Brouer <[email-masked]>
>>>
>>> Add BQL support to the veth driver to dynamically limit the number of
>>> bytes queued in the ptr_ring, giving the qdisc earlier feedback to shape
>>> traffic and reduce latency.
>>>
>>> The BQL charge (netdev_tx_sent_queue) is placed in veth_xmit() BEFORE
>>> veth_forward_skb() produces the SKB into the ptr_ring. This ordering is
>>> critical: with threaded NAPI the consumer runs on a separate CPU and can
>>> complete the SKB (calling dql_completed) before veth_xmit() returns. If
>>> the charge happened after the produce, the completion could race ahead
>>> of the charge, violating dql_completed()'s invariant that completed
>>> bytes never exceed queued bytes (BUG_ON).
>>>
>>> Whether an SKB was BQL-charged is tracked per-SKB using a VETH_BQL_FLAG
>>> bit in the ptr_ring pointer (BIT(1), alongside the existing VETH_XDP_FLAG
>>> BIT(0)). The do_bql flag from veth_xmit() propagates through
>>> veth_forward_skb() and veth_xdp_rx() into the ptr_ring entry. On the
>>> completion side in veth_xdp_rcv(), veth_ptr_is_bql() reads the flag to
>>> decide whether to call netdev_tx_completed_queue(). Per-SKB tracking is
>>> necessary because the qdisc can be replaced live (e.g. noqueue->sfq or
>>> vice versa via 'tc qdisc replace') while SKBs are already in-flight in
>>> the ptr_ring. SKBs charged under the old qdisc must complete correctly
>>> regardless of what qdisc is attached when the consumer runs, so each
>>> SKB carries its own BQL-charged state rather than re-checking the peer's
>>> qdisc at completion time.
>> 
>> It's not completely obvious to me why BQL can't be active regardless of
>> whether there's a qdisc installed or not? If there's no qdisc, shouldn't
>> BQL auto-tune to a higher value because the queue runs empty more?
>> 
>
> When net_device don't have qdisc we hit this code path:
>   - [0] 
> https://elixir.bootlin.com/linux/v7.0-rc4/source/net/core/dev.c#L4806-L4852
>   - Notice the check if(!netif_xmit_stopped(txq))
>   - resulting in "Virtual device %s asks to queue packet!"
>
> We cannot unconditionally track BQL as calling netdev_tx_sent_queue()
> can result in setting STACK_XOFF.  Resulting in above code dropping
> packets and complaining.  (It have no qdisc to requeue store back-
> pressured packet).

Ah, right. I realised the packet would be dropped, of course, but I did
not realise the stack would complain. That seems... odd? Why not just
get rid of the complaint instead of having this kludge to work around
it?

-Toke