Re: [PATCH RFC net-next 16/20] net/ipv6: Cleanup exception route handling

[David Ahern <dsahern@gmail.com>]

On 2/26/18 3:29 PM, Wei Wang wrote:
> On Sun, Feb 25, 2018 at 11:47 AM, David Ahern <dsahern@gmail.com> wrote:
>> IPv6 FIB will only contain FIB entries with exception routes added to
>> the FIB entry. Remove CACHE and dst checks from fib6 add and delete since
>> they can never happen once the data type changes.
>>
>> Fixup the lookup functions to use a f6i name for fib lookups and retain
>> the current rt name for return variables.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>>  net/ipv6/ip6_fib.c |  16 +------
>>  net/ipv6/route.c   | 122 ++++++++++++++++++++++++++++++-----------------------
>>  2 files changed, 71 insertions(+), 67 deletions(-)
>>
>> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
>> index 5b03f7e8d850..63a91db61749 100644
>> --- a/net/ipv6/ip6_fib.c
>> +++ b/net/ipv6/ip6_fib.c
>> @@ -1046,7 +1046,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
>>  static void fib6_start_gc(struct net *net, struct rt6_info *rt)
>>  {
>>         if (!timer_pending(&net->ipv6.ip6_fib_timer) &&
>> -           (rt->rt6i_flags & (RTF_EXPIRES | RTF_CACHE)))
>> +           (rt->rt6i_flags & RTF_EXPIRES))
>>                 mod_timer(&net->ipv6.ip6_fib_timer,
>>                           jiffies + net->ipv6.sysctl.ip6_rt_gc_interval);
>>  }
>> @@ -1097,8 +1097,6 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
> 
> This rt here should be f6i?
> 
>>
>>         if (WARN_ON_ONCE(!atomic_read(&rt->dst.__refcnt)))
>>                 return -EINVAL;
>> -       if (WARN_ON_ONCE(rt->rt6i_flags & RTF_CACHE))
>> -               return -EINVAL;
>>
>>         if (info->nlh) {
>>                 if (!(info->nlh->nlmsg_flags & NLM_F_CREATE))
>> @@ -1622,8 +1620,6 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
>>
>>         RT6_TRACE("fib6_del_route\n");
>>
>> -       WARN_ON_ONCE(rt->rt6i_flags & RTF_CACHE);
>> -
>>         /* Unlink it */
>>         *rtp = rt->rt6_next;
> 
> This rt here is also f6i right?
> 
>>         rt->rt6i_node = NULL;
>> @@ -1692,21 +1688,11 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
> 
> This rt here is also f6i right?
> 
>>         struct rt6_info __rcu **rtp;
>>         struct rt6_info __rcu **rtp_next;
>>
>> -#if RT6_DEBUG >= 2
>> -       if (rt->dst.obsolete > 0) {
>> -               WARN_ON(fn);
>> -               return -ENOENT;
>> -       }
>> -#endif
>>         if (!fn || rt == net->ipv6.fib6_null_entry)
>>                 return -ENOENT;
>>
>>         WARN_ON(!(fn->fn_flags & RTN_RTINFO));
>>
>> -       /* remove cached dst from exception table */
>> -       if (rt->rt6i_flags & RTF_CACHE)
>> -               return rt6_remove_exception_rt(rt);
> 
> Could you help delete rt6_remove_exception_rt() function? I don't
> think it is used anymore.

It is still used by ip6_negative_advice, ip6_link_failure and
ip6_del_cached_rt. It can be made static; will fix.


The rest of your comments for this patch are renaming rt to f6i. My
thought is to follow up with another patch that does the rename of rt to
f6i for all fib6_info. Given how large this change is already I did not
want to add extra diffs for that. If there is agreement to fold that
part in now, I can do it.