Message-ID: <CAJZOPZJcxb1j+_A2T=yN8bkjBH3CFjnb05q56dxAwtZ4i85Mpw@mail.gmail.com>
Date:	Tue, 15 Jul 2014 17:06:35 +0300
From:	Or Gerlitz <or.gerlitz@...il.com>
To:	Jerry Chu <hkchu@...gle.com>
Cc:	Tom Herbert <therbert@...gle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Or Gerlitz <ogerlitz@...lanox.com>,
	Wolfgang Walter <linux@...m.de>,
	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net-gre-gro: Fix a bug that breaks the
 forwarding path

On Tue, Jul 15, 2014 at 12:30 AM, Jerry Chu <hkchu@...gle.com> wrote:
>
> On Mon, Jul 14, 2014 at 1:38 PM, Or Gerlitz <or.gerlitz@...il.com> wrote:
> > On Mon, Jul 14, 2014 at 10:16 PM, Jerry Chu <hkchu@...gle.com> wrote:
> >> On Mon, Jul 14, 2014 at 12:05 PM, Or Gerlitz <or.gerlitz@...il.com> wrote:
> >>> On Sun, Jul 13, 2014 at 5:46 PM, H.K. Jerry Chu <hkchu@...gle.com> wrote:
> >>>> Fixed a bug that was introduced by my GRE-GRO patch
> >>>> (bf5a755f5e9186406bbf50f4087100af5bd68e40 net-gre-gro: Add GRE
> >>>> support to the GRO stack) that breaks the forwarding path
> >>>> because various GSO related fields were not set. The bug will
> >>>> cause on the egress path either the GSO code to fail, or a
> >>>> GRE-TSO capable (NETIF_F_GSO_GRE) NICs to choke. The following
> >>>> fix has been tested for both cases.
> >
> >>> Anything different in this version vs. the one you posted earlier on
> >>> February or this is a plain re-post?
> >
> >> I simply moved the patch against the latest net-next and resolved some small
> >> conflict so yes it's pretty much the same.
> >
> > OK, got it.
> >
> >> Also I don't see the subsequent
> >> discussion on skb->encapsulation affects the validity of this patch so
> >> i'm resubmitting it. Also the patch has been confirmed to address the problem
> >> Wolfgang reported last week. Feel free to test against the configuration (VXLAN?)
> >> you had some question about earlier.
> >
> > Well, re-reading that thread, we were not very decisive there... my
> > comment of setting the inner network header twice in inet_gro_complete
> > doesn't apply only to vxlan but to other tunneling protocols.
>
> Understood, but my reply about it applying to the inner most hdr in the end
> is not limited to vxlan either.
>
> > Also, if
> > we really need (why do we? or explained it on the Feb thread) to set
> > skb->encapsulation for the sake of TX in a protocol (GRE) gro
> > complete, looks a bit fishy to me...
>
> Why does it look fishy? When h/w support sLRO on tunneled pkts
> driver will set that bit. So it seems very reasonable for the GRO stack
> that supports tunneled pkts to set that bit too.


what do you mean by "h/w support sLRO on tunneled pkts" here? as far
as I understand, the driver should set the bit when they know it's an
encapsulated
packet for which the HW offloaded checksum verification.

So my fishiness comment was referring to the fact that we see here
that the stack
is setting this bit too sometimes.

> > but saying this I think brings us
> > back to that incomplete discussion [1]  sounds as this needs some
> > plumbering... Still, this way or another I understand a regression was
> > introduced here and should be fixed.

> Yep. I thought we've made reasonable progress on the skb->encapsualtion
> even though we can't close all the questions/issues

where/how exactly (or more or less) this progress comes into play, is it
by the description exchanged in the emails sent over the Feb/Mar thread
or in Tom's 3.15/16/17 patches?

> (but we have a real bug that needs to be fixed here).

agree...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists