| 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
| ||
|
Message-ID: <20190401182033.khnhgfphfn3rptb4@kafai-mbp.dhcp.thefacebook.com>
Date: Mon, 1 Apr 2019 18:20:35 +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 v2 net-next 2/5] ipv4: Add fib_nh_common to fib_result
On Mon, Apr 01, 2019 at 09:26:00AM -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 | 28 ++++++++++++++++++----
> net/ipv4/fib_trie.c | 13 ++++++-----
> net/ipv4/route.c | 60 ++++++++++++++++++++++++++++++++----------------
> 7 files changed, 102 insertions(+), 65 deletions(-)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 12a6d759cf57..5070bc531ca4 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;
Is nh_sel still needed?
> + 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])
> -#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_NH(res) ((res).nhc)
s/FIB_RES_NH/FIB_RES_NHC/ ?
> +#define FIB_RES_DEV(res) (FIB_RES_NH(res)->nhc_dev)
> +#define FIB_RES_OIF(res) (FIB_RES_NH(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)
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 4a8455757507..199e7d289a7d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4555,11 +4555,11 @@ static int bpf_fib_set_fwd_params(struct bpf_fib_lookup *params,
> static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
> u32 flags, bool check_mtu)
> {
> + struct fib_nh_common *nhc;
> struct in_device *in_dev;
> struct neighbour *neigh;
> struct net_device *dev;
> struct fib_result res;
> - struct fib_nh *nh;
> struct flowi4 fl4;
> int err;
> u32 mtu;
> @@ -4632,15 +4632,15 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
> return BPF_FIB_LKUP_RET_FRAG_NEEDED;
> }
>
> - nh = &res.fi->fib_nh[res.nh_sel];
> + nhc = res.nhc;
>
> /* do not handle lwt encaps right now */
> - if (nh->fib_nh_lws)
> + if (nhc->nhc_lwtstate)
> return BPF_FIB_LKUP_RET_UNSUPP_LWT;
>
> - dev = nh->fib_nh_dev;
> - if (nh->fib_nh_gw4)
> - params->ipv4_dst = nh->fib_nh_gw4;
> + dev = nhc->nhc_dev;
> + if (nhc->nhc_has_gw)
> + params->ipv4_dst = nhc->nhc_gw.ipv4;
>
> params->rt_metric = res.fi->fib_priority;
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index ffbe24397dbe..00edebab2ca0 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -307,7 +307,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
> .flowi4_mark = vmark ? skb->mark : 0,
> };
> if (!fib_lookup(net, &fl4, &res, 0))
> - return FIB_RES_PREFSRC(net, res);
> + return fib_result_prefsrc(net, &res);
> } else {
> scope = RT_SCOPE_LINK;
> }
> @@ -390,7 +390,7 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
>
> dev_match = fib_info_nh_uses_dev(res.fi, dev);
> if (dev_match) {
> - ret = FIB_RES_NH(res).fib_nh_scope >= RT_SCOPE_HOST;
> + ret = FIB_RES_NH(res)->nhc_scope >= RT_SCOPE_HOST;
> return ret;
> }
> if (no_addr)
> @@ -402,7 +402,7 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
> ret = 0;
> if (fib_lookup(net, &fl4, &res, FIB_LOOKUP_IGNORE_LINKSTATE) == 0) {
> if (res.type == RTN_UNICAST)
> - ret = FIB_RES_NH(res).fib_nh_scope >= RT_SCOPE_HOST;
> + ret = FIB_RES_NH(res)->nhc_scope >= RT_SCOPE_HOST;
> }
> return ret;
>
> diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
> index e6ff282bb7f4..7945f0534db7 100644
> --- a/net/ipv4/fib_lookup.h
> +++ b/net/ipv4/fib_lookup.h
> @@ -45,6 +45,7 @@ static inline void fib_result_assign(struct fib_result *res,
> {
> /* we used to play games with refcounts, but we now use RCU */
> res->fi = fi;
> + res->nhc = fib_info_nhc(fi, 0);
> }
>
> struct fib_prop {
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index df777af7e278..f81c7dc7ff59 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -1075,6 +1075,24 @@ __be32 fib_info_update_nh_saddr(struct net *net, struct fib_nh *nh)
> return nh->nh_saddr;
> }
>
> +__be32 fib_result_prefsrc(struct net *net, struct fib_result *res)
> +{
> + struct fib_nh_common *nhc = res->nhc;
> + struct fib_nh *nh;
> +
> + if (res->fi->fib_prefsrc)
> + return res->fi->fib_prefsrc;
> +
> + if (unlikely(nhc->nhc_family != AF_INET))
Comparing with FIB_RES_PREFSRC, it is not immediately obvious
why this test is needed. May be a comment or a commit log message?
> + return inet_select_addr(nhc->nhc_dev, 0, res->fi->fib_scope);
> +
> + nh = container_of(nhc, struct fib_nh, nh_common);
> + if (nh->nh_saddr_genid == atomic_read(&net->ipv4.dev_addr_genid))
> + return nh->nh_saddr;
> +
> + return fib_info_update_nh_saddr(net, nh);
> +}
> +
> static bool fib_valid_prefsrc(struct fib_config *cfg, __be32 fib_prefsrc)
> {
> if (cfg->fc_type != RTN_LOCAL || !cfg->fc_dst ||
> @@ -1762,20 +1780,22 @@ void fib_select_multipath(struct fib_result *res, int hash)
> struct net *net = fi->fib_net;
> bool first = false;
>
> - for_nexthops(fi) {
> + change_nexthops(fi) {
Is it mainly because of the 'const'?
> if (net->ipv4.sysctl_fib_multipath_use_neigh) {
> - if (!fib_good_nh(nh))
> + if (!fib_good_nh(nexthop_nh))
> continue;
> if (!first) {
> res->nh_sel = nhsel;
> + res->nhc = &nexthop_nh->nh_common;
> first = true;
> }
> }
>
> - if (hash > atomic_read(&nh->fib_nh_upper_bound))
> + if (hash > atomic_read(&nexthop_nh->fib_nh_upper_bound))
> continue;
>
> res->nh_sel = nhsel;
> + res->nhc = &nexthop_nh->nh_common;
> return;
> } endfor_nexthops(fi);
> }
> @@ -1802,5 +1822,5 @@ void fib_select_path(struct net *net, struct fib_result *res,
>
> check_saddr:
> if (!fl4->saddr)
> - fl4->saddr = FIB_RES_PREFSRC(net, *res);
> + fl4->saddr = fib_result_prefsrc(net, res);
> }
Powered by blists - more mailing lists