* [PATCH] ethoc: use NET_IP_ALIGN in skb_reserve()
@ 2009-10-08 2:32 Thomas Chou
2009-10-08 3:11 ` [PATCH] net: Add netdev_alloc_skb_ip_align() helper Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Chou @ 2009-10-08 2:32 UTC (permalink / raw)
To: netdev; +Cc: thierry.reding, Nios2 development list, linux-kernel, Thomas Chou
As suggested by Stephen Hemminger.
Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] net: Add netdev_alloc_skb_ip_align() helper
2009-10-08 2:32 [PATCH] ethoc: use NET_IP_ALIGN in skb_reserve() Thomas Chou
@ 2009-10-08 3:11 ` Eric Dumazet
2009-10-08 4:36 ` Thomas Chou
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Eric Dumazet @ 2009-10-08 3:11 UTC (permalink / raw)
To: Thomas Chou
Cc: netdev, thierry.reding, Nios2 development list, linux-kernel,
David S. Miller
Thomas Chou a écrit :
> As suggested by Stephen Hemminger.
>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
> 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 <eric.dumazet@gmail.com>
---
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);
/**
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] net: Add netdev_alloc_skb_ip_align() helper
2009-10-08 3:11 ` [PATCH] net: Add netdev_alloc_skb_ip_align() helper Eric Dumazet
@ 2009-10-08 4:36 ` Thomas Chou
2009-10-08 5:40 ` David Miller
2009-10-13 10:45 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: Thomas Chou @ 2009-10-08 4:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, thierry.reding, Nios2 development list, linux-kernel,
David S. Miller
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: Add netdev_alloc_skb_ip_align() helper
2009-10-08 3:11 ` [PATCH] net: Add netdev_alloc_skb_ip_align() helper Eric Dumazet
2009-10-08 4:36 ` Thomas Chou
@ 2009-10-08 5:40 ` David Miller
2009-10-09 16:31 ` Eric Dumazet
2009-10-13 10:45 ` David Miller
2 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2009-10-08 5:40 UTC (permalink / raw)
To: eric.dumazet; +Cc: thomas, netdev, thierry.reding, nios2-dev, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
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 <eric.dumazet@gmail.com>
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 :-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: Add netdev_alloc_skb_ip_align() helper
2009-10-08 5:40 ` David Miller
@ 2009-10-09 16:31 ` Eric Dumazet
2009-10-10 3:43 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2009-10-09 16:31 UTC (permalink / raw)
To: David Miller; +Cc: thomas, netdev, thierry.reding, nios2-dev, linux-kernel
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 ?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: Add netdev_alloc_skb_ip_align() helper
2009-10-09 16:31 ` Eric Dumazet
@ 2009-10-10 3:43 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2009-10-10 3:43 UTC (permalink / raw)
To: eric.dumazet; +Cc: thomas, netdev, thierry.reding, nios2-dev, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: Add netdev_alloc_skb_ip_align() helper
2009-10-08 3:11 ` [PATCH] net: Add netdev_alloc_skb_ip_align() helper Eric Dumazet
2009-10-08 4:36 ` Thomas Chou
2009-10-08 5:40 ` David Miller
@ 2009-10-13 10:45 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2009-10-13 10:45 UTC (permalink / raw)
To: eric.dumazet; +Cc: thomas, netdev, thierry.reding, nios2-dev, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
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 <eric.dumazet@gmail.com>
Applied to net-next-2.6
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-10-13 10:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-08 2:32 [PATCH] ethoc: use NET_IP_ALIGN in skb_reserve() Thomas Chou
2009-10-08 3:11 ` [PATCH] net: Add netdev_alloc_skb_ip_align() helper Eric Dumazet
2009-10-08 4:36 ` Thomas Chou
2009-10-08 5:40 ` David Miller
2009-10-09 16:31 ` Eric Dumazet
2009-10-10 3:43 ` David Miller
2009-10-13 10:45 ` David Miller
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).