netdev - RE: [PATCH 1/1] AX88179_178A: Add FLAG_HW_IPALIGN to determine whether reserving 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: <AE90C24D6B3A694183C094C60CF0A2F6026B7490@saturn3.aculab.com>
Date:	Mon, 16 Dec 2013 10:09:40 -0000
From:	"David Laight" <David.Laight@...LAB.COM>
To:	"Freddy Xin" <freddy@...x.com.tw>, <netdev@...r.kernel.org>,
	<davem@...emloft.net>, <linux-usb@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <louis@...x.com.tw>,
	<allan@...x.com.tw>
Subject: RE: [PATCH 1/1] AX88179_178A: Add FLAG_HW_IPALIGN to determine whether reserving NET_IP_ALIGN bytes for an SKB.

> From: Freddy Xin
> On 2013年12月13日 18:36, David Laight wrote:
> >> From: freddy@...x.com.tw
> > ...
> >> -	skb = __netdev_alloc_skb_ip_align(dev->net, size, flags);
> >> +	if (dev->driver_info->flags & FLAG_HW_IPALIGN)
> >> +		skb = __netdev_alloc_skb(dev->net, size, flags);
> >> +	else
> >> +		skb = __netdev_alloc_skb_ip_align(dev->net, size, flags);
> > Given the definition:
> > static inline struct sk_buff *__netdev_alloc_skb_ip_align(struct net_device *dev,
> >                  unsigned int length, gfp_t gfp)
> > {
> >          struct sk_buff *skb = __netdev_alloc_skb(dev, length + NET_IP_ALIGN, gfp);
> >
> >          if (NET_IP_ALIGN && skb)
> >                  skb_reserve(skb, NET_IP_ALIGN);
> >          return skb;
> > }
> >
> > It really ought to be possible to code that without an extra conditional.
> >
> > 	David
>
> The AX88179_178A driver do need to know the value of NET_IP_ALIGN
> to determine whether enabling the feature that makes IP header align
> on a dword-aligned address, but according to the comments from David
> Miller, I need to consider all situations, not just for the case that
> NET_IP_ALIGN is zero, so the condition added in rx_submit is just used to
> determine whether reserving NET_IP_ALIGN bytes for each SKB.

I was thinking of something like:

	skb = netdev_alloc_skb(dev, length + dev->skb_align, gfp);
	if (NET_IP_ALIGN && skb && !(ev->driver_flags & FLAG_HW_IPALIGN))
		skb_reserve(skb, NET_IP_ALIGN);

It might even be reasonable to remove the length adjustment - provided
that all the later code uses the skb length.

	David