[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <willemdebruijn.kernel.1b99d2d13dcba@gmail.com>
Date: Mon, 01 Dec 2025 12:57:20 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>,
Toke Høiland-Jørgensen <toke@...e.dk>,
Jamal Hadi Salim <jhs@...atatu.com>,
Cong Wang <xiyou.wangcong@...il.com>,
Jiri Pirko <jiri@...nulli.us>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>
Cc: Jonas Köppeler <j.koeppeler@...berlin.de>,
cake@...ts.bufferbloat.net,
netdev@...r.kernel.org,
Toke Høiland-Jørgensen <toke@...hat.com>
Subject: Re: [PATCH net-next v3 1/5] net/sched: Export mq functions for reuse
Toke Høiland-Jørgensen wrote:
> To enable the cake_mq qdisc to reuse code from the mq qdisc, export a
> bunch of functions from sch_mq. Split common functionality out from some
> functions so it can be composed with other code, and export other
> functions wholesale.
>
> No functional change intended.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
> include/net/sch_generic.h | 19 +++++++++++++
> net/sched/sch_mq.c | 69 ++++++++++++++++++++++++++++++++---------------
> 2 files changed, 67 insertions(+), 21 deletions(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index c3a7268b567e..f2281914d962 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
We probably want to avoid random users. This may be better suited to a
local header, similar to net/core/devmem.h.
I don't mean to derail this feature if these are the only concerns.
This can be revised later in -rcX too.
> @@ -1419,7 +1419,26 @@ void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
> void mini_qdisc_pair_block_init(struct mini_Qdisc_pair *miniqp,
> struct tcf_block *block);
>
> +struct mq_sched {
> + struct Qdisc **qdiscs;
> +};
> +
> +int mq_init_common(struct Qdisc *sch, struct nlattr *opt,
> + struct netlink_ext_ack *extack,
> + const struct Qdisc_ops *qdisc_ops);
> +void mq_destroy_common(struct Qdisc *sch);
> +void mq_attach(struct Qdisc *sch);
> void mq_change_real_num_tx(struct Qdisc *sch, unsigned int new_real_tx);
> +void mq_dump_common(struct Qdisc *sch, struct sk_buff *skb);
> +struct netdev_queue *mq_select_queue(struct Qdisc *sch,
> + struct tcmsg *tcm);
> +struct Qdisc *mq_leaf(struct Qdisc *sch, unsigned long cl);
> +unsigned long mq_find(struct Qdisc *sch, u32 classid);
> +int mq_dump_class(struct Qdisc *sch, unsigned long cl,
> + struct sk_buff *skb, struct tcmsg *tcm);
> +int mq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
> + struct gnet_dump *d);
> +void mq_walk(struct Qdisc *sch, struct qdisc_walker *arg);
>
> int sch_frag_xmit_hook(struct sk_buff *skb, int (*xmit)(struct sk_buff *skb));
>
> diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
> index c860119a8f09..0bcabdcd1f44 100644
> --- a/net/sched/sch_mq.c
> +++ b/net/sched/sch_mq.c
> @@ -17,10 +17,6 @@
> #include <net/pkt_sched.h>
> #include <net/sch_generic.h>
>
> -struct mq_sched {
> - struct Qdisc **qdiscs;
> -};
> -
> static int mq_offload(struct Qdisc *sch, enum tc_mq_command cmd)
> {
> struct net_device *dev = qdisc_dev(sch);
> @@ -49,23 +45,29 @@ static int mq_offload_stats(struct Qdisc *sch)
> return qdisc_offload_dump_helper(sch, TC_SETUP_QDISC_MQ, &opt);
> }
>
> -static void mq_destroy(struct Qdisc *sch)
> +void mq_destroy_common(struct Qdisc *sch)
> {
> struct net_device *dev = qdisc_dev(sch);
> struct mq_sched *priv = qdisc_priv(sch);
> unsigned int ntx;
>
> - mq_offload(sch, TC_MQ_DESTROY);
> -
> if (!priv->qdiscs)
> return;
> for (ntx = 0; ntx < dev->num_tx_queues && priv->qdiscs[ntx]; ntx++)
> qdisc_put(priv->qdiscs[ntx]);
> kfree(priv->qdiscs);
> }
> +EXPORT_SYMBOL(mq_destroy_common);
On a similar note, this would be a good use of EXPORT_SYMBOL_NS_GPL.
Maybe not even NETDEV_INTERNAL but a dedicated NET_SCHED_MQ.
> -static int mq_init(struct Qdisc *sch, struct nlattr *opt,
> - struct netlink_ext_ack *extack)
> +static void mq_destroy(struct Qdisc *sch)
> +{
> + mq_offload(sch, TC_MQ_DESTROY);
> + mq_destroy_common(sch);
> +}
> +
> +int mq_init_common(struct Qdisc *sch, struct nlattr *opt,
> + struct netlink_ext_ack *extack,
> + const struct Qdisc_ops *qdisc_ops)
> {
> struct net_device *dev = qdisc_dev(sch);
> struct mq_sched *priv = qdisc_priv(sch);
> @@ -87,7 +89,8 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt,
>
> for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
> dev_queue = netdev_get_tx_queue(dev, ntx);
> - qdisc = qdisc_create_dflt(dev_queue, get_default_qdisc_ops(dev, ntx),
> + qdisc = qdisc_create_dflt(dev_queue,
> + qdisc_ops ?: get_default_qdisc_ops(dev, ntx),
> TC_H_MAKE(TC_H_MAJ(sch->handle),
> TC_H_MIN(ntx + 1)),
> extack);
> @@ -98,12 +101,24 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt,
> }
>
> sch->flags |= TCQ_F_MQROOT;
> + return 0;
> +}
> +EXPORT_SYMBOL(mq_init_common);
> +
> +static int mq_init(struct Qdisc *sch, struct nlattr *opt,
> + struct netlink_ext_ack *extack)
> +{
> + int ret;
> +
> + ret = mq_init_common(sch, opt, extack, NULL);
> + if (ret)
> + return ret;
>
> mq_offload(sch, TC_MQ_CREATE);
> return 0;
> }
>
> -static void mq_attach(struct Qdisc *sch)
> +void mq_attach(struct Qdisc *sch)
> {
> struct net_device *dev = qdisc_dev(sch);
> struct mq_sched *priv = qdisc_priv(sch);
> @@ -124,8 +139,9 @@ static void mq_attach(struct Qdisc *sch)
> kfree(priv->qdiscs);
> priv->qdiscs = NULL;
> }
> +EXPORT_SYMBOL(mq_attach);
>
> -static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
> +void mq_dump_common(struct Qdisc *sch, struct sk_buff *skb)
> {
> struct net_device *dev = qdisc_dev(sch);
> struct Qdisc *qdisc;
> @@ -152,7 +168,12 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
>
> spin_unlock_bh(qdisc_lock(qdisc));
> }
> +}
> +EXPORT_SYMBOL(mq_dump_common);
>
> +static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
> +{
> + mq_dump_common(sch, skb);
> return mq_offload_stats(sch);
> }
>
> @@ -166,11 +187,12 @@ static struct netdev_queue *mq_queue_get(struct Qdisc *sch, unsigned long cl)
> return netdev_get_tx_queue(dev, ntx);
> }
>
> -static struct netdev_queue *mq_select_queue(struct Qdisc *sch,
> - struct tcmsg *tcm)
> +struct netdev_queue *mq_select_queue(struct Qdisc *sch,
> + struct tcmsg *tcm)
> {
> return mq_queue_get(sch, TC_H_MIN(tcm->tcm_parent));
> }
> +EXPORT_SYMBOL(mq_select_queue);
>
> static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
> struct Qdisc **old, struct netlink_ext_ack *extack)
> @@ -198,14 +220,15 @@ static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
> return 0;
> }
>
> -static struct Qdisc *mq_leaf(struct Qdisc *sch, unsigned long cl)
> +struct Qdisc *mq_leaf(struct Qdisc *sch, unsigned long cl)
> {
> struct netdev_queue *dev_queue = mq_queue_get(sch, cl);
>
> return rtnl_dereference(dev_queue->qdisc_sleeping);
> }
> +EXPORT_SYMBOL(mq_leaf);
>
> -static unsigned long mq_find(struct Qdisc *sch, u32 classid)
> +unsigned long mq_find(struct Qdisc *sch, u32 classid)
> {
> unsigned int ntx = TC_H_MIN(classid);
>
> @@ -213,9 +236,10 @@ static unsigned long mq_find(struct Qdisc *sch, u32 classid)
> return 0;
> return ntx;
> }
> +EXPORT_SYMBOL(mq_find);
>
> -static int mq_dump_class(struct Qdisc *sch, unsigned long cl,
> - struct sk_buff *skb, struct tcmsg *tcm)
> +int mq_dump_class(struct Qdisc *sch, unsigned long cl,
> + struct sk_buff *skb, struct tcmsg *tcm)
> {
> struct netdev_queue *dev_queue = mq_queue_get(sch, cl);
>
> @@ -224,9 +248,10 @@ static int mq_dump_class(struct Qdisc *sch, unsigned long cl,
> tcm->tcm_info = rtnl_dereference(dev_queue->qdisc_sleeping)->handle;
> return 0;
> }
> +EXPORT_SYMBOL(mq_dump_class);
>
> -static int mq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
> - struct gnet_dump *d)
> +int mq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
> + struct gnet_dump *d)
> {
> struct netdev_queue *dev_queue = mq_queue_get(sch, cl);
>
> @@ -236,8 +261,9 @@ static int mq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
> return -1;
> return 0;
> }
> +EXPORT_SYMBOL(mq_dump_class_stats);
>
> -static void mq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
> +void mq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
> {
> struct net_device *dev = qdisc_dev(sch);
> unsigned int ntx;
> @@ -251,6 +277,7 @@ static void mq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
> break;
> }
> }
> +EXPORT_SYMBOL(mq_walk);
>
> static const struct Qdisc_class_ops mq_class_ops = {
> .select_queue = mq_select_queue,
>
> --
> 2.52.0
>
Powered by blists - more mailing lists