[PATCH net-next] net: vrf: don't down netif when add slave

[PATCH net-next] net: vrf: don't down netif when add slave

[Menglong Dong]

For now, cycle_netdev() will be called to flush the neighbor cache when
add slave by downing and upping the slave netdev. When the slave has
vlan devices, the data transmission can interrupted.

Optimize it by notifying the NETDEV_CHANGEADDR instead, which will also
flush the neighbor cache. It's a little ugly, and maybe we can introduce
a new event to do such flush :/

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 drivers/net/vrf.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 3ccd649913b5..d90bdf1fe747 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1042,17 +1042,15 @@ static int vrf_rtable_create(struct net_device *dev)
 static void cycle_netdev(struct net_device *dev,
 			 struct netlink_ext_ack *extack)
 {
-	unsigned int flags = dev->flags;
 	int ret;
 
 	if (!netif_running(dev))
 		return;
 
-	ret = dev_change_flags(dev, flags & ~IFF_UP, extack);
-	if (ret >= 0)
-		ret = dev_change_flags(dev, flags, extack);
+	ret = call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+	ret = notifier_to_errno(ret);
 
-	if (ret < 0) {
+	if (ret) {
 		netdev_err(dev,
 			   "Failed to cycle device %s; route tables might be wrong!\n",
 			   dev->name);
-- 
2.50.1

Re: [PATCH net-next] net: vrf: don't down netif when add slave

[Ido Schimmel]

On Thu, Jul 31, 2025 at 07:22:19PM +0800, Menglong Dong wrote:
> For now, cycle_netdev() will be called to flush the neighbor cache when
> add slave by downing and upping the slave netdev. When the slave has
> vlan devices, the data transmission can interrupted.
> 
> Optimize it by notifying the NETDEV_CHANGEADDR instead, which will also
> flush the neighbor cache. It's a little ugly, and maybe we can introduce
> a new event to do such flush :/

Cycling the netdev is not only about neighbors, but also about moving
routes to the correct table (see the comment above the function):

ip link add name dummy1 up type dummy
sysctl -wq net.ipv6.conf.dummy1.keep_addr_on_down=1
ip address add 192.0.2.1/24 dev dummy1
ip address add 2001:db8:1::1/64 dev dummy1
ip link add name vrf1 up type vrf table 100
ip link set dev dummy1 master vrf1
ip -4 route show table 100
192.0.2.0/24 dev dummy1 proto kernel scope link src 192.0.2.1 
local 192.0.2.1 dev dummy1 proto kernel scope host src 192.0.2.1 
broadcast 192.0.2.255 dev dummy1 proto kernel scope link src 192.0.2.1 
ip -6 route show table 100
local 2001:db8:1::1 dev dummy1 proto kernel metric 0 pref medium
2001:db8:1::/64 dev dummy1 proto kernel metric 256 pref medium
local fe80::f877:f7ff:fecb:bfb dev dummy1 proto kernel metric 0 pref medium
fe80::/64 dev dummy1 proto kernel metric 256 pref medium
multicast ff00::/8 dev dummy1 proto kernel metric 256 pref medium

And it doesn't happen with your patch:

ip link add name dummy1 up type dummy
sysctl -wq net.ipv6.conf.dummy1.keep_addr_on_down=1
ip address add 192.0.2.1/24 dev dummy1
ip address add 2001:db8:1::1/64 dev dummy1
ip link add name vrf1 up type vrf table 100
ip link set dev dummy1 master vrf1
ip -4 route show table 100
ip -6 route show table 100

You can try configuring the VLAN devices with "loose_binding on".

Re: [PATCH net-next] net: vrf: don't down netif when add slave

[Menglong Dong]

On Thu, Jul 31, 2025 at 8:08 PM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Thu, Jul 31, 2025 at 07:22:19PM +0800, Menglong Dong wrote:
> > For now, cycle_netdev() will be called to flush the neighbor cache when
> > add slave by downing and upping the slave netdev. When the slave has
> > vlan devices, the data transmission can interrupted.
> >
> > Optimize it by notifying the NETDEV_CHANGEADDR instead, which will also
> > flush the neighbor cache. It's a little ugly, and maybe we can introduce
> > a new event to do such flush :/
>
> Cycling the netdev is not only about neighbors, but also about moving
> routes to the correct table (see the comment above the function):
>
> ip link add name dummy1 up type dummy
> sysctl -wq net.ipv6.conf.dummy1.keep_addr_on_down=1
> ip address add 192.0.2.1/24 dev dummy1
> ip address add 2001:db8:1::1/64 dev dummy1
> ip link add name vrf1 up type vrf table 100
> ip link set dev dummy1 master vrf1
> ip -4 route show table 100
> 192.0.2.0/24 dev dummy1 proto kernel scope link src 192.0.2.1
> local 192.0.2.1 dev dummy1 proto kernel scope host src 192.0.2.1
> broadcast 192.0.2.255 dev dummy1 proto kernel scope link src 192.0.2.1
> ip -6 route show table 100
> local 2001:db8:1::1 dev dummy1 proto kernel metric 0 pref medium
> 2001:db8:1::/64 dev dummy1 proto kernel metric 256 pref medium
> local fe80::f877:f7ff:fecb:bfb dev dummy1 proto kernel metric 0 pref medium
> fe80::/64 dev dummy1 proto kernel metric 256 pref medium
> multicast ff00::/8 dev dummy1 proto kernel metric 256 pref medium
>
> And it doesn't happen with your patch:
>
> ip link add name dummy1 up type dummy
> sysctl -wq net.ipv6.conf.dummy1.keep_addr_on_down=1
> ip address add 192.0.2.1/24 dev dummy1
> ip address add 2001:db8:1::1/64 dev dummy1
> ip link add name vrf1 up type vrf table 100
> ip link set dev dummy1 master vrf1
> ip -4 route show table 100
> ip -6 route show table 100

You are right, something is missing in this patch :/

>
> You can try configuring the VLAN devices with "loose_binding on".

Great! I'll have a try on this param.

Thanks!
Menglong Dong