From 819ddcafb33136f2ba18018a22edcf857f640528 Mon Sep 17 00:00:00 2001 From: Lennert Buytenhek Date: Sun, 24 Aug 2008 02:45:32 +0200 Subject: mv643xx_eth: fix NAPI 'rotting packet' issue When a receive interrupt occurs, mv643xx_eth would first process the receive descriptors and then ACK the receive interrupt, instead of the other way round. This would leave a small race window between processing the last receive descriptor and clearing the receive interrupt status in which a new packet could come in, which would then 'rot' in the receive ring until the next receive interrupt would come in. Fix this by ACKing (clearing) the receive interrupt condition before processing the receive descriptors. Signed-off-by: Lennert Buytenhek --- drivers/net/mv643xx_eth.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c index 46819af3b062..af279f26c407 100644 --- a/drivers/net/mv643xx_eth.c +++ b/drivers/net/mv643xx_eth.c @@ -650,8 +650,6 @@ static int mv643xx_eth_poll(struct napi_struct *napi, int budget) if (rx < budget) { netif_rx_complete(mp->dev, napi); - wrl(mp, INT_CAUSE(mp->port_num), 0); - wrl(mp, INT_CAUSE_EXT(mp->port_num), 0); wrl(mp, INT_MASK(mp->port_num), INT_TX_END | INT_RX | INT_EXT); } @@ -1796,6 +1794,7 @@ static irqreturn_t mv643xx_eth_irq(int irq, void *dev_id) */ #ifdef MV643XX_ETH_NAPI if (int_cause & INT_RX) { + wrl(mp, INT_CAUSE(mp->port_num), ~(int_cause & INT_RX)); wrl(mp, INT_MASK(mp->port_num), 0x00000000); rdl(mp, INT_MASK(mp->port_num)); -- cgit v1.2.3 From 92c70f27d2a78873c940e77c7f075cd8e2e60a2d Mon Sep 17 00:00:00 2001 From: Lennert Buytenhek Date: Sun, 24 Aug 2008 01:59:05 +0200 Subject: mv643xx_eth: fix double add_timer() on the receive oom timer Commit 12e4ab79cd828563dc090d2117dc8626b344bc8f ("mv643xx_eth: be more agressive about RX refill") changed the condition for the receive out-of-memory timer to be scheduled from "the receive ring is empty" to "the receive ring is not full". This can lead to a situation where the receive out-of-memory timer is pending because a previous rxq_refill() didn't manage to refill the receive ring entirely as a result of being out of memory, and rxq_refill() is then called again as a side effect of a packet receive interrupt, and that rxq_refill() call then again does not succeed to refill the entire receive ring with fresh empty skbuffs because we are still out of memory, and then tries to call add_timer() on the already scheduled out-of-memory timer. This patch fixes this issue by changing the add_timer() call in rxq_refill() to a mod_timer() call. If the OOM timer was not already scheduled, this will behave as before, whereas if it was already scheduled, this patch will push back its firing time a bit, which is safe because we've (unsuccessfully) attempted to refill the receive ring just before we do this. Signed-off-by: Lennert Buytenhek --- drivers/net/mv643xx_eth.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c index af279f26c407..8a91d79383dd 100644 --- a/drivers/net/mv643xx_eth.c +++ b/drivers/net/mv643xx_eth.c @@ -509,10 +509,8 @@ static void rxq_refill(struct rx_queue *rxq) skb_reserve(skb, 2); } - if (rxq->rx_desc_count != rxq->rx_ring_size) { - rxq->rx_oom.expires = jiffies + (HZ / 10); - add_timer(&rxq->rx_oom); - } + if (rxq->rx_desc_count != rxq->rx_ring_size) + mod_timer(&rxq->rx_oom, jiffies + (HZ / 10)); spin_unlock_irqrestore(&mp->lock, flags); } -- cgit v1.2.3 From 8e0b1bf6ac6c4c6dd985e586cd765aede4678bba Mon Sep 17 00:00:00 2001 From: Lennert Buytenhek Date: Sun, 24 Aug 2008 02:30:42 +0200 Subject: mv643xx_eth: fix inconsistent lock semantics Nicolas Pitre noted that mv643xx_eth_poll was incorrectly using non-IRQ-safe locks while checking whether to wake up the netdevice's transmit queue. Convert the locking to *_irq() variants, since we are running from softirq context where interrupts are enabled. Signed-off-by: Lennert Buytenhek --- drivers/net/mv643xx_eth.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c index 8a91d79383dd..30e6d4b8d564 100644 --- a/drivers/net/mv643xx_eth.c +++ b/drivers/net/mv643xx_eth.c @@ -634,9 +634,9 @@ static int mv643xx_eth_poll(struct napi_struct *napi, int budget) txq_reclaim(mp->txq + i, 0); if (netif_carrier_ok(mp->dev)) { - spin_lock(&mp->lock); + spin_lock_irq(&mp->lock); __txq_maybe_wake(mp->txq + mp->txq_primary); - spin_unlock(&mp->lock); + spin_unlock_irq(&mp->lock); } } #endif -- cgit v1.2.3 From 9e1f37724265725ad4c14fc2ef60a162dc13ac64 Mon Sep 17 00:00:00 2001 From: Lennert Buytenhek Date: Sun, 24 Aug 2008 02:33:47 +0200 Subject: mv643xx_eth: fix NULL pointer dereference in rxq_process() When we are low on memory, the assumption that every descriptor in the receive ring will have an skbuff associated with it does not hold. rxq_process() was assuming that if the receive descriptor it is working on is not owned by the hardware, it can safely be processed and handed to the networking stack. But a descriptor in the receive ring not being owned by the hardware can also happen when we are low on memory and did not manage to refill the receive ring fully. This patch changes rxq_process()'s bailout condition from "the first receive descriptor to be processed is owned by the hardware" to "the first receive descriptor to be processed is owned by the hardware OR the number of valid receive descriptors in the ring is zero". Signed-off-by: Lennert Buytenhek --- drivers/net/mv643xx_eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c index 30e6d4b8d564..e33dfc0165f6 100644 --- a/drivers/net/mv643xx_eth.c +++ b/drivers/net/mv643xx_eth.c @@ -527,7 +527,7 @@ static int rxq_process(struct rx_queue *rxq, int budget) int rx; rx = 0; - while (rx < budget) { + while (rx < budget && rxq->rx_desc_count) { struct rx_desc *rx_desc; unsigned int cmd_sts; struct sk_buff *skb; -- cgit v1.2.3 From abe787170bb3888c5e587e8e986711fe32ddf2f9 Mon Sep 17 00:00:00 2001 From: Lennert Buytenhek Date: Sun, 24 Aug 2008 03:00:20 +0200 Subject: mv643xx_eth: enforce multiple-of-8-bytes receive buffer size restriction The mv643xx_eth hardware ignores the lower three bits of the buffer size field in receive descriptors, causing the reception of full-sized packets to fail at some MTUs. Fix this by rounding the size of allocated receive buffers up to a multiple of eight bytes. While we are at it, add a bit of extra space to each receive buffer so that we can handle multiple vlan tags on ingress. Signed-off-by: Lennert Buytenhek --- drivers/net/mv643xx_eth.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c index e33dfc0165f6..a02a5f4a0294 100644 --- a/drivers/net/mv643xx_eth.c +++ b/drivers/net/mv643xx_eth.c @@ -474,11 +474,19 @@ static void rxq_refill(struct rx_queue *rxq) /* * Reserve 2+14 bytes for an ethernet header (the * hardware automatically prepends 2 bytes of dummy - * data to each received packet), 4 bytes for a VLAN - * header, and 4 bytes for the trailing FCS -- 24 - * bytes total. + * data to each received packet), 16 bytes for up to + * four VLAN tags, and 4 bytes for the trailing FCS + * -- 36 bytes total. */ - skb_size = mp->dev->mtu + 24; + skb_size = mp->dev->mtu + 36; + + /* + * Make sure that the skb size is a multiple of 8 + * bytes, as the lower three bits of the receive + * descriptor's buffer size field are ignored by + * the hardware. + */ + skb_size = (skb_size + 7) & ~7; skb = dev_alloc_skb(skb_size + dma_get_cache_alignment() - 1); if (skb == NULL) @@ -552,7 +560,7 @@ static int rxq_process(struct rx_queue *rxq, int budget) spin_unlock_irqrestore(&mp->lock, flags); dma_unmap_single(NULL, rx_desc->buf_ptr + 2, - mp->dev->mtu + 24, DMA_FROM_DEVICE); + rx_desc->buf_size, DMA_FROM_DEVICE); rxq->rx_desc_count--; rx++; -- cgit v1.2.3 From c4560318cf8fed91e3146a3c1247e11542d91d91 Mon Sep 17 00:00:00 2001 From: Lennert Buytenhek Date: Sun, 24 Aug 2008 07:33:05 +0200 Subject: mv643xx_eth: bump version to 1.3 Signed-off-by: Lennert Buytenhek --- drivers/net/mv643xx_eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c index a02a5f4a0294..0a18b9e96da1 100644 --- a/drivers/net/mv643xx_eth.c +++ b/drivers/net/mv643xx_eth.c @@ -55,7 +55,7 @@ #include static char mv643xx_eth_driver_name[] = "mv643xx_eth"; -static char mv643xx_eth_driver_version[] = "1.2"; +static char mv643xx_eth_driver_version[] = "1.3"; #define MV643XX_ETH_CHECKSUM_OFFLOAD_TX #define MV643XX_ETH_NAPI -- cgit v1.2.3