[Cake] [PATCH net-next v2 0/4] net/sched: sch_cake: fixes for accounting and reconfiguration

[Cake] [PATCH net-next v2 0/4] net/sched: sch_cake: fixes for accounting and reconfiguration

[Jamal Hadi Salim]

Four independent fixes in the CAKE qdisc.  Three are follow-ups to the
autorate reconfiguration work around commit 7cbfb180945c ("net/sched:
sch_cake: fix autorate reconfiguration throttling") and the 32-bit
admission-counter wraps fixed in gred/bfifo/plug by commit 4c660ee8c809
("net: sched: fix 32-bit backlog wrap in gred, bfifo and plug enqueue");
the fourth is a dead-code cleanup found while auditing the dump path.

Patch 1 serializes a 'tc qdisc change' with the datapath's autorate
update under the qdisc lock, so the netlink writer and the autorate
writer can no longer lose the configured rate.  rate_bps needs no atomic
(it is only loaded and stored), so it stays a plain u64 with
READ_ONCE()/WRITE_ONCE() annotations.

Patch 2 propagates the backlog reduction of the reconfigure-time tin
purge to classful parents, so a CAKE leaf under HTB no longer leaves the
purged packets in the parent's qlen/backlog forever.

Patch 3 widens buffer_used and buffer_max_used to u64.  The u32
accumulator wraps once the resident truesize passes 2^32, bypassing the
over-limit drop loop while the configured limit is a u32 attribute; the
u32 MEMORY_USED export is clamped so widening the counter does not
truncate the reported value.

Patch 4 drops an unreachable 'nla_nest_end() < 0' error check: the
helper never returns a negative value.

All four are hardening for net-next (no Fixes:, no Cc: stable): the
counter wrap, the missing parent-backlog propagation and the
reconfiguration race all predate any contract we could anchor a
regression on.  Patches 1-2 were reported by Sashiko (gemini) and patch
3 comes from the gred 32-bit-wrap family; patch 4 is a static cleanup.

v1 -> v2:
 - Patch 1/4: drop atomic64_t; rate_bps stays a plain u64 with
   READ_ONCE()/WRITE_ONCE() annotations (per Toke's review: the accesses
   are plain loads/stores, so no atomic is needed).
 - Patch 2/4: no change; carries Toke's v1 Acked-by.
 - Patch 3/4: clamp the MEMORY_USED export to U32_MAX, so widening the
   counter does not truncate the u32 stat (Sashiko nipa Medium).
 - Patch 4/4: no change; carries Toke's v1 Acked-by.

Jamal Hadi Salim (4):
  net/sched: sch_cake: serialize reconfiguration with the datapath
  net/sched: sch_cake: reduce parent backlog when clearing a tin
  net/sched: sch_cake: widen buffer_used to u64
  net/sched: sch_cake: drop dead nla_nest_end() error check

 net/sched/sch_cake.c | 52 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 17 deletions(-)

-- 
2.43.0

[Cake] [PATCH net-next v2 1/4] net/sched/sch_cake: serialize reconfiguration with the datapath

[Jamal Hadi Salim]

This is a follow-up to commit 7cbfb180945c ("net/sched: sch_cake: fix
autorate reconfiguration throttling").  That change only stored
last_reconfig_time; it did not address the concurrency between the
autorate rate update in cake_enqueue() and the netlink reconfiguration
path, which the original review flagged.

The issue:

cake_change() -> cake_config_change() updates the fields of the shared
struct cake_sched_config under RTNL but not under the qdisc lock: it only
took sch_tree_lock() later, around cake_reconfigure().  The datapath in
turn serializes cake_enqueue() with itself under the qdisc lock, and its
autorate-ingress path stores the estimated rate into
q->config->rate_bps.  Two writers therefore update the same rate with no
common lock:

  CPU0: cake_enqueue() sees CAKE_FLAG_AUTORATE_INGRESS, computes an
        estimate, and is about to store it.
  CPU1: 'tc qdisc change ... bandwidth X' clears autorate in its local
        rate_flags, writes rate_bps = X, publishes the cleared flag, and
        waits in sch_tree_lock().
  CPU0: stores the estimate, calls cake_reconfigure(), and unlocks.
  CPU1: reconfigures from the estimate and returns.

The qdisc is left with autorate disabled but with the estimate installed
instead of X.

The fix:

Commit the parsed configuration and reconfigure while holding the same
qdisc lock that serializes the datapath, so the netlink writer and the
autorate writer can no longer interleave.  rate_bps needs no atomic: it is
only ever loaded and stored, so plain READ_ONCE()/WRITE_ONCE() annotations
cover the readers that run outside the lock (cake_config_dump(), which is
not serialized, and the cake_mq shared config, observed under different
child locks).

Conditions to recreate the bug: build with CONFIG_KCSAN=y.  Two distinct
effects are reachable.  (a) The lost update of the installed rate is
reproducible without a sanitizer: put cake in autorate-ingress on a
device, drive bursty traffic with gaps so the 250ms autorate
reconfiguration fires, and concurrently loop 'tc qdisc change dev ...
cake bandwidth <N>kbit autorate-ingress', then read the installed rate
back and observe the estimate instead of the configured rate.  (b) The
non-atomic 64-bit store that KCSAN reports needs a 32-bit machine, where
the torn store of rate_bps is observable; on 64-bit the value is written
atomically and KCSAN flags only the unsynchronized access.

Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260816012109.2865223-1-ooonea@gmail.com
Link: https://lore.kernel.org/netdev/20260816012109.2865223-1-ooonea@gmail.com/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_cake.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index dc93267029e7..a05da73b5f84 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1906,7 +1906,8 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			if (ktime_after(now,
 					ktime_add_ms(q->last_reconfig_time,
 						     250))) {
-				q->config->rate_bps = (q->avg_peak_bandwidth * 15) >> 4;
+				WRITE_ONCE(q->config->rate_bps,
+					   (q->avg_peak_bandwidth * 15) >> 4);
 				q->last_reconfig_time = now;
 				cake_reconfigure(sch);
 			}
@@ -2021,7 +2022,7 @@ static struct sk_buff *cake_dequeue(struct Qdisc *sch)
 	    now - q->last_checked_active >= q->config->sync_time) {
 		struct net_device *dev = qdisc_dev(sch);
 		struct cake_sched_data *other_priv;
-		u64 new_rate = q->config->rate_bps;
+		u64 new_rate = READ_ONCE(q->config->rate_bps);
 		u64 other_qlen, other_last_active;
 		struct Qdisc *other_sch;
 		u32 num_active_qs = 1;
@@ -2042,7 +2043,8 @@ static struct sk_buff *cake_dequeue(struct Qdisc *sch)
 		}
 
 		if (num_active_qs > 1)
-			new_rate = div64_u64(q->config->rate_bps, num_active_qs);
+			new_rate = div64_u64(READ_ONCE(q->config->rate_bps),
+					     num_active_qs);
 
 		cake_configure_rates(sch, new_rate, true);
 		q->last_checked_active = now;
@@ -2626,12 +2628,12 @@ static void cake_reconfigure(struct Qdisc *sch)
 	struct cake_sched_config *q = qd->config;
 	u32 buffer_limit;
 
-	cake_configure_rates(sch, qd->config->rate_bps, false);
+	cake_configure_rates(sch, READ_ONCE(qd->config->rate_bps), false);
 
 	if (q->buffer_config_limit) {
 		buffer_limit = q->buffer_config_limit;
-	} else if (q->rate_bps) {
-		u64 t = q->rate_bps * q->interval;
+	} else if (READ_ONCE(q->rate_bps)) {
+		u64 t = READ_ONCE(q->rate_bps) * q->interval;
 
 		do_div(t, USEC_PER_SEC / 4);
 		buffer_limit = max_t(u32, t, 4U << 20);
@@ -2784,9 +2786,16 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
 		return -EOPNOTSUPP;
 	}
 
+	/* Once the qdisc is live, commit and reconfigure under the lock that
+	 * serializes the datapath, so a concurrent 'tc qdisc change' and the
+	 * autorate update cannot lose the configured rate.
+	 */
+	if (qd->tins)
+		sch_tree_lock(sch);
+
 	ret = cake_config_change(q, opt, extack, &overhead_changed);
 	if (ret)
-		return ret;
+		goto unlock;
 
 	if (overhead_changed) {
 		WRITE_ONCE(qd->max_netlen, 0);
@@ -2795,13 +2804,13 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
 		WRITE_ONCE(qd->min_adjlen, ~0);
 	}
 
-	if (qd->tins) {
-		sch_tree_lock(sch);
+	if (qd->tins)
 		cake_reconfigure(sch);
+unlock:
+	if (qd->tins)
 		sch_tree_unlock(sch);
-	}
 
-	return 0;
+	return ret;
 }
 
 static void cake_destroy(struct Qdisc *sch)

[Cake] [PATCH net-next v2 2/4] net/sched/sch_cake: reduce parent backlog when clearing a tin

[Jamal Hadi Salim]

The issue:
CAKE has purged unused tins on DiffServ reconfiguration since commit
83f8fd69af4f ("sch_cake: Add DiffServ handling") without propagating the
resulting backlog reduction to classful parents. The issue was noticed
while reviewing 7cbfb180945c, but predates that change.

cake_clear_tin() drops every packet from a tin with
kfree_skb_reason() but never calls qdisc_tree_reduce_backlog(), so a
CAKE qdisc under a classful parent (e.g. HTB) leaves the dropped packets
in the parent qdisc's and class's qlen/backlog accounting forever. The
normal dequeue path does reduce the parent; only the live reconfigure
purge misses it.

The fix:
Tins are cleared from cake_reset() and, on reconfigure, from
cake_configure_rates(). The qdisc core already owns the parent delta for
the reset path: qdisc_purge_queue() snapshots qlen/backlog, runs
qdisc_reset(), then reduces the tree, so propagating the reduction from
cake_clear_tin() itself would subtract the same backlog twice. Snapshot
CAKE's qlen/backlog around only the reconfiguration purge in
cake_configure_rates() and reduce the parent once afterwards, matching
the delta pattern already used by cake_enqueue().

Conditions to recreate the bug:
put CAKE (diffserv8) under an HTB class, flood DSCP-classified traffic
into a high tin so it backlogs, then change CAKE to diffserv3. The HTB
parent keeps the purged packets in its qlen and backlog; a kernel with
the fix shows the parent at the leaf's backlog (426b/5p) instead of
retaining the purged packets.

Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260816012109.2865223-1-ooonea@gmail.com
Link: https://lore.kernel.org/netdev/20260816012109.2865223-1-ooonea@gmail.com/
Link: https://lore.kernel.org/netdev/8733ugtry8.fsf@toke.dk/
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>
---
v1 -> v2:
- No changes to this patch. Resent as part of the
  series_cake_clear_tin_backlog_underflow v2, which revises patches 1/4
  and 3/4. It is unchanged and carries Toke Høiland-Jørgensen's Acked-by
  from the v1 posting.

 net/sched/sch_cake.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 1c29695dc928..2bad0b6eb13f 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -2611,10 +2611,17 @@ static void cake_configure_rates(struct Qdisc *sch, u64 rate, bool rate_adjust)
 	}
 
 	if (!rate_adjust) {
+		u32 backlog = sch->qstats.backlog, qlen = sch->q.qlen;
+
 		for (c = qd->tin_cnt; c < CAKE_MAX_TINS; c++) {
 			cake_clear_tin(sch, c);
 			qd->tins[c].cparams.mtu_time = qd->tins[ft].cparams.mtu_time;
 		}
+
+		backlog -= sch->qstats.backlog;
+		qlen -= sch->q.qlen;
+		if (qlen)
+			qdisc_tree_reduce_backlog(sch, qlen, backlog);
 	}
 
 	qd->rate_ns   = qd->tins[ft].tin_rate_ns;
-- 
2.43.0

[Cake] [PATCH net-next v2 3/4] net/sched/sch_cake: widen buffer_used to u64

[Jamal Hadi Salim]

This is a follow-up to commit 4c660ee8c809 ("net: sched: fix 32-bit
backlog wrap in gred, bfifo and plug enqueue"), which promoted the
backlog + length admission sums in gred, bfifo and plug to u64.  CAKE's
own memory accounting has the same 32-bit wrap.

The issue:

cake_sched_data.buffer_used accumulates skb->truesize and is compared
against memory_limit:

	q->buffer_used += skb->truesize;
	if (q->buffer_used <= q->buffer_limit)
		return NET_XMIT_SUCCESS;
	while (q->buffer_used > q->buffer_limit)
		drop...

buffer_used is u32, so once the resident truesize passes 2^32 it wraps to
a small value, the <= test passes and the drop loop is skipped even
though far more than the configured limit is queued.  memory_limit is a
u32 attribute (TCA_CAKE_MEMORY), so buffer_limit can be as large as
U32_MAX and never caps buffer_used below the wrap point; with a large
memory_limit the queue then grows until the machine is out of memory.

The fix:

Widen buffer_used and buffer_max_used to u64, the same shape as the
gred/bfifo/plug fix.  buffer_limit stays u32; the comparison promotes it
to u64, so the bounded queue stops at the configured limit below 2^32 and
the counter can no longer wrap.  buffer_used is subtracted on dequeue and
drop, so the widened type flows through unchanged.  The ACK-filter
replacement applied the net delta as one expression,
q->buffer_used += skb->truesize - ack->truesize.  Both operands are
unsigned int, so the subtraction is evaluated at u32 and a larger removed
ACK wraps the delta; the old u32 accumulator folded that back to the
correct net value, but the widened u64 accumulator would persist it as a
roughly 4 GiB over-count.  Apply the replacement as two exact operations
on the widened counter instead; the queue already accounts ack->truesize,
so the subtraction cannot underflow.

Widening the counters alone is incomplete because cake_dump_stats() still
exports buffer_max_used through PUT_STAT_U32(MEMORY_USED, ...), and
tc.yaml declares memory-used as u32.  buffer_max_used only ever rises, so
once the widened counter passes 2^32 the export reports peak - 2^32 for
the rest of the qdisc's life.  Clamp the exported value to U32_MAX; the
counter itself is unaffected.

Conditions to recreate the bug: CAP_NET_ADMIN (root or a user namespace,
the qdisc attach is gated by netlink_net_capable(CAP_NET_ADMIN)).

  # hold a tun device's tx ring so the root qdisc parks packets
  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: cake memlimit 4294967295

  # drive >4 GiB of resident truesize through the qdisc (e.g. several UDP
  # sockets with SO_SNDBUFFORCE). On the unfixed kernel buffer_used wraps,
  # the drop loop is bypassed and the guest OOMs; with the fix the counter
  # passes the limit without wrapping and drops the excess.

  # the ACK-filter over-count is reached with ack-filter enabled and a
  # queued pure ACK whose truesize is larger than the replacement ACK:
  tc qdisc add dev tun0 root handle 1: cake ack-filter

Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260818095927.15901-1-jhs@mojatatu.com
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260818095927.15901-1-jhs@mojatatu.com
Link: https://lore.kernel.org/netdev/20260818095927.15901-1-jhs@mojatatu.com/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_cake.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index dc93267029e7..79a07fa80b35 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -234,8 +234,9 @@ struct cake_sched_data {
 	u16		tin_cnt;
 
 	/* resource tracking */
-	u32		buffer_used;
-	u32		buffer_max_used;
+
+	u64		buffer_max_used;
+	u64		buffer_used;
 	u32		buffer_limit;
 
 	/* indices for dequeue */
@@ -1850,7 +1851,8 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			qdisc_qstats_drop(sch);
 			ack_pkt_len = qdisc_pkt_len(ack);
 			WRITE_ONCE(b->bytes, b->bytes + ack_pkt_len);
-			q->buffer_used += skb->truesize - ack->truesize;
+			q->buffer_used += skb->truesize;
+			q->buffer_used -= ack->truesize;
 			if (q->config->rate_flags & CAKE_FLAG_INGRESS)
 				cake_advance_shaper(q, b, ack, now, true);
 
@@ -3012,7 +3014,8 @@ static int cake_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
 
 	PUT_STAT_U64(CAPACITY_ESTIMATE64, READ_ONCE(q->avg_peak_bandwidth));
 	PUT_STAT_U32(MEMORY_LIMIT, READ_ONCE(q->buffer_limit));
-	PUT_STAT_U32(MEMORY_USED, READ_ONCE(q->buffer_max_used));
+	PUT_STAT_U32(MEMORY_USED,
+		     min_t(u64, READ_ONCE(q->buffer_max_used), U32_MAX));
 	PUT_STAT_U32(AVG_NETOFF, ((READ_ONCE(q->avg_netoff) + 0x8000) >> 16));
 	PUT_STAT_U32(MAX_NETLEN, READ_ONCE(q->max_netlen));
 	PUT_STAT_U32(MAX_ADJLEN, READ_ONCE(q->max_adjlen));

[Cake] [PATCH net-next v2 4/4] net/sched/sch_cake: drop dead nla_nest_end() error check

[Jamal Hadi Salim]

cake_dump_class_stats() closes the TCA_STATS_APP nest with

	if (nla_nest_end(d->skb, stats) < 0)
		return -1;

nla_nest_end() never returns a negative value: it has a single
`return skb->len;` and no error path (that is why the overflow-aware
nla_nest_end_safe() helper was later added). The '< 0' test can never
be true, so the 'return -1' is unreachable dead code.

Drop the check and close the nest with the bare statement, matching the
sibling cake nests in the same file.

This issue was discovered while fixing a bug on the gate action, see:
https://lore.kernel.org/netdev/QDISC-H19Z.v1.20261001053234@mojatatu.com/

Conditions to recreate the bug:
- not runtime observable: the issue is found by inspection.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Link: https://lore.kernel.org/netdev/87v77csd5q.fsf@toke.dk/
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v1 -> v2:
 - No changes to this patch. Re-sent as part of the
   series_cake_clear_tin_backlog_underflow v2; it keeps Toke's v1
   Acked-by, carried forward.
 - Subject version tag bumped to v2; Message-Id re-baked.

 net/sched/sch_cake.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 4caf9718c9bd..124f552023f8 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -3211,8 +3211,7 @@ static int cake_dump_class_stats(struct Qdisc *sch, unsigned long cl,
 						       READ_ONCE(flow->cvars.drop_next))));
 		}
 
-		if (nla_nest_end(d->skb, stats) < 0)
-			return -1;
+		nla_nest_end(d->skb, stats);
 	}
 
 	return 0;
-- 
2.43.0