From: Eric Dumazet <hidden> Date: 2021-11-12 16:19:56
From: Eric Dumazet <edumazet@google.com>
With more NIC supporting CHECKSUM_COMPLETE, and IPv6 being widely used.
csum_partial() is heavily used with small amount of bytes,
and is consuming many cycles.
IPv6 header size for instance is 40 bytes.
Another thing to consider is that NET_IP_ALIGN is 0 on x86,
meaning that network headers are not word-aligned, unless
the driver forces this.
This means that csum_partial() fetches one u16
to 'align the buffer', then perform three u64 additions
with carry in a loop, then a remaining u32, then a remaining u16.
With this new version, we perform a loop only for the 64 bytes blocks,
then the remaining is bisected.
Tested on various cpus, all of them show a big reduction in
csum_partial() cost (by 50 to 80 %)
v3: - use "+r" (temp64) asm constraints (Andrew).
- fold do_csum() in csum_partial(), as gcc does not inline it.
- fix bug added in v2 for the "odd" case.
- back using addcq, as Andrew pointed the clang bug that was adding
a stall on my hosts.
(separate patch to add32_with_carry() will follow)
- use load_unaligned_zeropad() for final 1-7 bytes (Peter & Alexander).
v2: - removed the hard-coded switch(), as it was not RETPOLINE aware.
- removed the final add32_with_carry() that we were doing
in csum_partial(), we can simply pass @sum to do_csum().
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Alexander Duyck <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Cooper <redacted>
---
arch/x86/lib/csum-partial_64.c | 162 ++++++++++++++-------------------
1 file changed, 67 insertions(+), 95 deletions(-)
From: Peter Zijlstra <peterz@infradead.org> Date: 2021-11-12 16:45:40
On Fri, Nov 12, 2021 at 08:19:50AM -0800, Eric Dumazet wrote:
From: Eric Dumazet <edumazet@google.com>
With more NIC supporting CHECKSUM_COMPLETE, and IPv6 being widely used.
csum_partial() is heavily used with small amount of bytes,
and is consuming many cycles.
IPv6 header size for instance is 40 bytes.
Another thing to consider is that NET_IP_ALIGN is 0 on x86,
meaning that network headers are not word-aligned, unless
the driver forces this.
This means that csum_partial() fetches one u16
to 'align the buffer', then perform three u64 additions
with carry in a loop, then a remaining u32, then a remaining u16.
With this new version, we perform a loop only for the 64 bytes blocks,
then the remaining is bisected.
Tested on various cpus, all of them show a big reduction in
csum_partial() cost (by 50 to 80 %)
v3: - use "+r" (temp64) asm constraints (Andrew).
- fold do_csum() in csum_partial(), as gcc does not inline it.
- fix bug added in v2 for the "odd" case.
- back using addcq, as Andrew pointed the clang bug that was adding
a stall on my hosts.
(separate patch to add32_with_carry() will follow)
- use load_unaligned_zeropad() for final 1-7 bytes (Peter & Alexander).
v2: - removed the hard-coded switch(), as it was not RETPOLINE aware.
- removed the final add32_with_carry() that we were doing
in csum_partial(), we can simply pass @sum to do_csum().
Signed-off-by: Eric Dumazet <edumazet@google.com>
Looks nice, happen to have shiny perf numbers to show how awesome it it?
:-)
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-12 17:23:57
On Fri, Nov 12, 2021 at 8:45 AM Peter Zijlstra [off-list ref] wrote:
Looks nice, happen to have shiny perf numbers to show how awesome it it?
:-)
On a networking load on cascadlake, line rate received on a single thread, I see
perf -e cycles:pp -C <cpu>
Before:
4.16% [kernel] [k] csum_partial
After:
0.83% [kernel] [k] csum_partial
If run in a loop 1,000,000 times,
Before:
26,922,913 cycles # 3846130.429 GHz
80,302,961 instructions # 2.98 insn per
cycle
21,059,816 branches # 3008545142.857
M/sec
2,896 branch-misses # 0.01% of all
branches
After:
17,960,709 cycles # 3592141.800 GHz
41,292,805 instructions # 2.30 insn per
cycle
11,058,119 branches # 2211623800.000
M/sec
2,997 branch-misses # 0.03% of all
branches
Thanks for your help !
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-12 17:36:37
On Fri, Nov 12, 2021 at 9:23 AM Eric Dumazet [off-list ref] wrote:
On Fri, Nov 12, 2021 at 8:45 AM Peter Zijlstra [off-list ref] wrote:
quotedquoted
Looks nice, happen to have shiny perf numbers to show how awesome it it?
:-)
On a networking load on cascadlake, line rate received on a single thread, I see
perf -e cycles:pp -C <cpu>
Before:
4.16% [kernel] [k] csum_partial
After:
0.83% [kernel] [k] csum_partial
If run in a loop 1,000,000 times,
However, there must be an error in my patch, return values are not the
same on unaligned buffers.
From: Eric Dumazet <edumazet@google.com> Date: 2021-11-12 17:42:39
On Fri, Nov 12, 2021 at 9:36 AM Eric Dumazet [off-list ref] wrote:
On Fri, Nov 12, 2021 at 9:23 AM Eric Dumazet [off-list ref] wrote:
quoted
On Fri, Nov 12, 2021 at 8:45 AM Peter Zijlstra [off-list ref] wrote:
quotedquoted
Looks nice, happen to have shiny perf numbers to show how awesome it it?
:-)
On a networking load on cascadlake, line rate received on a single thread, I see
perf -e cycles:pp -C <cpu>
Before:
4.16% [kernel] [k] csum_partial
After:
0.83% [kernel] [k] csum_partial
If run in a loop 1,000,000 times,
However, there must be an error in my patch, return values are not the
same on unaligned buffers.
Oh silly me, the 32bit value is different, but the 16bit csum is good,
sorry for the noise.
From: Alexander Duyck <hidden> Date: 2021-11-13 01:13:41
On Fri, Nov 12, 2021 at 8:19 AM Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <edumazet@google.com>
With more NIC supporting CHECKSUM_COMPLETE, and IPv6 being widely used.
csum_partial() is heavily used with small amount of bytes,
and is consuming many cycles.
IPv6 header size for instance is 40 bytes.
Another thing to consider is that NET_IP_ALIGN is 0 on x86,
meaning that network headers are not word-aligned, unless
the driver forces this.
This means that csum_partial() fetches one u16
to 'align the buffer', then perform three u64 additions
with carry in a loop, then a remaining u32, then a remaining u16.
With this new version, we perform a loop only for the 64 bytes blocks,
then the remaining is bisected.
Tested on various cpus, all of them show a big reduction in
csum_partial() cost (by 50 to 80 %)
v3: - use "+r" (temp64) asm constraints (Andrew).
- fold do_csum() in csum_partial(), as gcc does not inline it.
- fix bug added in v2 for the "odd" case.
- back using addcq, as Andrew pointed the clang bug that was adding
a stall on my hosts.
(separate patch to add32_with_carry() will follow)
- use load_unaligned_zeropad() for final 1-7 bytes (Peter & Alexander).
v2: - removed the hard-coded switch(), as it was not RETPOLINE aware.
- removed the final add32_with_carry() that we were doing
in csum_partial(), we can simply pass @sum to do_csum().
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Alexander Duyck <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Cooper <redacted>
---
arch/x86/lib/csum-partial_64.c | 162 ++++++++++++++-------------------
1 file changed, 67 insertions(+), 95 deletions(-)
Looks good to me.
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
From: Peter Zijlstra <peterz@infradead.org> Date: 2021-11-13 07:44:02
On Fri, Nov 12, 2021 at 09:23:39AM -0800, Eric Dumazet wrote:
On Fri, Nov 12, 2021 at 8:45 AM Peter Zijlstra [off-list ref] wrote:
quotedquoted
Looks nice, happen to have shiny perf numbers to show how awesome it it?
:-)
On a networking load on cascadlake, line rate received on a single thread, I see
perf -e cycles:pp -C <cpu>
Before:
4.16% [kernel] [k] csum_partial
After:
0.83% [kernel] [k] csum_partial
If run in a loop 1,000,000 times,
Before:
26,922,913 cycles # 3846130.429 GHz
80,302,961 instructions # 2.98 insn per
cycle
21,059,816 branches # 3008545142.857
M/sec
2,896 branch-misses # 0.01% of all
branches
After:
17,960,709 cycles # 3592141.800 GHz
41,292,805 instructions # 2.30 insn per
cycle
11,058,119 branches # 2211623800.000
M/sec
2,997 branch-misses # 0.03% of all
branches
Thanks for your help !
I've added these numbers to the Changelog and will queue the patch in
x86/core once -rc1 happens.
Thanks!
From: David Laight <hidden> Date: 2021-12-01 10:51:25
From: Eric Dumazet
Sent: 12 November 2021 16:20
With more NIC supporting CHECKSUM_COMPLETE, and IPv6 being widely used.
csum_partial() is heavily used with small amount of bytes,
and is consuming many cycles.
IPv6 header size for instance is 40 bytes.
Another thing to consider is that NET_IP_ALIGN is 0 on x86,
meaning that network headers are not word-aligned, unless
the driver forces this.
This means that csum_partial() fetches one u16
to 'align the buffer', then perform three u64 additions
with carry in a loop, then a remaining u32, then a remaining u16.
With this new version, we perform a loop only for the 64 bytes blocks,
then the remaining is bisected.
I missed this going through, a couple of comments.
I've removed all the old lines from the patch to make it readable.
Do you need to special case an odd buffer address?
You are doing misaligned reads for other (more likely)
misaligned addresses so optimising for odd buffer addresses
is rather pointless.
If misaligned reads do cost an extra clock then it might
be worth detecting the more likely '4n+2' alignment of a receive
buffer and aligning that to 8n.
I suspect it is worth doing:
switch (len & 24) {
}
and separately coding the 24 byte case
to reduce the number of 'adc $0,%reg'.
Although writing the conditions by hand might needed to get
the likely code first (whichever length it is).
If you do the 'len & 7' test at the top the 56bit 'trail' value
can just be added to the 32bit 'sum' input.
Just:
temp64 += *(u64 *)(buff + len - 8) << shift;
would do - except it can fall off the beginning of a page :-(
Maybe:
temp64 += load_unaligned_zeropad(buff + (len & ~7)) & (~0ull >> shift);
Generating the mask reduces the register dependency chain length.
Although I remember trying to do something like this and finding
it was actually slower than the old code.
The problem is likely to be the long register chain generating 'shift'
compared to the latency of multiple memory reads that you only get once.
So potentially a 'switch (len & 6)' followed by a final test for odd
length may in fact be better - who knows.
+ result = add32_with_carry(temp64 >> 32, temp64 & 0xffffffff);
if (unlikely(odd)) {
result = from32to16(result);
result = ((result >> 8) & 0xff) | ((result & 0xff) << 8);
}
+ return (__force __wsum)result;
}
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)