netdev - RE: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP

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]

Message-ID: <AE90C24D6B3A694183C094C60CF0A2F6026B747B@saturn3.aculab.com>
Date:	Wed, 11 Dec 2013 10:33:18 -0000
From:	"David Laight" <David.Laight@...LAB.COM>
To:	"Freddy Xin" <freddy@...x.com.tw>,
	"David Miller" <davem@...emloft.net>
Cc:	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<netdev@...r.kernel.org>, <louis@...x.com.tw>, <allan@...x.com.tw>
Subject: RE: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0

> From: Freddy Xin
> On 2013年12月10日 09:01, David Miller wrote:
> > From: freddy@...x.com.tw
> > Date: Fri,  6 Dec 2013 17:58:18 +0800
> >
> >> From: Freddy Xin <freddy@...x.com.tw>
> >>
> >> The AX88179_178A has a hardware feature that it can insert a 2-bytes pseudo
> >> header in front of each received frame by setting the AX_RX_CTL_IPE bit.
> >> This feature is used to let the IP header be aligned on a doubleword-aligned address,
> >> but the NET_IP_ALIGN may equals to 2 and the __netdev_alloc_skb_ip_align in USBNET will
> >> reserve 2 bytes also, so in this case the driver shouldn't enable this bit.
> >>
> >> This patch modifies the driver to set AX_RX_CTL_IPE just in case of the NET_IP_ALIGN equals 0.
> >>
> >> Signed-off-by: Freddy Xin <freddy@...x.com.tw>
> > Please avoid larger than 80 column lines in your commit messages,
> > people use text-only tools to viee these.
> >
> > Next, it makes no sense to restrict your change to NET_IP_ALIGN==0
> >
> > Simply handle any case, by undoing the reservation if it's getting
> > in the way.  If there isn't an appropriate helper for this, add one.
> >
> I think there is no way of undoing the reservation in the driver.
> Can I add a flag of the driver_info, and use it to determine
> whether undoing the reservation in rx_submit of usbnet?

You probably want to arrange to have save the appropriate offset in
one of the structures - so avoiding any conditionals during the allocate.

I've noticed that the RX URB are 20k bytes long.
Allocating that much physically and virtually contiguous memory for
every rx frame seems sub-optimal to say the least!
What does the skb's 'realsize' end up as?
I don't remember seeing a copy for short frames either.

ISTR there is code to debatch multiple ethernet frames from a single
URB. I think that means that an ethernet frame might end up in two URB.
I'm not sure there is code to handle that either - but I've not looked
that closely at that part of the code.

	David