From owner-freebsd-virtualization@FreeBSD.ORG Thu Jan 20 20:48:15 2011 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 93BAE106566B for ; Thu, 20 Jan 2011 20:48:15 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id E732C8FC15 for ; Thu, 20 Jan 2011 20:48:14 +0000 (UTC) Received: by fxm16 with SMTP id 16so1076493fxm.13 for ; Thu, 20 Jan 2011 12:48:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:from:to:cc:subject:date:message-id:user-agent :mime-version:content-type; bh=DLAMfsmMQYTu6YF/916rHvbwky9GD7rPJzZseRbNTQc=; b=Mhl+0CrzPriEa8zc9FsVELFCbl4tkGr0qBtQefc6gxAnaYqLmFXodQSXjudF7pHIOt ohqZ/qCPw4eUhYTjPZWvIfRKMnk8HkeQHUiJD+rieXYJJuHn/v8XKYRe7zS/+yfipPzY cgGxbrTzpNQnZvab3ptWxbG4wbybxzjtyTyLw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:cc:subject:date:message-id:user-agent:mime-version :content-type; b=PxPER6eV55V25SeBp2v6zvpSHOg6t8Cmk/KnVXBfGalXxABzUc7WL3f1daekL3Gj9Y MLWH7xCgc4eRxMvKS5SIJ1524Dnc/5cgg9vTLi0eA3cxlt1LK7Ghbwqfq7fYW66s3hA1 5x1Do7Z890XoJ88IMrLQDMXezpvB32UlBZZhA= Received: by 10.223.71.200 with SMTP id i8mr2534120faj.142.1295556428301; Thu, 20 Jan 2011 12:47:08 -0800 (PST) Received: from localhost ([95.69.174.185]) by mx.google.com with ESMTPS id 21sm3324965fav.41.2011.01.20.12.47.03 (version=TLSv1/SSLv3 cipher=RC4-MD5); Thu, 20 Jan 2011 12:47:04 -0800 (PST) From: Mikolaj Golub To: freebsd-virtualization@FreeBSD.org Date: Thu, 20 Jan 2011 22:47:02 +0200 Message-ID: <86k4hz5mm1.fsf@kopusha.home.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Cc: "Bjoern A. Zeeb" Subject: epair: panic after destroying X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jan 2011 20:48:15 -0000 --=-=-= Hi, 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
) at /usr/src/sys/libkern/strlen.c:41 #10 0xc09216c7 in kvprintf (fmt=0xc0d32ec7 " @ %s:%d", func=0xc0920950 , arg=0xeb5dfa30, radix=10, ap=0xeb5dfa74 "\202l\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 "\202l\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 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=if.c.panic.patch 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); } /* --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=if_epair.c.epair_clone_destroy.patch 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); --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=if.c.warning.patch 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? */ + } } /* --=-=-=--