Re: [PATCH net-next v3 1/5] net/sched: Export mq functions for reuse

["Toke Høiland-Jørgensen" <toke@toke.dk>]

Willem de Bruijn <willemdebruijn.kernel@gmail.com> writes:

> 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@redhat.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.

Hmm, right; I just put them in sch_generic.h because that's where the
existing mq_change_real_num_tx() was. I can move them, though, don't
feel strongly about it either way :)

> I don't mean to derail this feature if these are the only concerns.
> This can be revised later in -rcX too.

Sure, let's see if our benevolent maintainers decide to merge this
before or after the merge window; I'll follow up as appropriate.

>> @@ -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.

Huh, didn't know about namespaced exports, neat. Can do that too :)

-Toke