My bad (was: More Boomerang Fixes)

Rob Riggs rob@devilsthumb.com
Tue Jul 7 11:29:53 1998


This is a multi-part message in MIME format.
--------------A1C258ED4F851B861B6B696B
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

OK... Attempt #3 (three's a charm, right?)

It just goes to show, that you read the code over and over
and you still miss the important bits... until just after
you hit "send".

rx_work_limit is required, however, we put the test
*before* any work is done, rather than after, so cases
where rx_work_limit == 0 are caught before we try to
write to NULL pointers. (It is required because we
use parts of the ring buffers when small frames come
in, but leave vp->rx_skbuff[entry] alone. It still
points to a valid skb, but we can end up putting data
in a used vp->rx_ring[entry].)

Also vp->cur_rx and vp->dirty_rx are "unsigned ints"
(what was I thinking?!?) but still prone to the problem
of overflowing (or wrapping). Fixed that as well.

-Rob
--------------A1C258ED4F851B861B6B696B
Content-Type: text/plain; charset=us-ascii; name="3c59x.patch3"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="3c59x.patch3"

--- 3c59x.c.orig	Mon Jul  6 12:16:25 1998
+++ 3c59x.c	Tue Jul  7 13:10:08 1998
@@ -1777,6 +1777,9 @@
 			int pkt_len = rx_status & 0x1fff;
 			struct sk_buff *skb;
 
+			if (--rx_work_limit < 0)
+				break;
+
 			if (vortex_debug > 4)
 				printk(KERN_DEBUG "Receiving packet size %d status %4.4x.\n",
 					   pkt_len, rx_status);
@@ -1801,6 +1804,10 @@
 				void *temp;
 				/* Pass up the skbuff already on the Rx ring. */
 				skb = vp->rx_skbuff[entry];
+				if (skb == NULL) {
+					printk(KERN_WARNING "boomerang: attempt to write to NULL skb caught.\n");
+					break;
+				}
 				vp->rx_skbuff[entry] = NULL;
 #if LINUX_VERSION_CODE >= 0x10300
 				temp = skb_put(skb, pkt_len);
@@ -1833,8 +1840,11 @@
 			vp->stats.rx_packets++;
 		}
 		entry = (++vp->cur_rx) % RX_RING_SIZE;
-		if (--rx_work_limit < 0)
-			break;
+		/* Wrap our counters -- otherwise the next for() loop fails */
+		if (vp->cur_rx == (INT_MAX + RX_RING_SIZE + 1)) {
+			vp->cur_rx -= (INT_MAX + 1);
+			vp->dirty_rx -= (INT_MAX + 1);
+		}
 	}
 	/* Refill the Rx ring buffers. */
 	for (; vp->dirty_rx < vp->cur_rx; vp->dirty_rx++) {
@@ -1842,8 +1852,10 @@
 		entry = vp->dirty_rx % RX_RING_SIZE;
 		if (vp->rx_skbuff[entry] == NULL) {
 			skb = DEV_ALLOC_SKB(PKT_BUF_SZ);
-			if (skb == NULL)
+			if (skb == NULL) {
+				printk(KERN_DEBUG "boomerang: Could not allocate skb on ring buffer.\n");
 				break;			/* Bad news!  */
+			}
 			skb->dev = dev;			/* Mark as being used by this device. */
 #if LINUX_VERSION_CODE > 0x10300
 			skb_reserve(skb, 2);	/* Align IP on 16 byte boundaries */

--------------A1C258ED4F851B861B6B696B--