Skip to content

Commit 1ada03f

Browse files
edumazetkuba-moo
authored andcommitted
net/sched: sch_sfb: annotate data-races in sfb_dump_stats()
sfb_dump_stats() only runs with RTNL held, reading fields that can be changed in qdisc fast path. Add READ_ONCE()/WRITE_ONCE() annotations. Alternative would be to acquire the qdisc spinlock, but our long-term goal is to make qdisc dump operations lockless as much as we can. tc_sfb_xstats fields don't need to be latched atomically, otherwise this bug would have been caught earlier. Fixes: edb09eb ("net: sched: do not acquire qdisc spinlock in qdisc/class stats dump") Signed-off-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20260421141655.3953721-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent a8f5192 commit 1ada03f

1 file changed

Lines changed: 32 additions & 22 deletions

File tree

net/sched/sch_sfb.c

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ static void increment_one_qlen(u32 sfbhash, u32 slot, struct sfb_sched_data *q)
130130

131131
sfbhash >>= SFB_BUCKET_SHIFT;
132132
if (b[hash].qlen < 0xFFFF)
133-
b[hash].qlen++;
133+
WRITE_ONCE(b[hash].qlen, b[hash].qlen + 1);
134134
b += SFB_NUMBUCKETS; /* next level */
135135
}
136136
}
@@ -159,7 +159,7 @@ static void decrement_one_qlen(u32 sfbhash, u32 slot,
159159

160160
sfbhash >>= SFB_BUCKET_SHIFT;
161161
if (b[hash].qlen > 0)
162-
b[hash].qlen--;
162+
WRITE_ONCE(b[hash].qlen, b[hash].qlen - 1);
163163
b += SFB_NUMBUCKETS; /* next level */
164164
}
165165
}
@@ -179,12 +179,12 @@ static void decrement_qlen(const struct sk_buff *skb, struct sfb_sched_data *q)
179179

180180
static void decrement_prob(struct sfb_bucket *b, struct sfb_sched_data *q)
181181
{
182-
b->p_mark = prob_minus(b->p_mark, q->decrement);
182+
WRITE_ONCE(b->p_mark, prob_minus(b->p_mark, q->decrement));
183183
}
184184

185185
static void increment_prob(struct sfb_bucket *b, struct sfb_sched_data *q)
186186
{
187-
b->p_mark = prob_plus(b->p_mark, q->increment);
187+
WRITE_ONCE(b->p_mark, prob_plus(b->p_mark, q->increment));
188188
}
189189

190190
static void sfb_zero_all_buckets(struct sfb_sched_data *q)
@@ -202,11 +202,14 @@ static u32 sfb_compute_qlen(u32 *prob_r, u32 *avgpm_r, const struct sfb_sched_da
202202
const struct sfb_bucket *b = &q->bins[q->slot].bins[0][0];
203203

204204
for (i = 0; i < SFB_LEVELS * SFB_NUMBUCKETS; i++) {
205-
if (qlen < b->qlen)
206-
qlen = b->qlen;
207-
totalpm += b->p_mark;
208-
if (prob < b->p_mark)
209-
prob = b->p_mark;
205+
u32 b_qlen = READ_ONCE(b->qlen);
206+
u32 b_mark = READ_ONCE(b->p_mark);
207+
208+
if (qlen < b_qlen)
209+
qlen = b_qlen;
210+
totalpm += b_mark;
211+
if (prob < b_mark)
212+
prob = b_mark;
210213
b++;
211214
}
212215
*prob_r = prob;
@@ -295,7 +298,8 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
295298

296299
if (unlikely(sch->q.qlen >= q->limit)) {
297300
qdisc_qstats_overlimit(sch);
298-
q->stats.queuedrop++;
301+
WRITE_ONCE(q->stats.queuedrop,
302+
q->stats.queuedrop + 1);
299303
goto drop;
300304
}
301305

@@ -348,7 +352,8 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
348352

349353
if (unlikely(minqlen >= q->max)) {
350354
qdisc_qstats_overlimit(sch);
351-
q->stats.bucketdrop++;
355+
WRITE_ONCE(q->stats.bucketdrop,
356+
q->stats.bucketdrop + 1);
352357
goto drop;
353358
}
354359

@@ -374,7 +379,8 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
374379
}
375380
if (sfb_rate_limit(skb, q)) {
376381
qdisc_qstats_overlimit(sch);
377-
q->stats.penaltydrop++;
382+
WRITE_ONCE(q->stats.penaltydrop,
383+
q->stats.penaltydrop + 1);
378384
goto drop;
379385
}
380386
goto enqueue;
@@ -390,14 +396,17 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
390396
* In either case, we want to start dropping packets.
391397
*/
392398
if (r < (p_min - SFB_MAX_PROB / 2) * 2) {
393-
q->stats.earlydrop++;
399+
WRITE_ONCE(q->stats.earlydrop,
400+
q->stats.earlydrop + 1);
394401
goto drop;
395402
}
396403
}
397404
if (INET_ECN_set_ce(skb)) {
398-
q->stats.marked++;
405+
WRITE_ONCE(q->stats.marked,
406+
q->stats.marked + 1);
399407
} else {
400-
q->stats.earlydrop++;
408+
WRITE_ONCE(q->stats.earlydrop,
409+
q->stats.earlydrop + 1);
401410
goto drop;
402411
}
403412
}
@@ -410,7 +419,8 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
410419
sch->q.qlen++;
411420
increment_qlen(&cb, q);
412421
} else if (net_xmit_drop_count(ret)) {
413-
q->stats.childdrop++;
422+
WRITE_ONCE(q->stats.childdrop,
423+
q->stats.childdrop + 1);
414424
qdisc_qstats_drop(sch);
415425
}
416426
return ret;
@@ -599,12 +609,12 @@ static int sfb_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
599609
{
600610
struct sfb_sched_data *q = qdisc_priv(sch);
601611
struct tc_sfb_xstats st = {
602-
.earlydrop = q->stats.earlydrop,
603-
.penaltydrop = q->stats.penaltydrop,
604-
.bucketdrop = q->stats.bucketdrop,
605-
.queuedrop = q->stats.queuedrop,
606-
.childdrop = q->stats.childdrop,
607-
.marked = q->stats.marked,
612+
.earlydrop = READ_ONCE(q->stats.earlydrop),
613+
.penaltydrop = READ_ONCE(q->stats.penaltydrop),
614+
.bucketdrop = READ_ONCE(q->stats.bucketdrop),
615+
.queuedrop = READ_ONCE(q->stats.queuedrop),
616+
.childdrop = READ_ONCE(q->stats.childdrop),
617+
.marked = READ_ONCE(q->stats.marked),
608618
};
609619

610620
st.maxqlen = sfb_compute_qlen(&st.maxprob, &st.avgprob, q);

0 commit comments

Comments
 (0)