On Tue, Apr 09, 2019 at 02:41:19PM -0700, David Ahern wrote:
> From: David Ahern <dsah...@gmail.com>
>
> Move the nexthop evaluation of a fib entry to a helper that can be
> leveraged for each fib6_nh in a multipath nexthop object.
>
> In the move, 'continue' statements means the helper returns false
> (loop should continue) and 'break' means return true (found the entry
> of interest).
>
> Signed-off-by: David Ahern <dsah...@gmail.com>
> ---
> net/ipv6/route.c | 56
> +++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 33 insertions(+), 23 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 0e8becb1e455..d555edaaff13 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2407,6 +2407,35 @@ void ip6_sk_dst_store_flow(struct sock *sk, struct
> dst_entry *dst,
> NULL);
> }
>
> +static bool ip6_redirect_nh_match(struct fib6_info *f6i,
> + struct fib6_nh *nh,
Why both "struct fib6_info" and "struct fib6_nh" are needed?
I assume nh cannot be obtained from f6i in the later patch
when nh is decoupled from f6i?
> + struct flowi6 *fl6,
> + const struct in6_addr *gw,
> + struct rt6_info **ret)
> +{
> + if (nh->fib_nh_flags & RTNH_F_DEAD || !nh->fib_nh_gw_family ||
> + fl6->flowi6_oif != nh->fib_nh_dev->ifindex)
> + return false;
> +
> + /* rt_cache's gateway might be different from its 'parent'
> + * in the case of an ip redirect.
> + * So we keep searching in the exception table if the gateway
> + * is different.
> + */
> + if (!ipv6_addr_equal(gw, &nh->fib_nh_gw6)) {
> + struct rt6_info *rt_cache;
> +
> + rt_cache = rt6_find_cached_rt(f6i, &fl6->daddr, &fl6->saddr);
> + if (rt_cache &&
> + ipv6_addr_equal(gw, &rt_cache->rt6i_gateway)) {
> + *ret = rt_cache;
> + return true;
> + }
> + return false;
> + }
> + return true;
> +}
> +
> /* Handle redirects */
> struct ip6rd_flowi {
> struct flowi6 fl6;
> @@ -2420,7 +2449,7 @@ static struct rt6_info *__ip6_route_redirect(struct net
> *net,
> int flags)
> {
> struct ip6rd_flowi *rdfl = (struct ip6rd_flowi *)fl6;
> - struct rt6_info *ret = NULL, *rt_cache;
> + struct rt6_info *ret = NULL;
> struct fib6_info *rt;
The "rt" naming is becoming hard to review.
It is fortunate that the type context is kept in this diff.
In this local case, the s/rt/f6i/ rename is quite doable?
It can be the first clean-up step.
> struct fib6_node *fn;
>
> @@ -2438,34 +2467,15 @@ static struct rt6_info *__ip6_route_redirect(struct
> net *net,
> fn = fib6_node_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
> restart:
> for_each_fib6_node_rt_rcu(fn) {
> - if (rt->fib6_nh.fib_nh_flags & RTNH_F_DEAD)
> - continue;
> if (fib6_check_expired(rt))
> continue;
> if (rt->fib6_flags & RTF_REJECT)
> break;
> - if (!rt->fib6_nh.fib_nh_gw_family)
> - continue;
> if (fl6->flowi6_oif != rt->fib6_nh.fib_nh_dev->ifindex)
This check is also copied to the new ip6_redirect_nh_match.
Should this one be removed from here?
> continue;
> - /* rt_cache's gateway might be different from its 'parent'
> - * in the case of an ip redirect.
> - * So we keep searching in the exception table if the gateway
> - * is different.
> - */
> - if (!ipv6_addr_equal(&rdfl->gateway, &rt->fib6_nh.fib_nh_gw6)) {
> - rt_cache = rt6_find_cached_rt(rt,
> - &fl6->daddr,
> - &fl6->saddr);
> - if (rt_cache &&
> - ipv6_addr_equal(&rdfl->gateway,
> - &rt_cache->rt6i_gateway)) {
> - ret = rt_cache;
> - break;
> - }
> - continue;
> - }
> - break;
> + if (ip6_redirect_nh_match(rt, &rt->fib6_nh, fl6,
> + &rdfl->gateway, &ret))
> + goto out;
> }
>
> if (!rt)
> --
> 2.11.0
>
