[PATCH net-next v6 0/4] net/sched: cap the accounted backlog before it can wrap
flat view
HOTtoday
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-10-14 09:55:37
This series bounds the accounted backlog of the AQM qdiscs so that it
cannot wrap mod 2^32.
v1 widened the counters to u64 and was rejected; from v2 on the fix drops
at enqueue instead. The v5 posting was a single patch; at the reviewer's
request it is split here into four:
1/4 classifier refactor: cake_tcf_classify() and fq_codel_classify()
return their result to the caller.
2/4 cake: use a dedicated split-GSO list iterator.
3/4 the fix: drop at enqueue once the stored backlog is within one
maximum-size packet of the wrap point, and clamp the stabless GSO
producer in qdisc_pkt_len_segs_init(). Covers fq_codel, cake,
codel, pie, fq_pie, dualpi2, RED, pfifo, plug and gred.
4/4 cake: split cake_hash() into side-effect-free key extraction (on
the unsegmented skb) and set-associative resolution (after
admission), and move the split-GSO path ahead of the resolution.
The commit logs and the in-code comments are rewritten for readability.
The guard half is a net fix, but the CAKE hash split is new-code net-next
material, so the series targets net-next with no Fixes: (the aggregate
wrap predates every touched qdisc).
Changes in v6:
- Rewrite as a four-patch series per Toke Høiland-Jørgensen's review of
v5: split the classifier refactor, the split-GSO iterator, the overflow
guard and the cake hash split into separate patches, and drop the
verbose commit prose and the in-code comments that read as
machine-written.
- Considered the Sashiko gemini v5 TC_ACT_CONSUMED report; not folded.
No in-tree producer can deliver TC_ACT_CONSUMED to a qdisc classify
switch, so the fall-through it describes is unreachable: act_ct returns
it only when bound to a clsact or ingress qdisc (11cb63b0d1a0), and
act_mirred returns it only for a redirect that does not clone, which
requires an ingress skb. cls_bpf and act_bpf map an unrecognized opcode
such as CONSUMED to TC_ACT_UNSPEC.
v5 Considered Comments from Sashiko nipa and Simon:
- cake: the cake_tcf_classify() block comment had an embedded "/*"
("else *flow/*host"), which trips a -Wcomment / W=1 build warning.
Simon Horman reported it on v4 and noted it likely suppressed the
Sashiko nipa review; reworded to drop the nested comment.
- cake: reword the same comment so it no longer says the function
returns true for "a matched no usable flow" result (Sashiko nipa v3
Low): cake_tcf_classify() returns true only for a terminal TC action;
a matched-but-unusable classid leaves the overrides at 0 and the
packet is hashed as before (fq_codel_classify() differs).
- commit log: soften the claim that the 1 MiB headroom always bounds a
single packet (Sashiko nipa v3 High).
v4 fold the Sashiko nipa v3 review:
- cake: compute the flow/host hash keys from the packet as received
(before skb_gso_segment(), which can rewrite an encapsulated GSO
packet's headers to the inner ones), but defer the set-associative
resolution that writes q->tags[], the flow host indices, the way
counters and the host bulk-flow counts until after the split-GSO
segment-sum admission check, so a packet rejected by that check
commits no flow state.
- cake: in the split-GSO rejection, handle a fraglist GSO (whose
skb_segment_list() returns the original skb as the list head with an
extra reference, segs == skb) by dropping the caller's reference and
queueing the list once, instead of calling __qdisc_drop_all() and
qdisc_drop_reason() on the same skb, which self-linked *to_free.
v3 fold the v2 review (Eric Dumazet) and Sashiko gemini v2:
- Test the stored backlog via qdisc_backlog_at_max() instead of
(u64)backlog + qdisc_pkt_len() > QDISC_MAX_BACKLOG: no double-count
of the reserved 1 MiB headroom, no per-enqueue 64-bit add.
- cake: run the external classifier first, honor its terminal TC
actions, then the ceiling check, then the hash and the shaper state;
the SPLIT_GSO segment sum is checked before any flow/host state is
committed and the list is deferred to to_free rather than freed
under the qdisc lock.
- fq_codel: run the external classifier (terminal actions honored)
first, then the ceiling check before fq_codel_hash(); document the
fq_codel_drop() NULL flow->head consequence.
- pfifo: same ceiling guard, since RED reads a (graftable) child's
backlog and SFB's default child is pfifo.
v2 approach rewrite per list discussion of v1:
v1 approached the wrap by widening the per-flow backlog counters to
u64. Eric Dumazet rejected the framing ("Storing 4GB in a qdisc is
absolutely insane") and asked for a drop at enqueue once the backlog
approaches the wrap point. Sebastian Moeller noted tc-stab is the
generic overhead-accounting mechanism, so the fix must not penalize
normal stab use.
v2 replaces the u64 widening with an enqueue-side pre-check against
QDISC_MAX_BACKLOG and drops with QDISC_DROP_OVERLIMIT. No counter is
widened, so the 32-bit sch->qstats.backlog the AQM reads can no longer
wrap. Same-pattern siblings fixed in one patch: fq_codel, cake, codel,
pie, fq_pie, dualpi2, RED.
Jamal Hadi Salim (4):
net/sched: cake, fq_codel: return the classifier result to the caller
net/sched: cake: use a dedicated split-GSO list iterator
net/sched: cap the accounted backlog before it can wrap
net/sched: cake: hash before segmenting and reject an overlong split
include/net/pkt_sched.h | 1 -
include/net/sch_generic.h | 15 ++++
net/core/dev.c | 15 +++-
net/sched/sch_cake.c | 183 +++++++++++++++++++++++++++-----------
net/sched/sch_codel.c | 16 ++--
net/sched/sch_dualpi2.c | 1 +
net/sched/sch_fifo.c | 6 ++
net/sched/sch_fq_codel.c | 55 +++++++++---
net/sched/sch_fq_pie.c | 3 +-
net/sched/sch_gred.c | 10 +++
net/sched/sch_pie.c | 3 +-
net/sched/sch_plug.c | 9 +-
net/sched/sch_red.c | 25 ++++--
13 files changed, 262 insertions(+), 80 deletions(-)
--
2.43.0