Message-ID: <CA+mtBx-wp5qNHGM0jG+2RjtXor3x1zJcSvZafz7kEKNEm4jYLA@mail.gmail.com>
Date:	Tue, 30 Sep 2014 17:05:40 -0700
From:	Tom Herbert <therbert@...gle.com>
To:	Stephen Hemminger <stephen@...workplumber.org>
Cc:	Or Gerlitz <gerlitz.or@...il.com>,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	Alexander Duyck <alexander.h.duyck@...el.com>,
	David Miller <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Thomas Graf <tgraf@...g.ch>,
	Pravin Shelar <pshelar@...ira.com>,
	John Fastabend <john.r.fastabend@...el.com>,
	Andy Zhou <azhou@...ira.com>
Subject: Re: [PATCH] net: Add ndo_gso_check

On Tue, Sep 30, 2014 at 2:37 PM, Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> On Tue, 30 Sep 2014 08:34:14 -0700
> Tom Herbert <therbert@...gle.com> wrote:
>
> > On Tue, Sep 30, 2014 at 7:30 AM, Or Gerlitz <gerlitz.or@...il.com> wrote:
> > > On Tue, Sep 30, 2014 at 12:38 AM, Tom Herbert <therbert@...gle.com> wrote:
> > >> On Mon, Sep 29, 2014 at 2:10 PM, Or Gerlitz <gerlitz.or@...il.com> wrote:
> > >>> On Mon, Sep 29, 2014 at 11:53 PM, Tom Herbert <therbert@...gle.com> wrote:
> > >>>> On Mon, Sep 29, 2014 at 12:59 PM, Or Gerlitz <gerlitz.or@...il.com> wrote:
> > >>>>> On Mon, Sep 29, 2014 at 6:50 AM, Tom Herbert <therbert@...gle.com> wrote:
> > >>>>>> Add ndo_gso_check which a device can define to indicate whether is
> > >>>>>> is capable of doing GSO on a packet. This funciton would be called from
> > >>>>>> the stack to determine whether software GSO is needed to be done.
> > >>>>>
> > >>>>> please, no...
> > >>>>>
> > >>>>> We should strive to have a model/architecture under which the driver
> > >>>>> can clearly advertize up something (bits, collection of bits,
> > >>>>> whatever) which the stack can run through some dec-sion making code
> > >>>>> and decide if to GSO yes/no, do it ala SW or ala HW. As life, this
> > >>>>> model need not be perfect and can be biased towards being
> > >>>>> simple/robust/conservative and developed incrementally.
> > >
> > >>>> Please make a specific proposal then.
> > >
> > > OK
> > >
> > >>> We 1st need to bring the system back into a consistent state, see my
> > >>> band-aid proposal. BTW, this band-aid might turn to be the basis for
> > >>> the longer term solution too. I admit that saying "no" is ten (100)
> > >>> times harder vs. say "let's do it this way", but IMHO the fm10k call
> > >>> chain I pointed on is what you are suggesting more-or-less and is no-no
> > >
> > >> I'd much rather have drivers do this, than inflict the stack with more
> > >> complexity. As you describe "the driver can clearly advertise up
> > >> something (bits, collection of bits, whatever) which the stack can run
> > >> through some dec-sion making code and decide if to GSO yes/no"-- seems
> > >> very complex to me. My proposed alternative is to just ask the driver
> > >
> > > I see the point you are trying to make, but
> > >
> > >> and they can implement whatever policy they want, stack should doesn't
> > >> care about the specifics, just needs an answer. Neither does this
> > >> necessarily mean that driver needs to inspect packet, for instance I
> > >> suspect that just be looking at inner header lengths and skb->protocol
> > >> being TEB would be standard check to match VXLAN.
> > >
> > > I'm not sure how exactly this (inner protocol being Ethernet and inner
> > > header lengths)
> > > is going to work to differentiate between VXLAN and NVGRE (or @ least
> > > the GRE-ing done
> > > by OVS on guest Ethernet frames).
> > >
> > GSO processing for VXLAN and NVGRE should be identical. They both have
> > a four byte header that needs to be copied per packet and both only
> > carry Ethernet frames.
> >
> > >> In any case, if you can formulate your proposal in a patch that would
> > >> be very helpful.
> > >
> > > Quick idea is the following:
> > >
> > > It's common that when someone along the stack (e,g OVS vxlan/gre datapath logic)
> > > encapsulates a packet, they do know what sort of encapsulation they are doing.
> > >
> > > So the encapsulating entity can color the packet skb and the driver
> > > would advertize
> > > to what colors (UDP encap types) they can do GSO. When we come to a
> > > point where the
> > > stack has to decide if go for SW or HW GSO, they attempt to match the colors.
> > >
> > This would be equivalent to adding more protocol specific GSO feature
> > bits. I still don't see how this will scale. The number of protocols
> > that we might want to encapsulate over UDP is vast-- even before FOU
> > adding possibility of encapsulating any IP protocol in UDP. And, as
> > already pointed out, devices might have other arbitrary limitations
> > such as length of inner headers that wouldn't easily be represented in
> > features.
> >
> > Also, this does not benefit the stack or drivers that already support
> > generic SKB_GSO_UDP_TUNNEL mechanism.
> >
> > Would any other driver maintainers like to chime in on this?
>
> I prefer the simplistic "yes I can do GSO" flag and let the
> driver do software GSO for the cases where it detects "no never mind
> that, I can't' do GSO on a Q-in-Q VLAN with NVGRE".
>
That would be nice since it entails no change to the stack and follows
same model used for checksum (I couldn't find any drivers that call
skb_mac_gso_segment right now). Though the driver might need to handle
the case where it is unable to queue all the segments created.

> Software GSO at driver level is sometimes better anyway because it means driver can
> enqueue a burst of packets into Tx ring.


Will this still be an advantage with the burst TX implementation?
--
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