Date: Wed, 22 Dec 2010 15:21:32 GMT From: Alexandre Fiveg <afiveg@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 187132 for review Message-ID: <201012221521.oBMFLW7d051707@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@187132?ac=10 Change 187132 by afiveg@cottonmouth on 2010/12/22 15:20:52 Bugfix: panic by kldunload during the capturing thread sleeps and waits for new packets. Affected files ... .. //depot/projects/soc2010/ringmap/current/sys/net/ringmap.c#55 edit .. //depot/projects/soc2010/ringmap/current/sys/net/ringmap.h#54 edit .. //depot/projects/soc2010/ringmap/current/sys/net/ringmap_kernel.h#23 edit .. //depot/projects/soc2010/ringmap/scripts/build_ringmap.sh#39 edit .. //depot/projects/soc2010/ringmap/scripts/set_ringmap.sh#40 edit .. //depot/projects/soc2010/ringmap/scripts/tailf_ringmap_msgs.sh#34 edit .. //depot/projects/soc2010/ringmap/stable_8/contrib/libpcap/ringmap_pcap.c#4 edit .. //depot/projects/soc2010/ringmap/stable_8/sys/net/ringmap.c#4 edit .. //depot/projects/soc2010/ringmap/stable_8/sys/net/ringmap.h#3 edit .. //depot/projects/soc2010/ringmap/stable_8/sys/net/ringmap_kernel.h#3 edit Differences ... ==== //depot/projects/soc2010/ringmap/current/sys/net/ringmap.c#55 (text+ko) ==== @@ -13,6 +13,7 @@ #include <sys/conf.h> #include <sys/kernel.h> #include <sys/ioccom.h> +#include <sys/signalvar.h> #include <machine/bus.h> #include <machine/atomic.h> @@ -274,6 +275,7 @@ co->ring = ring; co->td = td; co->rm = rm; + co->can_sleep = 1; SLIST_INSERT_HEAD(&rm->object_list, co, objects); @@ -332,17 +334,27 @@ RINGMAP_FUNC_DEBUG(start); co = (struct capt_object *)data; + if (co == NULL) { + return ; + } RINGMAP_LOCK(co->rm); CAPT_OBJECT_DEB(co); rm = co->rm; + + /* probably we are sleeping now */ + co->can_sleep = 0; + wakeup(co->ring); + contigfree(co->ring, sizeof(struct ring), M_DEVBUF); co->ring = NULL; - SLIST_REMOVE(&rm->object_list, co, capt_object, objects); - FREE(co, M_DEVBUF); + if (!SLIST_EMPTY(&rm->object_list)) { + SLIST_REMOVE(&rm->object_list, co, capt_object, objects); + FREE(co, M_DEVBUF); + } if (rm->open_cnt > 0) --rm->open_cnt; @@ -410,7 +422,7 @@ if (err != 0) { RINGMAP_IOCTL(Error! Can not get private data!); return (err); - } + } rm = co->rm; @@ -418,30 +430,40 @@ /* Sleep and wait for new packets */ case IOCTL_SLEEP_WAIT: + RINGMAP_LOCK(rm); /* Set the new value into the adapter's TAIL register */ rm->funcs->set_tail(co->ring->userrp, co->hw_rx_ring); CAPT_OBJECT_DEB(co); + /* Is it allowed to sleep ? */ + if (co->can_sleep == 0) { + err = EINVAL; + RINGMAP_UNLOCK(rm); + goto out; + } + /* * Before we are going to sleep it makes a sence to check if we - * really must do it + * really must do it. For example if the ring is not empty, then + * the tread does not need seep and wait for new packets. */ - while (RING_IS_EMPTY(co->ring)) { - RINGMAP_IOCTL(Sleep and wait for new packets); + if (RING_NOT_EMPTY(co->ring)) { + err = EINVAL; + RINGMAP_UNLOCK(rm); + goto out; + } + + RINGMAP_IOCTL(Sleep and wait for new packets); - /* Count how many times we wait for new packets */ - co->ring->user_wait_kern++; + /* Count how many times we wait for new packets */ + co->ring->user_wait_kern++; + RINGMAP_UNLOCK(rm); - err = tsleep(co->ring, - (PRI_MAX_ITHD) | PCATCH, "ioctl", 0); - /* go back into user-space by catching signal */ - if (err) - goto out; - } + err = tsleep(co->ring, + (PRI_MAX_ITHD) | PCATCH, "ioctl", 0); break; - /* Synchronize sowftware ring-tail with hardware-ring-tail (RDT) */ case IOCTL_SYNC_TAIL: RINGMAP_LOCK(rm); @@ -449,7 +471,6 @@ RINGMAP_UNLOCK(rm); break; - case IOCTL_SETFILTER: bpf_prog = (struct bpf_program *)data; flen = bpf_prog->bf_len; @@ -477,14 +498,12 @@ co->rm->funcs->pkt_filter = ringmap_bpf_filter; break; - /* Tell user how many queues we have */ case IOCTL_GETQUEUE_NUM: qn = rm->funcs->get_queuesnum(); *(int *)data = qn; break; - /* Associate the ring/queue with the capturing object */ case IOCTL_ATTACH_RING: /* First stop receive and interupts while we allocate our data */ @@ -512,7 +531,6 @@ rm->funcs->receive_enable(rm); break; - default: RINGMAP_ERROR("Undefined command!"); err = ENODEV; @@ -567,10 +585,12 @@ #if (RINGMAP_INTR_DEB) PRINT_RING_PTRS(co->ring); #endif - rm->funcs->set_tail(co->ring->userrp, co->hw_rx_ring); /* set hardware speciffic time stamp function */ getmicrotime(&co->ring->last_ts); + ++co->ring->intr_num; + rm->funcs->set_tail(co->ring->userrp, co->hw_rx_ring); + break; } } @@ -596,7 +616,7 @@ if (rm->funcs->pkt_filter != NULL) rm->funcs->pkt_filter(co, slot_num); - co->ring->kernrp = slot_num; + co->ring->kernrp = R_MODULO(slot_num + 1); #ifdef RINGMAP_TIMESTAMP co->ring->slot[slot_num].ts = co->ring->last_ts; #endif ==== //depot/projects/soc2010/ringmap/current/sys/net/ringmap.h#54 (text+ko) ==== @@ -274,7 +274,7 @@ #ifndef __RINGMAP_DEB #define __RINGMAP_DEB 0 -#elif +#else #define __RINGMAP_DEB 1 #endif ==== //depot/projects/soc2010/ringmap/current/sys/net/ringmap_kernel.h#23 (text+ko) ==== @@ -32,6 +32,9 @@ */ void *intr_context; + /* If the thread can sleep */ + int volatile can_sleep; + /* Let's concatenate our objects */ SLIST_ENTRY(capt_object) objects; }; ==== //depot/projects/soc2010/ringmap/scripts/build_ringmap.sh#39 (text+ko) ==== @@ -1,16 +1,16 @@ #!/usr/local/bin/bash # wich system release -uname -r | grep STABLE | grep 8 +uname -r | grep STABLE | grep 8 1>/dev/null if [ $? -eq 0 ] then - echo "Building ringmap for FreeBSD-STABLE..." + echo ; echo "Building ringmap for FreeBSD-STABLE..." echo sleep 1 RINGMAP_BUILD_DIR=../stable_8/sys/modules/ringmap/ LIBPCAP_BUILD_DIR=../stable_8/lib/libpcap/ else - echo "Building ringmap for FreeBSD-CURRENT..." + echo ; echo "Building ringmap for FreeBSD-CURRENT..." echo sleep 1 RINGMAP_BUILD_DIR=../current/sys/modules/ringmap/ ==== //depot/projects/soc2010/ringmap/scripts/set_ringmap.sh#40 (text+ko) ==== @@ -1,7 +1,7 @@ #!/bin/sh # wich system release -uname -r | grep STABLE | grep 8 +uname -r | grep STABLE | grep 8 1>/dev/null if [ $? -eq 0 ] then echo "Installing ringmap for FreeBSD-STABLE..." ==== //depot/projects/soc2010/ringmap/scripts/tailf_ringmap_msgs.sh#34 (text+ko) ==== ==== //depot/projects/soc2010/ringmap/stable_8/contrib/libpcap/ringmap_pcap.c#4 (text+ko) ==== @@ -267,7 +267,7 @@ /* catching signals */ if (err_sleep) { - if (errno == EINTR) { + if ((errno == EINTR) || (errno == EINVAL)) { pcap_close(p); exit(0); } ==== //depot/projects/soc2010/ringmap/stable_8/sys/net/ringmap.c#4 (text+ko) ==== @@ -13,6 +13,7 @@ #include <sys/conf.h> #include <sys/kernel.h> #include <sys/ioccom.h> +#include <sys/signalvar.h> #include <machine/bus.h> #include <machine/atomic.h> @@ -274,6 +275,7 @@ co->ring = ring; co->td = td; co->rm = rm; + co->can_sleep = 1; SLIST_INSERT_HEAD(&rm->object_list, co, objects); @@ -332,18 +334,27 @@ RINGMAP_FUNC_DEBUG(start); co = (struct capt_object *)data; + if (co == NULL) { + return ; + } RINGMAP_LOCK(co->rm); CAPT_OBJECT_DEB(co); rm = co->rm; - /* TODO: wake up if we are currently sleeping */ + + /* probably we are sleeping now */ + co->can_sleep = 0; + wakeup(co->ring); + contigfree(co->ring, sizeof(struct ring), M_DEVBUF); co->ring = NULL; - SLIST_REMOVE(&rm->object_list, co, capt_object, objects); - FREE(co, M_DEVBUF); + if (!SLIST_EMPTY(&rm->object_list)) { + SLIST_REMOVE(&rm->object_list, co, capt_object, objects); + FREE(co, M_DEVBUF); + } if (rm->open_cnt > 0) --rm->open_cnt; @@ -411,7 +422,7 @@ if (err != 0) { RINGMAP_IOCTL(Error! Can not get private data!); return (err); - } + } rm = co->rm; @@ -419,30 +430,40 @@ /* Sleep and wait for new packets */ case IOCTL_SLEEP_WAIT: + RINGMAP_LOCK(rm); /* Set the new value into the adapter's TAIL register */ rm->funcs->set_tail(co->ring->userrp, co->hw_rx_ring); CAPT_OBJECT_DEB(co); + /* Is it allowed to sleep ? */ + if (co->can_sleep == 0) { + err = EINVAL; + RINGMAP_UNLOCK(rm); + goto out; + } + /* * Before we are going to sleep it makes a sence to check if we - * really must do it + * really must do it. For example if the ring is not empty, then + * the tread does not need seep and wait for new packets. */ - while (RING_IS_EMPTY(co->ring)) { - RINGMAP_IOCTL(Sleep and wait for new packets); + if (RING_NOT_EMPTY(co->ring)) { + err = EINVAL; + RINGMAP_UNLOCK(rm); + goto out; + } + + RINGMAP_IOCTL(Sleep and wait for new packets); - /* Count how many times we wait for new packets */ - co->ring->user_wait_kern++; + /* Count how many times we wait for new packets */ + co->ring->user_wait_kern++; + RINGMAP_UNLOCK(rm); - err = tsleep(co->ring, - (PRI_MAX_ITHD) | PCATCH, "ioctl", 0); - /* go back into user-space by catching signal */ - if (err) - goto out; - } + err = tsleep(co->ring, + (PRI_MAX_ITHD) | PCATCH, "ioctl", 0); break; - /* Synchronize sowftware ring-tail with hardware-ring-tail (RDT) */ case IOCTL_SYNC_TAIL: RINGMAP_LOCK(rm); @@ -450,7 +471,6 @@ RINGMAP_UNLOCK(rm); break; - case IOCTL_SETFILTER: bpf_prog = (struct bpf_program *)data; flen = bpf_prog->bf_len; @@ -478,14 +498,12 @@ co->rm->funcs->pkt_filter = ringmap_bpf_filter; break; - /* Tell user how many queues we have */ case IOCTL_GETQUEUE_NUM: qn = rm->funcs->get_queuesnum(); *(int *)data = qn; break; - /* Associate the ring/queue with the capturing object */ case IOCTL_ATTACH_RING: /* First stop receive and interupts while we allocate our data */ @@ -513,7 +531,6 @@ rm->funcs->receive_enable(rm); break; - default: RINGMAP_ERROR("Undefined command!"); err = ENODEV; ==== //depot/projects/soc2010/ringmap/stable_8/sys/net/ringmap.h#3 (text+ko) ==== ==== //depot/projects/soc2010/ringmap/stable_8/sys/net/ringmap_kernel.h#3 (text+ko) ==== @@ -32,6 +32,9 @@ */ void *intr_context; + /* If the thread can sleep */ + int volatile can_sleep; + /* Let's concatenate our objects */ SLIST_ENTRY(capt_object) objects; };
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201012221521.oBMFLW7d051707>