Date: Wed, 20 Jul 2011 20:37:21 GMT From: Takuya ASADA <syuu@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 196458 for review Message-ID: <201107202037.p6KKbLNG068973@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@196458?ac=10 Change 196458 by syuu@kikurage on 2011/07/20 20:37:16 lock bpf_d during reading/writing qmask, reduce number of threads on test_mqbpf Affected files ... .. //depot/projects/soc2011/mq_bpf/src/sys/net/bpf.c#16 edit .. //depot/projects/soc2011/mq_bpf/src/sys/net/bpfdesc.h#6 edit .. //depot/projects/soc2011/mq_bpf/tests/test_mqbpf.c#4 edit Differences ... ==== //depot/projects/soc2011/mq_bpf/src/sys/net/bpf.c#16 (text+ko) ==== @@ -1,3 +1,4 @@ + /*- * Copyright (c) 1990, 1991, 1993 * The Regents of the University of California. All rights reserved. @@ -1519,16 +1520,19 @@ { struct ifnet *ifp; + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (d->bd_qmask.qm_enabled) { log(LOG_ERR, "d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1536,6 +1540,7 @@ if (!(ifp->if_capabilities & (IFCAP_MULTIQUEUE | IFCAP_SOFT_MULTIQUEUE))) { log(LOG_ERR, "if doesn't support multiqueue\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1549,21 +1554,25 @@ malloc(ifp->if_get_txqueue_len(ifp) * sizeof(boolean_t), M_BPF, M_WAITOK | M_ZERO); d->bd_qmask.qm_other_mask = FALSE; + BPFD_UNLOCK(d); break; } case BIOCDISQMASK: { + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (!d->bd_qmask.qm_enabled) { log(LOG_ERR, "!d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1573,6 +1582,7 @@ free(d->bd_qmask.qm_rxq_mask, M_BPF); if (d->bd_qmask.qm_txq_mask != NULL) free(d->bd_qmask.qm_txq_mask, M_BPF); + BPFD_UNLOCK(d); break; } @@ -1581,16 +1591,19 @@ struct ifnet *ifp; int index; + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (!d->bd_qmask.qm_enabled) { log(LOG_ERR, "!d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1598,10 +1611,12 @@ index = *(uint32_t *)addr; if (index > ifp->if_get_rxqueue_len(ifp)) { log(LOG_ERR, "BIOCSTRXQMASK: index too large index:%x rxq_num:%x\n", index, ifp->if_get_rxqueue_len(ifp)); + BPFD_UNLOCK(d); error = EINVAL; break; } d->bd_qmask.qm_rxq_mask[index] = TRUE; + BPFD_UNLOCK(d); break; } @@ -1610,16 +1625,19 @@ int index; struct ifnet *ifp; + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (!d->bd_qmask.qm_enabled) { log(LOG_ERR, "!d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1627,10 +1645,12 @@ index = *(uint32_t *)addr; if (index > ifp->if_get_rxqueue_len(ifp)) { log(LOG_ERR, "BIOCCRRXQMASK: index too large index:%x rxq_num:%x\n", index, ifp->if_get_rxqueue_len(ifp)); + BPFD_UNLOCK(d); error = EINVAL; break; } d->bd_qmask.qm_rxq_mask[index] = FALSE; + BPFD_UNLOCK(d); break; } @@ -1639,16 +1659,19 @@ int index; struct ifnet *ifp; + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (!d->bd_qmask.qm_enabled) { log(LOG_ERR, "!d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1656,10 +1679,12 @@ index = *(uint32_t *)addr; if (index > ifp->if_get_rxqueue_len(ifp)) { log(LOG_ERR, "BIOCGTRXQMASK: index too large index:%x rxq_num:%x\n", index, ifp->if_get_rxqueue_len(ifp)); + BPFD_UNLOCK(d); error = EINVAL; break; } *(uint32_t *)addr = d->bd_qmask.qm_rxq_mask[index]; + BPFD_UNLOCK(d); break; } @@ -1668,16 +1693,19 @@ struct ifnet *ifp; int index; + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (!d->bd_qmask.qm_enabled) { log(LOG_ERR, "!d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1686,10 +1714,12 @@ index = *(uint32_t *)addr; if (index > ifp->if_get_txqueue_len(ifp)) { log(LOG_ERR, "BIOCSTTXQMASK: index too large index:%x txq_num:%x\n", index, ifp->if_get_txqueue_len(ifp)); + BPFD_UNLOCK(d); error = EINVAL; break; } d->bd_qmask.qm_txq_mask[index] = TRUE; + BPFD_UNLOCK(d); break; } @@ -1698,16 +1728,19 @@ struct ifnet *ifp; int index; + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (!d->bd_qmask.qm_enabled) { log(LOG_ERR, "!d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1716,10 +1749,12 @@ index = *(uint32_t *)addr; if (index > ifp->if_get_txqueue_len(ifp)) { log(LOG_ERR, "BIOCCRTXQMASK: index too large index:%x txq_num:%x\n", index, ifp->if_get_txqueue_len(ifp)); + BPFD_UNLOCK(d); error = EINVAL; break; } d->bd_qmask.qm_txq_mask[index] = FALSE; + BPFD_UNLOCK(d); break; } @@ -1728,16 +1763,19 @@ int index; struct ifnet *ifp; + BPFD_LOCK(d); if (d->bd_bif == NULL) { log(LOG_ERR, "d->bd_bif == NULL\n"); /* * No interface attached yet. */ + BPFD_UNLOCK(d); error = EINVAL; break; } if (!d->bd_qmask.qm_enabled) { log(LOG_ERR, "!d->bd_qmask.qm_enabled\n"); + BPFD_UNLOCK(d); error = EINVAL; break; } @@ -1745,23 +1783,31 @@ index = *(uint32_t *)addr; if (index > ifp->if_get_txqueue_len(ifp)) { log(LOG_ERR, "BIOCGTTXQMASK: index too large index:%x txq_num:%x\n", index, ifp->if_get_txqueue_len(ifp)); + BPFD_UNLOCK(d); error = EINVAL; break; } *(uint32_t *)addr = d->bd_qmask.qm_txq_mask[index]; + BPFD_UNLOCK(d); break; } case BIOCSTOTHERMASK: + BPFD_LOCK(d); d->bd_qmask.qm_other_mask = TRUE; + BPFD_UNLOCK(d); break; case BIOCCROTHERMASK: + BPFD_LOCK(d); d->bd_qmask.qm_other_mask = FALSE; + BPFD_UNLOCK(d); break; case BIOCGTOTHERMASK: + BPFD_LOCK(d); *(uint32_t *)addr = (uint32_t)d->bd_qmask.qm_other_mask; + BPFD_UNLOCK(d); break; } CURVNET_RESTORE(); @@ -2075,12 +2121,14 @@ gottime = BPF_TSTAMP_NONE; BPFIF_RLOCK(bp, &tracker); LIST_FOREACH(d, &bp->bif_dlist, bd_next) { + BPFD_LOCK(d); if (d->bd_qmask.qm_enabled) { - if (!d->bd_qmask.qm_other_mask) + if (!d->bd_qmask.qm_other_mask) { + BPFD_UNLOCK(d); continue; + } } - BPFD_LOCK(d); ++d->bd_rcount; /* * NB: We dont call BPF_CHECK_DIRECTION() here since there is no @@ -2136,36 +2184,45 @@ gottime = BPF_TSTAMP_NONE; BPFIF_RLOCK(bp, &tracker); - struct ifnet *ifp = bp->bif_ifp; LIST_FOREACH(d, &bp->bif_dlist, bd_next) { + if (BPF_CHECK_DIRECTION(d, m->m_pkthdr.rcvif, bp->bif_ifp)) + continue; + BPFD_LOCK(d); if (d->bd_qmask.qm_enabled) { + M_ASSERTPKTHDR(m); if (!(m->m_flags & M_FLOWID)) { - if (!d->bd_qmask.qm_other_mask) + if (!d->bd_qmask.qm_other_mask) { + BPFD_UNLOCK(d); continue; + } } else { - log(LOG_DEBUG, "%s: ifp->if_get_rxqueue_len:%p\n", __func__, ifp->if_get_rxqueue_len); - if (m->m_pkthdr.rxqueue != (uint32_t)-1 && m->m_pkthdr.rxqueue >= ifp->if_get_rxqueue_len(ifp)) { - log(LOG_DEBUG, "rxqueue is not vaild rxqueue:%x rxq_num:%x\n", m->m_pkthdr.rxqueue, ifp->if_get_rxqueue_len(ifp)); - BPFIF_RUNLOCK(bp, &tracker); - return; + struct ifnet *ifp = bp->bif_ifp; + if (m->m_pkthdr.rxqueue != (uint32_t)-1) { + if (m->m_pkthdr.rxqueue >= ifp->if_get_rxqueue_len(ifp)) { + log(LOG_DEBUG, "invalid rxqueue:%d len:%d\n", m->m_pkthdr.rxqueue, ifp->if_get_rxqueue_len(ifp)); + BPFD_UNLOCK(d); + BPFIF_RUNLOCK(bp, &tracker); + return; + } + if (!d->bd_qmask.qm_rxq_mask[m->m_pkthdr.rxqueue]) { + BPFD_UNLOCK(d); + continue; + } } - log(LOG_DEBUG, "%s: ifp->if_get_txqueue_len:%p\n", __func__, ifp->if_get_txqueue_len); - if (m->m_pkthdr.txqueue != (uint32_t)-1 && m->m_pkthdr.txqueue >= ifp->if_get_txqueue_len(ifp)) { - log(LOG_DEBUG, "txqueue is not vaild txqueue:%x txq_num:%x\n", m->m_pkthdr.txqueue, ifp->if_get_txqueue_len(ifp)); - BPFIF_RUNLOCK(bp, &tracker); - return; + if (m->m_pkthdr.txqueue != (uint32_t)-1) { + if (m->m_pkthdr.txqueue >= ifp->if_get_txqueue_len(ifp)) { + log(LOG_DEBUG, "invalid txqueue:%d len:%d\n", m->m_pkthdr.txqueue, ifp->if_get_txqueue_len(ifp)); + BPFD_UNLOCK(d); + BPFIF_RUNLOCK(bp, &tracker); + return; + } + if (!d->bd_qmask.qm_txq_mask[m->m_pkthdr.txqueue]) { + BPFD_UNLOCK(d); + continue; + } } - if (m->m_pkthdr.rxqueue != (uint32_t)-1 && - !d->bd_qmask.qm_rxq_mask[m->m_pkthdr.rxqueue]) - continue; - if (m->m_pkthdr.txqueue != (uint32_t)-1 && - !d->bd_qmask.qm_txq_mask[m->m_pkthdr.txqueue]) - continue; } } - if (BPF_CHECK_DIRECTION(d, m->m_pkthdr.rcvif, bp->bif_ifp)) - continue; - BPFD_LOCK(d); ++d->bd_rcount; #ifdef BPF_JITTER bf = bpf_jitter_enable != 0 ? d->bd_bfilter : NULL; @@ -2224,17 +2281,44 @@ gottime = BPF_TSTAMP_NONE; BPFIF_RLOCK(bp, &tracker); LIST_FOREACH(d, &bp->bif_dlist, bd_next) { - if (d->bd_qmask.qm_enabled) { - if (m->m_pkthdr.rxqueue != (uint32_t)-1 && - !d->bd_qmask.qm_rxq_mask[m->m_pkthdr.rxqueue]) - continue; - if (m->m_pkthdr.txqueue != (uint32_t)-1 && - !d->bd_qmask.qm_txq_mask[m->m_pkthdr.txqueue]) - continue; - } if (BPF_CHECK_DIRECTION(d, m->m_pkthdr.rcvif, bp->bif_ifp)) continue; BPFD_LOCK(d); + if (d->bd_qmask.qm_enabled) { + M_ASSERTPKTHDR(m); + if (!(m->m_flags & M_FLOWID)) { + if (!d->bd_qmask.qm_other_mask) { + BPFD_UNLOCK(d); + continue; + } + } else { + struct ifnet *ifp = bp->bif_ifp; + if (m->m_pkthdr.rxqueue != (uint32_t)-1) { + if (m->m_pkthdr.rxqueue >= ifp->if_get_rxqueue_len(ifp)) { + log(LOG_DEBUG, "invalid rxqueue:%d len:%d\n", m->m_pkthdr.rxqueue, ifp->if_get_rxqueue_len(ifp)); + BPFD_UNLOCK(d); + BPFIF_RUNLOCK(bp, &tracker); + return; + } + if (!d->bd_qmask.qm_rxq_mask[m->m_pkthdr.rxqueue]) { + BPFD_UNLOCK(d); + continue; + } + } + if (m->m_pkthdr.txqueue != (uint32_t)-1) { + if (m->m_pkthdr.txqueue >= ifp->if_get_txqueue_len(ifp)) { + log(LOG_DEBUG, "invalid txqueue:%d len:%d\n", m->m_pkthdr.txqueue, ifp->if_get_txqueue_len(ifp)); + BPFD_UNLOCK(d); + BPFIF_RUNLOCK(bp, &tracker); + return; + } + if (!d->bd_qmask.qm_txq_mask[m->m_pkthdr.txqueue]) { + BPFD_UNLOCK(d); + continue; + } + } + } + } ++d->bd_rcount; slen = bpf_filter(d->bd_rfilter, (u_char *)&mb, pktlen, 0); if (slen != 0) { ==== //depot/projects/soc2011/mq_bpf/src/sys/net/bpfdesc.h#6 (text+ko) ==== @@ -153,7 +153,7 @@ }; #define BPFIF_LOCK_INIT(bif, d) \ - mtx_init(&(bif)->bif_lock, (d), "bpf if lock", MTX_DEF); + mtx_init(&(bif)->bif_lock, "bpf interface lock", NULL, MTX_DEF); #define BPFIF_LOCK_DESTROY(bif) mtx_destroy(&(bif)->bif_lock) #define BPFIF_RLOCK(bif, tracker) mtx_lock(&(bif)->bif_lock) #define BPFIF_RUNLOCK(bif, tracker) mtx_unlock(&(bif)->bif_lock) ==== //depot/projects/soc2011/mq_bpf/tests/test_mqbpf.c#4 (text+ko) ==== @@ -48,6 +48,7 @@ #include <sys/time.h> #include <sys/param.h> #include <sys/cpuset.h> +#include <sys/sysctl.h> #include <net/bpf.h> #include <net/if.h> #include <netinet/in_systm.h> @@ -66,7 +67,7 @@ #include <pthread.h> #include <signal.h> -#define CAP_FORMAT "cap.dat.%d.%d" +#define CAP_FORMAT "cap.dat.%d" #define BPF_FORMAT "/dev/bpf%d" #define QUEUE_TYPE_RX 0 @@ -75,8 +76,9 @@ struct bpf_thread_instance { pthread_t thread; - int type; - int queue; + int rxqueue; + int txqueue; + int other; int cpu; cpuset_t cpuset; int fd; @@ -103,13 +105,12 @@ ifr.ifr_addr.sa_family = AF_LOCAL; strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)); - if (instance->type != QUEUE_TYPE_OTHER) { - CPU_ZERO(&instance->cpuset); - CPU_SET(instance->cpu, &instance->cpuset); - cpuset_setaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, -1, - sizeof(cpuset_t), &instance->cpuset); - } - snprintf(filename, sizeof(filename), CAP_FORMAT, instance->type, instance->queue); + CPU_ZERO(&instance->cpuset); + CPU_SET(instance->cpu, &instance->cpuset); + cpuset_setaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, -1, + sizeof(cpuset_t), &instance->cpuset); + + snprintf(filename, sizeof(filename), CAP_FORMAT, instance->cpu); instance->fd = open(filename, O_RDWR | O_CREAT); if (instance->fd < 0) { perror("open"); @@ -156,30 +157,30 @@ perror("malloc"); return -1; } + if (ioctl(bpf, BIOCENAQMASK, NULL) < 0) { perror("enable qmask"); return -1; } - switch (instance->type) { - case QUEUE_TYPE_RX: - if (ioctl(bpf, BIOCSTRXQMASK, &instance->queue) < 0) { + if (instance->rxqueue > -1) { + if (ioctl(bpf, BIOCSTRXQMASK, &instance->rxqueue) < 0) { perror("rx qmask"); return -1; } - break; - case QUEUE_TYPE_TX: - if (ioctl(bpf, BIOCSTTXQMASK, &instance->queue) < 0) { + } + + if (instance->txqueue > -1) { + if (ioctl(bpf, BIOCSTTXQMASK, &instance->txqueue) < 0) { perror("tx qmask"); return -1; } - break; - case QUEUE_TYPE_OTHER: - if (ioctl(bpf, BIOCSTOTHERMASK, &instance->queue) < 0) { + } + if (instance->other > -1) { + if (ioctl(bpf, BIOCSTOTHERMASK, &instance->other) < 0) { perror("other qmask"); return -1; } - break; } instance->wrote = 0; @@ -271,7 +272,8 @@ int main(int argc, char **argv) { struct ifreq ifr; - int i, j, s, ret; + int i, s, ret, maxcpus; + size_t size = sizeof(maxcpus); struct sigaction action = { .sa_handler = timer_handler, .sa_flags = 0 @@ -307,8 +309,11 @@ } rxqlen = ifr.ifr_rxqueue_len; txqlen = ifr.ifr_txqueue_len; + + sysctlbyname("hw.ncpu", &maxcpus, &size, NULL, 0); + instances = (struct bpf_thread_instance *) - malloc(sizeof(struct bpf_thread_instance) * (rxqlen + txqlen + 1)); + calloc(maxcpus, sizeof(struct bpf_thread_instance)); sigemptyset(&action.sa_mask); if (sigaction(SIGALRM, &action, NULL) < 0) { @@ -321,36 +326,40 @@ } gettimeofday(&start, NULL); + + for (i = 0; i < maxcpus; i++) { + instances[i].cpu = i; + instances[i].rxqueue = -1; + instances[i].txqueue = -1; + instances[i].other = 0; + } + for (i = 0; i < rxqlen; i++) { - instances[i].type = QUEUE_TYPE_RX; - instances[i].queue = ifr.ifr_queue_affinity_index = i; + ifr.ifr_queue_affinity_index = i; if (ioctl(s, SIOCGIFRXQAFFINITY, &ifr)) { perror("SIOCGIFRXQAFFINITY"); return -1; } - instances[i].cpu = ifr.ifr_queue_affinity_cpu; - pthread_create(&instances[i].thread, NULL, - (void *(*)(void *))bpf_thread, &instances[i]); + instances[ifr.ifr_queue_affinity_cpu].rxqueue = i; } - for (i = 0, j = rxqlen; i < txqlen; i++, j++) { - instances[j].type = QUEUE_TYPE_TX; - instances[j].queue = ifr.ifr_queue_affinity_index = i; + + for (i = 0; i < txqlen; i++) { + ifr.ifr_queue_affinity_index = i; if (ioctl(s, SIOCGIFTXQAFFINITY, &ifr)) { - perror("SIOCGIFRXQAFFINITY"); + perror("SIOCGIFTXQAFFINITY"); return -1; } - instances[j].cpu = ifr.ifr_queue_affinity_cpu; - pthread_create(&instances[j].thread, NULL, - (void *(*)(void *))bpf_thread, &instances[j]); + instances[ifr.ifr_queue_affinity_cpu].txqueue = i; } - instances[j].type = QUEUE_TYPE_OTHER; - instances[j].queue = 1; - pthread_create(&instances[j].thread, NULL, - (void *(*)(void *))bpf_thread, &instances[j]); + + instances[0].other = 1; + + for (i = 0; i < maxcpus; i++) + pthread_create(&instances[i].thread, NULL, + (void *(*)(void *))bpf_thread, &instances[i]); - for (i = 0; i < (rxqlen + txqlen + 1); i++) { + for (i = 0; i < maxcpus; i++) pthread_join(instances[i].thread, (void **)&ret); - } return 0; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201107202037.p6KKbLNG068973>