3c59x.c 0.99F patch for occasional NULL pointers..
Matti Aarnio
matti.aarnio@sonera.fi
Thu Sep 24 06:59:51 1998
Hello,
Following has patches for 3c59x.c driver version 0.99F so that it
will never (?) succumb to NULL-pointer reference and to all the bad
karma, that such an act causes...
Some explanations per 0.99F:
Sometimes the pre-allocated rx_skbuff[] elements can not be
replinished after they are passed onwards, but at the same time
the address used for direct inbound DMA of the block still
points to OLD buffer location.
The act of passing a frame onwards is done by means of picking
a skb pointer from rx_skbuff[], and replacing it in there with
a NULL pointer.
As the rx_skbuff[] replinishing happens only during RX interrupt
processing, there apparently can be a situation, where DEV_ALLOC_SKB()
yields NULL, and thus the slot can't be replinished.
At the same time, 0.99F will not touch buffer-descriptor, which
contains the physical address of the expected incoming frame, and
thus it is possible that a frame is overwritten with a new data.
What I did:
I have changed here things so that if the rx_skbuff[] replinishing
fails, it will put there NULL, but the physical address will be
that of fixed private area buffer 'backup_buffer', although this
type of '/dev/null' buffer could equally well be a driver-module
local one too. There is no real need to have it in the driver
instance private variables.
Now, if the DEV_ALLOC_SKB() yields NULL, the private buffer is used,
and reception of data into it is counted as 'error', and 'dropped'.
The data is lost, but it has not been thrown at some random location
to cause possible further damage, and the driver won't commit sacriledge
of NULL pointer referral.
There is also one spelling bug, and (for me) usefull module
load-time debugging printout about the location of the load.
The last one of which simplifies the search for the failure
location within the object module dramatically.
Now I still can get hung systems from other causes, but not PANICs
from 3c59x driver.
Now that I have a crossed over full-duplex CAT5 100baseTx wire in
between my two high-speed machines, I see some of:
eth1: Transmit error, Tx status register 82.
messages at my Alpha with this new driver -- like 61 messages per
9.5 million ping packets. The sender does: 'ping -fs 1480 the-other-host'
My Alpha is clocking about 6400 interrupts per second on this card now.
(One interrupt per each packet received, or sent.)
/Matti Aarnio <matti.aarnio@sonera.fi>
--- drivers/net/3c59x.c-099F Mon Sep 14 16:09:31 1998
+++ drivers/net/3c59x.c Thu Sep 24 12:05:03 1998
@@ -439,6 +443,8 @@
u16 capabilities, info1, info2; /* Various, from EEPROM. */
u16 advertising; /* NWay media advertisement */
unsigned char phys[2]; /* MII device addresses. */
+
+ u8 backup_buffer[PKT_BUF_SZ+32]; /* Anything coming here is lost.. */
};
/* The action to take with a media selection timer tick.
@@ -588,8 +592,13 @@
{
if (debug >= 0)
vortex_debug = debug;
- if (vortex_debug)
+ if (vortex_debug) {
printk(version);
+ /* For module debugging knowing the load address of these
+ two are extremely usefull! */
+ printk(" 3c59x.c: &version = 0x%p &init_module = 0x%p\n",
+ &version, &init_module);
+ }
root_vortex_dev = NULL;
#ifdef CARDBUS
@@ -1165,19 +1174,27 @@
for (i = 0; i < RX_RING_SIZE; i++) {
struct sk_buff *skb;
vp->rx_ring[i].next = cpu_to_le32(virt_to_bus(&vp->rx_ring[i+1]));
- vp->rx_ring[i].status = 0; /* Clear complete bit. */
vp->rx_ring[i].length = cpu_to_le32(PKT_BUF_SZ | LAST_FRAG);
skb = DEV_ALLOC_SKB(PKT_BUF_SZ);
vp->rx_skbuff[i] = skb;
- if (skb == NULL)
- break; /* Bad news! */
- skb->dev = dev; /* Mark as being used by this device. */
+ if (skb != NULL) {
+ vp->rx_ring[i].status = 0; /* Clear complete bit. */
+ 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 */
+ vp->rx_ring[i].addr = cpu_to_le32(virt_to_bus(skb->tail));
+#else
+ vp->rx_ring[i].addr = virt_to_bus(skb->data);
+#endif
+ } else {
+ /* skb == NULL */
+ vp->rx_ring[i].status = 0; /* Clear complete bit. */
#if LINUX_VERSION_CODE >= 0x10300
- skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
- vp->rx_ring[i].addr = cpu_to_le32(virt_to_bus(skb->tail));
+ vp->rx_ring[i].addr = cpu_to_le32(virt_to_bus(vp->backup_buffer));
#else
- vp->rx_ring[i].addr = virt_to_bus(skb->data);
+ vp->rx_ring[i].addr = virt_to_bus(vp->backup_buffer);
#endif
+ }
}
/* Wrap the ring. */
vp->rx_ring[i-1].next = cpu_to_le32(virt_to_bus(&vp->rx_ring[0]));
@@ -1192,7 +1209,7 @@
vp->tx_skbuff[i] = 0;
outl(0, ioaddr + DownListPtr);
}
- /* Set reciever mode: presumably accept b-case and phys addr only. */
+ /* Set receiver mode: presumably accept b-case and phys addr only. */
set_rx_mode(dev);
outw(StatsEnable, ioaddr + EL3_CMD); /* Turn on statistics. */
@@ -1832,7 +1849,8 @@
/* Check if the packet is long enough to just accept without
copying to a properly sized skbuff. */
if (pkt_len < rx_copybreak
- && (skb = DEV_ALLOC_SKB(pkt_len + 2)) != 0) {
+ && vp->rx_skbuff[entry] /* NULL stores to 'backup_buffer' */
+ && (skb = DEV_ALLOC_SKB(pkt_len + 2)) != NULL) {
skb->dev = dev;
#if LINUX_VERSION_CODE >= 0x10300
skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
@@ -1851,18 +1869,31 @@
/* Pass up the skbuff already on the Rx ring. */
skb = vp->rx_skbuff[entry];
vp->rx_skbuff[entry] = NULL;
+ if (skb != NULL) {
#if LINUX_VERSION_CODE >= 0x10300
- temp = skb_put(skb, pkt_len);
+ temp = skb_put(skb, pkt_len);
#else
- temp = skb->data;
+ temp = skb->data;
#endif
- /* Remove this checking code for final release. */
- if (bus_to_virt(le32_to_cpu(vp->rx_ring[entry].addr)) != temp)
- printk(KERN_ERR "%s: Warning -- the skbuff addresses do not match"
+ /* Remove this checking code for final release. */
+ if (bus_to_virt(le32_to_cpu(vp->rx_ring[entry].addr)) != temp)
+ printk(KERN_ERR "%s: Warning -- the skbuff addresses do not match"
" in boomerang_rx: %p vs. %p.\n", dev->name,
bus_to_virt(le32_to_cpu(vp->rx_ring[entry].addr)),
temp);
- rx_nocopy++;
+ rx_nocopy++;
+ } else {
+ /* SKB == NULL! */
+#if 0
+ printk(KERN_ERR "%s: skb=NULL; addr=0x%p status=0x%x\n",
+ dev->name,
+ bus_to_virt(le32_to_cpu(vp->rx_ring[entry].addr)),
+ rx_status);
+#endif
+ vp->stats.rx_errors++;
+ vp->stats.rx_dropped++;
+ goto next_frame;
+ }
}
#if LINUX_VERSION_CODE > 0x10300
skb->protocol = eth_type_trans(skb, dev);
@@ -1882,6 +1913,7 @@
dev->last_rx = jiffies;
vp->stats.rx_packets++;
}
+ next_frame:
entry = (++vp->cur_rx) % RX_RING_SIZE;
if (--rx_work_limit < 0)
break;
@@ -1892,18 +1924,28 @@
entry = vp->dirty_rx % RX_RING_SIZE;
if (vp->rx_skbuff[entry] == NULL) {
skb = DEV_ALLOC_SKB(PKT_BUF_SZ);
- if (skb == NULL)
- break; /* Bad news! */
- skb->dev = dev; /* Mark as being used by this device. */
+ if (skb != NULL) {
+ 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 */
- vp->rx_ring[entry].addr = cpu_to_le32(virt_to_bus(skb->tail));
+ skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
+ vp->rx_ring[entry].addr = cpu_to_le32(virt_to_bus(skb->tail));
#else
- vp->rx_ring[entry].addr = virt_to_bus(skb->data);
+ vp->rx_ring[entry].addr = virt_to_bus(skb->data);
#endif
- vp->rx_skbuff[entry] = skb;
+ vp->rx_skbuff[entry] = skb;
+ } else {
+ vp->rx_ring[entry].status = RxDComplete|RxDError|(0x80 << 16); /* Mark unusable with an error code! */
+#if LINUX_VERSION_CODE > 0x10300
+ vp->rx_ring[entry].addr = cpu_to_le32(virt_to_bus(vp->backup_buffer));
+#else
+ vp->rx_ring[entry].addr = virt_to_bus(vp->backup_buffer);
+#endif
+ /* rx_skbuff[] is already NULL */
+ break; /* Bad news! */
+ }
}
- vp->rx_ring[entry].status = 0; /* Clear complete bit. */
+ if (vp->rx_skbuff[entry] != NULL)
+ vp->rx_ring[entry].status = 0; /* Clear complete bit. */
outw(UpUnstall, ioaddr + EL3_CMD);
}
return 0;
@@ -1958,7 +2000,7 @@
vp->rx_skbuff[i]->free = 1;
#endif
DEV_FREE_SKB(vp->rx_skbuff[i]);
- vp->rx_skbuff[i] = 0;
+ vp->rx_skbuff[i] = NULL;
}
}
if (vp->full_bus_master_tx) { /* Free Boomerang bus master Tx buffers. */