Change the ifr_media operation to only get its value and only set

its value once per ifconfig run.  Use Sam's new callback
operation to set it when everything is done.

The purpose for this is that if you did something like
	ifconfig bge0 media 100baseTX mediaopt full-duplex
multiple times it would end up causing the PHY to re-sync
since it would send the IOCTLs:
	ifconfig bge0 media 100baseTX -mediaopt full-duplex
	ifconfig bge0 media 100baseTX mediaopt full-duplex
This would cause the PHY to be updated twice even though
there really wasn't any change since the check in
sys/net/if_media.c would always fail.

Caveat is that this doesn't fix the case of:
	ifconfig bge0 media autoselect
etc. since in sys/net/if_media.c it forces an autoselect to go through
the entire process in ifmedia_ioctl :-( :
          /*
           * If no change, we're done.
           * XXX Automedia may invole software intervention.
           *     Keep going in case the the connected media changed.
           *     Similarly, if best match changed (kernel debugger?).
           */
           if ((IFM_SUBTYPE(newmedia) != IFM_AUTO) &&
                (newmedia == ifm->ifm_media) &&
                (match == ifm->ifm_cur))
                     return 0;

Briefly looked at by:	sam

1 files changed, 81 insertions, 79 deletions

diff --git a/sbin/ifconfig/ifmedia.c b/sbin/ifconfig/ifmedia.c
index 13773ce..780a16f 100644
--- a/sbin/ifconfig/ifmedia.c
+++ b/sbin/ifconfig/ifmedia.c
@@ -190,28 +190,72 @@ media_status(int s)
 	free(media_list);
 }
 
-static void
-setmedia(const char *val, int d, int s, const struct afswtch *afp)
+static struct ifmediareq *
+getifmediastate(int s)
 {
-	struct ifmediareq ifmr;
-	int first_type, subtype;
+	static struct ifmediareq *ifmr = NULL;
+	int *mwords;
 
-	(void) memset(&ifmr, 0, sizeof(ifmr));
-	(void) strncpy(ifmr.ifm_name, name, sizeof(ifmr.ifm_name));
+	if (ifmr == NULL) {
+		ifmr = (struct ifmediareq *)malloc(sizeof(struct ifmediareq));
+		if (ifmr == NULL)
+			err(1, "malloc");
+
+		(void) memset(ifmr, 0, sizeof(struct ifmediareq));
+		(void) strncpy(ifmr->ifm_name, name,
+		    sizeof(ifmr->ifm_name));
+
+		ifmr->ifm_count = 0;
+		ifmr->ifm_ulist = NULL;
 
-	ifmr.ifm_count = 1;
-	ifmr.ifm_ulist = &first_type;
-	if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0) {
 		/*
-		 * If we get E2BIG, the kernel is telling us
-		 * that there are more, so we can ignore it.
+		 * We must go through the motions of reading all
+		 * supported media because we need to know both
+		 * the current media type and the top-level type.
 		 */
-		if (errno != E2BIG)
+
+		if (ioctl(s, SIOCGIFMEDIA, (caddr_t)ifmr) < 0) {
+			err(1, "SIOCGIFMEDIA");
+		}
+
+		if (ifmr->ifm_count == 0)
+			errx(1, "%s: no media types?", name);
+
+		mwords = (int *)malloc(ifmr->ifm_count * sizeof(int));
+		if (mwords == NULL)
+			err(1, "malloc");
+  
+		ifmr->ifm_ulist = mwords;
+		if (ioctl(s, SIOCGIFMEDIA, (caddr_t)ifmr) < 0)
 			err(1, "SIOCGIFMEDIA");
 	}
 
-	if (ifmr.ifm_count == 0)
-		errx(1, "%s: no media types?", name);
+	return ifmr;
+}
+
+static void
+setifmediacallback(int s, void *arg)
+{
+	struct ifmediareq *ifmr = (struct ifmediareq *)arg;
+	static int did_it = 0;
+
+	if (!did_it) {
+		if (ioctl(s, SIOCSIFMEDIA, (caddr_t)&ifr) < 0)
+			err(1, "SIOCSIFMEDIA (media)");
+		free(ifmr->ifm_ulist);
+		free(ifmr);
+		did_it = 1;
+	}
+}
+
+static void
+setmedia(const char *val, int d, int s, const struct afswtch *afp)
+{
+	struct ifmediareq *ifmr;
+	int subtype;
+	
+
+	ifmr = getifmediastate(s);
 
 	/*
 	 * We are primarily concerned with the top-level type.
@@ -222,14 +266,18 @@ setmedia(const char *val, int d, int s, const struct afswtch *afp)
 	 * (I'm assuming that all supported media types for a given
 	 * interface will be the same top-level type..)
 	 */
-	subtype = get_media_subtype(IFM_TYPE(first_type), val);
+	subtype = get_media_subtype(IFM_TYPE(ifmr->ifm_ulist[0]), val);
 
 	strncpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
-	ifr.ifr_media = (ifmr.ifm_current & ~(IFM_NMASK|IFM_TMASK)) |
-	    IFM_TYPE(first_type) | subtype;
+	ifr.ifr_media = (ifmr->ifm_current & ~(IFM_NMASK|IFM_TMASK)) |
+	    IFM_TYPE(ifmr->ifm_ulist[0]) | subtype;
+
+	if ((ifr.ifr_media & IFM_TMASK) == 0) {
+		ifr.ifr_media &= ~IFM_GMASK;
+	}
 
-	if (ioctl(s, SIOCSIFMEDIA, (caddr_t)&ifr) < 0)
-		err(1, "SIOCSIFMEDIA (media)");
+	ifmr->ifm_current = ifr.ifr_media;
+	callback_register(setifmediacallback, (void *)ifmr);
 }
 
 static void
@@ -249,86 +297,40 @@ unsetmediaopt(const char *val, int d, int s, const struct afswtch *afp)
 static void
 domediaopt(const char *val, int clear, int s)
 {
-	struct ifmediareq ifmr;
-	int *mwords, options;
-
-	(void) memset(&ifmr, 0, sizeof(ifmr));
-	(void) strncpy(ifmr.ifm_name, name, sizeof(ifmr.ifm_name));
-
-	/*
-	 * We must go through the motions of reading all
-	 * supported media because we need to know both
-	 * the current media type and the top-level type.
-	 */
+	struct ifmediareq *ifmr;
+	int options;
 
-	if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0)
-		err(1, "SIOCGIFMEDIA");
+	ifmr = getifmediastate(s);
 
-	if (ifmr.ifm_count == 0)
-		errx(1, "%s: no media types?", name);
-
-	mwords = (int *)malloc(ifmr.ifm_count * sizeof(int));
-	if (mwords == NULL)
-		err(1, "malloc");
-
-	ifmr.ifm_ulist = mwords;
-	if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0)
-		err(1, "SIOCGIFMEDIA");
-
-	options = get_media_options(IFM_TYPE(mwords[0]), val);
-
-	free(mwords);
+	options = get_media_options(IFM_TYPE(ifmr->ifm_ulist[0]), val);
 
 	strncpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
-	ifr.ifr_media = ifmr.ifm_current;
+	ifr.ifr_media = ifmr->ifm_current;
 	if (clear)
 		ifr.ifr_media &= ~options;
 	else
 		ifr.ifr_media |= options;
 
-	if (ioctl(s, SIOCSIFMEDIA, (caddr_t)&ifr) < 0)
-		err(1, "SIOCSIFMEDIA (mediaopt)");
+	ifmr->ifm_current = ifr.ifr_media;
+	callback_register(setifmediacallback, (void *)ifmr);
 }
 
 
 static void
 setmediamode(const char *val, int d, int s, const struct afswtch *afp)
 {
-	struct ifmediareq ifmr;
-	int *mwords, mode;
-
-	(void) memset(&ifmr, 0, sizeof(ifmr));
-	(void) strncpy(ifmr.ifm_name, name, sizeof(ifmr.ifm_name));
-
-	/*
-	 * We must go through the motions of reading all
-	 * supported media because we need to know both
-	 * the current media type and the top-level type.
-	 */
-
-	if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0)
-		err(1, "SIOCGIFMEDIA");
-
-	if (ifmr.ifm_count == 0)
-		errx(1, "%s: no media types?", name);
-
-	mwords = (int *)malloc(ifmr.ifm_count * sizeof(int));
-	if (mwords == NULL)
-		err(1, "malloc");
-
-	ifmr.ifm_ulist = mwords;
-	if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0)
-		err(1, "SIOCGIFMEDIA");
+	struct ifmediareq *ifmr;
+	int mode;
 
-	mode = get_media_mode(IFM_TYPE(mwords[0]), val);
+	ifmr = getifmediastate(s);
 
-	free(mwords);
+	mode = get_media_mode(IFM_TYPE(ifmr->ifm_ulist[0]), val);
 
 	strncpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
-	ifr.ifr_media = (ifmr.ifm_current & ~IFM_MMASK) | mode;
+	ifr.ifr_media = (ifmr->ifm_current & ~IFM_MMASK) | mode;
 
-	if (ioctl(s, SIOCSIFMEDIA, (caddr_t)&ifr) < 0)
-		err(1, "SIOCSIFMEDIA (mode)");
+	ifmr->ifm_current = ifr.ifr_media;
+	callback_register(setifmediacallback, (void *)ifmr);
 }
 
 /**********************************************************************