Re: [PATCH net-next 02/12] net: sched: add tcf_block_cb_alloc()

[Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>]

On Thu, Jun 20, 2019 at 09:49:07PM +0200, Pablo Neira Ayuso wrote:
> Add a new helper function to allocate tcf_block_cb objects.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/net/pkt_cls.h |  8 ++++++++
>  net/sched/cls_api.c   | 23 +++++++++++++++++++----
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 720f2b32fc2f..276a17a3547b 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -72,6 +72,8 @@ static inline struct Qdisc *tcf_block_q(struct tcf_block *block)
>  	return block->q;
>  }
>  
> +struct tcf_block_cb *tcf_block_cb_alloc(tc_setup_cb_t *cb,
> +					void *cb_ident, void *cb_priv);
>  void *tcf_block_cb_priv(struct tcf_block_cb *block_cb);
>  struct tcf_block_cb *tcf_block_cb_lookup(struct tcf_block *block,
>  					 tc_setup_cb_t *cb, void *cb_ident);
> @@ -150,6 +152,12 @@ void tc_setup_cb_block_unregister(struct tcf_block *block, tc_setup_cb_t *cb,
>  {
>  }
>  
> +static inline struct tcf_block_cb *
> +tcf_block_cb_alloc(tc_setup_cb_t *cb, void *cb_ident, void *cb_priv)
> +{
> +	return NULL;

[A] This...

> +}
> +
>  static inline
>  void *tcf_block_cb_priv(struct tcf_block_cb *block_cb)
>  {
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index b2417fda26ec..c01d825edab5 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -746,6 +746,23 @@ unsigned int tcf_block_cb_decref(struct tcf_block_cb *block_cb)
>  }
>  EXPORT_SYMBOL(tcf_block_cb_decref);
>  
> +struct tcf_block_cb *tcf_block_cb_alloc(tc_setup_cb_t *cb,
> +					void *cb_ident, void *cb_priv)
> +{
> +	struct tcf_block_cb *block_cb;
> +
> +	block_cb = kzalloc(sizeof(*block_cb), GFP_KERNEL);
> +	if (!block_cb)
> +		return NULL;
> +
> +	block_cb->cb = cb;
> +	block_cb->cb_ident = cb_ident;
> +	block_cb->cb_priv = cb_priv;
> +
> +	return block_cb;
> +}
> +EXPORT_SYMBOL(tcf_block_cb_alloc);
> +
>  struct tcf_block_cb *__tcf_block_cb_register(struct tcf_block *block,
>  					     tc_setup_cb_t *cb, void *cb_ident,
>  					     void *cb_priv,
> @@ -761,12 +778,10 @@ struct tcf_block_cb *__tcf_block_cb_register(struct tcf_block *block,
>  	if (err)
>  		return ERR_PTR(err);
>  
> -	block_cb = kzalloc(sizeof(*block_cb), GFP_KERNEL);
> +	block_cb = tcf_block_cb_alloc(cb, cb_ident, cb_priv);
>  	if (!block_cb)
>  		return ERR_PTR(-ENOMEM);

... will be translated into -ENOMEM here.
Would be nice to in [A] return ERR_PTR(-EOPNOTSUPP) instead and adjust
the actual implementation as well.
With patch 11, this will be visible to the user then.

> -	block_cb->cb = cb;
> -	block_cb->cb_ident = cb_ident;
> -	block_cb->cb_priv = cb_priv;
> +
>  	list_add(&block_cb->list, &block->cb_list);
>  	return block_cb;
>  }
> -- 
> 2.11.0
>