netdev - Re: [PATCH v3 net-next 2/5] ipv4: Add fib_nh_common to fib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190402171012.5p2hbd377cidkjff@kafai-mbp.dhcp.thefacebook.com>
Date:   Tue, 2 Apr 2019 17:10:15 +0000
From:   Martin Lau <kafai@...com>
To:     David Ahern <dsahern@...nel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "idosch@...lanox.com" <idosch@...lanox.com>,
        "David Ahern" <dsahern@...il.com>
Subject: Re: [PATCH v3 net-next 2/5] ipv4: Add fib_nh_common to fib_result

On Mon, Apr 01, 2019 at 08:02:31PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@...il.com>
> 
> Most of the ipv4 code only needs data from fib_nh_common. Add
> fib_nh_common selection to fib_result and update users to use it.
> 
> Right now, fib_nh_common in fib_result will point to a nexthop within
> a fib_info. Later, fib_nh can point to data with a nexthop struct.
> 
> Signed-off-by: David Ahern <dsahern@...il.com>
> ---
>  include/net/ip_fib.h     | 47 +++++++++++++++++--------------------
>  net/core/filter.c        | 12 +++++-----
>  net/ipv4/fib_frontend.c  |  6 ++---
>  net/ipv4/fib_lookup.h    |  1 +
>  net/ipv4/fib_semantics.c | 25 ++++++++++++++++----
>  net/ipv4/fib_trie.c      | 13 ++++++-----
>  net/ipv4/route.c         | 60 ++++++++++++++++++++++++++++++++----------------
>  7 files changed, 99 insertions(+), 65 deletions(-)
> 
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 12a6d759cf57..1f4a3b8bf584 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -156,15 +156,16 @@ struct fib_rule;
>  
>  struct fib_table;
>  struct fib_result {
> -	__be32		prefix;
> -	unsigned char	prefixlen;
> -	unsigned char	nh_sel;
> -	unsigned char	type;
> -	unsigned char	scope;
> -	u32		tclassid;
> -	struct fib_info *fi;
> -	struct fib_table *table;
> -	struct hlist_head *fa_head;
> +	__be32			prefix;
> +	unsigned char		prefixlen;
> +	unsigned char		nh_sel;
> +	unsigned char		type;
> +	unsigned char		scope;
> +	u32			tclassid;
> +	struct fib_nh_common	*nhc;
> +	struct fib_info		*fi;
> +	struct fib_table	*table;
> +	struct hlist_head	*fa_head;
>  };
>  
>  struct fib_result_nl {
> @@ -182,11 +183,10 @@ struct fib_result_nl {
>  	int             err;
>  };
>  
> -#ifdef CONFIG_IP_ROUTE_MULTIPATH
> -#define FIB_RES_NH(res)		((res).fi->fib_nh[(res).nh_sel])
If nh_sel is still needed, I am likely missing something to understand why
a new '*nhc' pointer is needed.  Is nhc always pointing to the "nh_sel"-ed nh,
like the current FIB_RES_NH() does?

Can you explain more on the commit message:
"Later, fib_nh can point to data with a nexthop struct."

> -#else /* CONFIG_IP_ROUTE_MULTIPATH */
> -#define FIB_RES_NH(res)		((res).fi->fib_nh[0])
> -#endif /* CONFIG_IP_ROUTE_MULTIPATH */
> +static inline struct fib_nh_common *fib_info_nhc(struct fib_info *fi, int nhsel)
> +{
> +	return &fi->fib_nh[nhsel].nh_common;
> +}
>  
>  #ifdef CONFIG_IP_MULTIPLE_TABLES
>  #define FIB_TABLE_HASHSZ 256
> @@ -195,18 +195,11 @@ struct fib_result_nl {
>  #endif
>  
>  __be32 fib_info_update_nh_saddr(struct net *net, struct fib_nh *nh);
> +__be32 fib_result_prefsrc(struct net *net, struct fib_result *res);
>  
> -#define FIB_RES_SADDR(net, res)				\
> -	((FIB_RES_NH(res).nh_saddr_genid ==		\
> -	  atomic_read(&(net)->ipv4.dev_addr_genid)) ?	\
> -	 FIB_RES_NH(res).nh_saddr :			\
> -	 fib_info_update_nh_saddr((net), &FIB_RES_NH(res)))
> -#define FIB_RES_GW(res)			(FIB_RES_NH(res).fib_nh_gw4)
> -#define FIB_RES_DEV(res)		(FIB_RES_NH(res).fib_nh_dev)
> -#define FIB_RES_OIF(res)		(FIB_RES_NH(res).fib_nh_oif)
> -
> -#define FIB_RES_PREFSRC(net, res)	((res).fi->fib_prefsrc ? : \
> -					 FIB_RES_SADDR(net, res))
> +#define FIB_RES_NHC(res)		((res).nhc)
> +#define FIB_RES_DEV(res)	(FIB_RES_NHC(res)->nhc_dev)
> +#define FIB_RES_OIF(res)	(FIB_RES_NHC(res)->nhc_oif)
>  
>  struct fib_entry_notifier_info {
>  	struct fib_notifier_info info; /* must be first */
> @@ -453,10 +446,12 @@ struct fib_table *fib_trie_table(u32 id, struct fib_table *alias);
>  static inline void fib_combine_itag(u32 *itag, const struct fib_result *res)
>  {
>  #ifdef CONFIG_IP_ROUTE_CLASSID
> +	struct fib_nh_common *nhc = res->nhc;
> +	struct fib_nh *nh = container_of(nhc, struct fib_nh, nh_common);
>  #ifdef CONFIG_IP_MULTIPLE_TABLES
>  	u32 rtag;
>  #endif
> -	*itag = FIB_RES_NH(*res).nh_tclassid<<16;
> +	*itag = nh->nh_tclassid << 16;
>  #ifdef CONFIG_IP_MULTIPLE_TABLES
>  	rtag = res->tclassid;
>  	if (*itag == 0)