ifmedia_ioctl() tweak for NET_MPSAFE

[Jason Thorpe <thorpej%me.com@localhost>]

I'm working on a simplified-as-much-as-possible recipe for converting "simple"[*] Ethernet drivers to be compatible with NET_MPSAFE.  One of my goals is to have as few #ifdefs in a converted driver as possible, to keep things tidy.

I noted that if_wm.c had a peculiar construct around the call to ifmedia_ioctl() ... specifically, it raises to splnet in the NET_MPSAFE case around the ifmedia_ioctl() call.  This is presumably because ifmedia_ioctl() itself is not MP-safe.

I think the best thing to do is to hide the not-MP-safe complexity inside ifmedia_ioctl() itself, so that the splnet calls can be removed from the driver.  No driver that's running in NET_MPSAFE mode should be calling spl-*anything* directly, which will simplify the removal of the not-MP-safe code paths later.

Attached is a diff that makes this small tweak.  I would appreciate comments from the NET_MPSAFE gurus before I check it in.

[*] "Simple" as in single TX queue / single RX queue... PCnet-PCI (if_pcn.c) is the guinea pig I'm using to develop the recipe that should translate easily to the vast majority of the Ethernet drivers we have ... even older PIO ISA types or old-school Sun3 types (even those drivers need to get converted, because the core chip drivers could be used on other boards in MP-capable systems and the kernel networking APIs will change over time in any case).

Index: dev/pci/if_wm.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_wm.c,v
retrieving revision 1.658
diff -u -p -r1.658 if_wm.c
--- dev/pci/if_wm.c	13 Dec 2019 02:03:46 -0000	1.658
+++ dev/pci/if_wm.c	16 Jan 2020 04:49:16 -0000
@@ -3398,13 +3398,7 @@ wm_ioctl(struct ifnet *ifp, u_long cmd, 
 			sc->sc_flowflags = ifr->ifr_media & IFM_ETH_FMASK;
 		}
 		WM_CORE_UNLOCK(sc);
-#ifdef WM_MPSAFE
-		s = splnet();
-#endif
 		error = ifmedia_ioctl(ifp, ifr, &sc->sc_mii.mii_media, cmd);
-#ifdef WM_MPSAFE
-		splx(s);
-#endif
 		break;
 	case SIOCINITIFADDR:
 		WM_CORE_LOCK(sc);
Index: net/if_media.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_media.c,v
retrieving revision 1.48
diff -u -p -r1.48 if_media.c
--- net/if_media.c	1 Oct 2019 17:45:25 -0000	1.48
+++ net/if_media.c	16 Jan 2020 04:49:16 -0000
@@ -78,6 +78,10 @@
 #include <sys/cdefs.h>
 __KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.48 2019/10/01 17:45:25 chs Exp $");
 
+#ifdef _KERNEL_OPT
+#include "opt_net_mpsafe.h"
+#endif
+
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/errno.h>
@@ -361,13 +365,32 @@ ifmedia_ioctl(struct ifnet *ifp, struct 
 {
 	int e;
 
+#ifdef NET_MPSAFE
 	/*
-	 * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here,
-	 * but ifmedia_ioctl_locked isn't MP-safe yet, so we must hold the lock.
+	 * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here and
+	 * ipl will not have been raised (well, maybe it has, but
+	 * we can safely raise it regardless), but ifmedia_ioctl_locked
+	 * isn't MP-safe yet, so we go to splnet and hold the lock.
+	 *
+	 * In the non-mpsafe case, the interface's ioctl routine
+	 * will already be running at splnet.
 	 */
-	KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
+	int s;
+	if (if_is_mpsafe(ifp)) {
+		s = splnet();
+		KERNEL_LOCK(1, NULL);
+	}
+#endif /* NET_MPSAFE */
+
 	e = ifmedia_ioctl_locked(ifp, ifr, ifm, cmd);
-	KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
+
+#ifdef NET_MPSAFE
+	if (if_is_mpsafe(ifp)) {
+		KERNEL_UNLOCK_ONE(NULL);
+		splx(s);
+	}
+#endif /* NET_MPSAFE */
+
 	return e;
 }
 

-- thorpej