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=WnWdTKYN; arc=none (Message is not ARC signed); dmarc=none Received: from mail-qt1-x82f.google.com (mail-qt1-x82f.google.com [IPv6:2607:f8b0:4864:20::82f]) by mail.toke.dk (Postfix) with ESMTPS id 657B6177C4DD for ; Thu, 08 Oct 2026 09:47:30 +0200 (CEST) Received: by mail-qt1-x82f.google.com with SMTP id d75a77b69052e-533972e0cccso25722201cf.0 for ; Thu, 08 Oct 2026 00:47:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mojatatu.com; s=google; t=1791445649; x=1792050449; 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=BBt09e6X116fpikNWJ8doAPuw7URW+MowBdwTa6ZXP8=; b=WnWdTKYN2gnscY4G6BebflACkoHZqcYkGeGhh1/oiwpJoM5QNQw7W2hvPldLh2fJzH Wd4ywD9n+Ystl6uJtSnEqRoV8ktUUz//49Vp5Vx/MGRJL+PzEmxQVY79jpihtr6bfoJL 9yyiJ7X/9H7Mq0777F7tLEIF/3SS3+8YtmAxQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1791445649; x=1792050449; 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=BBt09e6X116fpikNWJ8doAPuw7URW+MowBdwTa6ZXP8=; b=ka6GPZqsK5wYRU1ukDGCqKzheQU9/Xj1XwnWi1AD1L7IOY1RLgg7LXLfeExFKZ/CtU HBfz64iGyG1+YnJSz6+FeF+wXYz/hpTD53eTgCV+t2GBoKXoEOAgvzlFaw4KMf/PeiPT qw8aY5/RNM+yGr/QJpRtryQFT0J9OcQYSNxrDizS8UnU6q4vLdrb6z4W9wh+grqy9FkX vOMB+Qa1atm29bpNyXE2+coaBES20SR36VzwMZB8wxeKkrnZkBkIZkl6K/3xK7YRk6VU 4obUbGwg6jcn4mlS4M86fpiHwzt8S+A/IDy9DK7YkmVPJ/WpcFk0GDpwHTs6shjcY88G AhmQ== X-Forwarded-Encrypted: i=1; AKwUvBwRxaxDa4+Z2eY5FBKi/8hmk5wCxudlwxYOnsDK8BnToGWgXwlNfgt5LA/xp2DtzqoYhqRb@lists.bufferbloat.net X-Gm-Message-State: AFuF++n9/ulx36zlbKc+HtkufXAn44h7DnVi8ntzuqWQNsTj/XuUKnZF 6pgchtOIa3PFXWKi7sPMX4qqLlEL5U90MQiPcSxFBKjH71lATj0PQggzV9R7sY5Tiw== X-Gm-Gg: AYBFou2gyKSeCTrrFtiwL9NLF+hoRT7dJD0f/rj6hWBMym2aem8cXleuY1+WTkwHNy6 lOKendOYDXexGUcI2CgBL/bPtRxI6iKtH+fLf7MZmFMMHMO2IXqHm+G11UXRXleZVKlyOSOEHtp wOEe3aQJ8WhXomKnjyqORZXphx3sAlh/+O+6EItEU8yfhzJQMEIdXY5DmDN84rn+krp0SYUb4TI +SDrJP3fo4bMFWgbh8dNhlWSPOF3RSqRLDP7DZmzxdltrA8XGCvdPfMJKYchjraWD9XD/sdq5p3 VnwNg915ZfnSsfou88StbDutyRZpEP+InLbblMmgLk5+aOstACr2kXa4/aUGD7+Lui9ipzFZzHS DBmviNz4eZVHxp3oSuckeInE5Kyp4BURs0Wqupn0ybe5IIUGb5KoXsA449mdrXhL+eVcBbAauQx gbXhFN8GflcFLuVxqZ6LBaW+XFhIWnBn3LjVOe3z9Pbu6heTlnlBcK6sW615Lg/1Q/N1M+UGpmM /h6wD38d3IsEC2vbm3Uf6Xe/id3O9W41bMGv9PcY5uORJJKOy8= X-Received: by 2002:ac8:5c83:0:b0:533:8900:4eb2 with SMTP id d75a77b69052e-5357573264amr78725401cf.58.1791445648684; Thu, 08 Oct 2026 00:47:28 -0700 (PDT) Received: from majuu.waya ([184.147.180.207]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-5357213f3dcsm38253961cf.17.2026.10.08.00.47.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Oct 2026 00:47:28 -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, 8 Oct 2026 03:47:09 -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: PQE6TFBPLOLG567PVTEMRFO3NHA5TZAY X-Message-ID-Hash: PQE6TFBPLOLG567PVTEMRFO3NHA5TZAY 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 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. 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. On 32-bit machines the unlocked 64-bit rate_bps store is also not atomic, so a reader can observe a torn rate. iproute2 reaches this because parsing 'bandwidth X' sets autorate = 0 and emits both TCA_CAKE_BASE_RATE64 and TCA_CAKE_AUTORATE. 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. Make rate_bps an atomic64_t so a 64-bit rate update is atomic on 32-bit machines as well, and keep the READ_ONCE()/WRITE_ONCE() annotations for the remaining lockless readers (cake_config_dump(), which runs without the lock, and the cake_mq shared config, which readers observe under different child locks). Conditions to recreate the bug: build with CONFIG_KCSAN=y (the race is otherwise not observable on 64-bit), 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'. The lost update of the installed rate is reproducible without a sanitizer by running bursty senders while repeatedly changing an autorate qdisc to a fixed 'bandwidth X' and reading the installed rate back. 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 | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index dc93267029e7..1c29695dc928 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c @@ -199,7 +199,7 @@ struct cake_tin_data { }; /* number of tins is small, so size of this struct doesn't matter much */ struct cake_sched_config { - u64 rate_bps; + atomic64_t rate_bps; u64 interval; u64 target; u64 sync_time; @@ -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; + atomic64_set(&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 = atomic64_read(&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(atomic64_read(&q->config->rate_bps), num_active_qs); cake_configure_rates(sch, new_rate, true); q->last_checked_active = now; @@ -2626,12 +2627,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, atomic64_read(&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 (atomic64_read(&q->rate_bps)) { + u64 t = atomic64_read(&q->rate_bps) * q->interval; do_div(t, USEC_PER_SEC / 4); buffer_limit = max_t(u32, t, 4U << 20); @@ -2686,8 +2687,8 @@ static int cake_config_change(struct cake_sched_config *q, struct nlattr *opt, } if (tb[TCA_CAKE_BASE_RATE64]) - WRITE_ONCE(q->rate_bps, - nla_get_u64(tb[TCA_CAKE_BASE_RATE64])); + atomic64_set(&q->rate_bps, + nla_get_u64(tb[TCA_CAKE_BASE_RATE64])); if (tb[TCA_CAKE_DIFFSERV_MODE]) WRITE_ONCE(q->tin_mode, @@ -2784,9 +2785,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 +2803,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) @@ -2818,7 +2826,7 @@ static void cake_config_init(struct cake_sched_config *q, bool is_shared) q->tin_mode = CAKE_DIFFSERV_DIFFSERV3; q->flow_mode = CAKE_FLOW_TRIPLE; - q->rate_bps = 0; /* unlimited by default */ + atomic64_set(&q->rate_bps, 0); /* unlimited by default */ q->interval = 100000; /* 100ms default */ q->target = 5000; /* 5ms: codel RFC argues @@ -2889,7 +2897,7 @@ static int cake_init(struct Qdisc *sch, struct nlattr *opt, } cake_reconfigure(sch); - qd->avg_peak_bandwidth = q->rate_bps; + qd->avg_peak_bandwidth = atomic64_read(&q->rate_bps); qd->min_netlen = ~0; qd->min_adjlen = ~0; qd->active_queues = 0; @@ -2917,7 +2925,7 @@ static int cake_config_dump(struct cake_sched_config *q, struct sk_buff *skb) goto nla_put_failure; if (nla_put_u64_64bit(skb, TCA_CAKE_BASE_RATE64, - READ_ONCE(q->rate_bps), TCA_CAKE_PAD)) + atomic64_read(&q->rate_bps), TCA_CAKE_PAD)) goto nla_put_failure; flow_mode = READ_ONCE(q->flow_mode); -- 2.43.0