[Cake] Re: [PATCH net-next] net/sched: fq_codel, cake: widen backlogs to u64

[Sebastian Moeller <moeller0@gmx.de>]

Hi Eric,


> On Sep 25, 2026, at 15:03, Eric Dumazet via Cake <cake@lists.bufferbloat.net> wrote:
> 
> On Fri, Sep 25, 2026 at 1:28 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>> 
>> On Fri, Sep 25, 2026 at 5:13 AM Eric Dumazet <edumazet@google.com> wrote:
>>> 
>>> On Fri, Sep 25, 2026 at 10:54 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>>> 
>>>> This is a follow-up to commit 8f735d64382d ("net/sched: bound
>>>> qdisc_pkt_len to prevent qdisc soft lockup"), which capped
>>>> qdisc_pkt_len() at QDISC_PKT_LEN_MAX (1 MiB). That cap bounds the stab
>>>> amplifier but leaves the per-flow backlog counter u32:
>>>> fq_codel_enqueue() accumulates qdisc_pkt_len(skb) into q->backlogs[idx],
>>>> so a flow can still accumulate 4096 packets of 1 MiB each and wrap the
>>>> counter mod 2^32. After a wrap, fq_codel_drop() sees a tiny maxbacklog
>>>> and drops from an almost-empty flow, and the dequeue-side subtractions
>>>> corrupt the counter further.
>>>> 
>>>> Widen the fq_codel backlogs table, the fat-flow scan (maxbacklog/len) and
>>>> the drop threshold to u64. fq_codel is not lockless: every writer runs
>>>> under the root qdisc lock, so plain u64 arithmetic keeps the WRITE_ONCE
>>>> publish / READ_ONCE-consume pattern. The dump path
>>>> (fq_codel_dump_class_stats) stays a lockless stat-only read.
>>>> 
>>>> CAKE accumulates the same generic qdisc_pkt_len(skb) into its per-flow
>>>> b->backlogs[] and per-tin b->tin_backlog and consumes the values for
>>>> longest-flow pruning (cake_heapify/cake_heapify_up) and for the shaper
>>>> staleness check, so it shares the bug. Widen those counters and the heap
>>>> comparison locals to u64; the class/tin stats keep exporting the low 32
>>>> bits through the unchanged uAPI fields.
>>>> 
>>>> Conditions to recreate the bug: CAP_NET_ADMIN in a user namespace;
>>>> CONFIG_NET_SCH_FQ_CODEL=y.
>>>> 
>>>>  ip tuntap add tun0 mode tun
>>>>  ip link set tun0 txqueuelen 32 up
>>>>  ip addr add 10.99.0.1/24 dev tun0
>>>>  tc qdisc add dev tun0 root handle 1: stab overhead 2000000000 \
>>>>      fq_codel flows 1 limit 4200 ecn drop_batch 4096
>>>>  # hold the tun fd open without reading (IFF_BACKPRESSURE) so the qdisc
>>>>  # backlog persists, then send at least 4300 packets (the wrap starts
>>>>  # at 4096 resident; the over-limit drop that reads the wrapped
>>>>  # threshold fires past the 4200 limit): backlogs[0] wraps at 4096 x
>>>>  # 1 MiB and the fat-flow threshold reads the wrapped value.
>>>> 
>>>> With a 1 MiB qdisc_pkt_len cap the counter wraps at 4096 resident
>>>> packets. At limit 4200 the first over-limit enqueue (the 4201st) sees a
>>>> wrapped 105 MiB (half-backlog threshold 52 MiB, a ~52 packet drop burst),
>>>> where the u64 counter sees 4201 MiB (threshold 2100 MiB, a ~2100 packet
>>>> drop burst).
>>>> 
>>>> Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
>>>> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260818101130.16203-1-jhs@mojatatu.com
>>>> Link: https://lore.kernel.org/netdev/20260818101130.16203-1-jhs@mojatatu.com/
>>>> Tested-by: hybris <hybris@mojatatu.ai>
>>>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>>>> ---
>>> 
>>> This makes no sense.
>>> 
>> 
>> As absurd as it looks that code is reachable ;->
>> 
>>> These qdisc have been developped to address bufferbloat issues.
>>> 
>>> Storing 4GB in a qdisc is absolutely insane.
>>> 
>> 
>> The counter wrap is not because we stored 4GB, rather it is because
>> "tc .. stab overhead ..." inflates qdisc_pkt_len() for a 64B pkt to
>> 1MB. So ~4K packets (put in other words a few "real" KB) makes that
>> backlog[0] cross 2^32.
> 
> Kill this stuff ? Who is still using this, for what reason ?

tc-stab? Same as before, if you need want to model a remote bottleneck with a local traffic shaper tc-stab is the generic solution (off the top of my head I only can enumerate cake as having its own traffic shaper that handles overheads).
One could argue that the "virtual" length should be accounted against cake's memlimit or fq-codel's memory_limit somehow...
In sane?/typical configurations overhead is expected to stay relatively small, so this would not limit the actual queue size too much, while potentially silencing this issue.

I might be off my rocker, in which has ignore (or preferably enlighten me).

Regards
	Sebastian

> 
> Really, it is time we stop adding code only for fuzzers.
> 
>> Result is pruning the wrong flow..
>> 
>>> Let's drop at enqueue if the current backlog is approaching 4GB (this
>>> can later be a new config/attribute in net-next)
>> 
>> Note, this is net-next already. Enqueue is fast path - are you ok with that?
>> 
>> cheers,
>> jamal
> _______________________________________________
> Cake mailing list -- cake@lists.bufferbloat.net
> To unsubscribe send an email to cake-leave@lists.bufferbloat.net