Re: [PATCH net-next 3/3] vrf: Remove unnecessary synchronize_rcu() invocations

[Ido Schimmel <idosch@nvidia.com>]

On Wed, Mar 25, 2026 at 07:57:37AM -0700, Eric Dumazet wrote:
> On Wed, Mar 25, 2026 at 7:42 AM Ido Schimmel <idosch@nvidia.com> wrote:
> >
> > On Wed, Mar 25, 2026 at 08:17:30AM -0600, David Ahern wrote:
> > > On 3/24/26 2:39 PM, Eric Dumazet wrote:
> > > > On Tue, Mar 24, 2026 at 8:56 AM Ido Schimmel <idosch@nvidia.com> wrote:
> > > >>
> > > >> vrf_rtable_release() and vrf_rt6_release() are called as part of the VRF
> > > >> device's ndo_uninit() callback. As such, an RCU grace period passes
> > > >> between the removal of the VRF slaves and the closing of the VRF device
> > > >> until these functions are invoked.
> > > >>
> > > >> Therefore, it should not be possible for any concurrent RCU readers to
> > > >> try and acquire a reference on the dst entry while we are potentially
> > > >> releasing the last reference via dst_release().
> > > >>
> > > >> Given the above, the synchronize_rcu() invocations in these functions
> > > >> seem unnecessary and only add unnecessary delay when deleting VRF
> > > >> devices. Remove them.
> > > >>
> > > >> Reviewed-by: Petr Machata <petrm@nvidia.com>
> > > >> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> > > >> ---
> > > >>  drivers/net/vrf.c | 2 --
> > > >>  1 file changed, 2 deletions(-)
> > > >>
> > > >> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> > > >> index bfc9ea91ac20..75edb8eba87f 100644
> > > >> --- a/drivers/net/vrf.c
> > > >> +++ b/drivers/net/vrf.c
> > > >> @@ -753,7 +753,6 @@ static void vrf_rt6_release(struct net_device *dev, struct net_vrf *vrf)
> > > >>         struct rt6_info *rt6 = rtnl_dereference(vrf->rt6);
> > > >>
> > > >>         RCU_INIT_POINTER(vrf->rt6, NULL);
> > > >
> > > > We could argue that the vrf->rt6 clearing is not needed,
> > > > or should be done before the RCU grace period mentioned in the
> > > > changelog started ?
> > > >
> > >
> > > AIUI Ido's point is that the synchronize_net() in
> > > unregister_netdevice_many_notify() occurs before the ndo_uninit call
> > > making the synchronize_rcu here unnecessary. There is also a second
> > > synchronize_net after it.
> > >
> > > I get your point about skipping the clearing the poiniters.
> >
> > We can probably remove more code than just the synchronize_rcu() calls.
> > Looking into that right now.
> 
> This was a minor point, the synchronize_rcu() is far more interesting of course.

Did some more digging. I don't see a reason for keeping the dst pointers
RCU protected. I can replace patch #3 with the following one in v2:

https://github.com/idosch/linux/commit/3fbc1cad610df95cfcde117394c2c3d9e9a04e65.patch