[PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver

[John Linn]

> -----Original Message-----
> From: Steven J. Magnani [mailto:steve at digidescorp.com]
> Sent: Tuesday, April 06, 2010 2:04 PM
> To: grant.likely at secretlab.ca
> Cc: John Linn; Eric Dumazet; netdev at vger.kernel.org;
linuxppc-dev at ozlabs.org;
> jwboyer at linux.vnet.ibm.com; john.williams at petalogix.com;
michal.simek at petalogix.com; John Tyner
> Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
> 
> On Tue, 2010-04-06 at 12:53 -0600, Grant Likely wrote:
> 
> > Hold on.... BUFFER_ALIGN is being used to align the DMA buffer on a
> > cache line boundary.  I don't think netdev_alloc_skb() makes any
> > guarantees about how the start of the IP header lines up against
cache
> > line boundaries.  The amount of padding needed is not known until an
> > skbuff is obtained from netdev_alloc_skb(), and
> > netdev_alloc_skb_ip_align() can only handle a fixed size padding,
> >
> > It doesn't look like netdev_alloc_skb_ip_align() is the right thing
in
> > this regard.
> 
> __netdev_alloc_skb reserves NET_SKB_PAD bytes which gets us cacheline
> alignment on Microblaze. From include/linux/skbuff.h:
> 

Good find. I'll give it a test on MicroBlaze and PowerPC.

> /*
>  * The networking layer reserves some headroom in skb data (via
>  * dev_alloc_skb). This is used to avoid having to reallocate skb data
> when
>  * the header has to grow. In the default case, if the header has to
> grow
>  * 32 bytes or less we avoid the reallocation.
>  *
>  * Unfortunately this headroom changes the DMA alignment of the
> resulting
>  * network packet. As for NET_IP_ALIGN, this unaligned DMA is
expensive
>  * on some architectures. An architecture can override this value,
>  * perhaps setting it to a cacheline in size (since that will maintain
>  * cacheline alignment of the DMA). It must be a power of 2.
>  *
>  * Various parts of the networking layer expect at least 32 bytes of
>  * headroom, you should not reduce this.
>  */
> #ifndef NET_SKB_PAD
> #define NET_SKB_PAD     32
> #endif
> 
> If this doesn't work for some of the PPC variants with larger cache
> lines, maybe one of the PPC header files needs to define NET_SKB_PAD?

Looks like it is defined in system.h in powerpc so that it works.

> And if we want to guard against possible future changes to the default
> NET_SKB_PAD breaking Microblaze operation, maybe one of its headers
> should define NET_SKB_PAD as well?

Good idea, we can add that to system.h for MicroBlaze also.

Thanks,
John

> 
>
------------------------------------------------------------------------
>  Steven J. Magnani               "I claim this network for MARS!
>  www.digidescorp.com              Earthling, return my space
modulator!"
> 
>  #include <standard.disclaimer>
> 
> 
> 


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.