RE: [PATCH 1/1] AX88179_178A: Add FLAG_HW_IPALIGN to determine whether reserving NET_IP_ALIGN bytes for an SKB.

[David Laight]

> From: Freddy Xin
> On 2013å12æ13æ 18:36, David Laight wrote:
> >> From: freddy@xxxxxxxxxxx
> > ...
> >> -	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

¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_