Skip to content

Commit 3951adc

Browse files
davem330davem330
committed
Merge branch 'net-sched-parsing-prints'
Pedro Tammela says: ==================== net/sched: cleanup parsing prints in htb and qfq These two qdiscs are still using prints on dmesg to report parsing errors. Since the parsing code has access to extack, convert these error messages to extack. QFQ also had the opportunity to remove some redundant code in the parameters parsing by transforming some attributes into parsing policies. v4->v5: Rebased v3->v4: Drop 'BITification' as suggested by Eric v2->v3: Address suggestions by Jakub and Simon v1->v2: Address suggestions by Jakub ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents fadfc57 + 7eb060a commit 3951adc

File tree

3 files changed

+97
-26
lines changed

3 files changed

+97
-26
lines changed

‎net/sched/sch_htb.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,7 +1786,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
17861786
goto failure;
17871787

17881788
err = nla_parse_nested_deprecated(tb, TCA_HTB_MAX, opt, htb_policy,
1789-
NULL);
1789+
extack);
17901790
if (err < 0)
17911791
goto failure;
17921792

@@ -1858,7 +1858,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
18581858

18591859
/* check maximal depth */
18601860
if (parent && parent->parent && parent->parent->level < 2) {
1861-
pr_err("htb: tree is too deep\n");
1861+
NL_SET_ERR_MSG_MOD(extack, "tree is too deep");
18621862
goto failure;
18631863
}
18641864
err = -ENOBUFS;
@@ -1917,8 +1917,8 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
19171917
};
19181918
err = htb_offload(dev, &offload_opt);
19191919
if (err) {
1920-
pr_err("htb: TC_HTB_LEAF_ALLOC_QUEUE failed with err = %d\n",
1921-
err);
1920+
NL_SET_ERR_MSG_WEAK(extack,
1921+
"Failed to offload TC_HTB_LEAF_ALLOC_QUEUE");
19221922
goto err_kill_estimator;
19231923
}
19241924
dev_queue = netdev_get_tx_queue(dev, offload_opt.qid);
@@ -1937,8 +1937,8 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
19371937
};
19381938
err = htb_offload(dev, &offload_opt);
19391939
if (err) {
1940-
pr_err("htb: TC_HTB_LEAF_TO_INNER failed with err = %d\n",
1941-
err);
1940+
NL_SET_ERR_MSG_WEAK(extack,
1941+
"Failed to offload TC_HTB_LEAF_TO_INNER");
19421942
htb_graft_helper(dev_queue, old_q);
19431943
goto err_kill_estimator;
19441944
}
@@ -2067,8 +2067,9 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
20672067
qdisc_put(parent_qdisc);
20682068

20692069
if (warn)
2070-
pr_warn("HTB: quantum of class %X is %s. Consider r2q change.\n",
2071-
cl->common.classid, (warn == -1 ? "small" : "big"));
2070+
NL_SET_ERR_MSG_FMT_MOD(extack,
2071+
"quantum of class %X is %s. Consider r2q change.",
2072+
cl->common.classid, (warn == -1 ? "small" : "big"));
20722073

20732074
qdisc_class_hash_grow(sch, &q->clhash);
20742075

‎net/sched/sch_qfq.c

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@
113113

114114
#define QFQ_MTU_SHIFT 16 /* to support TSO/GSO */
115115
#define QFQ_MIN_LMAX 512 /* see qfq_slot_insert */
116+
#define QFQ_MAX_LMAX (1UL << QFQ_MTU_SHIFT)
116117

117118
#define QFQ_MAX_AGG_CLASSES 8 /* max num classes per aggregate allowed */
118119

@@ -214,9 +215,14 @@ static struct qfq_class *qfq_find_class(struct Qdisc *sch, u32 classid)
214215
return container_of(clc, struct qfq_class, common);
215216
}
216217

218+
static struct netlink_range_validation lmax_range = {
219+
.min = QFQ_MIN_LMAX,
220+
.max = QFQ_MAX_LMAX,
221+
};
222+
217223
static const struct nla_policy qfq_policy[TCA_QFQ_MAX + 1] = {
218-
[TCA_QFQ_WEIGHT] = { .type = NLA_U32 },
219-
[TCA_QFQ_LMAX] = { .type = NLA_U32 },
224+
[TCA_QFQ_WEIGHT] = NLA_POLICY_RANGE(NLA_U32, 1, QFQ_MAX_WEIGHT),
225+
[TCA_QFQ_LMAX] = NLA_POLICY_FULL_RANGE(NLA_U32, &lmax_range),
220226
};
221227

222228
/*
@@ -402,35 +408,26 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
402408
int err;
403409
int delta_w;
404410

405-
if (tca[TCA_OPTIONS] == NULL) {
406-
pr_notice("qfq: no options\n");
411+
if (NL_REQ_ATTR_CHECK(extack, NULL, tca, TCA_OPTIONS)) {
412+
NL_SET_ERR_MSG_MOD(extack, "missing options");
407413
return -EINVAL;
408414
}
409415

410416
err = nla_parse_nested_deprecated(tb, TCA_QFQ_MAX, tca[TCA_OPTIONS],
411-
qfq_policy, NULL);
417+
qfq_policy, extack);
412418
if (err < 0)
413419
return err;
414420

415-
if (tb[TCA_QFQ_WEIGHT]) {
421+
if (tb[TCA_QFQ_WEIGHT])
416422
weight = nla_get_u32(tb[TCA_QFQ_WEIGHT]);
417-
if (!weight || weight > (1UL << QFQ_MAX_WSHIFT)) {
418-
pr_notice("qfq: invalid weight %u\n", weight);
419-
return -EINVAL;
420-
}
421-
} else
423+
else
422424
weight = 1;
423425

424426
if (tb[TCA_QFQ_LMAX])
425427
lmax = nla_get_u32(tb[TCA_QFQ_LMAX]);
426428
else
427429
lmax = psched_mtu(qdisc_dev(sch));
428430

429-
if (lmax < QFQ_MIN_LMAX || lmax > (1UL << QFQ_MTU_SHIFT)) {
430-
pr_notice("qfq: invalid max length %u\n", lmax);
431-
return -EINVAL;
432-
}
433-
434431
inv_w = ONE_FP / weight;
435432
weight = ONE_FP / inv_w;
436433

@@ -442,8 +439,9 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
442439
delta_w = weight - (cl ? cl->agg->class_weight : 0);
443440

444441
if (q->wsum + delta_w > QFQ_MAX_WSUM) {
445-
pr_notice("qfq: total weight out of range (%d + %u)\n",
446-
delta_w, q->wsum);
442+
NL_SET_ERR_MSG_FMT_MOD(extack,
443+
"total weight out of range (%d + %u)\n",
444+
delta_w, q->wsum);
447445
return -EINVAL;
448446
}
449447

‎tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,30 @@
4646
"$IP link del dev $DUMMY type dummy"
4747
]
4848
},
49+
{
50+
"id": "d364",
51+
"name": "Test QFQ with max class weight setting",
52+
"category": [
53+
"qdisc",
54+
"qfq"
55+
],
56+
"plugins": {
57+
"requires": "nsPlugin"
58+
},
59+
"setup": [
60+
"$IP link add dev $DUMMY type dummy || /bin/true",
61+
"$TC qdisc add dev $DUMMY handle 1: root qfq"
62+
],
63+
"cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 9999",
64+
"expExitCode": "2",
65+
"verifyCmd": "$TC class show dev $DUMMY",
66+
"matchPattern": "class qfq 1:1 root weight 9999 maxpkt",
67+
"matchCount": "0",
68+
"teardown": [
69+
"$TC qdisc del dev $DUMMY handle 1: root",
70+
"$IP link del dev $DUMMY type dummy"
71+
]
72+
},
4973
{
5074
"id": "8452",
5175
"name": "Create QFQ with class maxpkt setting",
@@ -70,6 +94,54 @@
7094
"$IP link del dev $DUMMY type dummy"
7195
]
7296
},
97+
{
98+
"id": "22df",
99+
"name": "Test QFQ class maxpkt setting lower bound",
100+
"category": [
101+
"qdisc",
102+
"qfq"
103+
],
104+
"plugins": {
105+
"requires": "nsPlugin"
106+
},
107+
"setup": [
108+
"$IP link add dev $DUMMY type dummy || /bin/true",
109+
"$TC qdisc add dev $DUMMY handle 1: root qfq"
110+
],
111+
"cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq maxpkt 128",
112+
"expExitCode": "2",
113+
"verifyCmd": "$TC class show dev $DUMMY",
114+
"matchPattern": "class qfq 1:1 root weight 1 maxpkt 128",
115+
"matchCount": "0",
116+
"teardown": [
117+
"$TC qdisc del dev $DUMMY handle 1: root",
118+
"$IP link del dev $DUMMY type dummy"
119+
]
120+
},
121+
{
122+
"id": "92ee",
123+
"name": "Test QFQ class maxpkt setting upper bound",
124+
"category": [
125+
"qdisc",
126+
"qfq"
127+
],
128+
"plugins": {
129+
"requires": "nsPlugin"
130+
},
131+
"setup": [
132+
"$IP link add dev $DUMMY type dummy || /bin/true",
133+
"$TC qdisc add dev $DUMMY handle 1: root qfq"
134+
],
135+
"cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq maxpkt 99999",
136+
"expExitCode": "2",
137+
"verifyCmd": "$TC class show dev $DUMMY",
138+
"matchPattern": "class qfq 1:1 root weight 1 maxpkt 99999",
139+
"matchCount": "0",
140+
"teardown": [
141+
"$TC qdisc del dev $DUMMY handle 1: root",
142+
"$IP link del dev $DUMMY type dummy"
143+
]
144+
},
73145
{
74146
"id": "d920",
75147
"name": "Create QFQ with multiple class setting",

0 commit comments

Comments
 (0)