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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 6 Apr 2010 14:32:08 -0600
From:	John Linn <John.Linn@...inx.com>
To:	"Eric Dumazet" <eric.dumazet@...il.com>,
	<grant.likely@...retlab.ca>
CC:	<netdev@...r.kernel.org>, <linuxppc-dev@...abs.org>,
	<jwboyer@...ux.vnet.ibm.com>, <john.williams@...alogix.com>,
	<michal.simek@...alogix.com>, "John Tyner" <jtyner@...ucr.edu>
Subject: RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@...il.com]
> Sent: Tuesday, April 06, 2010 2:24 PM
> To: grant.likely@...retlab.ca
> Cc: John Linn; netdev@...r.kernel.org; linuxppc-dev@...abs.org; jwboyer@...ux.vnet.ibm.com;
> john.williams@...alogix.com; michal.simek@...alogix.com; John Tyner
> Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
> 
> Le mardi 06 avril 2010 à 12:53 -0600, Grant Likely a écrit :
> 
> > 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.
> >
> 
> OK, time to have a long explanation :
> 
> netdev_alloc_skb_ip_align() is doing the right thing, but it seems you
> guys insist to invent a new private stuff.
> 
> I am only wondering if you really know why you do this.
> 
> Many drivers do have same requirements, so every driver should reinvent
> the wheel ? Really this is beyond me.
> 
> Original code was aligning the buffer on a 32 bytes boundary (because of
> a hardware requirement on NIC, I only trust original code on this).
> 
> Then you want to change this to align buffer on this 32 bytes boundary
> PLUS 2. What is this kind of new requirement ?
> 
> 1) Hardware requirement really changed that much. (firmware changed on
> NIC). If not using this new alignement, NIC doesnt work at all.
> 
> 2) or Microblaze arch requires that IP header is aligned on a word
> boundary to avoid unaligned traps in IP stack ? (like many arches)

Yes.

> 
> If this is the latest requirement, then use standard mechanism.
> skb data is naturally aligned on L1_CACHE_SIZE + SKB_PAD boundaries.
> (32 bytes alignment)
> 
> netdev_alloc_skb_ip_align()() then skips 2 bytes to make skb data
> aligned so that 2 + 14 (sizeof eth header) = 16 : IP header is aligned
> (modulo 16)
> 
> It just works. If not, we should correct it, please fill a bug report.
> 

Trying it now, thanks for the help and patience :)

-- John

> Thanks
> 
> 


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.