Re: [PATCH] net: Fix GRO for multiple page fragments

[David Miller <davem@davemloft.net>]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 16 Apr 2009 19:04:20 +0100

> This loop over fragments in napi_fraginfo_skb() was "interesting".
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> This is not tested, as only cxgb3 will currently pass in multiple
> fragments at the same time.  skb_shinfo(skb)->nr_frags would already be
> 0 but it makes no sense to rely on that.  I hope I'm not missing some
> subtlety...

I think the code still isn't right after your changes.

The intent seems to be to append frags from 'info' into the SKB,
so that it works even if the SKB already has some frags.

And being that cxgb3 is pretty well tested with GRO I'm suspicious
even moreso :-)

Herbert?

> diff --git a/net/core/dev.c b/net/core/dev.c
> index ea8eb22..15ecc51 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2539,9 +2539,9 @@ struct sk_buff *napi_fraginfo_skb(struct napi_struct *napi,
>  	}
>  
>  	BUG_ON(info->nr_frags > MAX_SKB_FRAGS);
> -	frag = &info->frags[info->nr_frags - 1];
> +	frag = info->frags;
>  
> -	for (i = skb_shinfo(skb)->nr_frags; i < info->nr_frags; i++) {
> +	for (i = 0; i < info->nr_frags; i++) {
>  		skb_fill_page_desc(skb, i, frag->page, frag->page_offset,
>  				   frag->size);
>  		frag++;
> 
> -- 
> Ben Hutchings, Senior Software Engineer, Solarflare Communications
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>