Date: Thu, 20 Jan 2011 22:47:02 +0200 From: Mikolaj Golub <to.my.trociny@gmail.com> To: freebsd-virtualization@FreeBSD.org Cc: "Bjoern A. Zeeb" <bz@FreeBSD.org> Subject: epair: panic after destroying Message-ID: <86k4hz5mm1.fsf@kopusha.home.net>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On CURRENT when doing something like below:
jail -c name=test vnet persist
ifconfig epair0 create
ifconfig epair0b vnet test
ifconfig epair0a destroy
jexec test ifconfig
The system panics on the last command:
#8 0xc0c32f7c in calltrap () at /usr/src/sys/i386/i386/exception.s:168
#9 0xc0994618 in strlen (str=0xdeadc0de <Address 0xdeadc0de out of bounds>)
at /usr/src/sys/libkern/strlen.c:41
#10 0xc09216c7 in kvprintf (fmt=0xc0d32ec7 " @ %s:%d",
func=0xc0920950 <snprintf_func>, arg=0xeb5dfa30, radix=10,
ap=0xeb5dfa74 "\202l<D4><C0><C4>\005") at /usr/src/sys/kern/subr_prf.c:728
#11 0xc0921f4b in vsnprintf (str=0xc0ebd700 "mtx_lock() of spin mutex ",
size=256, format=0xc0d32eac "mtx_lock() of spin mutex %s @ %s:%d",
ap=0xeb5dfa70 "<DE><C0><AD><DE>\202l<D4><C0><C4>\005") at /usr/src/sys/kern/subr_prf.c:461
#12 0xc08e914b in panic (fmt=0xc0d32eac "mtx_lock() of spin mutex %s @ %s:%d")
at /usr/src/sys/kern/kern_shutdown.c:557
#13 0xc08d882a in _mtx_lock_flags (m=0xc2ec7628, opts=0,
file=0xc0d46c82 "/usr/src/sys/net/rtsock.c", line=1476)
at /usr/src/sys/kern/kern_mutex.c:197
#14 0xc09b8389 in sysctl_rtsock (oidp=0xc0e412a0, arg1=0xeb5dfbe4, arg2=4,
req=0xeb5dfb70) at /usr/src/sys/net/rtsock.c:1476
#15 0xc08f3eed in sysctl_root (oidp=Variable "oidp" is not available.
) at /usr/src/sys/kern/kern_sysctl.c:1456
#16 0xc08f4145 in userland_sysctl (td=0xc3ab9870, name=0xeb5dfbdc, namelen=6,
old=0x0, oldlenp=0xbfbfe538, inkernel=0, new=0x0, newlen=0,
retval=0xeb5dfc3c, flags=0) at /usr/src/sys/kern/kern_sysctl.c:1566
#17 0xc08f4514 in __sysctl (td=0xc3ab9870, uap=0xeb5dfcec)
at /usr/src/sys/kern/kern_sysctl.c:1492
#18 0xc092b793 in syscallenter (td=0xc3ab9870, sa=0xeb5dfce4)
at /usr/src/sys/kern/subr_trap.c:318
#19 0xc0c4975f in syscall (frame=0xeb5dfd28)
at /usr/src/sys/i386/i386/trap.c:1094
#20 0xc0c33011 in Xint0x80_syscall ()
at /usr/src/sys/i386/i386/exception.s:266
#21 0x00000033 in ?? ()
Previous frame inner to this frame (corrupt stack?)
It crashes in sysctl_iflist when traversing V_ifnet list because it contains
dead ifp.
This is because when epair_clone_destroy() calls ether_ifdetach() for its
second half it does not switch to its vnet and if_detach_internal() can't find
the interface and just returns. This can be checked when adding the first
patch from the attaches:
<5>epair20a: link state changed to DOWN
<5>epair20b: link state changed to DOWN
panic: if_detach_internal: ifp=0xc35b8800 not on the ifnet tailq 0xc3e43038
#11 0xc08e91f4 in panic (
fmt=0xc0d44def "%s: ifp=%p not on the ifnet tailq %p")
at /usr/src/sys/kern/kern_shutdown.c:574
#12 0xc09a19b1 in if_detach_internal (ifp=0xc35b8800, vmove=0)
at /usr/src/sys/net/if.c:831
#13 0xc09a4190 in if_detach (ifp=0xc35b8800) at /usr/src/sys/net/if.c:805
#14 0xc09a5fdd in ether_ifdetach (ifp=0xc35b8800)
at /usr/src/sys/net/if_ethersubr.c:989
#15 0xc3e63ec8 in epair_clone_destroy (ifc=0xc3e661c0, ifp=0xc2ec7400)
at /usr/src/sys/modules/if_epair/../../net/if_epair.c:871
#16 0xc09a4b17 in if_clone_destroyif (ifc=0xc3e661c0, ifp=0xc2ec7400)
at /usr/src/sys/net/if_clone.c:269
#17 0xc09a53c7 in if_clone_destroy (name=0xc303e940 "epair20a")
at /usr/src/sys/net/if_clone.c:230
#18 0xc09a2a21 in ifioctl (so=0xc3aa3680, cmd=2149607801,
data=0xc303e940 "epair20a", td=0xc3bcb5a0) at /usr/src/sys/net/if.c:2464
#19 0xc093c977 in soo_ioctl (fp=0xc3aa5000, cmd=2149607801, data=0xc303e940,
active_cred=0xc2d8dc80, td=0xc3bcb5a0)
at /usr/src/sys/kern/sys_socket.c:212
#20 0xc0936aed in kern_ioctl (td=0xc3bcb5a0, fd=3, com=2149607801,
data=0xc303e940 "epair20a") at file.h:254
#21 0xc0936c74 in ioctl (td=0xc3bcb5a0, uap=0xeb5fdcec)
at /usr/src/sys/kern/sys_generic.c:679
#22 0xc092b793 in syscallenter (td=0xc3bcb5a0, sa=0xeb5fdce4)
at /usr/src/sys/kern/subr_trap.c:318
#23 0xc0c4974f in syscall (frame=0xeb5fdd28)
at /usr/src/sys/i386/i386/trap.c:1094
#24 0xc0c33001 in Xint0x80_syscall ()
at /usr/src/sys/i386/i386/exception.s:266
Adding vnet switch to epair_clone_destroy() like below fixes the issue:
- ether_ifdetach(oifp);
+ {
+ CURVNET_SET_QUIET(oifp->if_vnet);
+ ether_ifdetach(oifp);
+ CURVNET_RESTORE();
+ }
ether_ifdetach(ifp);
Although it looks a bit ugly :-). What do you think about some reordering like
in the second patch attached?
Also it looks wrong that if_detach_internal() silently returned in that case.
I think it should panic or at least warns (like in the third patch).
--
Mikolaj Golub
[-- Attachment #2 --]
Index: sys/net/if.c
===================================================================
--- sys/net/if.c (revision 217617)
+++ sys/net/if.c (working copy)
@@ -828,11 +828,8 @@ if_detach_internal(struct ifnet *ifp, int vmove)
#endif
IFNET_WUNLOCK();
if (!found) {
- if (vmove)
- panic("%s: ifp=%p not on the ifnet tailq %p",
- __func__, ifp, &V_ifnet);
- else
- return; /* XXX this should panic as well? */
+ panic("%s: ifp=%p not on the ifnet tailq %p",
+ __func__, ifp, &V_ifnet);
}
/*
[-- Attachment #3 --]
Index: sys/net/if_epair.c
===================================================================
--- sys/net/if_epair.c (revision 217596)
+++ sys/net/if_epair.c (working copy)
@@ -865,38 +865,41 @@ epair_clone_destroy(struct if_clone *ifc, struct i
if_link_state_change(oifp, LINK_STATE_DOWN);
ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
oifp->if_drv_flags &= ~IFF_DRV_RUNNING;
+
+ /*
+ * Get rid of our second half. As the other of the two
+ * interfaces my reside in a different vnet, we need to
+ * switch before freeing them.
+ */
+ CURVNET_SET_QUIET(oifp->if_vnet);
ether_ifdetach(oifp);
- ether_ifdetach(ifp);
/*
* Wait for all packets to be dispatched to if_input.
- * The numbers can only go down as the interfaces are
+ * The numbers can only go down as the interface is
* detached so there is no need to use atomics.
*/
- DPRINTF("sca refcnt=%u scb refcnt=%u\n", sca->refcount, scb->refcount);
- EPAIR_REFCOUNT_ASSERT(sca->refcount == 1 && scb->refcount == 1,
- ("%s: ifp=%p sca->refcount!=1: %d || ifp=%p scb->refcount!=1: %d",
- __func__, ifp, sca->refcount, oifp, scb->refcount));
-
- /*
- * Get rid of our second half.
- */
+ DPRINTF("scb refcnt=%u\n", scb->refcount);
+ EPAIR_REFCOUNT_ASSERT(scb->refcount == 1,
+ ("%s: ifp=%p scb->refcount!=1: %d", __func__, oifp, scb->refcount));
oifp->if_softc = NULL;
error = if_clone_destroyif(ifc, oifp);
if (error)
panic("%s: if_clone_destroyif() for our 2nd iface failed: %d",
__func__, error);
+ if_free(oifp);
+ free(scb, M_EPAIR);
+ CURVNET_RESTORE();
+ ether_ifdetach(ifp);
/*
- * Finish cleaning up. Free them and release the unit.
- * As the other of the two interfaces my reside in a different vnet,
- * we need to switch before freeing them.
+ * Wait for all packets to be dispatched to if_input.
*/
- CURVNET_SET_QUIET(oifp->if_vnet);
- if_free(oifp);
- CURVNET_RESTORE();
+ DPRINTF("sca refcnt=%u\n", sca->refcount);
+ EPAIR_REFCOUNT_ASSERT(sca->refcount == 1,
+ ("%s: ifp=%p sca->refcount!=1: %d", __func__, ifp, sca->refcount));
if_free(ifp);
- free(scb, M_EPAIR);
free(sca, M_EPAIR);
+
ifc_free_unit(ifc, unit);
return (0);
[-- Attachment #4 --]
Index: sys/net/if.c
===================================================================
--- sys/net/if.c (revision 217577)
+++ sys/net/if.c (working copy)
@@ -831,8 +831,11 @@ if_detach_internal(struct ifnet *ifp, int vmove)
if (vmove)
panic("%s: ifp=%p not on the ifnet tailq %p",
__func__, ifp, &V_ifnet);
- else
+ else {
+ log(LOG_WARNING, "%s: ifp=%p not on the ifnet tailq %p",
+ __func__, ifp, &V_ifnet);
return; /* XXX this should panic as well? */
+ }
}
/*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86k4hz5mm1.fsf>