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
> default            192.168.178.1      UGS        5     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       0        0     -     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       0        0     -     1 iwn0 
> 192.168.178.255    192.168.178.52     UHPb       0        0     -     1 iwn0 
> 192.168.178.255    192.168.178.53     UHPb       0        0     -     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
> default            192.168.178.1      UGS        5     4955     -    12 iwn0 
> 192.168.178/24     192.168.178.52     UCn        0       51     -     8 iwn0 
> 192.168.178.52     a4:4e:31:38:70:7c  UHLl       0     3754     -     1 iwn0 
> 192.168.178.255    192.168.178.52     UHb        0        0     -     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 -0000      1.371
+++ sys/net/route.c     19 Feb 2018 09:55:31 -0000
@@ -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 -0000      1.27
+++ regress/sbin/route/Makefile 19 Feb 2018 09:45:43 -0000
@@ -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:d4:c8:1d up
+       ${SUDO} ifconfig vether99 192.168.178.52/24
+       ${SUDO} ifconfig vether99 alias 192.168.178.53/24
+       ${RCMD} add default 192.168.178.1
+       # Remove the alias
+       ${SUDO} ifconfig vether99 192.168.178.53 delete
+       ${RCMD} show -inet 2>&1 | \
+               sed -e "s,link\#[0-9 ]*U,link#              U," | \
+               diff -u ${.CURDIR}/${.TARGET}.ok /dev/stdin
 
 REGRESS_TARGETS=netmask ${RTTEST_TARGETS}
 REGRESS_ROOT_TARGETS=${REGRESS_TARGETS}
Index: regress/sbin/route/rttest33.ok
===================================================================
RCS file: regress/sbin/route/rttest33.ok
diff -N regress/sbin/route/rttest33.ok
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ regress/sbin/route/rttest33.ok      19 Feb 2018 09:57:34 -0000
@@ -0,0 +1,13 @@
+Routing tables
+
+Internet:
+Destination        Gateway            Flags   Refs      Use   Mtu  Prio Iface
+default            192.168.178.1      UGS        0        0     -     8 
vether99
+192.0.2.1          192.0.2.1          UHl        0        0 32768     1 lo10001
+192.0.2.2          192.0.2.2          UHl        0        0 32768     1 lo10002
+192.0.2.3          192.0.2.3          UHl        0        0 32768     1 lo10003
+192.0.2.4          192.0.2.4          UHl        0        0 32768     1 lo10004
+192.168.178/24     192.168.178.52     UCn        1        1     -     4 
vether99
+192.168.178.1      link#              UHLch      1        3     -     3 
vether99
+192.168.178.52     fe:e1:ba:d4:c8:1d  UHLl       0        0     -     1 
vether99
+192.168.178.255    192.168.178.52     UHb        0        0     -     1 
vether99

Reply via email to