Re: [PATCH net-next 0/5] veth: add Byte Queue Limits (BQL) support

["Toke Høiland-Jørgensen" <toke@redhat.com>]

hawk@kernel.org writes:

> From: Jesper Dangaard Brouer <hawk@kernel.org>
>
> This series adds BQL (Byte Queue Limits) to the veth driver, reducing
> latency by dynamically limiting in-flight bytes in the ptr_ring and
> moving buffering into the qdisc where AQM algorithms can act on it.
>
> Problem:
>   veth's 256-entry ptr_ring acts as a "dark buffer" -- packets queued
>   there are invisible to the qdisc's AQM.  Under load, the ring fills
>   completely (DRV_XOFF backpressure), adding up to 256 packets of
>   unmanaged latency before the qdisc even sees congestion.
>
> Solution:
>   BQL (STACK_XOFF) dynamically limits in-flight bytes, stopping the
>   queue before the ring fills.  This keeps the ring shallow and pushes
>   excess packets into the qdisc, where sojourn-based AQM can measure
>   and drop them.

So one question here: Is *Byte* queue limits really the right thing for
veth? As you mention above, the ptr_ring is sized in a number of
packets. On a physical NIC, accounting bytes makes sense because there's
a fixed line rate, so bytes turn directly into latency.

But on a veth device, the stack processing is per packet, and most
processing takes the same amount of time regardless of the size of the
packet (e.g., netfilter rules that operate on the skb only).

So my worry would be that when you're accounting in bytes, if there's a
mix of big and small packets, you'd end up with the BQL algorithm
scaling to a "too large" value, which would allow a lot of small packets
to be queued up, adding extra latency (or even overflowing the ring
buffer if the ratio is large enough).

Have you run any such experiments? And have you tried just accounting
the queue in packets, so instead of:

+		netdev_tx_sent_queue(txq, skb->len);

you'd just do:

+		netdev_tx_sent_queue(txq, 1);


?

-Toke