Messages in this thread Patch in this message |  | | | Date | Thu, 25 Nov 1999 03:23:17 +0100 | | From | Wolfgang Walter <> | | Subject | bug in tulip_rx? corrected patch |
| |
Hi,
a just realised that my patch is buggy :-). Of course, if I drop the packet I must not give it to netif_rx() :-).
Wolfgang Walter
Here is the corrected version:
--- 2.2.14pre4/drivers/net/tulip.c Fri Nov 5 18:02:35 1999 +++ 2.2.14pre4-m/drivers/net/tulip.c Thu Nov 25 03:48:23 1999 @@ -509,6 +509,7 @@ int interrupt; /* In-interrupt flag. */ unsigned int cur_rx, cur_tx; /* The next free ring entry */ unsigned int dirty_rx, dirty_tx; /* The ring entries to be free()ed. */ + unsigned int nr_skbs_rx; /* number of skbs */ unsigned int tx_full:1; /* The Tx queue is full. */ unsigned int full_duplex:1; /* Full-duplex operation requested. */ unsigned int full_duplex_lock:1; @@ -2484,6 +2485,7 @@ tp->tx_full = 0; tp->cur_rx = tp->cur_tx = 0; tp->dirty_rx = tp->dirty_tx = 0; + tp->nr_skbs_rx = 0; for (i = 0; i < RX_RING_SIZE; i++) { tp->rx_ring[i].status = 0x00000000; @@ -2503,6 +2505,7 @@ tp->rx_skbuff[i] = skb; if (skb == NULL) break; + tp->nr_skbs_rx++; skb->dev = dev; /* Mark as being used by this device. */ tp->rx_ring[i].status = cpu_to_le32(DescOwned); /* Owned by Tulip chip */ tp->rx_ring[i].buffer1 = virt_to_le32desc(skb->tail); @@ -2595,6 +2598,13 @@ dev->interrupt = 1; #endif + /* tp->nr_skbs_rx == 0 can only happen if we never succeeded to allocate at least + * one skb (which we usually do at initialisation time. + * We then hope do do so now or any later non-RD/RU interrupt + */ + if (tp->nr_skbs_rx == 0) + work_budget -= tulip_rx(dev); + do { csr5 = inl(ioaddr + CSR5); /* Acknowledge all of the current interrupt sources ASAP. */ @@ -2729,7 +2739,7 @@ break; } } while (1); - + if (tulip_debug > 4) printk(KERN_DEBUG "%s: exiting interrupt, csr5=%#4.4x.\n", dev->name, inl(ioaddr + CSR5)); @@ -2797,7 +2807,7 @@ #endif /* Check if the packet is long enough to accept without copying to a minimally-sized skbuff. */ - if (pkt_len < rx_copybreak + if ((pkt_len < rx_copybreak || tp->nr_skbs_rx < 2) && (skb = dev_alloc_skb(pkt_len + 2)) != NULL) { skb->dev = dev; skb_reserve(skb, 2); /* 16 byte align the IP header */ @@ -2809,9 +2819,10 @@ pkt_len); #endif work_done++; - } else { /* Pass up the skb already on the Rx ring. */ + } else if (tp->nr_skbs_rx > 1) { /* Pass up the skb already on the Rx ring. */ char *temp = skb_put(skb = tp->rx_skbuff[entry], pkt_len); tp->rx_skbuff[entry] = NULL; + tp->nr_skbs_rx--; #ifndef final_version if (le32desc_to_virt(tp->rx_ring[entry].buffer1) != temp) printk(KERN_ERR "%s: Internal fault: The skbuff addresses " @@ -2820,14 +2831,20 @@ le32desc_to_virt(tp->rx_ring[entry].buffer1), skb->head, temp); #endif - } - skb->protocol = eth_type_trans(skb, dev); - netif_rx(skb); - dev->last_rx = jiffies; - tp->stats.rx_packets++; + } else { + /* we have only one skb; we must keep it: drop the packet */ + tp->stats.rx_dropped++; + skb = NULL; + } + if (skb) { + skb->protocol = eth_type_trans(skb, dev); + netif_rx(skb); + dev->last_rx = jiffies; + tp->stats.rx_packets++; #if LINUX_VERSION_CODE > 0x20127 - tp->stats.rx_bytes += pkt_len; + tp->stats.rx_bytes += pkt_len; #endif + } } entry = (++tp->cur_rx) % RX_RING_SIZE; } @@ -2840,11 +2857,55 @@ skb = tp->rx_skbuff[entry] = dev_alloc_skb(PKT_BUF_SZ); if (skb == NULL) break; + tp->nr_skbs_rx++; skb->dev = dev; /* Mark as being used by this device. */ tp->rx_ring[entry].buffer1 = virt_to_le32desc(skb->tail); work_done++; } tp->rx_ring[entry].status = cpu_to_le32(DescOwned); + } + + /* + * We must catch the situation: + * the card run out of RX-buffers when this interrupt was triggered (but we have a skb available) + * + * This is the case, if + * tp->nr_skbs_rx > 0 + * and + * all buffers are dirty + * <=> tp->dirty_rx + RX_RING_SIZE == tp->cur_rx + * <=> tp->cur_rx has no skb associated + * + * Why? + * => + * If at this stage all buffers are dirty this means that + * tp->dirty_rx + RX_RING_SIZE == tp->cur_rx + * must hold. This means that the above refill did not change tp->dirty_rx at all + * which means that the associated skb must be null. As this is the same in the ring as + * tp->cur_rx represents, there is no skb associated with tp->cur_rx. + * + * <= + * If tp->cur_rx has no skb associated the rx-loop must be terminated with + * rx_work_limit<0 (tp->rx_ring[entry].status & cpu_to_le32(DescOwned) can't be + * true if no skb is associated). This means that + * tp->dirty_rx + RX_RING_SIZE == tp->cur_rx + * holds which means that all buffers are dirty + * + */ + entry = tp->cur_rx % RX_RING_SIZE; + if (tp->nr_skbs_rx > 0 && tp->rx_skbuff[entry] == NULL) { + int i; + /* search the one free buffer */ + for (i = 0; i < RX_RING_SIZE; i++) { + if (tp->rx_skbuff[i] != NULL) { + tp->rx_skbuff[entry] = tp->rx_skbuff[i]; + tp->rx_ring[entry].buffer1 = tp->rx_ring[i].buffer1; + tp->rx_skbuff[i] = NULL; + tp->rx_ring[entry].status = cpu_to_le32(DescOwned); + tp->dirty_rx++; + break; + } + } } return work_done; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
|  |