From owner-svn-src-stable@FreeBSD.ORG Sat Oct 12 00:42:42 2013 Return-Path: Delivered-To: svn-src-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id A0CBF8F2; Sat, 12 Oct 2013 00:42:42 +0000 (UTC) (envelope-from grehan@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 8E8312E4D; Sat, 12 Oct 2013 00:42:42 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id r9C0ggPd029651; Sat, 12 Oct 2013 00:42:42 GMT (envelope-from grehan@svn.freebsd.org) Received: (from grehan@localhost) by svn.freebsd.org (8.14.7/8.14.5/Submit) id r9C0ggGn029649; Sat, 12 Oct 2013 00:42:42 GMT (envelope-from grehan@svn.freebsd.org) Message-Id: <201310120042.r9C0ggGn029649@svn.freebsd.org> From: Peter Grehan Date: Sat, 12 Oct 2013 00:42:42 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r256363 - stable/10/sys/dev/hyperv/netvsc X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Oct 2013 00:42:42 -0000 Author: grehan Date: Sat Oct 12 00:42:41 2013 New Revision: 256363 URL: http://svnweb.freebsd.org/changeset/base/256363 Log: MFC r256362 Fix a lock-order reversal in the net driver by dropping the lock and holding a reference prior to calling further into the hyperv stack. Added missing FreeBSD idents. Approved by: re@ (gjb) Modified: stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c Directory Properties: stable/10/sys/ (props changed) stable/10/sys/dev/hyperv/ (props changed) Modified: stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h ============================================================================== --- stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h Sat Oct 12 00:32:34 2013 (r256362) +++ stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h Sat Oct 12 00:42:41 2013 (r256363) @@ -24,6 +24,8 @@ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * $FreeBSD$ */ /* @@ -970,6 +972,8 @@ typedef struct hn_softc { int hn_if_flags; struct mtx hn_lock; int hn_initdone; + /* See hv_netvsc_drv_freebsd.c for rules on how to use */ + int temp_unusable; struct hv_device *hn_dev_obj; netvsc_dev *net_dev; } hn_softc_t; Modified: stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c ============================================================================== --- stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c Sat Oct 12 00:32:34 2013 (r256362) +++ stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c Sat Oct 12 00:42:41 2013 (r256363) @@ -52,6 +52,9 @@ * SUCH DAMAGE. */ +#include +__FBSDID("$FreeBSD$"); + #include #include #include @@ -702,6 +705,17 @@ netvsc_recv(struct hv_device *device_ctx } /* + * Rules for using sc->temp_unusable: + * 1. sc->temp_unusable can only be read or written while holding NV_LOCK() + * 2. code reading sc->temp_unusable under NV_LOCK(), and finding + * sc->temp_unusable set, must release NV_LOCK() and exit + * 3. to retain exclusive control of the interface, + * sc->temp_unusable must be set by code before releasing NV_LOCK() + * 4. only code setting sc->temp_unusable can clear sc->temp_unusable + * 5. code setting sc->temp_unusable must eventually clear sc->temp_unusable + */ + +/* * Standard ioctl entry point. Called when the user wants to configure * the interface. */ @@ -713,7 +727,8 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, netvsc_device_info device_info; struct hv_device *hn_dev; int mask, error = 0; - + int retry_cnt = 500; + switch(cmd) { case SIOCSIFADDR: @@ -723,38 +738,80 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, case SIOCSIFMTU: hn_dev = vmbus_get_devctx(sc->hn_dev); - NV_LOCK(sc); + /* Check MTU value change */ + if (ifp->if_mtu == ifr->ifr_mtu) + break; if (ifr->ifr_mtu > NETVSC_MAX_CONFIGURABLE_MTU) { error = EINVAL; - NV_UNLOCK(sc); break; } + /* Obtain and record requested MTU */ ifp->if_mtu = ifr->ifr_mtu; + + do { + NV_LOCK(sc); + if (!sc->temp_unusable) { + sc->temp_unusable = TRUE; + retry_cnt = -1; + } + NV_UNLOCK(sc); + if (retry_cnt > 0) { + retry_cnt--; + DELAY(5 * 1000); + } + } while (retry_cnt > 0); - /* - * We must remove and add back the device to cause the new + if (retry_cnt == 0) { + error = EINVAL; + break; + } + + /* We must remove and add back the device to cause the new * MTU to take effect. This includes tearing down, but not * deleting the channel, then bringing it back up. */ error = hv_rf_on_device_remove(hn_dev, HV_RF_NV_RETAIN_CHANNEL); if (error) { + NV_LOCK(sc); + sc->temp_unusable = FALSE; NV_UNLOCK(sc); break; } error = hv_rf_on_device_add(hn_dev, &device_info); if (error) { + NV_LOCK(sc); + sc->temp_unusable = FALSE; NV_UNLOCK(sc); break; } hn_ifinit_locked(sc); + NV_LOCK(sc); + sc->temp_unusable = FALSE; NV_UNLOCK(sc); break; case SIOCSIFFLAGS: - NV_LOCK(sc); + do { + NV_LOCK(sc); + if (!sc->temp_unusable) { + sc->temp_unusable = TRUE; + retry_cnt = -1; + } + NV_UNLOCK(sc); + if (retry_cnt > 0) { + retry_cnt--; + DELAY(5 * 1000); + } + } while (retry_cnt > 0); + + if (retry_cnt == 0) { + error = EINVAL; + break; + } + if (ifp->if_flags & IFF_UP) { /* * If only the state of the PROMISC flag changed, @@ -766,21 +823,14 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, */ #ifdef notyet /* Fixme: Promiscuous mode? */ - /* No promiscuous mode with Xen */ if (ifp->if_drv_flags & IFF_DRV_RUNNING && ifp->if_flags & IFF_PROMISC && !(sc->hn_if_flags & IFF_PROMISC)) { /* do something here for Hyper-V */ - ; -/* XN_SETBIT(sc, XN_RX_MODE, */ -/* XN_RXMODE_RX_PROMISC); */ } else if (ifp->if_drv_flags & IFF_DRV_RUNNING && - !(ifp->if_flags & IFF_PROMISC) && - sc->hn_if_flags & IFF_PROMISC) { + !(ifp->if_flags & IFF_PROMISC) && + sc->hn_if_flags & IFF_PROMISC) { /* do something here for Hyper-V */ - ; -/* XN_CLRBIT(sc, XN_RX_MODE, */ -/* XN_RXMODE_RX_PROMISC); */ } else #endif hn_ifinit_locked(sc); @@ -789,8 +839,10 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, hn_stop(sc); } } - sc->hn_if_flags = ifp->if_flags; + NV_LOCK(sc); + sc->temp_unusable = FALSE; NV_UNLOCK(sc); + sc->hn_if_flags = ifp->if_flags; error = 0; break; case SIOCSIFCAP: @@ -838,7 +890,6 @@ hn_stop(hn_softc_t *sc) int ret; struct hv_device *device_ctx = vmbus_get_devctx(sc->hn_dev); - NV_LOCK_ASSERT(sc); ifp = sc->hn_ifp; printf(" Closing Device ...\n"); @@ -859,6 +910,10 @@ hn_start(struct ifnet *ifp) sc = ifp->if_softc; NV_LOCK(sc); + if (sc->temp_unusable) { + NV_UNLOCK(sc); + return; + } hn_start_locked(ifp); NV_UNLOCK(sc); } @@ -873,8 +928,6 @@ hn_ifinit_locked(hn_softc_t *sc) struct hv_device *device_ctx = vmbus_get_devctx(sc->hn_dev); int ret; - NV_LOCK_ASSERT(sc); - ifp = sc->hn_ifp; if (ifp->if_drv_flags & IFF_DRV_RUNNING) { @@ -902,7 +955,17 @@ hn_ifinit(void *xsc) hn_softc_t *sc = xsc; NV_LOCK(sc); + if (sc->temp_unusable) { + NV_UNLOCK(sc); + return; + } + sc->temp_unusable = TRUE; + NV_UNLOCK(sc); + hn_ifinit_locked(sc); + + NV_LOCK(sc); + sc->temp_unusable = FALSE; NV_UNLOCK(sc); }