[PATCH net-next 0/3] vrf: A few cleanups

[PATCH net-next 0/3] vrf: A few cleanups

[Ido Schimmel]

Perform a few cleanups in the VRF driver. Noticed these while reviewing
a recent patch [1]. See individual patches for more details.

[1] https://lore.kernel.org/netdev/20260310105331.2371-1-lirongqing@baidu.com/

Ido Schimmel (3):
  vrf: Remove unnecessary NULL check
  vrf: Use dst_dev_put() instead of using loopback device
  vrf: Remove unnecessary synchronize_rcu() invocations

 drivers/net/vrf.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

-- 
2.53.0

[PATCH net-next 1/3] vrf: Remove unnecessary NULL check

[Ido Schimmel]

The VRF driver always allocates an IPv4 dst entry for a VRF device and
prevents the device from being registered if the allocation fails.

Therefore, there is no need to check if the entry exists when tearing
down a VRF device. Remove the check.

Note that the same is not true for the IPv6 dst entry. Its creation can
be skipped if IPv6 is administratively disabled (i.e.,
'ipv6.disable=1').

Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/vrf.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 8c009bcaa8e7..0952ab6a2571 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1007,13 +1007,11 @@ static void vrf_rtable_release(struct net_device *dev, struct net_vrf *vrf)
 	/* move dev in dst's to loopback so this VRF device can be deleted
 	 * - based on dst_ifdown
 	 */
-	if (rth) {
-		dst = &rth->dst;
-		netdev_ref_replace(dst->dev, net->loopback_dev,
-				   &dst->dev_tracker, GFP_KERNEL);
-		dst->dev = net->loopback_dev;
-		dst_release(dst);
-	}
+	dst = &rth->dst;
+	netdev_ref_replace(dst->dev, net->loopback_dev,
+			   &dst->dev_tracker, GFP_KERNEL);
+	dst->dev = net->loopback_dev;
+	dst_release(dst);
 }
 
 static int vrf_rtable_create(struct net_device *dev)
-- 
2.53.0

Re: [PATCH net-next 1/3] vrf: Remove unnecessary NULL check

[David Ahern]

On 3/24/26 9:55 AM, Ido Schimmel wrote:
> The VRF driver always allocates an IPv4 dst entry for a VRF device and
> prevents the device from being registered if the allocation fails.
> 
> Therefore, there is no need to check if the entry exists when tearing
> down a VRF device. Remove the check.
> 
> Note that the same is not true for the IPv6 dst entry. Its creation can
> be skipped if IPv6 is administratively disabled (i.e.,
> 'ipv6.disable=1').
> 
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  drivers/net/vrf.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

[PATCH net-next 2/3] vrf: Use dst_dev_put() instead of using loopback device

[Ido Schimmel]

Use dst_dev_put() to clean up the device referenced by the dst entry
instead of partially open coding it. Internally, the helper uses the
blackhole device instead of the loopback device.

Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/vrf.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 0952ab6a2571..bfc9ea91ac20 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -751,21 +751,13 @@ static struct sk_buff *vrf_ip6_out(struct net_device *vrf_dev,
 static void vrf_rt6_release(struct net_device *dev, struct net_vrf *vrf)
 {
 	struct rt6_info *rt6 = rtnl_dereference(vrf->rt6);
-	struct net *net = dev_net(dev);
-	struct dst_entry *dst;
 
 	RCU_INIT_POINTER(vrf->rt6, NULL);
 	synchronize_rcu();
 
-	/* move dev in dst's to loopback so this VRF device can be deleted
-	 * - based on dst_ifdown
-	 */
 	if (rt6) {
-		dst = &rt6->dst;
-		netdev_ref_replace(dst->dev, net->loopback_dev,
-				   &dst->dev_tracker, GFP_KERNEL);
-		dst->dev = net->loopback_dev;
-		dst_release(dst);
+		dst_dev_put(&rt6->dst);
+		dst_release(&rt6->dst);
 	}
 }
 
@@ -998,20 +990,12 @@ static struct sk_buff *vrf_l3_out(struct net_device *vrf_dev,
 static void vrf_rtable_release(struct net_device *dev, struct net_vrf *vrf)
 {
 	struct rtable *rth = rtnl_dereference(vrf->rth);
-	struct net *net = dev_net(dev);
-	struct dst_entry *dst;
 
 	RCU_INIT_POINTER(vrf->rth, NULL);
 	synchronize_rcu();
 
-	/* move dev in dst's to loopback so this VRF device can be deleted
-	 * - based on dst_ifdown
-	 */
-	dst = &rth->dst;
-	netdev_ref_replace(dst->dev, net->loopback_dev,
-			   &dst->dev_tracker, GFP_KERNEL);
-	dst->dev = net->loopback_dev;
-	dst_release(dst);
+	dst_dev_put(&rth->dst);
+	dst_release(&rth->dst);
 }
 
 static int vrf_rtable_create(struct net_device *dev)
-- 
2.53.0

Re: [PATCH net-next 2/3] vrf: Use dst_dev_put() instead of using loopback device

[David Ahern]

On 3/24/26 9:55 AM, Ido Schimmel wrote:
> Use dst_dev_put() to clean up the device referenced by the dst entry
> instead of partially open coding it. Internally, the helper uses the
> blackhole device instead of the loopback device.
> 
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  drivers/net/vrf.c | 24 ++++--------------------
>  1 file changed, 4 insertions(+), 20 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

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

[Ido Schimmel]

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);
-	synchronize_rcu();
 
 	if (rt6) {
 		dst_dev_put(&rt6->dst);
@@ -992,7 +991,6 @@ static void vrf_rtable_release(struct net_device *dev, struct net_vrf *vrf)
 	struct rtable *rth = rtnl_dereference(vrf->rth);
 
 	RCU_INIT_POINTER(vrf->rth, NULL);
-	synchronize_rcu();
 
 	dst_dev_put(&rth->dst);
 	dst_release(&rth->dst);
-- 
2.53.0

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

[David Ahern]

On 3/24/26 9:55 AM, Ido Schimmel 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(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

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

[Eric Dumazet]

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 ?

> -       synchronize_rcu();
>
>         if (rt6) {
>                 dst_dev_put(&rt6->dst);
> @@ -992,7 +991,6 @@ static void vrf_rtable_release(struct net_device *dev, struct net_vrf *vrf)
>         struct rtable *rth = rtnl_dereference(vrf->rth);
>
>         RCU_INIT_POINTER(vrf->rth, NULL);
> -       synchronize_rcu();
>
>         dst_dev_put(&rth->dst);
>         dst_release(&rth->dst);
> --
> 2.53.0
>

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

[David Ahern]

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.

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

[Ido Schimmel]

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.

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

[Eric Dumazet]

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.

Thanks.

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

[Ido Schimmel]

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

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

[David Ahern]

On 3/25/26 2:11 PM, Ido Schimmel wrote:
> 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

LGTM. Thanks for digging into the sequence.