* [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).