All of lore.kernel.org
 help / color / mirror / Atom feed
From: Edward Cree <ecree@solarflare.com>
To: Tom Herbert <tom@herbertland.com>
Cc: netdev <netdev@vger.kernel.org>, David Miller <davem@davemloft.net>
Subject: Re: [PATCH 1/2] net: udp: local checksum offload for encapsulation
Date: Tue, 15 Dec 2015 18:07:48 +0000	[thread overview]
Message-ID: <567056F4.1050107@solarflare.com> (raw)
In-Reply-To: <CALx6S34pyayogC3ZrifL9TGCzQbu_sC9R2Xtp8GsnybbZ9Ve6g@mail.gmail.com>

On 14/12/15 17:16, Tom Herbert wrote:
> It's clever, but I'm not sure this saves much. The outer checksum
> could still be offloaded to the device without the extra work.
The main thing it saves you is having to fit a second csum_start/offset pair
 into (a) the SKB and (b) the TX descriptor.  Also, it will (at least in
 principle) support arbitrarily deep nesting of encapsulation.  (Whether
 that's a remotely sensible thing to do is another matter!)

Also, local checksum offload probably helps by simplifying TSO.
I've just been talking to our firmware team and we concluded that if the
 outer checksum (of the superpacket) is filled in by the stack, datapath
 firmware on the sfc 8000 series could fold appropriate corrections (to
 account for changes in outer length and the length fields in both pseudo
 headers) into the outer checksum when performing TSO, which may (we
 haven't benchmarked it yet!) improve performance, compared to computing the
 whole outer checksum from scratch.
> Where
> this technique would be nice is if the device doesn't support checksum
> offload at all, then we would definitely avoid doing multiple
> checksums. That's going to be harder since we won't see
> CHECKSUM_PARTIAL in that case for the inner checksum, but it would get
> us to the principle that we only ever calculate the packet checksum
> once or zero times.
Presumably in that case the inner checksum would be marked as
 CHECKSUM_UNNECESSARY, and the code that fills in the inner checksum could
 also fill in skb->csum_start and skb->csum_offset.  Then in udp_set_csum()
 we could just treat it exactly the same as CHECKSUM_PARTIAL.  Then we could
 update start and offset to refer to the checksum we've just filled in, so
 that any nested encap doesn't have to re-checksum our header.
I have no objections to that, but I don't think I grok the rest of the stack
 deeply enough to implement it.

In the meantime, it seems worthwhile getting the CHECKSUM_PARTIAL version
 in, and working any bugs out of it; do you have any objections to my patch
 series as it stands or should I repost without RFC tags?  I notice it's
 marked "Changes Requested" in patchwork but it's not entirely clear to me
 what changes are wanted.

  reply	other threads:[~2015-12-15 18:07 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-07 15:39 Checksum offload queries Edward Cree
2015-12-07 17:29 ` Tom Herbert
2015-12-07 17:52   ` Tom Herbert
2015-12-08 16:03   ` Edward Cree
2015-12-08 16:43     ` Tom Herbert
2015-12-08 18:03       ` Edward Cree
2015-12-08 17:09     ` David Miller
2015-12-08 17:24       ` Edward Cree
2015-12-08 17:28         ` David Miller
2015-12-07 19:38 ` David Miller
2015-12-08 14:42   ` Edward Cree
2015-12-08 17:04     ` Tom Herbert
2015-12-09  1:56       ` Thomas Graf
2015-12-09 16:08         ` Tom Herbert
2015-12-09 22:29           ` Thomas Graf
2015-12-09 22:51             ` Tom Herbert
2015-12-09 23:13               ` Thomas Graf
2015-12-08 17:06     ` David Miller
2015-12-09 12:14       ` Edward Cree
2015-12-09 16:01         ` Tom Herbert
2015-12-09 17:28           ` Edward Cree
2015-12-09 17:31             ` David Laight
2015-12-09 18:00             ` Tom Herbert
2015-12-09 22:21               ` Thomas Graf
2015-12-09 22:42                 ` Tom Herbert
2015-12-09 22:44                   ` Thomas Graf
2015-12-10 15:49               ` Edward Cree
2015-12-10 16:26                 ` Tom Herbert
2015-12-10 20:28                   ` Edward Cree
2015-12-10 21:02                     ` Rustad, Mark D
2015-12-14 15:11                     ` [RFC PATCH net-next 0/2] Local checksum offload for VXLAN Edward Cree
2015-12-14 15:13                       ` [PATCH 1/2] net: udp: local checksum offload for encapsulation Edward Cree
2015-12-14 17:16                         ` Tom Herbert
2015-12-15 18:07                           ` Edward Cree [this message]
2015-12-14 15:13                       ` [PATCH 2/2] net: vxlan: enable local checksum offload on HW_CSUM devices Edward Cree
2015-12-11 23:50             ` Checksum offload queries Tom Herbert
2015-12-12 16:41               ` Sowmini Varadhan
2015-12-12 17:24                 ` Tom Herbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=567056F4.1050107@solarflare.com \
    --to=ecree@solarflare.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=tom@herbertland.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.