I'm starting to work my way through locking for the various netgraph nodes shipped with FreeBSD, but have a problem in that I can't easily configure and test all of them. As such, I've done some initial global variable locking, and am working on tricking various FreeBSD developers into locking down per-node/per-instance variables, etc. The attached patch locks down global variables found in the following node implementations: //depot/vendor/freebsd/src/sys/netgraph/ng_eiface.c //depot/vendor/freebsd/src/sys/netgraph/ng_fec.c //depot/vendor/freebsd/src/sys/netgraph/ng_iface.c //depot/vendor/freebsd/src/sys/netgraph/ng_ppp.c //depot/vendor/freebsd/src/sys/netgraph/ng_tty.c If people using these netgraph modules could give the patch a spin, that would be good. There are a couple of comments regarding locking nits. If you're the owner of a netgraph module and want to take a lock at locking down the softc/per-instance variables, I'd appreciate any help I can get, as there's quite a bit of other locking work for the stack left in the queue. Robert N M Watson FreeBSD Core Team, TrustedBSD Projects robert_at_fledge.watson.org Principal Research Scientist, McAfee Research --- //depot/vendor/freebsd/src/sys/netgraph/ng_eiface.c 2004/07/04 16:15:35 +++ //depot/user/rwatson/netperf/sys/netgraph/ng_eiface.c 2004/07/09 21:19:53 _at__at_ -33,8 +33,10 _at__at_ #include <sys/systm.h> #include <sys/errno.h> #include <sys/kernel.h> +#include <sys/lock.h> #include <sys/malloc.h> #include <sys/mbuf.h> +#include <sys/mutex.h> #include <sys/errno.h> #include <sys/sockio.h> #include <sys/socket.h> _at__at_ -119,6 +121,8 _at__at_ #define UNITS_BITSPERWORD (sizeof(*ng_eiface_units) * NBBY) +static struct mtx ng_eiface_mtx; +MTX_SYSINIT(ng_eiface, &ng_eiface_mtx, "ng_eiface", MTX_DEF); /************************************************************************ HELPER STUFF _at__at_ -132,6 +136,7 _at__at_ { int index, bit; + mtx_lock(&ng_eiface_mtx); for (index = 0; index < ng_eiface_units_len && ng_eiface_units[index] == 0; index++); if (index == ng_eiface_units_len) { /* extend array */ _at__at_ -140,8 +145,10 _at__at_ newlen = (2 * ng_eiface_units_len) + 4; MALLOC(newarray, int *, newlen * sizeof(*ng_eiface_units), M_NETGRAPH, M_NOWAIT); - if (newarray == NULL) + if (newarray == NULL) { + mtx_unlock(&ng_eiface_mtx); return (ENOMEM); + } bcopy(ng_eiface_units, newarray, ng_eiface_units_len * sizeof(*ng_eiface_units)); for (i = ng_eiface_units_len; i < newlen; i++) _at__at_ -157,6 +164,7 _at__at_ ng_eiface_units[index] &= ~(1 << bit); *unit = (index * UNITS_BITSPERWORD) + bit; ng_units_in_use++; + mtx_unlock(&ng_eiface_mtx); return (0); } _at__at_ -170,6 +178,7 _at__at_ index = unit / UNITS_BITSPERWORD; bit = unit % UNITS_BITSPERWORD; + mtx_lock(&ng_eiface_mtx); KASSERT(index < ng_eiface_units_len, ("%s: unit=%d len=%d", __func__, unit, ng_eiface_units_len)); KASSERT((ng_eiface_units[index] & (1 << bit)) == 0, _at__at_ -187,6 +196,7 _at__at_ ng_eiface_units_len = 0; ng_eiface_units = NULL; } + mtx_unlock(&ng_eiface_mtx); } /************************************************************************ --- //depot/vendor/freebsd/src/sys/netgraph/ng_fec.c 2004/07/04 16:15:35 +++ //depot/user/rwatson/netperf/sys/netgraph/ng_fec.c 2004/07/09 21:19:53 _at__at_ -257,6 +257,9 _at__at_ #define UNITS_BITSPERWORD (sizeof(*ng_fec_units) * NBBY) +static struct mtx ng_fec_mtx; +MTX_SYSINIT(ng_fec, &ng_fec_mtx, "ng_fec", MTX_DEF); + /* * Find the first free unit number for a new interface. * Increase the size of the unit bitmap as necessary. _at__at_ -266,6 +269,7 _at__at_ { int index, bit; + mtx_lock(&ng_fec_mtx); for (index = 0; index < ng_fec_units_len && ng_fec_units[index] == 0; index++); if (index == ng_fec_units_len) { /* extend array */ _at__at_ -274,8 +278,10 _at__at_ newlen = (2 * ng_fec_units_len) + 4; MALLOC(newarray, int *, newlen * sizeof(*ng_fec_units), M_NETGRAPH, M_NOWAIT); - if (newarray == NULL) + if (newarray == NULL) { + mtx_unlock(&ng_fec_mtx); return (ENOMEM); + } bcopy(ng_fec_units, newarray, ng_fec_units_len * sizeof(*ng_fec_units)); for (i = ng_fec_units_len; i < newlen; i++) _at__at_ -291,6 +297,7 _at__at_ ng_fec_units[index] &= ~(1 << bit); *unit = (index * UNITS_BITSPERWORD) + bit; ng_units_in_use++; + mtx_unlock(&ng_fec_mtx); return (0); } _at__at_ -304,6 +311,7 _at__at_ index = unit / UNITS_BITSPERWORD; bit = unit % UNITS_BITSPERWORD; + mtx_lock(&ng_fec_mtx); KASSERT(index < ng_fec_units_len, ("%s: unit=%d len=%d", __FUNCTION__, unit, ng_fec_units_len)); KASSERT((ng_fec_units[index] & (1 << bit)) == 0, _at__at_ -321,6 +329,7 _at__at_ ng_fec_units_len = 0; ng_fec_units = NULL; } + mtx_unlock(&ng_fec_mtx); } /************************************************************************ --- //depot/vendor/freebsd/src/sys/netgraph/ng_iface.c 2004/07/04 16:15:35 +++ //depot/user/rwatson/netperf/sys/netgraph/ng_iface.c 2004/07/09 21:19:53 _at__at_ -59,8 +59,10 _at__at_ #include <sys/systm.h> #include <sys/errno.h> #include <sys/kernel.h> +#include <sys/lock.h> #include <sys/malloc.h> #include <sys/mbuf.h> +#include <sys/mutex.h> #include <sys/errno.h> #include <sys/random.h> #include <sys/sockio.h> _at__at_ -218,6 +220,9 _at__at_ #define UNITS_BITSPERWORD (sizeof(*ng_iface_units) * NBBY) +static struct mtx ng_iface_mtx; +MTX_SYSINIT(ng_iface, &ng_iface_mtx, "ng_iface", MTX_DEF); + /************************************************************************ HELPER STUFF ************************************************************************/ _at__at_ -289,6 +294,7 _at__at_ { int index, bit; + mtx_lock(&ng_iface_mtx); for (index = 0; index < ng_iface_units_len && ng_iface_units[index] == 0; index++); if (index == ng_iface_units_len) { /* extend array */ _at__at_ -297,8 +303,10 _at__at_ newlen = (2 * ng_iface_units_len) + 4; MALLOC(newarray, int *, newlen * sizeof(*ng_iface_units), M_NETGRAPH_IFACE, M_NOWAIT); - if (newarray == NULL) + if (newarray == NULL) { + mtx_unlock(&ng_iface_mtx); return (ENOMEM); + } bcopy(ng_iface_units, newarray, ng_iface_units_len * sizeof(*ng_iface_units)); for (i = ng_iface_units_len; i < newlen; i++) _at__at_ -314,6 +322,7 _at__at_ ng_iface_units[index] &= ~(1 << bit); *unit = (index * UNITS_BITSPERWORD) + bit; ng_units_in_use++; + mtx_unlock(&ng_iface_mtx); return (0); } _at__at_ -327,6 +336,7 _at__at_ index = unit / UNITS_BITSPERWORD; bit = unit % UNITS_BITSPERWORD; + mtx_lock(&ng_iface_mtx); KASSERT(index < ng_iface_units_len, ("%s: unit=%d len=%d", __func__, unit, ng_iface_units_len)); KASSERT((ng_iface_units[index] & (1 << bit)) == 0, _at__at_ -344,6 +354,7 _at__at_ ng_iface_units_len = 0; ng_iface_units = NULL; } + mtx_unlock(&ng_iface_mtx); } /************************************************************************ --- //depot/vendor/freebsd/src/sys/netgraph/ng_ppp.c 2004/06/26 22:25:30 +++ //depot/user/rwatson/netperf/sys/netgraph/ng_ppp.c 2004/06/27 16:41:59 _at__at_ -362,6 +362,13 _at__at_ static int *compareLatencies; /* hack for ng_ppp_intcmp() */ +/* + * XXXRW: An ugly synchronization hack to protect an ugly hack. + */ +static struct mtx ng_ppp_latencies_mtx; +MTX_SYSINIT(ng_ppp_latencies, &ng_ppp_latencies_mtx, "ng_ppp_latencies", + MTX_DEF); + /* Address and control field header */ static const u_char ng_ppp_acf[2] = { 0xff, 0x03 }; _at__at_ -1764,10 +1771,12 _at__at_ } /* Sort active links by latency */ + mtx_lock(&ng_ppp_latencies_mtx); compareLatencies = latency; qsort(sortByLatency, priv->numActiveLinks, sizeof(*sortByLatency), ng_ppp_intcmp); compareLatencies = NULL; + mtx_unlock(&ng_ppp_latencies_mtx); /* Find the interval we need (add links in sortByLatency[] order) */ for (numFragments = 1; _at__at_ -1858,6 +1867,8 _at__at_ const int index1 = *((const int *) v1); const int index2 = *((const int *) v2); + mtx_assert(&ng_ppp_latencies_mtx, MA_OWNED); + return compareLatencies[index1] - compareLatencies[index2]; } --- //depot/vendor/freebsd/src/sys/netgraph/ng_pppoe.c 2004/06/26 22:25:30 +++ //depot/user/rwatson/netperf/sys/netgraph/ng_pppoe.c 2004/06/27 16:41:59 _at__at_ -238,6 +238,10 _at__at_ }; typedef struct PPPOE *priv_p; +/* + * XXXRW: Leave this unsynchronized, since only a single field is modified, + * and it's done so infrequently. Likewise, pppoe_mode. + */ struct ether_header eh_prototype = {{0xff,0xff,0xff,0xff,0xff,0xff}, {0x00,0x00,0x00,0x00,0x00,0x00}, --- //depot/vendor/freebsd/src/sys/netgraph/ng_tty.c 2004/06/26 08:45:27 +++ //depot/user/rwatson/netperf/sys/netgraph/ng_tty.c 2004/06/27 03:09:51 _at__at_ -166,10 +166,18 _at__at_ }; NETGRAPH_INIT(tty, &typestruct); +/* + * XXXRW: ngt_unit is protected by ng_tty_mtx. ngt_ldisc is constant once + * ng_tty is initialized. ngt_nodeop_ok is untouched, and might want to be a + * sleep lock in the future? + */ static int ngt_unit; static int ngt_nodeop_ok; /* OK to create/remove node */ static int ngt_ldisc; +static struct mtx ng_tty_mtx; +MTX_SYSINIT(ng_tty, &ng_tty_mtx, "ng_tty", MTX_DEF); + /****************************************************************** LINE DISCIPLINE METHODS ******************************************************************/ _at__at_ -214,7 +222,9 _at__at_ FREE(sc, M_NETGRAPH); goto done; } + mtx_lock(&ng_tty_mtx); snprintf(name, sizeof(name), "%s%d", typestruct.name, ngt_unit++); + mtx_unlock(&ng_tty_mtx); /* Assign node its name */ if ((error = ng_name_node(sc->node, name))) {Received on Wed Jul 14 2004 - 02:30:57 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:38:01 UTC