Message-ID: <CA+mtBx-sP6g9hXAYGfHKCPYkCtjgCFM0srueh1uhoNnC2ACr1g@mail.gmail.com>
Date:	Tue, 30 Sep 2014 08:34:14 -0700
From:	Tom Herbert <therbert@...gle.com>
To:	Or Gerlitz <gerlitz.or@...il.com>
Cc:	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 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?

Thanks,
Tom

> Commit 6a93cc9052748c6355ec9d5b6c38b77f85f1cb0d "udp-tunnel: Add a few
> more UDP tunnel APIs"
> added encap_type field to sockets. And commit
> acbf74a763002bdc74ccfcdac22360bf18e305c5
> "vxlan: Refactor vxlan driver to make use of the common UDP tunnel
> functions" sets encap_type
> for vxlan sockets. We can apply the same idea on packets going by
> specific UDP encapsulation drivers.
>
> Or.
--
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