gro: Fix handling of imprecisely split packets

gro: Fix handling of imprecisely split packets

[Herbert Xu]

Hi Jeff:

Please let me know if the ixgbe problems persist with this patch
against net-next.

gro: Fix handling of imprecisely split packets

The commit 89a1b249edcf9be884e71f92df84d48355c576aa (gro: Avoid
copying headers of unmerged packets) only worked for packets
which are either completely linear, completely non-linear, or
packets which exactly split at the boundary between headers and
payload.

Anything else would cause bits in the header to go missing if
the packet is held by GRO.

This may have broken drivers such as ixgbe.

This patch fixes the places that assumed or only worked with
the above cases.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/core/dev.c b/net/core/dev.c
index dd56c32..089c7c9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -217,7 +217,7 @@ static inline struct hlist_head *dev_index_hash(struct net *net, int ifindex)
 
 static inline void *skb_gro_mac_header(struct sk_buff *skb)
 {
-	return skb_headlen(skb) ? skb_mac_header(skb) :
+	return skb_mac_header(skb) < skb->data ? skb_mac_header(skb) :
 	       page_address(skb_shinfo(skb)->frags[0].page) +
 	       skb_shinfo(skb)->frags[0].page_offset;
 }
@@ -2479,11 +2479,19 @@ int dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 	napi->gro_list = skb;
 	ret = GRO_HELD;
 
+pull:
+	if (unlikely(!pskb_may_pull(skb, skb_gro_offset(skb)))) {
+		if (napi->gro_list == skb)
+			napi->gro_list = skb->next;
+		ret = GRO_DROP;
+	}
+
 ok:
 	return ret;
 
 normal:
-	return GRO_NORMAL;
+	ret = GRO_NORMAL;
+	goto pull;
 }
 EXPORT_SYMBOL(dev_gro_receive);
 
@@ -2599,14 +2607,10 @@ EXPORT_SYMBOL(napi_fraginfo_skb);
 int napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb, int ret)
 {
 	int err = NET_RX_SUCCESS;
-	int may;
 
 	switch (ret) {
 	case GRO_NORMAL:
 	case GRO_HELD:
-		may = pskb_may_pull(skb, skb_gro_offset(skb));
-		BUG_ON(!may);
-
 		skb->protocol = eth_type_trans(skb, napi->dev);
 
 		if (ret == GRO_NORMAL)

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: gro: Fix handling of imprecisely split packets

[Jeff Kirsher]

On Sat, Jan 31, 2009 at 11:01 PM, Herbert Xu
<herbert@gondor.apana.org.au> wrote:
> Hi Jeff:
>
> Please let me know if the ixgbe problems persist with this patch
> against net-next.
>
> gro: Fix handling of imprecisely split packets
>
> The commit 89a1b249edcf9be884e71f92df84d48355c576aa (gro: Avoid
> copying headers of unmerged packets) only worked for packets
> which are either completely linear, completely non-linear, or
> packets which exactly split at the boundary between headers and
> payload.
>
> Anything else would cause bits in the header to go missing if
> the packet is held by GRO.
>
> This may have broken drivers such as ixgbe.
>
> This patch fixes the places that assumed or only worked with
> the above cases.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>

Thanks Herbert!

I have applied the patch and hopefully I will have more information to
you soon.  At the very least, I should have updated information for
you on Monday.

-- 
Cheers,
Jeff

Re: gro: Fix handling of imprecisely split packets

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat, 31 Jan 2009 23:49:49 -0800

> On Sat, Jan 31, 2009 at 11:01 PM, Herbert Xu
> <herbert@gondor.apana.org.au> wrote:
> > Hi Jeff:
> >
> > Please let me know if the ixgbe problems persist with this patch
> > against net-next.
> >
> > gro: Fix handling of imprecisely split packets
> >
> > The commit 89a1b249edcf9be884e71f92df84d48355c576aa (gro: Avoid
> > copying headers of unmerged packets) only worked for packets
> > which are either completely linear, completely non-linear, or
> > packets which exactly split at the boundary between headers and
> > payload.
> >
> > Anything else would cause bits in the header to go missing if
> > the packet is held by GRO.
> >
> > This may have broken drivers such as ixgbe.
> >
> > This patch fixes the places that assumed or only worked with
> > the above cases.
> >
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> >
> 
> Thanks Herbert!
> 
> I have applied the patch and hopefully I will have more information to
> you soon.  At the very least, I should have updated information for
> you on Monday.

Meanwhile I'll toss Herbert's patch into net-next-2.6

Re: gro: Fix handling of imprecisely split packets

[Jeff Kirsher]

On Sun, Feb 1, 2009 at 1:25 AM, David Miller <davem@davemloft.net> wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Sat, 31 Jan 2009 23:49:49 -0800
>
>> On Sat, Jan 31, 2009 at 11:01 PM, Herbert Xu
>> <herbert@gondor.apana.org.au> wrote:
>> > Hi Jeff:
>> >
>> > Please let me know if the ixgbe problems persist with this patch
>> > against net-next.
>> >
>> > gro: Fix handling of imprecisely split packets
>> >
>> > The commit 89a1b249edcf9be884e71f92df84d48355c576aa (gro: Avoid
>> > copying headers of unmerged packets) only worked for packets
>> > which are either completely linear, completely non-linear, or
>> > packets which exactly split at the boundary between headers and
>> > payload.
>> >
>> > Anything else would cause bits in the header to go missing if
>> > the packet is held by GRO.
>> >
>> > This may have broken drivers such as ixgbe.
>> >
>> > This patch fixes the places that assumed or only worked with
>> > the above cases.
>> >
>> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>> >
>>
>> Thanks Herbert!
>>
>> I have applied the patch and hopefully I will have more information to
>> you soon.  At the very least, I should have updated information for
>> you on Monday.
>
> Meanwhile I'll toss Herbert's patch into net-next-2.6
> --

Sounds good to me, thanks Dave.

-- 
Cheers,
Jeff