Re: mpath cloning routes and cloned routes
On 02/19/18 11:01, Martin Pieuchot wrote:
On 14/02/18(Wed) 21:53, Florian Riehm wrote:
If we delete cloning routes, we also delete their cloned routes.
This doesn't make sense if we delete a multipath cloning route and may result in
broken gateway routes:
That's a bug!
# netstat -rn | grep 192.168.178
default192.168.178.1 UGS5 4939 -12 iwn0
192.168.178/24 192.168.178.52 UCPn 1 51 - 8 iwn0
192.168.178/24 192.168.178.53 UCPn 00 - 8 iwn0
192.168.178.1 34:31:c4:24:83:d4 UHLch 1 118 - 7 iwn0
192.168.178.52 a4:4e:31:38:70:7c UHLl 0 3749 - 1 iwn0
192.168.178.53 a4:4e:31:38:70:7c UHLl 00 - 1 iwn0
192.168.178.255192.168.178.52 UHPb 00 - 1 iwn0
192.168.178.255192.168.178.53 UHPb 00 - 1 iwn0
As you can see above, iwn0 has 192.168.178.52/24 and 192.168.178.53/24 assigned
and therefore we have 2 mpath cloning routes (P). Their is a cloned route to
192.168.178.1 with RTF_CACHED (h) to reach the default gateway.
# ifconfig iwn0 inet 192.168.178.53 delete
# netstat -rn | grep 192.168.178
default192.168.178.1 UGS5 4955 -12 iwn0
192.168.178/24 192.168.178.52 UCn0 51 - 8 iwn0
192.168.178.52 a4:4e:31:38:70:7c UHLl 0 3754 - 1 iwn0
192.168.178.255192.168.178.52 UHb00 - 1 iwn0
Now 192.168.178.53/24 was deleted, therefore the cloned route to the gateway
(192.168.178.1) is also gone and the default route is 'broken':
# ping 8.8.8.8
# dmesg | tail
arpresolve: 192.168.178.1: route contains no arp information
arpresolve: 192.168.178.1: route contains no arp information
arpresolve: 192.168.178.1: route contains no arp information
arpresolve: 192.168.178.1: route contains no arp information
I think there is no need to delete cloned routes as long as we don't delete
the last cloning route to a network.
Not flushing any RTF_CLONED is a too big hammer and will break setups
where the cloning routes are on two different interfaces.
A better fix is to not delete RTF_CACHED routes until their own parent
is going away. Diff below does that and includes a regression test.
Does that work for you? ok?
Successfully tested.
Thanks & ok
friehm
Index: sys/net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.371
diff -u -p -r1.371 route.c
--- sys/net/route.c 10 Feb 2018 09:17:56 - 1.371
+++ sys/net/route.c 19 Feb 2018 09:55:31 -
@@ -707,26 +707,31 @@ rtequal(struct rtentry *a, struct rtentr
int
rtflushclone1(struct rtentry *rt, void *arg, u_int id)
{
- struct rtentry *parent = arg;
+ struct rtentry *cloningrt = arg;
struct ifnet *ifp;
int error;
- ifp = if_get(rt->rt_ifidx);
+ if (!ISSET(rt->rt_flags, RTF_CLONED))
+ return 0;
+
+ /* Cached route must stay alive as long as their parent are alive. */
+ if (ISSET(rt->rt_flags, RTF_CACHED) && (rt->rt_parent != cloningrt))
+ return 0;
+ if (!rtequal(rt->rt_parent, cloningrt))
+ return 0;
/*
* This happens when an interface with a RTF_CLONING route is
* being detached. In this case it's safe to bail because all
* the routes are being purged by rt_ifa_purge().
*/
+ ifp = if_get(rt->rt_ifidx);
if (ifp == NULL)
return 0;
- if (ISSET(rt->rt_flags, RTF_CLONED) && rtequal(rt->rt_parent, parent)) {
- error = rtdeletemsg(rt, ifp, id);
- if (error == 0)
- error = EAGAIN;
- } else
- error = 0;
+ error = rtdeletemsg(rt, ifp, id);
+ if (error == 0)
+ error = EAGAIN;
if_put(ifp);
return error;
Index: regress/sbin/route/Makefile
===
RCS file: /cvs/src/regress/sbin/route/Makefile,v
retrieving revision 1.27
diff -u -p -r1.27 Makefile
--- regress/sbin/route/Makefile 14 Feb 2018 08:42:22 - 1.27
+++ regress/sbin/route/Makefile 19 Feb 2018 09:45:43 -
@@ -376,7 +376,24 @@ rttest${n}:
${RCMD} add -net 192.168.67.128/25 130.102.71.67 -priority 3 -ifp vlan99
${RCMD} show -inet 2>&1 | \
sed -e "s,link\#[0-9 ]*U,link# U," | \
- cat > ${.CURDIR}/${.TARGET}.ok
+ diff -u ${.CURDIR}/${.TARGET}.ok /dev/stdin
+
+# Check that removing a RTF_CLONING route do not remove children from
+# another cloning route.
+n= 33
+RTTEST_TARGETS+:=rttest${n}
+rttest${n}:
+ # Use vether(4) because we need IFT_ETHER interfaces
+ # for the auto-magic RTF_CLONING routes.
+ ${SUDO} ifconfig vether99 rdomain ${RDOMAIN} lladdr fe:e1:ba:
Re: mpath cloning routes and cloned routes
On 14/02/18(Wed) 21:53, Florian Riehm wrote:
> If we delete cloning routes, we also delete their cloned routes.
> This doesn't make sense if we delete a multipath cloning route and may result
> in
> broken gateway routes:
That's a bug!
> # netstat -rn | grep 192.168.178
> default192.168.178.1 UGS5 4939 -12 iwn0
> 192.168.178/24 192.168.178.52 UCPn 1 51 - 8 iwn0
> 192.168.178/24 192.168.178.53 UCPn 00 - 8 iwn0
> 192.168.178.1 34:31:c4:24:83:d4 UHLch 1 118 - 7 iwn0
> 192.168.178.52 a4:4e:31:38:70:7c UHLl 0 3749 - 1 iwn0
> 192.168.178.53 a4:4e:31:38:70:7c UHLl 00 - 1 iwn0
> 192.168.178.255192.168.178.52 UHPb 00 - 1 iwn0
> 192.168.178.255192.168.178.53 UHPb 00 - 1 iwn0
> As you can see above, iwn0 has 192.168.178.52/24 and 192.168.178.53/24
> assigned
> and therefore we have 2 mpath cloning routes (P). Their is a cloned route to
> 192.168.178.1 with RTF_CACHED (h) to reach the default gateway.
>
> # ifconfig iwn0 inet 192.168.178.53 delete
> # netstat -rn | grep 192.168.178
> default192.168.178.1 UGS5 4955 -12 iwn0
> 192.168.178/24 192.168.178.52 UCn0 51 - 8 iwn0
> 192.168.178.52 a4:4e:31:38:70:7c UHLl 0 3754 - 1 iwn0
> 192.168.178.255192.168.178.52 UHb00 - 1 iwn0
> Now 192.168.178.53/24 was deleted, therefore the cloned route to the gateway
> (192.168.178.1) is also gone and the default route is 'broken':
>
> # ping 8.8.8.8
> # dmesg | tail
> arpresolve: 192.168.178.1: route contains no arp information
> arpresolve: 192.168.178.1: route contains no arp information
> arpresolve: 192.168.178.1: route contains no arp information
> arpresolve: 192.168.178.1: route contains no arp information
>
> I think there is no need to delete cloned routes as long as we don't delete
> the last cloning route to a network.
Not flushing any RTF_CLONED is a too big hammer and will break setups
where the cloning routes are on two different interfaces.
A better fix is to not delete RTF_CACHED routes until their own parent
is going away. Diff below does that and includes a regression test.
Does that work for you? ok?
Index: sys/net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.371
diff -u -p -r1.371 route.c
--- sys/net/route.c 10 Feb 2018 09:17:56 - 1.371
+++ sys/net/route.c 19 Feb 2018 09:55:31 -
@@ -707,26 +707,31 @@ rtequal(struct rtentry *a, struct rtentr
int
rtflushclone1(struct rtentry *rt, void *arg, u_int id)
{
- struct rtentry *parent = arg;
+ struct rtentry *cloningrt = arg;
struct ifnet *ifp;
int error;
- ifp = if_get(rt->rt_ifidx);
+ if (!ISSET(rt->rt_flags, RTF_CLONED))
+ return 0;
+
+ /* Cached route must stay alive as long as their parent are alive. */
+ if (ISSET(rt->rt_flags, RTF_CACHED) && (rt->rt_parent != cloningrt))
+ return 0;
+ if (!rtequal(rt->rt_parent, cloningrt))
+ return 0;
/*
* This happens when an interface with a RTF_CLONING route is
* being detached. In this case it's safe to bail because all
* the routes are being purged by rt_ifa_purge().
*/
+ ifp = if_get(rt->rt_ifidx);
if (ifp == NULL)
return 0;
- if (ISSET(rt->rt_flags, RTF_CLONED) && rtequal(rt->rt_parent, parent)) {
- error = rtdeletemsg(rt, ifp, id);
- if (error == 0)
- error = EAGAIN;
- } else
- error = 0;
+ error = rtdeletemsg(rt, ifp, id);
+ if (error == 0)
+ error = EAGAIN;
if_put(ifp);
return error;
Index: regress/sbin/route/Makefile
===
RCS file: /cvs/src/regress/sbin/route/Makefile,v
retrieving revision 1.27
diff -u -p -r1.27 Makefile
--- regress/sbin/route/Makefile 14 Feb 2018 08:42:22 - 1.27
+++ regress/sbin/route/Makefile 19 Feb 2018 09:45:43 -
@@ -376,7 +376,24 @@ rttest${n}:
${RCMD} add -net 192.168.67.128/25 130.102.71.67 -priority 3 -ifp vlan99
${RCMD} show -inet 2>&1 | \
sed -e "s,link\#[0-9 ]*U,link# U," | \
- cat > ${.CURDIR}/${.TARGET}.ok
+ diff -u ${.CURDIR}/${.TARGET}.ok /dev/stdin
+
+# Check that removing a RTF_CLONING route do not remove children from
+# another cloning route.
+n= 33
+RTTEST_TARGETS+:=rttest${n}
+rttest${n}:
+ # Use vether(4) because we need IFT_ETHER interfaces
+ # for the auto-magic RTF_CLONING routes.
+ ${SUDO} ifconfig vether99 rdomain ${RDOMAIN} lladdr fe:
