Re: [PATCH RESEND v2 net-next 02/14] ipv6: Get rid of RTNL for SIOCDELRT and RTM_DELROUTE.

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Paolo Abeni <pabeni@redhat.com>
Date: Wed, 16 Apr 2025 10:49:53 +0200
> On 4/14/25 8:14 PM, Kuniyuki Iwashima wrote:
> > Basically, removing an IPv6 route does not require RTNL because
> > the IPv6 routing tables are protected by per table lock.
> > 
> > inet6_rtm_delroute() calls nexthop_find_by_id() to check if the
> > nexthop specified by RTA_NH_ID exists.  nexthop uses rbtree and
> > the top-down walk can be safely performed under RCU.
> > 
> > ip6_route_del() already relies on RCU and the table lock, but we
> > need to extend the RCU critical section a bit more to cover
> > __ip6_del_rt().  For example, nexthop_for_each_fib6_nh() and
> > inet6_rt_notify() needs RCU.
> 
> The last statement is not clear to me. I don't see __ip6_del_rt()
> calling nexthop_for_each_fib6_nh() or inet6_rt_notify() ?!?

Thank you for review!

It's burried in the depths, and I noticed this from the v1 test result.
https://lore.kernel.org/netdev/Z91yk90LZy9yJexG@mini-arch/

inet6_rtm_delroute
 ip6_route_del
  __ip6_del_rt
   fib6_del
    fib6_del_route
     fib6_purge_rt
      nexthop_for_each_fib6_nh

inet6_rtm_delroute
 ip6_route_del
  __ip6_del_rt
   fib6_del
    fib6_del_route
     inet6_rt_notify


> 
> Also after this patch we have this chunk in ip6_route_del():
> 
> 	table = fib6_get_table(cfg->fc_nlinfo.nl_net, cfg->fc_table);
> 	if (!table)
> 		//..
> 
> 	rcu_read_lock();
> 
> which AFAICS should be safe because 'table' is freed only at netns exit
> time,

Right, and there are few other functions assuming the same thing.

addrconf_get_prefix_route()
rt6_get_route_info()

> but acquiring the rcu lock after grabbing the rcu protected struct
> is confusing. It should be good adding a comment or moving the rcu lock
> before the lookup (and dropping the RCU lock from fib6_get_table())

There are other callers of fib6_get_table(), so I'd move rcu_read_lock()
before it, and will look into them if we can drop it from fib6_get_table().