| Message ID | 20230718065253.2730396-1-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ipv6: do not match device when remove source route | expand |
On Tue, Jul 18, 2023 at 02:52:53PM +0800, Hangbin Liu wrote: > After deleting an IPv6 address on an interface and cleaning up the > related preferred source entries, it is important to ensure that all > routes associated with the deleted address are properly cleared. The > current implementation of rt6_remove_prefsrc() only checks the preferred > source addresses bound to the current device. However, there may be > routes that are bound to other devices but still utilize the same > preferred source address. > > To address this issue, it is necessary to also delete entries that are > bound to other interfaces but share the same source address with the > current device. Failure to delete these entries would leave routes that > are bound to the deleted address unclear. Here is an example reproducer > (I have omitted unrelated routes): [...] > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index 64e873f5895f..ab8c364e323c 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -4607,7 +4607,6 @@ void rt6_remove_prefsrc(struct inet6_ifaddr *ifp) > { > struct net *net = dev_net(ifp->idev->dev); > struct arg_dev_net_ip adni = { > - .dev = ifp->idev->dev, Wouldn't this affect routes in different VRFs? See commit 5a56a0b3a45d ("net: Don't delete routes in different VRFs") and related fixes: 8a2618e14f81 ipv4: Fix incorrect table ID in IOCTL path c0d999348e01 ipv4: Fix incorrect route flushing when table ID 0 is used f96a3d74554d ipv4: Fix incorrect route flushing when source address is deleted e0a312629fef ipv4: Fix table id reference in fib_sync_down_addr Anyway, please add tests to tools/testing/selftests/net/fib_tests.sh > .net = net, > .addr = &ifp->addr, > }; > -- > 2.38.1 > >
On Tue, Jul 18, 2023 at 02:42:02PM +0300, Ido Schimmel wrote: > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > > index 64e873f5895f..ab8c364e323c 100644 > > --- a/net/ipv6/route.c > > +++ b/net/ipv6/route.c > > @@ -4607,7 +4607,6 @@ void rt6_remove_prefsrc(struct inet6_ifaddr *ifp) > > { > > struct net *net = dev_net(ifp->idev->dev); > > struct arg_dev_net_ip adni = { > > - .dev = ifp->idev->dev, > > Wouldn't this affect routes in different VRFs? > > See commit 5a56a0b3a45d ("net: Don't delete routes in different VRFs") > and related fixes: Thanks for this notify. I saw this is for IPv4 only and there is no IPv6 version. I can try add an IPv6 version patch for this issue. The fib_tb_id is based on table id. So in same table we still need to not check the device and remove all source routes. > 8a2618e14f81 ipv4: Fix incorrect table ID in IOCTL path > c0d999348e01 ipv4: Fix incorrect route flushing when table ID 0 is used > f96a3d74554d ipv4: Fix incorrect route flushing when source address is deleted > e0a312629fef ipv4: Fix table id reference in fib_sync_down_addr > > Anyway, please add tests to tools/testing/selftests/net/fib_tests.sh The fib_tests.sh result looks good as my patch affects IPv6 only. # ./fib_tests.sh Single path route test Start point TEST: IPv4 fibmatch [ OK ] TEST: IPv6 fibmatch [ OK ] Nexthop device deleted TEST: IPv4 fibmatch - no route [ OK ] TEST: IPv6 fibmatch - no route [ OK ] [...] IPv4 broadcast neighbour tests TEST: Resolved neighbour for broadcast address [ OK ] TEST: Resolved neighbour for network broadcast address [ OK ] TEST: Unresolved neighbour for broadcast address [ OK ] TEST: Unresolved neighbour for network broadcast address [ OK ] Tests passed: 203 Tests failed: 0 BTW, It's a bit different for IPv4 and IPv6. IPv4 will remove the total source routes, while IPv6 only remove the source address and keep the route. e.g. IPv4: + ip -netns x addr add 192.168.5.5/24 dev net1 + ip -netns x route add 7.7.7.0/24 dev net2 src 192.168.5.5 + ip -netns x -4 route 7.7.7.0/24 dev net2 scope link src 192.168.5.5 192.168.5.0/24 dev net1 proto kernel scope link src 192.168.5.5 + ip -netns x addr del 192.168.5.5/24 dev net1 + ip -netns x -4 route IPv6: + ip addr add 1:2:3:4::5/64 dev dummy1 + ip route add 7:7:7:0::1 dev dummy1 src 1:2:3:4::5 + ip -6 route show 1:2:3:4::/64 dev dummy1 proto kernel metric 256 pref medium 7:7:7::1 dev dummy1 src 1:2:3:4::5 metric 1024 pref medium + ip addr del 1:2:3:4::5/64 dev dummy1 + ip -6 route show 7:7:7::1 dev dummy1 metric 1024 pref medium Thanks Hangbin
On Wed, Jul 19, 2023 at 03:21:48PM +0800, Hangbin Liu wrote: > On Tue, Jul 18, 2023 at 02:42:02PM +0300, Ido Schimmel wrote: > > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > > > index 64e873f5895f..ab8c364e323c 100644 > > > --- a/net/ipv6/route.c > > > +++ b/net/ipv6/route.c > > > @@ -4607,7 +4607,6 @@ void rt6_remove_prefsrc(struct inet6_ifaddr *ifp) > > > { > > > struct net *net = dev_net(ifp->idev->dev); > > > struct arg_dev_net_ip adni = { > > > - .dev = ifp->idev->dev, > > > > Wouldn't this affect routes in different VRFs? > > > > See commit 5a56a0b3a45d ("net: Don't delete routes in different VRFs") > > and related fixes: > > Thanks for this notify. I saw this is for IPv4 only and there is no IPv6 version. > I can try add an IPv6 version patch for this issue. The fib_tb_id is based > on table id. So in same table we still need to not check the device and remove > all source routes. Oh, I saw struct fib6_info has fib6_table, I think we can check this when remove prefsrc. Thanks Hangbin
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 64e873f5895f..ab8c364e323c 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -4607,7 +4607,6 @@ void rt6_remove_prefsrc(struct inet6_ifaddr *ifp) { struct net *net = dev_net(ifp->idev->dev); struct arg_dev_net_ip adni = { - .dev = ifp->idev->dev, .net = net, .addr = &ifp->addr, };
After deleting an IPv6 address on an interface and cleaning up the related preferred source entries, it is important to ensure that all routes associated with the deleted address are properly cleared. The current implementation of rt6_remove_prefsrc() only checks the preferred source addresses bound to the current device. However, there may be routes that are bound to other devices but still utilize the same preferred source address. To address this issue, it is necessary to also delete entries that are bound to other interfaces but share the same source address with the current device. Failure to delete these entries would leave routes that are bound to the deleted address unclear. Here is an example reproducer (I have omitted unrelated routes): + ip link add dummy1 type dummy + ip link add dummy2 type dummy + ip link set dummy1 up + ip link set dummy2 up + ip addr add 1:2:3:4::5/64 dev dummy1 + ip route add 7:7:7:0::1 dev dummy1 src 1:2:3:4::5 + ip route add 7:7:7:0::2 dev dummy2 src 1:2:3:4::5 + ip -6 route show 1:2:3:4::/64 dev dummy1 proto kernel metric 256 pref medium 7:7:7::1 dev dummy1 src 1:2:3:4::5 metric 1024 pref medium 7:7:7::2 dev dummy2 src 1:2:3:4::5 metric 1024 pref medium + ip addr del 1:2:3:4::5/64 dev dummy1 + ip -6 route show 7:7:7::1 dev dummy1 metric 1024 pref medium 7:7:7::2 dev dummy2 src 1:2:3:4::5 metric 1024 pref medium After fix: + ip addr del 1:2:3:4::5/64 dev dummy1 + ip -6 route show 7:7:7::1 dev dummy1 metric 1024 pref medium 7:7:7::2 dev dummy2 metric 1024 pref medium Reported-by: Thomas Haller <thaller@redhat.com> Fixes: c3968a857a6b ("ipv6: RTA_PREFSRC support for ipv6 route source address selection") Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- net/ipv6/route.c | 1 - 1 file changed, 1 deletion(-)