Re: [PATCH net-next 7/8] net: ip_tunnel: remove 'csum_help' argument to iptunnel_handle_offloads

[Edward Cree <ecree@solarflare.com>]

On 11/01/16 18:15, Alexander Duyck wrote:
> On Mon, Jan 11, 2016 at 9:31 AM, Edward Cree <ecree@solarflare.com> wrote:
>> Looks OK to me.  I'd rather tack both of your patches onto the endof the
>>  series, rather than incorporating your patch [1/2] directly into my patch
>>  [1/8]; that way (a) the history allows to understand regular LCO before
>>  adding in the GSO flavour, (b) you're credited for your improved lco_csum.
> Actually if you don't plan to incorporate the function I am fine with
> you just leaving it out for now.  I can focus on the GSO portions of
> all this and rewrite this to just be an update of your patch.  Then
> when you submit your patches and they are accepted I will submit the
> GSO implementations.
Ok, sounds good.
>> As for your patch [2/2], I don't pretend to understand GSO right now but it
>>  looks plausible enough.  Perhaps you could add a document about GSO to go
>>  alongside the checksum-offloads.txt one?
> I don't know if we really need to.  The logic is essentially the same
> as what you already have for the local checksum offload.  The only
> difference is there is the GSO logic floating around in there to also
> compute the outer checksum offload based on the inner that GSO had
> already retained when it did the inner offload via software.
I didn't mean a GSO LCO document, I just meant something to explain GSOas
 a whole.  There doesn't appear to be any documentation in the tree
 defining what e.g. gso_size or gso_segs mean, and yet they are part of
 the driver API for TSO.  Even some comments on struct skb_shared_info
 would be an improvement.
The comment in skbuff.h seems very much to gloss over how GSO and checksum
 offload interact in general.  It also says two checksums can be offloaded
 with UDP tunnels - you might want to update that in your patch.
But hopefully the GSO stack can be fully LCOified at which point the
 checksum offload semantics for a GSO skb will be the same as a regular
 skb.  As for TSO, if the driver/hw has enough information to do TSO, it
 must know where all the headers are, so it must be able to do the right
 checksum corrections on all of them.  (I think outer checksums are only
 affected by the Length and pseudo-header Length field changes.)
However, this does raise the question - will TSO be possible with nested
 encap?  And if not, will current drivers do the right thing in that case,
 or will they try to do encap TSO and only get two of the three layers?

-ed