Skip to content

Commit 3ed3857

Browse files
davem330davem330
committed
Merge branch 'net-sched-prepare-for-more-Qdisc-offloads'
Jakub Kicinski says: ==================== net: sched: prepare for more Qdisc offloads This series refactors the "switchdev" Qdisc offloads a little. We have a few Qdiscs which can be fully offloaded today to the forwarding plane of switching devices. First patch adds a helper for handing statistic dumps, the code seems to be copy pasted between PRIO and RED. Second patch removes unnecessary parameter from RED offload function. Third patch makes the MQ offload use the dump helper which helps it behave much like PRIO and RED when it comes to the TCQ_F_OFFLOADED flag. Patch 4 adds a graft helper, similar to the dump helper. Patch 5 is unrelated to offloads, qdisc_graft() code seemed ripe for a small refactor - no functional changes there. Last two patches move the qdisc_put() call outside of the sch_tree_lock section for RED and PRIO. The child Qdiscs will get removed from the hierarchy under the lock, but having the put (and potentially destroy) called outside of the lock helps offload which may choose to sleep, and it should generally lower the Qdisc change impact. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 80b6265 + 7b8e0b6 commit 3ed3857

File tree

5 files changed

+107
-80
lines changed

5 files changed

+107
-80
lines changed

‎include/net/sch_generic.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,30 @@ void qdisc_put(struct Qdisc *qdisc);
579579
void qdisc_put_unlocked(struct Qdisc *qdisc);
580580
void qdisc_tree_reduce_backlog(struct Qdisc *qdisc, unsigned int n,
581581
unsigned int len);
582+
#ifdef CONFIG_NET_SCHED
583+
int qdisc_offload_dump_helper(struct Qdisc *q, enum tc_setup_type type,
584+
void *type_data);
585+
void qdisc_offload_graft_helper(struct net_device *dev, struct Qdisc *sch,
586+
struct Qdisc *new, struct Qdisc *old,
587+
enum tc_setup_type type, void *type_data,
588+
struct netlink_ext_ack *extack);
589+
#else
590+
static inline int
591+
qdisc_offload_dump_helper(struct Qdisc *q, enum tc_setup_type type,
592+
void *type_data)
593+
{
594+
q->flags &= ~TCQ_F_OFFLOADED;
595+
return 0;
596+
}
597+
598+
static inline void
599+
qdisc_offload_graft_helper(struct net_device *dev, struct Qdisc *sch,
600+
struct Qdisc *new, struct Qdisc *old,
601+
enum tc_setup_type type, void *type_data,
602+
struct netlink_ext_ack *extack)
603+
{
604+
}
605+
#endif
582606
struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
583607
const struct Qdisc_ops *ops,
584608
struct netlink_ext_ack *extack);

‎net/sched/sch_api.c

Lines changed: 64 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,56 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, unsigned int n,
810810
}
811811
EXPORT_SYMBOL(qdisc_tree_reduce_backlog);
812812

813+
int qdisc_offload_dump_helper(struct Qdisc *sch, enum tc_setup_type type,
814+
void *type_data)
815+
{
816+
struct net_device *dev = qdisc_dev(sch);
817+
int err;
818+
819+
sch->flags &= ~TCQ_F_OFFLOADED;
820+
if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
821+
return 0;
822+
823+
err = dev->netdev_ops->ndo_setup_tc(dev, type, type_data);
824+
if (err == -EOPNOTSUPP)
825+
return 0;
826+
827+
if (!err)
828+
sch->flags |= TCQ_F_OFFLOADED;
829+
830+
return err;
831+
}
832+
EXPORT_SYMBOL(qdisc_offload_dump_helper);
833+
834+
void qdisc_offload_graft_helper(struct net_device *dev, struct Qdisc *sch,
835+
struct Qdisc *new, struct Qdisc *old,
836+
enum tc_setup_type type, void *type_data,
837+
struct netlink_ext_ack *extack)
838+
{
839+
bool any_qdisc_is_offloaded;
840+
int err;
841+
842+
if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
843+
return;
844+
845+
err = dev->netdev_ops->ndo_setup_tc(dev, type, type_data);
846+
847+
/* Don't report error if the graft is part of destroy operation. */
848+
if (!err || !new || new == &noop_qdisc)
849+
return;
850+
851+
/* Don't report error if the parent, the old child and the new
852+
* one are not offloaded.
853+
*/
854+
any_qdisc_is_offloaded = new->flags & TCQ_F_OFFLOADED;
855+
any_qdisc_is_offloaded |= sch && sch->flags & TCQ_F_OFFLOADED;
856+
any_qdisc_is_offloaded |= old && old->flags & TCQ_F_OFFLOADED;
857+
858+
if (any_qdisc_is_offloaded)
859+
NL_SET_ERR_MSG(extack, "Offloading graft operation failed.");
860+
}
861+
EXPORT_SYMBOL(qdisc_offload_graft_helper);
862+
813863
static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
814864
u32 portid, u32 seq, u16 flags, int event)
815865
{
@@ -957,7 +1007,6 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
9571007
{
9581008
struct Qdisc *q = old;
9591009
struct net *net = dev_net(dev);
960-
int err = 0;
9611010

9621011
if (parent == NULL) {
9631012
unsigned int i, num_q, ingress;
@@ -1012,28 +1061,29 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
10121061
dev_activate(dev);
10131062
} else {
10141063
const struct Qdisc_class_ops *cops = parent->ops->cl_ops;
1064+
unsigned long cl;
1065+
int err;
10151066

10161067
/* Only support running class lockless if parent is lockless */
10171068
if (new && (new->flags & TCQ_F_NOLOCK) &&
10181069
parent && !(parent->flags & TCQ_F_NOLOCK))
10191070
new->flags &= ~TCQ_F_NOLOCK;
10201071

1021-
err = -EOPNOTSUPP;
1022-
if (cops && cops->graft) {
1023-
unsigned long cl = cops->find(parent, classid);
1072+
if (!cops || !cops->graft)
1073+
return -EOPNOTSUPP;
10241074

1025-
if (cl) {
1026-
err = cops->graft(parent, cl, new, &old,
1027-
extack);
1028-
} else {
1029-
NL_SET_ERR_MSG(extack, "Specified class not found");
1030-
err = -ENOENT;
1031-
}
1075+
cl = cops->find(parent, classid);
1076+
if (!cl) {
1077+
NL_SET_ERR_MSG(extack, "Specified class not found");
1078+
return -ENOENT;
10321079
}
1033-
if (!err)
1034-
notify_and_destroy(net, skb, n, classid, old, new);
1080+
1081+
err = cops->graft(parent, cl, new, &old, extack);
1082+
if (err)
1083+
return err;
1084+
notify_and_destroy(net, skb, n, classid, old, new);
10351085
}
1036-
return err;
1086+
return 0;
10371087
}
10381088

10391089
static int qdisc_block_indexes_set(struct Qdisc *sch, struct nlattr **tca,

‎net/sched/sch_mq.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@ static int mq_offload(struct Qdisc *sch, enum tc_mq_command cmd)
3838
return dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_MQ, &opt);
3939
}
4040

41-
static void mq_offload_stats(struct Qdisc *sch)
41+
static int mq_offload_stats(struct Qdisc *sch)
4242
{
43-
struct net_device *dev = qdisc_dev(sch);
4443
struct tc_mq_qopt_offload opt = {
4544
.command = TC_MQ_STATS,
4645
.handle = sch->handle,
@@ -50,8 +49,7 @@ static void mq_offload_stats(struct Qdisc *sch)
5049
},
5150
};
5251

53-
if (tc_can_offload(dev) && dev->netdev_ops->ndo_setup_tc)
54-
dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_MQ, &opt);
52+
return qdisc_offload_dump_helper(sch, TC_SETUP_QDISC_MQ, &opt);
5553
}
5654

5755
static void mq_destroy(struct Qdisc *sch)
@@ -171,9 +169,8 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
171169

172170
spin_unlock_bh(qdisc_lock(qdisc));
173171
}
174-
mq_offload_stats(sch);
175172

176-
return 0;
173+
return mq_offload_stats(sch);
177174
}
178175

179176
static struct netdev_queue *mq_queue_get(struct Qdisc *sch, unsigned long cl)

‎net/sched/sch_prio.c

Lines changed: 7 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ static int prio_tune(struct Qdisc *sch, struct nlattr *opt,
220220

221221
qdisc_tree_reduce_backlog(child, child->q.qlen,
222222
child->qstats.backlog);
223-
qdisc_put(child);
224223
}
225224

226225
for (i = oldbands; i < q->bands; i++) {
@@ -230,6 +229,9 @@ static int prio_tune(struct Qdisc *sch, struct nlattr *opt,
230229
}
231230

232231
sch_tree_unlock(sch);
232+
233+
for (i = q->bands; i < oldbands; i++)
234+
qdisc_put(q->queues[i]);
233235
return 0;
234236
}
235237

@@ -251,7 +253,6 @@ static int prio_init(struct Qdisc *sch, struct nlattr *opt,
251253

252254
static int prio_dump_offload(struct Qdisc *sch)
253255
{
254-
struct net_device *dev = qdisc_dev(sch);
255256
struct tc_prio_qopt_offload hw_stats = {
256257
.command = TC_PRIO_STATS,
257258
.handle = sch->handle,
@@ -263,21 +264,8 @@ static int prio_dump_offload(struct Qdisc *sch)
263264
},
264265
},
265266
};
266-
int err;
267-
268-
sch->flags &= ~TCQ_F_OFFLOADED;
269-
if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
270-
return 0;
271-
272-
err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_PRIO,
273-
&hw_stats);
274-
if (err == -EOPNOTSUPP)
275-
return 0;
276267

277-
if (!err)
278-
sch->flags |= TCQ_F_OFFLOADED;
279-
280-
return err;
268+
return qdisc_offload_dump_helper(sch, TC_SETUP_QDISC_PRIO, &hw_stats);
281269
}
282270

283271
static int prio_dump(struct Qdisc *sch, struct sk_buff *skb)
@@ -309,43 +297,22 @@ static int prio_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
309297
{
310298
struct prio_sched_data *q = qdisc_priv(sch);
311299
struct tc_prio_qopt_offload graft_offload;
312-
struct net_device *dev = qdisc_dev(sch);
313300
unsigned long band = arg - 1;
314-
bool any_qdisc_is_offloaded;
315-
int err;
316301

317302
if (new == NULL)
318303
new = &noop_qdisc;
319304

320305
*old = qdisc_replace(sch, new, &q->queues[band]);
321306

322-
if (!tc_can_offload(dev))
323-
return 0;
324-
325307
graft_offload.handle = sch->handle;
326308
graft_offload.parent = sch->parent;
327309
graft_offload.graft_params.band = band;
328310
graft_offload.graft_params.child_handle = new->handle;
329311
graft_offload.command = TC_PRIO_GRAFT;
330312

331-
err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_PRIO,
332-
&graft_offload);
333-
334-
/* Don't report error if the graft is part of destroy operation. */
335-
if (err && new != &noop_qdisc) {
336-
/* Don't report error if the parent, the old child and the new
337-
* one are not offloaded.
338-
*/
339-
any_qdisc_is_offloaded = sch->flags & TCQ_F_OFFLOADED;
340-
any_qdisc_is_offloaded |= new->flags & TCQ_F_OFFLOADED;
341-
if (*old)
342-
any_qdisc_is_offloaded |= (*old)->flags &
343-
TCQ_F_OFFLOADED;
344-
345-
if (any_qdisc_is_offloaded)
346-
NL_SET_ERR_MSG(extack, "Offloading graft operation failed.");
347-
}
348-
313+
qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, *old,
314+
TC_SETUP_QDISC_PRIO, &graft_offload,
315+
extack);
349316
return 0;
350317
}
351318

‎net/sched/sch_red.c

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,10 @@ static const struct nla_policy red_policy[TCA_RED_MAX + 1] = {
193193
static int red_change(struct Qdisc *sch, struct nlattr *opt,
194194
struct netlink_ext_ack *extack)
195195
{
196+
struct Qdisc *old_child = NULL, *child = NULL;
196197
struct red_sched_data *q = qdisc_priv(sch);
197198
struct nlattr *tb[TCA_RED_MAX + 1];
198199
struct tc_red_qopt *ctl;
199-
struct Qdisc *child = NULL;
200200
int err;
201201
u32 max_P;
202202

@@ -233,7 +233,7 @@ static int red_change(struct Qdisc *sch, struct nlattr *opt,
233233
if (child) {
234234
qdisc_tree_reduce_backlog(q->qdisc, q->qdisc->q.qlen,
235235
q->qdisc->qstats.backlog);
236-
qdisc_put(q->qdisc);
236+
old_child = q->qdisc;
237237
q->qdisc = child;
238238
}
239239

@@ -252,7 +252,11 @@ static int red_change(struct Qdisc *sch, struct nlattr *opt,
252252
red_start_of_idle_period(&q->vars);
253253

254254
sch_tree_unlock(sch);
255+
255256
red_offload(sch, true);
257+
258+
if (old_child)
259+
qdisc_put(old_child);
256260
return 0;
257261
}
258262

@@ -279,9 +283,8 @@ static int red_init(struct Qdisc *sch, struct nlattr *opt,
279283
return red_change(sch, opt, extack);
280284
}
281285

282-
static int red_dump_offload_stats(struct Qdisc *sch, struct tc_red_qopt *opt)
286+
static int red_dump_offload_stats(struct Qdisc *sch)
283287
{
284-
struct net_device *dev = qdisc_dev(sch);
285288
struct tc_red_qopt_offload hw_stats = {
286289
.command = TC_RED_STATS,
287290
.handle = sch->handle,
@@ -291,22 +294,8 @@ static int red_dump_offload_stats(struct Qdisc *sch, struct tc_red_qopt *opt)
291294
.stats.qstats = &sch->qstats,
292295
},
293296
};
294-
int err;
295-
296-
sch->flags &= ~TCQ_F_OFFLOADED;
297-
298-
if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
299-
return 0;
300-
301-
err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_RED,
302-
&hw_stats);
303-
if (err == -EOPNOTSUPP)
304-
return 0;
305-
306-
if (!err)
307-
sch->flags |= TCQ_F_OFFLOADED;
308297

309-
return err;
298+
return qdisc_offload_dump_helper(sch, TC_SETUP_QDISC_RED, &hw_stats);
310299
}
311300

312301
static int red_dump(struct Qdisc *sch, struct sk_buff *skb)
@@ -324,7 +313,7 @@ static int red_dump(struct Qdisc *sch, struct sk_buff *skb)
324313
};
325314
int err;
326315

327-
err = red_dump_offload_stats(sch, &opt);
316+
err = red_dump_offload_stats(sch);
328317
if (err)
329318
goto nla_put_failure;
330319

0 commit comments

Comments
 (0)