--- //depot/vendor/freebsd/src/sys/fs/portalfs/portal_vnops.c 2004/06/24 00:50:46 +++ //depot/user/rwatson/netperf/sys/fs/portalfs/portal_vnops.c 2004/06/24 03:49:05 @@ -194,6 +194,7 @@ unp2 = sotounpcb(so2); unp3 = sotounpcb(so3); + /* XXXRW: Locking? */ if (unp2->unp_addr) unp3->unp_addr = (struct sockaddr_un *) sodupsockaddr((struct sockaddr *)unp2->unp_addr, --- //depot/vendor/freebsd/src/sys/i386/conf/GENERIC 2004/07/11 03:15:30 +++ //depot/user/rwatson/netperf/sys/i386/conf/GENERIC 2004/07/11 18:41:02 @@ -68,6 +68,7 @@ options INVARIANT_SUPPORT # Extra sanity checks of internal structures, required by INVARIANTS options WITNESS # Enable checks to detect deadlocks and cycles options WITNESS_SKIPSPIN # Don't run witness on spinlocks for speed +options BREAK_TO_DEBUGGER # To make an SMP kernel, the next two are needed options SMP # Symmetric MultiProcessor Kernel --- //depot/vendor/freebsd/src/sys/i386/include/param.h 2004/07/02 20:25:25 +++ //depot/user/rwatson/netperf/sys/i386/include/param.h 2004/07/11 20:06:17 @@ -97,7 +97,9 @@ #define NBPDR (1<f_type; + switch(type) { + case DTYPE_SOCKET: + case DTYPE_PIPE: + break; + default: + mtx_lock(&Giant); + } if (fp->f_count < 0) panic("fdrop: count < 0"); - mtx_lock(&Giant); if (fp->f_ops != &badfileops) error = fo_close(fp, td); else error = 0; ffree(fp); - mtx_unlock(&Giant); + switch(type) { + case DTYPE_SOCKET: + case DTYPE_PIPE: + break; + default: + mtx_unlock(&Giant); + } return (error); } --- //depot/vendor/freebsd/src/sys/kern/kern_mbuf.c 2004/06/01 16:20:40 +++ //depot/user/rwatson/netperf/sys/kern/kern_mbuf.c 2004/06/08 21:28:41 @@ -214,7 +214,7 @@ #endif } else m->m_data = m->m_dat; - mbstat.m_mbufs += 1; /* XXX */ + atomic_add_long(&mbstat.m_mbufs, 1); /* return 1; */ } @@ -230,7 +230,7 @@ m = (struct mbuf *)mem; if ((m->m_flags & M_PKTHDR) != 0) m_tag_delete_chain(m, NULL); - mbstat.m_mbufs -= 1; /* XXX */ + atomic_subtract_long(&mbstat.m_mbufs, 1); } /* XXX Only because of stats */ @@ -242,8 +242,8 @@ m = (struct mbuf *)mem; if ((m->m_flags & M_PKTHDR) != 0) m_tag_delete_chain(m, NULL); - mbstat.m_mbufs -= 1; /* XXX */ - mbstat.m_mclusts -= 1; /* XXX */ + atomic_subtract_long(&mbstat.m_mbufs, 1); + atomic_subtract_long(&mbstat.m_mclusts, 1); } /* @@ -268,7 +268,7 @@ m->m_ext.ref_cnt = (u_int *)uma_find_refcnt(zone_clust, m->m_ext.ext_buf); *(m->m_ext.ref_cnt) = 1; - mbstat.m_mclusts += 1; /* XXX */ + atomic_add_long(&mbstat.m_mclusts, 1); /* return 1; */ } @@ -277,7 +277,7 @@ static void mb_dtor_clust(void *mem, int size, void *arg) { - mbstat.m_mclusts -= 1; /* XXX */ + atomic_subtract_long(&mbstat.m_mclusts, 1); } /* @@ -294,7 +294,7 @@ uma_zalloc_arg(zone_clust, m, M_NOWAIT); if (m->m_ext.ext_buf == NULL) /* XXX */ panic("mb_init_pack(): Can't deal with failure yet."); - mbstat.m_mclusts -= 1; /* XXX */ + atomic_subtract_long(&mbstat.m_mclusts, 1); } /* @@ -309,7 +309,7 @@ m = (struct mbuf *)mem; uma_zfree_arg(zone_clust, m->m_ext.ext_buf, NULL); m->m_ext.ext_buf = NULL; - mbstat.m_mclusts += 1; /* XXX */ + atomic_add_long(&mbstat.m_mclusts, 1); } /* @@ -353,8 +353,8 @@ } #endif } - mbstat.m_mbufs += 1; /* XXX */ - mbstat.m_mclusts += 1; /* XXX */ + atomic_add_long(&mbstat.m_mbufs, 1); + atomic_add_long(&mbstat.m_mclusts, 1); /* return 1; */ } @@ -375,7 +375,7 @@ WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK | WARN_PANIC, NULL, "mb_reclaim()"); - mbstat.m_drain++; + atomic_add_long(&mbstat.m_drain, 1); for (dp = domains; dp != NULL; dp = dp->dom_next) for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) if (pr->pr_drain != NULL) --- //depot/vendor/freebsd/src/sys/kern/kern_timeout.c 2004/04/25 04:10:43 +++ //depot/user/rwatson/netperf/sys/kern/kern_timeout.c 2004/05/04 02:32:28 @@ -44,6 +44,7 @@ #include #include #include +#include #include static int avg_depth; @@ -55,6 +56,89 @@ static int avg_mpcalls; SYSCTL_INT(_debug, OID_AUTO, to_avg_mpcalls, CTLFLAG_RD, &avg_mpcalls, 0, "Average number of MP callouts made per softclock call. Units = 1/1000"); + +/*- + * Sampling buffer of function pointers executed by timeouts and callouts. + * This circular buffer wraps when it fills, and uses an inefficient + * sbuf-based sysctl to dump sample data to userspace. Sysctls can select + * to monitor mpsafe and !mpsafe callouts/timeouts as desired. Suggested + * use is: (1) set sample of interest (mpsafe/notmpsafe), (2) reset the + * buffer, (3) do some benchmark/test, (5) disable sampling, (6) dump + * buffer. + * + * XXX: ifdef TIMEOUT_SAMPLING? + */ + +#define MAXFUNC 200000 +static void * func_array[MAXFUNC]; +static int array_off; + +static void +push_cfunc(void *ptr) +{ + + /* XXX */ + func_array[array_off % MAXFUNC] = ptr; + array_off++; +} + +static int +sysctl_cfunc(SYSCTL_HANDLER_ARGS) +{ + struct sbuf sb; + int error, i; + + if (req->newptr != NULL) + return (EINVAL); + + sbuf_new(&sb, NULL, 0, SBUF_AUTOEXTEND); + + for (i = 0; i < MAXFUNC; i++) { + if (func_array[i] == NULL) + break; + sbuf_printf(&sb, "%p ", func_array[i]); + } + sbuf_finish(&sb); + + error = sysctl_handle_string(oidp, sbuf_data(&sb), sbuf_len(&sb) + 1, req); + + sbuf_delete(&sb); + + return (error); +} + +SYSCTL_PROC(_debug, OID_AUTO, to_cfunc, CTLTYPE_STRING|CTLFLAG_RD, 0, 0, + sysctl_cfunc, "A", "callout/timeout sample"); + +static int +sysctl_cfunc_reset(SYSCTL_HANDLER_ARGS) +{ + int dummy, error; + + dummy = 0; + error = sysctl_handle_int(oidp, &dummy, 0, req); + if (error) + return (error); + + if (dummy != 0) { + bzero(func_array, sizeof(void *) * MAXFUNC); + array_off = 0; + } + + return (0); +} + +SYSCTL_PROC(_debug, OID_AUTO, to_cfunc_reset, CTLTYPE_INT|CTLFLAG_RW, 0, 0, + sysctl_cfunc_reset, "I", "Reset sample"); + +static int cfunc_sample_mpsafe; +static int cfunc_sample_notmpsafe; + +SYSCTL_INT(_debug, OID_AUTO, to_cfunc_mpsafe, CTLFLAG_RW, + &cfunc_sample_mpsafe, 0, "Sample mpsafe callouts"); +SYSCTL_INT(_debug, OID_AUTO, to_cfunc_notmpsafe, CTLFLAG_RW, + &cfunc_sample_notmpsafe, 0, "Sample !mpsafe callouts"); + /* * TODO: * allocate more timeout table slots when table overflows. @@ -245,8 +329,12 @@ if (!(c_flags & CALLOUT_MPSAFE)) { mtx_lock(&Giant); gcalls++; + if (cfunc_sample_mpsafe) + push_cfunc(c_func); } else { mpcalls++; + if (cfunc_sample_notmpsafe) + push_cfunc(c_func); } #ifdef DIAGNOSTIC binuptime(&bt1); --- //depot/vendor/freebsd/src/sys/kern/subr_log.c 2004/06/16 09:52:13 +++ //depot/user/rwatson/netperf/sys/kern/subr_log.c 2004/06/18 00:24:39 @@ -83,7 +83,12 @@ struct callout sc_callout; /* callout to wakeup syslog */ } logsoftc; -int log_open; /* also used in log() */ +/* + * log_mtx protects logsoftc, log_open. Note that log_mtx does *not* + * protect the structures associated with msgbuf, which require Giant. + */ +struct mtx log_mtx; +int log_open; /* also used in log() */ /* Times per second to check for a pending syslog wakeup. */ static int log_wakeups_per_second = 5; @@ -94,17 +99,24 @@ static int logopen(struct cdev *dev, int flags, int mode, struct thread *td) { - if (log_open) + + mtx_lock(&log_mtx); + if (log_open) { + mtx_unlock(&log_mtx); return (EBUSY); + } log_open = 1; - callout_init(&logsoftc.sc_callout, 0); + callout_init(&logsoftc.sc_callout, CALLOUT_MPSAFE); + mtx_unlock(&log_mtx); fsetown(td->td_proc->p_pid, &logsoftc.sc_sigio); /* signal process only */ + mtx_lock(&log_mtx); if (log_wakeups_per_second < 1) { printf("syslog wakeup is less than one. Adjusting to 1.\n"); log_wakeups_per_second = 1; } callout_reset(&logsoftc.sc_callout, hz / log_wakeups_per_second, logtimeout, NULL); + mtx_unlock(&log_mtx); return (0); } @@ -113,9 +125,11 @@ logclose(struct cdev *dev, int flag, int mode, struct thread *td) { + mtx_lock(&log_mtx); log_open = 0; callout_stop(&logsoftc.sc_callout); logsoftc.sc_state = 0; + mtx_unlock(&log_mtx); funsetown(&logsoftc.sc_sigio); return (0); } @@ -134,14 +148,18 @@ splx(s); return (EWOULDBLOCK); } + mtx_lock(&log_mtx); logsoftc.sc_state |= LOG_RDWAIT; + mtx_unlock(&log_mtx); if ((error = tsleep(mbp, LOG_RDPRI | PCATCH, "klog", 0))) { splx(s); return (error); } } splx(s); + mtx_lock(&log_mtx); logsoftc.sc_state &= ~LOG_RDWAIT; + mtx_unlock(&log_mtx); while (uio->uio_resid > 0) { l = imin(sizeof(buf), uio->uio_resid); @@ -178,8 +196,11 @@ logtimeout(void *arg) { - if (!log_open) + mtx_lock(&log_mtx); + if (!log_open) { + mtx_unlock(&log_mtx); return; + } if (log_wakeups_per_second < 1) { printf("syslog wakeup is less than one. Adjusting to 1.\n"); log_wakeups_per_second = 1; @@ -187,6 +208,7 @@ if (msgbuftrigger == 0) { callout_reset(&logsoftc.sc_callout, hz / log_wakeups_per_second, logtimeout, NULL); + mtx_unlock(&log_mtx); return; } msgbuftrigger = 0; @@ -199,6 +221,7 @@ } callout_reset(&logsoftc.sc_callout, hz / log_wakeups_per_second, logtimeout, NULL); + mtx_unlock(&log_mtx); } /*ARGSUSED*/ @@ -217,10 +240,12 @@ break; case FIOASYNC: + mtx_lock(&log_mtx); if (*(int *)data) logsoftc.sc_state |= LOG_ASYNC; else logsoftc.sc_state &= ~LOG_ASYNC; + mtx_unlock(&log_mtx); break; case FIOSETOWN: @@ -249,6 +274,7 @@ log_drvinit(void *unused) { + mtx_init(&log_mtx, "log_mtx", NULL, MTX_DEF); make_dev(&log_cdevsw, 0, UID_ROOT, GID_WHEEL, 0600, "klog"); } --- //depot/vendor/freebsd/src/sys/kern/subr_prf.c 2004/07/10 21:40:38 +++ //depot/user/rwatson/netperf/sys/kern/subr_prf.c 2004/07/10 22:08:23 @@ -84,6 +84,9 @@ size_t remain; }; +/* + * XXXRW: We access subr_log.c's log_open variable unlocked. + */ extern int log_open; static void msglogchar(int c, int pri); --- //depot/vendor/freebsd/src/sys/kern/uipc_mbuf.c 2004/06/11 18:20:48 +++ //depot/user/rwatson/netperf/sys/kern/uipc_mbuf.c 2004/06/12 17:06:18 @@ -413,12 +413,12 @@ np = &n->m_next; } if (top == NULL) - mbstat.m_mcfail++; /* XXX: No consistency. */ + atomic_add_long(&mbstat.m_mcfail, 1); return (top); nospace: m_freem(top); - mbstat.m_mcfail++; /* XXX: No consistency. */ + atomic_add_long(&mbstat.m_mcfail, 1); return (NULL); } @@ -478,7 +478,7 @@ return top; nospace: m_freem(top); - mbstat.m_mcfail++; /* XXX: No consistency. */ + atomic_add_long(&mbstat.m_mcfail, 1); return (NULL); } @@ -580,7 +580,7 @@ nospace: m_freem(top); - mbstat.m_mcfail++; /* XXX: No consistency. */ + atomic_add_long(&mbstat.m_mcfail, 1); return (NULL); } @@ -740,7 +740,7 @@ return (m); bad: m_freem(n); - mbstat.m_mpfail++; /* XXX: No consistency. */ + atomic_add_long(&mbstat.m_mpfail, 1); return (NULL); } --- //depot/vendor/freebsd/src/sys/kern/uipc_socket.c 2004/07/12 21:45:34 +++ //depot/user/rwatson/netperf/sys/kern/uipc_socket.c 2004/07/14 20:52:14 @@ -295,6 +295,7 @@ * SO_ACCEPTCONN before the call to pru_listen()? * XXXRW: General atomic test-and-set concerns here also. */ + /* Unlocked read. */ if (so->so_state & (SS_ISCONNECTED | SS_ISCONNECTING | SS_ISDISCONNECTING)) return (EINVAL); @@ -323,6 +324,7 @@ KASSERT(so->so_count == 0, ("socket %p so_count not 0", so)); SOCK_LOCK_ASSERT(so); + /* XXXRW: Why would SS_NOFDREF be unset here? so_count is 0. */ if (so->so_pcb != NULL || (so->so_state & SS_NOFDREF) == 0) { SOCK_UNLOCK(so); return; @@ -381,6 +383,45 @@ sodealloc(so); } +#if 0 +static void +dump_socket(struct socket *so) +{ + + printf("so = %p\n", so); + printf(" so_count = %d\n", so->so_count); + printf(" so_type = %d\n", so->so_type); + printf(" so_options = %d\n", so->so_options); + printf(" so_linger = %d\n", so->so_linger); + printf(" so_state = %d\n", so->so_state); + printf(" so_qstate = %d\n", so->so_qstate); + printf(" so_pcb = %p\n", so->so_pcb); + printf(" so_proto = %p\n", so->so_proto); + printf(" pr_type = %d\n", so->so_proto->pr_type); + printf(" pr_domain = %p\n", so->so_proto->pr_domain); + printf(" dom_family = %d\n", so->so_proto->pr_domain->dom_family); + printf(" dom_name = %s\n", so->so_proto->pr_domain->dom_name); + printf(" pr_protocol = %d\n", so->so_proto->pr_protocol); + printf(" pr_flags = %d\n", so->so_proto->pr_flags); + printf(" so_head = %p\n", so->so_head); + printf(" TAILQ_FIRST(so_incomp) = %p\n", TAILQ_FIRST(&so->so_incomp)); + printf(" TAILQ_FIRST(so_comp) = %p\n", TAILQ_FIRST(&so->so_comp)); + printf(" so_qlen = %d\n", so->so_qlen); + printf(" so_incqlen = %d\n", so->so_incqlen); + printf(" so_qlimit = %d\n", so->so_qlimit); + printf(" so_timeo = %d\n", so->so_timeo); + printf(" so_error = %d\n", so->so_error); + printf(" so_sigio = %p\n", so->so_sigio); + printf(" so_oobmark = %lu\n", so->so_oobmark); + printf(" so_rcv.sb_state = %d\n", so->so_rcv.sb_state); + printf(" so_snd.sb_state = %d\n", so->so_snd.sb_state); + printf(" so_upcall = %p\n", so->so_upcall); + printf(" so_upcallarg = %p\n", so->so_upcallarg); + printf(" so_cred = %p\n", so->so_cred); + printf(" so_gencnt = %d\n", (int)so->so_gencnt); +} +#endif + /* * Close a socket on last file table reference removal. * Initiate disconnect if connected. @@ -396,7 +437,26 @@ { int error = 0; + KASSERT(!(so->so_state & SS_NOFDREF), ("soclose: SS_NOFDREF on enter")); + funsetown(&so->so_sigio); + /*- + * XXXRW: Lots of locking problems here. For one thing, + * we should probably clear the SO_ACCEPTCONN flag and + * push that down to the protocol layer. Also, the locking + * for the queue draining is probably not right either: + * what prevents new ones from being inserted after we get + * past this? Ideally, the downcall would prevent this, but + * there isn't one. + */ + /* + * XXXRW: soclose() should be split into two parts: one to handle + * listen sockets, and the other to handle the remainder. To + * prevent races with the protocol code, we need to detach before + * draining the queues. For non-listen sockets, the disconnect + * has to happen before the detach. + */ + /* Unlocked read. */ if (so->so_options & SO_ACCEPTCONN) { struct socket *sp; ACCEPT_LOCK(); @@ -422,6 +482,7 @@ } if (so->so_pcb == NULL) goto discard; + /* XXXRW: so_state locking? */ if (so->so_state & SS_ISCONNECTED) { if ((so->so_state & SS_ISDISCONNECTING) == 0) { error = sodisconnect(so); @@ -506,6 +567,7 @@ * This allows user to disconnect by connecting to, e.g., * a null address. */ + /* Unlocked read. */ if (so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING) && ((so->so_proto->pr_flags & PR_CONNREQUIRED) || (error = sodisconnect(so)))) @@ -530,6 +592,7 @@ { int error; + /* Unlocked read. */ if ((so->so_state & SS_ISCONNECTED) == 0) return (ENOTCONN); if (so->so_state & SS_ISDISCONNECTING) @@ -620,8 +683,6 @@ #define snderr(errno) { error = (errno); goto release; } SOCKBUF_LOCK(&so->so_snd); -restart: - SOCKBUF_LOCK_ASSERT(&so->so_snd); error = sblock(&so->so_snd, SBLOCKWAIT(flags)); if (error) goto out_locked; @@ -660,11 +721,10 @@ (atomic || space < so->so_snd.sb_lowat || space < clen)) { if ((so->so_state & SS_NBIO) || (flags & MSG_NBIO)) snderr(EWOULDBLOCK); - sbunlock(&so->so_snd); error = sbwait(&so->so_snd); if (error) - goto out_locked; - goto restart; + goto release; + continue; } SOCKBUF_UNLOCK(&so->so_snd); mp = ⊤ @@ -776,6 +836,15 @@ break; } } while (space > 0 && atomic); + /* + * XXXRW: There may be a race condition here regarding + * SO_DONTROUTE. We hold the sblock() so in theory + * there won't be other consumers of the socket doing + * a pru_send(), but SO_DONTROUTE is also consumed + * from the protocol side. It sounds like we might + * want to use MSG_DONTROUTE here, if the semantics are + * right. + */ if (dontroute) { SOCK_LOCK(so); so->so_options |= SO_DONTROUTE; @@ -786,7 +855,7 @@ * done could be out of date. We could have recieved * a reset packet in an interrupt or maybe we slept * while doing page faults in uiomove() etc. We could - * probably recheck again inside the splnet() protection + * probably recheck again inside the locking protection * here, but there are probably other places that this * also happens. We must rethink this. */ @@ -804,6 +873,10 @@ /* If there is more to send set PRUS_MORETOCOME */ (resid > 0 && space > 0) ? PRUS_MORETOCOME : 0, top, addr, control, td); + /* + * XXXRW: Second half of above comment on SO_DONTROUTE. + * and so_options. + */ if (dontroute) { SOCK_LOCK(so); so->so_options &= ~SO_DONTROUTE; @@ -974,16 +1047,17 @@ return (soreceive_rcvoob(so, uio, flags)); if (mp != NULL) *mp = NULL; + /* Unlocked read. */ if (so->so_state & SS_ISCONFIRMING && uio->uio_resid) (*pr->pr_usrreqs->pru_rcvd)(so, 0); SOCKBUF_LOCK(&so->so_rcv); -restart: - SOCKBUF_LOCK_ASSERT(&so->so_rcv); error = sblock(&so->so_rcv, SBLOCKWAIT(flags)); if (error) goto out; +restart: + SOCKBUF_LOCK_ASSERT(&so->so_rcv); m = so->so_rcv.sb_mb; /* * If we have less data than requested, block awaiting more @@ -1001,9 +1075,8 @@ (so->so_rcv.sb_cc < so->so_rcv.sb_lowat || ((flags & MSG_WAITALL) && uio->uio_resid <= so->so_rcv.sb_hiwat)) && m->m_nextpkt == NULL && (pr->pr_flags & PR_ATOMIC) == 0)) { - KASSERT(m != NULL || !so->so_rcv.sb_cc, - ("receive: m == %p so->so_rcv.sb_cc == %u", - m, so->so_rcv.sb_cc)); + KASSERT(!(m == NULL && so->so_rcv.sb_cc), + ("m %p so->so_rcv.sb_cc %u", m, so->so_rcv.sb_cc)); if (so->so_error) { if (m != NULL) goto dontblock; @@ -1024,6 +1097,7 @@ m = so->so_rcv.sb_mb; goto dontblock; } + /* XXXRW: so_state locking? */ if ((so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING)) == 0 && (so->so_proto->pr_flags & PR_CONNREQUIRED)) { error = ENOTCONN; @@ -1031,6 +1105,7 @@ } if (uio->uio_resid == 0) goto release; + /* XXXRW: so_state locking? */ if ((so->so_state & SS_NBIO) || (flags & (MSG_DONTWAIT|MSG_NBIO))) { error = EWOULDBLOCK; @@ -1038,10 +1113,9 @@ } SBLASTRECORDCHK(&so->so_rcv); SBLASTMBUFCHK(&so->so_rcv); - sbunlock(&so->so_rcv); error = sbwait(&so->so_rcv); if (error) - goto out; + goto release; goto restart; } dontblock: @@ -1061,6 +1135,7 @@ * readers from pulling off the front of the socket buffer. */ SOCKBUF_LOCK_ASSERT(&so->so_rcv); + KASSERT(error == 0, ("unexpected state, error %u", error)); if (uio->uio_td) uio->uio_td->td_proc->p_stats->p_ru.ru_msgrcv++; KASSERT(m == so->so_rcv.sb_mb, ("soreceive: m != so->so_rcv.sb_mb")); @@ -1070,10 +1145,25 @@ if (pr->pr_flags & PR_ADDR) { KASSERT(m->m_type == MT_SONAME, ("m->m_type == %d", m->m_type)); - orig_resid = 0; - if (psa != NULL) + if (psa != NULL) { *psa = sodupsockaddr(mtod(m, struct sockaddr *), M_NOWAIT); + if (*psa == NULL) { + error = ENOMEM; + goto release; + } + /* + * XXXRW: In the rwatson_netperf branch, we don't + * release the socket buffer lock here because + * we always use M_NOWAIT. In the main tree, + * or if we restore conditional waiting, we + * need to refresh nextpacket from the socket + * buffer version of the head mbuf, or we may + * have a stale value if it was previously NULL + * and now isn't. + */ + nextrecord = m->m_nextpkt; + } if (flags & MSG_PEEK) { m = m->m_next; } else { @@ -1082,6 +1172,7 @@ m = so->so_rcv.sb_mb; sockbuf_pushsync(&so->so_rcv, nextrecord); } + orig_resid = 0; } /* @@ -1096,6 +1187,14 @@ do { if (flags & MSG_PEEK) { + /* + * XXXRW: In the BSD/OS version of this + * code, m_copym() is called with M_TRYWAIT, + * and we catch allcation failures and + * jump to release with error of ENOBUFS. + * For consistency with the current + * implementation, we don't. + */ if (controlp != NULL) { *controlp = m_copy(m, 0, m->m_len); controlp = &(*controlp)->m_next; @@ -1116,6 +1215,12 @@ m = so->so_rcv.sb_mb; } } while (m != NULL && m->m_type == MT_CONTROL); + /* + * XXXRW: Since we're dropping the socket buffer locks, and + * may have modified the mbuf list on the socket buffer, push + * out the latest version so it can be seen by any other code + * that accesses the buffer in the mean time. + */ if ((flags & MSG_PEEK) == 0) sockbuf_pushsync(&so->so_rcv, nextrecord); if (cm != NULL) { @@ -1127,10 +1232,21 @@ } else m_freem(cm); } + /* + * XXXRW: During externalization, additional mbuf chains + * may have been added to the socket buffer. Update our + * local cache to avoid using a stale value and corrupting + * the list. + */ nextrecord = so->so_rcv.sb_mb->m_nextpkt; orig_resid = 0; } if (m != NULL) { + /* + * XXXRW: Before the advent of sockbuf_pushsync() above, it + * was necessary to do the equivilent work here. Now, we + * just assert that it is the case. + */ if ((flags & MSG_PEEK) == 0) { KASSERT(m->m_nextpkt == nextrecord, ("soreceive: post-control, nextrecord !sync")); @@ -1145,6 +1261,11 @@ if (type == MT_OOBDATA) flags |= MSG_OOB; } else { + /* + * XXXRW: Before the advent of sockbuf_pushsync() above, it + * was necessary to update sb_mb to nextrecord here. Now, we + * just assert that is the case. + */ if ((flags & MSG_PEEK) == 0) { KASSERT(so->so_rcv.sb_mb == nextrecord, ("soreceive: sb_mb != nextrecord")); @@ -1248,14 +1369,7 @@ so->so_rcv.sb_mb = m_free(m); m = so->so_rcv.sb_mb; } - if (m != NULL) { - m->m_nextpkt = nextrecord; - if (nextrecord == NULL) - so->so_rcv.sb_lastrecord = m; - } else { - so->so_rcv.sb_mb = nextrecord; - SB_EMPTY_FIXUP(&so->so_rcv); - } + sockbuf_pushsync(&so->so_rcv, nextrecord); SBLASTRECORDCHK(&so->so_rcv); SBLASTMBUFCHK(&so->so_rcv); } @@ -1304,6 +1418,12 @@ /* * Notify the protocol that some data has been * drained before blocking. + * + * XXXRW: We drop the socket buffer lock here + * because we know the protocol will need to do its + * own locking. However, after calling pru_rcvd() + * and re-grabbing the buffer mutex, we don't + * re-check the condition before sleeping. */ if (pr->pr_flags & PR_WANTRCVD && so->so_pcb != NULL) { SOCKBUF_UNLOCK(&so->so_rcv); @@ -1313,8 +1433,10 @@ SBLASTRECORDCHK(&so->so_rcv); SBLASTMBUFCHK(&so->so_rcv); error = sbwait(&so->so_rcv); - if (error) + if (error) { + error = 0; goto release; + } m = so->so_rcv.sb_mb; if (m != NULL) nextrecord = m->m_nextpkt; @@ -1343,6 +1465,11 @@ } SBLASTRECORDCHK(&so->so_rcv); SBLASTMBUFCHK(&so->so_rcv); + /* + * XXXRW: We drop the socket buffer lock before calling + * down into the protocol. Is that OK in the calling + * context? + */ if (pr->pr_flags & PR_WANTRCVD && so->so_pcb) { SOCKBUF_UNLOCK(&so->so_rcv); (*pr->pr_usrreqs->pru_rcvd)(so, flags); @@ -1351,10 +1478,8 @@ } SOCKBUF_LOCK_ASSERT(&so->so_rcv); if (orig_resid == uio->uio_resid && orig_resid && - (flags & MSG_EOR) == 0 && (so->so_rcv.sb_state & SBS_CANTRCVMORE) == 0) { - sbunlock(&so->so_rcv); - goto restart; - } + (flags & MSG_EOR) == 0 && (so->so_rcv.sb_state & SBS_CANTRCVMORE) == 0) + goto restart; /* XXX multi-counts msgs */ if (flagsp != NULL) *flagsp |= flags; @@ -1422,6 +1547,7 @@ sizeof(*sb) - offsetof(struct sockbuf, sb_startzero)); SOCKBUF_UNLOCK(sb); + /* XXXRW: is passing in sb_mb this way really safe? */ SOCKBUF_LOCK_INIT(&asb, "so_rcv"); if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose != NULL) (*pr->pr_domain->dom_dispose)(asb.sb_mb); @@ -1435,20 +1561,31 @@ struct socket *so; struct sockopt *sopt; { - struct accept_filter_arg *afap = NULL; + struct accept_filter_arg *afap; struct accept_filter *afp; - struct so_accf *af = so->so_accf; + struct so_accf *newaf; int error = 0; + newaf = NULL; + afap = NULL; + + /* + * XXXRW: Configuring accept filters should be an atomic + * test-and-set operation to prevent races during setup and + * and attach. There may be more general issues of racing + * and ordering here that are not yet addressed by locking. + */ /* do not set/remove accept filters on non listen sockets */ + SOCK_LOCK(so); if ((so->so_options & SO_ACCEPTCONN) == 0) { - error = EINVAL; - goto out; + SOCK_UNLOCK(so); + return (EINVAL); } /* removing the filter */ if (sopt == NULL) { - if (af != NULL) { + if (so->so_accf != NULL) { + struct so_accf *af = so->so_accf; if (af->so_accept_filter != NULL && af->so_accept_filter->accf_destroy != NULL) { af->so_accept_filter->accf_destroy(so); @@ -1460,47 +1597,80 @@ so->so_accf = NULL; } so->so_options &= ~SO_ACCEPTFILTER; + SOCK_UNLOCK(so); return (0); } - /* adding a filter */ - /* must remove previous filter first */ - if (af != NULL) { - error = EINVAL; - goto out; - } + SOCK_UNLOCK(so); + + /*- + * Adding a filter. + * + * Do memory allocation, copyin, and filter lookup now while we're + * not holding any locks. Avoids sleeping with a mutex, as well + * as introducing a lock order between accept filter locks and + * socket locks here. + */ + MALLOC(afap, struct accept_filter_arg *, sizeof(*afap), M_TEMP, + M_WAITOK); /* don't put large objects on the kernel stack */ - MALLOC(afap, struct accept_filter_arg *, sizeof(*afap), M_TEMP, M_WAITOK); error = sooptcopyin(sopt, afap, sizeof *afap, sizeof *afap); afap->af_name[sizeof(afap->af_name)-1] = '\0'; afap->af_arg[sizeof(afap->af_arg)-1] = '\0'; - if (error) - goto out; + if (error) { + FREE(afap, M_TEMP); + return (error); + } afp = accept_filt_get(afap->af_name); if (afp == NULL) { - error = ENOENT; + FREE(afap, M_TEMP); + return (ENOENT); + } + + /* + * Allocate the new accept filter instance storage. We may have + * to free it again later if we fail to attach it. If attached + * properly, 'newaf' is NULLed to avoid a free() while in use. + */ + MALLOC(newaf, struct so_accf *, sizeof(*newaf), M_ACCF, M_WAITOK | + M_ZERO); + if (afp->accf_create != NULL && afap->af_name[0] != '\0') { + int len = strlen(afap->af_name) + 1; + MALLOC(newaf->so_accept_filter_str, char *, len, M_ACCF, + M_WAITOK); + strcpy(newaf->so_accept_filter_str, afap->af_name); + } + + SOCK_LOCK(so); + /* must remove previous filter first */ + if (so->so_accf != NULL) { + error = EINVAL; goto out; } - MALLOC(af, struct so_accf *, sizeof(*af), M_ACCF, M_WAITOK | M_ZERO); + /* + * Invoke the accf_create() method of the filter if required. + * XXXRW: the socket mutex is held over this call, so the + * create method cannot block. This may be something we have + * to change, but it would require addressing possible races. + */ if (afp->accf_create != NULL) { - if (afap->af_name[0] != '\0') { - int len = strlen(afap->af_name) + 1; - - MALLOC(af->so_accept_filter_str, char *, len, M_ACCF, M_WAITOK); - strcpy(af->so_accept_filter_str, afap->af_name); - } - af->so_accept_filter_arg = afp->accf_create(so, afap->af_arg); - if (af->so_accept_filter_arg == NULL) { - FREE(af->so_accept_filter_str, M_ACCF); - FREE(af, M_ACCF); - so->so_accf = NULL; + newaf->so_accept_filter_arg = + afp->accf_create(so, afap->af_arg); + if (newaf->so_accept_filter_arg == NULL) { error = EINVAL; goto out; } } - af->so_accept_filter = afp; - so->so_accf = af; + newaf->so_accept_filter = afp; + so->so_accf = newaf; so->so_options |= SO_ACCEPTFILTER; + newaf = NULL; out: + SOCK_UNLOCK(so); + if (newaf != NULL) { + if (newaf->so_accept_filter_str != NULL) + FREE(newaf->so_accept_filter_str, M_ACCF); + FREE(newaf, M_ACCF); + } if (afap != NULL) FREE(afap, M_TEMP); return (error); @@ -1788,15 +1958,18 @@ switch (sopt->sopt_name) { #ifdef INET case SO_ACCEPTFILTER: + /* Unlocked read. */ if ((so->so_options & SO_ACCEPTCONN) == 0) return (EINVAL); MALLOC(afap, struct accept_filter_arg *, sizeof(*afap), M_TEMP, M_WAITOK | M_ZERO); + SOCK_LOCK(so); if ((so->so_options & SO_ACCEPTFILTER) != 0) { strcpy(afap->af_name, so->so_accf->so_accept_filter->accf_name); if (so->so_accf->so_accept_filter_str != NULL) strcpy(afap->af_arg, so->so_accf->so_accept_filter_str); } + SOCK_UNLOCK(so); error = sooptcopyout(sopt, afap, sizeof(*afap)); FREE(afap, M_TEMP); break; @@ -1826,6 +1999,7 @@ case SO_TIMESTAMP: case SO_BINTIME: case SO_NOSIGPIPE: + /* Unlocked read. */ optval = so->so_options & sopt->sopt_name; integer: error = sooptcopyout(sopt, &optval, sizeof optval); @@ -2031,10 +2205,16 @@ { int revents = 0; + /* + * XXXRW: Lots of unlocked reads, and some writes. Probably + * some more locking is called for here, especially when + * setting the sb_sel flags. + */ if (events & (POLLIN | POLLRDNORM)) if (soreadable(so)) revents |= events & (POLLIN | POLLRDNORM); + /* Unlocked read. */ if (events & POLLINIGNEOF) if (so->so_rcv.sb_cc >= so->so_rcv.sb_lowat || !TAILQ_EMPTY(&so->so_comp) || so->so_error) @@ -2077,6 +2257,7 @@ switch (kn->kn_filter) { case EVFILT_READ: + /* Unlocked read. */ if (so->so_options & SO_ACCEPTCONN) kn->kn_fop = &solisten_filtops; else @@ -2193,6 +2374,7 @@ { struct socket *so = kn->kn_fp->f_data; + /* Unlocked read. */ kn->kn_data = so->so_qlen; return (! TAILQ_EMPTY(&so->so_comp)); } --- //depot/vendor/freebsd/src/sys/kern/uipc_socket2.c 2004/06/26 19:15:28 +++ //depot/user/rwatson/netperf/sys/kern/uipc_socket2.c 2004/06/26 20:54:44 @@ -106,21 +106,43 @@ { SOCK_LOCK(so); + /* + * XXXRW: so_state locking? + * + * XXXRW: Why not clear SS_ISDISCONNECTED, SS_ISCONFIRMING? + */ so->so_state &= ~(SS_ISCONNECTED|SS_ISDISCONNECTING); so->so_state |= SS_ISCONNECTING; SOCK_UNLOCK(so); } +/* + * soisconnected() transitions a socket to a fully connected state. If + * so->so_head is non-NULL, we transition it from the incompletely + * connected queue to the completely connected queue. Otherwise, we + * just wake up the socket since it was an out-bound connection. + */ void soisconnected(so) struct socket *so; { struct socket *head; +#ifdef INVARIANTS + int need_lock = !SOCK_OWNED(so); +#endif + KASSERT(need_lock == 1, ("soisconnected(): called with lock!")); + /* XXXRW: so_state locking? */ SOCK_LOCK(so); so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING|SS_ISCONFIRMING); so->so_state |= SS_ISCONNECTED; SOCK_UNLOCK(so); + + /* + * XXXRW: Maybe an unlocked read of so_head would be desirable + * here? Unlocked read of so_qstate probably not a good idea + * regardless. so_options handling here is a little unsavory. + */ ACCEPT_LOCK(); head = so->so_head; if (head != NULL && (so->so_qstate & SQ_INCOMP)) { @@ -135,15 +157,37 @@ sorwakeup(head); wakeup_one(&head->so_timeo); } else { - ACCEPT_UNLOCK(); + void (*so_upcall)(struct socket *, void *, int); + void *so_upcallarg; + /* + * XXXRW: Should probably copy the upcall fields to + * local stack variables while holding the socket + * lock (and clear the option), then release the + * lock and make the call. This will prevent lock + * order reversals/recursion between this code and + * the upcall implementation, which will likely + * also want to frob the socket using locks. + * + * NOTE: We keep a local copy of the function + * pointer so we can invoke the upcall without + * holding locks. However, we also have to copy + * in the upcall fields from the head because the + * filter may need to be called more than once. + */ SOCK_LOCK(so); - so->so_upcall = + so_upcall = so->so_upcall = head->so_accf->so_accept_filter->accf_callback; - so->so_upcallarg = head->so_accf->so_accept_filter_arg; + so_upcallarg = so->so_upcallarg = + head->so_accf->so_accept_filter_arg; so->so_rcv.sb_flags |= SB_UPCALL; so->so_options &= ~SO_ACCEPTFILTER; SOCK_UNLOCK(so); - so->so_upcall(so, so->so_upcallarg, M_TRYWAIT); + ACCEPT_UNLOCK(); + /* + * Call with our existing reference, but without + * any locks. + */ + so_upcall(so, so_upcallarg, M_TRYWAIT); } return; } @@ -233,6 +277,8 @@ so->so_type = head->so_type; so->so_options = head->so_options &~ SO_ACCEPTCONN; so->so_linger = head->so_linger; + /* XXXRW: so_state locking? */ + /* XXXRW: should mask additional head->so_state bits here? */ so->so_state = head->so_state | SS_NOFDREF; so->so_proto = head->so_proto; so->so_timeo = head->so_timeo; @@ -277,6 +323,7 @@ } ACCEPT_UNLOCK(); if (connstatus) { + /* XXXRW: so_state locking? */ so->so_state |= connstatus; sorwakeup(head); wakeup_one(&head->so_timeo); @@ -405,6 +452,10 @@ } KNOTE(&sb->sb_sel.si_note, 0); SOCKBUF_UNLOCK(sb); + /* + * XXXRW: What locks should be held over what parts of the + * following? Currently we hold none. + */ if ((so->so_state & SS_ASYNC) && so->so_sigio != NULL) pgsigio(&so->so_sigio, SIGIO, 0); if (sb->sb_flags & SB_UPCALL) @@ -451,7 +502,7 @@ register struct socket *so; u_long sndcc, rcvcc; { - struct thread *td = curthread; + struct thread *td = curthread; /* XXX */ SOCKBUF_LOCK(&so->so_snd); SOCKBUF_LOCK(&so->so_rcv); @@ -515,6 +566,13 @@ /* * td will only be NULL when we're in an interrupt * (e.g. in tcp_input()) + * + * XXXRW: This comment is true, but only because the caller passed + * in NULL, not for the 4.x reason that there is no thread + * available. Need to be careful of callers that do this wrong; + * I suspect many do it wrong, and therefore many socket buffers + * end up with the wrong limits, especially via the soreserve() + * path. */ if (cc > sb_max_adj) return (0); @@ -1368,6 +1426,7 @@ xso->so_type = so->so_type; xso->so_options = so->so_options; xso->so_linger = so->so_linger; + /* Unlocked read. */ xso->so_state = so->so_state; xso->so_pcb = so->so_pcb; xso->xso_protocol = so->so_proto->pr_protocol; --- //depot/vendor/freebsd/src/sys/kern/uipc_syscalls.c 2004/07/10 15:45:28 +++ //depot/user/rwatson/netperf/sys/kern/uipc_syscalls.c 2004/07/10 22:08:23 @@ -278,6 +278,7 @@ error = fgetsock(td, uap->s, &head, &fflag); if (error) goto done2; + /* Unlocked read. */ if ((head->so_options & SO_ACCEPTCONN) == 0) { error = EINVAL; goto done; @@ -285,6 +286,10 @@ error = falloc(td, &nfp, &fd); if (error) goto done; + /* + * Unlocked reads. + * XXXRW: Dubious use of so->so_error. + */ ACCEPT_LOCK(); if ((head->so_state & SS_NBIO) && TAILQ_EMPTY(&head->so_comp)) { ACCEPT_UNLOCK(); @@ -333,6 +338,11 @@ /* An extra reference on `nfp' has been held for us by falloc(). */ td->td_retval[0] = fd; + /* + * XXXRW: Might make life simpler to grab the socket buffer lock + * here so it's held when KNOTE() calls back into the socket + * code. + */ /* connection has been removed from the listen queue */ KNOTE(&head->so_rcv.sb_sel.si_note, 0); @@ -482,6 +492,7 @@ NET_LOCK_GIANT(); if ((error = fgetsock(td, fd, &so, NULL)) != 0) goto done2; + /* XXXRW: so_state locking? */ if (so->so_state & SS_ISCONNECTING) { error = EALREADY; goto done1; @@ -496,12 +507,14 @@ error = soconnect(so, sa, td); if (error) goto bad; + /* XXXRW: so_state locking? */ if ((so->so_state & SS_NBIO) && (so->so_state & SS_ISCONNECTING)) { error = EINPROGRESS; goto done1; } s = splnet(); SOCK_LOCK(so); + /* XXXRW: so_state locking? */ while ((so->so_state & SS_ISCONNECTING) && so->so_error == 0) { error = msleep(&so->so_timeo, SOCK_MTX(so), PSOCK | PCATCH, "connec", 0); @@ -518,6 +531,7 @@ SOCK_UNLOCK(so); splx(s); bad: + /* XXXRW: so_state locking? */ if (!interrupted) so->so_state &= ~SS_ISCONNECTING; if (error == ERESTART) @@ -1429,6 +1443,7 @@ NET_LOCK_GIANT(); if ((error = fgetsock(td, uap->fdes, &so, NULL)) != 0) goto done2; + /* XXXRW: so_state locking? */ if ((so->so_state & (SS_ISCONNECTED|SS_ISCONFIRMING)) == 0) { error = ENOTCONN; goto done1; @@ -1670,6 +1685,7 @@ error = EINVAL; goto done; } + /* XXXRW: so_state locking? */ if ((so->so_state & SS_ISCONNECTED) == 0) { error = ENOTCONN; goto done; @@ -1761,6 +1777,7 @@ * before going to the extra work of constituting the sf_buf. */ SOCKBUF_LOCK(&so->so_snd); + /* XXXRW: so_state locking? */ if ((so->so_state & SS_NBIO) && sbspace(&so->so_snd) <= 0) { if (so->so_snd.sb_state & SBS_CANTSENDMORE) error = EPIPE; @@ -1826,6 +1843,7 @@ * Get the page from backing store. */ bsize = vp->v_mount->mnt_stat.f_iosize; + mtx_lock(&Giant); /* VFS */ vn_lock(vp, LK_SHARED | LK_NOPAUSE | LK_RETRY, td); /* * XXXMAC: Because we don't have fp->f_cred here, @@ -1837,6 +1855,7 @@ IO_VMIO | ((MAXBSIZE / bsize) << IO_SEQSHIFT), td->td_ucred, NOCRED, &resid, td); VOP_UNLOCK(vp, 0, td); + mtx_unlock(&Giant); /* VFS */ if (error) VM_OBJECT_LOCK(obj); vm_page_lock_queues(); @@ -1947,6 +1966,7 @@ * after checking the connection state above in order to avoid * a race condition with sbwait(). */ + /* XXXRW: so_state locking? */ if (sbspace(&so->so_snd) < so->so_snd.sb_lowat) { if (so->so_state & SS_NBIO) { m_freem(m); --- //depot/vendor/freebsd/src/sys/kern/uipc_usrreq.c 2004/07/02 07:40:38 +++ //depot/user/rwatson/netperf/sys/kern/uipc_usrreq.c 2004/07/03 05:01:01 @@ -372,6 +372,7 @@ * Note: A better implementation would complain * if not equal to the peer's address. */ + /* Unlocked read. */ if ((so->so_state & SS_ISCONNECTED) == 0) { if (nam != NULL) { error = unp_connect(so, nam, td); @@ -383,6 +384,7 @@ } } + /* Unlocked read. */ if (so->so_snd.sb_state & SBS_CANTSENDMORE) { error = EPIPE; break; @@ -451,6 +453,7 @@ sb->st_blksize = so->so_snd.sb_hiwat; if (so->so_type == SOCK_STREAM && unp->unp_conn != NULL) { so2 = unp->unp_conn->unp_socket; + /* Unlocked read. */ sb->st_blksize += so2->so_rcv.sb_cc; } sb->st_dev = NODEV; @@ -1400,6 +1403,10 @@ UNP_LOCK_ASSERT(); + /* + * XXXRW: unp_gcing seems like a bad idea. Use a real lock + * instead? + */ if (unp_gcing) return; unp_gcing = 1; @@ -1482,6 +1489,9 @@ * of a signal. If sbwait does return * an error, we'll go into an infinite * loop. Delete all of this for now. + * + * XXXRW: We will need to lock the socket + * buffer here if we enable this code. */ (void) sbwait(&so->so_rcv); goto restart; --- //depot/vendor/freebsd/src/sys/net/if.c 2004/06/22 20:15:41 +++ //depot/user/rwatson/netperf/sys/net/if.c 2004/06/23 00:01:14 @@ -366,6 +366,10 @@ struct sockaddr_dl *sdl; struct ifaddr *ifa; + /* + * XXXRW: Shouldn't we add to the global list only once the ifnet + * is ready for use? + */ IF_AFDATA_LOCK_INIT(ifp); ifp->if_afdata_initialized = 0; IFNET_WLOCK(); --- //depot/vendor/freebsd/src/sys/net/if_gif.c 2004/07/15 08:30:31 +++ //depot/user/rwatson/netperf/sys/net/if_gif.c 2004/07/15 22:19:17 @@ -89,6 +89,10 @@ * gif_mtx protects the global gif_softc_list. * XXX: Per-softc locking is still required. */ +/* + * XXXRW: Note that gif_mtx only protects global gif-related data, not + * per-softc data. See also netinet/in_gif.c for locking needs. + */ static struct mtx gif_mtx; static MALLOC_DEFINE(M_GIF, "gif", "Generic Tunnel Interface"); static LIST_HEAD(, gif_softc) gif_softc_list; @@ -501,6 +505,9 @@ } /* XXX how should we handle IPv6 scope on SIOC[GS]IFPHYADDR? */ +/* + * XXXRW: per-gif softc locking required. + */ int gif_ioctl(ifp, cmd, data) struct ifnet *ifp; @@ -757,8 +764,10 @@ int s; int error = 0; + /* + * XXXRW: per-gif softc locking required. + */ s = splnet(); - mtx_lock(&gif_mtx); LIST_FOREACH(sc2, &gif_softc_list, gif_list) { if (sc2 == sc) @@ -787,6 +796,9 @@ } mtx_unlock(&gif_mtx); + /* + * XXXRW: Lock gif softc fields. + */ /* XXX we can detach from both, but be polite just in case */ if (sc->gif_psrc) switch (sc->gif_psrc->sa_family) { --- //depot/vendor/freebsd/src/sys/net/if_gre.c 2004/07/15 08:30:31 +++ //depot/user/rwatson/netperf/sys/net/if_gre.c 2004/07/15 22:19:17 @@ -95,7 +95,8 @@ /* * gre_mtx protects all global variables in if_gre.c. - * XXX: gre_softc data not protected yet. + * + * XXXRW: It does not protect softc-specific data. */ struct mtx gre_mtx; static MALLOC_DEFINE(M_GRE, GRENAME, "Generic Routing Encapsulation"); --- //depot/vendor/freebsd/src/sys/net/if_gre.h 2004/03/22 16:06:54 +++ //depot/user/rwatson/netperf/sys/net/if_gre.h 2004/03/22 16:18:59 @@ -54,6 +54,12 @@ WCCP_V2 } wccp_ver_t; +/* + * XXXRW: softc fields need locking. + * + * XXXRW: gre's notion of a 'called' count is not MP-safe, as it assumes + * only one packet can be processed at a time. + */ struct gre_softc { struct ifnet sc_if; LIST_ENTRY(gre_softc) sc_list; --- //depot/vendor/freebsd/src/sys/net/if_sl.c 2004/07/15 08:30:31 +++ //depot/user/rwatson/netperf/sys/net/if_sl.c 2004/07/15 22:19:17 @@ -79,6 +79,7 @@ #include #include #include +#include #include #include @@ -162,6 +163,7 @@ #define ABT_WINDOW (ABT_COUNT*2+2) /* in seconds - time to count */ static LIST_HEAD(sl_list, sl_softc) sl_list; +static struct mtx slip_mtx; #define FRAME_END 0xc0 /* Frame End */ #define FRAME_ESCAPE 0xdb /* Frame Esc */ @@ -182,7 +184,9 @@ static int slopen(struct cdev *, struct tty *); static int sloutput(struct ifnet *, struct mbuf *, struct sockaddr *, struct rtentry *); -static int slstart(struct tty *); +static int slstart(struct tty *tp); +static void slstart_schedule(struct sl_softc *); +static void slstart_task(void *, int); static struct linesw slipdisc = { .l_open = slopen, @@ -203,6 +207,7 @@ { switch (type) { case MOD_LOAD: + mtx_init(&slip_mtx, "slip_mtx", NULL, MTX_DEF); ldisc_register(SLIPDISC, &slipdisc); LIST_INIT(&sl_list); break; @@ -224,6 +229,7 @@ DECLARE_MODULE(if_sl, sl_mod, SI_SUB_PSEUDO, SI_ORDER_ANY); +/* Locked using slip_mtx. */ static int *st_unit_list; static size_t st_unit_max = 0; @@ -232,6 +238,7 @@ { struct sl_softc *nc; + mtx_assert(&slip_mtx, MA_OWNED); LIST_FOREACH(nc, &sl_list, sl_next) { if (nc->sc_if.if_dunit == unit) return (0); @@ -245,6 +252,7 @@ { size_t i; + mtx_assert(&slip_mtx, MA_OWNED); for (i = 0; i < st_unit_max; i++) if (st_unit_list[i] == unit) return 1; @@ -257,6 +265,7 @@ { int *t; + mtx_assert(&slip_mtx, MA_OWNED); if (slisstatic(unit)) return; @@ -319,10 +328,12 @@ sc->sc_if.if_linkmib = sc; sc->sc_if.if_linkmiblen = sizeof *sc; mtx_init(&sc->sc_fastq.ifq_mtx, "sl_fastq", NULL, MTX_DEF); + mtx_init(&sc->sc_mtx, "slip sc_mtx", NULL, MTX_DEF); /* * Find a suitable unit number. */ + mtx_lock(&slip_mtx); for (unit=0; ; unit++) { if (slisstatic(unit)) continue; @@ -332,6 +343,7 @@ } if_initname(&sc->sc_if, "sl", unit); LIST_INSERT_HEAD(&sl_list, sc, sl_next); + mtx_unlock(&slip_mtx); if_attach(&sc->sc_if); bpfattach(&sc->sc_if, DLT_SLIP, SLIP_HDRLEN); @@ -364,6 +376,7 @@ tp->t_sc = (caddr_t)sc; sc->sc_ttyp = tp; sc->sc_if.if_baudrate = tp->t_ospeed; + TASK_INIT(&sc->sc_task, 0, slstart_task, sc->sc_ttyp); ttyflush(tp, FREAD | FWRITE); /* @@ -391,10 +404,20 @@ static void sldestroy(struct sl_softc *sc) { + + /* + * XXXRW: Slight race here: we may detach bpf/if before we + * attach. This appears to be a property of the unit selection + * process, which might be better handled by the interface + * cloning subsystem? + */ bpfdetach(&sc->sc_if); if_detach(&sc->sc_if); + mtx_lock(&slip_mtx); LIST_REMOVE(sc, sl_next); + mtx_unlock(&slip_mtx); m_free(sc->sc_mbuf); + mtx_destroy(&sc->sc_mtx); mtx_destroy(&sc->sc_fastq.ifq_mtx); if (sc->bpfbuf) free(sc->bpfbuf, M_SL); @@ -423,6 +446,10 @@ clist_free_cblocks(&tp->t_outq); sc = (struct sl_softc *)tp->t_sc; if (sc != NULL) { + /* + * XXXRW: tear-down race between timeout and slclose()? + */ + mtx_lock(&sc->sc_mtx); if (sc->sc_outfill) { sc->sc_outfill = 0; untimeout(sl_outfill, sc, sc->sc_ofhandle); @@ -431,6 +458,7 @@ sc->sc_keepalive = 0; untimeout(sl_keepalive, sc, sc->sc_kahandle); } + mtx_unlock(&sc->sc_mtx); if_down(&sc->sc_if); sc->sc_ttyp = NULL; tp->t_sc = NULL; @@ -468,12 +496,21 @@ splx(s); return (ENXIO); } + /* + * XXXRW: we hold the mutex over all of this to protect + * the unit change and global list consistency. However, + * some of these functions probably sleep, making this + * wrong. If we have to support renumbering, we probably + * need a way to reserve both numbers to prevent them + * from being reused during the change, or a way to sleep + * waiting for a change to end (i.e., a CV). + */ + mtx_lock(&slip_mtx); if (sc->sc_if.if_dunit != unit) { if (!slisunitfree(unit)) { - splx(s); + mtx_unlock(&slip_mtx); return (ENXIO); } - wasup = sc->sc_if.if_flags & IFF_UP; bpfdetach(&sc->sc_if); if_detach(&sc->sc_if); @@ -491,9 +528,11 @@ SLIP_HIWAT + 2 * sc->sc_if.if_mtu + 1); } slmarkstatic(unit); + mtx_unlock(&slip_mtx); break; case SLIOCSKEEPAL: + mtx_lock(&sc->sc_mtx); sc->sc_keepalive = *(u_int *)data * hz; if (sc->sc_keepalive) { sc->sc_flags |= SC_KEEPALIVE; @@ -505,6 +544,7 @@ sc->sc_flags &= ~SC_KEEPALIVE; } } + mtx_unlock(&sc->sc_mtx); break; case SLIOCGKEEPAL: @@ -512,6 +552,7 @@ break; case SLIOCSOUTFILL: + mtx_lock(&sc->sc_mtx); sc->sc_outfill = *(u_int *)data * hz; if (sc->sc_outfill) { sc->sc_flags |= SC_OUTWAIT; @@ -523,9 +564,11 @@ sc->sc_flags &= ~SC_OUTWAIT; } } + mtx_unlock(&sc->sc_mtx); break; case SLIOCGOUTFILL: + /* Unlocked read. */ *(int *)data = sc->sc_outfill / hz; break; @@ -588,12 +631,26 @@ return (ENOBUFS); } s = splimp(); + /* + * XXXRW: We run slstart() in a task queue to avoid running tty code + * out of a network context. However, there's a race here wherein + * the task may not run until after the SLIP interface is reused or + * destroyed. + */ if (sc->sc_ttyp->t_outq.c_cc == 0) slstart(sc->sc_ttyp); splx(s); return (0); } +static void +slstart_schedule(sc) + struct sl_softc *sc; +{ + + taskqueue_enqueue(taskqueue_swi_giant, &sc->sc_task); +} + /* * Start output on interface. Get another datagram * to send from the interface queue and map it to @@ -604,6 +661,18 @@ register struct tty *tp; { register struct sl_softc *sc = (struct sl_softc *)tp->t_sc; + + slstart_schedule(sc); + return 0; +} + +static void +slstart_task(context, pending) + void *context; + int pending; +{ + register struct sl_softc *sc = (struct sl_softc *)context; + register struct tty *tp = sc->sc_ttyp; register struct mbuf *m; register u_char *cp; register struct ip *ip; @@ -619,17 +688,20 @@ (*tp->t_oproc)(tp); if (tp->t_outq.c_cc != 0) { - if (sc != NULL) + if (sc != NULL) { + mtx_lock(&sc->sc_mtx); sc->sc_flags &= ~SC_OUTWAIT; + mtx_unlock(&sc->sc_mtx); + } if (tp->t_outq.c_cc > SLIP_HIWAT) - return 0; + return; } /* * This happens briefly when the line shuts down. */ if (sc == NULL) - return 0; + return; /* * Get a packet and send it to the interface. @@ -642,7 +714,7 @@ IF_DEQUEUE(&sc->sc_if.if_snd, m); splx(s); if (m == NULL) - return 0; + return; /* * We do the header compression here rather than in sloutput @@ -650,6 +722,7 @@ * queueing, and the connection id compression will get * munged when this happens. */ + mtx_lock(&sc->sc_mtx); if (sc->sc_if.if_bpf) { /* * We need to save the TCP/IP header before it's @@ -680,9 +753,10 @@ } ip = mtod(m, struct ip *); if (ip->ip_v == IPVERSION && ip->ip_p == IPPROTO_TCP) { - if (sc->sc_if.if_flags & SC_COMPRESS) + if (sc->sc_if.if_flags & SC_COMPRESS) { *mtod(m, u_char *) |= sl_compress_tcp(m, ip, &sc->sc_comp, 1); + } } if (sc->sc_if.if_bpf && sc->bpfbuf) { /* @@ -694,6 +768,7 @@ bcopy(mtod(m, caddr_t), &sc->bpfbuf[SLX_CHDR], CHDR_LEN); BPF_TAP(&sc->sc_if, sc->bpfbuf, len + SLIP_HDRLEN); } + mtx_unlock(&sc->sc_mtx); /* * If system is getting low on clists, just flush our @@ -709,7 +784,9 @@ continue; } + mtx_lock(&sc->sc_mtx); sc->sc_flags &= ~SC_OUTWAIT; + mtx_unlock(&sc->sc_mtx); /* * The extra FRAME_END will start up a new packet, and thus * will flush any accumulated garbage. We do this whenever @@ -786,7 +863,6 @@ sc->sc_if.if_opackets++; } } - return 0; } /* @@ -799,6 +875,8 @@ { struct mbuf *m, *newm; + mtx_assert(&sc->sc_mtx, MA_OWNED); + MGETHDR(m, M_DONTWAIT, MT_DATA); if (m == NULL) return (NULL); @@ -854,13 +932,16 @@ if (sc == NULL) return 0; if (c & TTY_ERRORMASK || (tp->t_state & TS_CONNECTED) == 0) { + mtx_lock(&sc->sc_mtx); sc->sc_flags |= SC_ERROR; + mtx_unlock(&sc->sc_mtx); return 0; } c &= TTY_CHARMASK; ++sc->sc_if.if_ibytes; + mtx_lock(&sc->sc_mtx); if (sc->sc_if.if_flags & IFF_DEBUG) { if (c == ABT_ESC) { /* @@ -879,6 +960,7 @@ sc->sc_starttime = time_second; if (sc->sc_abortcount >= ABT_COUNT) { slclose(tp,0); + mtx_unlock(&sc->sc_mtx); return 0; } } @@ -901,6 +983,7 @@ case FRAME_ESCAPE: sc->sc_escape = 1; + mtx_unlock(&sc->sc_mtx); return 0; case FRAME_END: @@ -985,6 +1068,7 @@ if (sc->sc_mp < sc->sc_ep) { *sc->sc_mp++ = c; sc->sc_escape = 0; + mtx_unlock(&sc->sc_mtx); return 0; } @@ -996,6 +1080,7 @@ newpack: sc->sc_mp = sc->sc_buf = sc->sc_ep - SLRMAX; sc->sc_escape = 0; + mtx_unlock(&sc->sc_mtx); return 0; } @@ -1079,6 +1164,7 @@ { struct sl_softc *sc = chan; + mtx_lock(&sc->sc_mtx); if (sc->sc_keepalive) { if (sc->sc_flags & SC_KEEPALIVE) { if (sc->sc_ttyp->t_pgrp != NULL) { @@ -1092,6 +1178,7 @@ } else { sc->sc_flags &= ~SC_KEEPALIVE; } + mtx_unlock(&sc->sc_mtx); } static void @@ -1102,6 +1189,7 @@ register struct tty *tp = sc->sc_ttyp; int s; + mtx_lock(&sc->sc_mtx); if (sc->sc_outfill && tp != NULL) { if (sc->sc_flags & SC_OUTWAIT) { s = splimp (); @@ -1115,4 +1203,5 @@ } else { sc->sc_flags &= ~SC_OUTWAIT; } + mtx_unlock(&sc->sc_mtx); } --- //depot/vendor/freebsd/src/sys/net/if_slvar.h 2004/04/07 20:52:05 +++ //depot/user/rwatson/netperf/sys/net/if_slvar.h 2004/07/14 06:07:17 @@ -34,13 +34,17 @@ #ifndef _NET_IF_SLVAR_H_ #define _NET_IF_SLVAR_H_ +#include +#include #include /* * Definitions for SLIP interface data structures * * (This exists so programs like slstats can get at the definition - * of sl_softc.) + * of sl_softc.) Fields owned by the SLIP subsystem are protected + * using sc_mtx, with the exception of sc_next, which is protected + * by the global slip_mtx. */ struct sl_softc { struct ifnet sc_if; /* network-visible interface */ @@ -66,6 +70,8 @@ struct slcompress sc_comp; /* tcp compression data */ LIST_ENTRY(sl_softc) sl_next; u_char *bpfbuf; /* hang buffer for bpf here */ + struct task sc_task; /* run slstart() w/Giant for ttys */ + struct mtx sc_mtx; }; /* internal flags */ --- //depot/vendor/freebsd/src/sys/net/if_spppsubr.c 2004/07/15 08:30:31 +++ //depot/user/rwatson/netperf/sys/net/if_spppsubr.c 2004/07/15 22:19:17 @@ -92,12 +92,8 @@ #include #if defined(__FreeBSD__) && __FreeBSD__ >= 3 -# define UNTIMEOUT(fun, arg, handle) untimeout(fun, arg, handle) -# define TIMEOUT(fun, arg1, arg2, handle) handle = timeout(fun, arg1, arg2) # define IOCTL_CMD_T u_long #else -# define UNTIMEOUT(fun, arg, handle) untimeout(fun, arg) -# define TIMEOUT(fun, arg1, arg2, handle) timeout(fun, arg1, arg2) # define IOCTL_CMD_T int #endif @@ -259,10 +255,11 @@ void (*scr)(struct sppp *sp); }; +struct mtx sppp_mtx; +MTX_SYSINIT(sppp_mtx, &sppp_mtx, "sppp_mtx", MTX_DEF); + static struct sppp *spppq; -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 -static struct callout_handle keepalive_ch; -#endif +static struct callout keepalive_callout; #if defined(__FreeBSD__) && __FreeBSD__ >= 3 && __FreeBSD_version < 501113 #define SPP_FMT "%s%d: " @@ -964,13 +961,18 @@ { struct sppp *sp = (struct sppp*) ifp; + mtx_lock(&sppp_mtx); /* Initialize keepalive handler. */ - if (spppq == NULL) - TIMEOUT(sppp_keepalive, 0, hz * 10, keepalive_ch); + if (spppq == NULL) { + callout_init(&keepalive_callout, 0); + callout_reset(&keepalive_callout, hz * 10, sppp_keepalive, + NULL); + } /* Insert new entry into the keepalive list. */ sp->pp_next = spppq; spppq = sp; + mtx_unlock(&sppp_mtx); sp->pp_if.if_mtu = PP_MTU; sp->pp_if.if_flags = IFF_POINTOPOINT | IFF_MULTICAST; @@ -1016,6 +1018,7 @@ struct sppp **q, *p, *sp = (struct sppp*) ifp; int i; + mtx_lock(&sppp_mtx); /* Remove the entry from the keepalive list. */ for (q = &spppq; (p = *q); q = &p->pp_next) if (p == sp) { @@ -1025,11 +1028,12 @@ /* Stop keepalive handler. */ if (spppq == NULL) - UNTIMEOUT(sppp_keepalive, 0, keepalive_ch); + callout_stop(&keepalive_callout); + mtx_unlock(&sppp_mtx); for (i = 0; i < IDX_COUNT; i++) - UNTIMEOUT((cps[i])->TO, (void *)sp, sp->ch[i]); - UNTIMEOUT(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); + untimeout((cps[i])->TO, (void *)sp, sp->ch[i]); + untimeout(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); mtx_destroy(&sp->pp_cpq.ifq_mtx); mtx_destroy(&sp->pp_fastq.ifq_mtx); } @@ -2008,8 +2012,8 @@ case STATE_STOPPING: sppp_cp_send(sp, cp->proto, TERM_REQ, ++sp->pp_seq[cp->protoidx], 0, 0); - TIMEOUT(cp->TO, (void *)sp, sp->lcp.timeout, - sp->ch[cp->protoidx]); + sp->ch[cp->protoidx] = timeout(cp->TO, (void *)sp, + sp->lcp.timeout); break; case STATE_REQ_SENT: case STATE_ACK_RCVD: @@ -2019,8 +2023,8 @@ break; case STATE_ACK_SENT: (cp->scr)(sp); - TIMEOUT(cp->TO, (void *)sp, sp->lcp.timeout, - sp->ch[cp->protoidx]); + sp->ch[cp->protoidx] = timeout(cp->TO, (void *)sp, + sp->lcp.timeout); break; } @@ -2036,7 +2040,7 @@ { sp->state[cp->protoidx] = newstate; - UNTIMEOUT(cp->TO, (void *)sp, sp->ch[cp->protoidx]); + untimeout(cp->TO, (void *)sp, sp->ch[cp->protoidx]); switch (newstate) { case STATE_INITIAL: case STATE_STARTING: @@ -2049,8 +2053,8 @@ case STATE_REQ_SENT: case STATE_ACK_RCVD: case STATE_ACK_SENT: - TIMEOUT(cp->TO, (void *)sp, sp->lcp.timeout, - sp->ch[cp->protoidx]); + sp->ch[cp->protoidx] = timeout(cp->TO, (void *)sp, + sp->lcp.timeout); break; } } @@ -4147,7 +4151,7 @@ * a number between 300 and 810 seconds. */ i = 300 + ((unsigned)(random() & 0xff00) >> 7); - TIMEOUT(chap.TO, (void *)sp, i * hz, sp->ch[IDX_CHAP]); + sp->ch[IDX_CHAP] = timeout(chap.TO, (void *)sp, i * hz); } if (debug) { @@ -4191,7 +4195,7 @@ if (debug) log(LOG_DEBUG, SPP_FMT "chap tld\n", SPP_ARGS(ifp)); - UNTIMEOUT(chap.TO, (void *)sp, sp->ch[IDX_CHAP]); + untimeout(chap.TO, (void *)sp, sp->ch[IDX_CHAP]); sp->lcp.protos &= ~(1 << IDX_CHAP); lcp.Close(sp); @@ -4327,7 +4331,7 @@ /* ack and nak are his authproto */ case PAP_ACK: - UNTIMEOUT(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); + untimeout(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); if (debug) { log(LOG_DEBUG, SPP_FMT "pap success", SPP_ARGS(ifp)); @@ -4356,7 +4360,7 @@ break; case PAP_NAK: - UNTIMEOUT(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); + untimeout(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); if (debug) { log(LOG_INFO, SPP_FMT "pap failure", SPP_ARGS(ifp)); @@ -4413,8 +4417,8 @@ if (sp->myauth.proto == PPP_PAP) { /* we are peer, send a request, and start a timer */ pap.scr(sp); - TIMEOUT(sppp_pap_my_TO, (void *)sp, sp->lcp.timeout, - sp->pap_my_to_ch); + sp->pap_my_to_ch = timeout(sppp_pap_my_TO, (void *)sp, + sp->lcp.timeout); } } @@ -4517,8 +4521,8 @@ if (debug) log(LOG_DEBUG, SPP_FMT "pap tld\n", SPP_ARGS(ifp)); - UNTIMEOUT(pap.TO, (void *)sp, sp->ch[IDX_PAP]); - UNTIMEOUT(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); + untimeout(pap.TO, (void *)sp, sp->ch[IDX_PAP]); + untimeout(sppp_pap_my_TO, (void *)sp, sp->pap_my_to_ch); sp->lcp.protos &= ~(1 << IDX_PAP); lcp.Close(sp); @@ -4645,7 +4649,12 @@ struct sppp *sp; int s; + /* + * XXXRW: It would be nice to avoid calling all this stuff while + * holding sppp_mtx, or we risk lock order reversals. + */ s = splimp(); + mtx_lock(&sppp_mtx); for (sp=spppq; sp; sp=sp->pp_next) { struct ifnet *ifp = &sp->pp_if; @@ -4684,8 +4693,9 @@ sp->lcp.echoid, 4, &nmagic); } } + callout_reset(&keepalive_callout, hz * 10, sppp_keepalive, NULL); + mtx_unlock(&sppp_mtx); splx(s); - TIMEOUT(sppp_keepalive, 0, hz * 10, keepalive_ch); } /* --- //depot/vendor/freebsd/src/sys/net/if_stf.c 2004/07/15 08:30:31 +++ //depot/user/rwatson/netperf/sys/net/if_stf.c 2004/07/15 22:19:17 @@ -139,13 +139,11 @@ #define sc_ro __sc_ro46.__sc_ro4 const struct encaptab *encap_cookie; LIST_ENTRY(stf_softc) sc_list; /* all stf's are linked */ + struct mtx sc_mtx; /* protect sc_ro */ }; /* * All mutable global variables in if_stf.c are protected by stf_mtx. - * XXXRW: Note that mutable fields in the softc are not currently locked: - * in particular, sc_ro needs to be protected from concurrent entrance - * of stf_output(). */ static struct mtx stf_mtx; static LIST_HEAD(, stf_softc) stf_softc_list; @@ -231,6 +229,7 @@ ifc_free_unit(ifc, unit); return (ENOMEM); } + mtx_init(&sc->sc_mtx, "stf sc_mtx", NULL, MTX_DEF); ifp->if_mtu = IPV6_MMTU; ifp->if_ioctl = stf_ioctl; @@ -255,6 +254,7 @@ bpfdetach(&sc->sc_if); if_detach(&sc->sc_if); + mtx_destroy(&sc->sc_mtx); free(sc, M_STF); } @@ -430,9 +430,10 @@ struct ip *ip; struct ip6_hdr *ip6; struct in6_ifaddr *ia6; -#ifdef MAC + struct route ro; int error; +#ifdef MAC error = mac_check_ifnet_transmit(ifp, m); if (error) { m_freem(m); @@ -534,9 +535,7 @@ else ip_ecn_ingress(ECN_NOCARE, &ip->ip_tos, &tos); - /* - * XXXRW: Locking of sc_ro required. - */ + mtx_lock(&sc->sc_mtx); dst4 = (struct sockaddr_in *)&sc->sc_ro.ro_dst; if (dst4->sin_family != AF_INET || bcmp(&dst4->sin_addr, &ip->ip_dst, sizeof(ip->ip_dst)) != 0) { @@ -555,12 +554,21 @@ if (sc->sc_ro.ro_rt == NULL) { m_freem(m); ifp->if_oerrors++; + mtx_unlock(&sc->sc_mtx); return ENETUNREACH; } } + /* + * XXXRW: Holding mutex over call to ip_output(): potential lock + * order issue? Hard to resolve cleanly with the current route + * caching model, as we have to synchronize access to shared softc + * state. + */ ifp->if_opackets++; - return ip_output(m, NULL, &sc->sc_ro, 0, NULL, NULL); + error = ip_output(m, NULL, &ro, 0, NULL, NULL); + mtx_unlock(&sc->sc_mtx); + return (error); } static int --- //depot/vendor/freebsd/src/sys/net/if_tap.c 2004/06/17 17:21:12 +++ //depot/user/rwatson/netperf/sys/net/if_tap.c 2004/06/18 00:24:39 @@ -113,6 +113,10 @@ * All global variables in if_tap.c are locked with tapmtx, with the * exception of tapdebug, which is accessed unlocked; tapclones is * static at runtime. + * + * XXXRW: si_flags appears not to be protected from concurrent access, + * and is written at run-time. + * XXXRW: si_drv1 is also used for test-and-set, and isn't synchronized. */ static struct mtx tapmtx; static int tapdebug = 0; /* debug flag */ @@ -162,6 +166,7 @@ * The EBUSY algorithm here can't quite atomically * guarantee that this is race-free since we have to * release the tap mtx to deregister the clone handler. + * XXXRW: is this true? */ mtx_lock(&tapmtx); SLIST_FOREACH(tp, &taphead, tap_next) { @@ -693,6 +698,7 @@ case SIOCSIFADDR: /* set MAC address of the remote side */ mtx_lock(&tp->tap_mtx); + /* XXXRW: Does this actually do anything? */ bcopy(data, tp->ether_addr, sizeof(tp->ether_addr)); mtx_unlock(&tp->tap_mtx); break; @@ -747,6 +753,7 @@ if (flag & IO_NDELAY) return (EWOULDBLOCK); + /* This looks like a wanna-be condition variable. */ mtx_lock(&tp->tap_mtx); tp->tap_flags |= TAP_RWAIT; mtx_unlock(&tp->tap_mtx); --- //depot/vendor/freebsd/src/sys/net/if_tun.c 2004/07/15 08:30:31 +++ //depot/user/rwatson/netperf/sys/net/if_tun.c 2004/07/15 22:19:17 @@ -59,6 +59,12 @@ * tun_list is protected by global tunmtx. Other mutable fields are * protected by tun->tun_mtx, or by their owning subsystem. tun_dev is * static for the duration of a tunnel interface. + * + * XXXRW: we allocate si_drv1 for the dev_t on demand, rather than when + * the dev_t is instantiated. Nothing serializes the test/set of that + * field. + * + * XXXRW: what serializes access to si_flags? */ struct tun_softc { TAILQ_ENTRY(tun_softc) tun_list; @@ -121,6 +127,9 @@ static d_ioctl_t tunioctl; static d_poll_t tunpoll; +/* + * XXXRW: can remove D_NEEDGIANT? Probably not because of si_drv1 for now. + */ static struct cdevsw tun_cdevsw = { .d_version = D_VERSION, .d_flags = D_PSEUDO | D_NEEDGIANT, @@ -382,6 +391,9 @@ ifp->if_flags |= IFF_UP | IFF_RUNNING; getmicrotime(&ifp->if_lastchange); + /* + * XXXRW: interface locking. + */ for (ifa = TAILQ_FIRST(&ifp->if_addrhead); ifa; ifa = TAILQ_NEXT(ifa, ifa_link)) { if (ifa->ifa_addr == NULL) --- //depot/vendor/freebsd/src/sys/net/raw_cb.c 2004/06/15 04:15:33 +++ //depot/user/rwatson/netperf/sys/net/raw_cb.c 2004/06/18 00:24:39 @@ -123,6 +123,7 @@ m_freem(dtom(rp->rcb_faddr)); rp->rcb_faddr = 0; #endif + /* Unlocked read. */ if (rp->rcb_socket->so_state & SS_NOFDREF) raw_detach(rp); } --- //depot/vendor/freebsd/src/sys/net/raw_usrreq.c 2004/06/15 04:15:33 +++ //depot/user/rwatson/netperf/sys/net/raw_usrreq.c 2004/06/18 00:24:39 @@ -76,6 +76,10 @@ register struct mbuf *m = m0; struct socket *last; + /* + * XXXRW: Potential lock order issues due to holding the + * rawcb_mtx across all this stuff. Need to revisit. + */ last = 0; mtx_lock(&rawcb_mtx); LIST_FOREACH(rp, &rawcb_list, list) { --- //depot/vendor/freebsd/src/sys/net/rtsock.c 2004/07/06 03:30:34 +++ //depot/user/rwatson/netperf/sys/net/rtsock.c 2004/07/09 21:19:53 @@ -54,10 +54,18 @@ MALLOC_DEFINE(M_RTABLE, "routetbl", "routing tables"); /* NB: these are not modified */ +/* + * XXXRW: It would be really nice to add const to these, but that may + * not be possible due to where they are passed in. We might need + * to const-poison a whole boatload of APIs...? + */ static struct sockaddr route_dst = { 2, PF_ROUTE, }; static struct sockaddr route_src = { 2, PF_ROUTE, }; static struct sockaddr sa_zero = { sizeof(sa_zero), AF_INET, }; +/* + * XXXRW: These fields are locked by RTSOCK_LOCK(). + */ static struct { int ip_count; /* attacked w/ AF_INET */ int ip6_count; /* attached w/ AF_INET6 */ --- //depot/vendor/freebsd/src/sys/netatalk/aarp.c 2004/07/12 18:35:53 +++ //depot/user/rwatson/netperf/sys/netatalk/aarp.c 2004/07/12 19:43:55 @@ -1,4 +1,5 @@ /* + * Copyright (c) 2004 Robert N. M. Watson * Copyright (c) 1990,1991 Regents of The University of Michigan. * All Rights Reserved. * @@ -63,10 +64,6 @@ #define AARPT_KILLC 20 #define AARPT_KILLI 3 -# if !defined(__FreeBSD__) -extern u_char etherbroadcastaddr[6]; -# endif /* __FreeBSD__ */ - static const u_char atmulticastaddr[ 6 ] = { 0x09, 0x00, 0x07, 0xff, 0xff, 0xff, }; @@ -78,6 +75,9 @@ 0x00, 0x00, 0x00, }; +/* + * XXXRW: Make use callouts, not timeouts. + */ static struct callout_handle aarptimer_ch = CALLOUT_HANDLE_INITIALIZER(&aarptimer_ch); @@ -636,6 +636,9 @@ struct aarptab *aat; int i; + /* + * XXXRW: Should grab mutex before untimeout? + */ untimeout(aarptimer, 0, aarptimer_ch); AARPTAB_LOCK(); for (i = 0, aat = aarptab; i < AARPTAB_SIZE; i++, aat++) { --- //depot/vendor/freebsd/src/sys/netatalk/at_control.c 2004/07/12 04:35:31 +++ //depot/user/rwatson/netperf/sys/netatalk/at_control.c 2004/07/12 14:54:27 @@ -21,6 +21,9 @@ #include #include +/* + * XXXRW: Requires synchronization. + */ struct at_ifaddr *at_ifaddr_list; static int aa_dorangeroute(struct ifaddr *ifa, --- //depot/vendor/freebsd/src/sys/netatalk/at_rmx.c 2004/07/12 18:40:50 +++ //depot/user/rwatson/netperf/sys/netatalk/at_rmx.c 2004/07/12 19:43:55 @@ -40,6 +40,19 @@ int at_inithead(void **head, int off); +/* + * XXXRW: hexdump was a static global variable, but I moved it into the + * stack rather than stick a mutex around it. 256 bytes is smaller than + * it used to be, but this still might be a problem. Needs to be + * revisited. Should probably just use the new hexdump(9). + * + * XXXRW: All this appears to be present just so as to printf debugging + * information. Assuming that this code is known to work, we could just + * scrap all this. In fact, this code isn't even used as it stands, it's + * here for debugging purposes only and requires modifications to + * at_proto.c. + */ + #define HEXBUF_LEN 256 static const char * --- //depot/vendor/freebsd/src/sys/netatalk/ddp_input.c 2004/07/12 18:40:50 +++ //depot/user/rwatson/netperf/sys/netatalk/ddp_input.c 2004/07/12 19:43:55 @@ -32,6 +32,11 @@ static volatile int ddp_firewall = 0; static struct ddpstat ddpstat; +/* + * XXXRW: If we're going to keep this cached route data, we'll need to lock it + * down, and change later function-local use of it to grab an extra reference + * after deciding it is useful. + */ static struct route forwro; static void ddp_input(struct mbuf *, struct ifnet *, struct elaphdr *, int); --- //depot/vendor/freebsd/src/sys/netatalk/ddp_pcb.c 2004/07/12 18:40:50 +++ //depot/user/rwatson/netperf/sys/netatalk/ddp_pcb.c 2004/07/12 19:43:55 @@ -257,6 +257,10 @@ DDP_LOCK_INIT(ddp); ddp->ddp_lsat.sat_port = ATADDR_ANYPORT; + /* + * XXXRW: Is this unlocked assignment payer for socket and + * back-pointer something that needs to be protected? + */ ddp->ddp_socket = so; so->so_pcb = (caddr_t)ddp; --- //depot/vendor/freebsd/src/sys/netatalk/ddp_usrreq.c 2004/07/12 18:40:50 +++ //depot/user/rwatson/netperf/sys/netatalk/ddp_usrreq.c 2004/07/12 19:43:55 @@ -23,6 +23,11 @@ #include #include +/* + * XXXRW: These structures are currently not mutable. They're not marked + * with 'const' because I suspect that consumers of this code would like for + * them to be mutable someday. + */ static u_long ddp_sendspace = DDP_MAXSZ; /* Max ddp size + 1 (ddp_type) */ static u_long ddp_recvspace = 10 * (587 + sizeof(struct sockaddr_at)); @@ -149,6 +154,13 @@ return (0); } +/* + * XXXRW: If an explicit address is specified, then we temporarily change + * the address on the pcb for sending. This is inefficient because it + * requires us to perform global rather than pcb-local operations. It + * may also create a race if other users of the socket are simultaneously + * sending. + */ static int ddp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr, struct mbuf *control, struct thread *td) --- //depot/vendor/freebsd/src/sys/netgraph/ng_ksocket.c 2004/06/25 19:25:33 +++ //depot/user/rwatson/netperf/sys/netgraph/ng_ksocket.c 2004/06/26 16:46:29 @@ -998,6 +998,9 @@ * before dereferencing the socket pointer. */ +/* + * XXXRW: ng_ksocket_incoming() is called without Giant. Is that OK? + */ static void ng_ksocket_incoming(struct socket *so, void *arg, int waitflag) { --- //depot/vendor/freebsd/src/sys/netinet/accf_http.c 2004/06/14 18:20:38 +++ //depot/user/rwatson/netperf/sys/netinet/accf_http.c 2004/06/14 19:35:13 @@ -161,6 +161,7 @@ sohashttpget(struct socket *so, void *arg, int waitflag) { + /* Unlocked read. */ if ((so->so_rcv.sb_state & SBS_CANTRCVMORE) == 0 && !sbfull(&so->so_rcv)) { struct mbuf *m; char *cmp; @@ -214,6 +215,7 @@ struct mbuf *m, *n; int i, cc, spaces, inspaces; + /* Unlocked read. */ if ((so->so_rcv.sb_state & SBS_CANTRCVMORE) != 0 || sbfull(&so->so_rcv)) goto fallout; @@ -301,6 +303,7 @@ int ccleft, copied; DPRINT("start"); + /* Unlocked read. */ if ((so->so_rcv.sb_state & SBS_CANTRCVMORE) != 0 || sbfull(&so->so_rcv)) goto gotit; --- //depot/vendor/freebsd/src/sys/netinet/if_ether.c 2004/06/13 10:56:09 +++ //depot/user/rwatson/netperf/sys/netinet/if_ether.c 2004/06/13 20:00:27 @@ -98,6 +98,11 @@ #define la_timer la_rt->rt_rmx.rmx_expire /* deletion time in seconds */ }; +/* + * XXXRW: Need to document (and/or fix) locking for this. We always + * seem to hold a lock (and assert) when referencing this list, but it's + * not clear it's always the same lock. + */ static LIST_HEAD(, llinfo_arp) llinfo_arp; static struct ifqueue arpintrq; --- //depot/vendor/freebsd/src/sys/netinet/igmp.c 2004/06/11 03:45:34 +++ //depot/user/rwatson/netperf/sys/netinet/igmp.c 2004/06/11 13:24:13 @@ -99,6 +99,9 @@ static u_long igmp_all_hosts_group; static u_long igmp_all_rtrs_group; +/* + * XXXRW: These variables make me vaguely nervous. + */ static struct mbuf *router_alert; static struct route igmprt; @@ -123,6 +126,7 @@ /* * Construct a Router Alert option to use in outgoing packets + * XXXRW: This might actually need a MAC label. */ MGET(router_alert, M_DONTWAIT, MT_DATA); ra = mtod(router_alert, struct ipoption *); --- //depot/vendor/freebsd/src/sys/netinet/in_gif.c 2004/06/18 02:06:42 +++ //depot/user/rwatson/netperf/sys/netinet/in_gif.c 2004/06/18 03:27:04 @@ -174,6 +174,9 @@ } bcopy(&iphdr, mtod(m, struct ip *), sizeof(struct ip)); + /* + * XXXRW: locking of gif's softc. + */ if (dst->sin_family != sin_dst->sin_family || dst->sin_addr.s_addr != sin_dst->sin_addr.s_addr) { /* cache route doesn't match */ @@ -321,6 +324,10 @@ case 0: case 127: case 255: return 0; } + + /* + * XXXRW: Lock in_ifaddrhead walking. + */ /* reject packets with broadcast on source */ TAILQ_FOREACH(ia4, &in_ifaddrhead, ia_link) { if ((ia4->ia_ifa.ifa_ifp->if_flags & IFF_BROADCAST) == 0) @@ -329,6 +336,7 @@ return 0; } + /* XXXRW: unlocked read. */ /* ingress filters on outer source */ if ((sc->gif_if.if_flags & IFF_LINK2) == 0 && ifp) { struct sockaddr_in sin; @@ -384,6 +392,11 @@ in_gif_attach(sc) struct gif_softc *sc; { + + /* + * XXXRW: Technically, NULL can also be returned for ENOMEM, + * not just EEXIST. + */ sc->encap_cookie4 = encap_attach_func(AF_INET, -1, gif_encapcheck, &in_gif_protosw, sc); if (sc->encap_cookie4 == NULL) --- //depot/vendor/freebsd/src/sys/netinet/in_pcb.c 2004/06/16 10:05:58 +++ //depot/user/rwatson/netperf/sys/netinet/in_pcb.c 2004/06/18 00:24:39 @@ -671,6 +671,7 @@ #ifdef IPSEC ipsec_pcbdisconn(inp->inp_sp); #endif + /* Unlocked read. */ if (inp->inp_socket->so_state & SS_NOFDREF) in_pcbdetach(inp); } --- //depot/vendor/freebsd/src/sys/netinet/in_pcb.h 2004/07/13 16:10:39 +++ //depot/user/rwatson/netperf/sys/netinet/in_pcb.h 2004/07/14 20:52:14 @@ -243,16 +243,10 @@ #define INP_LOCK_DESTROY(inp) mtx_destroy(&(inp)->inp_mtx) #define INP_LOCK(inp) mtx_lock(&(inp)->inp_mtx) #define INP_UNLOCK(inp) mtx_unlock(&(inp)->inp_mtx) -#ifndef INET6 #define INP_LOCK_ASSERT(inp) do { \ mtx_assert(&(inp)->inp_mtx, MA_OWNED); \ NET_ASSERT_GIANT(); \ } while (0) -#else -#define INP_LOCK_ASSERT(inp) do { \ - NET_ASSERT_GIANT(); \ -} while (0) -#endif #define INP_INFO_LOCK_INIT(ipi, d) \ mtx_init(&(ipi)->ipi_mtx, (d), NULL, MTX_DEF | MTX_RECURSE) @@ -260,7 +254,6 @@ #define INP_INFO_WLOCK(ipi) mtx_lock(&(ipi)->ipi_mtx) #define INP_INFO_RUNLOCK(ipi) mtx_unlock(&(ipi)->ipi_mtx) #define INP_INFO_WUNLOCK(ipi) mtx_unlock(&(ipi)->ipi_mtx) -#ifndef INET6 #define INP_INFO_RLOCK_ASSERT(ipi) do { \ mtx_assert(&(ipi)->ipi_mtx, MA_OWNED); \ NET_ASSERT_GIANT(); \ @@ -269,14 +262,6 @@ mtx_assert(&(ipi)->ipi_mtx, MA_OWNED); \ NET_ASSERT_GIANT(); \ } while (0) -#else -#define INP_INFO_RLOCK_ASSERT(ipi) do { \ - NET_ASSERT_GIANT(); \ -} while (0) -#define INP_INFO_WLOCK_ASSERT(ipi) do { \ - NET_ASSERT_GIANT(); \ -} while (0) -#endif #define INP_PCBHASH(faddr, lport, fport, mask) \ (((faddr) ^ ((faddr) >> 16) ^ ntohs((lport) ^ (fport))) & (mask)) --- //depot/vendor/freebsd/src/sys/netinet/ip_divert.c 2004/06/27 21:55:39 +++ //depot/user/rwatson/netperf/sys/netinet/ip_divert.c 2004/06/28 05:39:25 @@ -350,6 +350,9 @@ m->m_pkthdr.rcvif = ifa->ifa_ifp; } #ifdef MAC + /* + * XXXRW: perhaps should be mac_create_mbuf_from_inpcb()? + */ SOCK_LOCK(so); mac_create_mbuf_from_socket(so, m); SOCK_UNLOCK(so); @@ -421,6 +424,7 @@ /* The socket is always "connected" because we always know "where" to send the packet */ INP_UNLOCK(inp); + /* XXXRW: Not clear if this is adequate. */ SOCK_LOCK(so); so->so_state |= SS_ISCONNECTED; SOCK_UNLOCK(so); @@ -465,6 +469,8 @@ static int div_disconnect(struct socket *so) { + + /* Unlocked read. */ if ((so->so_state & SS_ISCONNECTED) == 0) return ENOTCONN; return div_abort(so); --- //depot/vendor/freebsd/src/sys/netinet/ip_encap.c 2004/03/10 02:50:38 +++ //depot/user/rwatson/netperf/sys/netinet/ip_encap.c 2004/03/10 03:47:45 @@ -1,4 +1,4 @@ -/* $FreeBSD: src/sys/netinet/ip_encap.c,v 1.19 2004/03/10 02:48:50 rwatson Exp $ */ +/* $FreeBSD: src/sys/netinet/ip_encap.c,v 1.18 2003/06/01 09:20:38 phk Exp $ */ /* $KAME: ip_encap.c,v 1.41 2001/03/15 08:35:08 itojun Exp $ */ /* @@ -106,8 +106,7 @@ LIST_HEAD(, encaptab) encaptab = LIST_HEAD_INITIALIZER(&encaptab); /* - * We currently keey encap_init() for source code compatibility reasons -- - * it's referenced by KAME pieces in netinet6. + * XXXRW: encap_init() was entirely useless, so I deleted it. */ void encap_init() @@ -185,6 +184,10 @@ } mtx_unlock(&encapmtx); + /* + * XXXRW: Need drain mechanism to prevent the encapsulation + * entry from being released while in use. + */ if (match) { /* found a match, "match" has the best one */ psw = match->psw; @@ -255,6 +258,10 @@ } mtx_unlock(&encapmtx); + /* + * XXXRW: Need drain mechanism so the encap entry isn't freed + * while in use. + */ if (match) { /* found a match */ psw = (const struct ip6protosw *)match->psw; --- //depot/vendor/freebsd/src/sys/netinet/ip_encap.h 2002/03/19 21:30:39 +++ //depot/user/rwatson/netperf/sys/netinet/ip_encap.h 2004/03/12 06:46:13 @@ -35,6 +35,15 @@ #ifdef _KERNEL +/* + * This structure is entirely static after registration, and other than + * its entry in the encapsulation table, requires no locking. The chain + * field is locked using the global encapmtx. + * + * XXXRW: Need to add a refcount/drain mechanism so that encapsulation + * entries can't be removed while in use. This likely requires a + * refcount and cv to wait for it to drain, or an sx lock. + */ struct encaptab { LIST_ENTRY(encaptab) chain; int af; --- //depot/vendor/freebsd/src/sys/netinet/ip_fastfwd.c 2004/06/27 09:10:34 +++ //depot/user/rwatson/netperf/sys/netinet/ip_fastfwd.c 2004/06/27 16:41:59 @@ -642,6 +642,7 @@ /* * Return packet for processing by ip_input */ + /* XXX statistic */ if (ro.ro_rt) RTFREE(ro.ro_rt); return 0; --- //depot/vendor/freebsd/src/sys/netinet/ip_fw2.c 2004/07/15 08:30:31 +++ //depot/user/rwatson/netperf/sys/netinet/ip_fw2.c 2004/07/15 22:19:17 @@ -1541,7 +1541,8 @@ } static int -check_uidgid(ipfw_insn_u32 *insn, +check_uidgid(struct ip_fw_chain *chain, + ipfw_insn_u32 *insn, int proto, struct ifnet *oif, struct in_addr dst_ip, u_int16_t dst_port, struct in_addr src_ip, u_int16_t src_port, @@ -1571,7 +1572,9 @@ return 0; match = 0; if (*lookup == 0) { + IPFW_UNLOCK(chain); INP_INFO_RLOCK(pi); /* XXX LOR with IPFW */ + IPFW_LOCK(chain); pcb = (oif) ? in_pcblookup_hash(pi, dst_ip, htons(dst_port), @@ -1936,7 +1939,7 @@ break; if (proto == IPPROTO_TCP || proto == IPPROTO_UDP) - match = check_uidgid( + match = check_uidgid(chain, (ipfw_insn_u32 *)cmd, proto, oif, dst_ip, dst_port, --- //depot/vendor/freebsd/src/sys/netinet/ip_id.c 2004/02/26 03:55:40 +++ //depot/user/rwatson/netperf/sys/netinet/ip_id.c 2004/03/12 03:03:57 @@ -79,6 +79,9 @@ 2729 }; +/* + * XXXRW: Locking? + */ static u_int16_t ru_x; static u_int16_t ru_seed, ru_seed2; static u_int16_t ru_a, ru_b; --- //depot/vendor/freebsd/src/sys/netinet/ip_input.c 2004/07/08 22:35:48 +++ //depot/user/rwatson/netperf/sys/netinet/ip_input.c 2004/07/09 21:19:53 @@ -938,8 +938,10 @@ /* attach next hop info for TCP */ struct m_tag *mtag = m_tag_get(PACKET_TAG_IPFORWARD, sizeof(struct sockaddr_in *), M_NOWAIT); - if (mtag == NULL) + if (mtag == NULL) { + /* XXX statistic */ goto bad; + } *(struct sockaddr_in **)(mtag+1) = args.next_hop; m_tag_prepend(m, mtag); } @@ -1871,6 +1873,7 @@ struct m_tag *mtag = m_tag_get(PACKET_TAG_IPFORWARD, sizeof(struct sockaddr_in *), M_NOWAIT); if (mtag == NULL) { + /* XXX statistic */ m_freem(m); return; } --- //depot/vendor/freebsd/src/sys/netinet/ip_output.c 2004/06/24 02:11:02 +++ //depot/user/rwatson/netperf/sys/netinet/ip_output.c 2004/06/24 03:49:05 @@ -157,6 +157,12 @@ M_ASSERTPKTHDR(m); + /* + * When packet comes from dummynet restore state from + * previous processing instead of the header. Yech! + * + * XXX add conditional compilation? + */ args.next_hop = m_claim_next(m, PACKET_TAG_IPFORWARD); dummytag = m_tag_find(m, PACKET_TAG_DUMMYNET, NULL); if (dummytag != NULL) { @@ -878,6 +884,7 @@ PACKET_TAG_IPFORWARD, sizeof(struct sockaddr_in *), M_NOWAIT); if (mtag == NULL) { + /* XXX statistic */ error = ENOBUFS; goto bad; } @@ -896,6 +903,7 @@ CSUM_IP_CHECKED | CSUM_IP_VALID; ip->ip_len = htons(ip->ip_len); ip->ip_off = htons(ip->ip_off); + /* XXX netisr_queue(NETISR_IP, m); */ ip_input(m); goto done; } @@ -1410,9 +1418,11 @@ m->m_len = sopt->sopt_valsize; error = sooptcopyin(sopt, mtod(m, char *), m->m_len, m->m_len); - - return (ip_pcbopts(sopt->sopt_name, &inp->inp_options, - m)); + INP_LOCK(inp); + error = ip_pcbopts(sopt->sopt_name, &inp->inp_options, + m); + INP_UNLOCK(inp); + return (error); } case IP_TOS: @@ -1483,7 +1493,15 @@ case IP_MULTICAST_LOOP: case IP_ADD_MEMBERSHIP: case IP_DROP_MEMBERSHIP: + /* + * XXXRW: ip_setmoptions() will calling a blocking + * memory allocation, so the inpcb lock should + * really be acquired in ip_setmoptions(), which + * isn't possible with the current API. + */ + INP_LOCK(inp); error = ip_setmoptions(sopt, &inp->inp_moptions); + INP_UNLOCK(inp); break; case IP_PORTRANGE: @@ -1534,7 +1552,9 @@ req = mtod(m, caddr_t); len = m->m_len; optname = sopt->sopt_name; + INP_LOCK(inp); error = ipsec4_set_policy(inp, optname, req, len, priv); + INP_UNLOCK(inp); m_freem(m); break; } @@ -1550,6 +1570,12 @@ switch (sopt->sopt_name) { case IP_OPTIONS: case IP_RETOPTS: + /* + * XXXRW: We should make a local copy of the + * options while holding the inpcb lock, and then + * copy out without holding the lock. Currently, + * we race. + */ if (inp->inp_options) error = sooptcopyout(sopt, mtod(inp->inp_options, @@ -1627,7 +1653,15 @@ case IP_MULTICAST_LOOP: case IP_ADD_MEMBERSHIP: case IP_DROP_MEMBERSHIP: + /* + * XXXRW: ip_getmoptions() may perform a copy out + * to user space, and should do that without + * holding the inpcb lock. However, we don't pass + * the inpcb down, so it can't do that. + */ + INP_LOCK(inp); error = ip_getmoptions(sopt, inp->inp_moptions); + INP_UNLOCK(inp); break; #if defined(IPSEC) || defined(FAST_IPSEC) --- //depot/vendor/freebsd/src/sys/netinet/raw_ip.c 2004/06/26 19:15:28 +++ //depot/user/rwatson/netperf/sys/netinet/raw_ip.c 2004/07/11 21:34:02 @@ -87,6 +87,9 @@ * so leave them not initialized and rely on BSS being set to 0. */ +/* + * XXXRW: Locking for mrouter bits? + */ /* The socket used to communicate with the multicast routing daemon. */ struct socket *ip_mrouter; @@ -261,7 +264,7 @@ m_freem(m); return(EMSGSIZE); } - M_PREPEND(m, sizeof(struct ip), M_TRYWAIT); + M_PREPEND(m, sizeof(struct ip), M_DONTWAIT); if (m == NULL) return(ENOBUFS); @@ -632,6 +635,7 @@ } INP_LOCK(inp); soisdisconnected(so); + /* Unlocked read. */ if (so->so_state & SS_NOFDREF) rip_pcbdetach(so, inp); else @@ -643,6 +647,8 @@ static int rip_disconnect(struct socket *so) { + + /* Unlocked read. */ if ((so->so_state & SS_ISCONNECTED) == 0) return ENOTCONN; return rip_abort(so); @@ -739,6 +745,7 @@ INP_INFO_WLOCK(&ripcbinfo); inp = sotoinpcb(so); + /* Unlocked read. */ if (so->so_state & SS_ISCONNECTED) { if (nam) { INP_INFO_WUNLOCK(&ripcbinfo); --- //depot/vendor/freebsd/src/sys/netinet/tcp_debug.c 2004/04/07 20:52:05 +++ //depot/user/rwatson/netperf/sys/netinet/tcp_debug.c 2004/04/08 03:11:34 @@ -50,6 +50,7 @@ #include #include #include +#include #include #include --- //depot/vendor/freebsd/src/sys/netinet/tcp_input.c 2004/07/12 19:30:33 +++ //depot/user/rwatson/netperf/sys/netinet/tcp_input.c 2004/07/12 19:43:55 @@ -429,7 +429,7 @@ struct tcpopt to; /* options in this segment */ struct rmxp_tao tao; /* our TAO cache entry */ int headlocked = 0; - struct sockaddr_in *next_hop = NULL; + struct sockaddr_in *next_hop; int rstreason; /* For badport_bandlim accounting purposes */ struct ip6_hdr *ip6 = NULL; @@ -1226,6 +1226,7 @@ tcp_timer_rexmt, tp); sowwakeup(so); + /* Unlocked read. */ if (so->so_snd.sb_cc) (void) tcp_output(tp); goto check_delack; @@ -1744,6 +1745,7 @@ * If new data are received on a connection after the * user processes are gone, then RST the other end. */ + /* Unlocked read. */ if ((so->so_state & SS_NOFDREF) && tp->t_state > TCPS_CLOSE_WAIT && tlen) { tp = tcp_close(tp); @@ -2219,6 +2221,7 @@ * we should release the tp also, and use a * compressed state. */ + /* Unlocked read. */ if (so->so_rcv.sb_state & SBS_CANTRCVMORE) { soisdisconnected(so); callout_reset(tp->tt_2msl, tcp_maxidle, @@ -2305,6 +2308,7 @@ * soreceive. It's hard to imagine someone * actually wanting to send this much urgent data. */ + /* Unlocked read. */ if (th->th_urp + so->so_rcv.sb_cc > sb_max) { th->th_urp = 0; /* XXX */ thflags &= ~TH_URG; /* XXX */ @@ -2324,6 +2328,7 @@ * of data past the urgent section as the original * spec states (in one of two places). */ + /* Unlocked read of sb_cc. */ if (SEQ_GT(th->th_seq+th->th_urp, tp->rcv_up)) { tp->rcv_up = th->th_seq + th->th_urp; SOCKBUF_LOCK(&so->so_rcv); --- //depot/vendor/freebsd/src/sys/netinet/tcp_output.c 2004/06/23 21:05:32 +++ //depot/user/rwatson/netperf/sys/netinet/tcp_output.c 2004/06/23 21:39:47 @@ -250,6 +250,7 @@ * to send then the probe will be the FIN * itself. */ + /* Unlocked read of sb_cc. */ if (off < so->so_snd.sb_cc) flags &= ~TH_FIN; sendwin = 1; @@ -274,6 +275,7 @@ * If sack_rxmit is true we are retransmitting from the scoreboard * in which case len is already set. */ + /* Unlocked read of sb_cc. */ if (!sack_rxmit) len = ((long)ulmin(so->so_snd.sb_cc, sendwin) - off); @@ -335,6 +337,7 @@ len = tp->t_maxseg; sendalot = 1; } + /* Unlocked read of sb_cc. */ if (off + len < so->so_snd.sb_cc) flags &= ~TH_FIN; @@ -362,6 +365,7 @@ * * note: the len + off check is almost certainly unnecessary. */ + /* Unlocked read of sb_cc. */ if (!(tp->t_flags & TF_MORETOCOME) && /* normal case */ (idle || (tp->t_flags & TF_NODELAY)) && len + off >= so->so_snd.sb_cc && @@ -453,6 +457,7 @@ * if window is nonzero, transmit what we can, * otherwise force out a byte. */ + /* Unlocked read of sb_cc. */ if (so->so_snd.sb_cc && !callout_active(tp->tt_rexmt) && !callout_active(tp->tt_persist)) { tp->t_rxtshift = 0; @@ -759,6 +764,7 @@ * give data to the user when a buffer fills or * a PUSH comes in.) */ + /* Unlocked read of sb_cc. */ if (off + len == so->so_snd.sb_cc) flags |= TH_PUSH; } else { --- //depot/vendor/freebsd/src/sys/netinet/tcp_subr.c 2004/06/23 21:36:11 +++ //depot/user/rwatson/netperf/sys/netinet/tcp_subr.c 2004/06/24 03:49:05 @@ -1618,7 +1618,7 @@ /* * Move a TCP connection into TIME_WAIT state. - * tcbinfo is unlocked. + * tcbinfo is locked. * inp is locked, and is unlocked before returning. */ void @@ -1630,6 +1630,11 @@ int tw_time, acknow; struct socket *so; + INP_INFO_WLOCK_ASSERT(&tcbinfo); +#if 0 + INP_LOCK_ASSERT(tp); +#endif + tw = uma_zalloc(tcptw_zone, M_NOWAIT); if (tw == NULL) { tw = tcp_timer_2msl_tw(1); @@ -1763,6 +1768,8 @@ int isipv6 = inp->inp_inc.inc_isipv6; #endif + INP_LOCK_ASSERT(inp); + m = m_gethdr(M_DONTWAIT, MT_HEADER); if (m == NULL) return (ENOBUFS); --- //depot/vendor/freebsd/src/sys/netinet/tcp_syncache.c 2004/06/23 21:05:32 +++ //depot/user/rwatson/netperf/sys/netinet/tcp_syncache.c 2004/06/23 21:39:47 @@ -560,6 +560,9 @@ goto abort2; } #ifdef MAC + /* + * XXXRW: Would prefer inpcb. + */ SOCK_LOCK(so); mac_set_socket_peer_from_mbuf(m, so); SOCK_UNLOCK(so); @@ -730,6 +733,14 @@ abort: INP_UNLOCK(inp); abort2: + /* + * XXXRW: Technically speaking, this soabort() likely doesn't + * do anything, since we insert sockets into the accept queues + * in an already completed state, and soabort() leaves it to + * the close() on the listen socket to remove completed + * connections. However, this means a TCP socket without + * full TCP state could slip through...? + */ if (so != NULL) (void) soabort(so); return (NULL); --- //depot/vendor/freebsd/src/sys/netinet/tcp_timer.c 2004/06/23 21:05:32 +++ //depot/user/rwatson/netperf/sys/netinet/tcp_timer.c 2004/06/23 21:39:47 @@ -271,6 +271,9 @@ } } +/* + * XXXRW: This doesn't look MPSAFE. + */ void tcp_timer_2msl_reset(struct tcptw *tw, int timeo) { @@ -285,6 +288,9 @@ LIST_INSERT_BEFORE(tw_tail, tw, tw_2msl); } +/* + * XXXRW: This doesn't look MPSAFE. + */ void tcp_timer_2msl_stop(struct tcptw *tw) { @@ -293,6 +299,9 @@ LIST_REMOVE(tw, tw_2msl); } +/* + * XXXRW: This doesn't look MPSAFE. + */ struct tcptw * tcp_timer_2msl_tw(int reuse) { --- //depot/vendor/freebsd/src/sys/netinet/tcp_usrreq.c 2004/06/26 17:55:36 +++ //depot/user/rwatson/netperf/sys/netinet/tcp_usrreq.c 2004/06/26 20:54:44 @@ -145,6 +145,50 @@ } /* + * Common code to setup and teardown locking. Most + * code begins with a COMMON_START macro and finishes + * with COMMON_END. You indicate whether the inpcb + * and enclosing head are to be locked read or write. + */ +#define INI_NOLOCK 0 /* no head lock */ +#define INI_READ 1 /* read head lock */ +#define INI_WRITE 2 /* write head lock */ + +#define COMMON_START0(_headrw) do { \ + if (_headrw == INI_READ) \ + INP_INFO_RLOCK(&tcbinfo); \ + else if (_headrw == INI_WRITE) \ + INP_INFO_WLOCK(&tcbinfo); \ + inp = sotoinpcb(so); \ + if (inp == 0) { \ + if (_headrw == INI_READ) \ + INP_INFO_RUNLOCK(&tcbinfo); \ + else if (_headrw == INI_WRITE) \ + INP_INFO_WUNLOCK(&tcbinfo); \ + return EINVAL; \ + } \ + INP_LOCK(inp); \ + if (_headrw == INI_READ) \ + INP_INFO_RUNLOCK(&tcbinfo); \ + tp = intotcpcb(inp); \ + TCPDEBUG1(); \ +} while(0) + +#define COMMON_START(_headrw) do { \ + TCPDEBUG0; \ + COMMON_START0(_headrw); \ +} while (0) + +#define COMMON_END(_headrw, req) \ + TCPDEBUG2(req); \ + do { \ + if (tp) \ + INP_UNLOCK(inp); \ + if (_headrw == INI_WRITE) \ + INP_INFO_WUNLOCK(&tcbinfo); \ + } while(0) + +/* * pru_detach() detaches the TCP protocol from the socket. * If the protocol state is non-embryonic, then can't * do this directly: have to initiate a pru_disconnect(), @@ -157,63 +201,13 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - TCPDEBUG0; - INP_INFO_WLOCK(&tcbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_WUNLOCK(&tcbinfo); - return EINVAL; /* XXX */ - } - INP_LOCK(inp); - tp = intotcpcb(inp); - TCPDEBUG1(); + COMMON_START(INI_WRITE); tp = tcp_disconnect(tp); - - TCPDEBUG2(PRU_DETACH); - if (tp) - INP_UNLOCK(inp); - INP_INFO_WUNLOCK(&tcbinfo); + COMMON_END(INI_WRITE, PRU_DETACH); return error; } -#define INI_NOLOCK 0 -#define INI_READ 1 -#define INI_WRITE 2 - -#define COMMON_START() \ - TCPDEBUG0; \ - do { \ - if (inirw == INI_READ) \ - INP_INFO_RLOCK(&tcbinfo); \ - else if (inirw == INI_WRITE) \ - INP_INFO_WLOCK(&tcbinfo); \ - inp = sotoinpcb(so); \ - if (inp == 0) { \ - if (inirw == INI_READ) \ - INP_INFO_RUNLOCK(&tcbinfo); \ - else if (inirw == INI_WRITE) \ - INP_INFO_WUNLOCK(&tcbinfo); \ - return EINVAL; \ - } \ - INP_LOCK(inp); \ - if (inirw == INI_READ) \ - INP_INFO_RUNLOCK(&tcbinfo); \ - tp = intotcpcb(inp); \ - TCPDEBUG1(); \ -} while(0) - -#define COMMON_END(req) \ -out: TCPDEBUG2(req); \ - do { \ - if (tp) \ - INP_UNLOCK(inp); \ - if (inirw == INI_WRITE) \ - INP_INFO_WUNLOCK(&tcbinfo); \ - return error; \ - goto out; \ -} while(0) - /* * Give the socket an address. */ @@ -224,7 +218,6 @@ struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in *sinp; - const int inirw = INI_WRITE; sinp = (struct sockaddr_in *)nam; if (nam->sa_len != sizeof (*sinp)) @@ -237,11 +230,10 @@ IN_MULTICAST(ntohl(sinp->sin_addr.s_addr))) return (EAFNOSUPPORT); - COMMON_START(); + COMMON_START(INI_WRITE); error = in_pcbbind(inp, nam, td->td_ucred); - if (error) - goto out; - COMMON_END(PRU_BIND); + COMMON_END(INI_WRITE, PRU_BIND); + return error; } #ifdef INET6 @@ -252,7 +244,6 @@ struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in6 *sin6p; - const int inirw = INI_WRITE; sin6p = (struct sockaddr_in6 *)nam; if (nam->sa_len != sizeof (*sin6p)) @@ -265,7 +256,7 @@ IN6_IS_ADDR_MULTICAST(&sin6p->sin6_addr)) return (EAFNOSUPPORT); - COMMON_START(); + COMMON_START(INI_WRITE); inp->inp_vflag &= ~INP_IPV4; inp->inp_vflag |= INP_IPV6; if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0) { @@ -283,9 +274,9 @@ } } error = in6_pcbbind(inp, nam, td->td_ucred); - if (error) - goto out; - COMMON_END(PRU_BIND); +out: + COMMON_END(INI_WRITE, PRU_BIND); + return error; } #endif /* INET6 */ @@ -298,14 +289,14 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE); if (inp->inp_lport == 0) error = in_pcbbind(inp, (struct sockaddr *)0, td->td_ucred); if (error == 0) tp->t_state = TCPS_LISTEN; - COMMON_END(PRU_LISTEN); + COMMON_END(INI_WRITE, PRU_LISTEN); + return error; } #ifdef INET6 @@ -315,9 +306,8 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE); if (inp->inp_lport == 0) { inp->inp_vflag &= ~INP_IPV4; if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0) @@ -326,7 +316,8 @@ } if (error == 0) tp->t_state = TCPS_LISTEN; - COMMON_END(PRU_LISTEN); + COMMON_END(INI_WRITE, PRU_LISTEN); + return error; } #endif /* INET6 */ @@ -344,7 +335,6 @@ struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in *sinp; - const int inirw = INI_WRITE; sinp = (struct sockaddr_in *)nam; if (nam->sa_len != sizeof (*sinp)) @@ -358,11 +348,13 @@ if (td && jailed(td->td_ucred)) prison_remote_ip(td->td_ucred, 0, &sinp->sin_addr.s_addr); - COMMON_START(); + COMMON_START(INI_WRITE); if ((error = tcp_connect(tp, nam, td)) != 0) goto out; error = tcp_output(tp); - COMMON_END(PRU_CONNECT); +out: + COMMON_END(INI_WRITE, PRU_CONNECT); + return error; } #ifdef INET6 @@ -373,7 +365,6 @@ struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in6 *sin6p; - const int inirw = INI_WRITE; sin6p = (struct sockaddr_in6 *)nam; if (nam->sa_len != sizeof (*sin6p)) @@ -385,7 +376,7 @@ && IN6_IS_ADDR_MULTICAST(&sin6p->sin6_addr)) return (EAFNOSUPPORT); - COMMON_START(); + COMMON_START(INI_WRITE); if (IN6_IS_ADDR_V4MAPPED(&sin6p->sin6_addr)) { struct sockaddr_in sin; @@ -408,7 +399,9 @@ if ((error = tcp6_connect(tp, nam, td)) != 0) goto out; error = tcp_output(tp); - COMMON_END(PRU_CONNECT); +out: + COMMON_END(INI_WRITE, PRU_CONNECT); + return error; } #endif /* INET6 */ @@ -429,11 +422,11 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE); tp = tcp_disconnect(tp); - COMMON_END(PRU_DISCONNECT); + COMMON_END(INI_WRITE, PRU_DISCONNECT); + return error; } /* @@ -451,33 +444,24 @@ in_port_t port = 0; TCPDEBUG0; + /* Unlocked read. */ if (so->so_state & SS_ISDISCONNECTED) { error = ECONNABORTED; - goto out; + goto out; /* NB: ok 'cuz tp is NULL */ } - INP_INFO_RLOCK(&tcbinfo); - inp = sotoinpcb(so); - if (!inp) { - INP_INFO_RUNLOCK(&tcbinfo); - return (EINVAL); - } - INP_LOCK(inp); - INP_INFO_RUNLOCK(&tcbinfo); - tp = intotcpcb(inp); - TCPDEBUG1(); + COMMON_START0(INI_READ); /* - * We inline in_setpeeraddr and COMMON_END here, so that we can - * copy the data of interest and defer the malloc until after we - * release the lock. + * We inline in_setpeeraddr so that we can copy the + * data of interest and defer the malloc until after + * we release the lock. */ port = inp->inp_fport; addr = inp->inp_faddr; -out: TCPDEBUG2(PRU_ACCEPT); - if (tp) - INP_UNLOCK(inp); +out: + COMMON_END(INI_READ, PRU_ACCEPT); if (error == 0) *nam = in_sockaddr(port, &addr); return error; @@ -496,25 +480,17 @@ int v4 = 0; TCPDEBUG0; + /* Unlocked read. */ if (so->so_state & SS_ISDISCONNECTED) { error = ECONNABORTED; - goto out; + goto out; /* NB: ok 'cuz tp is NULL */ } - INP_INFO_RLOCK(&tcbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_RUNLOCK(&tcbinfo); - return (EINVAL); - } - INP_LOCK(inp); - INP_INFO_RUNLOCK(&tcbinfo); - tp = intotcpcb(inp); - TCPDEBUG1(); + COMMON_START0(INI_READ); /* - * We inline in6_mapped_peeraddr and COMMON_END here, so that we can - * copy the data of interest and defer the malloc until after we - * release the lock. + * We inline in6_mapped_peeraddr so that we can + * copy the data of interest and defer the malloc + * until after we release the lock. */ if (inp->inp_vflag & INP_IPV4) { v4 = 1; @@ -525,9 +501,8 @@ addr6 = inp->in6p_faddr; } -out: TCPDEBUG2(PRU_ACCEPT); - if (tp) - INP_UNLOCK(inp); +out: + COMMON_END(INI_READ, PRU_ACCEPT); if (error == 0) { if (v4) *nam = in6_v4mapsin6_sockaddr(port, &addr); @@ -568,14 +543,14 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE); socantsendmore(so); tp = tcp_usrclosed(tp); if (tp) error = tcp_output(tp); - COMMON_END(PRU_SHUTDOWN); + COMMON_END(INI_WRITE, PRU_SHUTDOWN); + return error; } /* @@ -587,11 +562,11 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_READ; - COMMON_START(); + COMMON_START(INI_READ); tcp_output(tp); - COMMON_END(PRU_RCVD); + COMMON_END(INI_READ, PRU_RCVD); + return error; } /* @@ -608,7 +583,6 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; #ifdef INET6 int isipv6; #endif @@ -723,13 +697,16 @@ tp->snd_wnd = TTCP_CLIENT_SND_WND; tcp_mss(tp, -1); } + /* Unlocked read of sb_cc. */ tp->snd_up = tp->snd_una + so->so_snd.sb_cc; tp->t_force = 1; error = tcp_output(tp); tp->t_force = 0; } - COMMON_END((flags & PRUS_OOB) ? PRU_SENDOOB : +out: + COMMON_END(INI_WRITE, (flags & PRUS_OOB) ? PRU_SENDOOB : ((flags & PRUS_EOF) ? PRU_SEND_EOF : PRU_SEND)); + return error; } /* @@ -741,11 +718,11 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE); tp = tcp_drop(tp, ECONNABORTED); - COMMON_END(PRU_ABORT); + COMMON_END(INI_WRITE, PRU_ABORT); + return error; } /* @@ -757,9 +734,9 @@ int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_READ; - COMMON_START(); + COMMON_START(INI_READ); + /* Unlocked read. */ if ((so->so_oobmark == 0 && (so->so_rcv.sb_state & SBS_RCVATMARK) == 0) || so->so_options & SO_OOBINLINE || @@ -775,7 +752,9 @@ *mtod(m, caddr_t) = tp->t_iobc; if ((flags & MSG_PEEK) == 0) tp->t_oobflags ^= (TCPOOB_HAVEDATA | TCPOOB_HADDATA); - COMMON_END(PRU_RCVOOB); +out: + COMMON_END(INI_READ, PRU_RCVOOB); + return error; } /* xxx - should be const */ @@ -1171,16 +1150,28 @@ inp->inp_vflag |= INP_IPV4; tp = tcp_newtcpcb(inp); if (tp == 0) { - int nofd = so->so_state & SS_NOFDREF; /* XXX */ - - so->so_state &= ~SS_NOFDREF; /* don't free the socket yet */ + int nofd; + /* + * XXXRW: This is a potentially racy scenario: we perform + * a read-update-write on so_state but don't hold a lock, + * not to mention calling out to external code that may + * grab locks. This section requires attention. + */ + SOCK_LOCK(so); + nofd = so->so_state & SS_NOFDREF; + /* don't free the socket yet */ + if (nofd) + so->so_state &= ~SS_NOFDREF; + SOCK_UNLOCK(so); #ifdef INET6 if (isipv6) in6_pcbdetach(inp); else #endif in_pcbdetach(inp); + SOCK_LOCK(so); so->so_state |= nofd; + SOCK_UNLOCK(so); return (ENOBUFS); } tp->t_state = TCPS_CLOSED; @@ -1260,4 +1251,3 @@ } return (tp); } - --- //depot/vendor/freebsd/src/sys/netinet/udp_usrreq.c 2004/06/26 19:15:28 +++ //depot/user/rwatson/netperf/sys/netinet/udp_usrreq.c 2004/06/26 21:14:54 @@ -904,24 +904,50 @@ SYSCTL_INT(_net_inet_udp, UDPCTL_RECVSPACE, recvspace, CTLFLAG_RW, &udp_recvspace, 0, "Maximum space for incoming UDP datagrams"); +/* + * Common code to setup and teardown locking. Most + * code begins with a COMMON_START macro and finishes + * with COMMON_END. You indicate whether the inpcb + * and enclosing head are to be locked read or write . + */ +#define INI_NOLOCK 0 /* no head lock */ +#define INI_READ 1 /* read head lock */ +#define INI_WRITE 2 /* write head lock */ + +#define COMMON_START(_headrw) do { \ + if (_headrw == INI_READ) \ + INP_INFO_RLOCK(&udbinfo); \ + else if (_headrw == INI_WRITE) \ + INP_INFO_WLOCK(&udbinfo); \ + inp = sotoinpcb(so); \ + if (inp == 0) { \ + if (_headrw == INI_READ) \ + INP_INFO_RUNLOCK(&udbinfo); \ + else if (_headrw == INI_WRITE) \ + INP_INFO_WUNLOCK(&udbinfo); \ + return EINVAL; \ + } \ + INP_LOCK(inp); \ + if (_headrw == INI_READ) \ + INP_INFO_RUNLOCK(&udbinfo); \ +} while(0) + +#define COMMON_END(_headrw) \ + do { \ + INP_UNLOCK(inp); \ + if (_headrw == INI_WRITE) \ + INP_INFO_WUNLOCK(&udbinfo); \ + } while(0) + static int udp_abort(struct socket *so) { struct inpcb *inp; - int s; - INP_INFO_WLOCK(&udbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_WUNLOCK(&udbinfo); - return EINVAL; /* ??? possible? panic instead? */ - } - INP_LOCK(inp); + COMMON_START(INI_WRITE); soisdisconnected(so); - s = splnet(); in_pcbdetach(inp); INP_INFO_WUNLOCK(&udbinfo); - splx(s); return 0; } @@ -963,20 +989,11 @@ udp_bind(struct socket *so, struct sockaddr *nam, struct thread *td) { struct inpcb *inp; - int s, error; + int error; - INP_INFO_WLOCK(&udbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_WUNLOCK(&udbinfo); - return EINVAL; - } - INP_LOCK(inp); - s = splnet(); + COMMON_START(INI_WRITE); error = in_pcbbind(inp, nam, td->td_ucred); - splx(s); - INP_UNLOCK(inp); - INP_INFO_WUNLOCK(&udbinfo); + COMMON_END(INI_WRITE); return error; } @@ -984,31 +1001,22 @@ udp_connect(struct socket *so, struct sockaddr *nam, struct thread *td) { struct inpcb *inp; - int s, error; + int error; struct sockaddr_in *sin; - INP_INFO_WLOCK(&udbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_WUNLOCK(&udbinfo); - return EINVAL; - } - INP_LOCK(inp); + COMMON_START(INI_WRITE); if (inp->inp_faddr.s_addr != INADDR_ANY) { - INP_UNLOCK(inp); - INP_INFO_WUNLOCK(&udbinfo); - return EISCONN; + error = EISCONN; + goto out; } - s = splnet(); sin = (struct sockaddr_in *)nam; if (td && jailed(td->td_ucred)) prison_remote_ip(td->td_ucred, 0, &sin->sin_addr.s_addr); error = in_pcbconnect(inp, nam, td->td_ucred); - splx(s); if (error == 0) soisconnected(so); - INP_UNLOCK(inp); - INP_INFO_WUNLOCK(&udbinfo); +out: + COMMON_END(INI_WRITE); return error; } @@ -1016,49 +1024,31 @@ udp_detach(struct socket *so) { struct inpcb *inp; - int s; - INP_INFO_WLOCK(&udbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_WUNLOCK(&udbinfo); - return EINVAL; - } - INP_LOCK(inp); - s = splnet(); + COMMON_START(INI_WRITE); in_pcbdetach(inp); INP_INFO_WUNLOCK(&udbinfo); - splx(s); return 0; } static int udp_disconnect(struct socket *so) { + int error = 0; struct inpcb *inp; - int s; - INP_INFO_WLOCK(&udbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_WUNLOCK(&udbinfo); - return EINVAL; - } - INP_LOCK(inp); + COMMON_START(INI_WRITE); if (inp->inp_faddr.s_addr == INADDR_ANY) { - INP_INFO_WUNLOCK(&udbinfo); - INP_UNLOCK(inp); - return ENOTCONN; + error = ENOTCONN; + goto out; } - s = splnet(); in_pcbdisconnect(inp); inp->inp_laddr.s_addr = INADDR_ANY; - INP_UNLOCK(inp); - INP_INFO_WUNLOCK(&udbinfo); - splx(s); + COMMON_END(INI_WRITE); so->so_state &= ~SS_ISCONNECTED; /* XXX */ - return 0; +out: + return error; } static int @@ -1066,7 +1056,7 @@ struct mbuf *control, struct thread *td) { struct inpcb *inp; - int ret; + int error; INP_INFO_WLOCK(&udbinfo); inp = sotoinpcb(so); @@ -1076,10 +1066,10 @@ return EINVAL; } INP_LOCK(inp); - ret = udp_output(inp, m, addr, control, td); + error = udp_output(inp, m, addr, control, td); INP_UNLOCK(inp); INP_INFO_WUNLOCK(&udbinfo); - return ret; + return error; } int @@ -1087,16 +1077,9 @@ { struct inpcb *inp; - INP_INFO_RLOCK(&udbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_RUNLOCK(&udbinfo); - return EINVAL; - } - INP_LOCK(inp); - INP_INFO_RUNLOCK(&udbinfo); + COMMON_START(INI_READ); socantsendmore(so); - INP_UNLOCK(inp); + COMMON_END(INI_READ); return 0; } --- //depot/vendor/freebsd/src/sys/netinet6/in6_gif.c 2003/10/29 15:10:52 +++ //depot/user/rwatson/netperf/sys/netinet6/in6_gif.c 2004/03/09 23:48:47 @@ -82,6 +82,11 @@ &rip6_usrreqs }; +/* + * XXXRW: in6_gif per-softc locking required. Need to lock both the + * members, and also prevent the softc from disappearing during use + * including the route). + */ int in6_gif_output(ifp, family, m) struct ifnet *ifp; @@ -379,6 +384,10 @@ in6_gif_attach(sc) struct gif_softc *sc; { + + /* + * XXXRW: Technically, encap_attach() can return NULL due to ENOMEM? + */ sc->encap_cookie6 = encap_attach_func(AF_INET6, -1, gif_encapcheck, (struct protosw *)&in6_gif_protosw, sc); if (sc->encap_cookie6 == NULL) --- //depot/vendor/freebsd/src/sys/netinet6/in6_ifattach.c 2004/02/26 03:55:40 +++ //depot/user/rwatson/netperf/sys/netinet6/in6_ifattach.c 2004/03/07 18:23:16 @@ -226,8 +226,8 @@ struct sockaddr_dl *sdl; u_int8_t *addr; size_t addrlen; - static u_int8_t allzero[8] = { 0, 0, 0, 0, 0, 0, 0, 0 }; - static u_int8_t allone[8] = + static const u_int8_t allzero[8] = { 0, 0, 0, 0, 0, 0, 0, 0 }; + static const u_int8_t allone[8] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; for (ifa = ifp->if_addrlist.tqh_first; --- //depot/vendor/freebsd/src/sys/netinet6/in6_pcb.c 2004/06/12 21:01:46 +++ //depot/user/rwatson/netperf/sys/netinet6/in6_pcb.c 2004/07/01 05:51:43 @@ -130,6 +130,9 @@ u_short lport = 0; int wild = 0, reuseport = (so->so_options & SO_REUSEPORT); + INP_INFO_WLOCK_ASSERT(pcbinfo); + INP_LOCK_ASSERT(inp); + if (!in6_ifaddr) /* XXX broken! */ return (EADDRNOTAVAIL); if (inp->inp_lport || !IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_laddr)) @@ -313,6 +316,9 @@ if (sin6->sin6_port == 0) return (EADDRNOTAVAIL); + INP_INFO_WLOCK_ASSERT(inp->inp_pcbinfo); + INP_LOCK_ASSERT(inp); + /* KAME hack: embed scopeid */ if (in6_embedscope(&sin6->sin6_addr, sin6, inp, &ifp) != 0) return EINVAL; @@ -365,6 +371,9 @@ register struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)nam; int error; + INP_INFO_WLOCK_ASSERT(inp->inp_pcbinfo); + INP_LOCK_ASSERT(inp); + /* * Call inner routine, to assign local interface address. * in6_pcbladdr() may automatically fill in sin6_scope_id. @@ -411,6 +420,10 @@ in6_pcbdisconnect(inp) struct inpcb *inp; { + + INP_INFO_WLOCK_ASSERT(inp->inp_pcbinfo); + INP_LOCK_ASSERT(inp); + bzero((caddr_t)&inp->in6p_faddr, sizeof(inp->in6p_faddr)); inp->inp_fport = 0; /* clear flowinfo - draft-itojun-ipv6-flowlabel-api-00 */ @@ -419,6 +432,7 @@ #ifdef IPSEC ipsec_pcbdisconn(inp->inp_sp); #endif + /* Unlocked read. */ if (inp->inp_socket->so_state & SS_NOFDREF) in6_pcbdetach(inp); } @@ -430,6 +444,9 @@ struct socket *so = inp->inp_socket; struct inpcbinfo *ipi = inp->inp_pcbinfo; + INP_INFO_WLOCK_ASSERT(inp->inp_pcbinfo); + INP_LOCK_ASSERT(inp); + #if defined(IPSEC) || defined(FAST_IPSEC) if (inp->in6p_sp != NULL) ipsec6_delete_pcbpolicy(inp); --- //depot/vendor/freebsd/src/sys/netinet6/in6_prefix.c 2004/04/07 20:52:05 +++ //depot/user/rwatson/netperf/sys/netinet6/in6_prefix.c 2004/04/08 03:11:34 @@ -96,6 +96,8 @@ static int link_stray_ia6s __P((struct rr_prefix *rpp)); static void rp_remove __P((struct rr_prefix *rpp)); +static int delete_each_prefix __P((struct rr_prefix *rpp, u_char origin)); + /* * Copy bits from src to tgt, from off bit for len bits. * Caller must specify collect tgtsize and srcsize. @@ -951,7 +953,7 @@ } } -int +static int delete_each_prefix(struct rr_prefix *rpp, u_char origin) { int error = 0; --- //depot/vendor/freebsd/src/sys/netinet6/in6_prefix.h 2001/07/17 07:20:42 +++ //depot/user/rwatson/netperf/sys/netinet6/in6_prefix.h 2004/03/07 19:31:09 @@ -88,4 +88,3 @@ void in6_rr_timer __P((void *)); extern struct callout in6_rr_timer_ch; -int delete_each_prefix __P((struct rr_prefix *rpp, u_char origin)); --- //depot/vendor/freebsd/src/sys/netinet6/in6_proto.c 2004/04/07 20:52:05 +++ //depot/user/rwatson/netperf/sys/netinet6/in6_proto.c 2004/04/08 03:11:34 @@ -67,6 +67,8 @@ #include "opt_random_ip_id.h" #include +#include +#include #include #include #include --- //depot/vendor/freebsd/src/sys/netinet6/in6_rmx.c 2003/11/20 20:10:44 +++ //depot/user/rwatson/netperf/sys/netinet6/in6_rmx.c 2004/02/28 22:29:37 @@ -79,6 +79,8 @@ #include #include #include +#include +#include #include #include #include --- //depot/vendor/freebsd/src/sys/netinet6/ip6_output.c 2004/05/14 04:01:38 +++ //depot/user/rwatson/netperf/sys/netinet6/ip6_output.c 2004/05/23 16:56:02 @@ -2068,6 +2068,7 @@ bzero(&sro, sizeof(sro)); + /* Unlocked read. */ if (!(so->so_state & SS_ISCONNECTED)) return (ENOTCONN); /* --- //depot/vendor/freebsd/src/sys/netinet6/nd6.c 2004/04/26 20:35:32 +++ //depot/user/rwatson/netperf/sys/netinet6/nd6.c 2004/05/04 02:32:28 @@ -98,6 +98,9 @@ /* for debugging? */ static int nd6_inuse, nd6_allocated; +/* + * XXXRW: What follows requires locking. + */ struct llinfo_nd6 llinfo_nd6 = {&llinfo_nd6, &llinfo_nd6}; struct nd_drhead nd_defrouter; struct nd_prhead nd_prefix = { 0 }; --- //depot/vendor/freebsd/src/sys/netinet6/raw_ip6.c 2004/04/07 20:52:05 +++ //depot/user/rwatson/netperf/sys/netinet6/raw_ip6.c 2004/06/27 18:15:08 @@ -149,24 +149,29 @@ init_sin6(&fromsa, m); /* general init */ + INP_INFO_RLOCK(&ripcbinfo); LIST_FOREACH(in6p, &ripcb, inp_list) { - if ((in6p->in6p_vflag & INP_IPV6) == 0) + INP_LOCK(in6p); + if ((in6p->in6p_vflag & INP_IPV6) == 0) { +docontinue: + INP_UNLOCK(in6p); continue; + } if (in6p->in6p_ip6_nxt && in6p->in6p_ip6_nxt != proto) - continue; + goto docontinue; if (!IN6_IS_ADDR_UNSPECIFIED(&in6p->in6p_laddr) && !IN6_ARE_ADDR_EQUAL(&in6p->in6p_laddr, &ip6->ip6_dst)) - continue; + goto docontinue; if (!IN6_IS_ADDR_UNSPECIFIED(&in6p->in6p_faddr) && !IN6_ARE_ADDR_EQUAL(&in6p->in6p_faddr, &ip6->ip6_src)) - continue; + goto docontinue; if (in6p->in6p_cksum != -1) { rip6stat.rip6s_isum++; if (in6_cksum(m, ip6->ip6_nxt, *offp, m->m_pkthdr.len - *offp)) { rip6stat.rip6s_badsum++; - continue; + goto docontinue; } } if (last) { @@ -201,6 +206,7 @@ sorwakeup(last->in6p_socket); opts = NULL; } + INP_UNLOCK(last); } last = in6p; } @@ -231,6 +237,7 @@ rip6stat.rip6s_fullsock++; } else sorwakeup(last->in6p_socket); + INP_UNLOCK(last); } else { rip6stat.rip6s_nosock++; if (m->m_flags & M_MCAST) @@ -245,6 +252,7 @@ } ip6stat.ip6s_delivered--; } + INP_INFO_RLOCK(&ripcbinfo); return IPPROTO_DONE; } @@ -329,6 +337,13 @@ va_end(ap); in6p = sotoin6pcb(so); + /* + * XXXRW: In IPv6, we don't start referencing the contents of the + * inpcb until after all M_TRYWAIT allocations have finished. We may + * want to reorder this function to provide similar guarantees here, + * so as to avoid holding a mutex over M_TRYWAIT. + */ + INP_LOCK(in6p); stickyopt = in6p->in6p_outputopts; priv = 0; @@ -472,6 +487,7 @@ in6p->in6p_outputopts = stickyopt; m_freem(control); } + INP_UNLOCK(in6p); return (error); } @@ -547,21 +563,31 @@ struct inpcb *inp; int error, s; + INP_INFO_WLOCK(&ripcbinfo); inp = sotoinpcb(so); - if (inp) + if (inp) { + INP_INFO_WUNLOCK(&ripcbinfo); panic("rip6_attach"); - if (td && (error = suser(td)) != 0) + } + if (td && (error = suser(td)) != 0) { + INP_INFO_WUNLOCK(&ripcbinfo); return error; - + } error = soreserve(so, rip_sendspace, rip_recvspace); - if (error) + if (error) { + INP_INFO_WUNLOCK(&ripcbinfo); return error; + } s = splnet(); error = in_pcballoc(so, &ripcbinfo, "raw6inp"); splx(s); - if (error) + if (error) { + INP_INFO_WUNLOCK(&ripcbinfo); return error; + } inp = (struct inpcb *)so->so_pcb; + INP_LOCK(inp); + INP_INFO_WUNLOCK(&ripcbinfo); inp->inp_vflag |= INP_IPV6; inp->in6p_ip6_nxt = (long)proto; inp->in6p_hops = -1; /* use kernel default */ @@ -569,6 +595,7 @@ MALLOC(inp->in6p_icmp6filt, struct icmp6_filter *, sizeof(struct icmp6_filter), M_PCB, M_NOWAIT); ICMP6_FILTER_SETPASSALL(inp->in6p_icmp6filt); + INP_UNLOCK(inp); return 0; } @@ -577,9 +604,12 @@ { struct inpcb *inp; + INP_INFO_WLOCK(&ripcbinfo); inp = sotoinpcb(so); - if (inp == 0) + if (inp == 0) { + INP_INFO_WUNLOCK(&ripcbinfo); panic("rip6_detach"); + } /* xxx: RSVP */ if (so == ip6_mrouter) ip6_mrouter_done(); @@ -587,7 +617,9 @@ FREE(inp->in6p_icmp6filt, M_PCB); inp->in6p_icmp6filt = NULL; } + INP_LOCK(inp); in6_pcbdetach(inp); + INP_INFO_WUNLOCK(&ripcbinfo); return 0; } @@ -603,6 +635,7 @@ { struct inpcb *inp = sotoinpcb(so); + /* Unlocked read. */ if ((so->so_state & SS_ISCONNECTED) == 0) return ENOTCONN; inp->in6p_faddr = in6addr_any; @@ -634,7 +667,11 @@ IN6_IFF_DETACHED|IN6_IFF_DEPRECATED)) { return (EADDRNOTAVAIL); } + INP_INFO_WLOCK(&ripcbinfo); + INP_LOCK(inp); inp->in6p_laddr = addr->sin6_addr; + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&ripcbinfo); return 0; } @@ -663,22 +700,36 @@ addr->sin6_scope_id = scope6_addr2default(&addr->sin6_addr); } #endif + INP_INFO_WLOCK(&ripcbinfo); + INP_LOCK(inp); /* Source address selection. XXX: need pcblookup? */ in6a = in6_selectsrc(addr, inp->in6p_outputopts, inp->in6p_moptions, NULL, &inp->in6p_laddr, &error); - if (in6a == NULL) + if (in6a == NULL) { + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&ripcbinfo); return (error ? error : EADDRNOTAVAIL); + } inp->in6p_laddr = *in6a; inp->in6p_faddr = addr->sin6_addr; soisconnected(so); + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&ripcbinfo); return 0; } static int rip6_shutdown(struct socket *so) { + struct inpcb *inp; + + INP_INFO_RLOCK(&ripcbinfo); + inp = sotoinpcb(so); + INP_LOCK(inp); + INP_INFO_RUNLOCK(&ripcbinfo); socantsendmore(so); + INP_UNLOCK(inp); return 0; } @@ -689,10 +740,14 @@ struct inpcb *inp = sotoinpcb(so); struct sockaddr_in6 tmp; struct sockaddr_in6 *dst; + int ret; + INP_INFO_WLOCK(&ripcbinfo); /* always copy sockaddr to avoid overwrites */ + /* Unlocked read. */ if (so->so_state & SS_ISCONNECTED) { if (nam) { + INP_INFO_WUNLOCK(&ripcbinfo); m_freem(m); return EISCONN; } @@ -705,6 +760,7 @@ dst = &tmp; } else { if (nam == NULL) { + INP_INFO_WUNLOCK(&ripcbinfo); m_freem(m); return ENOTCONN; } @@ -716,7 +772,9 @@ dst->sin6_scope_id = scope6_addr2default(&dst->sin6_addr); } #endif - return rip6_output(m, so, dst, control); + ret = rip6_output(m, so, dst, control); + INP_INFO_WUNLOCK(&ripcbinfo); + return (ret); } struct pr_usrreqs rip6_usrreqs = { --- //depot/vendor/freebsd/src/sys/netinet6/udp6_usrreq.c 2004/04/07 20:52:05 +++ //depot/user/rwatson/netperf/sys/netinet6/udp6_usrreq.c 2004/06/27 05:15:31 @@ -514,21 +514,30 @@ struct inpcb *inp; int s, error; + INP_INFO_WLOCK(&udbinfo); inp = sotoinpcb(so); - if (inp != 0) + if (inp != 0) { + INP_INFO_WUNLOCK(&udbinfo); return EINVAL; + } if (so->so_snd.sb_hiwat == 0 || so->so_rcv.sb_hiwat == 0) { error = soreserve(so, udp_sendspace, udp_recvspace); - if (error) + if (error) { + INP_INFO_WUNLOCK(&udbinfo); return error; + } } s = splnet(); error = in_pcballoc(so, &udbinfo, "udp6inp"); splx(s); - if (error) + if (error) { + INP_INFO_WUNLOCK(&udbinfo); return error; + } inp = (struct inpcb *)so->so_pcb; + INP_LOCK(inp); + INP_INFO_WUNLOCK(&udbinfo); inp->inp_vflag |= INP_IPV6; if (!ip6_v6only) inp->inp_vflag |= INP_IPV4; @@ -541,6 +550,7 @@ * which may match an IPv4-mapped IPv6 address. */ inp->inp_ip_ttl = ip_defttl; + INP_UNLOCK(inp); return 0; } @@ -550,9 +560,13 @@ struct inpcb *inp; int s, error; + INP_INFO_WLOCK(&udbinfo); inp = sotoinpcb(so); - if (inp == 0) + if (inp == 0) { + INP_INFO_WUNLOCK(&udbinfo); return EINVAL; + } + INP_LOCK(inp); inp->inp_vflag &= ~INP_IPV4; inp->inp_vflag |= INP_IPV6; @@ -572,13 +586,15 @@ s = splnet(); error = in_pcbbind(inp, (struct sockaddr *)&sin, td->td_ucred); - splx(s); - return error; + goto out; } } s = splnet(); error = in6_pcbbind(inp, nam, td->td_ucred); +out: + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&udbinfo); splx(s); return error; } @@ -589,9 +605,13 @@ struct inpcb *inp; int s, error; + INP_INFO_WLOCK(&udbinfo); inp = sotoinpcb(so); - if (inp == 0) + if (inp == 0) { + INP_INFO_WUNLOCK(&udbinfo); return EINVAL; + } + INP_LOCK(inp); if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0) { struct sockaddr_in6 *sin6_p; @@ -612,11 +632,13 @@ inp->inp_vflag &= ~INP_IPV6; soisconnected(so); } - return error; + goto out; } } - if (!IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr)) - return EISCONN; + if (!IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr)) { + error = EISCONN; + goto out; + } s = splnet(); error = in6_pcbconnect(inp, nam, td->td_ucred); splx(s); @@ -627,6 +649,9 @@ } soisconnected(so); } +out: + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&udbinfo); return error; } @@ -636,12 +661,17 @@ struct inpcb *inp; int s; + INP_INFO_WLOCK(&udbinfo); inp = sotoinpcb(so); - if (inp == 0) + if (inp == 0) { + INP_INFO_WUNLOCK(&udbinfo); return EINVAL; + } + INP_LOCK(inp); s = splnet(); in6_pcbdetach(inp); splx(s); + INP_INFO_WUNLOCK(&udbinfo); return 0; } @@ -649,29 +679,40 @@ udp6_disconnect(struct socket *so) { struct inpcb *inp; - int s; + int error, s; + INP_INFO_WLOCK(&udbinfo); inp = sotoinpcb(so); - if (inp == 0) + if (inp == 0) { + INP_INFO_WUNLOCK(&udbinfo); return EINVAL; + } + INP_LOCK(inp); #ifdef INET if (inp->inp_vflag & INP_IPV4) { struct pr_usrreqs *pru; pru = inetsw[ip_protox[IPPROTO_UDP]].pr_usrreqs; - return ((*pru->pru_disconnect)(so)); + error = (*pru->pru_disconnect)(so); + goto out; } #endif - if (IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr)) - return ENOTCONN; + if (IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr)) { + error = ENOTCONN; + goto out; + } s = splnet(); in6_pcbdisconnect(inp); inp->in6p_laddr = in6addr_any; splx(s); + /* XXXRW: so_state locking? */ so->so_state &= ~SS_ISCONNECTED; /* XXX */ +out: + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&udbinfo); return 0; } @@ -682,11 +723,14 @@ struct inpcb *inp; int error = 0; + INP_INFO_WLOCK(&udbinfo); inp = sotoinpcb(so); if (inp == 0) { - error = EINVAL; - goto bad; + INP_INFO_WUNLOCK(&udbinfo); + m_freem(m); + return EINVAL; } + INP_LOCK(inp); if (addr) { if (addr->sa_len != sizeof(struct sockaddr_in6)) { @@ -720,7 +764,8 @@ * IPV6_V6ONLY flag, we discard this * datagram destined to a v4 addr. */ - return EINVAL; + error = EINVAL; + goto out; } if (!IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_laddr) && !IN6_IS_ADDR_V4MAPPED(&inp->in6p_laddr)) { @@ -731,7 +776,8 @@ * determine how to map IPv6 source * address to IPv4. */ - return EINVAL; + error = EINVAL; + goto out; } if (sin6) in6_sin6_2_sin_in_sock(addr); @@ -739,14 +785,20 @@ error = ((*pru->pru_send)(so, flags, m, addr, control, td)); /* addr will just be freed in sendit(). */ - return error; + goto out; } } #endif - return udp6_output(inp, m, addr, control, td); + error = udp6_output(inp, m, addr, control, td); +out: + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&udbinfo); + return error; bad: + INP_UNLOCK(inp); + INP_INFO_WUNLOCK(&udbinfo); m_freem(m); return (error); } --- //depot/vendor/freebsd/src/sys/netipsec/keysock.c 2004/06/23 02:01:09 +++ //depot/user/rwatson/netperf/sys/netipsec/keysock.c 2004/06/23 21:39:47 @@ -306,6 +306,11 @@ pfkeystat.in_msgtype[msg->sadb_msg_type]++; } + /* + * XXXRW: This will almost certainly cause lock order problems; + * we may need a netisr solution similar to that used in rtsock. + */ + mtx_lock(&rawcb_mtx); LIST_FOREACH(rp, &rawcb_list, list) { if (rp->rcb_proto.sp_family != PF_KEY) @@ -354,18 +359,21 @@ continue; if ((n = m_copy(m, 0, (int)M_COPYALL)) == NULL) { + mtx_unlock(&rawcb_mtx); m_freem(m); pfkeystat.in_nomem++; return ENOBUFS; } if ((error = key_sendup0(rp, n, 0)) != 0) { + mtx_unlock(&rawcb_mtx); m_freem(m); return error; } n = NULL; } + mtx_lock(&rawcb_mtx); if (so) { error = key_sendup0(sotorawcb(so), m, 0); --- //depot/vendor/freebsd/src/sys/netipx/ipx.c 2003/06/11 05:25:41 +++ //depot/user/rwatson/netperf/sys/netipx/ipx.c 2004/03/18 14:20:12 @@ -50,6 +50,9 @@ #include #include +/* + * XXXRW: Requires synchronization. + */ struct ipx_ifaddr *ipx_ifaddr; static void ipx_ifscrub(struct ifnet *ifp, struct ipx_ifaddr *ia); --- //depot/vendor/freebsd/src/sys/netipx/ipx_input.c 2003/11/08 22:30:39 +++ //depot/user/rwatson/netperf/sys/netipx/ipx_input.c 2004/03/19 10:34:30 @@ -72,23 +72,39 @@ SYSCTL_INT(_net_ipx, OID_AUTO, ipxnetbios, CTLFLAG_RW, &ipxnetbios, 0, ""); -union ipx_net ipx_zeronet; -union ipx_host ipx_zerohost; +const union ipx_net ipx_zeronet; +const union ipx_host ipx_zerohost; -union ipx_net ipx_broadnet; -union ipx_host ipx_broadhost; +const union ipx_net ipx_broadnet = { .s_net[0] = 0xffff, + .s_net[1] = 0xffff }; +const union ipx_host ipx_broadhost = { .s_host[0] = 0xffff, + .s_host[1] = 0xffff, + .s_host[2] = 0xffff }; +/* + * XXXRW: Locking needed here. + */ struct ipxstat ipxstat; + +/* + * XXXRW: These should/could also be const, since they're set only at + * init time. + */ struct sockaddr_ipx ipx_netmask, ipx_hostmask; -static u_short allones[] = {-1, -1, -1}; +/* + * XXXRW: Locking needed here. + */ +u_short ipxpcb_lport_cache; +struct ipxpcbhead ipxpcb_list; +struct ipxpcbhead ipxrawpcb_list; -struct ipxpcb ipxpcb; -struct ipxpcb ipxrawpcb; - static int ipxqmaxlen = IFQ_MAXLEN; static struct ifqueue ipxintrq; +/* + * XXXRW: Locking needed here. + */ long ipx_pexseq; static int ipx_do_route(struct ipx_addr *src, struct route *ro); @@ -103,13 +119,14 @@ void ipx_init() { - ipx_broadnet = *(union ipx_net *)allones; - ipx_broadhost = *(union ipx_host *)allones; read_random(&ipx_pexseq, sizeof ipx_pexseq); - ipxpcb.ipxp_next = ipxpcb.ipxp_prev = &ipxpcb; - ipxrawpcb.ipxp_next = ipxrawpcb.ipxp_prev = &ipxrawpcb; - + LIST_INIT(&ipxpcb_list); + LIST_INIT(&ipxrawpcb_list); + + /* + * XXXRW: These should be const? + */ ipx_netmask.sipx_len = 6; ipx_netmask.sipx_addr.x_net = ipx_broadnet; @@ -133,6 +150,9 @@ struct ipx_ifaddr *ia; int len; + /* + * XXXRW: Would be nice to remove this. + */ GIANT_REQUIRED; /* @@ -153,8 +173,7 @@ /* * Give any raw listeners a crack at the packet */ - for (ipxp = ipxrawpcb.ipxp_next; ipxp != &ipxrawpcb; - ipxp = ipxp->ipxp_next) { + LIST_FOREACH(ipxp, &ipxrawpcb_list, ipxp_list) { struct mbuf *m1 = m_copy(m, 0, (int)M_COPYALL); if (m1 != NULL) ipx_input(m1, ipxp); @@ -467,8 +486,7 @@ /* * Give any raw listeners a crack at the packet */ - for (ipxp = ipxrawpcb.ipxp_next; ipxp != &ipxrawpcb; - ipxp = ipxp->ipxp_next) { + LIST_FOREACH(ipxp, &ipxrawpcb_list, ipxp_list) { struct mbuf *m0 = m_copy(m, 0, (int)M_COPYALL); if (m0 != NULL) { register struct ipx *ipx; --- //depot/vendor/freebsd/src/sys/netipx/ipx_pcb.c 2004/06/12 20:50:42 +++ //depot/user/rwatson/netperf/sys/netipx/ipx_pcb.c 2004/06/12 21:17:12 @@ -56,7 +56,7 @@ int ipx_pcballoc(so, head, td) struct socket *so; - struct ipxpcb *head; + struct ipxpcbhead *head; struct thread *td; { register struct ipxpcb *ipxp; @@ -107,13 +107,16 @@ } ipxp->ipxp_laddr = sipx->sipx_addr; noname: + /* + * XXXRW: I wonder what causes this loop to terminate... + */ if (lport == 0) do { - ipxpcb.ipxp_lport++; - if ((ipxpcb.ipxp_lport < IPXPORT_RESERVED) || - (ipxpcb.ipxp_lport >= IPXPORT_WELLKNOWN)) - ipxpcb.ipxp_lport = IPXPORT_RESERVED; - lport = htons(ipxpcb.ipxp_lport); + ipxpcb_lport_cache++; + if ((ipxpcb_lport_cache < IPXPORT_RESERVED) || + (ipxpcb_lport_cache >= IPXPORT_WELLKNOWN)) + ipxpcb_lport_cache = IPXPORT_RESERVED; + lport = htons(ipxpcb_lport_cache); } while (ipx_pcblookup(&zeroipx_addr, lport, 0)); ipxp->ipxp_lport = lport; return (0); @@ -324,18 +327,27 @@ register struct ipxpcb *ipxp, *oinp; int s = splimp(); - for (ipxp = (&ipxpcb)->ipxp_next; ipxp != (&ipxpcb);) { + for (ipxp = LIST_FIRST(&ipxpcb_list); ipxp != NULL;) { if (!ipx_hosteq(*dst,ipxp->ipxp_faddr)) { - next: - ipxp = ipxp->ipxp_next; +next: + ipxp = LIST_NEXT(ipxp, ipxp_list); + } + if (ipxp->ipxp_socket == 0) { + goto next; continue; } - if (ipxp->ipxp_socket == 0) - goto next; if (errno) ipxp->ipxp_socket->so_error = errno; + /* + * XXXRW: I can't find any consumers of this interface, and + * so don't know if calling the notify function could result + * in the ipxp list pointers changing. Before moving this to + * the queue(9) macros, there was some fancy footwork here + * that didn't seem to be useful. If the list can be changed + * by a notification, it will make locking very difficult. + */ oinp = ipxp; - ipxp = ipxp->ipxp_next; + ipxp = LIST_NEXT(ipxp, ipxp_list); oinp->ipxp_notify_param = param; (*notify)(oinp); } @@ -373,7 +385,7 @@ u_short fport; fport = faddr->x_port; - for (ipxp = (&ipxpcb)->ipxp_next; ipxp != (&ipxpcb); ipxp = ipxp->ipxp_next) { + LIST_FOREACH(ipxp, &ipxpcb_list, ipxp_list) { if (ipxp->ipxp_lport != lport) continue; wildcard = 0; --- //depot/vendor/freebsd/src/sys/netipx/ipx_pcb.h 2003/01/01 18:50:52 +++ //depot/user/rwatson/netperf/sys/netipx/ipx_pcb.h 2004/03/19 10:34:30 @@ -43,9 +43,7 @@ * IPX protocol interface control block. */ struct ipxpcb { - struct ipxpcb *ipxp_next; /* doubly linked list */ - struct ipxpcb *ipxp_prev; - struct ipxpcb *ipxp_head; + LIST_ENTRY(ipxpcb) ipxp_list; /* list of ipxpcbs */ struct socket *ipxp_socket; /* back pointer to socket */ struct ipx_addr ipxp_faddr; /* destination address */ struct ipx_addr ipxp_laddr; /* socket's address */ @@ -58,6 +56,11 @@ u_char ipxp_rpt; /* last received packet type by ipx_input() */ }; +LIST_HEAD(ipxpcbhead, ipxpcb); +extern struct ipxpcbhead ipxpcb_list; +extern struct ipxpcbhead ipxrawpcb_list; +extern u_short ipxpcb_lport_cache; + /* possible flags */ #define IPXP_IN_ABORT 0x1 /* calling abort through socket */ @@ -82,7 +85,7 @@ #ifdef _KERNEL extern struct ipxpcb ipxpcb; /* head of list */ -int ipx_pcballoc(struct socket *so, struct ipxpcb *head, +int ipx_pcballoc(struct socket *so, struct ipxpcbhead *head, struct thread *p); int ipx_pcbbind(struct ipxpcb *ipxp, struct sockaddr *nam, struct thread *p); --- //depot/vendor/freebsd/src/sys/netipx/ipx_usrreq.c 2004/06/12 20:50:42 +++ //depot/user/rwatson/netperf/sys/netipx/ipx_usrreq.c 2004/06/12 21:17:12 @@ -442,7 +442,7 @@ if (ipxp != NULL) return (EINVAL); s = splnet(); - error = ipx_pcballoc(so, &ipxpcb, td); + error = ipx_pcballoc(so, &ipxpcb_list, td); splx(s); if (error == 0) error = soreserve(so, ipxsendspace, ipxrecvspace); @@ -603,7 +603,7 @@ if (td != NULL && (error = suser(td)) != 0) return (error); s = splnet(); - error = ipx_pcballoc(so, &ipxrawpcb, td); + error = ipx_pcballoc(so, &ipxrawpcb_list, td); splx(s); if (error) return (error); --- //depot/vendor/freebsd/src/sys/netipx/ipx_var.h 2003/03/04 23:20:46 +++ //depot/user/rwatson/netperf/sys/netipx/ipx_var.h 2004/03/19 10:34:30 @@ -66,19 +66,19 @@ extern int ipxcksum; extern long ipx_pexseq; extern struct ipxstat ipxstat; -extern struct ipxpcb ipxrawpcb; extern struct pr_usrreqs ipx_usrreqs; extern struct pr_usrreqs ripx_usrreqs; extern struct sockaddr_ipx ipx_netmask; extern struct sockaddr_ipx ipx_hostmask; -extern union ipx_net ipx_zeronet; -extern union ipx_host ipx_zerohost; -extern union ipx_net ipx_broadnet; -extern union ipx_host ipx_broadhost; +extern const union ipx_net ipx_zeronet; +extern const union ipx_host ipx_zerohost; +extern const union ipx_net ipx_broadnet; +extern const union ipx_host ipx_broadhost; struct ifnet; struct ipx_addr; +struct ipxpcb; struct mbuf; struct thread; struct route; --- //depot/vendor/freebsd/src/sys/netipx/spx_usrreq.c 2004/07/12 19:36:23 +++ //depot/user/rwatson/netperf/sys/netipx/spx_usrreq.c 2004/07/12 19:43:55 @@ -1340,7 +1340,7 @@ if (ipxp != NULL) return (EISCONN); s = splnet(); - error = ipx_pcballoc(so, &ipxpcb, td); + error = ipx_pcballoc(so, &ipxpcb_list, td); if (error) goto spx_attach_end; if (so->so_snd.sb_hiwat == 0 || so->so_rcv.sb_hiwat == 0) { @@ -1551,6 +1551,7 @@ ipxp = sotoipxpcb(so); cb = ipxtospxpcb(ipxp); + /* XXXRW: sb_state locking? */ if ((cb->s_oobflags & SF_IOOB) || so->so_oobmark || (so->so_rcv.sb_state & SBS_RCVATMARK)) { m->m_len = 1; @@ -1761,9 +1762,7 @@ register struct spxpcb *cb; int s = splnet(); - ipxp = ipxpcb.ipxp_next; - if (ipxp != NULL) - for (; ipxp != &ipxpcb; ipxp = ipxp->ipxp_next) + LIST_FOREACH(ipxp, &ipxpcb_list, ipxp_list) { if ((cb = (struct spxpcb *)ipxp->ipxp_pcb) != NULL && (cb->s_flags & SF_DELACK)) { cb->s_flags &= ~SF_DELACK; @@ -1771,6 +1770,7 @@ spxstat.spxs_delack++; spx_output(cb, (struct mbuf *)NULL); } + } splx(s); } @@ -1789,15 +1789,15 @@ /* * Search through tcb's and update active timers. + * + * XXXRW: spx_timers() may remove an ipxpcb entry, so we have to be + * ready to continue despite that. The logic here is a bit + * obfuscated. */ - ip = ipxpcb.ipxp_next; - if (ip == NULL) { - splx(s); - return; - } - while (ip != &ipxpcb) { + ip = LIST_FIRST(&ipxpcb_list); + while (ip != NULL) { + ipnxt = LIST_NEXT(ip, ipxp_list); cb = ipxtospxpcb(ip); - ipnxt = ip->ipxp_next; if (cb == NULL) goto tpgone; for (i = 0; i < SPXT_NTIMERS; i++) { @@ -1811,7 +1811,7 @@ if (cb->s_rtt) cb->s_rtt++; tpgone: - ip = ipnxt; + ip = LIST_NEXT(ip, ipxp_list); } spx_iss += SPX_ISSINCR/PR_SLOWHZ; /* increment iss */ splx(s); --- //depot/vendor/freebsd/src/sys/netkey/keysock.c 2004/06/21 00:21:24 +++ //depot/user/rwatson/netperf/sys/netkey/keysock.c 2004/06/21 03:56:40 @@ -214,6 +214,11 @@ pfkeystat.in_msgtype[msg->sadb_msg_type]++; } + /* + * XXXRW: This will almost certainly cause lock order problems; + * we may need a netisr solution similar to that used in rtsock. + */ + mtx_lock(&rawcb_mtx); LIST_FOREACH(rp, &rawcb_list, list) { if (rp->rcb_proto.sp_family != PF_KEY) continue; @@ -261,6 +266,7 @@ continue; if ((n = m_copy(m, 0, (int)M_COPYALL)) == NULL) { + mtx_unlock(&rawcb_mtx); m_freem(m); pfkeystat.in_nomem++; return ENOBUFS; @@ -275,6 +281,7 @@ n = NULL; } + mtx_unlock(&rawcb_mtx); if (so) { error = key_sendup0(sotorawcb(so), m, 0); --- //depot/vendor/freebsd/src/sys/netsmb/smb_trantcp.c 2004/06/17 22:51:43 +++ //depot/user/rwatson/netperf/sys/netsmb/smb_trantcp.c 2004/06/18 00:24:39 @@ -184,6 +184,9 @@ return 0; } +/* + * XXXRW: nb_upcall() is called without Giant, which is probably safeish. + */ static void nb_upcall(struct socket *so, void *arg, int waitflag) { --- //depot/vendor/freebsd/src/sys/nfsclient/bootp_subr.c 2004/07/08 22:35:48 +++ //depot/user/rwatson/netperf/sys/nfsclient/bootp_subr.c 2004/07/09 21:19:53 @@ -983,7 +983,7 @@ struct ifaddr *ifa; struct sockaddr_dl *sdl; - GIANT_REQUIRED; /* XXX until socket locking done */ + NET_ASSERT_GIANT(); error = socreate(AF_INET, &ifctx->so, SOCK_DGRAM, 0, td->td_ucred, td); if (error != 0) --- //depot/vendor/freebsd/src/sys/nfsclient/nfs_socket.c 2004/07/13 05:45:19 +++ //depot/user/rwatson/netperf/sys/nfsclient/nfs_socket.c 2004/07/14 20:52:14 @@ -1199,6 +1199,7 @@ * Set r_rtt to -1 in case we fail to send it now. */ rep->r_rtt = -1; + /* XXXRW: Unlocked reads. */ if (sbspace(&so->so_snd) >= rep->r_mreq->m_pkthdr.len && ((nmp->nm_flag & NFSMNT_DUMBTIMR) || (rep->r_flags & R_SENT) || --- //depot/vendor/freebsd/src/sys/nfsclient/nfs_vfsops.c 2004/07/12 08:15:29 +++ //depot/user/rwatson/netperf/sys/nfsclient/nfs_vfsops.c 2004/07/12 14:54:27 @@ -393,7 +393,7 @@ u_long l; char buf[128]; - GIANT_REQUIRED; /* XXX until socket locking done */ + NET_ASSERT_GIANT(); #if defined(BOOTP_NFSROOT) && defined(BOOTP) bootpc_init(); /* use bootp to get nfs_diskless filled in */ --- //depot/vendor/freebsd/src/sys/nfsserver/nfs_syscalls.c 2004/06/17 22:51:43 +++ //depot/user/rwatson/netperf/sys/nfsserver/nfs_syscalls.c 2004/06/18 00:24:39 @@ -607,6 +607,10 @@ NFSD_UNLOCK(); slp->ns_fp = NULL; so = slp->ns_so; + /* + * XXXRW: More general locking issues here relating to + * upcalls. + */ SOCKBUF_LOCK(&so->so_rcv); so->so_rcv.sb_flags &= ~SB_UPCALL; SOCKBUF_UNLOCK(&so->so_rcv); --- //depot/vendor/freebsd/src/sys/rpc/rpcclnt.c 2004/07/12 19:40:41 +++ //depot/user/rwatson/netperf/sys/rpc/rpcclnt.c 2004/07/12 19:43:55 @@ -145,7 +145,7 @@ /* XXX ugly debug strings */ #define RPC_ERRSTR_ACCEPTED_SIZE 6 -char *rpc_errstr_accepted[RPC_ERRSTR_ACCEPTED_SIZE] = { +const char *rpc_errstr_accepted[RPC_ERRSTR_ACCEPTED_SIZE] = { "", /* no good message... */ "remote server hasn't exported program.", "remote server can't support version number.", @@ -154,13 +154,13 @@ "remote error. remote side memory allocation failure?" }; -char *rpc_errstr_denied[2] = { +const char *rpc_errstr_denied[2] = { "remote server doesnt support rpc version 2!", "remote server authentication error." }; #define RPC_ERRSTR_AUTH_SIZE 6 -char *rpc_errstr_auth[RPC_ERRSTR_AUTH_SIZE] = { +const char *rpc_errstr_auth[RPC_ERRSTR_AUTH_SIZE] = { "", "auth error: bad credential (seal broken).", "auth error: client must begin new session.", @@ -360,7 +360,7 @@ RPC_RETURN(EFAULT); } - GIANT_REQUIRED; /* XXX until socket locking done */ + NET_ASSERT_GIANT(); /* create the socket */ rpc->rc_so = NULL; @@ -618,7 +618,7 @@ { struct socket *so; - GIANT_REQUIRED; /* XXX until socket locking done */ + NET_ASSERT_GIANT(); if (rpc->rc_so) { so = rpc->rc_so; @@ -669,7 +669,7 @@ #endif int error, soflags, flags; - GIANT_REQUIRED; /* XXX until socket locking done */ + NET_ASSERT_GIANT(); if (rep) { if (rep->r_flags & R_SOFTTERM) { @@ -754,7 +754,7 @@ #endif int error, sotype, rcvflg; - GIANT_REQUIRED; /* XXX until socket locking done */ + NET_ASSERT_GIANT(); /* * Set up arguments for soreceive() @@ -1439,6 +1439,7 @@ * Set r_rtt to -1 in case we fail to send it now. */ rep->r_rtt = -1; + SOCKBUF_LOCK(&so->so_snd); if (sbspace(&so->so_snd) >= rep->r_mreq->m_pkthdr.len && ((rpc->rc_flag & RPCCLNT_DUMBTIMR) || (rep->r_flags & R_SENT) || @@ -1473,6 +1474,7 @@ rep->r_rtt = 0; } } + SOCKBUF_UNLOCK(&so->so_snd); } splx(s); --- //depot/vendor/freebsd/src/sys/security/mac/mac_net.c 2004/06/24 03:35:33 +++ //depot/user/rwatson/netperf/sys/security/mac/mac_net.c 2004/06/24 03:49:05 @@ -260,6 +260,11 @@ MAC_PERFORM(copy_ifnet_label, src, dest); } +/* + * XXXRW: Need to use a routine to copy the ifnet label while holding the + * ifnet mutex, similar to sockets, pipes, vnodes, et al, to avoid holding + * the mutex over the copyin/copyout. + */ static int mac_externalize_ifnet_label(struct label *label, char *elements, char *outbuf, size_t outbuflen) @@ -271,6 +276,9 @@ return (error); } +/* + * XXXRW: See comment for mac_externalize_ifnet_label(). + */ static int mac_internalize_ifnet_label(struct label *label, char *string) { --- //depot/vendor/freebsd/src/sys/sys/socketvar.h 2004/07/12 21:45:34 +++ //depot/user/rwatson/netperf/sys/sys/socketvar.h 2004/07/14 20:52:14 @@ -350,6 +350,7 @@ sofree(so); \ else \ SOCK_UNLOCK(so); \ + so = NULL; \ } while (0) #define sotryfree(so) do { \ @@ -358,6 +359,7 @@ sofree(so); \ else \ SOCK_UNLOCK(so); \ + so = NULL; \ } while(0) /*