| Message ID | 20230719095449.2998778-1-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [PATCHv2,net] ipv6: do not match device when remove source route | expand |
On Wed, Jul 19, 2023 at 05:54:49PM +0800, Hangbin Liu wrote: > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index 64e873f5895f..4f49677e24a2 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -4590,10 +4590,11 @@ static int fib6_remove_prefsrc(struct fib6_info *rt, void *arg) > struct net_device *dev = ((struct arg_dev_net_ip *)arg)->dev; > struct net *net = ((struct arg_dev_net_ip *)arg)->net; > struct in6_addr *addr = ((struct arg_dev_net_ip *)arg)->addr; > + u32 tb6_id = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN; l3mdev_fib_table() handles the case of 'dev' being NULL, so this looks OK. > > if (!rt->nh && > - ((void *)rt->fib6_nh->fib_nh_dev == dev || !dev) && Now that we are not checking the nexthop device, I believe we should remove the '!rt->nh' check. With this check, the source address is not removed from IPv6 routes that are using a nexthop object. This is in contrast to IPv4 [1]. After removing the check, IPv4 and IPv6 are consistent (disregarding the fundamental difference of removing the route vs. only the source address) [2]. > rt != net->ipv6.fib6_null_entry && > + rt->fib6_table->tb6_id == tb6_id && > ipv6_addr_equal(addr, &rt->fib6_prefsrc.addr)) { > spin_lock_bh(&rt6_exception_lock); > /* remove prefsrc entry */ [1] + ip link add name dummy1 up type dummy + ip link add name dummy2 up type dummy + ip -4 nexthop add id 1 dev dummy1 + ip -6 nexthop add id 2 dev dummy1 + ip address add 192.0.2.1/24 dev dummy2 + ip address add 2001:db8:1::1/64 dev dummy2 + ip route add 198.51.100.0/24 nhid 1 src 192.0.2.1 + ip route add 2001:db8:10::/64 nhid 2 src 2001:db8:1::1 + ip -4 route show 198.51.100.0/24 198.51.100.0/24 nhid 1 dev dummy1 src 192.0.2.1 + ip -6 route show 2001:db8:10::/64 2001:db8:10::/64 nhid 2 dev dummy1 src 2001:db8:1::1 metric 1024 pref medium + ip address del 192.0.2.1/24 dev dummy2 + ip address del 2001:db8:1::1/64 dev dummy2 + ip -4 route show 198.51.100.0/24 + ip -6 route show 2001:db8:10::/64 2001:db8:10::/64 nhid 2 dev dummy1 src 2001:db8:1::1 metric 1024 pref medium [2] + ip link add name dummy1 up type dummy + ip link add name dummy2 up type dummy + ip -4 nexthop add id 1 dev dummy1 + ip -6 nexthop add id 2 dev dummy1 + ip address add 192.0.2.1/24 dev dummy2 + ip address add 2001:db8:1::1/64 dev dummy2 + ip route add 198.51.100.0/24 nhid 1 src 192.0.2.1 + ip route add 2001:db8:10::/64 nhid 2 src 2001:db8:1::1 + ip -4 route show 198.51.100.0/24 198.51.100.0/24 nhid 1 dev dummy1 src 192.0.2.1 + ip -6 route show 2001:db8:10::/64 2001:db8:10::/64 nhid 2 dev dummy1 src 2001:db8:1::1 metric 1024 pref medium + ip address del 192.0.2.1/24 dev dummy2 + ip address del 2001:db8:1::1/64 dev dummy2 + ip -4 route show 198.51.100.0/24 + ip -6 route show 2001:db8:10::/64 2001:db8:10::/64 nhid 2 dev dummy1 metric 1024 pref medium
On Wed, Jul 19, 2023 at 05:54:49PM +0800, Hangbin Liu wrote: > +ipv6_del_addr_test() > +{ > + echo > + echo "IPv6 delete address route tests" > + > + setup > + > + set -e > + $IP li add dummy1 type dummy > + $IP li set dummy1 up > + $IP li add dummy2 type dummy > + $IP li set dummy2 up > + $IP li add red type vrf table 1111 > + $IP li set red up > + $IP ro add vrf red unreachable default > + $IP li set dummy2 vrf red > + > + $IP addr add dev dummy1 2001:db8:104::1/64 > + $IP addr add dev dummy1 2001:db8:104::11/64 > + $IP addr add dev dummy1 2001:db8:104::12/64 > + $IP addr add dev dummy1 2001:db8:104::13/64 > + $IP addr add dev dummy2 2001:db8:104::1/64 > + $IP addr add dev dummy2 2001:db8:104::11/64 > + $IP addr add dev dummy2 2001:db8:104::12/64 > + $IP route add 2001:db8:105::/64 via 2001:db8:104::2 src 2001:db8:104::11 > + $IP route add 2001:db8:106::/64 dev lo src 2001:db8:104::12 > + $IP route add table 0 2001:db8:107::/64 via 2001:db8:104::2 src 2001:db8:104::13 > + $IP route add vrf red 2001:db8:105::/64 via 2001:db8:104::2 src 2001:db8:104::11 > + $IP route add vrf red 2001:db8:106::/64 dev lo src 2001:db8:104::12 > + set +e > + > + # removing address from device in vrf should only remove route from vrf table removing address from device in vrf should only remove it as a preferred source address from routes in vrf table > + echo " Regular FIB info" > + > + $IP addr del dev dummy2 2001:db8:104::11/64 > + # Checking if the source address exist instead of the dest subnet s/exist/exists/ > + # as IPv6 only remove the preferred source address, not whole route. s/remove/removes/ > + $IP -6 ro ls vrf red | grep -q 2001:db8:104::11 I prefer "src 2001:db8:104::11". Same in other places. > + log_test $? 1 "Route removed from VRF when source address deleted" Unlike IPv4, the route is not removed so maybe "Preferred source address removed ..." (or something similar). Same in other places. > + > + $IP -6 ro ls | grep -q 2001:db8:104::11 > + log_test $? 0 "Route in default VRF not removed" > + > + $IP addr add dev dummy2 2001:db8:104::11/64 > + $IP route replace vrf red 2001:db8:105::/64 via 2001:db8:104::2 src 2001:db8:104::11 > + > + $IP addr del dev dummy1 2001:db8:104::11/64 > + $IP -6 ro ls | grep -q 2001:db8:104::11 > + log_test $? 1 "Route removed in default VRF when source address deleted" > + > + $IP -6 ro ls vrf red | grep -q 2001:db8:104::11 > + log_test $? 0 "Route in VRF is not removed by address delete" > + > + # removing address from device in vrf should only remove route from vrf > + # table even when the associated fib info only differs in table ID Likewise, route not remove. > + echo " Identical FIB info with different table ID" > + > + $IP addr del dev dummy2 2001:db8:104::12/64 > + $IP -6 ro ls vrf red | grep -q 2001:db8:104::12 > + log_test $? 1 "Route removed from VRF when source address deleted" > + > + $IP -6 ro ls | grep -q 2001:db8:104::12 > + log_test $? 0 "Route in default VRF not removed" > + > + $IP addr add dev dummy2 2001:db8:104::12/64 > + $IP route replace vrf red 2001:db8:106::/64 dev lo src 2001:db8:104::12 > + > + $IP addr del dev dummy1 2001:db8:104::12/64 > + $IP -6 ro ls | grep -q 2001:db8:104::12 > + log_test $? 1 "Route removed in default VRF when source address deleted" > + > + $IP -6 ro ls vrf red | grep -q 2001:db8:104::12 > + log_test $? 0 "Route in VRF is not removed by address delete" > + > + # removing address from device in default vrf should remove route from > + # the default vrf even when route was inserted with a table ID of 0. Likewise. > + echo " Table ID 0" > + > + $IP addr del dev dummy1 2001:db8:104::13/64 > + $IP -6 ro ls | grep -q 2001:db8:104::13 > + log_test $? 1 "Route removed in default VRF when source address deleted" > + > + $IP li del dummy1 > + $IP li del dummy2 > + cleanup > +}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 64e873f5895f..4f49677e24a2 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -4590,10 +4590,11 @@ static int fib6_remove_prefsrc(struct fib6_info *rt, void *arg) struct net_device *dev = ((struct arg_dev_net_ip *)arg)->dev; struct net *net = ((struct arg_dev_net_ip *)arg)->net; struct in6_addr *addr = ((struct arg_dev_net_ip *)arg)->addr; + u32 tb6_id = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN; if (!rt->nh && - ((void *)rt->fib6_nh->fib_nh_dev == dev || !dev) && rt != net->ipv6.fib6_null_entry && + rt->fib6_table->tb6_id == tb6_id && ipv6_addr_equal(addr, &rt->fib6_prefsrc.addr)) { spin_lock_bh(&rt6_exception_lock); /* remove prefsrc entry */ diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh index 35d89dfa6f11..0c24ea973598 100755 --- a/tools/testing/selftests/net/fib_tests.sh +++ b/tools/testing/selftests/net/fib_tests.sh @@ -9,7 +9,7 @@ ret=0 ksft_skip=4 # all tests in this script. Can be overridden with -t option -TESTS="unregister down carrier nexthop suppress ipv6_notify ipv4_notify ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter ipv4_del_addr ipv4_mangle ipv6_mangle ipv4_bcast_neigh" +TESTS="unregister down carrier nexthop suppress ipv6_notify ipv4_notify ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter ipv4_del_addr ipv6_del_addr ipv4_mangle ipv6_mangle ipv4_bcast_neigh" VERBOSE=0 PAUSE_ON_FAIL=no @@ -1869,6 +1869,93 @@ ipv4_del_addr_test() cleanup } +ipv6_del_addr_test() +{ + echo + echo "IPv6 delete address route tests" + + setup + + set -e + $IP li add dummy1 type dummy + $IP li set dummy1 up + $IP li add dummy2 type dummy + $IP li set dummy2 up + $IP li add red type vrf table 1111 + $IP li set red up + $IP ro add vrf red unreachable default + $IP li set dummy2 vrf red + + $IP addr add dev dummy1 2001:db8:104::1/64 + $IP addr add dev dummy1 2001:db8:104::11/64 + $IP addr add dev dummy1 2001:db8:104::12/64 + $IP addr add dev dummy1 2001:db8:104::13/64 + $IP addr add dev dummy2 2001:db8:104::1/64 + $IP addr add dev dummy2 2001:db8:104::11/64 + $IP addr add dev dummy2 2001:db8:104::12/64 + $IP route add 2001:db8:105::/64 via 2001:db8:104::2 src 2001:db8:104::11 + $IP route add 2001:db8:106::/64 dev lo src 2001:db8:104::12 + $IP route add table 0 2001:db8:107::/64 via 2001:db8:104::2 src 2001:db8:104::13 + $IP route add vrf red 2001:db8:105::/64 via 2001:db8:104::2 src 2001:db8:104::11 + $IP route add vrf red 2001:db8:106::/64 dev lo src 2001:db8:104::12 + set +e + + # removing address from device in vrf should only remove route from vrf table + echo " Regular FIB info" + + $IP addr del dev dummy2 2001:db8:104::11/64 + # Checking if the source address exist instead of the dest subnet + # as IPv6 only remove the preferred source address, not whole route. + $IP -6 ro ls vrf red | grep -q 2001:db8:104::11 + log_test $? 1 "Route removed from VRF when source address deleted" + + $IP -6 ro ls | grep -q 2001:db8:104::11 + log_test $? 0 "Route in default VRF not removed" + + $IP addr add dev dummy2 2001:db8:104::11/64 + $IP route replace vrf red 2001:db8:105::/64 via 2001:db8:104::2 src 2001:db8:104::11 + + $IP addr del dev dummy1 2001:db8:104::11/64 + $IP -6 ro ls | grep -q 2001:db8:104::11 + log_test $? 1 "Route removed in default VRF when source address deleted" + + $IP -6 ro ls vrf red | grep -q 2001:db8:104::11 + log_test $? 0 "Route in VRF is not removed by address delete" + + # removing address from device in vrf should only remove route from vrf + # table even when the associated fib info only differs in table ID + echo " Identical FIB info with different table ID" + + $IP addr del dev dummy2 2001:db8:104::12/64 + $IP -6 ro ls vrf red | grep -q 2001:db8:104::12 + log_test $? 1 "Route removed from VRF when source address deleted" + + $IP -6 ro ls | grep -q 2001:db8:104::12 + log_test $? 0 "Route in default VRF not removed" + + $IP addr add dev dummy2 2001:db8:104::12/64 + $IP route replace vrf red 2001:db8:106::/64 dev lo src 2001:db8:104::12 + + $IP addr del dev dummy1 2001:db8:104::12/64 + $IP -6 ro ls | grep -q 2001:db8:104::12 + log_test $? 1 "Route removed in default VRF when source address deleted" + + $IP -6 ro ls vrf red | grep -q 2001:db8:104::12 + log_test $? 0 "Route in VRF is not removed by address delete" + + # removing address from device in default vrf should remove route from + # the default vrf even when route was inserted with a table ID of 0. + echo " Table ID 0" + + $IP addr del dev dummy1 2001:db8:104::13/64 + $IP -6 ro ls | grep -q 2001:db8:104::13 + log_test $? 1 "Route removed in default VRF when source address deleted" + + $IP li del dummy1 + $IP li del dummy2 + cleanup +} + ipv4_route_v6_gw_test() { @@ -2211,6 +2298,7 @@ do ipv6_addr_metric) ipv6_addr_metric_test;; ipv4_addr_metric) ipv4_addr_metric_test;; ipv4_del_addr) ipv4_del_addr_test;; + ipv6_del_addr) ipv6_del_addr_test;; ipv6_route_metrics) ipv6_route_metrics_test;; ipv4_route_metrics) ipv4_route_metrics_test;; ipv4_route_v6_gw) ipv4_route_v6_gw_test;;
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 Ido notified that there is a commit 5a56a0b3a45d ("net: Don't delete routes in different VRFs") to not affect the route in different VRFs. So let's only remove the rt dev checking and add an table id checking. I also added a ipv6_del_addr test for fib_tests.sh. Note that instead of removing the whole route for IPv4, IPv6 only remove the preferred source address for source routing. So in the testing I use "grep -q $src_ipv6_address" instead of "grep -q $dst_ipv6_subnet/64" when checking if the source route deleted. Here is the fib_tests.sh ipv6_del_addr test result. Before fix: IPv6 delete address route tests Regular FIB info TEST: Route removed from VRF when source address deleted [ OK ] TEST: Route in default VRF not removed [FAIL] TEST: Route removed in default VRF when source address deleted [ OK ] TEST: Route in VRF is not removed by address delete [FAIL] Identical FIB info with different table ID TEST: Route removed from VRF when source address deleted [ OK ] TEST: Route in default VRF not removed [FAIL] TEST: Route removed in default VRF when source address deleted [ OK ] TEST: Route in VRF is not removed by address delete [FAIL] Table ID 0 TEST: Route removed in default VRF when source address deleted [ OK ] After fix: IPv6 delete address route tests Regular FIB info TEST: Route removed from VRF when source address deleted [ OK ] TEST: Route in default VRF not removed [ OK ] TEST: Route removed in default VRF when source address deleted [ OK ] TEST: Route in VRF is not removed by address delete [ OK ] Identical FIB info with different table ID TEST: Route removed from VRF when source address deleted [ OK ] TEST: Route in default VRF not removed [ OK ] TEST: Route removed in default VRF when source address deleted [ OK ] TEST: Route in VRF is not removed by address delete [ OK ] Table ID 0 TEST: Route removed in default VRF when source address deleted [ OK ] 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> --- v2: checking table id and update fib_test.sh --- net/ipv6/route.c | 3 +- tools/testing/selftests/net/fib_tests.sh | 90 +++++++++++++++++++++++- 2 files changed, 91 insertions(+), 2 deletions(-)