[PATCH net] ipv6: do not match device when remove source route

[PATCH net] ipv6: do not match device when remove source route

[Hangbin Liu]

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(-)

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,
 	};
-- 
2.38.1

Re: [PATCH net] ipv6: do not match device when remove source route

[Ido Schimmel]

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
> 
> 

Re: [PATCH net] ipv6: do not match device when remove source route

[Hangbin Liu]

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

Re: [PATCH net] ipv6: do not match device when remove source route

[Hangbin Liu]

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