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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241125161222.40448603@hermes.local>
Date: Mon, 25 Nov 2024 16:12:22 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Martin Ottens <martin.ottens@....de>
Cc: Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang
 <xiyou.wangcong@...il.com>, Jiri Pirko <jiri@...nulli.us>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
 <horms@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH] net/sched: netem: account for backlog updates from
 child qdisc

On Tue, 26 Nov 2024 00:18:25 +0100
Martin Ottens <martin.ottens@....de> 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 <martin.ottens@....de>

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

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