* [PATCH net-next v4 1/4] net/sched: sch_htb: use extack on errors messages
2023-04-21 17:53 [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq Pedro Tammela
@ 2023-04-21 17:53 ` Pedro Tammela
2023-04-21 17:53 ` [PATCH net-next v4 2/4] net/sched: sch_qfq: " Pedro Tammela
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pedro Tammela @ 2023-04-21 17:53 UTC (permalink / raw)
To: netdev
Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
simon.horman, Pedro Tammela
Some error messages are still being printed to dmesg.
Since extack is available, provide error messages there.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
net/sched/sch_htb.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 92f2975b6a82..8aef7dd9fb88 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1786,7 +1786,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
goto failure;
err = nla_parse_nested_deprecated(tb, TCA_HTB_MAX, opt, htb_policy,
- NULL);
+ extack);
if (err < 0)
goto failure;
@@ -1858,7 +1858,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
/* check maximal depth */
if (parent && parent->parent && parent->parent->level < 2) {
- pr_err("htb: tree is too deep\n");
+ NL_SET_ERR_MSG_MOD(extack, "tree is too deep");
goto failure;
}
err = -ENOBUFS;
@@ -1917,8 +1917,8 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
};
err = htb_offload(dev, &offload_opt);
if (err) {
- pr_err("htb: TC_HTB_LEAF_ALLOC_QUEUE failed with err = %d\n",
- err);
+ NL_SET_ERR_MSG_WEAK(extack,
+ "Failed to offload TC_HTB_LEAF_ALLOC_QUEUE");
goto err_kill_estimator;
}
dev_queue = netdev_get_tx_queue(dev, offload_opt.qid);
@@ -1937,8 +1937,8 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
};
err = htb_offload(dev, &offload_opt);
if (err) {
- pr_err("htb: TC_HTB_LEAF_TO_INNER failed with err = %d\n",
- err);
+ NL_SET_ERR_MSG_WEAK(extack,
+ "Failed to offload TC_HTB_LEAF_TO_INNER");
htb_graft_helper(dev_queue, old_q);
goto err_kill_estimator;
}
@@ -2067,8 +2067,9 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
qdisc_put(parent_qdisc);
if (warn)
- pr_warn("HTB: quantum of class %X is %s. Consider r2q change.\n",
- cl->common.classid, (warn == -1 ? "small" : "big"));
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "quantum of class %X is %s. Consider r2q change.",
+ cl->common.classid, (warn == -1 ? "small" : "big"));
qdisc_class_hash_grow(sch, &q->clhash);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next v4 2/4] net/sched: sch_qfq: use extack on errors messages
2023-04-21 17:53 [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq Pedro Tammela
2023-04-21 17:53 ` [PATCH net-next v4 1/4] net/sched: sch_htb: use extack on errors messages Pedro Tammela
@ 2023-04-21 17:53 ` Pedro Tammela
2023-04-21 17:53 ` [PATCH net-next v4 3/4] net/sched: sch_qfq: refactor parsing of netlink parameters Pedro Tammela
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pedro Tammela @ 2023-04-21 17:53 UTC (permalink / raw)
To: netdev
Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
simon.horman, Pedro Tammela
Some error messages are still being printed to dmesg.
Since extack is available, provide error messages there.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
net/sched/sch_qfq.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index cf5ebe43b3b4..323609cfbc67 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -402,8 +402,8 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
int err;
int delta_w;
- if (tca[TCA_OPTIONS] == NULL) {
- pr_notice("qfq: no options\n");
+ if (NL_REQ_ATTR_CHECK(extack, NULL, tca, TCA_OPTIONS)) {
+ NL_SET_ERR_MSG_MOD(extack, "missing options");
return -EINVAL;
}
@@ -441,8 +441,9 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
delta_w = weight - (cl ? cl->agg->class_weight : 0);
if (q->wsum + delta_w > QFQ_MAX_WSUM) {
- pr_notice("qfq: total weight out of range (%d + %u)\n",
- delta_w, q->wsum);
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "total weight out of range (%d + %u)\n",
+ delta_w, q->wsum);
return -EINVAL;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next v4 3/4] net/sched: sch_qfq: refactor parsing of netlink parameters
2023-04-21 17:53 [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq Pedro Tammela
2023-04-21 17:53 ` [PATCH net-next v4 1/4] net/sched: sch_htb: use extack on errors messages Pedro Tammela
2023-04-21 17:53 ` [PATCH net-next v4 2/4] net/sched: sch_qfq: " Pedro Tammela
@ 2023-04-21 17:53 ` Pedro Tammela
2023-04-21 17:53 ` [PATCH net-next v4 4/4] selftests: tc-testing: add more tests for sch_qfq Pedro Tammela
2023-04-22 4:08 ` [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq Jakub Kicinski
4 siblings, 0 replies; 6+ messages in thread
From: Pedro Tammela @ 2023-04-21 17:53 UTC (permalink / raw)
To: netdev
Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
simon.horman, Pedro Tammela
Two parameters can be transformed into netlink policies and
validated while parsing the netlink message.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
net/sched/sch_qfq.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 323609cfbc67..dfd9a99e6257 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -113,6 +113,7 @@
#define QFQ_MTU_SHIFT 16 /* to support TSO/GSO */
#define QFQ_MIN_LMAX 512 /* see qfq_slot_insert */
+#define QFQ_MAX_LMAX (1UL << QFQ_MTU_SHIFT)
#define QFQ_MAX_AGG_CLASSES 8 /* max num classes per aggregate allowed */
@@ -214,9 +215,14 @@ static struct qfq_class *qfq_find_class(struct Qdisc *sch, u32 classid)
return container_of(clc, struct qfq_class, common);
}
+static struct netlink_range_validation lmax_range = {
+ .min = QFQ_MIN_LMAX,
+ .max = QFQ_MAX_LMAX,
+};
+
static const struct nla_policy qfq_policy[TCA_QFQ_MAX + 1] = {
- [TCA_QFQ_WEIGHT] = { .type = NLA_U32 },
- [TCA_QFQ_LMAX] = { .type = NLA_U32 },
+ [TCA_QFQ_WEIGHT] = NLA_POLICY_RANGE(NLA_U32, 1, QFQ_MAX_WEIGHT),
+ [TCA_QFQ_LMAX] = NLA_POLICY_FULL_RANGE(NLA_U32, &lmax_range),
};
/*
@@ -408,26 +414,18 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
}
err = nla_parse_nested_deprecated(tb, TCA_QFQ_MAX, tca[TCA_OPTIONS],
- qfq_policy, NULL);
+ qfq_policy, extack);
if (err < 0)
return err;
- if (tb[TCA_QFQ_WEIGHT]) {
+ if (tb[TCA_QFQ_WEIGHT])
weight = nla_get_u32(tb[TCA_QFQ_WEIGHT]);
- if (!weight || weight > (1UL << QFQ_MAX_WSHIFT)) {
- pr_notice("qfq: invalid weight %u\n", weight);
- return -EINVAL;
- }
- } else
+ else
weight = 1;
- if (tb[TCA_QFQ_LMAX]) {
+ if (tb[TCA_QFQ_LMAX])
lmax = nla_get_u32(tb[TCA_QFQ_LMAX]);
- if (lmax < QFQ_MIN_LMAX || lmax > (1UL << QFQ_MTU_SHIFT)) {
- pr_notice("qfq: invalid max length %u\n", lmax);
- return -EINVAL;
- }
- } else
+ else
lmax = psched_mtu(qdisc_dev(sch));
inv_w = ONE_FP / weight;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next v4 4/4] selftests: tc-testing: add more tests for sch_qfq
2023-04-21 17:53 [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq Pedro Tammela
` (2 preceding siblings ...)
2023-04-21 17:53 ` [PATCH net-next v4 3/4] net/sched: sch_qfq: refactor parsing of netlink parameters Pedro Tammela
@ 2023-04-21 17:53 ` Pedro Tammela
2023-04-22 4:08 ` [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq Jakub Kicinski
4 siblings, 0 replies; 6+ messages in thread
From: Pedro Tammela @ 2023-04-21 17:53 UTC (permalink / raw)
To: netdev
Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
simon.horman, Pedro Tammela
The QFQ qdisc class has parameter bounds that are not being
checked for correctness.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
.../tc-testing/tc-tests/qdiscs/qfq.json | 72 +++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
index 330f1a25e0ab..147899a868d3 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
@@ -46,6 +46,30 @@
"$IP link del dev $DUMMY type dummy"
]
},
+ {
+ "id": "d364",
+ "name": "Test QFQ with max class weight setting",
+ "category": [
+ "qdisc",
+ "qfq"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$IP link add dev $DUMMY type dummy || /bin/true",
+ "$TC qdisc add dev $DUMMY handle 1: root qfq"
+ ],
+ "cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 9999",
+ "expExitCode": "2",
+ "verifyCmd": "$TC class show dev $DUMMY",
+ "matchPattern": "class qfq 1:1 root weight 9999 maxpkt",
+ "matchCount": "0",
+ "teardown": [
+ "$TC qdisc del dev $DUMMY handle 1: root",
+ "$IP link del dev $DUMMY type dummy"
+ ]
+ },
{
"id": "8452",
"name": "Create QFQ with class maxpkt setting",
@@ -70,6 +94,54 @@
"$IP link del dev $DUMMY type dummy"
]
},
+ {
+ "id": "22df",
+ "name": "Test QFQ class maxpkt setting lower bound",
+ "category": [
+ "qdisc",
+ "qfq"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$IP link add dev $DUMMY type dummy || /bin/true",
+ "$TC qdisc add dev $DUMMY handle 1: root qfq"
+ ],
+ "cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq maxpkt 128",
+ "expExitCode": "2",
+ "verifyCmd": "$TC class show dev $DUMMY",
+ "matchPattern": "class qfq 1:1 root weight 1 maxpkt 128",
+ "matchCount": "0",
+ "teardown": [
+ "$TC qdisc del dev $DUMMY handle 1: root",
+ "$IP link del dev $DUMMY type dummy"
+ ]
+ },
+ {
+ "id": "92ee",
+ "name": "Test QFQ class maxpkt setting upper bound",
+ "category": [
+ "qdisc",
+ "qfq"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$IP link add dev $DUMMY type dummy || /bin/true",
+ "$TC qdisc add dev $DUMMY handle 1: root qfq"
+ ],
+ "cmdUnderTest": "$TC class add dev $DUMMY parent 1: classid 1:1 qfq maxpkt 99999",
+ "expExitCode": "2",
+ "verifyCmd": "$TC class show dev $DUMMY",
+ "matchPattern": "class qfq 1:1 root weight 1 maxpkt 99999",
+ "matchCount": "0",
+ "teardown": [
+ "$TC qdisc del dev $DUMMY handle 1: root",
+ "$IP link del dev $DUMMY type dummy"
+ ]
+ },
{
"id": "d920",
"name": "Create QFQ with multiple class setting",
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq
2023-04-21 17:53 [PATCH net-next v4 0/4] net/sched: cleanup parsing prints in htb and qfq Pedro Tammela
` (3 preceding siblings ...)
2023-04-21 17:53 ` [PATCH net-next v4 4/4] selftests: tc-testing: add more tests for sch_qfq Pedro Tammela
@ 2023-04-22 4:08 ` Jakub Kicinski
4 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2023-04-22 4:08 UTC (permalink / raw)
To: Pedro Tammela
Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, pabeni,
simon.horman
On Fri, 21 Apr 2023 14:53:40 -0300 Pedro Tammela wrote:
> 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.
I haven't investigated in detail but doesn't seem to apply:
Applying: net/sched: sch_htb: use extack on errors messages
Applying: net/sched: sch_qfq: use extack on errors messages
Applying: net/sched: sch_qfq: refactor parsing of netlink parameters
error: patch failed: net/sched/sch_qfq.c:408
error: net/sched/sch_qfq.c: patch does not apply
Patch failed at 0003 net/sched: sch_qfq: refactor parsing of netlink parameters
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
fatal: It looks like 'git am' is in progress. Cannot rebase.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 6+ messages in thread