Skip to content

Commit d62f38c

Browse files
davem330davem330
committed
Merge branch 'sch_cake-leaf-qdisc-fixes'
Toke Høiland-Jørgensen says: ==================== sched: Fix qdisc interactions exposed by using sch_cake as a leaf qdisc This series fixes a couple of issues exposed by running sch_cake as a leaf qdisc in an HFSC tree, which were discovered and reported by Pete Heist. The interaction between CAKE's GSO splitting and the parent qdisc's notion of its own queue length could cause queue stalls. While investigating the report, I also noticed that several qdiscs would dereference the skb pointer after dequeue, which is potentially problematic since the GSO splitting code also frees the original skb. See the individual patches in the series for details. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents 80b3671 + 8c6c37f commit d62f38c

File tree

9 files changed

+35
-21
lines changed

9 files changed

+35
-21
lines changed

‎net/sched/sch_cake.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,7 +1667,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
16671667
if (skb_is_gso(skb) && q->rate_flags & CAKE_FLAG_SPLIT_GSO) {
16681668
struct sk_buff *segs, *nskb;
16691669
netdev_features_t features = netif_skb_features(skb);
1670-
unsigned int slen = 0;
1670+
unsigned int slen = 0, numsegs = 0;
16711671

16721672
segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
16731673
if (IS_ERR_OR_NULL(segs))
@@ -1683,6 +1683,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
16831683
flow_queue_add(flow, segs);
16841684

16851685
sch->q.qlen++;
1686+
numsegs++;
16861687
slen += segs->len;
16871688
q->buffer_used += segs->truesize;
16881689
b->packets++;
@@ -1696,7 +1697,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
16961697
sch->qstats.backlog += slen;
16971698
q->avg_window_bytes += slen;
16981699

1699-
qdisc_tree_reduce_backlog(sch, 1, len);
1700+
qdisc_tree_reduce_backlog(sch, 1-numsegs, len-slen);
17001701
consume_skb(skb);
17011702
} else {
17021703
/* not splitting */

‎net/sched/sch_cbs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,14 @@ static int cbs_child_enqueue(struct sk_buff *skb, struct Qdisc *sch,
8888
struct Qdisc *child,
8989
struct sk_buff **to_free)
9090
{
91+
unsigned int len = qdisc_pkt_len(skb);
9192
int err;
9293

9394
err = child->ops->enqueue(skb, child, to_free);
9495
if (err != NET_XMIT_SUCCESS)
9596
return err;
9697

97-
qdisc_qstats_backlog_inc(sch, skb);
98+
sch->qstats.backlog += len;
9899
sch->q.qlen++;
99100

100101
return NET_XMIT_SUCCESS;

‎net/sched/sch_drr.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,11 @@ static struct drr_class *drr_classify(struct sk_buff *skb, struct Qdisc *sch,
350350
static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
351351
struct sk_buff **to_free)
352352
{
353+
unsigned int len = qdisc_pkt_len(skb);
353354
struct drr_sched *q = qdisc_priv(sch);
354355
struct drr_class *cl;
355356
int err = 0;
357+
bool first;
356358

357359
cl = drr_classify(skb, sch, &err);
358360
if (cl == NULL) {
@@ -362,6 +364,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
362364
return err;
363365
}
364366

367+
first = !cl->qdisc->q.qlen;
365368
err = qdisc_enqueue(skb, cl->qdisc, to_free);
366369
if (unlikely(err != NET_XMIT_SUCCESS)) {
367370
if (net_xmit_drop_count(err)) {
@@ -371,12 +374,12 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
371374
return err;
372375
}
373376

374-
if (cl->qdisc->q.qlen == 1) {
377+
if (first) {
375378
list_add_tail(&cl->alist, &q->active);
376379
cl->deficit = cl->quantum;
377380
}
378381

379-
qdisc_qstats_backlog_inc(sch, skb);
382+
sch->qstats.backlog += len;
380383
sch->q.qlen++;
381384
return err;
382385
}

‎net/sched/sch_dsmark.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ static struct tcf_block *dsmark_tcf_block(struct Qdisc *sch, unsigned long cl,
199199
static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch,
200200
struct sk_buff **to_free)
201201
{
202+
unsigned int len = qdisc_pkt_len(skb);
202203
struct dsmark_qdisc_data *p = qdisc_priv(sch);
203204
int err;
204205

@@ -271,7 +272,7 @@ static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch,
271272
return err;
272273
}
273274

274-
qdisc_qstats_backlog_inc(sch, skb);
275+
sch->qstats.backlog += len;
275276
sch->q.qlen++;
276277

277278
return NET_XMIT_SUCCESS;

‎net/sched/sch_hfsc.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,8 +1539,10 @@ hfsc_dump_qdisc(struct Qdisc *sch, struct sk_buff *skb)
15391539
static int
15401540
hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
15411541
{
1542+
unsigned int len = qdisc_pkt_len(skb);
15421543
struct hfsc_class *cl;
15431544
int uninitialized_var(err);
1545+
bool first;
15441546

15451547
cl = hfsc_classify(skb, sch, &err);
15461548
if (cl == NULL) {
@@ -1550,6 +1552,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
15501552
return err;
15511553
}
15521554

1555+
first = !cl->qdisc->q.qlen;
15531556
err = qdisc_enqueue(skb, cl->qdisc, to_free);
15541557
if (unlikely(err != NET_XMIT_SUCCESS)) {
15551558
if (net_xmit_drop_count(err)) {
@@ -1559,9 +1562,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
15591562
return err;
15601563
}
15611564

1562-
if (cl->qdisc->q.qlen == 1) {
1563-
unsigned int len = qdisc_pkt_len(skb);
1564-
1565+
if (first) {
15651566
if (cl->cl_flags & HFSC_RSC)
15661567
init_ed(cl, len);
15671568
if (cl->cl_flags & HFSC_FSC)
@@ -1576,7 +1577,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
15761577

15771578
}
15781579

1579-
qdisc_qstats_backlog_inc(sch, skb);
1580+
sch->qstats.backlog += len;
15801581
sch->q.qlen++;
15811582

15821583
return NET_XMIT_SUCCESS;

‎net/sched/sch_htb.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
581581
struct sk_buff **to_free)
582582
{
583583
int uninitialized_var(ret);
584+
unsigned int len = qdisc_pkt_len(skb);
584585
struct htb_sched *q = qdisc_priv(sch);
585586
struct htb_class *cl = htb_classify(skb, sch, &ret);
586587

@@ -610,7 +611,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
610611
htb_activate(q, cl);
611612
}
612613

613-
qdisc_qstats_backlog_inc(sch, skb);
614+
sch->qstats.backlog += len;
614615
sch->q.qlen++;
615616
return NET_XMIT_SUCCESS;
616617
}

‎net/sched/sch_prio.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
7272
static int
7373
prio_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
7474
{
75+
unsigned int len = qdisc_pkt_len(skb);
7576
struct Qdisc *qdisc;
7677
int ret;
7778

@@ -88,7 +89,7 @@ prio_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
8889

8990
ret = qdisc_enqueue(skb, qdisc, to_free);
9091
if (ret == NET_XMIT_SUCCESS) {
91-
qdisc_qstats_backlog_inc(sch, skb);
92+
sch->qstats.backlog += len;
9293
sch->q.qlen++;
9394
return NET_XMIT_SUCCESS;
9495
}

‎net/sched/sch_qfq.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,10 +1210,12 @@ static struct qfq_aggregate *qfq_choose_next_agg(struct qfq_sched *q)
12101210
static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
12111211
struct sk_buff **to_free)
12121212
{
1213+
unsigned int len = qdisc_pkt_len(skb), gso_segs;
12131214
struct qfq_sched *q = qdisc_priv(sch);
12141215
struct qfq_class *cl;
12151216
struct qfq_aggregate *agg;
12161217
int err = 0;
1218+
bool first;
12171219

12181220
cl = qfq_classify(skb, sch, &err);
12191221
if (cl == NULL) {
@@ -1224,17 +1226,18 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
12241226
}
12251227
pr_debug("qfq_enqueue: cl = %x\n", cl->common.classid);
12261228

1227-
if (unlikely(cl->agg->lmax < qdisc_pkt_len(skb))) {
1229+
if (unlikely(cl->agg->lmax < len)) {
12281230
pr_debug("qfq: increasing maxpkt from %u to %u for class %u",
1229-
cl->agg->lmax, qdisc_pkt_len(skb), cl->common.classid);
1230-
err = qfq_change_agg(sch, cl, cl->agg->class_weight,
1231-
qdisc_pkt_len(skb));
1231+
cl->agg->lmax, len, cl->common.classid);
1232+
err = qfq_change_agg(sch, cl, cl->agg->class_weight, len);
12321233
if (err) {
12331234
cl->qstats.drops++;
12341235
return qdisc_drop(skb, sch, to_free);
12351236
}
12361237
}
12371238

1239+
gso_segs = skb_is_gso(skb) ? skb_shinfo(skb)->gso_segs : 1;
1240+
first = !cl->qdisc->q.qlen;
12381241
err = qdisc_enqueue(skb, cl->qdisc, to_free);
12391242
if (unlikely(err != NET_XMIT_SUCCESS)) {
12401243
pr_debug("qfq_enqueue: enqueue failed %d\n", err);
@@ -1245,16 +1248,17 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
12451248
return err;
12461249
}
12471250

1248-
bstats_update(&cl->bstats, skb);
1249-
qdisc_qstats_backlog_inc(sch, skb);
1251+
cl->bstats.bytes += len;
1252+
cl->bstats.packets += gso_segs;
1253+
sch->qstats.backlog += len;
12501254
++sch->q.qlen;
12511255

12521256
agg = cl->agg;
12531257
/* if the queue was not empty, then done here */
1254-
if (cl->qdisc->q.qlen != 1) {
1258+
if (!first) {
12551259
if (unlikely(skb == cl->qdisc->ops->peek(cl->qdisc)) &&
12561260
list_first_entry(&agg->active, struct qfq_class, alist)
1257-
== cl && cl->deficit < qdisc_pkt_len(skb))
1261+
== cl && cl->deficit < len)
12581262
list_move_tail(&cl->alist, &agg->active);
12591263

12601264
return err;

‎net/sched/sch_tbf.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch,
185185
struct sk_buff **to_free)
186186
{
187187
struct tbf_sched_data *q = qdisc_priv(sch);
188+
unsigned int len = qdisc_pkt_len(skb);
188189
int ret;
189190

190191
if (qdisc_pkt_len(skb) > q->max_size) {
@@ -200,7 +201,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch,
200201
return ret;
201202
}
202203

203-
qdisc_qstats_backlog_inc(sch, skb);
204+
sch->qstats.backlog += len;
204205
sch->q.qlen++;
205206
return NET_XMIT_SUCCESS;
206207
}

0 commit comments

Comments
 (0)