Re: [PATCH v3 net-next 13/13] net: Use common nexthop init and release helpers

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Wed, Mar 27, 2019 at 08:53:58PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> With fib_nh_common in place, move common initialization and release
> code into helpers used by both ipv4 and ipv6. For the moment, the init
> is just the lwt encap and the release is both the netdev reference and
> the the lwt state reference. More will be added later.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  include/net/ip_fib.h     |  4 ++++
>  net/ipv4/fib_semantics.c | 60 +++++++++++++++++++++++++++++++-----------------
>  net/ipv6/route.c         | 21 ++++-------------
>  3 files changed, 48 insertions(+), 37 deletions(-)
> 
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 70548b1a6322..12a6d759cf57 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -441,6 +441,10 @@ int fib_nh_init(struct net *net, struct fib_nh *fib_nh,
>  		struct fib_config *cfg, int nh_weight,
>  		struct netlink_ext_ack *extack);
>  void fib_nh_release(struct net *net, struct fib_nh *fib_nh);
> +int fib_nh_common_init(struct fib_nh_common *nhc, struct nlattr *fc_encap,
> +		       u16 fc_encap_type, void *cfg, gfp_t gfp_flags,
> +		       struct netlink_ext_ack *extack);
> +void fib_nh_common_release(struct fib_nh_common *nhc);
>  
>  /* Exported by fib_trie.c */
>  void fib_trie_init(void);
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index e9992407863e..df777af7e278 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -204,16 +204,22 @@ static void rt_fibinfo_free_cpus(struct rtable __rcu * __percpu *rtp)
>  	free_percpu(rtp);
>  }
>  
> +void fib_nh_common_release(struct fib_nh_common *nhc)
> +{
> +	if (nhc->nhc_dev)
> +		dev_put(nhc->nhc_dev);
> +
> +	lwtstate_put(nhc->nhc_lwtstate);
> +}
> +EXPORT_SYMBOL_GPL(fib_nh_common_release);
> +
>  void fib_nh_release(struct net *net, struct fib_nh *fib_nh)
>  {
>  #ifdef CONFIG_IP_ROUTE_CLASSID
>  	if (fib_nh->nh_tclassid)
>  		net->ipv4.fib_num_tclassid_users--;
>  #endif
> -	if (fib_nh->fib_nh_dev)
> -		dev_put(fib_nh->fib_nh_dev);
> -
> -	lwtstate_put(fib_nh->fib_nh_lws);
> +	fib_nh_common_release(&fib_nh->nh_common);
>  	free_nh_exceptions(fib_nh);
>  	rt_fibinfo_free_cpus(fib_nh->nh_pcpu_rth_output);
>  	rt_fibinfo_free(&fib_nh->nh_rth_input);
> @@ -462,6 +468,30 @@ static int fib_detect_death(struct fib_info *fi, int order,
>  	return 1;
>  }
>  
> +int fib_nh_common_init(struct fib_nh_common *nhc, struct nlattr *encap,
> +		       u16 encap_type, void *cfg, gfp_t gfp_flags,
> +		       struct netlink_ext_ack *extack)
> +{
> +	if (encap) {
> +		struct lwtunnel_state *lwtstate;
> +		int err;
> +
> +		if (encap_type == LWTUNNEL_ENCAP_NONE) {
> +			NL_SET_ERR_MSG(extack, "LWT encap type not specified");
> +			return -EINVAL;
> +		}
> +		err = lwtunnel_build_state(encap_type, encap, nhc->nhc_family,
> +					   cfg, &lwtstate, extack);
> +		if (err)
> +			return err;
> +
> +		nhc->nhc_lwtstate = lwtstate_get(lwtstate);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fib_nh_common_init);
> +
>  int fib_nh_init(struct net *net, struct fib_nh *nh,
>  		struct fib_config *cfg, int nh_weight,
>  		struct netlink_ext_ack *extack)
> @@ -474,22 +504,10 @@ int fib_nh_init(struct net *net, struct fib_nh *nh,
>  	if (!nh->nh_pcpu_rth_output)
>  		goto err_out;
>  
> -	if (cfg->fc_encap) {
> -		struct lwtunnel_state *lwtstate;
> -
> -		err = -EINVAL;
> -		if (cfg->fc_encap_type == LWTUNNEL_ENCAP_NONE) {
> -			NL_SET_ERR_MSG(extack, "LWT encap type not specified");
> -			goto lwt_failure;
> -		}
> -		err = lwtunnel_build_state(cfg->fc_encap_type,
> -					   cfg->fc_encap, AF_INET, cfg,
> -					   &lwtstate, extack);

Init of nhc_family should have been part of this patch instead of previous one,
but it's not a big deal.

patch 5 isn't pretty still and you didn't answer my question
why pointer to fib6_flags is worse than duplicated check.
But it's ok to me in this shape, so for the set:
Acked-by: Alexei Starovoitov <ast@kernel.org>