Permalink
Browse files

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)
  • Loading branch information...
1 parent c8e8fef commit 89a272e3b91ed0a23f538c485c1021adbfc81731 @grehan-freebsd grehan-freebsd committed Oct 12, 2013
Showing with 86 additions and 19 deletions.
  1. +4 −0 sys/dev/hyperv/netvsc/hv_net_vsc.h
  2. +82 −19 sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
@@ -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;
@@ -52,6 +52,9 @@
* SUCH DAMAGE.
*/
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/sockio.h>
@@ -701,6 +704,17 @@ netvsc_recv(struct hv_device *device_ctx, netvsc_packet *packet)
return (0);
}
+/*
+ * 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, caddr_t data)
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, caddr_t data)
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, caddr_t data)
*/
#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, caddr_t data)
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);
}

0 comments on commit 89a272e

Please sign in to comment.