Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 30 Apr 2008 22:19:37 GMT
From:      Andrew Thompson <thompsa@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 140951 for review
Message-ID:  <200804302219.m3UMJbOh016803@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=140951

Change 140951 by thompsa@thompsa_burger on 2008/04/30 22:19:13

	Unify all the wifi *_ioctl routines
	 - Limit grabbing the lock to SIOCSIFFLAGS.
	 - Move ieee80211_start_all() to SIOCSIFFLAGS.
	 - Remove SIOCSIFMEDIA as it is not useful.
	 - Limit ether_ioctl to only SIOCGIFADDR. SIOCSIFADDR and SIOCSIFMTU
	   have no affect as there is no input/output path in the vap parent.
	   The vap code will handle the reinit of the mac address changes.
	
	Discussed with:		sam

Affected files ...

.. //depot/projects/vap/sys/dev/ath/if_ath.c#66 edit
.. //depot/projects/vap/sys/dev/bwi/if_bwi.c#11 edit
.. //depot/projects/vap/sys/dev/ipw/if_ipw.c#15 edit
.. //depot/projects/vap/sys/dev/iwi/if_iwi.c#28 edit
.. //depot/projects/vap/sys/dev/iwn/if_iwn.c#16 edit
.. //depot/projects/vap/sys/dev/mwl/if_mwl.c#7 edit
.. //depot/projects/vap/sys/dev/ral/rt2560.c#31 edit
.. //depot/projects/vap/sys/dev/ral/rt2661.c#29 edit
.. //depot/projects/vap/sys/dev/usb/if_rum.c#20 edit
.. //depot/projects/vap/sys/dev/usb/if_zyd.c#19 edit
.. //depot/projects/vap/sys/dev/wi/if_wi.c#28 edit
.. //depot/projects/vap/sys/dev/wpi/if_wpi.c#21 edit

Differences ...

==== //depot/projects/vap/sys/dev/ath/if_ath.c#66 (text+ko) ====

@@ -6277,9 +6277,9 @@
 	struct ifreq *ifr = (struct ifreq *)data;
 	int error = 0;
 
-	ATH_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		ATH_LOCK(sc);
 		if (IS_RUNNING(ifp)) {
 			/*
 			 * To avoid rescanning another access point,
@@ -6301,6 +6301,7 @@
 				ath_init(sc);	/* XXX lose error */
 		} else
 			ath_stop_locked(ifp);
+		ATH_UNLOCK(sc);
 		break;
 	case SIOCGIFMEDIA:
 	case SIOCSIFMEDIA:
@@ -6315,27 +6316,20 @@
 			&sc->sc_stats.ast_rx_noise);
 #endif
 		sc->sc_stats.ast_tx_rate = sc->sc_hwmap[sc->sc_txrate].ieeerate;
-		ATH_UNLOCK(sc);
-		/*
-		 * NB: Drop the softc lock in case of a page fault;
-		 * we'll accept any potential inconsisentcy in the
-		 * statistics.  The alternative is to copy the data
-		 * to a local structure.
-		 */
 		return copyout(&sc->sc_stats,
-				ifr->ifr_data, sizeof (sc->sc_stats));
+		    ifr->ifr_data, sizeof (sc->sc_stats));
 #ifdef ATH_DIAGAPI
 	case SIOCGATHDIAG:
-		ATH_UNLOCK(sc);
 		error = ath_ioctl_diag(sc, (struct ath_diag *) ifr);
-		ATH_LOCK(sc);
 		break;
 #endif
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, data);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	ATH_UNLOCK(sc);
 	return error;
 #undef IS_RUNNING
 }

==== //depot/projects/vap/sys/dev/bwi/if_bwi.c#11 (text+ko) ====

@@ -1318,12 +1318,11 @@
 	struct bwi_softc *sc = ifp->if_softc;
 	struct ieee80211com *ic = ifp->if_l2com;
 	struct ifreq *ifr = (struct ifreq *)req;
-	int error = 0, startall = 0;
+	int error = 0;
 
-	BWI_LOCK(sc);
-
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		BWI_LOCK(sc);
 		if (IS_RUNNING(ifp)) {
 			struct bwi_mac *mac;
 			int promisc = -1;
@@ -1346,11 +1345,12 @@
 			if (promisc >= 0)
 				bwi_mac_set_promisc(mac, promisc);
 		}
+		BWI_UNLOCK(sc);
 
 		if (ifp->if_flags & IFF_UP) {
 			if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 				bwi_init(sc);
-				startall = 1;
+				ieee80211_start_all(ic);
 			}
 		} else {
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
@@ -1358,18 +1358,15 @@
 		}
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, req);
+		error = EINVAL;
 		break;
 	}
-
-	BWI_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 #undef IS_RUNNING
 }

==== //depot/projects/vap/sys/dev/ipw/if_ipw.c#15 (text+ko) ====

@@ -1847,9 +1847,9 @@
 	int error = 0, startall = 0;
 	IPW_LOCK_DECL;
 
-	IPW_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		IPW_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
 			if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
 				ipw_init_locked(sc);
@@ -1859,18 +1859,20 @@
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
 				ipw_stop_locked(sc);
 		}
+		IPW_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
+		break;
 	}
-	IPW_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/iwi/if_iwi.c#28 (text+ko) ====

@@ -2052,9 +2052,9 @@
 	int error = 0, startall = 0;
 	IWI_LOCK_DECL;
 
-	IWI_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		IWI_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
 			if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
 				iwi_init_locked(sc);
@@ -2064,19 +2064,20 @@
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
 				iwi_stop_locked(sc);
 		}
+		IWI_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	IWI_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/iwn/if_iwn.c#16 (text+kox) ====

@@ -2367,9 +2367,9 @@
 	struct ifreq *ifr = (struct ifreq *) data;
 	int error = 0, startall = 0;
 
-	IWN_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		IWN_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
 			if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
 				iwn_init_locked(sc);
@@ -2379,19 +2379,20 @@
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
 				iwn_stop_locked(sc);
 		}
+		IWN_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	IWN_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/mwl/if_mwl.c#7 (text+ko) ====

@@ -5227,10 +5227,6 @@
 		if (startall)
 			ieee80211_start_all(ic);
 		break;
-	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
-		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
-		break;
 	case SIOCGMVSTATS:
 		mwl_hal_gethwstats(sc->sc_mh, &sc->sc_stats.hw_stats);
 		/* NB: embed these numbers to get a consistent view */
@@ -5255,8 +5251,14 @@
 		MWL_UNLOCK(sc);
 		break;
 #endif /* MWL_DIAGAPI */
+	case SIOCGIFMEDIA:
+		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
+		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
 	return error;

==== //depot/projects/vap/sys/dev/ral/rt2560.c#31 (text) ====

@@ -2029,9 +2029,9 @@
 	struct ifreq *ifr = (struct ifreq *) data;
 	int error = 0, startall = 0;
 
-	RAL_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		RAL_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
 			if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 				rt2560_init_locked(sc);
@@ -2042,19 +2042,20 @@
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
 				rt2560_stop_locked(sc);
 		}
+		RAL_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	RAL_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/ral/rt2661.c#29 (text) ====

@@ -1771,9 +1771,9 @@
 	struct ifreq *ifr = (struct ifreq *) data;
 	int error = 0, startall = 0;
 
-	RAL_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		RAL_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
 			if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 				rt2661_init_locked(sc);
@@ -1784,19 +1784,20 @@
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
 				rt2661_stop_locked(sc);
 		}
+		RAL_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	RAL_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/usb/if_rum.c#20 (text+ko) ====

@@ -1451,9 +1451,9 @@
 	struct ifreq *ifr = (struct ifreq *) data;
 	int error = 0, startall = 0;
 
-	RUM_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		RUM_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
 			if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) {
 				rum_init(sc);
@@ -1464,18 +1464,20 @@
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
 				rum_stop(sc);
 		}
+		RUM_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
+		break;
 	}
-	RUM_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/usb/if_zyd.c#19 (text+ko) ====

@@ -2472,9 +2472,9 @@
 	struct ifreq *ifr = (struct ifreq *) data;
 	int error = 0, startall = 0;
 
-	ZYD_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		ZYD_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
 				if ((ifp->if_flags ^ sc->sc_if_flags) &
@@ -2489,19 +2489,20 @@
 				zyd_stop(sc, 1);
 		}
 		sc->sc_if_flags = ifp->if_flags;
+		ZYD_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	ZYD_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/wi/if_wi.c#28 (text+ko) ====

@@ -1181,9 +1181,9 @@
 	struct ifreq *ifr = (struct ifreq *) data;
 	int error = 0, startall = 0;
 
-	WI_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		WI_LOCK(sc);
 		/*
 		 * Can't do promisc and hostap at the same time.  If all that's
 		 * changing is the promisc flag, try to short-circuit a call to
@@ -1209,19 +1209,20 @@
 			sc->wi_gone = 0;
 		}
 		sc->sc_if_flags = ifp->if_flags;
+		WI_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	WI_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 

==== //depot/projects/vap/sys/dev/wpi/if_wpi.c#21 (text+ko) ====

@@ -2083,9 +2083,9 @@
 	struct ifreq *ifr = (struct ifreq *) data;
 	int error = 0, startall = 0;
 
-	WPI_LOCK(sc);
 	switch (cmd) {
 	case SIOCSIFFLAGS:
+		WPI_LOCK(sc);
 		if ((ifp->if_flags & IFF_UP)) {
 			if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
 				wpi_init_locked(sc, 0);
@@ -2094,19 +2094,20 @@
 		} else if ((ifp->if_drv_flags & IFF_DRV_RUNNING) ||
 			   (sc->flags & WPI_FLAG_HW_RADIO_OFF))
 			wpi_stop_locked(sc);
+		WPI_UNLOCK(sc);
+		if (startall)
+			ieee80211_start_all(ic);
 		break;
 	case SIOCGIFMEDIA:
-	case SIOCSIFMEDIA:
 		error = ifmedia_ioctl(ifp, ifr, &ic->ic_media, cmd);
 		break;
+	case SIOCGIFADDR:
+		error = ether_ioctl(ifp, cmd, req);
+		break;
 	default:
-		error = ether_ioctl(ifp, cmd, data);
+		error = EINVAL;
 		break;
 	}
-	WPI_UNLOCK(sc);
-
-	if (startall)
-		ieee80211_start_all(ic);
 	return error;
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200804302219.m3UMJbOh016803>