| 1 | From 54789a03adf9c924d0cf7b890323c9c1ca7ab042 Mon Sep 17 00:00:00 2001 |
| 2 | From: Daniel Hellstrom <daniel@gaisler.com> |
| 3 | Date: Wed, 1 Dec 2010 10:26:09 +0100 |
| 4 | Subject: [PATCH] GRETH: fixed skb buffer memory leak on frame errors |
| 5 | |
| 6 | A new SKB buffer should not be allocated when the old SKB is reused. |
| 7 | |
| 8 | Signed-off-by: Daniel Hellstrom <daniel@gaisler.com> |
| 9 | --- |
| 10 | drivers/net/greth.c | 17 +++++++++++------ |
| 11 | 1 files changed, 11 insertions(+), 6 deletions(-) |
| 12 | |
| 13 | --- a/drivers/net/greth.c |
| 14 | +++ b/drivers/net/greth.c |
| 15 | @@ -879,10 +879,8 @@ static int greth_rx_gbit(struct net_devi |
| 16 | } |
| 17 | } |
| 18 | |
| 19 | - /* Allocate new skb to replace current */ |
| 20 | - newskb = netdev_alloc_skb(dev, MAX_FRAME_SIZE + NET_IP_ALIGN); |
| 21 | - |
| 22 | - if (!bad && newskb) { |
| 23 | + /* Allocate new skb to replace current, not needed if the current skb can be reused */ |
| 24 | + if (!bad && (newskb=netdev_alloc_skb(dev, MAX_FRAME_SIZE + NET_IP_ALIGN))) { |
| 25 | skb_reserve(newskb, NET_IP_ALIGN); |
| 26 | |
| 27 | dma_addr = dma_map_single(greth->dev, |
| 28 | @@ -919,12 +917,19 @@ static int greth_rx_gbit(struct net_devi |
| 29 | if (net_ratelimit()) |
| 30 | dev_warn(greth->dev, "Could not create DMA mapping, dropping packet\n"); |
| 31 | dev_kfree_skb(newskb); |
| 32 | - dev->stats.rx_dropped++; |
| 33 | + dev->stats.rx_dropped++; /* reusing current skb, so it is a drop */ |
| 34 | } |
| 35 | + } else if ( bad ) { |
| 36 | + /* Bad Frame transfer, the skb is reused */ |
| 37 | + dev->stats.rx_dropped++; |
| 38 | } else { |
| 39 | + /* Failed Allocating a new skb. This is rather stupid but the current "filled" |
| 40 | + * skb is reused, as if transfer failure. One could argue that RX descriptor table |
| 41 | + * handling should be divided into cleaning and filling as the TX part of the driver |
| 42 | + */ |
| 43 | if (net_ratelimit()) |
| 44 | dev_warn(greth->dev, "Could not allocate SKB, dropping packet\n"); |
| 45 | - dev->stats.rx_dropped++; |
| 46 | + dev->stats.rx_dropped++; /* reusing current skb, so it is a drop */ |
| 47 | } |
| 48 | |
| 49 | status = GRETH_BD_EN | GRETH_BD_IE; |
| 50 | |
