Subject: Re: [PATCH] net/sched: netem: account for backlog updates from child qdisc

From: Stephen Hemminger <[email-masked]>
Date: Mon, 25 Nov 2024 16:12:22 -0800

On Tue, 26 Nov 2024 00:18:25 +0100
Martin Ottens <[email-masked]> wrote:

> When netem is used with a child qdisc, the child qdisc can use
> 'qdisc_tree_reduce_backlog' to inform its parent, netem, about created or
> dropped SKBs. This function updates 'qlen' and the backlog statistics of
> netem, but netem does not account for changes made by a child qdisc. If a
> child qdisc creates new SKBs during enqueue and informs its parent about
> this, netem's 'qlen' value is increased. When netem dequeues the newly
> created SKBs from the child, the 'qlen' in netem is not updated. If 'qlen'
> reaches the configured limit, the enqueue function stops working, even
> though the tfifo is not full.
> 
> Reproduce the bug:
> Ensure that the sender machine has GSO enabled. Configure netem as root
> qdisc and tbf as its child on the outgoing interface of the machine
> as follows:
> $ tc qdisc add dev <oif> root handle 1: netem delay 100ms limit 100
> $ tc qdisc add dev <oif> parent 1:0 tbf rate 50Mbit burst 1542 latency 50ms
> 
> Send bulk TCP traffic out via this interface, e.g., by running an iPerf3
> client on the machine. Check the qdisc statistics:
> $ tc -s qdisc show dev <oif>
> 
> tbf segments the GSO SKBs (tbf_segment) and updates the netem's 'qlen'.
> The interface fully stops transferring packets and "locks". In this case,
> the child qdisc and tfifo are empty, but 'qlen' indicates the tfifo is at
> its limit and no more packets are accepted.
> 
> This patch adds a counter for the entries in the tfifo. Netem's 'qlen' is
> only decreased when a packet is returned by its dequeue function, and not
> during enqueuing into the child qdisc. External updates to 'qlen' are thus
> accounted for and only the behavior of the backlog statistics changes.
> This statistics now show the total number/length of SKBs held in the tfifo
> and in the child qdisc. (Note: the 'backlog' byte-statistic of netem is
> incorrect in the example above even after the patch is applied due to a
> bug in tbf. See my previous patch ([PATCH] net/sched: tbf: correct backlog
> statistic for GSO packets)).
> 
> Signed-off-by: Martin Ottens <[email-masked]>

Does this also address this issue with running non-work conserving
inner qdisc?

See: https://marc.info/?l=linux-netdev&m=172779429511319&w=2