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

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]

Date:   Mon, 26 Feb 2018 16:32:28 -0800
From:   Wei Wang <weiwan@...gle.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Ido Schimmel <idosch@...sch.org>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Martin KaFai Lau <kafai@...com>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>
Subject: Re: [PATCH RFC net-next 16/20] net/ipv6: Cleanup exception route handling

On Mon, Feb 26, 2018 at 3:02 PM, David Ahern <dsahern@...il.com> wrote:
> On 2/26/18 3:29 PM, Wei Wang wrote:
>> On Sun, Feb 25, 2018 at 11:47 AM, David Ahern <dsahern@...il.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@...il.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.
>
Right. Missed those.

>
> 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.
Sure. Sounds good to me.