Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 22 Mar 2004 08:14:21 -0800 (PST)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 49530 for review
Message-ID:  <200403221614.i2MGELAk086326@repoman.freebsd.org>

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

Change 49530 by rwatson@rwatson_paprika on 2004/03/22 08:13:26

	Integrate netperf_socket:
	
	- uio_machdep for various architectures.
	- Remove NetBSDism from uipc_socket2.c (m_claim()).
	- Loop back global variable locking for if_gif, if_gre.

Affected files ...

.. //depot/projects/netperf_socket/sys/conf/files.pc98#6 integrate
.. //depot/projects/netperf_socket/sys/conf/files.sparc64#2 integrate
.. //depot/projects/netperf_socket/sys/conf/kern.post.mk#5 integrate
.. //depot/projects/netperf_socket/sys/kern/uipc_socket2.c#6 integrate
.. //depot/projects/netperf_socket/sys/net/if_gif.c#2 integrate
.. //depot/projects/netperf_socket/sys/net/if_gre.c#3 integrate
.. //depot/projects/netperf_socket/sys/net/if_gre.h#3 integrate
.. //depot/projects/netperf_socket/sys/netinet/ip_gre.c#3 integrate
.. //depot/projects/netperf_socket/sys/sparc64/sparc64/uio_machdep.c#1 branch

Differences ...

==== //depot/projects/netperf_socket/sys/conf/files.pc98#6 (text+ko) ====

@@ -3,7 +3,7 @@
 #
 # modified for PC-9801
 #
-# $FreeBSD: src/sys/conf/files.pc98,v 1.289 2004/03/14 23:03:56 imp Exp $
+# $FreeBSD: src/sys/conf/files.pc98,v 1.290 2004/03/22 13:37:11 nyan Exp $
 #
 # The long compile-with and dependency lines are required because of
 # limitations in config: backslash-newline doesn't work in strings, and
@@ -172,6 +172,7 @@
 i386/i386/sys_machdep.c		standard
 i386/i386/trap.c		standard
 i386/i386/tsc.c			standard
+i386/i386/uio_machdep.c		standard
 i386/i386/vm86.c		standard
 i386/i386/vm_machdep.c		standard
 i386/ibcs2/ibcs2_errno.c	optional	ibcs2

==== //depot/projects/netperf_socket/sys/conf/files.sparc64#2 (text+ko) ====

@@ -1,7 +1,7 @@
 # This file tells config what files go into building a kernel,
 # files marked standard are always included.
 #
-# $FreeBSD: src/sys/conf/files.sparc64,v 1.50 2004/01/14 08:38:13 des Exp $
+# $FreeBSD: src/sys/conf/files.sparc64,v 1.51 2004/03/22 08:08:25 alc Exp $
 #
 # The long compile-with and dependency lines are required because of
 # limitations in config: backslash-newline doesn't work in strings, and
@@ -104,6 +104,7 @@
 sparc64/sparc64/tlb.c		standard
 sparc64/sparc64/trap.c		standard
 sparc64/sparc64/tsb.c		standard
+sparc64/sparc64/uio_machdep.c	standard
 sparc64/sparc64/vm_machdep.c	standard
 #
 ukbdmap.h			optional	ukbd_dflt_keymap	\

==== //depot/projects/netperf_socket/sys/conf/kern.post.mk#5 (text+ko) ====

@@ -1,4 +1,4 @@
-# $FreeBSD: src/sys/conf/kern.post.mk,v 1.64 2004/03/21 19:06:54 obrien Exp $
+# $FreeBSD: src/sys/conf/kern.post.mk,v 1.65 2004/03/22 15:45:17 obrien Exp $
 
 # Part of a unified Makefile for building kernels.  This part includes all
 # the definitions that need to be after all the % directives except %RULES
@@ -116,7 +116,7 @@
 ./assym.s: assym.s
 
 assym.s: $S/kern/genassym.sh genassym.o
-	NM="${NM}" sh $S/kern/genassym.sh genassym.o > ${.TARGET}
+	NM='${NM}' sh $S/kern/genassym.sh genassym.o > ${.TARGET}
 
 genassym.o: $S/$M/$M/genassym.c
 	${CC} -c ${CFLAGS:N-fno-common} $S/$M/$M/genassym.c

==== //depot/projects/netperf_socket/sys/kern/uipc_socket2.c#6 (text+ko) ====

@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__FBSDID("$FreeBSD: src/sys/kern/uipc_socket2.c,v 1.121 2004/03/01 03:14:21 rwatson Exp $");
+__FBSDID("$FreeBSD: src/sys/kern/uipc_socket2.c,v 1.122 2004/03/22 10:17:40 ps Exp $");
 
 #include "opt_mac.h"
 #include "opt_param.h"
@@ -719,9 +719,6 @@
 
 	SBLASTMBUFCHK(sb);
 
-#ifdef MBUFTRACE
-	m_claim(m, sb->sb_mowner);
-#endif
 	sbcompress(sb, m, sb->sb_mbtail);
 
 	sb->sb_lastrecord = sb->sb_mb;

==== //depot/projects/netperf_socket/sys/net/if_gif.c#2 (text+ko) ====

@@ -1,4 +1,4 @@
-/*	$FreeBSD: src/sys/net/if_gif.c,v 1.41 2003/12/28 03:55:59 sam Exp $	*/
+/*	$FreeBSD: src/sys/net/if_gif.c,v 1.43 2004/03/22 15:43:14 rwatson Exp $	*/
 /*	$KAME: if_gif.c,v 1.87 2001/10/19 08:50:27 itojun Exp $	*/
 
 /*
@@ -83,9 +83,23 @@
 
 #define GIFNAME		"gif"
 
+/*
+ * gif_mtx protects the global gif_softc_list, and access to gif_called.
+ * XXX: See comment blow on gif_called.
+ * XXX: Per-softc locking is still required.
+ */
+static struct mtx gif_mtx;
 static MALLOC_DEFINE(M_GIF, "gif", "Generic Tunnel Interface");
 static LIST_HEAD(, gif_softc) gif_softc_list;
 
+/*
+ * XXX: gif_called is a recursion counter to prevent misconfiguration to
+ * cause unbounded looping in the network stack.  However, this is a flawed
+ * approach as it assumes non-reentrance in the stack.  This should be
+ * changed to use packet tags to track recusion..
+ */
+static int gif_called = 0;
+
 void	(*ng_gif_input_p)(struct ifnet *ifp, struct mbuf **mp, int af);
 void	(*ng_gif_input_orphan_p)(struct ifnet *ifp, struct mbuf *m, int af);
 void	(*ng_gif_attach_p)(struct ifnet *ifp);
@@ -145,7 +159,9 @@
 
 	gifattach0(sc);
 
+	mtx_lock(&gif_mtx);
 	LIST_INSERT_HEAD(&gif_softc_list, sc, gif_list);
+	mtx_unlock(&gif_mtx);
 	return (0);
 }
 
@@ -173,15 +189,13 @@
 		(*ng_gif_attach_p)(&sc->gif_if);
 }
 
-void
-gif_clone_destroy(ifp)
-	struct ifnet *ifp;
+static void
+gif_destroy(struct gif_softc *sc)
 {
+	struct ifnet *ifp = &sc->gif_if;
 	int err;
-	struct gif_softc *sc = ifp->if_softc;
 
-	gif_delete_tunnel(&sc->gif_if);
-	LIST_REMOVE(sc, gif_list);
+	gif_delete_tunnel(ifp);
 #ifdef INET6
 	if (sc->encap_cookie6 != NULL) {
 		err = encap_detach(sc->encap_cookie6);
@@ -203,15 +217,29 @@
 	free(sc, M_GIF);
 }
 
+void
+gif_clone_destroy(ifp)
+	struct ifnet *ifp;
+{
+	struct gif_softc *sc = ifp->if_softc;
+
+	mtx_lock(&gif_mtx);
+	LIST_REMOVE(sc, gif_list);
+	mtx_unlock(&gif_mtx);
+	gif_destroy(sc);
+}
+
 static int
 gifmodevent(mod, type, data)
 	module_t mod;
 	int type;
 	void *data;
 {
+	struct gif_softc *sc;
 
 	switch (type) {
 	case MOD_LOAD:
+		mtx_init(&gif_mtx, "gif_mtx", NULL, MTX_DEF);
 		LIST_INIT(&gif_softc_list);
 		if_clone_attach(&gif_cloner);
 
@@ -223,9 +251,15 @@
 	case MOD_UNLOAD:
 		if_clone_detach(&gif_cloner);
 
-		while (!LIST_EMPTY(&gif_softc_list))
-			gif_clone_destroy(&LIST_FIRST(&gif_softc_list)->gif_if);
-
+		mtx_lock(&gif_mtx);
+		while ((sc = LIST_FIRST(&gif_softc_list)) != NULL) {
+			LIST_REMOVE(sc, gif_list);
+			mtx_unlock(&gif_mtx);
+			gif_destroy(sc);
+			mtx_lock(&gif_mtx);
+		}
+		mtx_unlock(&gif_mtx);
+		mtx_destroy(&gif_mtx);
 #ifdef INET6
 		ip6_gif_hlim = 0;
 #endif
@@ -314,7 +348,6 @@
 {
 	struct gif_softc *sc = (struct gif_softc*)ifp;
 	int error = 0;
-	static int called = 0;	/* XXX: MUTEX */
 
 #ifdef MAC
 	error = mac_check_ifnet_transmit(ifp, m);
@@ -331,14 +364,17 @@
 	 *      mutual exclusion of the variable CALLED, especially if we
 	 *      use kernel thread.
 	 */
-	if (++called > max_gif_nesting) {
+	mtx_lock(&gif_mtx);
+	if (++gif_called > max_gif_nesting) {
+		mtx_unlock(&gif_mtx);
 		log(LOG_NOTICE,
 		    "gif_output: recursively called too many times(%d)\n",
-		    called);
+		    gif_called);
 		m_freem(m);
 		error = EIO;	/* is there better errno? */
 		goto end;
 	}
+	mtx_unlock(&gif_mtx);
 
 	m->m_flags &= ~(M_BCAST|M_MCAST);
 	if (!(ifp->if_flags & IFF_UP) ||
@@ -378,7 +414,9 @@
 	}
 
   end:
-	called = 0;		/* reset recursion counter */
+	mtx_lock(&gif_mtx);
+	gif_called = 0;		/* reset recursion counter */
+	mtx_unlock(&gif_mtx);
 	if (error)
 		ifp->if_oerrors++;
 	return error;
@@ -688,6 +726,13 @@
 	return error;
 }
 
+/*
+ * XXXRW: There's a general event-ordering issue here: the code to check
+ * if a given tunnel is already present happens before we perform a
+ * potentially blocking setup of the tunnel.  This code needs to be
+ * re-ordered so that the check and replacement can be atomic using
+ * a mutex.
+ */
 int
 gif_set_tunnel(ifp, src, dst)
 	struct ifnet *ifp;
@@ -702,6 +747,7 @@
 
 	s = splnet();
 
+	mtx_lock(&gif_mtx);
 	LIST_FOREACH(sc2, &gif_softc_list, gif_list) {
 		if (sc2 == sc)
 			continue;
@@ -721,11 +767,13 @@
 		    bcmp(sc2->gif_pdst, dst, dst->sa_len) == 0 &&
 		    bcmp(sc2->gif_psrc, src, src->sa_len) == 0) {
 			error = EADDRNOTAVAIL;
+			mtx_unlock(&gif_mtx);
 			goto bad;
 		}
 
 		/* XXX both end must be valid? (I mean, not 0.0.0.0) */
 	}
+	mtx_unlock(&gif_mtx);
 
 	/* XXX we can detach from both, but be polite just in case */
 	if (sc->gif_psrc)

==== //depot/projects/netperf_socket/sys/net/if_gre.c#3 (text+ko) ====

@@ -91,6 +91,11 @@
 
 #define GRENAME	"gre"
 
+/*
+ * gre_mtx protects all global variables in if_gre.c.
+ * XXX: gre_softc data not protected yet.
+ */
+struct mtx gre_mtx;
 static MALLOC_DEFINE(M_GRE, GRENAME, "Generic Routing Encapsulation");
 
 struct gre_softc_head gre_softc_list;
@@ -149,6 +154,7 @@
 greattach(void)
 {
 
+	mtx_init(&gre_mtx, "gre_mtx", NULL, MTX_DEF);
 	LIST_INIT(&gre_softc_list);
 	if_clone_attach(&gre_cloner);
 }
@@ -181,24 +187,35 @@
 	sc->wccp_ver = WCCP_V1;
 	if_attach(&sc->sc_if);
 	bpfattach(&sc->sc_if, DLT_NULL, sizeof(u_int32_t));
+	mtx_lock(&gre_mtx);
 	LIST_INSERT_HEAD(&gre_softc_list, sc, sc_list);
+	mtx_unlock(&gre_mtx);
 	return (0);
 }
 
 static void
-gre_clone_destroy(ifp)
-	struct ifnet *ifp;
+gre_destroy(struct gre_softc *sc)
 {
-	struct gre_softc *sc = ifp->if_softc;
 
 #ifdef INET
 	if (sc->encap != NULL)
 		encap_detach(sc->encap);
 #endif
+	bpfdetach(&sc->sc_if);
+	if_detach(&sc->sc_if);
+	free(sc, M_GRE);
+}
+
+static void
+gre_clone_destroy(ifp)
+	struct ifnet *ifp;
+{
+	struct gre_softc *sc = ifp->if_softc;
+
+	mtx_lock(&gre_mtx);
 	LIST_REMOVE(sc, sc_list);
-	bpfdetach(ifp);
-	if_detach(ifp);
-	free(sc, M_GRE);
+	mtx_unlock(&gre_mtx);
+	gre_destroy(sc);
 }
 
 /*
@@ -727,6 +744,7 @@
 static int
 gremodevent(module_t mod, int type, void *data)
 {
+	struct gre_softc *sc;
 
 	switch (type) {
 	case MOD_LOAD:
@@ -735,8 +753,15 @@
 	case MOD_UNLOAD:
 		if_clone_detach(&gre_cloner);
 
-		while (!LIST_EMPTY(&gre_softc_list))
-			gre_clone_destroy(&LIST_FIRST(&gre_softc_list)->sc_if);
+		mtx_lock(&gre_mtx);
+		while ((sc = LIST_FIRST(&gre_softc_list)) != NULL) {
+			LIST_REMOVE(sc, sc_list);
+			mtx_unlock(&gre_mtx);
+			gre_destroy(sc);
+			mtx_lock(&gre_mtx);
+		}
+		mtx_unlock(&gre_mtx);
+		mtx_destroy(&gre_mtx);
 		break;
 	}
 	return 0;

==== //depot/projects/netperf_socket/sys/net/if_gre.h#3 (text+ko) ====

@@ -176,6 +176,7 @@
 
 #ifdef _KERNEL
 LIST_HEAD(gre_softc_head, gre_softc);
+extern struct mtx gre_mtx;
 extern struct gre_softc_head gre_softc_list;
 
 u_int16_t	gre_in_cksum(u_int16_t *, u_int);

==== //depot/projects/netperf_socket/sys/netinet/ip_gre.c#3 (text+ko) ====

@@ -314,6 +314,13 @@
 
 /*
  * Find the gre interface associated with our src/dst/proto set.
+ *
+ * XXXRW: Need some sort of drain/refcount mechanism so that the softc
+ * reference remains valid after it's returned from gre_lookup().  Right
+ * now, I'm thinking it should be reference-counted with a gre_dropref()
+ * when the caller is done with the softc.  This is complicated by how
+ * to handle destroying the gre softc; probably using a gre_drain() in
+ * in_gre.c during destroy.
  */
 static struct gre_softc *
 gre_lookup(m, proto)
@@ -323,14 +330,18 @@
 	struct ip *ip = mtod(m, struct ip *);
 	struct gre_softc *sc;
 
+	mtx_lock(&gre_mtx);
 	for (sc = LIST_FIRST(&gre_softc_list); sc != NULL;
 	     sc = LIST_NEXT(sc, sc_list)) {
 		if ((sc->g_dst.s_addr == ip->ip_src.s_addr) &&
 		    (sc->g_src.s_addr == ip->ip_dst.s_addr) &&
 		    (sc->g_proto == proto) &&
-		    ((sc->sc_if.if_flags & IFF_UP) != 0))
+		    ((sc->sc_if.if_flags & IFF_UP) != 0)) {
+			mtx_unlock(&gre_mtx);
 			return (sc);
+		}
 	}
+	mtx_unlock(&gre_mtx);
 
 	return (NULL);
 }



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