PERFORCE change 187132 for review

Alexandre Fiveg afiveg at FreeBSD.org
Wed Dec 22 15:21:33 UTC 2010


http://p4web.freebsd.org/@@187132?ac=10

Change 187132 by afiveg at 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;
 };


More information about the p4-projects mailing list