From d23a16d907dc991614eb301830bff8e5b713fb56 Mon Sep 17 00:00:00 2001 From: Jamal Hadi Salim Date: Tue, 20 Oct 2026 07:00:05 -0400 Subject: [PATCH 1/4] net/sched/sch_cake: serialize reconfiguration with the datapath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 are enough for the accesses that run outside the lock. The lockless paths read the rate once into a local and reuse that value, so a single coherent snapshot is used per operation. The annotations do not make a 64-bit access single-copy atomic on 32-bit: READ_ONCE()/WRITE_ONCE() are still two 32-bit accesses there. The lockless readers -- cake_config_dump() and the cake_mq children, which observe the shared config under a different child's lock -- can therefore still see a torn rate on 32-bit. That rate is an advisory shaper value and is accepted as best-effort; the state that must be consistent (the configured rate and the autorate flag) is written under the qdisc lock. Conditions to recreate the bug: The lost update (a) needs no sanitizer. Put cake in autorate-ingress on a device, drive bursty traffic with gaps so the 250 ms autorate reconfiguration fires, and concurrently loop 'tc qdisc change ... cake bandwidth kbit' without autorate-ingress (specifying a bandwidth clears the flag). If the autorate store lands after the change wrote its rate but before the change reconfigures, the qdisc ends up with autorate disabled and the estimate installed instead of ; reading the rate back then shows the estimate, which an unfixed kernel can do and a fixed one cannot. (b) The unsynchronized access is reported by KCSAN: build with CONFIG_KCSAN=y and run the same trigger with the change loop keeping autorate-ingress set; KCSAN reports a data race in cake_config_change / cake_enqueue. The non-atomic 64-bit store only shows up as tearing on a 32-bit machine; on 64-bit the store is atomic and KCSAN flags the access discipline instead. Reported-by: Sashiko (gemini) 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/875wzctsky.fsf@toke.dk/ Link: https://lore.kernel.org/netdev/87ld7t3kl3.fsf@all.your.base.are.belong.to.us/ Reviewed-by: Victor Nogueira Signed-off-by: Jamal Hadi Salim Acked-by: Toke Høiland-Jørgensen Link: https://patch.msgid.link/QDISC-5RTM.v3.r0.20261020053711-1@mojatatu.com Signed-off-by: Jakub Kicinski --- net/sched/sch_cake.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index dc93267029e7..d71617aa56d9 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); } @@ -2019,9 +2020,9 @@ static struct sk_buff *cake_dequeue(struct Qdisc *sch) if (q->config->is_shared && q->rate_ns && now - q->last_checked_active >= q->config->sync_time) { + u64 new_rate = READ_ONCE(q->config->rate_bps); struct net_device *dev = qdisc_dev(sch); struct cake_sched_data *other_priv; - u64 new_rate = q->config->rate_bps; u64 other_qlen, other_last_active; struct Qdisc *other_sch; u32 num_active_qs = 1; @@ -2042,7 +2043,7 @@ 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(new_rate, num_active_qs); cake_configure_rates(sch, new_rate, true); q->last_checked_active = now; @@ -2625,13 +2626,15 @@ static void cake_reconfigure(struct Qdisc *sch) struct cake_sched_data *qd = qdisc_priv(sch); struct cake_sched_config *q = qd->config; u32 buffer_limit; + u64 rate_bps; - cake_configure_rates(sch, qd->config->rate_bps, false); + rate_bps = READ_ONCE(qd->config->rate_bps); + cake_configure_rates(sch, 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 (rate_bps) { + u64 t = rate_bps * q->interval; do_div(t, USEC_PER_SEC / 4); buffer_limit = max_t(u32, t, 4U << 20); @@ -2784,9 +2787,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 +2805,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) From d7010be1ba2337372ea32142951858f936c26e0d Mon Sep 17 00:00:00 2001 From: Jamal Hadi Salim Date: Tue, 20 Oct 2026 07:00:06 -0400 Subject: [PATCH 2/4] net/sched/sch_cake: reduce parent backlog when clearing a tin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) 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 Acked-by: Toke Høiland-Jørgensen Signed-off-by: Jamal Hadi Salim Link: https://patch.msgid.link/QDISC-5RTM.v3.r0.20261020053711-2@mojatatu.com Signed-off-by: Jakub Kicinski --- 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 d71617aa56d9..05b5f307884d 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; From dc48c97e7c69215483beeb7d1d80e8e3adbd92c9 Mon Sep 17 00:00:00 2001 From: Jamal Hadi Salim Date: Tue, 20 Oct 2026 07:00:07 -0400 Subject: [PATCH 3/4] net/sched/sch_cake: widen buffer_used to u64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. The dump path runs without the qdisc lock: tc_fill_qdisc() passes NULL to gnet_stats_start_copy_compat(), so the reader races with the enqueue-side update. READ_ONCE() is not an atomic 64-bit load on 32-bit CPUs, so a reader can observe a torn high-water mark and report a bogus MEMORY_USED. Publish the counter as u64_stats_t under the qdisc lock, which the enqueue path already holds, and take a seqcount-protected snapshot in the dump. The counter stays u64; only the exported value is bounded. 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)). A TUN device is needed so the root qdisc can park packets instead of dropping them. Open /dev/net/tun, request TUNSETIFF with IFF_TUN | IFF_NO_PI | IFF_BACKPRESSURE and keep the fd open without reading it (run it in the background and stop it at teardown): with IFF_BACKPRESSURE the full tx ring calls netif_tx_stop_queue() rather than dropping, so the queue behind the qdisc can grow past 4 GiB of resident truesize. A plain 'ip tuntap add' attaches no fd and does not request IFF_BACKPRESSURE, so it cannot park the queue this way. 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 # Then push more than 4 GiB of resident truesize through the qdisc: six # UDP senders using large SO_SNDBUFFORCE buffers send 1400-byte # datagrams (the reported OOM witness used 6 workers for 100 s on a # 12 GiB guest). On the unfixed kernel buffer_used wraps, the admission # test passes, the drop loop is bypassed and the guest runs out of # memory; with the fix the counter passes the limit without wrapping and # the excess is dropped. # The ACK-filter over-count is a separate corner: it needs ack-filter # enabled and a queued pure ACK whose truesize is larger than the # replacement ACK. Use a fresh interface (or 'tc qdisc change') rather # than a second root 'tc qdisc add' on an interface that already has a # qdisc: tc qdisc change dev tun0 root cake ack-filter CONFIG_NET_SCH_CAKE=y and CONFIG_TUN=y are required, and the memory-residency recipe needs a root-run sender envelope with enough guest RAM to hold the queued data. Reported-by: Sashiko (gemini) 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 Signed-off-by: Jamal Hadi Salim Acked-by: Toke Høiland-Jørgensen Link: https://patch.msgid.link/QDISC-5RTM.v3.r0.20261020053711-3@mojatatu.com Signed-off-by: Jakub Kicinski --- net/sched/sch_cake.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index 05b5f307884d..e74f85fc373f 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c @@ -63,6 +63,7 @@ #include #include #include +#include #include #include #include @@ -234,8 +235,10 @@ struct cake_sched_data { u16 tin_cnt; /* resource tracking */ - u32 buffer_used; - u32 buffer_max_used; + + struct u64_stats_sync buffer_max_used_sync; + u64_stats_t buffer_max_used; + u64 buffer_used; u32 buffer_limit; /* indices for dequeue */ @@ -1850,7 +1853,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); @@ -1941,8 +1945,11 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, cake_inc_dsthost_bulk_flow_count(b, flow, q->config->flow_mode); } - if (q->buffer_used > q->buffer_max_used) - WRITE_ONCE(q->buffer_max_used, q->buffer_used); + if (q->buffer_used > u64_stats_read(&q->buffer_max_used)) { + u64_stats_update_begin(&q->buffer_max_used_sync); + u64_stats_set(&q->buffer_max_used, q->buffer_used); + u64_stats_update_end(&q->buffer_max_used_sync); + } if (q->buffer_used <= q->buffer_limit) return NET_XMIT_SUCCESS; @@ -2862,6 +2869,7 @@ static int cake_init(struct Qdisc *sch, struct nlattr *opt, qd->cur_flow = 0; qd->config = q; + u64_stats_init(&qd->buffer_max_used_sync); qdisc_watchdog_init(&qd->watchdog, sch); if (opt) { @@ -3012,6 +3020,8 @@ static int cake_dump_stats(struct Qdisc *sch, struct gnet_dump *d) struct nlattr *stats = nla_nest_start_noflag(d->skb, TCA_STATS_APP); struct cake_sched_data *q = qdisc_priv(sch); struct nlattr *tstats, *ts; + u64 buffer_max_used; + unsigned int start; int i; if (!stats) @@ -3027,9 +3037,14 @@ static int cake_dump_stats(struct Qdisc *sch, struct gnet_dump *d) goto nla_put_failure; \ } while (0) + do { + start = u64_stats_fetch_begin(&q->buffer_max_used_sync); + buffer_max_used = u64_stats_read(&q->buffer_max_used); + } while (u64_stats_fetch_retry(&q->buffer_max_used_sync, start)); + 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, 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)); From 700b4c6c70d6a36e6878c5ea092c4877da3f10d8 Mon Sep 17 00:00:00 2001 From: Jamal Hadi Salim Date: Tue, 20 Oct 2026 07:00:08 -0400 Subject: [PATCH 4/4] net/sched/sch_cake: drop dead nla_nest_end() error check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Link: https://lore.kernel.org/netdev/87v77csd5q.fsf@toke.dk/ Reviewed-by: Victor Nogueira Acked-by: Toke Høiland-Jørgensen Signed-off-by: Jamal Hadi Salim Link: https://patch.msgid.link/QDISC-5RTM.v3.r0.20261020053711-4@mojatatu.com Signed-off-by: Jakub Kicinski --- 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 e74f85fc373f..aa4043d662c1 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c @@ -3226,8 +3226,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;