netdev - Re: [PATCH 1/2] net: udp: local checksum offload for encapsulation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <567056F4.1050107@solarflare.com>
Date:	Tue, 15 Dec 2015 18:07:48 +0000
From:	Edward Cree <ecree@...arflare.com>
To:	Tom Herbert <tom@...bertland.com>
CC:	netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>
Subject: Re: [PATCH 1/2] net: udp: local checksum offload for encapsulation

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.
--
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