From 9b949fa69129e4b694ed11ee3be6d6edd4a9b8f4 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 14 May 2026 09:59:32 +0000 Subject: net/sched: sch_htb: do not change sch->flags in htb_dump() htb_dump() runs without holding qdisc spinlock. It is illegal to touch sch->flags with non locked RMW, as concurrent readers might see intermediate wrong values. Set TCQ_F_OFFLOADED in control path (htb_init()) instead. Fixes: d03b195b5aa0 ("sch_htb: Hierarchical QoS hardware offload") Signed-off-by: Eric Dumazet Link: https://patch.msgid.link/20260514095935.3926276-2-edumazet@google.com Signed-off-by: Jakub Kicinski --- net/sched/sch_htb.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 1e600f65c876..873337ac1cca 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -1147,6 +1147,7 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt, * parts (especially calling ndo_setup_tc) on errors. */ q->offload = true; + sch->flags |= TCQ_F_OFFLOADED; return 0; } @@ -1207,11 +1208,6 @@ static int htb_dump(struct Qdisc *sch, struct sk_buff *skb) struct nlattr *nest; struct tc_htb_glob gopt; - if (q->offload) - sch->flags |= TCQ_F_OFFLOADED; - else - sch->flags &= ~TCQ_F_OFFLOADED; - sch->qstats.overlimits = q->overlimits; /* Its safe to not acquire qdisc lock. As we hold RTNL, * no change can happen on the qdisc parameters. -- cgit v1.2.3 From e54c33503bf7cebb1c1790251ce90f1252678081 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 14 May 2026 09:59:33 +0000 Subject: net/sched: sch_htb: annotate data-races (I) htb_dump() runs without holding qdisc spinlock. Add missing READ_ONCE()/WRITE_ONCE() annotations around q->overlimits and q->direct_pkts. Fixes: edb09eb17ed8 ("net: sched: do not acquire qdisc spinlock in qdisc/class stats dump") Signed-off-by: Eric Dumazet Link: https://patch.msgid.link/20260514095935.3926276-3-edumazet@google.com Signed-off-by: Jakub Kicinski --- net/sched/sch_htb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 873337ac1cca..5bf1889bc030 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -568,7 +568,7 @@ htb_change_class_mode(struct htb_sched *q, struct htb_class *cl, s64 *diff) if (new_mode == HTB_CANT_SEND) { cl->overlimits++; - q->overlimits++; + WRITE_ONCE(q->overlimits, q->overlimits + 1); } if (cl->prio_activity) { /* not necessary: speed optimization */ @@ -628,7 +628,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch, /* enqueue to helper queue */ if (q->direct_queue.qlen < q->direct_qlen) { __qdisc_enqueue_tail(skb, &q->direct_queue); - q->direct_pkts++; + WRITE_ONCE(q->direct_pkts, q->direct_pkts + 1); } else { return qdisc_drop(skb, sch, to_free); } @@ -1208,12 +1208,12 @@ static int htb_dump(struct Qdisc *sch, struct sk_buff *skb) struct nlattr *nest; struct tc_htb_glob gopt; - sch->qstats.overlimits = q->overlimits; + sch->qstats.overlimits = READ_ONCE(q->overlimits); /* Its safe to not acquire qdisc lock. As we hold RTNL, * no change can happen on the qdisc parameters. */ - gopt.direct_pkts = q->direct_pkts; + gopt.direct_pkts = READ_ONCE(q->direct_pkts); gopt.version = HTB_VER; gopt.rate2quantum = q->rate2quantum; gopt.defcls = q->defcls; -- cgit v1.2.3 From 55562d46621ce4750a68f252b2124deed70c0529 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 14 May 2026 09:59:34 +0000 Subject: net/sched: sch_htb: annotate data-races (II) htb_dump_class_stats() will soon run locklessly. (no RTNL, not qdisc spinlock). Remove cl->xstats and replace it with two fields: - xstats_lends - xstats_borrows Then use READ_ONCE()/WRITE_ONCE() annotations on them, and change htb_dump_class_stats to use a private struct tc_htb_xstats. Signed-off-by: Eric Dumazet Link: https://patch.msgid.link/20260514095935.3926276-4-edumazet@google.com Signed-off-by: Jakub Kicinski --- net/sched/sch_htb.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 5bf1889bc030..353467eb7611 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -114,7 +114,8 @@ struct htb_class { */ struct gnet_stats_basic_sync bstats; struct gnet_stats_basic_sync bstats_bias; - struct tc_htb_xstats xstats; /* our special stats */ + u32 xstats_lends; + u32 xstats_borrows; /* token bucket parameters */ s64 tokens, ctokens;/* current number of tokens */ @@ -707,10 +708,10 @@ static void htb_charge_class(struct htb_sched *q, struct htb_class *cl, diff = min_t(s64, q->now - cl->t_c, cl->mbuffer); if (cl->level >= level) { if (cl->level == level) - cl->xstats.lends++; + WRITE_ONCE(cl->xstats_lends, cl->xstats_lends + 1); htb_accnt_tokens(cl, bytes, diff); } else { - cl->xstats.borrows++; + WRITE_ONCE(cl->xstats_borrows, cl->xstats_borrows + 1); cl->tokens += diff; /* we moved t_c; update tokens */ } htb_accnt_ctokens(cl, bytes, diff); @@ -1319,6 +1320,10 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d) { struct htb_class *cl = (struct htb_class *)arg; struct htb_sched *q = qdisc_priv(sch); + struct tc_htb_xstats xstats = { + .lends = READ_ONCE(cl->xstats_lends), + .borrows = READ_ONCE(cl->xstats_borrows), + }; struct gnet_stats_queue qs = { .drops = cl->drops, .overlimits = cl->overlimits, @@ -1328,10 +1333,10 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d) if (!cl->level && cl->leaf.q) qdisc_qstats_qlen_backlog(cl->leaf.q, &qlen, &qs.backlog); - cl->xstats.tokens = clamp_t(s64, PSCHED_NS2TICKS(cl->tokens), - INT_MIN, INT_MAX); - cl->xstats.ctokens = clamp_t(s64, PSCHED_NS2TICKS(cl->ctokens), - INT_MIN, INT_MAX); + xstats.tokens = clamp_t(s64, PSCHED_NS2TICKS(cl->tokens), + INT_MIN, INT_MAX); + xstats.ctokens = clamp_t(s64, PSCHED_NS2TICKS(cl->ctokens), + INT_MIN, INT_MAX); if (q->offload) { if (!cl->level) { @@ -1352,7 +1357,7 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d) gnet_stats_copy_queue(d, NULL, &qs, qlen) < 0) return -1; - return gnet_stats_copy_app(d, &cl->xstats, sizeof(cl->xstats)); + return gnet_stats_copy_app(d, &xstats, sizeof(xstats)); } static struct netdev_queue * -- cgit v1.2.3 From e249ae717beb95fdf56e094faee1d98ffdc48596 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 14 May 2026 09:59:35 +0000 Subject: net/sched: sch_htb: annotate data-races (III) htb_dump_class_stats() will soon run locklessly. (no RTNL, not qdisc spinlock). Add READ_ONCE()/WRITE_ONCE() annotations on these fields: - cl->overlimits - cl->drops - cl->tokens - cl->ctokens Signed-off-by: Eric Dumazet Link: https://patch.msgid.link/20260514095935.3926276-5-edumazet@google.com Signed-off-by: Jakub Kicinski --- net/sched/sch_htb.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 353467eb7611..d8ef3efbe0d5 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -568,7 +568,7 @@ htb_change_class_mode(struct htb_sched *q, struct htb_class *cl, s64 *diff) return; if (new_mode == HTB_CANT_SEND) { - cl->overlimits++; + WRITE_ONCE(cl->overlimits, cl->overlimits + 1); WRITE_ONCE(q->overlimits, q->overlimits + 1); } @@ -644,7 +644,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch, to_free)) != NET_XMIT_SUCCESS) { if (net_xmit_drop_count(ret)) { qdisc_qstats_drop(sch); - cl->drops++; + WRITE_ONCE(cl->drops, cl->drops + 1); } return ret; } else { @@ -666,7 +666,7 @@ static inline void htb_accnt_tokens(struct htb_class *cl, int bytes, s64 diff) if (toks <= -cl->mbuffer) toks = 1 - cl->mbuffer; - cl->tokens = toks; + WRITE_ONCE(cl->tokens, toks); } static inline void htb_accnt_ctokens(struct htb_class *cl, int bytes, s64 diff) @@ -679,7 +679,7 @@ static inline void htb_accnt_ctokens(struct htb_class *cl, int bytes, s64 diff) if (toks <= -cl->mbuffer) toks = 1 - cl->mbuffer; - cl->ctokens = toks; + WRITE_ONCE(cl->ctokens, toks); } /** @@ -712,7 +712,8 @@ static void htb_charge_class(struct htb_sched *q, struct htb_class *cl, htb_accnt_tokens(cl, bytes, diff); } else { WRITE_ONCE(cl->xstats_borrows, cl->xstats_borrows + 1); - cl->tokens += diff; /* we moved t_c; update tokens */ + /* we moved t_c; update tokens */ + WRITE_ONCE(cl->tokens, cl->tokens + diff); } htb_accnt_ctokens(cl, bytes, diff); cl->t_c = q->now; @@ -1325,17 +1326,17 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d) .borrows = READ_ONCE(cl->xstats_borrows), }; struct gnet_stats_queue qs = { - .drops = cl->drops, - .overlimits = cl->overlimits, + .drops = READ_ONCE(cl->drops), + .overlimits = READ_ONCE(cl->overlimits), }; __u32 qlen = 0; if (!cl->level && cl->leaf.q) qdisc_qstats_qlen_backlog(cl->leaf.q, &qlen, &qs.backlog); - xstats.tokens = clamp_t(s64, PSCHED_NS2TICKS(cl->tokens), + xstats.tokens = clamp_t(s64, PSCHED_NS2TICKS(READ_ONCE(cl->tokens)), INT_MIN, INT_MAX); - xstats.ctokens = clamp_t(s64, PSCHED_NS2TICKS(cl->ctokens), + xstats.ctokens = clamp_t(s64, PSCHED_NS2TICKS(READ_ONCE(cl->ctokens)), INT_MIN, INT_MAX); if (q->offload) { @@ -1517,8 +1518,8 @@ static void htb_parent_to_leaf(struct Qdisc *sch, struct htb_class *cl, parent->level = 0; memset(&parent->inner, 0, sizeof(parent->inner)); parent->leaf.q = new_q ? new_q : &noop_qdisc; - parent->tokens = parent->buffer; - parent->ctokens = parent->cbuffer; + WRITE_ONCE(parent->tokens, parent->buffer); + WRITE_ONCE(parent->ctokens, parent->cbuffer); parent->t_c = ktime_get_ns(); parent->cmode = HTB_CAN_SEND; if (q->offload) -- cgit v1.2.3