Subject: Re: net: alignment problem in icmp code

From: Eric Dumazet <[email-masked]>
Date: Mon, 22 Oct 2007 11:52:12 +0200

Pierre Ossman a écrit :
> On Mon, 22 Oct 2007 02:05:38 -0700 (PDT)
> David Miller <[email-masked]> wrote:
>
>   
>> From: Pierre Ossman <[email-masked]>
>> Date: Mon, 22 Oct 2007 10:42:08 +0200
>>
>>     
>>> This seems like a rather evil layering violation.
>>>       
>> This has a 10+ year precedence and it's why the Linux networking stack
>> is so fast.  If you read any other driver you would have seen the
>> skb_reserve() call every one of them do to align the headers.
>>
>>     
>
> The norm seems to be to not comment this call. It's hardly obvious.
>
>   
This is not obvious you are right, but documented in 
include/linux/skbuff.h (lines 1150 ...)

/*
 * CPUs often take a performance hit when accessing unaligned memory
 * locations. The actual performance hit varies, it can be small if the
 * hardware handles it or large if we have to take an exception and fix it
 * in software.
 *
 * Since an ethernet header is 14 bytes network drivers often end up with
 * the IP header at an unaligned offset. The IP header can be aligned by
 * shifting the start of the packet by 2 bytes. Drivers should do this
 * with:
 *
 * skb_reserve(NET_IP_ALIGN);
 *
 * The downside to this alignment of the IP header is that the DMA is now
 * unaligned. On some architectures the cost of an unaligned DMA is high
 * and this cost outweighs the gains made by aligning the IP header.
 *
 * Since this trade off varies between architectures, we allow NET_IP_ALIGN
 * to be overridden.
 */
#ifndef NET_IP_ALIGN
#define NET_IP_ALIGN    2
#endif

But then, many drivers dont use NET_IP_ALIGN but a hardcoded 2

drivers/net/pcnet32.c:608:              skb_reserve(rx_skbuff, 2);
drivers/net/pcnet32.c:1214:                     skb_reserve(newskb, 2);
drivers/net/pcnet32.c:1245:             skb_reserve(skb, 2);    /* 16 
byte align */
drivers/net/pcnet32.c:2396:                     skb_reserve(rx_skbuff, 2);
drivers/net/sundance.c:989:             skb_reserve(skb, 2);    /* 16 
byte align the IP header. */
drivers/net/sundance.c:1312:                            skb_reserve(skb, 
2);    /* 16 byte align the IP header */
drivers/net/sundance.c:1375:                    skb_reserve(skb, 2); /* 
Align IP on 16 byte boundaries */