Skip to content

Commit

Permalink
Merge branch 'net-sched-parsing-prints'
Browse files Browse the repository at this point in the history 
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 <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Apr 23, 2023
2 parents fadfc57 + 7eb060a commit 3951adc
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 26 deletions.
17 changes: 9 additions & 8 deletions net/sched/sch_htb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);

Expand Down
34 changes: 16 additions & 18 deletions net/sched/sch_qfq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand Up @@ -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),
};

/*
Expand Down Expand Up @@ -402,35 +408,26 @@ 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;
}

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])
lmax = nla_get_u32(tb[TCA_QFQ_LMAX]);
else
lmax = psched_mtu(qdisc_dev(sch));

if (lmax < QFQ_MIN_LMAX || lmax > (1UL << QFQ_MTU_SHIFT)) {
pr_notice("qfq: invalid max length %u\n", lmax);
return -EINVAL;
}

inv_w = ONE_FP / weight;
weight = ONE_FP / inv_w;

Expand All @@ -442,8 +439,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;
}

Expand Down
72 changes: 72 additions & 0 deletions tools/testing/selftests/tc-testing/tc-tests/qdiscs/qfq.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down

0 comments on commit 3951adc

Please sign in to comment.