On 2/28/18 11:44 AM, Martin KaFai Lau wrote:
> On Sun, Feb 25, 2018 at 11:47:28AM -0800, David Ahern wrote:
>> Signed-off-by: David Ahern <dsah...@gmail.com>
>> ---
>> include/net/ip6_fib.h | 4 +-
>> include/net/ip6_route.h | 3 +-
>> net/ipv6/addrconf.c | 31 ++++++---
>> net/ipv6/anycast.c | 7 +-
>> net/ipv6/ip6_fib.c | 50 +++++++++------
>> net/ipv6/ip6_output.c | 3 +-
>> net/ipv6/ndisc.c | 6 +-
>> net/ipv6/route.c | 167
>> +++++++++++++++++-------------------------------
>> 8 files changed, 121 insertions(+), 150 deletions(-)
>>
>> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
>> index 70978deac538..ff16e3d571a2 100644
>> --- a/include/net/ip6_fib.h
>> +++ b/include/net/ip6_fib.h
>> @@ -315,9 +315,7 @@ static inline u32 rt6_get_cookie(const struct rt6_info
>> *rt)
>>
>> if (rt->rt6i_flags & RTF_PCPU ||
>> (unlikely(!list_empty(&rt->rt6i_uncached)) && rt->from))
>> - rt = rt->from;
>> -
>> - rt6_get_cookie_safe(rt, &cookie);
>> + rt6_get_cookie_safe(rt->from, &cookie);
>>
>> return cookie;
>> }
>> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
>> index 24c78fb6ac36..fcda09a58193 100644
>> --- a/include/net/ip6_route.h
>> +++ b/include/net/ip6_route.h
>> @@ -113,8 +113,7 @@ static inline int ip6_route_get_saddr(struct net *net,
>> struct rt6_info *rt,
>> unsigned int prefs,
>> struct in6_addr *saddr)
>> {
>> - struct inet6_dev *idev =
>> - rt ? ip6_dst_idev((struct dst_entry *)rt) : NULL;
>> + struct inet6_dev *idev = rt ? rt->rt6i_idev : NULL;
>> int err = 0;
>>
>> if (rt && rt->rt6i_prefsrc.plen)
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 2a032b932922..4dd7b4e9de4c 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -927,7 +927,7 @@ void inet6_ifa_finish_destroy(struct inet6_ifaddr *ifp)
>> pr_warn("Freeing alive inet6 address %p\n", ifp);
>> return;
>> }
>> - ip6_rt_put(ifp->rt);
>> + fib6_info_release(ifp->rt);
>>
>> kfree_rcu(ifp, rcu);
>> }
>> @@ -1080,6 +1080,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct
>> in6_addr *addr,
>> ifa->cstamp = ifa->tstamp = jiffies;
>> ifa->tokenized = false;
>>
>> + fib6_info_hold(rt);
> Did fib6_info_alloc() already bump the refcnt? Why
> another fib6_info_hold() is needed? Comment would be
> useful here.
The alloc does set it to 1; the extra bump here is because of the
ifa->rt assignment.
Without the additional reference, deleting the route would cause rt to
be freed, yet ifa has a reference to it. I did not want to wade into
changes to the accounting for this set, though I do think it needs to be
revisited.
I'll take another look at whether this is really needed.
>
>> ifa->rt = rt;
>>
>> ifa->idev = idev;
>> @@ -1114,8 +1115,12 @@ ipv6_add_addr(struct inet6_dev *idev, const struct
>> in6_addr *addr,
>> inet6addr_notifier_call_chain(NETDEV_UP, ifa);
>> out:
>> if (unlikely(err < 0)) {
>> - if (rt)
>> - ip6_rt_put(rt);
>> + /* one release for the hold taken when rt is set in ifa
>> + * and a second release for the hold taken on rt create
>> + */
>> + fib6_info_release(rt);
>> + fib6_info_release(rt);
> The extra release corresponds to the above fib6_info_hold()?
yes.
