On Fri, Feb 15, 2019 at 06:06:05PM +0100, [email protected] wrote:
>
> Hello,
>
> the following change lead to a kernerl panic after running
> /usr/src/regress/usr.sbin/bgpd/integrationtests/network_statement.sh
>
> https://cvsweb.openbsd.org/src/sys/net/route.c?rev=1.381&content-type=text/x-cvsweb-markup
>
> > CVSROOT: /cvs
> > Module name: src
> > Changes by: [email protected] 2019/02/13 16:47:43
> >
> > Modified files:
> > sys/net : if_mpe.c if_mpw.c route.c route.h
> > sys/netinet : in.c ip_mroute.c
> > sys/netinet6 : in6.c in6_ifattach.c ip6_mroute.c
> > share/man/man9 : rt_ifa_add.9
> >
> > Log message:
> > change rt_ifa_add and rt_ifa_del so they take an rdomain argument.
> >
> > this allows mpls interfaces (mpe, mpw) to pass the rdomain they
> > wish the local label to be in, rather than have it implicitly forced
> > to 0 by these functions. right now they'll pass 0, but it will soon
> > be possible to have them rx packets in other rdomains.
> >
> > previously the functions used ifp->if_rdomain for the rdomain.
> > everything other than mpls still passes ifp->if_rdomain.
> >
> > ok mpi@
>
> the Panic:
>
> panic: kernel diagnostic assertion "rdomain == rtable_l2(rdomain)" fail
> ed: file "/sys/net/route.c", line 1116
> Stopped at db_enter+0x12: popq %r11
> TID PID UID PRFLAGS PFLAGS CPU COMMAND
> *316919 99618 0 0x3 0 1K ifconfig
> db_enter() at db_enter+0x12
> panic() at panic+0x128
> __assert(ffffffff81a98516,ffffffff81a9c30b,45c,ffffffff81a4b70b) at
> __assert+0x
> 28
> rt_ifa_del(ffff80000054f600,200004,ffff80000054f658,b) at rt_ifa_del+0x30b
> rt_ifa_dellocal(ffff80000054f600) at rt_ifa_dellocal+0xc7
> in_purgeaddr(ffff80000054f600) at in_purgeaddr+0xab
> in_ifdetach(ffff80000054a800) at in_ifdetach+0x42
> if_detach(ffff80000054a800) at if_detach+0x11e
> loop_clone_destroy(ffff80000054a800) at loop_clone_destroy+0xb8
> if_clone_destroy(ffff800021ee8c20) at if_clone_destroy+0x10c
> ifioctl(fffffd8279ccdac0,80206979,ffff800021ee8c20,ffff800021fc5540) at
> ifioctl
> +0x1f4
> sys_ioctl(ffff800021fc5540,ffff800021ee8d50,ffff800021ee8d40) at
> sys_ioctl+0x3d
> 5
> syscall(ffff800021ee8df0) at syscall+0x338
> Xsyscall(6,36,1,36,7f7ffffe4438,7281f765060) at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7f7ffffe4340, count: 1
>
I removed the KASSERT() for now because destroying the lo(4) of a rdomain
triggers it (because until now the rdomain is switched to a rtable with
rdomain 0 as L2 before calling rt_ifa_del). A better fix is attached which
should result in a better order of operation.
Thanks for the heads up
--
:wq Claudio
Index: if_loop.c
===================================================================
RCS file: /cvs/src/sys/net/if_loop.c,v
retrieving revision 1.88
diff -u -p -r1.88 if_loop.c
--- if_loop.c 9 Sep 2018 10:11:41 -0000 1.88
+++ if_loop.c 15 Feb 2019 21:21:28 -0000
@@ -196,6 +196,7 @@ int
loop_clone_destroy(struct ifnet *ifp)
{
struct ifnet *p;
+ unsigned int rdomain = 0;
if (ifp->if_index == rtable_loindex(ifp->if_rdomain)) {
/* rdomain 0 always needs a loopback */
@@ -214,13 +215,16 @@ loop_clone_destroy(struct ifnet *ifp)
}
NET_UNLOCK();
- rtable_l2set(ifp->if_rdomain, 0, 0);
+ rdomain = ifp->if_rdomain;
}
if_ih_remove(ifp, loinput, NULL);
if_detach(ifp);
free(ifp, M_DEVBUF, sizeof(*ifp));
+
+ if (rdomain)
+ rtable_l2set(rdomain, 0, 0);
return (0);
}
Index: route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.382
diff -u -p -r1.382 route.c
--- route.c 15 Feb 2019 21:16:01 -0000 1.382
+++ route.c 15 Feb 2019 21:21:28 -0000
@@ -1040,6 +1040,8 @@ rt_ifa_add(struct ifaddr *ifa, int flags
uint8_t prio = ifp->if_priority + RTP_STATIC;
int error;
+ KASSERT(rdomain == rtable_l2(rdomain));
+
memset(&info, 0, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags;
@@ -1048,12 +1050,7 @@ rt_ifa_add(struct ifaddr *ifa, int flags
info.rti_info[RTAX_GATEWAY] = sdltosa(ifp->if_sadl);
else
info.rti_info[RTAX_GATEWAY] = ifa->ifa_addr;
-
- KASSERT(rdomain == rtable_l2(rdomain));
- if (rdomain == rtable_l2(ifp->if_rtlabelid)) {
- info.rti_info[RTAX_LABEL] =
- rtlabel_id2sa(ifp->if_rtlabelid, &sa_rl);
- }
+ info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp->if_rtlabelid, &sa_rl);
#ifdef MPLS
if ((flags & RTF_MPLS) == RTF_MPLS)
@@ -1097,6 +1094,8 @@ rt_ifa_del(struct ifaddr *ifa, int flags
uint8_t prio = ifp->if_priority + RTP_STATIC;
int error;
+ KASSERT(rdomain == rtable_l2(rdomain));
+
if ((flags & RTF_HOST) == 0 && ifa->ifa_netmask) {
m = m_get(M_DONTWAIT, MT_SONAME);
if (m == NULL)
@@ -1112,11 +1111,7 @@ rt_ifa_del(struct ifaddr *ifa, int flags
info.rti_info[RTAX_DST] = dst;
if ((flags & RTF_LLINFO) == 0)
info.rti_info[RTAX_GATEWAY] = ifa->ifa_addr;
-
- if (rdomain == rtable_l2(ifp->if_rtlabelid)) {
- info.rti_info[RTAX_LABEL] =
- rtlabel_id2sa(ifp->if_rtlabelid, &sa_rl);
- }
+ info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp->if_rtlabelid, &sa_rl);
if ((flags & RTF_HOST) == 0)
info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;
