[PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[Jamal Hadi Salim]

This is a follow-up to an issue found by Sashiko (nipa) during review of
commit d9ebd8f9aa8b ("net/sched: fq_codel: clamp default quantum and
mtu"), part of the quantum/mtu overflow series merged as a687f2ae995f.
Clamping the per-flow quantum and the CoDel mtu does not bound the
accounted backlog, so the per-flow backlog wrap the review flagged
remained.

fq_codel, cake, codel, pie, fq_pie and dualpi2 account every enqueued
packet's stab-adjusted length into a 32-bit sch->qstats.backlog.
fq_codel/codel use it to decide if they should drop a packet at dequeue;
cake uses it to prune the longest-flow heap from per-flow backlogs; pie
and fq_pie use it to make early drop decisions and dualpi2 decides
must_drop() on it. RED can decide on a child's backlog based on it.
A crafted tc size table (TC_STAB) inflates qdisc_pkt_len() up to
QDISC_PKT_LEN_MAX (1 MiB), 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 keep the counter corrupt.

In fq_codel the wrapped value is worse than a wrong drop: a wrapped
q->backlogs[idx] can leave fq_codel_drop()'s strict '>' scan selecting
idx 0 while flow 0 is empty, so dequeue_head() dereferences a NULL
flow->head. The wrap is what the reproducer below observes; the NULL
dereference is derived from code inspection (it needs flows >= 2, flow 0
empty, and another flow's counter back at exactly 0 mod 2^32 with the
qdisc at its drop trigger).

Fix:
Drop at enqueue once the stored backlog is within QDISC_MAX_BACKLOG
(U32_MAX - QDISC_PKT_LEN_MAX) of the wrap point, the largest backlog one
more stab-clamped packet cannot wrap. Test the stored backlog itself via
qdisc_backlog_at_max() rather than adding the incoming packet's length,
so there is no per-enqueue 64-bit add and the 1 MiB headroom already
reserved by QDISC_MAX_BACKLOG is not counted twice. The bound is exact for
every producer: a stab-clamped length is capped by QDISC_PKT_LEN_MAX, and
the stabless GSO producer is capped by qdisc_pkt_len_segs_init() (the
SPLIT_GSO path additionally re-checks its segment sum). This follows the
existing bfifo approach, which bounds bytes against the accounted
length; gred is a precedent only in WRED mode, where it bounds the
aggregate, while non-WRED gred bounds each virtual queue. The fixed
qdiscs' limits are packet counts or otherwise do not bound the aggregate
bytes. pfifo, reachable standalone or grafted as RED's or SFB's child,
is fixed too because RED reads a child's backlog into red_calc_qavg().
RED's guard bounds only the primary enqueue: a child that segments a GSO
skb (tbf, netem) still grows the parent aggregate through
qdisc_tree_reduce_backlog()'s unclamped byte delta, left to a separate
change.

fq_codel and cake both expose an optional external classifier whose
terminal TC actions (STOLEN/QUEUED/TRAP/SHOT) consume the packet without
enqueuing it. The ceiling check runs after that classifier and its
terminal result, but before flow-hash/shaper state, so a redirect, trap
or police action keeps working while the backlog sits at the ceiling,
and a packet about to be dropped still does not disturb the flow hash.
Only a packet admitted past the check mutates a flow: for fq_codel the
guard precedes fq_codel_hash(); for cake the guard precedes both the key
extraction and the split-GSO segmentation, so the flow/host keys
are taken from the packet as received rather than from a segment whose
encapsulated headers skb_gso_segment() may have rewritten. CAKE's
set-associative resolution, which writes q->tags[], the flow host
indices, the way counters and the host bulk-flow counts, runs only once
the packet is admitted. fq_codel's external filter selects a flow only
when the result's minor is nonzero and no larger than flows_cnt; a
matched filter that supplied no class ID (classid 0) is the no-flow
result and the packet is dropped, as before.

CAKE's SPLIT_GSO path accounts the sum of the segment lengths, which a
stab recomputes from skb->len and can exceed the pre-split
qdisc_pkt_len(). The list is summed before any segment is linked and
rejected as a whole before any flow state is resolved: the flow/host keys
are computed from the unsegmented packet, but the set-associative
resolution that commits q->tags[], the flow host indices, the way
counters and the host bulk-flow counts runs only after the whole
segment-sum admission test passes. A packet rejected by that test
therefore leaves no persistent state behind. The whole list is dropped
via to_free so the tail segment's sock_wfree() never runs under the qdisc
lock. A fraglist GSO is the exception: skb_segment_list() returns the
original skb as the list head carrying an extra reference, so segs ==
skb; dropping the caller's reference and queueing the list once avoids
self-linking *to_free, which would otherwise free the same skb repeatedly
in the deferred teardown.

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 20000 ecn
  # hold the tun fd open without reading (IFF_BACKPRESSURE) so the qdisc
  # backlog persists, then send 6000 packets. At 4096 resident 1 MiB
  # packets the counter wraps; the unfixed kernel then reports a wrapped
  # backlog for 5969 resident packets, the fixed one drops at the ceiling.

  # RED with a grafted packet-limited child reads the child's backlog the
  # same way; the default bfifo child is byte-limited and safe, pfifo is
  # packet-limited:
  tc qdisc add dev tun0 root handle 1: stab overhead 2000000000 \
      red limit 1000000000 min 5000 max 10000 avpkt 1000 burst 32 ecn
  tc qdisc add dev tun0 parent 1:1 handle 30: pfifo limit 100000

  # a terminal action at the ceiling: fill the queue as above, then
  #   tc filter add dev tun0 parent 1: protocol ip prio 1 matchall \
  #       action trap
  # and send more packets; the trap action's stats must advance.

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/CANn89i+99jPh7JjZf=G3Ouaom2tuOtAEiHUf2MEsRdVPd=nz4w@mail.gmail.com/
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-BA27.v3.20260929142417%40mojatatu.com
Link: https://lore.kernel.org/netdev/20261006153043.GM83879@horms.kernel.org/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v5 Considered Comments from Simon (and nipa on v3):
  - 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.
 include/net/pkt_sched.h   |   1 -
 include/net/sch_generic.h |  15 +++
 net/core/dev.c            |   8 ++
 net/sched/sch_cake.c      | 198 ++++++++++++++++++++++++++++----------
 net/sched/sch_codel.c     |  16 +--
 net/sched/sch_dualpi2.c   |   1 +
 net/sched/sch_fifo.c      |   6 ++
 net/sched/sch_fq_codel.c  |  57 ++++++++---
 net/sched/sch_fq_pie.c    |   3 +-
 net/sched/sch_pie.c       |   3 +-
 net/sched/sch_red.c       |   6 ++
 11 files changed, 241 insertions(+), 73 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..43a3e1e9ee89 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -915,6 +915,21 @@ 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 accounted backlog one more maximum-size packet cannot wrap
+ * the 32-bit sch->qstats.backlog past.
+ */
+#define QDISC_MAX_BACKLOG	(U32_MAX - QDISC_PKT_LEN_MAX)
+
+/* Backlog close enough to U32_MAX that one more packet could wrap it.
+ * A packet-limited qdisc must drop at enqueue when this holds.
+ */
+static inline bool qdisc_backlog_at_max(const struct Qdisc *sch)
+{
+	return READ_ONCE(sch->qstats.backlog) > QDISC_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/core/dev.c b/net/core/dev.c
index f587645e930a..e6ac94c61ea0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4268,6 +4268,14 @@ static enum skb_drop_reason qdisc_pkt_len_segs_init(struct sk_buff *skb)
 		qdisc_skb_cb(skb)->pkt_segs = gso_segs;
 	}
 	qdisc_skb_cb(skb)->pkt_len += (gso_segs - 1) * hdr_len;
+	/* (gso_segs - 1) * hdr_len can exceed QDISC_PKT_LEN_MAX for a dodgy
+	 * GSO skb, so clamp the producer the same way a size table is
+	 * clamped (__qdisc_calculate_pkt_len()). The accounted backlog stays
+	 * within one stab-clamped packet of U32_MAX and cannot wrap.
+	 */
+	qdisc_skb_cb(skb)->pkt_len = min_t(unsigned int,
+					   qdisc_skb_cb(skb)->pkt_len,
+					   QDISC_PKT_LEN_MAX);
 	return SKB_NOT_DROPPED_YET;
 }
 
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index dc93267029e7..6484c4fa0ccc 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -706,19 +706,33 @@ 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;
+};
+
+/* Compute the flow/host hash keys from the packet as received. Must run
+ * before skb_gso_segment(), which rewrites an encapsulated GSO packet's
+ * headers to the inner ones and would change the keys. Reads only the skb
+ * and commits no qdisc state, so it may run before admission is known.
+ */
+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
@@ -753,50 +767,70 @@ 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;
 	}
+}
+
+/* Resolve the keys from cake_hash_keys() into the set-associative flow
+ * table. Mutates q->tags[], the flow host indices, the way counters and
+ * the host bulk-flow counts, so run only after admission.
+ */
+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;
+
+	/* Duplicate of the guard in cake_hash_keys(): the original single
+	 * cake_hash() returned 0 here, short-circuiting both halves. Without
+	 * it the flowblind path would commit set-associative state it never
+	 * touched before.
+	 */
+	if (unlikely(flow_mode == CAKE_FLOW_NONE))
+		return 0;
 
 	reduced_hash = flow_hash % CAKE_QUEUES;
 
@@ -1712,18 +1746,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 optional external classifier. Returns true when a terminal TC
+ * action consumed the packet (reason in *qerr); otherwise *flow and *host
+ * hold the filter's overrides (0 if none) and the packet is hashed as
+ * before.
+ */
+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);
@@ -1737,17 +1778,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);
@@ -1758,22 +1797,87 @@ 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 *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;
 
-	/* choose flow to insert into */
-	idx = cake_classify(sch, &b, skb, q->config->flow_mode, &ret);
-	if (idx == 0) {
+	/* Classify first so terminal TC actions (STOLEN/QUEUED/TRAP/SHOT)
+	 * keep working at the ceiling; carry the overrides to
+	 * cake_hash_keys().
+	 */
+	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;
 	}
+
+	/* Drop before hash/shaper state so a rejected packet disturbs
+	 * neither; the QDISC_MAX_BACKLOG margin leaves room for one more
+	 * stab-clamped packet (SPLIT_GSO re-checks its segment sum below).
+	 */
+	if (unlikely(qdisc_backlog_at_max(sch))) {
+		qdisc_qstats_overlimit(sch);
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
+	/* Tin selection and key extraction must run on the unsegmented skb:
+	 * skb_gso_segment() rewrites an encapsulated GSO packet's headers to
+	 * the inner ones, changing the keys. Neither commits state.
+	 */
+	b = cake_select_tin(sch, skb);
+	cake_hash_keys(skb, q->config->flow_mode, flow_override,
+		       host_override, &keys);
+
+	if (qdisc_pkt_segs(skb) > 1 && q->config->rate_flags & CAKE_FLAG_SPLIT_GSO) {
+		struct sk_buff *nskb, *seg;
+		netdev_features_t features = netif_skb_features(skb);
+
+		segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+		if (IS_ERR_OR_NULL(segs))
+			return qdisc_drop(skb, sch, to_free);
+
+		/* The split path accounts the segment sum, not the
+		 * stab-adjusted qdisc_pkt_len(), so the top check does not
+		 * bound it. Sum the list and reject it whole.
+		 */
+		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() moved the sock_wfree destructor
+			 * to the tail segment; defer the whole list to
+			 * to_free, never free it under the qdisc lock.
+			 *
+			 * Fraglist GSO is the exception: skb_segment_list()
+			 * returns the original skb as the head with an extra
+			 * ref (segs == skb), so drop the caller's ref and
+			 * queue the list once, or the two drops self-link it.
+			 */
+			if (unlikely(segs == skb)) {
+				tcf_set_qdisc_drop_reason(skb,
+							  QDISC_DROP_OVERLIMIT);
+				consume_skb(skb);
+				return qdisc_drop_all(segs, sch, to_free);
+			}
+			__qdisc_drop_all(segs, to_free);
+			return qdisc_drop_reason(skb, sch, to_free,
+						 QDISC_DROP_OVERLIMIT);
+		}
+	}
+
+	/* Admitted: resolve the keys and commit flow/shaper state. */
+
+	idx = cake_hash_resolve(b, &keys, q->config->flow_mode);
 	tin = (u32)(b - q->tins);
-	idx--;
 	flow = &b->flows[idx];
 
 	/* ensure shaper state isn't stale */
@@ -1800,28 +1904,22 @@ 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;
-		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, 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;
 			WRITE_ONCE(b->packets, b->packets + 1);
 		}
 
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index a1ba2e355f2d..ff7e72989dfc 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_at_max(sch))) {
+		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_dualpi2.c b/net/sched/sch_dualpi2.c
index 4947def7c49e..5c746e9b60cd 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_at_max(sch)) ||
 	    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_fifo.c b/net/sched/sch_fifo.c
index 1b6388d50967..d5e5cd49377d 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_at_max(sch)))
+		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_at_max(sch)))
+		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_codel.c b/net/sched/sch_fq_codel.c
index e6c87a32950f..7c52acf201c6 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -73,22 +73,34 @@ static unsigned int fq_codel_hash(const struct fq_codel_sched_data *q,
 	return reciprocal_scale(skb_get_hash(skb), q->flows_cnt);
 }
 
-static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
-				      int *qerr)
+/* Run the optional external classifier. Returns true when the caller must
+ * drop (a terminal TC action consumed the packet, or the result carried no
+ * class ID); the reason is then in *qerr. Else *idx is the 1-based flow
+ * index, or *do_hash asks the caller to use fq_codel_hash().
+ */
+static bool fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
+			      unsigned int *idx, bool *do_hash, int *qerr)
 {
 	struct fq_codel_sched_data *q = qdisc_priv(sch);
 	struct tcf_proto *filter;
 	struct tcf_result res;
 	int result;
 
+	*idx = 0;
+	*do_hash = false;
+
 	if (TC_H_MAJ(skb->priority) == sch->handle &&
 	    TC_H_MIN(skb->priority) > 0 &&
-	    TC_H_MIN(skb->priority) <= q->flows_cnt)
-		return TC_H_MIN(skb->priority);
+	    TC_H_MIN(skb->priority) <= q->flows_cnt) {
+		*idx = TC_H_MIN(skb->priority);
+		return false;
+	}
 
 	filter = rcu_dereference_bh(q->filter_list);
-	if (!filter)
-		return fq_codel_hash(q, skb) + 1;
+	if (!filter) {
+		*do_hash = true;
+		return false;
+	}
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 	result = tcf_classify_qdisc(skb, filter, &res, false);
@@ -101,13 +113,16 @@ static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
 			*qerr = NET_XMIT_SUCCESS | __NET_XMIT_STOLEN;
 			fallthrough;
 		case TC_ACT_SHOT:
-			return 0;
+			return true;
 		}
 #endif
-		if (TC_H_MIN(res.classid) <= q->flows_cnt)
-			return TC_H_MIN(res.classid);
+		if (TC_H_MIN(res.classid) > 0 &&
+		    TC_H_MIN(res.classid) <= q->flows_cnt) {
+			*idx = TC_H_MIN(res.classid);
+			return false;
+		}
 	}
-	return 0;
+	return true;
 }
 
 /* helper functions : might be changed when/if skb use a standard list_head */
@@ -188,17 +203,33 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct fq_codel_sched_data *q = qdisc_priv(sch);
 	unsigned int idx, prev_backlog, prev_qlen;
 	struct fq_codel_flow *flow;
-	int ret;
+	bool do_hash = false;
 	unsigned int pkt_len;
 	bool memory_limited;
+	int ret;
 
-	idx = fq_codel_classify(skb, sch, &ret);
-	if (idx == 0) {
+	/* Classify first so terminal TC actions (STOLEN/QUEUED/TRAP/SHOT)
+	 * keep working at the ceiling; only hashing is deferred below.
+	 */
+	if (fq_codel_classify(skb, sch, &idx, &do_hash, &ret)) {
 		if (ret & __NET_XMIT_BYPASS)
 			qdisc_qstats_drop(sch);
 		__qdisc_drop(skb, to_free);
 		return ret;
 	}
+
+	/* Drop before hashing so a rejected packet disturbs no flow hash;
+	 * the QDISC_MAX_BACKLOG margin leaves room for one more stab-clamped
+	 * packet.
+	 */
+	if (unlikely(qdisc_backlog_at_max(sch))) {
+		q->drop_overlimit++;
+		return qdisc_drop_reason(skb, sch, to_free,
+					 QDISC_DROP_OVERLIMIT);
+	}
+
+	if (do_hash)
+		idx = fq_codel_hash(q, skb) + 1;
 	idx--;
 
 	codel_set_enqueue_time(skb);
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 5982847df8f8..394f015cec36 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_at_max(sch))) {
 		q->stats.overlimit++;
 		goto out;
 	} else if (unlikely(memory_limited)) {
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 3b7863ffd284..9e38f38ce923 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_at_max(sch))) {
 		WRITE_ONCE(q->stats.overlimit, q->stats.overlimit + 1);
 		goto out;
 	}
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index d7598214270b..e3a254f9dbdf 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -76,6 +76,12 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	unsigned int len;
 	int ret;
 
+	if (unlikely(qdisc_backlog_at_max(sch))) {
+		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);
-- 
2.43.0

Re: [PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[Toke Høiland-Jørgensen]

> This is a follow-up to an issue found by Sashiko (nipa) during review of
> commit d9ebd8f9aa8b ("net/sched: fq_codel: clamp default quantum and
> mtu"), part of the quantum/mtu overflow series merged as a687f2ae995f.
> Clamping the per-flow quantum and the CoDel mtu does not bound the
> accounted backlog, so the per-flow backlog wrap the review flagged
> remained.

This, and the rest of the commit message is obtuse LLM vomit, and close
to unreadable. Please rewrite the commit message without the verbosity,
so it's actually something a human reader can comprehend. For the cake
bits in particular:

[..]

> CAKE's SPLIT_GSO path accounts the sum of the segment lengths, which a
> stab recomputes from skb->len and can exceed the pre-split
> qdisc_pkt_len(). The list is summed before any segment is linked and
> rejected as a whole before any flow state is resolved: the flow/host keys
> are computed from the unsegmented packet, but the set-associative
> resolution that commits q->tags[], the flow host indices, the way
> counters and the host bulk-flow counts runs only after the whole
> segment-sum admission test passes. A packet rejected by that test
> therefore leaves no persistent state behind. The whole list is dropped
> via to_free so the tail segment's sock_wfree() never runs under the qdisc
> lock. A fraglist GSO is the exception: skb_segment_list() returns the
> original skb as the list head carrying an extra reference, so segs ==
> skb; dropping the caller's reference and queueing the list once avoids
> self-linking *to_free, which would otherwise free the same skb repeatedly
> in the deferred teardown.

I read this paragraph multiple times, and it's still not clear to me
what it's trying to say.

Also:

>  net/sched/sch_cake.c      | 198 ++++++++++++++++++++++++++++----------

You're changing hundreds of lines of code which is partly a refactor,
partly a fix for the actual overflow issue, and partly unrelated
changes. Please split them into separate patches.

[..]

> +/* Compute the flow/host hash keys from the packet as received. Must run
> + * before skb_gso_segment(), which rewrites an encapsulated GSO packet's
> + * headers to the inner ones and would change the keys. Reads only the skb
> + * and commits no qdisc state, so it may run before admission is known.
> + */

And drop the weird LLM comments, please. If you split the series
properly they are not needed.

[..]
> -		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) {

The skb_list_walk_safe(segs, segs) thing is admittedly a bit weird, and
fixing it is fine. But that's the "separate fix" I was referring to
above, so split it out; it's just making the actual fix harder to
comprehend here.

-Toke

pw-bot: cr

Re: [PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[Jamal Hadi Salim]

On Thu, Oct 8, 2026 at 6:29 AM Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>
> > This is a follow-up to an issue found by Sashiko (nipa) during review of
> > commit d9ebd8f9aa8b ("net/sched: fq_codel: clamp default quantum and
> > mtu"), part of the quantum/mtu overflow series merged as a687f2ae995f.
> > Clamping the per-flow quantum and the CoDel mtu does not bound the
> > accounted backlog, so the per-flow backlog wrap the review flagged
> > remained.
>
> This, and the rest of the commit message is obtuse LLM vomit, and close
> to unreadable. Please rewrite the commit message without the verbosity,
> so it's actually something a human reader can comprehend. For the cake
> bits in particular:
>

Ok, but there is a dilemma: The verbosity is an attempt to appease the
sashikos. The other day it complained that i didnt have a space after
a semi colon.
The more stuff i add the less it slows me back. It picks on the
accuracy of the commit text _and changelogs_ and prescribes what i
said or should have said. I can understand a future AI review would
benefit from that; there are probably a few humans that read the
commit logs after the patch has gone in.
So:
I have been more targetting the AI more than humans in these comments.


> [..]
>
> > CAKE's SPLIT_GSO path accounts the sum of the segment lengths, which a
> > stab recomputes from skb->len and can exceed the pre-split
> > qdisc_pkt_len(). The list is summed before any segment is linked and
> > rejected as a whole before any flow state is resolved: the flow/host keys
> > are computed from the unsegmented packet, but the set-associative
> > resolution that commits q->tags[], the flow host indices, the way
> > counters and the host bulk-flow counts runs only after the whole
> > segment-sum admission test passes. A packet rejected by that test
> > therefore leaves no persistent state behind. The whole list is dropped
> > via to_free so the tail segment's sock_wfree() never runs under the qdisc
> > lock. A fraglist GSO is the exception: skb_segment_list() returns the
> > original skb as the list head carrying an extra reference, so segs ==
> > skb; dropping the caller's reference and queueing the list once avoids
> > self-linking *to_free, which would otherwise free the same skb repeatedly
> > in the deferred teardown.
>
> I read this paragraph multiple times, and it's still not clear to me
> what it's trying to say.
>

In plain english, it says three things (applies to :
1) Splitting a GSO skb can account for more bytes than the original
packet, so we sum the whole segment list and test the total against
the ceiling before any segment is linked.
2) If it is over, we drop the entire list, and because no flow state
(tags, hash indices, counters) has been resolved yet, the reject
leaves nothing behind.
3) The fraglist sentence only picks which reference to drop for the
case skb_segment_list() returns the original skb, so the same skb is
not freed twice.

This text also applies to the codel piece.
Does re-writting as above sound better? I may please you but then
sashiko would ask me to change something in the wording ;->

> Also:
>
> >  net/sched/sch_cake.c      | 198 ++++++++++++++++++++++++++++----------
>
> You're changing hundreds of lines of code which is partly a refactor,
> partly a fix for the actual overflow issue, and partly unrelated
> changes. Please split them into separate patches.
>
> [..]
>
> > +/* Compute the flow/host hash keys from the packet as received. Must run
> > + * before skb_gso_segment(), which rewrites an encapsulated GSO packet's
> > + * headers to the inner ones and would change the keys. Reads only the skb
> > + * and commits no qdisc state, so it may run before admission is known.
> > + */
>
> And drop the weird LLM comments, please. If you split the series
> properly they are not needed.
>

Like I said, it's a dilemma.

> [..]
> > -             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) {
>
> The skb_list_walk_safe(segs, segs) thing is admittedly a bit weird, and
> fixing it is fine. But that's the "separate fix" I was referring to
> above, so split it out; it's just making the actual fix harder to
> comprehend here.


We have literally have about 100 followups to review from complaints
of "re-exsting issues" by sashikos and other ai officianados (there
are at least another 10 on cake) and the hard part is sifting through
and picking what is important enough to submit. And then deal with the
fallback from the sashikos turning transforming into bike-shedders.
I am trying to avoid sending many patches. But i could split this into:

- 0001 (prep, no behavior change): extract cake_tcf_classify() and the
fq_codel_classify() rewrite.
- 0002 (prep, no behavior change): skb_list_walk_safe(segs, seg) rename.
- 0003 (fix): the enqueue ceiling guard across fq_codel / codel / pie
/ fq_pie / dualpi2 / pfifo / RED / cake, plus the
qdisc_pkt_len_segs_init() producer clamp. pfifo stays here - RED reads
a grafted child's backlog, so it is the same overflow, not a separate
bug.
- 0004 (cake restructure): the cake_hash_keys() / cake_hash_resolve()
split and the split-GSO relocation.

Would that work?

cheers,
jamal
>
> pw-bot: cr

Re: [PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[Toke Høiland-Jørgensen]

Jamal Hadi Salim <jhs@mojatatu.com> writes:

> On Thu, Oct 8, 2026 at 6:29 AM Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>>
>> > This is a follow-up to an issue found by Sashiko (nipa) during review of
>> > commit d9ebd8f9aa8b ("net/sched: fq_codel: clamp default quantum and
>> > mtu"), part of the quantum/mtu overflow series merged as a687f2ae995f.
>> > Clamping the per-flow quantum and the CoDel mtu does not bound the
>> > accounted backlog, so the per-flow backlog wrap the review flagged
>> > remained.
>>
>> This, and the rest of the commit message is obtuse LLM vomit, and close
>> to unreadable. Please rewrite the commit message without the verbosity,
>> so it's actually something a human reader can comprehend. For the cake
>> bits in particular:
>>
>
> Ok, but there is a dilemma: The verbosity is an attempt to appease the
> sashikos. The other day it complained that i didnt have a space after
> a semi colon.

You know you can push back on Sashiko comments, right? :)

> The more stuff i add the less it slows me back. It picks on the
> accuracy of the commit text _and changelogs_ and prescribes what i
> said or should have said. I can understand a future AI review would
> benefit from that; there are probably a few humans that read the
> commit logs after the patch has gone in.
> So:
> I have been more targetting the AI more than humans in these comments.

Writing to appease the bots at the expense of human readers is
absolutely a mistake, IMO. Last time I checked, we are still a community
of humans developing the kernel together, regardless of the fashionable
assistance technology du jour. Having a comprehensive and understandable
commit log is one of the kernel's greatest assets, and often one of the
only things that makes complicated code understandable. We should not be
sacrificing that in an attempt to make the work guess robot guess other
words :/

>
>> [..]
>>
>> > CAKE's SPLIT_GSO path accounts the sum of the segment lengths, which a
>> > stab recomputes from skb->len and can exceed the pre-split
>> > qdisc_pkt_len(). The list is summed before any segment is linked and
>> > rejected as a whole before any flow state is resolved: the flow/host keys
>> > are computed from the unsegmented packet, but the set-associative
>> > resolution that commits q->tags[], the flow host indices, the way
>> > counters and the host bulk-flow counts runs only after the whole
>> > segment-sum admission test passes. A packet rejected by that test
>> > therefore leaves no persistent state behind. The whole list is dropped
>> > via to_free so the tail segment's sock_wfree() never runs under the qdisc
>> > lock. A fraglist GSO is the exception: skb_segment_list() returns the
>> > original skb as the list head carrying an extra reference, so segs ==
>> > skb; dropping the caller's reference and queueing the list once avoids
>> > self-linking *to_free, which would otherwise free the same skb repeatedly
>> > in the deferred teardown.
>>
>> I read this paragraph multiple times, and it's still not clear to me
>> what it's trying to say.
>>
>
> In plain english, it says three things (applies to :
> 1) Splitting a GSO skb can account for more bytes than the original
> packet, so we sum the whole segment list and test the total against
> the ceiling before any segment is linked.
> 2) If it is over, we drop the entire list, and because no flow state
> (tags, hash indices, counters) has been resolved yet, the reject
> leaves nothing behind.
> 3) The fraglist sentence only picks which reference to drop for the
> case skb_segment_list() returns the original skb, so the same skb is
> not freed twice.
>
> This text also applies to the codel piece. Does re-writting as above
> sound better?

Yes, much better!

> I may please you but then sashiko would ask me to change something in
> the wording ;->

Well, see above :)

>> Also:
>>
>> >  net/sched/sch_cake.c      | 198 ++++++++++++++++++++++++++++----------
>>
>> You're changing hundreds of lines of code which is partly a refactor,
>> partly a fix for the actual overflow issue, and partly unrelated
>> changes. Please split them into separate patches.
>>
>> [..]
>>
>> > +/* Compute the flow/host hash keys from the packet as received. Must run
>> > + * before skb_gso_segment(), which rewrites an encapsulated GSO packet's
>> > + * headers to the inner ones and would change the keys. Reads only the skb
>> > + * and commits no qdisc state, so it may run before admission is known.
>> > + */
>>
>> And drop the weird LLM comments, please. If you split the series
>> properly they are not needed.
>>
>
> Like I said, it's a dilemma.

See above.

>> [..]
>> > -             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) {
>>
>> The skb_list_walk_safe(segs, segs) thing is admittedly a bit weird, and
>> fixing it is fine. But that's the "separate fix" I was referring to
>> above, so split it out; it's just making the actual fix harder to
>> comprehend here.
>
>
> We have literally have about 100 followups to review from complaints
> of "re-exsting issues" by sashikos and other ai officianados (there
> are at least another 10 on cake) and the hard part is sifting through
> and picking what is important enough to submit. And then deal with the
> fallback from the sashikos turning transforming into bike-shedders.
> I am trying to avoid sending many patches. But i could split this into:
>
> - 0001 (prep, no behavior change): extract cake_tcf_classify() and the
> fq_codel_classify() rewrite.
> - 0002 (prep, no behavior change): skb_list_walk_safe(segs, seg) rename.
> - 0003 (fix): the enqueue ceiling guard across fq_codel / codel / pie
> / fq_pie / dualpi2 / pfifo / RED / cake, plus the
> qdisc_pkt_len_segs_init() producer clamp. pfifo stays here - RED reads
> a grafted child's backlog, so it is the same overflow, not a separate
> bug.
> - 0004 (cake restructure): the cake_hash_keys() / cake_hash_resolve()
> split and the split-GSO relocation.
>
> Would that work?

Yeah, this seems roughly along the lines of what I was imagining :)

-Toke

Re: [PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[Stephen Hemminger]

On Thu, 08 Oct 2026 14:54:08 +0200
Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> >>  
> >> > +/* Compute the flow/host hash keys from the packet as received. Must run
> >> > + * before skb_gso_segment(), which rewrites an encapsulated GSO packet's
> >> > + * headers to the inner ones and would change the keys. Reads only the skb
> >> > + * and commits no qdisc state, so it may run before admission is known.
> >> > + */  
> >>
> >> And drop the weird LLM comments, please. If you split the series
> >> properly they are not needed.
> >>  
> >
> > Like I said, it's a dilemma.  
> 
> See above.

As heavy LLM user, I often have to tell it.
"Thats good, now rewrite the commit and comments for humans. Make the result
 conform to the style of other places in the same code"

Re: [PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[Jamal Hadi Salim]

On Thu, Oct 8, 2026 at 8:54 AM Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>
> Jamal Hadi Salim <jhs@mojatatu.com> writes:
>
> > On Thu, Oct 8, 2026 at 6:29 AM Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> >>
> >> > This is a follow-up to an issue found by Sashiko (nipa) during review of
> >> > commit d9ebd8f9aa8b ("net/sched: fq_codel: clamp default quantum and
> >> > mtu"), part of the quantum/mtu overflow series merged as a687f2ae995f.
> >> > Clamping the per-flow quantum and the CoDel mtu does not bound the
> >> > accounted backlog, so the per-flow backlog wrap the review flagged
> >> > remained.
> >>
> >> This, and the rest of the commit message is obtuse LLM vomit, and close
> >> to unreadable. Please rewrite the commit message without the verbosity,
> >> so it's actually something a human reader can comprehend. For the cake
> >> bits in particular:
> >>
> >
> > Ok, but there is a dilemma: The verbosity is an attempt to appease the
> > sashikos. The other day it complained that i didnt have a space after
> > a semi colon.
>
> You know you can push back on Sashiko comments, right? :)
>

You can never if it listens to what you say ;-> See attached
screenshot on a small exchange where i pushed back. I did fix the
double space on the next one of course ;->



The human behind it is trying to teach it some bad habits ;->

> > The more stuff i add the less it slows me back. It picks on the
> > accuracy of the commit text _and changelogs_ and prescribes what i
> > said or should have said. I can understand a future AI review would
> > benefit from that; there are probably a few humans that read the
> > commit logs after the patch has gone in.
> > So:
> > I have been more targetting the AI more than humans in these comments.
>
> Writing to appease the bots at the expense of human readers is
> absolutely a mistake, IMO. Last time I checked, we are still a community
> of humans developing the kernel together, regardless of the fashionable
> assistance technology du jour. Having a comprehensive and understandable
> commit log is one of the kernel's greatest assets, and often one of the
> only things that makes complicated code understandable. We should not be
> sacrificing that in an attempt to make the work guess robot guess other
> words :/

I dont know ;-> Let's discuss in a year.

[..]

> > In plain english, it says three things (applies to :
> > 1) Splitting a GSO skb can account for more bytes than the original
> > packet, so we sum the whole segment list and test the total against
> > the ceiling before any segment is linked.
> > 2) If it is over, we drop the entire list, and because no flow state
> > (tags, hash indices, counters) has been resolved yet, the reject
> > leaves nothing behind.
> > 3) The fraglist sentence only picks which reference to drop for the
> > case skb_segment_list() returns the original skb, so the same skb is
> > not freed twice.
> >
> > This text also applies to the codel piece. Does re-writting as above
> > sound better?
>
> Yes, much better!
>


[..]

> > We have literally have about 100 followups to review from complaints
> > of "re-exsting issues" by sashikos and other ai officianados (there
> > are at least another 10 on cake) and the hard part is sifting through
> > and picking what is important enough to submit. And then deal with the
> > fallback from the sashikos turning transforming into bike-shedders.
> > I am trying to avoid sending many patches. But i could split this into:
> >
> > - 0001 (prep, no behavior change): extract cake_tcf_classify() and the
> > fq_codel_classify() rewrite.
> > - 0002 (prep, no behavior change): skb_list_walk_safe(segs, seg) rename.
> > - 0003 (fix): the enqueue ceiling guard across fq_codel / codel / pie
> > / fq_pie / dualpi2 / pfifo / RED / cake, plus the
> > qdisc_pkt_len_segs_init() producer clamp. pfifo stays here - RED reads
> > a grafted child's backlog, so it is the same overflow, not a separate
> > bug.
> > - 0004 (cake restructure): the cake_hash_keys() / cake_hash_resolve()
> > split and the split-GSO relocation.
> >
> > Would that work?
>
> Yeah, this seems roughly along the lines of what I was imagining :)
>

I will wait for sashiko to run - it does find good issues and will
resend with this and the one above in mind.

cheers,
jamal

> -Toke

Re: [PATCH net-next v5] net/sched: cap the accounted backlog before it can wrap

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 2 · Low: 4

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] The ceiling does not hold for stabless non-GSO packets, so the
  wrap and the fq_codel_drop() NULL dereference are still reachable.
- [Medium] A duplicating netem child lets RED's new backlog guard be
  bypassed.
- [Medium] sch_plug is a sibling qdisc whose accounted backlog can still
  wrap, and the patch does not cover it.
- [Low] The patch has no Fixes: tag (and no stable Cc), although it fixes
  a user-visible defect reachable from a user namespace: wrapped AQM…
- [Low] The kernel-doc for QDISC_DROP_OVERLIMIT (include/net/dropreason-
  qdisc.h) defines it as 'the qdisc queue length exceeded its configured…
- [Low] The new header comment on fq_codel_classify() says it returns
  true only when 'a terminal TC action consumed the packet, or the…
- [Low] When the new cake_enqueue() SPLIT_GSO segment-sum check rejects a
  packet, the segment list goes onto *to_free via __qdisc_drop_all(segs,…

--

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

This patch adds qdisc_backlog_at_max(), which drops packets at enqueue
once the stored 32-bit backlog is within QDISC_PKT_LEN_MAX of U32_MAX.
The guard is applied to fq_codel, cake, codel, pie, fq_pie, dualpi2,
pfifo and red. The GSO pkt_len producer in qdisc_pkt_len_segs_init() is
also clamped, and the fq_codel and cake classify/hash paths are
reordered so that rejected packets do not change flow state.

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/CANn89i+99jPh7JjZf=G3Ouaom2tuOtAEiHUf2MEsRdVPd=nz4w@mail.gmail.com/
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-BA27.v3.20260929142417%40mojatatu.com
Link: https://lore.kernel.org/netdev/20261006153043.GM83879@horms.kernel.org/

> The bound is exact for
> every producer: a stab-clamped length is capped by QDISC_PKT_LEN_MAX, and
> the stabless GSO producer is capped by qdisc_pkt_len_segs_init() (the
> SPLIT_GSO path additionally re-checks its segment sum). This follows the
> existing bfifo approach, which bounds bytes against the accounted
> length; gred is a precedent only in WRED mode, where it bounds the
> aggregate, while non-WRED gred bounds each virtual queue.

[Severity: Medium]
Should sch_plug get the same guard? It also makes admission decisions
on the accounted backlog, but it isn't covered by this patch.
plug_enqueue() checks skb->len, while qdisc_enqueue_tail() adds
qdisc_pkt_len():

net/sched/sch_plug.c:plug_enqueue() {
	if (likely((u64)sch->qstats.backlog + skb->len <= q->limit)) {
		if (!q->unplug_indefinite)
			q->pkts_current_epoch++;
		return qdisc_enqueue_tail(skb, sch);
	}
}

The user supplies q->limit as a u32, through plug_init() or
TCQ_PLUG_LIMIT. Take limit = U32_MAX and a 1 MiB stab. A 100 byte skb is
admitted at a backlog of 4095 MiB, because 4293918720 + 100 <= U32_MAX.
The backlog then reaches 2^32 and wraps to 0. While the qdisc is
plugged, the byte limit no longer applies.

Could plug_enqueue() use qdisc_backlog_at_max(), or compare against
qdisc_pkt_len()?

Non-WRED gred has a similar gap. Only each virtual queue is bounded, so
VQ limits that add up to more than 4 GiB can wrap the aggregate
sch->qstats.backlog. The pass-through branch in gred_enqueue() then
admits against the wrapped value:

	if (likely((u64)sch->qstats.backlog + qdisc_pkt_len(skb) <=
				sch->limit))
		return qdisc_enqueue_tail(skb, sch);

> Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/20260818101130.16203-1-jhs@mojatatu.com/

[ ... ]

> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

[Severity: Low]
Should this carry a Fixes: tag, and possibly a stable Cc? It fixes a
defect that users can reach from a user namespace: wrapped AQM backlogs
and the fq_codel_drop() NULL dereference described above.

The patch also depends on commit 8f735d64382d ("net/sched: bound
qdisc_pkt_len to prevent qdisc soft lockup"). That commit added this
clamp in __qdisc_calculate_pkt_len(), plus the QDISC_PKT_LEN_MAX define
that this patch moves:

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

8f735d64382d carries Fixes: 1da177e4c3f4 and will go to stable. The
QDISC_MAX_BACKLOG headroom argument only holds when it is applied.

Could the commit message name this dependency for backporters?

> diff --git a/net/core/dev.c b/net/core/dev.c
> index f587645e930a1..e6ac94c61ea07 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4268,6 +4268,14 @@ static enum skb_drop_reason qdisc_pkt_len_segs_init(struct sk_buff *skb)
>  		qdisc_skb_cb(skb)->pkt_segs = gso_segs;
>  	}
>  	qdisc_skb_cb(skb)->pkt_len += (gso_segs - 1) * hdr_len;
> +	/* (gso_segs - 1) * hdr_len can exceed QDISC_PKT_LEN_MAX for a dodgy
> +	 * GSO skb, so clamp the producer the same way a size table is
> +	 * clamped (__qdisc_calculate_pkt_len()). The accounted backlog stays
> +	 * within one stab-clamped packet of U32_MAX and cannot wrap.
> +	 */
> +	qdisc_skb_cb(skb)->pkt_len = min_t(unsigned int,
> +					   qdisc_skb_cb(skb)->pkt_len,
> +					   QDISC_PKT_LEN_MAX);
>  	return SKB_NOT_DROPPED_YET;
>  }

[Severity: High]
Does this clamp cover every exit of qdisc_pkt_len_segs_init()? The
function returns early for non-GSO skbs, and for GSO skbs that have no
transport header. In both cases pkt_len is left at skb->len:

	qdisc_skb_cb(skb)->pkt_len = skb->len;
	if (!shinfo->gso_size) {
		qdisc_skb_cb(skb)->pkt_segs = 1;
		return SKB_NOT_DROPPED_YET;
	}
	...
		if (unlikely(!skb_transport_header_was_set(skb)))
			return SKB_NOT_DROPPED_YET;

If the root has no size table, __qdisc_calculate_pkt_len() never runs.
A non-GSO skb larger than 1 MiB is then accounted at its full length.
qdisc_backlog_at_max() reserves only QDISC_PKT_LEN_MAX of headroom and
does not look at the incoming length.

Such skbs look reachable:

- tun_alloc_skb() enlarges the linear part for large writes instead of
  rejecting them.
- act_mirred egress hands them to dev_queue_xmit() without an MTU check.
- loopback sets no max_mtu.

Here is a possible sequence with fq_codel flows >= 2 and limit 4094:

  1. With a 1 MiB stab, queue 4094 small packets into flow 1 and leave
     flow 0 empty. The backlog is 4292870144, which is <=
     QDISC_MAX_BACKLOG (4293918719).

  2. Run tc qdisc change without TCA_STAB. qdisc_change() does:

	rcu_assign_pointer(sch->stab, stab);

     with stab == NULL. The queued packets keep their accounted lengths.

  3. Redirect a 2 MiB non-GSO frame into flow 1. It passes
     qdisc_backlog_at_max(). sch->qstats.backlog and q->backlogs[1] both
     become 4292870144 + 2097152 = 2^32, which is 0.

  4. qlen 4095 is above the limit, so fq_codel_drop() runs. Its strict
     '>' scan keeps idx 0, and dequeue_head(&q->flows[0]) dereferences a
     NULL flow->head.

If this sequence works, the commit message statement "The bound is
exact for every producer" would not hold for stabless non-GSO packets.

Could the clamp be applied on every return path of
qdisc_pkt_len_segs_init(), or after qdisc_calculate_pkt_len()?

> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
> index dc93267029e70..6484c4fa0ccc6 100644
> --- a/net/sched/sch_cake.c
> +++ b/net/sched/sch_cake.c

[ ... ]

> @@ -1758,22 +1797,87 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,

[ ... ]

> +	if (qdisc_pkt_segs(skb) > 1 && q->config->rate_flags & CAKE_FLAG_SPLIT_GSO) {
> +		struct sk_buff *nskb, *seg;
> +		netdev_features_t features = netif_skb_features(skb);
> +
> +		segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
> +		if (IS_ERR_OR_NULL(segs))
> +			return qdisc_drop(skb, sch, to_free);

[ ... ]

> +		if (unlikely((u64)sch->qstats.backlog + slen > QDISC_MAX_BACKLOG)) {
> +			qdisc_qstats_overlimit(sch);

[ ... ]

> +			if (unlikely(segs == skb)) {
> +				tcf_set_qdisc_drop_reason(skb,
> +							  QDISC_DROP_OVERLIMIT);
> +				consume_skb(skb);
> +				return qdisc_drop_all(segs, sch, to_free);
> +			}
> +			__qdisc_drop_all(segs, to_free);
> +			return qdisc_drop_reason(skb, sch, to_free,
> +						 QDISC_DROP_OVERLIMIT);
> +		}
> +	}

[Severity: Low]
What drop reason do the rejected segments carry when they are freed?
They go onto *to_free without one being set. Only skb, or the head in
the fraglist case, gets QDISC_DROP_OVERLIMIT.

tc_skb_cb()->drop_reason is at cb offset 32. That is the same word as
SKB_GSO_CB(skb)->mac_offset. __skb_gso_segment() writes it before the
segments copy the cb:

	SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb);

__tcf_kfree_skb_list() later reads it back:

	u32 reason = tc_skb_cb(skb)->drop_reason;

The qdisc subsystem bits are clear, so the headroom value goes to
kfree_skb_reason() as an unrelated core drop reason, and
trace_qdisc_drop is skipped. A headroom of 0 or 1 would be read as
SKB_NOT_DROPPED_YET or SKB_CONSUMED.

Should each segment get tcf_set_qdisc_drop_reason(seg,
QDISC_DROP_OVERLIMIT) before it is deferred?

> diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
> index a1ba2e355f2d1..ff7e72989dfc1 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_at_max(sch))) {
> +		WRITE_ONCE(q->drop_overlimit, q->drop_overlimit + 1);
> +		return qdisc_drop_reason(skb, sch, to_free,
> +					 QDISC_DROP_OVERLIMIT);
>  	}

[Severity: Low]
Does QDISC_DROP_OVERLIMIT still describe this drop? Its kernel-doc in
include/net/dropreason-qdisc.h says:

	 * @QDISC_DROP_OVERLIMIT: packet dropped because the qdisc queue
	 * length exceeded its configured limit (sch->limit). This typically
	 * indicates the queue is full and cannot accept more packets.

This patch also uses it for the byte backlog crossing QDISC_MAX_BACKLOG:

- codel_qdisc_enqueue(), where the qlen and ceiling cases can no longer
  be told apart.
- fq_codel_enqueue().
- cake_enqueue(), both the top-level check and the SPLIT_GSO segment-sum
  rejection. cake has no sch->limit packet check at all.
- red_enqueue().

These drops can happen with qlen far below sch->limit. For example,
fq_codel with limit 20000 drops at about 4096 packets. Drop monitor and
tracepoint users would be pointed at sch->limit, and raising it does not
help.

Existing usage is already a little loose here, since the dualpi2
memory_limit drops and the fq_codel memory-limited drops also report
this reason. Could the kernel-doc be updated, or a separate reason used?

[ ... ]

> diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
> index e6c87a32950f9..7c52acf201c63 100644
> --- a/net/sched/sch_fq_codel.c
> +++ b/net/sched/sch_fq_codel.c
> @@ -73,22 +73,34 @@ static unsigned int fq_codel_hash(const struct fq_codel_sched_data *q,
>  	return reciprocal_scale(skb_get_hash(skb), q->flows_cnt);
>  }
>  
> -static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
> -				      int *qerr)
> +/* Run the optional external classifier. Returns true when the caller must
> + * drop (a terminal TC action consumed the packet, or the result carried no
> + * class ID); the reason is then in *qerr. Else *idx is the 1-based flow
> + * index, or *do_hash asks the caller to use fq_codel_hash().
> + */

[Severity: Low]
Is this comment complete? The body also returns true in two other
cases:

- No filter matched (tcf_classify_qdisc() < 0).
- The minor is present but greater than q->flows_cnt.

		if (TC_H_MIN(res.classid) > 0 &&
		    TC_H_MIN(res.classid) <= q->flows_cnt) {
			*idx = TC_H_MIN(res.classid);
			return false;
		}
	}
	return true;

TC_ACT_SHOT is also a drop with __NET_XMIT_BYPASS, not a consumed
packet, and the cake_tcf_classify() comment uses the same wording.

The two new helpers handle no-match in opposite ways: cake hashes the
packet and fq_codel drops it. That matches the pre-patch behaviour, but
only the cake comment says so. Could the fq_codel comment list all the
drop cases?

[ ... ]

> diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
> index d7598214270b8..e3a254f9dbdf9 100644
> --- a/net/sched/sch_red.c
> +++ b/net/sched/sch_red.c
> @@ -76,6 +76,12 @@ static int red_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>  	unsigned int len;
>  	int ret;
>  
> +	if (unlikely(qdisc_backlog_at_max(sch))) {
> +		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);

[Severity: Medium]
Can a duplicating netem child get around this check? red_enqueue()
tests the backlog once and then calls qdisc_enqueue() on the child.
netem_enqueue() re-enters the root with the duplicate before it accounts
the original:

net/sched/sch_netem.c:netem_enqueue() {
	if (skb2) {
		struct Qdisc *rootq = qdisc_root_bh(sch);

		skb2->tc_depth++; /* prevent duplicating a dup... */
		rootq->enqueue(skb2, rootq, to_free);
		skb2 = NULL;
	}

	qdisc_qstats_backlog_inc(sch, skb);
}

The nested red_enqueue() sees the same RED backlog that the outer call
checked, so one check admits two packets. The outer call then does
qstats_backlog_add(sch, len) without checking again.

Take a 1 MiB stab, RED in ecn nodrop mode, and a netem child with
duplicate 100% and a large limit. After 2047 inputs the backlog is
4094 MiB, which is still <= QDISC_MAX_BACKLOG. The 2048th input adds two
1 MiB packets, and the RED and netem qstats.backlog both reach 2^32,
which is 0. red_calc_qavg() then reads a wrapped child backlog.

The commit message names child GSO segmentation through
qdisc_tree_reduce_backlog() as a known RED gap, but not this duplication
path. Is this case meant to be covered here, or in the separate change?

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