Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v10 0/4] net/sched: cap the accounted backlog before it can wrap
@ 2026-11-14 12:14 Jamal Hadi Salim
  2026-11-14 12:14 ` [PATCH net-next v10 1/4] net/sched: cake: return the classifier result to the caller Jamal Hadi Salim
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-14 12:14 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Stephen Hemminger,
	Vinicius Costa Gomes, Victor Nogueira,
	Toke Høiland-Jørgensen, Björn Töpel, cake,
	Sashiko

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 into four:

  1/4  cake: classifier refactor: cake_tcf_classify() returns its 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 cannot
       represent the packet's accounted length -- the standing
       QDISC_MAX_BACKLOG ceiling for ordinary packets, a per-packet
       representability check for the larger wire estimates. Covers
       every qdisc in net/sched that adds the incoming packet's
       qdisc_pkt_len() to its own 32-bit backlog: fq_codel, cake,
       codel, pie, fq_pie, dualpi2, RED, pfifo, gred, the leaves fq,
       sfq, hhf, skbprio, netem, choke, etf and taprio, the classful
       parents prio, htb, drr, ets, qfq, hfsc, cbs and sfb, and tbf's
       direct enqueue path.
  4/4  cake: split cake_hash() into side-effect-free key extraction (on
       the unsegmented skb) and set-associative resolution (after
       admission), move the split-GSO path ahead of the resolution, and
       tag every deferred split segment.

For the sashikos: This series targets net-next with no Fixes: (the
aggregate wrap predates every touched qdisc).
Unfortunately something broke in the sashiko reviews. Review for v7
showed up a while later after v8.  So you will see that this series
has fixes (majority in comments and commit logs) from both.

Changes in v10 (late Sashiko nipa review of v9, delivered on lore):
  - Fixed all the commit log complaints
  - 3/4: cover the rest of net/sched. The leaves fq, sfq, hhf, skbprio,
    netem, choke, etf and taprio, the classful parents prio, htb, drr,
    ets, qfq, hfsc, cbs and sfb, and tbf's direct enqueue path all add
    the incoming packet's qdisc_pkt_len() to their own 32-bit backlog,
    so an unguarded leaf can wrap its own counter and an unguarded
    parent can wrap the aggregate it keeps over its children.
  - 3/4: drop plug's redundant qdisc_backlog_overflow() test. plug is
    classless and q->limit is a u32, so its byte test alone already
    bounds the stored backlog below U32_MAX.
  - 3/4: refresh the RED comment that still described the pre-rework
    behaviour.
  - 3/4: drop the Acked-by from the v9 posting, since the patch now
    covers 29 files and the earlier ack no longer applies to it.
  - 2/4: carry the Acked-by from the v9 posting; the patch is unchanged.

Changes in v9 (late Sashiko nipa reviews of v7 and v8, delivered on lore):
  - 1/4: reword the "The Fix:" heading; this patch is preparation, not a
    fix.
  - 1/4: the classifier helper's comments now say the caller must drop
    the skb, since a terminal action does not take ownership of it.
  - 3/4: bound the child backlog as well as RED's aggregate, and re-derive
    the aggregate from snapshots taken before the reservation when the
    enqueue does not succeed. A child that returns NET_XMIT_CN may or may
    not have reduced its ancestors already: pfifo_head_drop, sfq and
    fq_codel do, while hhf and skbprio evict a packet and return without
    qdisc_tree_reduce_backlog(). Keep what the child actually accounted
    for and pass any difference on to the ancestors above RED, so neither
    counter can diverge. Verified with two A/B witnesses: RED over a
    grafted hhf child (control ends with red_backlog 2042016 !=
    child_backlog 6177600) and RED under an HTB class over a grafted
    pfifo_head_drop limit 1, where an unequal shrinking replacement
    otherwise wraps the aggregate and wedges the new admission guard --
    control 4294966052, fixed 92 with RED == child == HTB.
  - 3/4: note that CAKE's own split-GSO branch is the second exception;
    the segments it accounts are summed and checked against
    QDISC_MAX_BACKLOG by 4/4 before any of them is linked.
  - 4/4: add the split-GSO "Conditions to recreate the bug" recipe for the
    segment-sum rejection at the split ceiling.
  - 4/4: the IS_ERR_OR_NULL(segs) segmentation-failure return uses
    qdisc_drop_reason(..., QDISC_DROP_GENERIC); __skb_gso_segment() has
    already clobbered tc_skb_cb.drop_reason through SKB_GSO_CB, so the old
    qdisc_drop() reported an unrelated core reason and skipped
    trace_qdisc_drop.
  - 4/4: pass the flow_mode snapshot to the later flowchain calls
    (cake_get_flow_quantum(), cake_inc_srchost_bulk_flow_count(),
    cake_inc_dsthost_bulk_flow_count()) as well, so one enqueue sees one
    mode (KCSAN-visible race and stale host index).

Changes in v8:
  - 1/4 is cake-only. fq_codel_classify() is line-for-line the same as
    fq_pie_classify(), and fq_pie is not changed by this series, so the
    two stay in lockstep: fq_codel keeps its original classify shape and
    the fq_codel backlog guard is placed inline in its enqueue path
    (3/4) instead. (Toke Høiland-Jørgensen)
  - 3/4: the fq_codel guard sits between idx-- and
    codel_set_enqueue_time(), so a dropped packet cannot touch flow
    state. The commit message is reworded as suggested. (Toke
    Høiland-Jørgensen)
  - 4/4: the original skb is consumed unconditionally right after a
    successful split -- that is the caller's reference, since a fraglist
    GSO takes the list's own reference on the same skb in
    skb_segment_list() -- so the reject branch tags every deferred
    segment and drops the whole list with qdisc_drop_all(), and the
    admitted path no longer consumes the skb after the walk. (Toke
    Høiland-Jørgensen)

Changes in v7:
  - 1/4: complete the fq_codel_classify() comment (Sashiko nipa v6
    main-2-1, Low). The comment listed only the terminal-action and
    no-class-ID cases; the function also returns true when no filter
    matched or when the selected class minor is above q->flows_cnt,
    and those packets are BYPASS drops, not a fallback to hashing as
    in cake. Comment-only, no code change; folded into this resend
    since the series is revised anyway.
  - 3/4: do not clamp the shared qdisc_pkt_len() (Sashiko nipa v6
    main-2-6, Medium). The clamp silently undercharged BPF
    __sk_buff.wire_len, act_police and the HTB/TBF shapers for a valid
    large GSO estimate. The initializer now stays truthful for every
    input and the overflow protection is enqueue admission: each
    affected qdisc drops a packet once the stored backlog cannot
    represent its accounted length -- the standing QDISC_MAX_BACKLOG
    ceiling covers ordinary packets, and a per-packet representability
    check covers the larger wire estimates, which a supported
    small-MSS BIG TCP aggregate can legitimately carry above 1 MiB
    while its raw length stays within GSO_MAX_SIZE. The consumers read
    the real length, and sch_handle_ingress() keeps complete pkt_segs
    metadata.
  - 4/4: tag every deferred split-GSO segment with QDISC_DROP_OVERLIMIT
    (Sashiko nipa v6 gpt-6-astra-2-5, Medium). __skb_gso_segment() stores
    SKB_GSO_CB at cb offset 32, aliasing tc_skb_cb.drop_reason, and the cb
    is copied into every segment, so tagging only the head left the rest
    with GSO scratch as their drop reason.
  - 4/4: snapshot q->config->flow_mode once with READ_ONCE() and pass it
    to both hash helpers (Sashiko nipa v6 gpt-6-astra-5-9, Low). The split
    introduced two loads with the segmentation work between them.
  - Not folded: the 3/4 intermediate-commit split-GSO limitation (4/4
    closes it; the pre-series path was already unbounded) and the
    Fixes:/stable suggestions (intentional net-next target). The gemini
    v6 TC_ACT_CONSUMED High is the v5 finding already rebutted as
    unreachable from a qdisc classify switch.

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; drop the machine-written
    prose). Considered the Sashiko gemini v5 TC_ACT_CONSUMED report and
    did not fold it (unreachable; see below). Reworded the cake block
    comment that had an embedded "/*" (Simon Horman's W=1 warning) and
    reworded a comment that overstated the classifier's return cases. The
    TC_ACT_CONSUMED switch case was dropped from the final 0001 because it
    still reached __qdisc_drop(); no in-tree producer can deliver
    TC_ACT_CONSUMED to a qdisc classify switch (act_ct returns it only on
    a clsact/ingress qdisc, act_mirred only for a non-cloning ingress
    redirect, and cls_bpf/act_bpf remap an unknown verdict to UNSPEC).

v5/v4/v3/v2 history: see the v6 posting. In short, v3 adopted the stored
    backlog compare qdisc_backlog_at_max() (no double-count of the 1 MiB
    headroom, no per-enqueue 64-bit add), moved the classifier and the
    ceiling check ahead of the hash/shaper state, and added pfifo; v4
    split the hash keys from the set-associative resolution and handled
    the fraglist-GSO self-link; v2 replaced the rejected u64 widening with
    the enqueue drop; v1 widened the counters.

Jamal Hadi Salim (4):
  net/sched: cake: 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 |  20 +++++
 net/sched/sch_cake.c      | 185 ++++++++++++++++++++++++++------------
 net/sched/sch_cbs.c       |   6 ++
 net/sched/sch_choke.c     |   6 ++
 net/sched/sch_codel.c     |  16 ++--
 net/sched/sch_drr.c       |   6 ++
 net/sched/sch_dualpi2.c   |   1 +
 net/sched/sch_etf.c       |   6 ++
 net/sched/sch_ets.c       |   6 ++
 net/sched/sch_fifo.c      |   6 ++
 net/sched/sch_fq.c        |   6 ++
 net/sched/sch_fq_codel.c  |   9 ++
 net/sched/sch_fq_pie.c    |   3 +-
 net/sched/sch_gred.c      |   9 ++
 net/sched/sch_hfsc.c      |   6 ++
 net/sched/sch_hhf.c       |   6 ++
 net/sched/sch_htb.c       |   6 ++
 net/sched/sch_netem.c     |   6 ++
 net/sched/sch_pie.c       |   3 +-
 net/sched/sch_plug.c      |   3 +-
 net/sched/sch_prio.c      |   6 ++
 net/sched/sch_qfq.c       |   6 ++
 net/sched/sch_red.c       |  46 ++++++++--
 net/sched/sch_sfb.c       |   6 ++
 net/sched/sch_sfq.c       |   6 ++
 net/sched/sch_skbprio.c   |   6 ++
 net/sched/sch_taprio.c    |   6 ++
 net/sched/sch_tbf.c       |   6 ++
 29 files changed, 333 insertions(+), 71 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next v10 1/4] net/sched: cake: return the classifier result to the caller
  2026-11-14 12:14 [PATCH net-next v10 0/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
@ 2026-11-14 12:14 ` Jamal Hadi Salim
  2026-11-14 12:14 ` [PATCH net-next v10 2/4] net/sched: cake: use a dedicated split-GSO list iterator Jamal Hadi Salim
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-14 12:14 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Stephen Hemminger,
	Vinicius Costa Gomes, Victor Nogueira,
	Toke Høiland-Jørgensen, Björn Töpel, cake,
	Sashiko

Preparation:
cake runs an optional external classifier from its enqueue path and then
computes the flow hash inside the helper. Have the helper return the
filter's overrides instead, so the caller owns the flow selection.

This is preparation for the accounting changes that follow. Packet
classification and flow selection are unchanged.

Link: https://lore.kernel.org/netdev/179269577009.3135.5221412623031392098@kernel.org/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_cake.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 3faccdbc03f7..24853d90c17e 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1715,18 +1715,25 @@ static struct cake_tin_data *cake_select_tin(struct Qdisc *sch,
 	return &qd->tins[tin];
 }
 
-static u32 cake_classify(struct Qdisc *sch, struct cake_tin_data **t,
-			 struct sk_buff *skb, int flow_mode, int *qerr)
+/* Run the qdisc's optional external classifier. Returns true when a
+ * terminal TC action wants the packet dropped, with the reason in *qerr;
+ * the caller must drop the skb. Otherwise *flow and *host hold the
+ * filter's overrides (0 if none).
+ */
+static bool cake_tcf_classify(struct Qdisc *sch, struct sk_buff *skb,
+			      u16 *flow, u16 *host, int *qerr)
 {
 	struct cake_sched_data *q = qdisc_priv(sch);
 	struct tcf_proto *filter;
 	struct tcf_result res;
-	u16 flow = 0, host = 0;
 	int result;
 
+	*flow = 0;
+	*host = 0;
+
 	filter = rcu_dereference_bh(q->filter_list);
 	if (!filter)
-		goto hash;
+		return false;
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 	result = tcf_classify_qdisc(skb, filter, &res, false);
@@ -1740,17 +1747,15 @@ static u32 cake_classify(struct Qdisc *sch, struct cake_tin_data **t,
 			*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
 			fallthrough;
 		case TC_ACT_SHOT:
-			return 0;
+			return true;
 		}
 #endif
 		if (TC_H_MIN(res.classid) <= CAKE_QUEUES)
-			flow = TC_H_MIN(res.classid);
+			*flow = TC_H_MIN(res.classid);
 		if (TC_H_MAJ(res.classid) <= (CAKE_QUEUES << 16))
-			host = TC_H_MAJ(res.classid) >> 16;
+			*host = TC_H_MAJ(res.classid) >> 16;
 	}
-hash:
-	*t = cake_select_tin(sch, skb);
-	return cake_hash(*t, skb, flow_mode, flow, host) + 1;
+	return false;
 }
 
 static void cake_reconfigure(struct Qdisc *sch);
@@ -1761,20 +1766,25 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	u32 idx, tin, prev_qlen, prev_backlog, drop_id;
 	struct cake_sched_data *q = qdisc_priv(sch);
 	int len = qdisc_pkt_len(skb), ret;
+	u16 flow_override, host_override;
 	struct sk_buff *ack = NULL;
 	ktime_t now = ktime_get();
 	struct cake_tin_data *b;
 	struct cake_flow *flow;
 	bool same_flow = false;
 
-	/* choose flow to insert into */
-	idx = cake_classify(sch, &b, skb, q->config->flow_mode, &ret);
-	if (idx == 0) {
+	/* Classify first: a terminal verdict means do not enqueue. */
+	if (cake_tcf_classify(sch, skb, &flow_override, &host_override, &ret)) {
 		if (ret & __NET_XMIT_BYPASS)
 			qdisc_qstats_drop(sch);
 		__qdisc_drop(skb, to_free);
 		return ret;
 	}
+
+	/* choose flow to insert into */
+	b = cake_select_tin(sch, skb);
+	idx = cake_hash(b, skb, q->config->flow_mode, flow_override,
+			host_override) + 1;
 	tin = (u32)(b - q->tins);
 	idx--;
 	flow = &b->flows[idx];
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v10 2/4] net/sched: cake: use a dedicated split-GSO list iterator
  2026-11-14 12:14 [PATCH net-next v10 0/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
  2026-11-14 12:14 ` [PATCH net-next v10 1/4] net/sched: cake: return the classifier result to the caller Jamal Hadi Salim
@ 2026-11-14 12:14 ` Jamal Hadi Salim
  2026-11-14 12:14 ` [PATCH net-next v10 3/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
  2026-11-14 12:14 ` [PATCH net-next v10 4/4] net/sched: cake: hash before segmenting and reject an overlong split Jamal Hadi Salim
  3 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-14 12:14 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Stephen Hemminger,
	Vinicius Costa Gomes, Victor Nogueira,
	Toke Høiland-Jørgensen, Björn Töpel, cake,
	Sashiko

skb_list_walk_safe(segs, segs, nskb) reuses the list head as the walk
cursor, so segs no longer names the segment list once the walk finishes.
Use a separate seg cursor to keep the head and the walker distinct. No
behaviour change.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---

 net/sched/sch_cake.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 24853d90c17e..88ff3e4f6754 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1814,7 +1814,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		WRITE_ONCE(b->max_skblen, len);
 
 	if (qdisc_pkt_segs(skb) > 1 && q->config->rate_flags & CAKE_FLAG_SPLIT_GSO) {
-		struct sk_buff *segs, *nskb;
+		struct sk_buff *segs, *nskb, *seg;
 		netdev_features_t features = netif_skb_features(skb);
 		unsigned int slen = 0, numsegs = 0;
 
@@ -1822,19 +1822,19 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		if (IS_ERR_OR_NULL(segs))
 			return qdisc_drop(skb, sch, to_free);
 
-		skb_list_walk_safe(segs, segs, nskb) {
-			skb_mark_not_on_list(segs);
-			qdisc_skb_cb(segs)->pkt_len = segs->len;
-			qdisc_skb_cb(segs)->pkt_segs = 1;
-			cobalt_set_enqueue_time(segs, now);
-			get_cobalt_cb(segs)->adjusted_len = cake_overhead(q,
-									  segs);
-			flow_queue_add(flow, segs);
+		skb_list_walk_safe(segs, seg, nskb) {
+			skb_mark_not_on_list(seg);
+			qdisc_skb_cb(seg)->pkt_len = seg->len;
+			qdisc_skb_cb(seg)->pkt_segs = 1;
+			cobalt_set_enqueue_time(seg, now);
+			get_cobalt_cb(seg)->adjusted_len = cake_overhead(q,
+									 seg);
+			flow_queue_add(flow, seg);
 
 			qdisc_qlen_inc(sch);
 			numsegs++;
-			slen += segs->len;
-			q->buffer_used += segs->truesize;
+			q->buffer_used += seg->truesize;
+			slen += seg->len;
 			WRITE_ONCE(b->packets, b->packets + 1);
 		}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v10 3/4] net/sched: cap the accounted backlog before it can wrap
  2026-11-14 12:14 [PATCH net-next v10 0/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
  2026-11-14 12:14 ` [PATCH net-next v10 1/4] net/sched: cake: return the classifier result to the caller Jamal Hadi Salim
  2026-11-14 12:14 ` [PATCH net-next v10 2/4] net/sched: cake: use a dedicated split-GSO list iterator Jamal Hadi Salim
@ 2026-11-14 12:14 ` Jamal Hadi Salim
  2026-11-15 23:17   ` netdev-bot+sashiko
  2026-11-14 12:14 ` [PATCH net-next v10 4/4] net/sched: cake: hash before segmenting and reject an overlong split Jamal Hadi Salim
  3 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-14 12:14 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Stephen Hemminger,
	Vinicius Costa Gomes, Victor Nogueira,
	Toke Høiland-Jørgensen, Björn Töpel, cake,
	Sashiko

This is a followup of commit 709f34f7c28d ("net/sched: fq: add overflow
bounds to quantum and initial quantum") as reported by Sashiko.

The issue: fq_codel, cake, codel, pie, fq_pie, dualpi2, RED, pfifo,
plug and (non-WRED) gred account every enqueued packet's stab-adjusted
length into the 32-bit sch->qstats.backlog. A crafted tc size table
inflates qdisc_pkt_len() up to QDISC_PKT_LEN_MAX (1 MiB), and a
supported BIG TCP aggregate can account more than that through the
headers qdisc_pkt_len_segs_init() replicates per segment, so a few
thousand packets wrap the counter mod 2^32.

The AQM then reads a small backlog and makes the wrong drop decision,
and the dequeue-side subtractions leave it corrupt (for non-WRED gred
it is the reported stats and the dequeue-side subtractions that go
wrong instead). In fq_codel a wrapped per-flow backlog can also make
fq_codel_drop()'s scan pick an empty flow, dereferencing a NULL
flow->head.

The Fix: drop at enqueue once the stored backlog cannot represent the
incoming packet's accounted length.

Ordinary packets (qdisc_pkt_len() at most QDISC_PKT_LEN_MAX) are
covered by a standing ceiling: drop once the backlog is within
QDISC_MAX_BACKLOG (U32_MAX - QDISC_PKT_LEN_MAX) of the wrap point, the
largest backlog one more 1 MiB packet cannot wrap, so there is no
per-enqueue 64-bit add.

A larger wire estimate is checked against the space that actually
remains (len > U32_MAX - backlog): qdisc_pkt_len_segs_init() adds the
headers replicated on every output segment, so a supported small-MSS
BIG TCP aggregate can carry a wire estimate above 1 MiB while its raw
length stays within GSO_MAX_SIZE, and such traffic must pass on an
empty queue.

qdisc_pkt_len() itself stays truthful for every input: it is not
clamped and the initializer keeps succeeding, so BPF
__sk_buff.wire_len, act_police and the HTB/TBF shapers read the real
length, and sch_handle_ingress() -- which ignores the initializer's
return value and reads pkt_segs for packet statistics -- keeps
complete metadata.

Cover sch_plug, pfifo, non-WRED sch_gred, the leaves fq, sfq, hhf,
skbprio, netem, choke, etf and taprio, the classful parents prio, htb,
drr, ets, qfq, hfsc, cbs and sfb, and tbf's direct enqueue path as well,
since each adds the packet's own length to a 32-bit backlog. In RED,
account the packet before handing it to the child, bound the child
backlog too, and re-derive the aggregate from pre-reservation snapshots
on failure.

The guard bounds each qdisc's own enqueue path: a child that segments a
GSO skb (tbf, netem) can still grow a parent's aggregate through
qdisc_tree_reduce_backlog()'s unclamped byte delta, which is left to a
separate change.

CAKE's split-GSO branch is the other exception; 4/4 sums and checks those
segments before linking any of them.

Conditions to recreate the bug:
  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 20000 ecn
  # hold the tun fd open without reading so the backlog persists, then
  # send 6000 packets: the counter wraps and the unfixed kernel reports
  # a wrapped backlog, while the fixed one drops at the ceiling.

Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/20260818101130.16203-1-jhs@mojatatu.com/
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260818101130.16203-1-jhs@mojatatu.com
Link: https://lore.kernel.org/netdev/CANn89iLfMJV7ancKH1Gjzzm7ZUG-gKcrczkJjJEWN5sCuWd-ug@mail.gmail.com/
Link: https://lore.kernel.org/netdev/CANn89i+GOFH_8g+vSV0jUj0aBqLYVWuFOrHuDggZu9kPC_m6tQ@mail.gmail.com/
Link: https://lore.kernel.org/netdev/87a4oottcj.fsf@toke.dk/
Link: https://lore.kernel.org/netdev/87pkxks68f.fsf@toke.dk/
Link: https://lore.kernel.org/netdev/179231932946.3135.7497478990186385688@kernel.org/
Link: https://lore.kernel.org/netdev/179269577131.3135.3967915390403138015@kernel.org/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---

v9 -> v10:
- cover the rest of net/sched.  The leaves fq, sfq, hhf, skbprio,
netem, choke, etf and taprio, the classful parents prio, htb, drr,
ets, qfq, hfsc, cbs and sfb, and tbf's direct enqueue path all add
the incoming packet's qdisc_pkt_len() to their own 32-bit backlog, so a
leaf can wrap its own counter and a parent can wrap the aggregate it
keeps over its children.  (Sashiko nipa review of v9)
- drop plug's redundant qdisc_backlog_overflow() test. plug is classless
and q->limit is a u32, so its byte test alone already bounds the stored
backlog below U32_MAX.  (Sashiko nipa review of v9)
- refresh the RED comment, which still described the pre-rework behaviour.
- drop the Acked-by from the v9 posting, since the patch now covers 29 files

---
 include/net/pkt_sched.h   |  1 -
 include/net/sch_generic.h | 20 +++++++++++++++++
 net/sched/sch_cake.c      |  7 ++++++
 net/sched/sch_cbs.c       |  6 +++++
 net/sched/sch_choke.c     |  6 +++++
 net/sched/sch_codel.c     | 16 ++++++++------
 net/sched/sch_drr.c       |  6 +++++
 net/sched/sch_dualpi2.c   |  1 +
 net/sched/sch_etf.c       |  6 +++++
 net/sched/sch_ets.c       |  6 +++++
 net/sched/sch_fifo.c      |  6 +++++
 net/sched/sch_fq.c        |  6 +++++
 net/sched/sch_fq_codel.c  |  9 ++++++++
 net/sched/sch_fq_pie.c    |  3 ++-
 net/sched/sch_gred.c      |  9 ++++++++
 net/sched/sch_hfsc.c      |  6 +++++
 net/sched/sch_hhf.c       |  6 +++++
 net/sched/sch_htb.c       |  6 +++++
 net/sched/sch_netem.c     |  6 +++++
 net/sched/sch_pie.c       |  3 ++-
 net/sched/sch_plug.c      |  3 ++-
 net/sched/sch_prio.c      |  6 +++++
 net/sched/sch_qfq.c       |  6 +++++
 net/sched/sch_red.c       | 46 ++++++++++++++++++++++++++++++++++-----
 net/sched/sch_sfb.c       |  6 +++++
 net/sched/sch_sfq.c       |  6 +++++
 net/sched/sch_skbprio.c   |  6 +++++
 net/sched/sch_taprio.c    |  6 +++++
 net/sched/sch_tbf.c       |  6 +++++
 29 files changed, 210 insertions(+), 16 deletions(-)

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 90d3e7943b19..18a419cd9d94 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -12,7 +12,6 @@
 
 #define DEFAULT_TX_QUEUE_LEN	1000
 #define STAB_SIZE_LOG_MAX	30
-#define QDISC_PKT_LEN_MAX	(1 << 20)	/* 1 MiB */
 
 struct qdisc_walker {
 	int	stop;
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 1acaadb3e2cf..a5a6bdf55e0a 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -915,6 +915,26 @@ static inline unsigned int qdisc_pkt_len(const struct sk_buff *skb)
 	return qdisc_skb_cb(skb)->pkt_len;
 }
 
+#define QDISC_PKT_LEN_MAX	(1 << 20)	/* 1 MiB */
+
+/* Largest backlog that still has room for one more 1 MiB packet. */
+#define QDISC_MAX_BACKLOG	(U32_MAX - QDISC_PKT_LEN_MAX)
+
+/* Would this packet's accounted length wrap sch->qstats.backlog?
+ * Packets up to QDISC_PKT_LEN_MAX fit below QDISC_MAX_BACKLOG; GSO
+ * can make the length larger, so compare against the real headroom.
+ */
+static inline bool qdisc_backlog_overflow(const struct Qdisc *sch,
+					  const struct sk_buff *skb)
+{
+	u32 backlog = READ_ONCE(sch->qstats.backlog);
+	u32 len = qdisc_pkt_len(skb);
+
+	if (likely(len <= QDISC_PKT_LEN_MAX))
+		return backlog > QDISC_MAX_BACKLOG;
+	return len > U32_MAX - backlog;
+}
+
 static inline unsigned int qdisc_pkt_segs(const struct sk_buff *skb)
 {
 	u32 pkt_segs = qdisc_skb_cb(skb)->pkt_segs;
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 88ff3e4f6754..803ead499ae9 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1781,6 +1781,13 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		return ret;
 	}
 
+	/* Drop before touching the hash or shaper state. */
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	/* choose flow to insert into */
 	b = cake_select_tin(sch, skb);
 	idx = cake_hash(b, skb, q->config->flow_mode, flow_override,
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index 1c93469c56e3..3c018ef70e23 100644
--- a/net/sched/sch_cbs.c
+++ b/net/sched/sch_cbs.c
@@ -92,6 +92,12 @@ static int cbs_child_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	unsigned int len = qdisc_pkt_len(skb);
 	int err;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	err = child->ops->enqueue(skb, child, to_free);
 	if (err != NET_XMIT_SUCCESS)
 		return err;
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index 73d3e673dc7b..9ce02273b624 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -215,6 +215,12 @@ static int choke_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct choke_sched_data *q = qdisc_priv(sch);
 	const struct red_parms *p = &q->parms;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	choke_skb_cb(skb)->keys_valid = 0;
 	/* Compute average queue usage (see RED) */
 	q->vars.qavg = red_calc_qavg(p, &q->vars, sch->q.qlen);
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index a1ba2e355f2d..78c5d72d6406 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c
@@ -116,15 +116,17 @@ static struct sk_buff *codel_peek(struct Qdisc *sch)
 static int codel_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			       struct sk_buff **to_free)
 {
-	struct codel_sched_data *q;
+	struct codel_sched_data *q = qdisc_priv(sch);
 
-	if (likely(qdisc_qlen(sch) < sch->limit)) {
-		codel_set_enqueue_time(skb);
-		return qdisc_enqueue_tail(skb, sch);
+	if (unlikely(qdisc_qlen(sch) >= sch->limit) ||
+	    unlikely(qdisc_backlog_overflow(sch, skb))) {
+		WRITE_ONCE(q->drop_overlimit, q->drop_overlimit + 1);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
 	}
-	q = qdisc_priv(sch);
-	WRITE_ONCE(q->drop_overlimit, q->drop_overlimit + 1);
-	return qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_OVERLIMIT);
+
+	codel_set_enqueue_time(skb);
+	return qdisc_enqueue_tail(skb, sch);
 }
 
 static const struct nla_policy codel_policy[TCA_CODEL_MAX + 1] = {
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 8621d057edd9..eeef304f76a7 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -342,6 +342,12 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct drr_class *cl;
 	int err = 0;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	cl = drr_classify(skb, sch, &err);
 	if (cl == NULL) {
 		if (err & __NET_XMIT_BYPASS)
diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index 4947def7c49e..7331f0ec1c8f 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -392,6 +392,7 @@ static int dualpi2_enqueue_skb(struct sk_buff *skb, struct Qdisc *sch,
 	struct dualpi2_skb_cb *cb;
 
 	if (unlikely(qdisc_qlen(sch) >= sch->limit) ||
+	    unlikely(qdisc_backlog_overflow(sch, skb)) ||
 	    unlikely((u64)q->memory_used + skb->truesize > q->memory_limit)) {
 		qdisc_qstats_overlimit(sch);
 		if (skb_in_l_queue(skb))
diff --git a/net/sched/sch_etf.c b/net/sched/sch_etf.c
index aaafcf640bc7..a534b11eadd0 100644
--- a/net/sched/sch_etf.c
+++ b/net/sched/sch_etf.c
@@ -168,6 +168,12 @@ static int etf_enqueue_timesortedlist(struct sk_buff *nskb, struct Qdisc *sch,
 	ktime_t txtime = nskb->tstamp;
 	bool leftmost = true;
 
+	if (unlikely(qdisc_backlog_overflow(sch, nskb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(nskb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	if (!is_packet_valid(sch, nskb)) {
 		report_sock_error(nskb, EINVAL,
 				  SO_EE_CODE_TXTIME_INVALID_PARAM);
diff --git a/net/sched/sch_ets.c b/net/sched/sch_ets.c
index 6cc902a03838..7f74f79ea6b6 100644
--- a/net/sched/sch_ets.c
+++ b/net/sched/sch_ets.c
@@ -420,6 +420,12 @@ static int ets_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct ets_class *cl;
 	int err = 0;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	cl = ets_classify(skb, sch, &err);
 	if (!cl) {
 		if (err & __NET_XMIT_BYPASS)
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index 1b6388d50967..16c8d4f74e7e 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -29,6 +29,9 @@ static int bfifo_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 static int pfifo_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			 struct sk_buff **to_free)
 {
+	if (unlikely(qdisc_backlog_overflow(sch, skb)))
+		return qdisc_drop(skb, sch, to_free);
+
 	if (likely(sch->q.qlen < READ_ONCE(sch->limit)))
 		return qdisc_enqueue_tail(skb, sch);
 
@@ -43,6 +46,9 @@ static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	if (unlikely(READ_ONCE(sch->limit) == 0))
 		return qdisc_drop(skb, sch, to_free);
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb)))
+		return qdisc_drop(skb, sch, to_free);
+
 	if (likely(sch->q.qlen < READ_ONCE(sch->limit)))
 		return qdisc_enqueue_tail(skb, sch);
 
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index a282812c192e..03d06b65ed03 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -628,6 +628,12 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	u64 now;
 	u8 band;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	band = fq_prio2band(q->prio2band, skb->priority & TC_PRIO_MAX);
 	if (unlikely(q->band_pkt_count[band] >= sch->limit)) {
 		q->stat_band_drops[band]++;
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index e0fcda26ff62..5feb50db41e8 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -201,6 +201,15 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	}
 	idx--;
 
+	/* Drop before the flow is touched, so a dropped packet cannot
+	 * create or update flow state.
+	 */
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		q->drop_overlimit++;
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	codel_set_enqueue_time(skb);
 	flow = &q->flows[idx];
 	flow_queue_add(flow, skb);
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 5982847df8f8..9f927a08b7bc 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -155,7 +155,8 @@ static int fq_pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	memory_limited = q->memory_usage > q->memory_limit + skb->truesize;
 
 	/* Checks if the qdisc is full */
-	if (unlikely(qdisc_qlen(sch) >= sch->limit)) {
+	if (unlikely(qdisc_qlen(sch) >= sch->limit) ||
+	    unlikely(qdisc_backlog_overflow(sch, skb))) {
 		q->stats.overlimit++;
 		goto out;
 	} else if (unlikely(memory_limited)) {
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index f3d9a7252949..de96badff654 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -170,6 +170,15 @@ static int gred_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	unsigned long qavg = 0;
 	u16 dp = tc_index_to_dp(skb);
 
+	/* Per-VQ limits don't bound the total: every VQ adds into the
+	 * same u32 backlog, so check it before picking a VQ.
+	 */
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	if (dp >= t->DPs || (q = t->tab[dp]) == NULL) {
 		dp = t->def;
 
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 284490fd6ca9..c11457e6cc24 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1564,6 +1564,12 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 	int err;
 	bool first;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	cl = hfsc_classify(skb, sch, &err);
 	if (cl == NULL) {
 		if (err & __NET_XMIT_BYPASS)
diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index fca20c50c97c..df63051f43d8 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -377,6 +377,12 @@ static int hhf_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct wdrr_bucket *bucket;
 	unsigned int prev_backlog;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	idx = hhf_classify(skb, sch);
 
 	bucket = &q->buckets[idx];
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 1ba67b121de4..4ee8d3fe9cae 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -630,6 +630,12 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct htb_sched *q = qdisc_priv(sch);
 	struct htb_class *cl = htb_classify(skb, sch, &ret);
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	if (cl == HTB_DIRECT) {
 		/* enqueue to helper queue */
 		if (q->direct_queue.qlen < q->direct_qlen) {
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 1dc2924890eb..9a7b11594fff 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -467,6 +467,12 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	unsigned int prev_len = qdisc_pkt_len(skb);
 	int count = 1;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	/* Do not fool qdisc_drop_all() */
 	skb->prev = NULL;
 
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 3b7863ffd284..2a5fc493a7e5 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -89,7 +89,8 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct pie_sched_data *q = qdisc_priv(sch);
 	bool enqueue = false;
 
-	if (unlikely(qdisc_qlen(sch) >= sch->limit)) {
+	if (unlikely(qdisc_qlen(sch) >= sch->limit) ||
+	    unlikely(qdisc_backlog_overflow(sch, skb))) {
 		WRITE_ONCE(q->stats.overlimit, q->stats.overlimit + 1);
 		goto out;
 	}
diff --git a/net/sched/sch_plug.c b/net/sched/sch_plug.c
index b60ddfee6a68..5654e5310fb5 100644
--- a/net/sched/sch_plug.c
+++ b/net/sched/sch_plug.c
@@ -89,7 +89,8 @@ static int plug_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 {
 	struct plug_sched_data *q = qdisc_priv(sch);
 
-	if (likely((u64)sch->qstats.backlog + skb->len <= q->limit)) {
+	if (likely((u64)sch->qstats.backlog + qdisc_pkt_len(skb) <=
+		   q->limit)) {
 		if (!q->unplug_indefinite)
 			q->pkts_current_epoch++;
 		return qdisc_enqueue_tail(skb, sch);
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 79437c587e7e..11f9354f57c2 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -72,6 +72,12 @@ prio_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 	struct Qdisc *qdisc;
 	int ret;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	qdisc = prio_classify(skb, sch, &ret);
 #ifdef CONFIG_NET_CLS_ACT
 	if (qdisc == NULL) {
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index e900890e9c54..8e0b2702669e 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -1236,6 +1236,12 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct qfq_aggregate *agg;
 	int err = 0;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	cl = qfq_classify(skb, sch, &err);
 	if (cl == NULL) {
 		if (err & __NET_XMIT_BYPASS)
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index d7598214270b..f3783e436da7 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -73,9 +73,22 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	enum qdisc_drop_reason reason = QDISC_DROP_CONGESTED;
 	struct red_sched_data *q = qdisc_priv(sch);
 	struct Qdisc *child = q->qdisc;
+	u32 child_backlog;
+	u32 red_backlog;
 	unsigned int len;
 	int ret;
 
+	/* RED reads the child backlog into red_calc_qavg() and mirrors the
+	 * child's growth into its own counter, so both must have room for
+	 * the packet.
+	 */
+	if (unlikely(qdisc_backlog_overflow(sch, skb) ||
+		     qdisc_backlog_overflow(child, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	q->vars.qavg = red_calc_qavg(&q->parms,
 				     &q->vars,
 				     child->qstats.backlog);
@@ -136,14 +149,37 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	}
 
 	len = qdisc_pkt_len(skb);
+	/* Reserve the length before the child sees the packet: netem
+	 * can re-enqueue a clone here before the backlog is updated.
+	 */
+	red_backlog = READ_ONCE(sch->qstats.backlog);
+	child_backlog = READ_ONCE(child->qstats.backlog);
+	qstats_backlog_add(sch, len);
 	ret = qdisc_enqueue(skb, child, to_free);
 	if (likely(ret == NET_XMIT_SUCCESS)) {
-		qstats_backlog_add(sch, len);
 		qdisc_qlen_inc(sch);
-	} else if (net_xmit_drop_count(ret)) {
-		WRITE_ONCE(q->stats.pdrop,
-			   q->stats.pdrop + 1);
-		qdisc_qstats_drop(sch);
+	} else {
+		u32 propagated, grew;
+
+		/* Re-derive the aggregate from the snapshots: the child may
+		 * have dropped the packet or evicted another one, and may or
+		 * may not have reduced the ancestors' backlog itself
+		 * (pfifo_head_drop, sfq and fq_codel do; hhf and skbprio
+		 * return NET_XMIT_CN without qdisc_tree_reduce_backlog()).
+		 * Keep what the child actually accounted for, and pass the
+		 * difference on to the ancestors above RED.
+		 */
+		propagated = READ_ONCE(sch->qstats.backlog) -
+			     red_backlog - len;
+		grew = READ_ONCE(child->qstats.backlog) - child_backlog;
+		WRITE_ONCE(sch->qstats.backlog, red_backlog + grew);
+		if (propagated != grew)
+			qdisc_tree_reduce_backlog(sch, 0, propagated - grew);
+		if (net_xmit_drop_count(ret)) {
+			WRITE_ONCE(q->stats.pdrop,
+				   q->stats.pdrop + 1);
+			qdisc_qstats_drop(sch);
+		}
 	}
 	return ret;
 
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index ed39869199c0..7e8edb481edc 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -296,6 +296,12 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	u32 slot = q->slot;
 	int ret = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	if (unlikely(sch->q.qlen >= q->limit)) {
 		qdisc_qstats_overlimit(sch);
 		WRITE_ONCE(q->stats.queuedrop,
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 8bbcfc9e85d9..67d797f50755 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -351,6 +351,12 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 	struct sk_buff *head;
 	int delta;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	hash = sfq_classify(skb, sch, &ret);
 	if (hash == 0) {
 		if (ret & __NET_XMIT_BYPASS)
diff --git a/net/sched/sch_skbprio.c b/net/sched/sch_skbprio.c
index 52abfb4015a3..44e9dd97a859 100644
--- a/net/sched/sch_skbprio.c
+++ b/net/sched/sch_skbprio.c
@@ -75,6 +75,12 @@ static int skbprio_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct sk_buff *to_drop;
 	u16 prio, lp;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	/* Obtain the priority of @skb. */
 	prio = min(skb->priority, max_priority);
 
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 5a1dca9d5e9d..bdb2596de0ef 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -590,6 +590,12 @@ static int taprio_enqueue_one(struct sk_buff *skb, struct Qdisc *sch,
 {
 	struct taprio_sched *q = qdisc_priv(sch);
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	/* sk_flags are only safe to use on full sockets. */
 	if (skb->sk && sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_TXTIME)) {
 		if (!is_valid_interval(skb, sch))
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 58c9687efb0f..5a23b6749758 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -255,6 +255,12 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	unsigned int len = qdisc_pkt_len(skb);
 	int ret;
 
+	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
 	if (qdisc_pkt_len(skb) > q->max_size) {
 		if (skb_is_gso(skb) &&
 		    skb_gso_validate_mac_len(skb, q->max_size))
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v10 4/4] net/sched: cake: hash before segmenting and reject an overlong split
  2026-11-14 12:14 [PATCH net-next v10 0/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
                   ` (2 preceding siblings ...)
  2026-11-14 12:14 ` [PATCH net-next v10 3/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
@ 2026-11-14 12:14 ` Jamal Hadi Salim
  2026-11-15 23:17   ` netdev-bot+sashiko
  3 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-14 12:14 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Stephen Hemminger,
	Vinicius Costa Gomes, Victor Nogueira,
	Toke Høiland-Jørgensen, Björn Töpel, cake,
	Sashiko

The Fix:
with CAKE_FLAG_SPLIT_GSO, cake computes the flow/host hash keys, then
splits a GSO skb into segments it accounts individually. Key extraction
must run on the unsegmented skb -- skb_gso_segment() can rewrite an
encapsulated packet's headers to the inner ones -- and the resolution
must wait until the packet is admitted, since it commits the
set-associative tags, host indices and way counters that a later
rejection would leave mutated (a GSO skb whose segmentation fails was
dropped with that state already committed).

Split cake_hash() into a side-effect-free cake_hash_keys(), run on the
unsegmented skb, and cake_hash_resolve(), run only once the packet is
admitted. Move the split-GSO path ahead of the resolution: sum the
segment list, test the total against the ceiling before linking any
segment, and drop the whole list on rejection so no persistent state is
committed. The list is deferred to to_free so the tail segment's
sock_wfree() never runs under the qdisc lock.

Tag every deferred segment with QDISC_DROP_OVERLIMIT: __skb_gso_segment()
keeps SKB_GSO_CB at cb offset 32, aliasing tc_skb_cb.drop_reason, and
__copy_skb_header() copies it into every segment, so tagging only the
head left the rest with GSO scratch (a headroom-sized value) as their
drop reason, which __tcf_kfree_skb_list() would report as an unrelated
core reason. The segmentation-failure return has the same aliasing and
uses qdisc_drop_reason(..., QDISC_DROP_GENERIC) so its drop is still
reported as a qdisc drop.

Read q->config->flow_mode once with READ_ONCE() in cake_enqueue() and
pass that value to both cake_hash_keys() and cake_hash_resolve(), and to
the later flowchain calls (cake_get_flow_quantum(),
cake_inc_srchost_bulk_flow_count(), cake_inc_dsthost_bulk_flow_count()).
cake_mq_change() updates the shared cake_config outside the qdisc lock, so
a reconfiguration could extract and resolve keys in different modes.

Conditions to recreate the bug:
  ip tuntap add tun0 mode tun
  ip link set tun0 txqueuelen 32 up
  ip addr add 10.99.0.1/24 dev tun0
  # hold the tun fd open without reading so the backlog persists; a stab
  # overhead of 1048468 accounts each 92-byte packet at 1048560 bytes, so
  # 4095 queued packets leave 65519 bytes below QDISC_MAX_BACKLOG.  A
  # 65000-byte UDP_SEGMENT payload with a 1400-byte segment size then has
  # a segment sum past the remaining headroom, and cake rejects the whole
  # split.  Setup needs CAP_NET_ADMIN; the payload itself does not.
  tc qdisc add dev tun0 root handle 1: stab overhead 1048468 \
      cake split-gso memlimit 512Mb

Link: https://lore.kernel.org/netdev/179231933068.3135.80340792633077986@kernel.org/
Link: https://lore.kernel.org/netdev/179269577270.3135.16072065138845192450@kernel.org/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_cake.c | 130 +++++++++++++++++++++++++++++++------------
 1 file changed, 94 insertions(+), 36 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 803ead499ae9..4851097f0ae5 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -709,19 +709,28 @@ static u16 cake_get_flow_quantum(struct cake_tin_data *q,
 		get_random_u16()) >> 16;
 }
 
-static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb,
-		     int flow_mode, u16 flow_override, u16 host_override)
+struct cake_hash_keys {
+	u32 srchost_hash;
+	u32 dsthost_hash;
+	u32 flow_hash;
+};
+
+static void cake_hash_keys(const struct sk_buff *skb, int flow_mode,
+			   u16 flow_override, u16 host_override,
+			   struct cake_hash_keys *out)
 {
 	bool hash_flows = (!flow_override && !!(flow_mode & CAKE_FLOW_FLOWS));
 	bool hash_hosts = (!host_override && !!(flow_mode & CAKE_FLOW_HOSTS));
 	bool nat_enabled = !!(flow_mode & CAKE_FLOW_NAT_FLAG);
-	u32 flow_hash = 0, srchost_hash = 0, dsthost_hash = 0;
-	u16 reduced_hash, srchost_idx, dsthost_idx;
 	struct flow_keys keys, host_keys;
 	bool use_skbhash = skb->l4_hash;
 
+	out->flow_hash = 0;
+	out->srchost_hash = 0;
+	out->dsthost_hash = 0;
+
 	if (unlikely(flow_mode == CAKE_FLOW_NONE))
-		return 0;
+		return;
 
 	/* If both overrides are set, or we can use the SKB hash and nat mode is
 	 * disabled, we can skip packet dissection entirely. If nat mode is
@@ -756,50 +765,64 @@ static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb,
 	switch (host_keys.control.addr_type) {
 	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
 		host_keys.addrs.v4addrs.src = 0;
-		dsthost_hash = flow_hash_from_keys(&host_keys);
+		out->dsthost_hash = flow_hash_from_keys(&host_keys);
 		host_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
 		host_keys.addrs.v4addrs.dst = 0;
-		srchost_hash = flow_hash_from_keys(&host_keys);
+		out->srchost_hash = flow_hash_from_keys(&host_keys);
 		break;
 
 	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
 		memset(&host_keys.addrs.v6addrs.src, 0,
 		       sizeof(host_keys.addrs.v6addrs.src));
-		dsthost_hash = flow_hash_from_keys(&host_keys);
+		out->dsthost_hash = flow_hash_from_keys(&host_keys);
 		host_keys.addrs.v6addrs.src = keys.addrs.v6addrs.src;
 		memset(&host_keys.addrs.v6addrs.dst, 0,
 		       sizeof(host_keys.addrs.v6addrs.dst));
-		srchost_hash = flow_hash_from_keys(&host_keys);
+		out->srchost_hash = flow_hash_from_keys(&host_keys);
 		break;
 
 	default:
-		dsthost_hash = 0;
-		srchost_hash = 0;
+		out->dsthost_hash = 0;
+		out->srchost_hash = 0;
 	}
 
 	/* This *must* be after the above switch, since as a
 	 * side-effect it sorts the src and dst addresses.
 	 */
 	if (hash_flows && !use_skbhash)
-		flow_hash = flow_hash_from_keys(&keys);
+		out->flow_hash = flow_hash_from_keys(&keys);
 
 skip_hash:
 	if (flow_override)
-		flow_hash = flow_override - 1;
+		out->flow_hash = flow_override - 1;
 	else if (use_skbhash && (flow_mode & CAKE_FLOW_FLOWS))
-		flow_hash = skb->hash;
+		out->flow_hash = skb->hash;
 	if (host_override) {
-		dsthost_hash = host_override - 1;
-		srchost_hash = host_override - 1;
+		out->dsthost_hash = host_override - 1;
+		out->srchost_hash = host_override - 1;
 	}
 
 	if (!(flow_mode & CAKE_FLOW_FLOWS)) {
 		if (flow_mode & CAKE_FLOW_SRC_IP)
-			flow_hash ^= srchost_hash;
+			out->flow_hash ^= out->srchost_hash;
 
 		if (flow_mode & CAKE_FLOW_DST_IP)
-			flow_hash ^= dsthost_hash;
+			out->flow_hash ^= out->dsthost_hash;
 	}
+}
+
+/* Commit the flow into the set-associative table (q->tags[], host
+ * indices, bulk counters). Only after the packet is admitted.
+ */
+static u32 cake_hash_resolve(struct cake_tin_data *q,
+			     const struct cake_hash_keys *keys, int flow_mode)
+{
+	u32 flow_hash = keys->flow_hash, srchost_hash = keys->srchost_hash,
+	    dsthost_hash = keys->dsthost_hash;
+	u16 reduced_hash, srchost_idx, dsthost_idx;
+
+	if (unlikely(flow_mode == CAKE_FLOW_NONE))
+		return 0;
 
 	reduced_hash = flow_hash % CAKE_QUEUES;
 
@@ -1765,13 +1788,17 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 {
 	u32 idx, tin, prev_qlen, prev_backlog, drop_id;
 	struct cake_sched_data *q = qdisc_priv(sch);
+	int flow_mode = READ_ONCE(q->config->flow_mode);
 	int len = qdisc_pkt_len(skb), ret;
 	u16 flow_override, host_override;
+	struct sk_buff *segs = NULL;
+	struct cake_hash_keys keys;
 	struct sk_buff *ack = NULL;
 	ktime_t now = ktime_get();
 	struct cake_tin_data *b;
 	struct cake_flow *flow;
 	bool same_flow = false;
+	u64 slen = 0;
 
 	/* Classify first: a terminal verdict means do not enqueue. */
 	if (cake_tcf_classify(sch, skb, &flow_override, &host_override, &ret)) {
@@ -1788,12 +1815,50 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 					 QDISC_DROP_OVERLIMIT);
 	}
 
-	/* choose flow to insert into */
+	/* Select the tin and extract the keys before segmenting:
+	 * skb_gso_segment() rewrites encapsulated headers, which
+	 * changes the keys.
+	 */
 	b = cake_select_tin(sch, skb);
-	idx = cake_hash(b, skb, q->config->flow_mode, flow_override,
-			host_override) + 1;
+	cake_hash_keys(skb, flow_mode, flow_override, host_override, &keys);
+
+	if (qdisc_pkt_segs(skb) > 1 && q->config->rate_flags & CAKE_FLAG_SPLIT_GSO) {
+		netdev_features_t features = netif_skb_features(skb);
+		struct sk_buff *nskb, *seg;
+
+		segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+		if (IS_ERR_OR_NULL(segs))
+			return qdisc_drop_reason(skb, sch, to_free,
+						 QDISC_DROP_GENERIC);
+
+		/* we never use the original skb after splitting */
+		consume_skb(skb);
+		skb = NULL;
+
+		/* The split path accounts the segment sum, not the
+		 * stab-adjusted qdisc_pkt_len(), so check it here.
+		 */
+		skb_list_walk_safe(segs, seg, nskb)
+			slen += seg->len;
+
+		if (unlikely((u64)sch->qstats.backlog + slen > QDISC_MAX_BACKLOG)) {
+			qdisc_qstats_overlimit(sch);
+			/* __skb_gso_segment() puts SKB_GSO_CB at cb
+			 * offset 32, over tc_skb_cb.drop_reason; every
+			 * segment inherits it. Tag each one with the
+			 * qdisc drop reason.
+			 */
+			skb_list_walk_safe(segs, seg, nskb)
+				tcf_set_qdisc_drop_reason(seg,
+							  QDISC_DROP_OVERLIMIT);
+
+			return qdisc_drop_all(segs, sch, to_free);
+		}
+	}
+
+	/* Admitted: resolve the flow. */
+	idx = cake_hash_resolve(b, &keys, flow_mode);
 	tin = (u32)(b - q->tins);
-	idx--;
 	flow = &b->flows[idx];
 
 	/* ensure shaper state isn't stale */
@@ -1820,14 +1885,9 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	if (unlikely(len > b->max_skblen))
 		WRITE_ONCE(b->max_skblen, len);
 
-	if (qdisc_pkt_segs(skb) > 1 && q->config->rate_flags & CAKE_FLAG_SPLIT_GSO) {
-		struct sk_buff *segs, *nskb, *seg;
-		netdev_features_t features = netif_skb_features(skb);
-		unsigned int slen = 0, numsegs = 0;
-
-		segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
-		if (IS_ERR_OR_NULL(segs))
-			return qdisc_drop(skb, sch, to_free);
+	if (segs) {
+		struct sk_buff *nskb, *seg;
+		unsigned int numsegs = 0;
 
 		skb_list_walk_safe(segs, seg, nskb) {
 			skb_mark_not_on_list(seg);
@@ -1841,7 +1901,6 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			qdisc_qlen_inc(sch);
 			numsegs++;
 			q->buffer_used += seg->truesize;
-			slen += seg->len;
 			WRITE_ONCE(b->packets, b->packets + 1);
 		}
 
@@ -1853,9 +1912,8 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		WRITE_ONCE(b->backlogs[idx], b->backlogs[idx] + slen);
 
 		qdisc_tree_reduce_backlog(sch, 1-numsegs, len-slen);
-		consume_skb(skb);
 	} else {
-		/* not splitting */
+		/* not splitting - skb pointer still valid */
 		int ack_pkt_len = 0;
 
 		cobalt_set_enqueue_time(skb, now);
@@ -1949,7 +2007,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		flow->set = CAKE_SET_SPARSE;
 		WRITE_ONCE(b->sparse_flow_count, b->sparse_flow_count + 1);
 
-		WRITE_ONCE(flow->deficit, cake_get_flow_quantum(b, flow, q->config->flow_mode));
+		WRITE_ONCE(flow->deficit, cake_get_flow_quantum(b, flow, flow_mode));
 	} else if (flow->set == CAKE_SET_SPARSE_WAIT) {
 		/* this flow was empty, accounted as a sparse flow, but actually
 		 * in the bulk rotation.
@@ -1958,8 +2016,8 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		WRITE_ONCE(b->sparse_flow_count, b->sparse_flow_count - 1);
 		WRITE_ONCE(b->bulk_flow_count, b->bulk_flow_count + 1);
 
-		cake_inc_srchost_bulk_flow_count(b, flow, q->config->flow_mode);
-		cake_inc_dsthost_bulk_flow_count(b, flow, q->config->flow_mode);
+		cake_inc_srchost_bulk_flow_count(b, flow, flow_mode);
+		cake_inc_dsthost_bulk_flow_count(b, flow, flow_mode);
 	}
 
 	if (q->buffer_used > u64_stats_read(&q->buffer_max_used)) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v10 3/4] net/sched: cap the accounted backlog before it can wrap
  2026-11-14 12:14 ` [PATCH net-next v10 3/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
@ 2026-11-15 23:17   ` netdev-bot+sashiko
  0 siblings, 0 replies; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-11-15 23:17 UTC (permalink / raw)
  To: jhs
  Cc: netdev, jiri, davem, edumazet, kuba, pabeni, horms, stephen,
	vinicius.gomes, victor, toke, bjorn, sashiko-bot

Thank you for your contribution! Sashiko AI review found 8 potential
issue(s) to consider.

Critical: 0 · High: 3 · Medium: 2 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] netem duplication still lets the 32-bit backlog wrap, in netem
  itself and in every guarded parent above it.
- [High] The new guard turns an existing taprio accounting leak into a
  permanent egress blackhole.
- [High] Known parent-backlog drift becomes a permanent drop-all in the
  guarded classful parents.
- [Medium] htb_enqueue() (net/sched/sch_htb.c) runs htb_classify() in a
  declaration initializer (line 632), before the new…
- [Medium] On any non-success child return, red_enqueue()
  (net/sched/sch_red.c) now saves `red_backlog =…
- [Low] The commit has no Fixes: tag, and its only provenance reference
  is misleading.
- [Low] The commit message says 'qdisc_pkt_len() itself stays truthful
  for every input: it is not clamped and the initializer keeps…
- [Low] plug_enqueue() (net/sched/sch_plug.c, lines 92-93) changes its
  admission test from `(u64)sch->qstats.backlog + skb->len <= q->limit`…

--

net/sched: cap the accounted backlog before it can wrap

This patch adds qdisc_backlog_overflow() and calls it on the enqueue path
of most qdiscs under net/sched. The goal is that a stab-inflated or
GSO-inflated qdisc_pkt_len() can no longer wrap the 32-bit
sch->qstats.backlog. RED now reserves the length in its own backlog before
calling the child and re-derives the aggregate on failure, and plug's
admission test switches from skb->len to qdisc_pkt_len().

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260818101130.16203-1-jhs@mojatatu.com
Link: https://lore.kernel.org/netdev/CANn89iLfMJV7ancKH1Gjzzm7ZUG-gKcrczkJjJEWN5sCuWd-ug@mail.gmail.com/
Link: https://lore.kernel.org/netdev/CANn89i+GOFH_8g+vSV0jUj0aBqLYVWuFOrHuDggZu9kPC_m6tQ@mail.gmail.com/
Link: https://lore.kernel.org/netdev/87a4oottcj.fsf@toke.dk/
Link: https://lore.kernel.org/netdev/87pkxks68f.fsf@toke.dk/
Link: https://lore.kernel.org/netdev/179231932946.3135.7497478990186385688@kernel.org/
Link: https://lore.kernel.org/netdev/179269577131.3135.3967915390403138015@kernel.org/

> This is a followup of commit 709f34f7c28d ("net/sched: fq: add overflow
> bounds to quantum and initial quantum") as reported by Sashiko.

[Severity: Low]
Is 709f34f7c28d the right reference? That commit clamps fq's
psched_mtu() derived quantum and initial_quantum, and it carries its own
Fixes: afe4fd062416. It never touches qstats.backlog accounting.

The wrap fixed here comes from a stab-inflated or GSO-inflated
qdisc_pkt_len() being added to each qdisc's 32-bit backlog, and that is
much older.

The message describes a user-reachable NULL flow->head dereference in
fq_codel_drop(). Could a Fixes: tag be added so stable maintainers know
which trees need this?

> qdisc_pkt_len() itself stays truthful for every input: it is not
> clamped and the initializer keeps succeeding, so BPF

[Severity: Low]
Does this match the current tree? __qdisc_calculate_pkt_len() in
net/sched/sch_api.c clamps the stab-computed length:

	pkt_len = clamp_t(int, pkt_len, 1, QDISC_PKT_LEN_MAX);

Also, qdisc_pkt_len_segs_init() in net/core/dev.c rejects SKB_GSO_DODGY
skbs whose wire estimate is above QDISC_PKT_LEN_MAX:

		if (unlikely(gso_segs > GSO_MAX_SEGS ||
			     skb->len + (u64)(gso_segs - 1) * hdr_len >
			     QDISC_PKT_LEN_MAX))
			return SKB_DROP_REASON_SKB_BAD_GSO;

The sentence also contradicts the earlier statement that a stab inflates
qdisc_pkt_len() "up to QDISC_PKT_LEN_MAX". Was the intent that this patch
adds no further clamp on the trusted GSO path?

[ ... ]

> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
> index 1ba67b121de47..4ee8d3fe9cae9 100644
> --- a/net/sched/sch_htb.c
> +++ b/net/sched/sch_htb.c
> @@ -630,6 +630,12 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>  	struct htb_sched *q = qdisc_priv(sch);
>  	struct htb_class *cl = htb_classify(skb, sch, &ret);
>  
> +	if (unlikely(qdisc_backlog_overflow(sch, skb))) {

[Severity: Medium]
htb_classify() runs in the declaration initializer, before this guard.
That means filter actions such as mirred, police and ct have already run.

Suppose the classifier returned NULL with NET_XMIT_SUCCESS |
__NET_XMIT_STOLEN (TC_ACT_STOLEN, QUEUED or TRAP). The existing branch
would return ret without counting a drop:

	} else if (!cl) {
		if (ret & __NET_XMIT_BYPASS)
			qdisc_qstats_drop(sch);
		__qdisc_drop(skb, to_free);
		return ret;

When the backlog is at the ceiling, does this guard count an overlimit
and a drop instead, and return NET_XMIT_DROP for a packet the classifier
already consumed? Parents would then count a drop too, because
net_xmit_drop_count() is 1 for a non-STOLEN code.

The other classful qdiscs in the patch (prio, drr, ets, hfsc, qfq, sfq,
sfb) check before classifying. fq_codel, cake and fq_pie handle the
terminal verdict before the guard. Should htb do one of the two?

[Severity: High]
Can this guard permanently block the whole htb tree when a leaf is hhf
or skbprio? Both return NET_XMIT_CN after evicting a different packet,
and neither calls qdisc_tree_reduce_backlog(). The RED comment in this
patch names both:

net/sched/sch_hhf.c:hhf_enqueue() {
    ...
	if (hhf_drop(sch, to_free) == idx)
		return NET_XMIT_CN;
    ...
}

htb_enqueue() only adds len on NET_XMIT_SUCCESS. So it keeps counting
the evicted packet, and on dequeue it subtracts the new packet's length.
Take an hhf leaf at its limit: a 1500 byte heavy-hitter packet that
evicts a 100 byte head leaves htb undercounting by 1400 bytes.

When the queue drains, htb's backlog ends up at 2^32 - 1400. That is
above QDISC_MAX_BACKLOG (2^32 - 1 - 2^20). htb's backlog only changes on
enqueue and dequeue, so it stays there. qdisc_backlog_overflow() then
rejects every later packet for every class until the qdisc is
reconfigured. Any net undercount between 1 byte and 1 MiB at drain time
has the same effect.

skbprio (default limit 64) does the same under prio or htb when large
high-priority packets evict small low-priority ones. prio, drr, ets,
qfq, hfsc, cbs, sfb and tbf use the same add-on-success pattern.

Before this patch the drift only made the stats wrong. Only RED gets the
re-derivation. Do the other parents need similar handling?

> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index 1dc2924890eb5..9a7b11594fff0 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -467,6 +467,12 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>  	unsigned int prev_len = qdisc_pkt_len(skb);
>  	int count = 1;
>  
> +	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
> +		qdisc_qstats_overlimit(sch);
> +		return qdisc_drop_reason(skb, sch, to_free,
> +					 QDISC_DROP_OVERLIMIT);
> +	}

[Severity: High]
Can netem duplication still wrap the backlog? This guard runs once, on
the backlog from before either packet is accounted. Later in
netem_enqueue(), the duplicate goes back through the root before the
original is accounted:

net/sched/sch_netem.c:netem_enqueue() {
    ...
		skb2->tc_depth++; /* prevent duplicating a dup... */
		rootq->enqueue(skb2, rootq, to_free);
		skb2 = NULL;
	}

	qdisc_qstats_backlog_inc(sch, skb);
    ...
}

The re-entered root enqueue reaches each ancestor's guard, and netem's
own guard, with the same old backlog. They all pass and add len(skb2).
The outer call then adds len(skb).

Parents such as prio_enqueue() only add len after the child returns:

	ret = qdisc_enqueue(skb, qdisc, to_free);
	if (ret == NET_XMIT_SUCCESS) {
		qstats_backlog_add(sch, len);

So prio, htb, drr, ets, hfsc, qfq, sfb, cbs and tbf also take two
packets per check, but QDISC_MAX_BACKLOG only leaves room for one
QDISC_PKT_LEN_MAX packet.

Example: stab overhead 2000000000 (clamped to 1 MiB), netem duplicate
100%, a limit of 4096 or more, and a delay. The backlog grows in 2 MiB
steps. At 4094 MiB the guard passes and the backlog ends at 4096 MiB,
which wraps to 0. This happens with netem as root, and also as a leaf
under htb or prio.

Under RED, the reservation covers RED's own counter. But
qdisc_backlog_overflow(child, skb) still sees netem's stale value, so
netem's backlog, which RED feeds into red_calc_qavg(), can still wrap.

The RED comment mentions the netem clone re-entry, yet the commit
message lists netem and these parents as covered. Would reserving before
calling the child, or leaving headroom for two packets, close this path?

[ ... ]

> diff --git a/net/sched/sch_plug.c b/net/sched/sch_plug.c
> index b60ddfee6a68a..5654e5310fb58 100644
> --- a/net/sched/sch_plug.c
> +++ b/net/sched/sch_plug.c
> @@ -89,7 +89,8 @@ static int plug_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>  {
>  	struct plug_sched_data *q = qdisc_priv(sch);
>  
> -	if (likely((u64)sch->qstats.backlog + skb->len <= q->limit)) {
> +	if (likely((u64)sch->qstats.backlog + qdisc_pkt_len(skb) <=
> +		   q->limit)) {

[Severity: Low]
This isn't a bug, but it changes what plug's configured byte limit means.
The limit now includes stab overhead and the per-segment headers that
qdisc_pkt_len_segs_init() adds for GSO skbs.

Existing plug setups (for example Remus/COLO style checkpointing) that
carry GSO traffic or have a stab attached will reach the limit sooner.
Could the commit message mention this change in behaviour?

[ ... ]

> diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
> index d7598214270b8..f3783e436da7c 100644
> --- a/net/sched/sch_red.c
> +++ b/net/sched/sch_red.c

[ ... ]

> @@ -136,14 +149,37 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>  	}
>  
>  	len = qdisc_pkt_len(skb);
> +	/* Reserve the length before the child sees the packet: netem
> +	 * can re-enqueue a clone here before the backlog is updated.
> +	 */
> +	red_backlog = READ_ONCE(sch->qstats.backlog);
> +	child_backlog = READ_ONCE(child->qstats.backlog);
> +	qstats_backlog_add(sch, len);
>  	ret = qdisc_enqueue(skb, child, to_free);
>  	if (likely(ret == NET_XMIT_SUCCESS)) {
> -		qstats_backlog_add(sch, len);
>  		qdisc_qlen_inc(sch);
> -	} else if (net_xmit_drop_count(ret)) {
> -		WRITE_ONCE(q->stats.pdrop,
> -			   q->stats.pdrop + 1);
> -		qdisc_qstats_drop(sch);
> +	} else {
> +		u32 propagated, grew;
> +

[ ... ]

> +		propagated = READ_ONCE(sch->qstats.backlog) -
> +			     red_backlog - len;
> +		grew = READ_ONCE(child->qstats.backlog) - child_backlog;
> +		WRITE_ONCE(sch->qstats.backlog, red_backlog + grew);

[Severity: Medium]
Can this absolute write-back lose a concurrent update to
sch->qstats.backlog? red_backlog is sampled before
qdisc_enqueue(skb, child), and the field is overwritten only after the
whole child enqueue has finished. That can include classification and
netem's root re-enqueue.

One writer does not hold the root lock. tc_fill_qdisc() calls
q->ops->dump() under RTNL only, and red_dump()->red_dump_offload_stats()
passes the qstats to the driver:

	.stats.qstats = &sch->qstats,

mlxsw then does a read-modify-write on it:

drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c:mlxsw_sp_qdisc_update_stats() {
    ...
	stats_ptr->qstats->backlog += mlxsw_sp_cells_bytes(mlxsw_sp, backlog);
    ...
}

If that delta is lost, later negative deltas from the driver (or the
clean-stats qstats->backlog -= backlog) can push RED's backlog below its
true value and wrap it. Once the wrapped value is above
QDISC_MAX_BACKLOG, qdisc_backlog_overflow(sch, skb) drops every software
path packet at RED.

Before this patch, this writer could only race a single in-place +=.
Could the failure path apply a delta instead of restoring the snapshot?

[ ... ]

> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 5a1dca9d5e9de..bdb2596de0ef5 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -590,6 +590,12 @@ static int taprio_enqueue_one(struct sk_buff *skb, struct Qdisc *sch,
>  {
>  	struct taprio_sched *q = qdisc_priv(sch);
>  
> +	if (unlikely(qdisc_backlog_overflow(sch, skb))) {
> +		qdisc_qstats_overlimit(sch);
> +		return qdisc_drop_reason(skb, sch, to_free,
> +					 QDISC_DROP_OVERLIMIT);
> +	}

[Severity: High]
Does this guard turn taprio's existing backlog leak into a permanent
egress blackhole? taprio_enqueue_one() accounts the packet before the
child enqueue, and nothing undoes that if the child drops it:

net/sched/sch_taprio.c:taprio_enqueue_one() {
    ...
	qdisc_qstats_backlog_inc(sch, skb);
	qdisc_qlen_inc(sch);

	return qdisc_enqueue(skb, child, to_free);
}

In software mode, taprio_init() creates the per-txq children with
qdisc_create_dflt(..., &pfifo_qdisc_ops, ...). pfifo drops through
qdisc_drop() and does not call qdisc_tree_reduce_backlog().
taprio_dequeue_from_txq() only decrements the backlog for packets it
actually dequeues.

So each time a gate window is oversubscribed, every child drop leaves
its qdisc_pkt_len() in taprio's backlog. After about 4 GiB of such drops
(roughly 2.9M 1500 byte packets), the backlog passes QDISC_MAX_BACKLOG.
>From then on this check drops every packet on the interface, and nothing
brings the counter down until the qdisc is reset.

taprio_enqueue_segmented() leaks the same way through
taprio_enqueue_one(). Before this patch, the leak only made the stats
wrong.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-BA27.v10.r0.20261114071135%40mojatatu.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v10 4/4] net/sched: cake: hash before segmenting and reject an overlong split
  2026-11-14 12:14 ` [PATCH net-next v10 4/4] net/sched: cake: hash before segmenting and reject an overlong split Jamal Hadi Salim
@ 2026-11-15 23:17   ` netdev-bot+sashiko
  0 siblings, 0 replies; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-11-15 23:17 UTC (permalink / raw)
  To: jhs
  Cc: netdev, jiri, davem, edumazet, kuba, pabeni, horms, stephen,
	vinicius.gomes, victor, toke, bjorn, sashiko-bot

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit fixes three real bugs in cake_enqueue()
  (net/sched/sch_cake.c) but carries no Fixes: tag, so a stable…
- [Low] The "Conditions to recreate the bug" section does not produce any
  visible failure on the unpatched kernel.

--

net/sched: cake: hash before segmenting and reject an overlong split

This splits cake_hash() into a side-effect-free cake_hash_keys() and a
cake_hash_resolve() that runs only after admission. The split-GSO path
moves ahead of resolution, with a ceiling check on the segment sum and
per-segment drop reason tagging. flow_mode is read once with READ_ONCE()
in cake_enqueue().

Link: https://lore.kernel.org/netdev/179231933068.3135.80340792633077986@kernel.org/
Link: https://lore.kernel.org/netdev/179269577270.3135.16072065138845192450@kernel.org/

> net/sched: cake: hash before segmenting and reject an overlong split
>
> The Fix:
> with CAKE_FLAG_SPLIT_GSO, cake computes the flow/host hash keys, then
> splits a GSO skb into segments it accounts individually. Key extraction

[ ... ]

> Read q->config->flow_mode once with READ_ONCE() in cake_enqueue() and
> pass that value to both cake_hash_keys() and cake_hash_resolve(), and to
> the later flowchain calls (cake_get_flow_quantum(),
> cake_inc_srchost_bulk_flow_count(), cake_inc_dsthost_bulk_flow_count()).
> cake_mq_change() updates the shared cake_config outside the qdisc lock, so
> a reconfiguration could extract and resolve keys in different modes.

[Severity: Low]
The message calls these changes fixes, but there is no Fixes: tag. Could
Fixes: tags be added, so stable backporters can tell which trees need
which part?

The three problems fixed in cake_enqueue() seem to come from different
commits.

First, the set-associative state (q->tags[], host indices, way counters)
was committed before a segmentation failure dropped the packet. That
ordering goes back to the split-GSO path:

  0c850344d388 ("sch_cake: Conditionally split GSO segments")

Second, the segmentation-failure path called plain qdisc_drop():

	segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
	if (IS_ERR_OR_NULL(segs))
		return qdisc_drop(skb, sch, to_free);

By then __skb_gso_segment() had overwritten tc_skb_cb.drop_reason. That
only became a misreport once __dev_xmit_skb() started setting
QDISC_DROP_GENERIC and __tcf_kfree_skb_list() started reading the field:

  ff2998f29f39 ("net: sched: introduce qdisc-specific drop reason tracing")

Third, flow_mode could change between key extraction and resolution.
That depends on the shared cake_mq config, where cake_mq_change() calls
cake_config_change() without the child qdisc lock:

  bc0ce2bad36c ("net/sched: sch_cake: Factor out config variables into separate struct")
  ebc65a873eff ("net/sched: sch_cake: Add cake_mq qdisc for using cake on mq devices")
  87826c01837c ("net/sched: sch_cake: Share config across cake_mq sub-qdiscs")

The drop reason fix only matters on kernels that have ff2998f29f39. The
flow_mode fix only matters on kernels that have the shared cake_mq
config.

> Conditions to recreate the bug:
>   ip tuntap add tun0 mode tun
>   ip link set tun0 txqueuelen 32 up
>   ip addr add 10.99.0.1/24 dev tun0
>   # hold the tun fd open without reading so the backlog persists; a stab
>   # overhead of 1048468 accounts each 92-byte packet at 1048560 bytes, so
>   # 4095 queued packets leave 65519 bytes below QDISC_MAX_BACKLOG.  A
>   # 65000-byte UDP_SEGMENT payload with a 1400-byte segment size then has
>   # a segment sum past the remaining headroom, and cake rejects the whole
>   # split.  Setup needs CAP_NET_ADMIN; the payload itself does not.
>   tc qdisc add dev tun0 root handle 1: stab overhead 1048468 \
>       cake split-gso memlimit 512Mb

[Severity: Low]
Does this reproducer show any failure on the unpatched kernel? It seems
to only describe a case that the patched cake_enqueue() now rejects.

Working through the numbers:

  QDISC_MAX_BACKLOG = U32_MAX - (1 << 20) = 4293918719
  4095 * 1048560 = 4293853200, leaving 65519 bytes of headroom

The GSO skb's stab-adjusted length is 66316 + 1048468.
__qdisc_calculate_pkt_len() clamps it:

net/sched/sch_api.c:__qdisc_calculate_pkt_len() {
    ...
	pkt_len = clamp_t(int, pkt_len, 1, QDISC_PKT_LEN_MAX);
    ...
}

So qdisc_backlog_overflow() takes the first branch and admits the
packet:

include/net/sch_generic.h:qdisc_backlog_overflow() {
    ...
	if (likely(len <= QDISC_PKT_LEN_MAX))
		return backlog > QDISC_MAX_BACKLOG;
	return len > U32_MAX - backlog;
}

The 65000-byte payload with a 1400-byte segment size splits into 47
segments, so slen = 65000 + 47 * 28 = 66316.

On the parent commit the backlog then reaches 4293919516. That is 797
bytes over QDISC_MAX_BACKLOG but about 1 MiB below U32_MAX, so nothing
wraps. The next enqueue is rejected by qdisc_backlog_overflow() anyway.

The core helper itself lets the backlog go past QDISC_MAX_BACKLOG when a
packet of up to 1 MiB arrives while the backlog is still at or below the
ceiling. So going past the ceiling is not a failure on its own.

A real wrap in the split path seems to need slen above roughly 1 MiB
while the clamped pkt_len still passes the helper's check, for example a
trusted small-MSS BIG TCP aggregate. Could the message describe that
case, so maintainers can judge the severity and stable relevance?

The subject line also says "hash before segmenting", but the parent code
already called cake_hash(b, skb, ...) before skb_gso_segment(). The real
change is that cake_hash_resolve() now waits until after admission, and
segmentation and the ceiling check now run before it. Could the subject
say that instead?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-BA27.v10.r0.20261114071135%40mojatatu.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-11-15 23:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-14 12:14 [PATCH net-next v10 0/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
2026-11-14 12:14 ` [PATCH net-next v10 1/4] net/sched: cake: return the classifier result to the caller Jamal Hadi Salim
2026-11-14 12:14 ` [PATCH net-next v10 2/4] net/sched: cake: use a dedicated split-GSO list iterator Jamal Hadi Salim
2026-11-14 12:14 ` [PATCH net-next v10 3/4] net/sched: cap the accounted backlog before it can wrap Jamal Hadi Salim
2026-11-15 23:17   ` netdev-bot+sashiko
2026-11-14 12:14 ` [PATCH net-next v10 4/4] net/sched: cake: hash before segmenting and reject an overlong split Jamal Hadi Salim
2026-11-15 23:17   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox