Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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...
commit 89a272e3b91ed0a23f538c485c1021adbfc81731 1 parent c8e8fef
Peter Grehan authored October 12, 2013
4  sys/dev/hyperv/netvsc/hv_net_vsc.h
@@ -24,6 +24,8 @@
24 24
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25 25
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
26 26
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  27
+ *
  28
+ * $FreeBSD$
27 29
  */
28 30
 
29 31
 /*
@@ -970,6 +972,8 @@ typedef struct hn_softc {
970 972
 	int             hn_if_flags;
971 973
 	struct mtx      hn_lock;
972 974
 	int             hn_initdone;
  975
+	/* See hv_netvsc_drv_freebsd.c for rules on how to use */
  976
+	int             temp_unusable;
973 977
 	struct hv_device  *hn_dev_obj;
974 978
 	netvsc_dev  	*net_dev;
975 979
 } hn_softc_t;
101  sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
@@ -52,6 +52,9 @@
52 52
  * SUCH DAMAGE.
53 53
  */
54 54
 
  55
+#include <sys/cdefs.h>
  56
+__FBSDID("$FreeBSD$");
  57
+
55 58
 #include <sys/param.h>
56 59
 #include <sys/systm.h>
57 60
 #include <sys/sockio.h>
@@ -702,6 +705,17 @@ netvsc_recv(struct hv_device *device_ctx, netvsc_packet *packet)
702 705
 }
703 706
 
704 707
 /*
  708
+ * Rules for using sc->temp_unusable:
  709
+ * 1.  sc->temp_unusable can only be read or written while holding NV_LOCK()
  710
+ * 2.  code reading sc->temp_unusable under NV_LOCK(), and finding 
  711
+ *     sc->temp_unusable set, must release NV_LOCK() and exit
  712
+ * 3.  to retain exclusive control of the interface,
  713
+ *     sc->temp_unusable must be set by code before releasing NV_LOCK()
  714
+ * 4.  only code setting sc->temp_unusable can clear sc->temp_unusable
  715
+ * 5.  code setting sc->temp_unusable must eventually clear sc->temp_unusable
  716
+ */
  717
+
  718
+/*
705 719
  * Standard ioctl entry point.  Called when the user wants to configure
706 720
  * the interface.
707 721
  */
@@ -713,7 +727,8 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
713 727
 	netvsc_device_info device_info;
714 728
 	struct hv_device *hn_dev;
715 729
 	int mask, error = 0;
716  
-
  730
+	int retry_cnt = 500;
  731
+	
717 732
 	switch(cmd) {
718 733
 
719 734
 	case SIOCSIFADDR:
@@ -723,38 +738,80 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
723 738
 	case SIOCSIFMTU:
724 739
 		hn_dev = vmbus_get_devctx(sc->hn_dev);
725 740
 
726  
-		NV_LOCK(sc);
  741
+		/* Check MTU value change */
  742
+		if (ifp->if_mtu == ifr->ifr_mtu)
  743
+			break;
727 744
 
728 745
 		if (ifr->ifr_mtu > NETVSC_MAX_CONFIGURABLE_MTU) {
729 746
 			error = EINVAL;
730  
-			NV_UNLOCK(sc);
731 747
 			break;
732 748
 		}
  749
+
733 750
 		/* Obtain and record requested MTU */
734 751
 		ifp->if_mtu = ifr->ifr_mtu;
  752
+ 		
  753
+		do {
  754
+			NV_LOCK(sc);
  755
+			if (!sc->temp_unusable) {
  756
+				sc->temp_unusable = TRUE;
  757
+				retry_cnt = -1;
  758
+			}
  759
+			NV_UNLOCK(sc);
  760
+			if (retry_cnt > 0) {
  761
+				retry_cnt--;
  762
+				DELAY(5 * 1000);
  763
+			}
  764
+		} while (retry_cnt > 0);
735 765
 
736  
-		/*
737  
-		 * We must remove and add back the device to cause the new
  766
+		if (retry_cnt == 0) {
  767
+			error = EINVAL;
  768
+			break;
  769
+		}
  770
+
  771
+		/* We must remove and add back the device to cause the new
738 772
 		 * MTU to take effect.  This includes tearing down, but not
739 773
 		 * deleting the channel, then bringing it back up.
740 774
 		 */
741 775
 		error = hv_rf_on_device_remove(hn_dev, HV_RF_NV_RETAIN_CHANNEL);
742 776
 		if (error) {
  777
+			NV_LOCK(sc);
  778
+			sc->temp_unusable = FALSE;
743 779
 			NV_UNLOCK(sc);
744 780
 			break;
745 781
 		}
746 782
 		error = hv_rf_on_device_add(hn_dev, &device_info);
747 783
 		if (error) {
  784
+			NV_LOCK(sc);
  785
+			sc->temp_unusable = FALSE;
748 786
 			NV_UNLOCK(sc);
749 787
 			break;
750 788
 		}
751 789
 
752 790
 		hn_ifinit_locked(sc);
753 791
 
  792
+		NV_LOCK(sc);
  793
+		sc->temp_unusable = FALSE;
754 794
 		NV_UNLOCK(sc);
755 795
 		break;
756 796
 	case SIOCSIFFLAGS:
757  
-		NV_LOCK(sc);
  797
+		do {
  798
+                       NV_LOCK(sc);
  799
+                       if (!sc->temp_unusable) {
  800
+                               sc->temp_unusable = TRUE;
  801
+                               retry_cnt = -1;
  802
+                       }
  803
+                       NV_UNLOCK(sc);
  804
+                       if (retry_cnt > 0) {
  805
+                      	        retry_cnt--;
  806
+                        	DELAY(5 * 1000);
  807
+                       }
  808
+                } while (retry_cnt > 0);
  809
+
  810
+                if (retry_cnt == 0) {
  811
+                       error = EINVAL;
  812
+                       break;
  813
+                }
  814
+
758 815
 		if (ifp->if_flags & IFF_UP) {
759 816
 			/*
760 817
 			 * If only the state of the PROMISC flag changed,
@@ -766,21 +823,14 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
766 823
 			 */
767 824
 #ifdef notyet
768 825
 			/* Fixme:  Promiscuous mode? */
769  
-			/* No promiscuous mode with Xen */
770 826
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
771 827
 			    ifp->if_flags & IFF_PROMISC &&
772 828
 			    !(sc->hn_if_flags & IFF_PROMISC)) {
773 829
 				/* do something here for Hyper-V */
774  
-				;
775  
-/*				XN_SETBIT(sc, XN_RX_MODE,		*/
776  
-/*					  XN_RXMODE_RX_PROMISC);	*/
777 830
 			} else if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
778  
-				   !(ifp->if_flags & IFF_PROMISC) &&
779  
-				   sc->hn_if_flags & IFF_PROMISC) {
  831
+			    !(ifp->if_flags & IFF_PROMISC) &&
  832
+			    sc->hn_if_flags & IFF_PROMISC) {
780 833
 				/* do something here for Hyper-V */
781  
-				;
782  
-/*				XN_CLRBIT(sc, XN_RX_MODE,		*/
783  
-/*					  XN_RXMODE_RX_PROMISC);	*/
784 834
 			} else
785 835
 #endif
786 836
 				hn_ifinit_locked(sc);
@@ -789,8 +839,10 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
789 839
 				hn_stop(sc);
790 840
 			}
791 841
 		}
792  
-		sc->hn_if_flags = ifp->if_flags;
  842
+		NV_LOCK(sc);
  843
+		sc->temp_unusable = FALSE;
793 844
 		NV_UNLOCK(sc);
  845
+		sc->hn_if_flags = ifp->if_flags;
794 846
 		error = 0;
795 847
 		break;
796 848
 	case SIOCSIFCAP:
@@ -838,7 +890,6 @@ hn_stop(hn_softc_t *sc)
838 890
 	int ret;
839 891
 	struct hv_device *device_ctx = vmbus_get_devctx(sc->hn_dev);
840 892
 
841  
-	NV_LOCK_ASSERT(sc);
842 893
 	ifp = sc->hn_ifp;
843 894
 
844 895
 	printf(" Closing Device ...\n");
@@ -859,6 +910,10 @@ hn_start(struct ifnet *ifp)
859 910
 
860 911
 	sc = ifp->if_softc;
861 912
 	NV_LOCK(sc);
  913
+	if (sc->temp_unusable) {
  914
+		NV_UNLOCK(sc);
  915
+		return;
  916
+	}
862 917
 	hn_start_locked(ifp);
863 918
 	NV_UNLOCK(sc);
864 919
 }
@@ -873,8 +928,6 @@ hn_ifinit_locked(hn_softc_t *sc)
873 928
 	struct hv_device *device_ctx = vmbus_get_devctx(sc->hn_dev);
874 929
 	int ret;
875 930
 
876  
-	NV_LOCK_ASSERT(sc);
877  
-
878 931
 	ifp = sc->hn_ifp;
879 932
 
880 933
 	if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
@@ -902,7 +955,17 @@ hn_ifinit(void *xsc)
902 955
 	hn_softc_t *sc = xsc;
903 956
 
904 957
 	NV_LOCK(sc);
  958
+	if (sc->temp_unusable) {
  959
+		NV_UNLOCK(sc);
  960
+		return;
  961
+	}
  962
+	sc->temp_unusable = TRUE;
  963
+	NV_UNLOCK(sc);
  964
+
905 965
 	hn_ifinit_locked(sc);
  966
+
  967
+	NV_LOCK(sc);
  968
+	sc->temp_unusable = FALSE;
906 969
 	NV_UNLOCK(sc);
907 970
 }
908 971
 

0 notes on commit 89a272e

Please sign in to comment.
Something went wrong with that request. Please try again.