* [PATCH 0/5]: spidernet: misc fixes
@ 2007-02-10 0:01 Linas Vepstas
2007-02-10 0:14 ` [PATCH 1/5]: spidernet: compile break Linas Vepstas
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-02-10 0:01 UTC (permalink / raw)
To: jgarzik; +Cc: linuxppc-dev, James K Lewis, Jens Osterkamp, netdev
Jeff,
A series of five patches against the spidernet ethernet device driver.
The first fixes a compile break in the current kernel.rg tree.
The second restructures the descriptor ring, per an old suggestion.
The third & fourth fix a race condition seen under heavy load.
The fifth is a trite janitorial patch.
Please apply.
--linas
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/5]: spidernet: compile break.
2007-02-10 0:01 [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
@ 2007-02-10 0:14 ` Linas Vepstas
2007-02-10 0:16 ` [PATCH 2/5] spidernet: separate hardware state from driver state Linas Vepstas
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-02-10 0:14 UTC (permalink / raw)
To: jgarzik; +Cc: linuxppc-dev, James K Lewis, Jens Osterkamp, netdev
As of 2.6.20-git4, the spider_net driver does not compile.
This appears to be due to some archaic usage involving kobjects.
It also fixes a nasty double-free during ifdown of interface.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Jens Osterkamp <Jens.Osterkamp@de.ibm.com>
Cc: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
----
drivers/net/spider_net.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
Index: linux-2.6.20-git4/drivers/net/spider_net.c
===================================================================
--- linux-2.6.20-git4.orig/drivers/net/spider_net.c 2007-02-09 17:22:35.000000000 -0600
+++ linux-2.6.20-git4/drivers/net/spider_net.c 2007-02-09 17:24:04.000000000 -0600
@@ -1906,8 +1906,7 @@ spider_net_stop(struct net_device *netde
spider_net_write_reg(card, SPIDER_NET_GHIINT1MSK, 0);
spider_net_write_reg(card, SPIDER_NET_GHIINT2MSK, 0);
- /* free_irq(netdev->irq, netdev);*/
- free_irq(to_pci_dev(netdev->class_dev.dev)->irq, netdev);
+ free_irq(netdev->irq, netdev);
spider_net_write_reg(card, SPIDER_NET_GDTDMACCNTR,
SPIDER_NET_DMA_TX_FEND_VALUE);
@@ -1919,8 +1918,6 @@ spider_net_stop(struct net_device *netde
spider_net_release_tx_chain(card, 1);
spider_net_free_rx_chain_contents(card);
- spider_net_free_rx_chain_contents(card);
-
spider_net_free_chain(card, &card->tx_chain);
spider_net_free_chain(card, &card->rx_chain);
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/5] spidernet: separate hardware state from driver state.
2007-02-10 0:01 [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
2007-02-10 0:14 ` [PATCH 1/5]: spidernet: compile break Linas Vepstas
@ 2007-02-10 0:16 ` Linas Vepstas
2007-02-10 0:19 ` [PATCH: 3/5]: spidernet: fix racy double-free of skb Linas Vepstas
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-02-10 0:16 UTC (permalink / raw)
To: jgarzik; +Cc: linuxppc-dev, James K Lewis, Jens Osterkamp, netdev
This patch separates the hardware descriptor state from the
driver descriptor state, per (old) suggestion from Ben Herrenschmidt.
This compiles and boots and seems to work.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Jens Osterkamp <Jens.Osterkamp@de.ibm.com>
Cc: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
----
drivers/net/spider_net.c | 150 ++++++++++++++++++++++++++---------------------
drivers/net/spider_net.h | 16 +++--
2 files changed, 95 insertions(+), 71 deletions(-)
Index: linux-2.6.20-git4/drivers/net/spider_net.h
===================================================================
--- linux-2.6.20-git4.orig/drivers/net/spider_net.h 2007-02-09 17:22:15.000000000 -0600
+++ linux-2.6.20-git4/drivers/net/spider_net.h 2007-02-09 17:29:15.000000000 -0600
@@ -24,7 +24,7 @@
#ifndef _SPIDER_NET_H
#define _SPIDER_NET_H
-#define VERSION "1.6 B"
+#define VERSION "1.6 C"
#include "sungem_phy.h"
@@ -356,8 +356,8 @@ enum spider_net_int2_status {
#define SPIDER_NET_DESCR_NOT_IN_USE 0xF0000000
#define SPIDER_NET_DESCR_TXDESFLG 0x00800000
-struct spider_net_descr {
- /* as defined by the hardware */
+/* Descriptor, as defined by the hardware */
+struct spider_net_hw_descr {
u32 buf_addr;
u32 buf_size;
u32 next_descr_addr;
@@ -366,13 +366,15 @@ struct spider_net_descr {
u32 valid_size; /* all zeroes for tx */
u32 data_status;
u32 data_error; /* all zeroes for tx */
+} __attribute__((aligned(32)));
- /* used in the driver */
+struct spider_net_descr {
+ struct spider_net_hw_descr *hwdescr;
struct sk_buff *skb;
u32 bus_addr;
struct spider_net_descr *next;
struct spider_net_descr *prev;
-} __attribute__((aligned(32)));
+};
struct spider_net_descr_chain {
spinlock_t lock;
@@ -380,6 +382,7 @@ struct spider_net_descr_chain {
struct spider_net_descr *tail;
struct spider_net_descr *ring;
int num_desc;
+ struct spider_net_hw_descr *hwring;
dma_addr_t dma_addr;
};
@@ -452,6 +455,9 @@ struct spider_net_card {
struct net_device_stats netdev_stats;
struct spider_net_extra_stats spider_stats;
struct spider_net_options options;
+
+ /* Must be last item in struct */
+ struct spider_net_descr darray[0];
};
#define pr_err(fmt,arg...) \
Index: linux-2.6.20-git4/drivers/net/spider_net.c
===================================================================
--- linux-2.6.20-git4.orig/drivers/net/spider_net.c 2007-02-09 17:24:04.000000000 -0600
+++ linux-2.6.20-git4/drivers/net/spider_net.c 2007-02-09 17:29:15.000000000 -0600
@@ -263,9 +263,9 @@ spider_net_get_mac_address(struct net_de
* returns the status as in the dmac_cmd_status field of the descriptor
*/
static inline int
-spider_net_get_descr_status(struct spider_net_descr *descr)
+spider_net_get_descr_status(struct spider_net_hw_descr *hwdescr)
{
- return descr->dmac_cmd_status & SPIDER_NET_DESCR_IND_PROC_MASK;
+ return hwdescr->dmac_cmd_status & SPIDER_NET_DESCR_IND_PROC_MASK;
}
/**
@@ -283,12 +283,12 @@ spider_net_free_chain(struct spider_net_
descr = chain->ring;
do {
descr->bus_addr = 0;
- descr->next_descr_addr = 0;
+ descr->hwdescr->next_descr_addr = 0;
descr = descr->next;
} while (descr != chain->ring);
dma_free_coherent(&card->pdev->dev, chain->num_desc,
- chain->ring, chain->dma_addr);
+ chain->hwring, chain->dma_addr);
}
/**
@@ -307,31 +307,34 @@ spider_net_init_chain(struct spider_net_
{
int i;
struct spider_net_descr *descr;
+ struct spider_net_hw_descr *hwdescr;
dma_addr_t buf;
size_t alloc_size;
- alloc_size = chain->num_desc * sizeof (struct spider_net_descr);
+ alloc_size = chain->num_desc * sizeof(struct spider_net_hw_descr);
- chain->ring = dma_alloc_coherent(&card->pdev->dev, alloc_size,
+ chain->hwring = dma_alloc_coherent(&card->pdev->dev, alloc_size,
&chain->dma_addr, GFP_KERNEL);
- if (!chain->ring)
+ if (!chain->hwring)
return -ENOMEM;
- descr = chain->ring;
- memset(descr, 0, alloc_size);
+ memset(chain->ring, 0, chain->num_desc * sizeof(struct spider_net_descr));
/* Set up the hardware pointers in each descriptor */
+ descr = chain->ring;
+ hwdescr = chain->hwring;
buf = chain->dma_addr;
- for (i=0; i < chain->num_desc; i++, descr++) {
- descr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
+ for (i=0; i < chain->num_desc; i++, descr++, hwdescr++) {
+ hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
+ hwdescr->next_descr_addr = 0;
+ descr->hwdescr = hwdescr;
descr->bus_addr = buf;
- descr->next_descr_addr = 0;
descr->next = descr + 1;
descr->prev = descr - 1;
- buf += sizeof(struct spider_net_descr);
+ buf += sizeof(struct spider_net_hw_descr);
}
/* do actual circular list */
(descr-1)->next = chain->ring;
@@ -358,7 +361,7 @@ spider_net_free_rx_chain_contents(struct
do {
if (descr->skb) {
dev_kfree_skb(descr->skb);
- pci_unmap_single(card->pdev, descr->buf_addr,
+ pci_unmap_single(card->pdev, descr->hwdescr->buf_addr,
SPIDER_NET_MAX_FRAME,
PCI_DMA_BIDIRECTIONAL);
}
@@ -380,6 +383,7 @@ static int
spider_net_prepare_rx_descr(struct spider_net_card *card,
struct spider_net_descr *descr)
{
+ struct spider_net_hw_descr *hwdescr = descr->hwdescr;
dma_addr_t buf;
int offset;
int bufsize;
@@ -398,11 +402,11 @@ spider_net_prepare_rx_descr(struct spide
card->spider_stats.alloc_rx_skb_error++;
return -ENOMEM;
}
- descr->buf_size = bufsize;
- descr->result_size = 0;
- descr->valid_size = 0;
- descr->data_status = 0;
- descr->data_error = 0;
+ hwdescr->buf_size = bufsize;
+ hwdescr->result_size = 0;
+ hwdescr->valid_size = 0;
+ hwdescr->data_status = 0;
+ hwdescr->data_error = 0;
offset = ((unsigned long)descr->skb->data) &
(SPIDER_NET_RXBUF_ALIGN - 1);
@@ -411,21 +415,21 @@ spider_net_prepare_rx_descr(struct spide
/* iommu-map the skb */
buf = pci_map_single(card->pdev, descr->skb->data,
SPIDER_NET_MAX_FRAME, PCI_DMA_FROMDEVICE);
- descr->buf_addr = buf;
if (pci_dma_mapping_error(buf)) {
dev_kfree_skb_any(descr->skb);
if (netif_msg_rx_err(card) && net_ratelimit())
pr_err("Could not iommu-map rx buffer\n");
card->spider_stats.rx_iommu_map_error++;
- descr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
+ hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
} else {
- descr->next_descr_addr = 0;
+ hwdescr->buf_addr = buf;
+ hwdescr->next_descr_addr = 0;
wmb();
- descr->dmac_cmd_status = SPIDER_NET_DESCR_CARDOWNED |
+ hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_CARDOWNED |
SPIDER_NET_DMAC_NOINTR_COMPLETE;
wmb();
- descr->prev->next_descr_addr = descr->bus_addr;
+ descr->prev->hwdescr->next_descr_addr = descr->bus_addr;
}
return 0;
@@ -481,7 +485,7 @@ spider_net_refill_rx_chain(struct spider
if (!spin_trylock_irqsave(&chain->lock, flags))
return;
- while (spider_net_get_descr_status(chain->head) ==
+ while (spider_net_get_descr_status(chain->head->hwdescr) ==
SPIDER_NET_DESCR_NOT_IN_USE) {
if (spider_net_prepare_rx_descr(card, chain->head))
break;
@@ -643,6 +647,7 @@ spider_net_prepare_tx_descr(struct spide
struct sk_buff *skb)
{
struct spider_net_descr *descr;
+ struct spider_net_hw_descr *hwdescr;
dma_addr_t buf;
unsigned long flags;
@@ -657,30 +662,32 @@ spider_net_prepare_tx_descr(struct spide
spin_lock_irqsave(&card->tx_chain.lock, flags);
descr = card->tx_chain.head;
+ hwdescr = descr->hwdescr;
card->tx_chain.head = descr->next;
- descr->buf_addr = buf;
- descr->buf_size = skb->len;
- descr->next_descr_addr = 0;
descr->skb = skb;
- descr->data_status = 0;
+ hwdescr->buf_addr = buf;
+ hwdescr->buf_size = skb->len;
+ hwdescr->next_descr_addr = 0;
+ hwdescr->data_status = 0;
- descr->dmac_cmd_status =
+ hwdescr->dmac_cmd_status =
SPIDER_NET_DESCR_CARDOWNED | SPIDER_NET_DMAC_NOCS;
spin_unlock_irqrestore(&card->tx_chain.lock, flags);
if (skb->protocol == htons(ETH_P_IP))
switch (skb->nh.iph->protocol) {
case IPPROTO_TCP:
- descr->dmac_cmd_status |= SPIDER_NET_DMAC_TCP;
+ hwdescr->dmac_cmd_status |= SPIDER_NET_DMAC_TCP;
break;
case IPPROTO_UDP:
- descr->dmac_cmd_status |= SPIDER_NET_DMAC_UDP;
+ hwdescr->dmac_cmd_status |= SPIDER_NET_DMAC_UDP;
break;
}
/* Chain the bus address, so that the DMA engine finds this descr. */
- descr->prev->next_descr_addr = descr->bus_addr;
+ wmb();
+ descr->prev->hwdescr->next_descr_addr = descr->bus_addr;
card->netdev->trans_start = jiffies; /* set netdev watchdog timer */
return 0;
@@ -689,16 +696,17 @@ spider_net_prepare_tx_descr(struct spide
static int
spider_net_set_low_watermark(struct spider_net_card *card)
{
+ struct spider_net_descr *descr = card->tx_chain.tail;
+ struct spider_net_hw_descr *hwdescr;
unsigned long flags;
int status;
int cnt=0;
int i;
- struct spider_net_descr *descr = card->tx_chain.tail;
/* Measure the length of the queue. Measurement does not
* need to be precise -- does not need a lock. */
while (descr != card->tx_chain.head) {
- status = descr->dmac_cmd_status & SPIDER_NET_DESCR_NOT_IN_USE;
+ status = descr->hwdescr->dmac_cmd_status & SPIDER_NET_DESCR_NOT_IN_USE;
if (status == SPIDER_NET_DESCR_NOT_IN_USE)
break;
descr = descr->next;
@@ -717,10 +725,12 @@ spider_net_set_low_watermark(struct spid
/* Set the new watermark, clear the old watermark */
spin_lock_irqsave(&card->tx_chain.lock, flags);
- descr->dmac_cmd_status |= SPIDER_NET_DESCR_TXDESFLG;
- if (card->low_watermark && card->low_watermark != descr)
- card->low_watermark->dmac_cmd_status =
- card->low_watermark->dmac_cmd_status & ~SPIDER_NET_DESCR_TXDESFLG;
+ descr->hwdescr->dmac_cmd_status |= SPIDER_NET_DESCR_TXDESFLG;
+ if (card->low_watermark && card->low_watermark != descr) {
+ hwdescr = card->low_watermark->hwdescr;
+ hwdescr->dmac_cmd_status =
+ hwdescr->dmac_cmd_status & ~SPIDER_NET_DESCR_TXDESFLG;
+ }
card->low_watermark = descr;
spin_unlock_irqrestore(&card->tx_chain.lock, flags);
return cnt;
@@ -743,6 +753,7 @@ spider_net_release_tx_chain(struct spide
{
struct spider_net_descr_chain *chain = &card->tx_chain;
struct spider_net_descr *descr;
+ struct spider_net_hw_descr *hwdescr;
struct sk_buff *skb;
u32 buf_addr;
unsigned long flags;
@@ -751,8 +762,9 @@ spider_net_release_tx_chain(struct spide
while (chain->tail != chain->head) {
spin_lock_irqsave(&chain->lock, flags);
descr = chain->tail;
+ hwdescr = descr->hwdescr;
- status = spider_net_get_descr_status(descr);
+ status = spider_net_get_descr_status(hwdescr);
switch (status) {
case SPIDER_NET_DESCR_COMPLETE:
card->netdev_stats.tx_packets++;
@@ -788,9 +800,9 @@ spider_net_release_tx_chain(struct spide
}
chain->tail = descr->next;
- descr->dmac_cmd_status |= SPIDER_NET_DESCR_NOT_IN_USE;
+ hwdescr->dmac_cmd_status |= SPIDER_NET_DESCR_NOT_IN_USE;
skb = descr->skb;
- buf_addr = descr->buf_addr;
+ buf_addr = hwdescr->buf_addr;
spin_unlock_irqrestore(&chain->lock, flags);
/* unmap the skb */
@@ -826,7 +838,7 @@ spider_net_kick_tx_dma(struct spider_net
descr = card->tx_chain.tail;
for (;;) {
- if (spider_net_get_descr_status(descr) ==
+ if (spider_net_get_descr_status(descr->hwdescr) ==
SPIDER_NET_DESCR_CARDOWNED) {
spider_net_write_reg(card, SPIDER_NET_GDTDCHA,
descr->bus_addr);
@@ -922,17 +934,18 @@ static void
spider_net_pass_skb_up(struct spider_net_descr *descr,
struct spider_net_card *card)
{
+ struct spider_net_hw_descr *hwdescr= descr->hwdescr;
struct sk_buff *skb;
struct net_device *netdev;
u32 data_status, data_error;
- data_status = descr->data_status;
- data_error = descr->data_error;
+ data_status = hwdescr->data_status;
+ data_error = hwdescr->data_error;
netdev = card->netdev;
skb = descr->skb;
skb->dev = netdev;
- skb_put(skb, descr->valid_size);
+ skb_put(skb, hwdescr->valid_size);
/* the card seems to add 2 bytes of junk in front
* of the ethernet frame */
@@ -1008,9 +1021,10 @@ spider_net_decode_one_descr(struct spide
{
struct spider_net_descr_chain *chain = &card->rx_chain;
struct spider_net_descr *descr = chain->tail;
+ struct spider_net_hw_descr *hwdescr = descr->hwdescr;
int status;
- status = spider_net_get_descr_status(descr);
+ status = spider_net_get_descr_status(hwdescr);
/* Nothing in the descriptor, or ring must be empty */
if ((status == SPIDER_NET_DESCR_CARDOWNED) ||
@@ -1021,7 +1035,7 @@ spider_net_decode_one_descr(struct spide
chain->tail = descr->next;
/* unmap descriptor */
- pci_unmap_single(card->pdev, descr->buf_addr,
+ pci_unmap_single(card->pdev, hwdescr->buf_addr,
SPIDER_NET_MAX_FRAME, PCI_DMA_FROMDEVICE);
if ( (status == SPIDER_NET_DESCR_RESPONSE_ERROR) ||
@@ -1044,27 +1058,26 @@ spider_net_decode_one_descr(struct spide
}
/* The cases we'll throw away the packet immediately */
- if (descr->data_error & SPIDER_NET_DESTROY_RX_FLAGS) {
+ if (hwdescr->data_error & SPIDER_NET_DESTROY_RX_FLAGS) {
if (netif_msg_rx_err(card))
pr_err("%s: error in received descriptor found, "
"data_status=x%08x, data_error=x%08x\n",
card->netdev->name,
- descr->data_status, descr->data_error);
+ hwdescr->data_status, hwdescr->data_error);
goto bad_desc;
}
- if (descr->dmac_cmd_status & 0xfefe) {
+ if (hwdescr->dmac_cmd_status & 0xfefe) {
pr_err("%s: bad status, cmd_status=x%08x\n",
card->netdev->name,
- descr->dmac_cmd_status);
- pr_err("buf_addr=x%08x\n", descr->buf_addr);
- pr_err("buf_size=x%08x\n", descr->buf_size);
- pr_err("next_descr_addr=x%08x\n", descr->next_descr_addr);
- pr_err("result_size=x%08x\n", descr->result_size);
- pr_err("valid_size=x%08x\n", descr->valid_size);
- pr_err("data_status=x%08x\n", descr->data_status);
- pr_err("data_error=x%08x\n", descr->data_error);
- pr_err("bus_addr=x%08x\n", descr->bus_addr);
+ hwdescr->dmac_cmd_status);
+ pr_err("buf_addr=x%08x\n", hwdescr->buf_addr);
+ pr_err("buf_size=x%08x\n", hwdescr->buf_size);
+ pr_err("next_descr_addr=x%08x\n", hwdescr->next_descr_addr);
+ pr_err("result_size=x%08x\n", hwdescr->result_size);
+ pr_err("valid_size=x%08x\n", hwdescr->valid_size);
+ pr_err("data_status=x%08x\n", hwdescr->data_status);
+ pr_err("data_error=x%08x\n", hwdescr->data_error);
pr_err("which=%ld\n", descr - card->rx_chain.ring);
card->spider_stats.rx_desc_error++;
@@ -1073,12 +1086,12 @@ spider_net_decode_one_descr(struct spide
/* Ok, we've got a packet in descr */
spider_net_pass_skb_up(descr, card);
- descr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
+ hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
return 1;
bad_desc:
dev_kfree_skb_irq(descr->skb);
- descr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
+ hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
return 0;
}
@@ -2045,9 +2058,6 @@ spider_net_setup_netdev(struct spider_ne
card->options.rx_csum = SPIDER_NET_RX_CSUM_DEFAULT;
- card->tx_chain.num_desc = tx_descriptors;
- card->rx_chain.num_desc = rx_descriptors;
-
spider_net_setup_netdev_ops(netdev);
netdev->features = NETIF_F_HW_CSUM | NETIF_F_LLTX;
@@ -2095,8 +2105,11 @@ spider_net_alloc_card(void)
{
struct net_device *netdev;
struct spider_net_card *card;
+ size_t alloc_size;
- netdev = alloc_etherdev(sizeof(struct spider_net_card));
+ alloc_size = sizeof(struct spider_net_card) +
+ (tx_descriptors + rx_descriptors) * sizeof(struct spider_net_descr);
+ netdev = alloc_etherdev(alloc_size);
if (!netdev)
return NULL;
@@ -2107,6 +2120,11 @@ spider_net_alloc_card(void)
init_waitqueue_head(&card->waitq);
atomic_set(&card->tx_timeout_task_counter, 0);
+ card->rx_chain.num_desc = rx_descriptors;
+ card->rx_chain.ring = card->darray;
+ card->tx_chain.num_desc = tx_descriptors;
+ card->tx_chain.ring = card->darray + rx_descriptors;
+
return card;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH: 3/5]: spidernet: fix racy double-free of skb
2007-02-10 0:01 [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
2007-02-10 0:14 ` [PATCH 1/5]: spidernet: compile break Linas Vepstas
2007-02-10 0:16 ` [PATCH 2/5] spidernet: separate hardware state from driver state Linas Vepstas
@ 2007-02-10 0:19 ` Linas Vepstas
2007-02-10 0:22 ` [PATCH 4/5]: spidernet: transmit race Linas Vepstas
2007-02-10 0:23 ` [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
4 siblings, 0 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-02-10 0:19 UTC (permalink / raw)
To: jgarzik; +Cc: linuxppc-dev, James K Lewis, Jens Osterkamp, netdev
It appears that under certain circumstances, a race will result
in a double-free of an skb. This patch null's out the skb pointer
upon the skb free, avoiding the inadvertent deref of bogus data.
The next patch fixes the actual race.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Jens Osterkamp <Jens.Osterkamp@de.ibm.com>
Cc: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
----
drivers/net/spider_net.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
Index: linux-2.6.20-git4/drivers/net/spider_net.c
===================================================================
--- linux-2.6.20-git4.orig/drivers/net/spider_net.c 2007-02-09 16:56:25.000000000 -0600
+++ linux-2.6.20-git4/drivers/net/spider_net.c 2007-02-09 16:56:30.000000000 -0600
@@ -360,10 +360,11 @@ spider_net_free_rx_chain_contents(struct
descr = card->rx_chain.head;
do {
if (descr->skb) {
- dev_kfree_skb(descr->skb);
pci_unmap_single(card->pdev, descr->hwdescr->buf_addr,
SPIDER_NET_MAX_FRAME,
PCI_DMA_BIDIRECTIONAL);
+ dev_kfree_skb(descr->skb);
+ descr->skb = NULL;
}
descr = descr->next;
} while (descr != card->rx_chain.head);
@@ -417,6 +418,7 @@ spider_net_prepare_rx_descr(struct spide
SPIDER_NET_MAX_FRAME, PCI_DMA_FROMDEVICE);
if (pci_dma_mapping_error(buf)) {
dev_kfree_skb_any(descr->skb);
+ descr->skb = NULL;
if (netif_msg_rx_err(card) && net_ratelimit())
pr_err("Could not iommu-map rx buffer\n");
card->spider_stats.rx_iommu_map_error++;
@@ -646,6 +648,7 @@ static int
spider_net_prepare_tx_descr(struct spider_net_card *card,
struct sk_buff *skb)
{
+ struct spider_net_descr_chain *chain = &card->tx_chain;
struct spider_net_descr *descr;
struct spider_net_hw_descr *hwdescr;
dma_addr_t buf;
@@ -660,10 +663,15 @@ spider_net_prepare_tx_descr(struct spide
return -ENOMEM;
}
- spin_lock_irqsave(&card->tx_chain.lock, flags);
+ spin_lock_irqsave(&chain->lock, flags);
descr = card->tx_chain.head;
+ if (descr->next == chain->tail->prev) {
+ spin_unlock_irqrestore(&chain->lock, flags);
+ pci_unmap_single(card->pdev, buf, skb->len, PCI_DMA_TODEVICE);
+ return -ENOMEM;
+ }
hwdescr = descr->hwdescr;
- card->tx_chain.head = descr->next;
+ chain->head = descr->next;
descr->skb = skb;
hwdescr->buf_addr = buf;
@@ -673,7 +681,7 @@ spider_net_prepare_tx_descr(struct spide
hwdescr->dmac_cmd_status =
SPIDER_NET_DESCR_CARDOWNED | SPIDER_NET_DMAC_NOCS;
- spin_unlock_irqrestore(&card->tx_chain.lock, flags);
+ spin_unlock_irqrestore(&chain->lock, flags);
if (skb->protocol == htons(ETH_P_IP))
switch (skb->nh.iph->protocol) {
@@ -802,6 +810,7 @@ spider_net_release_tx_chain(struct spide
chain->tail = descr->next;
hwdescr->dmac_cmd_status |= SPIDER_NET_DESCR_NOT_IN_USE;
skb = descr->skb;
+ descr->skb = NULL;
buf_addr = hwdescr->buf_addr;
spin_unlock_irqrestore(&chain->lock, flags);
@@ -867,13 +876,10 @@ spider_net_xmit(struct sk_buff *skb, str
{
int cnt;
struct spider_net_card *card = netdev_priv(netdev);
- struct spider_net_descr_chain *chain = &card->tx_chain;
spider_net_release_tx_chain(card, 0);
- if ((chain->head->next == chain->tail->prev) ||
- (spider_net_prepare_tx_descr(card, skb) != 0)) {
-
+ if (spider_net_prepare_tx_descr(card, skb) != 0) {
card->netdev_stats.tx_dropped++;
netif_stop_queue(netdev);
return NETDEV_TX_BUSY;
@@ -1091,6 +1097,7 @@ spider_net_decode_one_descr(struct spide
bad_desc:
dev_kfree_skb_irq(descr->skb);
+ descr->skb = NULL;
hwdescr->dmac_cmd_status = SPIDER_NET_DESCR_NOT_IN_USE;
return 0;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4/5]: spidernet: transmit race
2007-02-10 0:01 [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
` (2 preceding siblings ...)
2007-02-10 0:19 ` [PATCH: 3/5]: spidernet: fix racy double-free of skb Linas Vepstas
@ 2007-02-10 0:22 ` Linas Vepstas
2007-02-10 0:23 ` [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
4 siblings, 0 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-02-10 0:22 UTC (permalink / raw)
To: jgarzik; +Cc: linuxppc-dev, James K Lewis, Jens Osterkamp, netdev
Multiple threads performing a transmit can race into
the spidernet tx ring cleanup code. This puts the
relevant check under a lock.
Signed-off-by: Linas Vepstas <lins@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Jens Osterkamp <Jens.Osterkamp@de.ibm.com>
Cc: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
----
drivers/net/spider_net.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Index: linux-2.6.20-git4/drivers/net/spider_net.c
===================================================================
--- linux-2.6.20-git4.orig/drivers/net/spider_net.c 2007-02-09 17:30:13.000000000 -0600
+++ linux-2.6.20-git4/drivers/net/spider_net.c 2007-02-09 17:37:41.000000000 -0600
@@ -767,8 +767,12 @@ spider_net_release_tx_chain(struct spide
unsigned long flags;
int status;
- while (chain->tail != chain->head) {
+ while (1) {
spin_lock_irqsave(&chain->lock, flags);
+ if (chain->tail == chain->head) {
+ spin_unlock_irqrestore(&chain->lock, flags);
+ return 0;
+ }
descr = chain->tail;
hwdescr = descr->hwdescr;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/5]: spidernet: misc fixes
2007-02-10 0:01 [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
` (3 preceding siblings ...)
2007-02-10 0:22 ` [PATCH 4/5]: spidernet: transmit race Linas Vepstas
@ 2007-02-10 0:23 ` Linas Vepstas
2007-02-10 0:25 ` [PATCH 5/5]: spidernet janitorial: typos Linas Vepstas
4 siblings, 1 reply; 7+ messages in thread
From: Linas Vepstas @ 2007-02-10 0:23 UTC (permalink / raw)
To: jgarzik; +Cc: linuxppc-dev, James K Lewis, Jens Osterkamp, netdev
Janitorial patch. Undo long lines, fix typo in err msg.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Jens Osterkamp <Jens.Osterkamp@de.ibm.com>
Cc: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
----
drivers/net/spider_net.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
Index: linux-2.6.20-git4/drivers/net/spider_net.c
===================================================================
--- linux-2.6.20-git4.orig/drivers/net/spider_net.c 2007-02-09 17:37:41.000000000 -0600
+++ linux-2.6.20-git4/drivers/net/spider_net.c 2007-02-09 17:40:57.000000000 -0600
@@ -1017,14 +1017,15 @@ static void show_rx_chain(struct spider_
#endif
/**
- * spider_net_decode_one_descr - processes an rx descriptor
+ * spider_net_decode_one_descr - processes an RX descriptor
* @card: card structure
*
- * Returns 1 if a packet has been sent to the stack, otherwise 0
+ * Returns 1 if a packet has been sent to the stack, otherwise 0.
*
- * Processes an rx descriptor by iommu-unmapping the data buffer and passing
- * the packet up to the stack. This function is called in softirq
- * context, e.g. either bottom half from interrupt or NAPI polling context
+ * Processes an RX descriptor by iommu-unmapping the data buffer
+ * and passing the packet up to the stack. This function is called
+ * in softirq context, e.g. either bottom half from interrupt or
+ * NAPI polling context.
*/
static int
spider_net_decode_one_descr(struct spider_net_card *card)
@@ -1061,7 +1062,7 @@ spider_net_decode_one_descr(struct spide
if ( (status != SPIDER_NET_DESCR_COMPLETE) &&
(status != SPIDER_NET_DESCR_FRAME_END) ) {
if (netif_msg_rx_err(card))
- pr_err("%s: RX descriptor with unkown state %d\n",
+ pr_err("%s: RX descriptor with unknown state %d\n",
card->netdev->name, status);
card->spider_stats.rx_desc_unk_state++;
goto bad_desc;
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 5/5]: spidernet janitorial: typos
2007-02-10 0:23 ` [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
@ 2007-02-10 0:25 ` Linas Vepstas
0 siblings, 0 replies; 7+ messages in thread
From: Linas Vepstas @ 2007-02-10 0:25 UTC (permalink / raw)
To: jgarzik; +Cc: linuxppc-dev, James K Lewis, Jens Osterkamp, netdev
Resend, The subject line was wrong on the last one.
End-of-day tiredness.
--linas
Janitorial patch. Undo long lines, fix typo in err msg.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Jens Osterkamp <Jens.Osterkamp@de.ibm.com>
Cc: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
----
drivers/net/spider_net.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
Index: linux-2.6.20-git4/drivers/net/spider_net.c
===================================================================
--- linux-2.6.20-git4.orig/drivers/net/spider_net.c 2007-02-09 17:37:41.000000000 -0600
+++ linux-2.6.20-git4/drivers/net/spider_net.c 2007-02-09 17:40:57.000000000 -0600
@@ -1017,14 +1017,15 @@ static void show_rx_chain(struct spider_
#endif
/**
- * spider_net_decode_one_descr - processes an rx descriptor
+ * spider_net_decode_one_descr - processes an RX descriptor
* @card: card structure
*
- * Returns 1 if a packet has been sent to the stack, otherwise 0
+ * Returns 1 if a packet has been sent to the stack, otherwise 0.
*
- * Processes an rx descriptor by iommu-unmapping the data buffer and passing
- * the packet up to the stack. This function is called in softirq
- * context, e.g. either bottom half from interrupt or NAPI polling context
+ * Processes an RX descriptor by iommu-unmapping the data buffer
+ * and passing the packet up to the stack. This function is called
+ * in softirq context, e.g. either bottom half from interrupt or
+ * NAPI polling context.
*/
static int
spider_net_decode_one_descr(struct spider_net_card *card)
@@ -1061,7 +1062,7 @@ spider_net_decode_one_descr(struct spide
if ( (status != SPIDER_NET_DESCR_COMPLETE) &&
(status != SPIDER_NET_DESCR_FRAME_END) ) {
if (netif_msg_rx_err(card))
- pr_err("%s: RX descriptor with unkown state %d\n",
+ pr_err("%s: RX descriptor with unknown state %d\n",
card->netdev->name, status);
card->spider_stats.rx_desc_unk_state++;
goto bad_desc;
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-02-10 0:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-02-10 0:01 [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
2007-02-10 0:14 ` [PATCH 1/5]: spidernet: compile break Linas Vepstas
2007-02-10 0:16 ` [PATCH 2/5] spidernet: separate hardware state from driver state Linas Vepstas
2007-02-10 0:19 ` [PATCH: 3/5]: spidernet: fix racy double-free of skb Linas Vepstas
2007-02-10 0:22 ` [PATCH 4/5]: spidernet: transmit race Linas Vepstas
2007-02-10 0:23 ` [PATCH 0/5]: spidernet: misc fixes Linas Vepstas
2007-02-10 0:25 ` [PATCH 5/5]: spidernet janitorial: typos Linas Vepstas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).