netdev - Re: [PATCH v2] net/sched: tbf: correct backlog statistic for GSO packets

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] [day] [month] [year] [list]

Message-ID: <CANn89iLeSEVDZv-Nx6RGghSJdpozBAdoU==VQLgH5v+Puc=i0w@mail.gmail.com>
Date: Mon, 25 Nov 2024 19:08:21 +0100
From: Eric Dumazet <edumazet@...gle.com>
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>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	"open list:TC subsystem" <netdev@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] net/sched: tbf: correct backlog statistic for GSO packets

On Mon, Nov 25, 2024 at 6:46 PM Martin Ottens <martin.ottens@....de> wrote:
>
> When the length of a GSO packet in the tbf qdisc is larger than the burst
> size configured the packet will be segmented by the tbf_segment function.
> Whenever this function is used to enqueue SKBs, the backlog statistic of
> the tbf is not increased correctly. This can lead to underflows of the
> 'backlog' byte-statistic value when these packets are dequeued from tbf.
>
> Reproduce the bug:
> Ensure that the sender machine has GSO enabled. Configured the tbf on
> the outgoing interface of the machine as follows (burstsize = 1 MTU):
> $ tc qdisc add dev <oif> root handle 1: tbf rate 50Mbit burst 1514 latency 50ms
>
> Send bulk TCP traffic out via this interface, e.g., by running an iPerf3
> client on this machine. Check the qdisc statistics:
> $ tc -s qdisc show dev <oif>
>
> The 'backlog' byte-statistic has incorrect values while traffic is
> transferred, e.g., high values due to u32 underflows. When the transfer
> is stopped, the value is != 0, which should never happen.
>
> This patch fixes this bug by updating the statistics correctly, even if
> single SKBs of a GSO SKB cannot be enqueued.
>
> Fixes: e43ac79a4bc6 ("sch_tbf: segment too big GSO packets")
> Signed-off-by: Martin Ottens <martin.ottens@....de>

This seems fine, please note we ask for a 24 hours delay between each
version, to let other reviewers chime in.

Reviewed-by: Eric Dumazet <edumazet@...gle.com>

Can you also take a look at net/sched/sch_taprio.c, it seems the bug
has been copy/pasted there as well.