From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mail.toke.dk; dkim=pass header.d=mojatatu.com header.i=@mojatatu.com header.a=rsa-sha256 header.s=google header.b=ZHnXE9lP; arc=none (Message is not ARC signed); dmarc=none Received: from mail-qv1-xf34.google.com (mail-qv1-xf34.google.com [IPv6:2607:f8b0:4864:20::f34]) by mail.toke.dk (Postfix) with ESMTPS id 88A201815FB0 for ; Thu, 15 Oct 2026 12:32:53 +0200 (CEST) Received: by mail-qv1-xf34.google.com with SMTP id 6a1803df08f44-91b8b456dbeso41127846d6.2 for ; Thu, 15 Oct 2026 03:32:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mojatatu.com; s=google; t=1792060372; x=1792665172; darn=lists.bufferbloat.net; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=RLIkzoYXG1Kln9Y1WBkDSiySFhY2wZnV2u+5d5Kewr4=; b=ZHnXE9lPqdlMCrM7vkOhQB//ew+j99g/r3Uew9TL6V07rRbvTIJL5ygcb0tJAWkXCL QaDo4QK0/aAWlAkDVvxZXcdZ2SBzz6vIE8692zJdEt1TbZGixTJ+1Rb9xYIotqvsoUOE U0w+iUNT8jyGjbYD0VYR+1JlCzj512ZrV4hiA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792060372; x=1792665172; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=RLIkzoYXG1Kln9Y1WBkDSiySFhY2wZnV2u+5d5Kewr4=; b=KMh5j5BOyXg26mXLDMmorvNPEsLPW3Y4MVJOPxMTXY/4S/0SY78iTee6xWoZaigrwv 4DsX/bsHeVs54UlueyhXUmwST6oGxofi0BJYoonNeO3hs69Sv1dkXEzsFAnkkVmY+XbQ wJ1PIFEptKpMzQ9BV2oKBzGi+nDfC+bog0uoVZB7hXuZhhGcLNk7NTCJA74DnE/wLj7Q UO9/0rvcEc67L16jJtKxEnmkh/pan7OO0hZdVJKceflA0R9VnScxnfBgjS3IHUOALgzZ uWzPgwq8Akz1N6qYyTV4QTZvv78xMplbuN7icd79HKHxB8zEn0B9TV8rdqqGXk8zQ55Y rUtg== X-Forwarded-Encrypted: i=1; AKwUvBxlTOXjKBM4xWcnbEtNW8Wmy0cU+C/BZEbpud+7oWUa7gyGv0WhZ0e0au/fMonFcqTp9yUk@lists.bufferbloat.net X-Gm-Message-State: AFq9FYJDEzx0FXgXNpXfCoyKKNF6iblS4w8yd/Lo+C7RAw6lMHZyFplH fvH6qgp14Pz37kragb0GNSCjQ2oAZP1DkateqpthTGg82ohEpy8RTLauToBQPiPFDw== X-Gm-Gg: AYBFou2GIuBhYcBQn7VdgJFtA0nE6UztaVFv6hBJdHuekMLpPDyortvKGGUZsKQsV8o AP6vJdve5yE/Af5jMXKzGA3Hj3KsHj2FJntpiZNsAYsMjUXcFF6H1GK0zQumo7D6F+1BtFg2MB/ e7WOi+e/AgwccgpMkGwncpXtddL9JUaepN+JPHQ7di/fPHxQZLzQ9zu7QwBsFa6hx8Cdhz7DfpY Rzo4wlOL9ur2sPJ6Xmy0+kd3RyYH6tOHI8vrTEygQDUVI44s+cuK5jy3cQxB+xdc5sEs7FMfczP dGvDPcSOO614v2pMvND3Mm+4z8UiZIKGyRoT76OpDL7x6pA414CyPiUS3AUCEAWmzFIs/0lgJeV eHDIuhjIi7WtUTCMZ+eyHuZtPKeMLZKW6uJM0yLn//vMWbiub+nf8eg9OKf0YKz4Ihhk5dQTLuu X9xHW+IMvJfM6IjyloOH7M6yZboTBAtWZDBdMTuNeQsKFvr2XT62Yw0IyCGO8TSZCe0YTIInU+X aCi0AtEc6E/dnp70qCHmYcppJFKkJn5W4pW+MDp6e/7 X-Received: by 2002:a05:620a:6f82:10b0:93e:fc4e:c4aa with SMTP id af79cd13be357-93efc4ee7afmr849923385a.45.1792060371848; Thu, 15 Oct 2026 03:32:51 -0700 (PDT) Received: from mbili.waya ([64.203.83.2]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-91ba684f404sm55261936d6.31.2026.10.15.03.32.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Oct 2026 03:32:51 -0700 (PDT) From: Jamal Hadi Salim To: netdev@vger.kernel.org Cc: Jamal Hadi Salim , =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= , Jiri Pirko , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , cake@lists.bufferbloat.net, Victor Nogueira , Sashiko Date: Thu, 15 Oct 2026 06:32:42 -0400 Message-Id: X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: OQQIV3RD7TX5IY27P6DRRMD7UJQQSIUY X-Message-ID-Hash: OQQIV3RD7TX5IY27P6DRRMD7UJQQSIUY X-MailFrom: jhs@mojatatu.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list Subject: [Cake] [PATCH net-next v2 1/4] net/sched/sch_cake: serialize reconfiguration with the datapath List-Id: Cake - FQ_codel the next generation Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 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) 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 Signed-off-by: Jamal Hadi Salim --- 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)