netdev - Re: [PATCH v4 net-next] net: Implement fast csum_partial for x86

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 26 Feb 2016 12:41:51 -0800
From:	Tom Herbert <tom@...bertland.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	David Miller <davem@...emloft.net>,
	Network Development <netdev@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Peter Anvin <hpa@...or.com>,
	"the arch/x86 maintainers" <x86@...nel.org>,
	kernel-team <kernel-team@...com>
Subject: Re: [PATCH v4 net-next] net: Implement fast csum_partial for x86_64

On Fri, Feb 26, 2016 at 12:29 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> Looks ok to me.
>
> I am left wondering if the code should just do that
>
>     add32_with_carry3(sum, result >> 32, result);
>
> in the caller instead - right now pretty much every return point in
> do_csum() effectively does that,  with the exception of
>
>  - the 0-length case, which is presumably not really an issue in real
> life and could as well just return 0
>
>  - the 8-byte case that does two 32-bit loads instead, but could just
> do a single 8-byte load and return it (and then the generic version in
> the caller would do a shift).

Right, it is slightly faster for those two cases to return the result
directly. (csum over 8 bytes might be common with some encapsulation
protocols).

>
> That would simplifiy the code a bit - it wouldn't need to pass in
> "sum" to do_csum at all, and we'd have just a single case of that
> special 3-input carry add..
>
> But I'm certainly ok with it as-is. I'm not sure how performance
> critical the whole csum routine is, but at least now it doesn't
> introduce a lot of new complex asm.
>
Micro-performance optimizations may become more relevant as we
introduce more high performance network paths to the kernel. But the
short term reason for this is to dispel any remaining notion that NIC
HW support for CHECKSUM_UNNECESSARY is somehow better than
CHECKSUM_COMPLETE because pulling up checksums in the protocols is too
expensive.

Tom

> And this version might be reasonable to make generic, so that non-x86
> architectures could use the same approach. That's what we ended up
> doing for the dcache word-at-a-time code too in the end.
>
>                 Linus