As suggested by Stephen Hemminger.
Signed-off-by: Thomas Chou <[email protected]>
---
drivers/net/ethoc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index ecc53d9..4a1ed81 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -409,7 +409,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
struct sk_buff *skb = netdev_alloc_skb(dev, size);
size -= 4; /* strip the CRC */
- skb_reserve(skb, 2); /* align TCP/IP header */
+ skb_reserve(skb, NET_IP_ALIGN);
if (likely(skb)) {
void *src = phys_to_virt(bd.addr);
--
1.6.2.5
Thomas Chou a ?crit :
> As suggested by Stephen Hemminger.
>
> Signed-off-by: Thomas Chou <[email protected]>
> ---
> drivers/net/ethoc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
> index ecc53d9..4a1ed81 100644
> --- a/drivers/net/ethoc.c
> +++ b/drivers/net/ethoc.c
> @@ -409,7 +409,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
> struct sk_buff *skb = netdev_alloc_skb(dev, size);
>
> size -= 4; /* strip the CRC */
> - skb_reserve(skb, 2); /* align TCP/IP header */
> + skb_reserve(skb, NET_IP_ALIGN);
>
> if (likely(skb)) {
> void *src = phys_to_virt(bd.addr);
Sorry to be dense here, but this code breaks if NET_IP_ALIGN > 4.
Its also suboptimal, you alloc two bytes in excess.
You should do :
size -= 4; /* strip the CRC */
skb = netdev_alloc_skb(dev, size + NET_IP_ALIGN);
if (skb) {
skb_reserve(skb, NET_IP_ALIGN);
...
}
Please check other implementations...
David, maybe we should add following helper :
[PATCH] net: Add netdev_alloc_skb_ip_align() helper
Instead of hardcoding NET_IP_ALIGN stuff in various network drivers, we can
add a helper around netdev_alloc_skb()
Signed-off-by: Eric Dumazet <[email protected]>
---
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index df7b23a..fed788e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1489,6 +1489,16 @@ static inline struct sk_buff *netdev_alloc_skb(struct net_device *dev,
return __netdev_alloc_skb(dev, length, GFP_ATOMIC);
}
+static inline struct sk_buff *netdev_alloc_skb_ip_align(struct net_device *dev,
+ unsigned int length)
+{
+ struct sk_buff *skb = netdev_alloc_skb(dev, length + NET_IP_ALIGN);
+
+ if (NET_IP_ALIGN && skb)
+ skb_reserve(skb, NET_IP_ALIGN);
+ return skb;
+}
+
extern struct page *__netdev_alloc_page(struct net_device *dev, gfp_t gfp_mask);
/**
On 10/08/2009 11:11 AM, Eric Dumazet wrote:
> Thomas Chou a ?crit :
> You should do :
>
> size -= 4; /* strip the CRC */
> skb = netdev_alloc_skb(dev, size + NET_IP_ALIGN);
> if (skb) {
> skb_reserve(skb, NET_IP_ALIGN);
> ...
> }
>
>
> Please check other implementations...
>
> David, maybe we should add following helper :
>
> [PATCH] net: Add netdev_alloc_skb_ip_align() helper
Hi Eric,
Thanks for the suggestion. I will follow your commit and send revised patch.
- Thomas
From: Eric Dumazet <[email protected]>
Date: Thu, 08 Oct 2009 05:11:23 +0200
> David, maybe we should add following helper :
>
> [PATCH] net: Add netdev_alloc_skb_ip_align() helper
>
> Instead of hardcoding NET_IP_ALIGN stuff in various network drivers, we can
> add a helper around netdev_alloc_skb()
>
> Signed-off-by: Eric Dumazet <[email protected]>
Looks ok, but I want to look at how often this exact sequence
would match. If it applies to a lot of cases, I'll add this
but I know of many exceptions in my head already :-)
David Miller a ?crit :
> Looks ok, but I want to look at how often this exact sequence
> would match. If it applies to a lot of cases, I'll add this
> but I know of many exceptions in my head already :-)
Well, it was more as a reference. I believe about 20-30 call sites
could use it. Do you want me to provide a combo patch ?
From: Eric Dumazet <[email protected]>
Date: Fri, 9 Oct 2009 18:31:30 +0200
> David Miller a ?crit :
>
>> Looks ok, but I want to look at how often this exact sequence
>> would match. If it applies to a lot of cases, I'll add this
>> but I know of many exceptions in my head already :-)
>
> Well, it was more as a reference. I believe about 20-30 call sites
> could use it. Do you want me to provide a combo patch ?
No, that's not necessary.
From: Eric Dumazet <[email protected]>
Date: Thu, 08 Oct 2009 05:11:23 +0200
> [PATCH] net: Add netdev_alloc_skb_ip_align() helper
>
> Instead of hardcoding NET_IP_ALIGN stuff in various network drivers, we can
> add a helper around netdev_alloc_skb()
>
> Signed-off-by: Eric Dumazet <[email protected]>
Applied to net-next-2.6