* Re: [RESEND] test_verifier #13 fails on arm64: "retval 65507 != -29" [not found] ` <68248069-bcf6-69dd-b0a9-f4ec11e50092@fb.com> @ 2019-07-10 10:02 ` Paolo Pisati 2019-07-10 22:52 ` Andrii Nakryiko 0 siblings, 1 reply; 8+ messages in thread From: Paolo Pisati @ 2019-07-10 10:02 UTC (permalink / raw) To: Yonghong Song Cc: Paolo Pisati, Daniel Borkmann, Alexei Starovoitov, Martin Lau, Song Liu, bpf, netdev On Mon, Jul 01, 2019 at 09:51:25PM +0000, Yonghong Song wrote: > > Below is the test case. > { > "valid read map access into a read-only array 2", > .insns = { > BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), > BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), > BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), > BPF_LD_MAP_FD(BPF_REG_1, 0), > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, > BPF_FUNC_map_lookup_elem), > BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6), > > BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), > BPF_MOV64_IMM(BPF_REG_2, 4), > BPF_MOV64_IMM(BPF_REG_3, 0), > BPF_MOV64_IMM(BPF_REG_4, 0), > BPF_MOV64_IMM(BPF_REG_5, 0), > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, > BPF_FUNC_csum_diff), > BPF_EXIT_INSN(), > }, > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > .fixup_map_array_ro = { 3 }, > .result = ACCEPT, > .retval = -29, > }, > > The issue may be with helper bpf_csum_diff(). > Maybe you can check bpf_csum_diff() helper return value > to confirm and take a further look at bpf_csum_diff implementations > between x64 and amd64. Indeed, the different result comes from csum_partial() or, more precisely, do_csum(). x86-64 uses an asm optimized version residing in arch/x86/lib/csum-partial_64.c, while the generic version is in lib/checksum.c. I replaced the x86-64 csum_partial() / do_csum() code, with the one in lib/checksum.c and by doing so i reproduced the same error on x86-64 (thus, it's not an arch dependent issue). I added some debugging to bpf_csum_diff(), and here are the results with different checksum implementation code: http://paste.debian.net/1091037/ lib/checksum.c: ... [ 206.084537] ____bpf_csum_diff from_size: 1 to_size: 0 [ 206.085274] ____bpf_csum_diff from[0]: 28 [ 206.085276] ____bpf_csum_diff diff[0]: 4294967267 [ 206.085277] ____bpf_csum_diff diff_size: 4 seed: 0 After csum_partial() call: [ 206.086059] ____bpf_csum_diff csum: 65507 - 0xffe3 arch/x86/lib/csum-partial_64.c ... [ 40.467308] ____bpf_csum_diff from_size: 1 to_size: 0 [ 40.468141] ____bpf_csum_diff from[0]: 28 [ 40.468143] ____bpf_csum_diff diff[0]: 4294967267 [ 40.468144] ____bpf_csum_diff diff_size: 4 seed: 0 After csum_partial() call: [ 40.468937] ____bpf_csum_diff csum: -29 - 0xffffffe3 One thing that i noticed, x86-64 csum-partial_64.c::do_csum() doesn't reduce the calculated checksum to 16bit before returning it (unless the input value is odd - *): arch/x86/lib/csum-partial_64.c::do_csum() ... if (unlikely(odd)) { result = from32to16(result); result = ((result >> 8) & 0xff) | ((result & 0xff) << 8); } return result; } contrary to all the other do_csum() implementations (that i could understand): lib/checksum.c::do_csum() arch/alpha/lib/checksum.c::do_csum() arch/parisc/lib/checksum.c::do_csum() Apparently even ia64 does the folding (arch/ia64/lib/do_csum.S see a comment right before .do_csum_exit:), and arch/c6x/lib/csum_64plus.S too (see arch/c6x/lib/csum_64plus.S). Funnily enough, if i change do_csum() for x86-64, folding the checksum to 16 bit (following all the other implementations): --- a/arch/x86/lib/csum-partial_64.c +++ b/arch/x86/lib/csum-partial_64.c @@ -112,8 +112,8 @@ static unsigned do_csum(const unsigned char *buff, unsigned len) if (len & 1) result += *buff; result = add32_with_carry(result>>32, result & 0xffffffff); + result = from32to16(result); if (unlikely(odd)) { - result = from32to16(result); result = ((result >> 8) & 0xff) | ((result & 0xff) << 8); } return result; then, the x86-64 result match the others: 65507 or 0xffe3. As a last attempt, i tried running the bpf test_verifier on an armhf platform, and i got a completely different number: [ 57.667999] ____bpf_csum_diff from_size: 1 to_size: 0 [ 57.668016] ____bpf_csum_diff from[0]: 28 [ 57.668028] ____bpf_csum_diff diff[0]: 4294967267 [ 57.668039] ____bpf_csum_diff diff_size: 4 seed: 0 After csum_partial() call: [ 57.668052] ____bpf_csum_diff::2002 csum: 131042 - 0x0001ffe2 Not sure what to make of these number, but i have a question: whats is the correct checksum of the memory chunk passed to csum_partial()? Is it really -29? Because, at least 2 other implementations i tested (the arm assembly code and the c implementation in lib/checksum.c) computes a different value, so either there's a bug in checksum calcution (2 out of 3???), or we are interpreting the returned value from csum_partial() somehow wrongly. *: originally, the x86-64 did the 16bit folding, but the logic was changed to what we have today during a big rewrite - search for: commit 3ef076bb685a461bbaff37a1f06010fc4d7ce733 Author: Andi Kleen <ak@suse.de> Date: Fri Jun 13 04:27:34 2003 -0700 [PATCH] x86-64 merge in this historic repo: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git -- bye, p. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND] test_verifier #13 fails on arm64: "retval 65507 != -29" 2019-07-10 10:02 ` [RESEND] test_verifier #13 fails on arm64: "retval 65507 != -29" Paolo Pisati @ 2019-07-10 22:52 ` Andrii Nakryiko 2019-07-10 22:54 ` Andrii Nakryiko 0 siblings, 1 reply; 8+ messages in thread From: Andrii Nakryiko @ 2019-07-10 22:52 UTC (permalink / raw) To: Paolo Pisati Cc: Yonghong Song, Daniel Borkmann, Alexei Starovoitov, Martin Lau, Song Liu, bpf, Networking, ak cc Andi Kleen re: refactoring, do you have any insight here? On Wed, Jul 10, 2019 at 3:12 AM Paolo Pisati <p.pisati@gmail.com> wrote: > > On Mon, Jul 01, 2019 at 09:51:25PM +0000, Yonghong Song wrote: > > > > Below is the test case. > > { > > "valid read map access into a read-only array 2", > > .insns = { > > BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), > > BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), > > BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), > > BPF_LD_MAP_FD(BPF_REG_1, 0), > > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, > > BPF_FUNC_map_lookup_elem), > > BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6), > > > > BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), > > BPF_MOV64_IMM(BPF_REG_2, 4), > > BPF_MOV64_IMM(BPF_REG_3, 0), > > BPF_MOV64_IMM(BPF_REG_4, 0), > > BPF_MOV64_IMM(BPF_REG_5, 0), > > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, > > BPF_FUNC_csum_diff), > > BPF_EXIT_INSN(), > > }, > > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > > .fixup_map_array_ro = { 3 }, > > .result = ACCEPT, > > .retval = -29, > > }, > > > > The issue may be with helper bpf_csum_diff(). > > Maybe you can check bpf_csum_diff() helper return value > > to confirm and take a further look at bpf_csum_diff implementations > > between x64 and amd64. > > Indeed, the different result comes from csum_partial() or, more precisely, > do_csum(). I assume this is checksum used for ipv4 header checksum ([0]), right? It's defined as "16-bit one's complement of the one's complement sum of all 16-bit words", so at the end it has to be folded into 16-bit anyways. Reading csum_partial/csum_fold, seems like after calculation of checksum (so-called unfolded checksum), it is supposed to be passed into csum_fold() to convert it into 16-bit one and invert. So maybe that's what is missing from bpf_csum_diff()? Though I've never used it and don't even know exactly what is its purpose, so might be (and probably am) totally wrong here (e.g., it might be that BPF app is supposed to do that or something). [0] https://en.wikipedia.org/wiki/IPv4_header_checksum > > x86-64 uses an asm optimized version residing in arch/x86/lib/csum-partial_64.c, > while the generic version is in lib/checksum.c. > > I replaced the x86-64 csum_partial() / do_csum() code, with the one in > lib/checksum.c and by doing so i reproduced the same error on x86-64 (thus, it's > not an arch dependent issue). > > I added some debugging to bpf_csum_diff(), and here are the results with different > checksum implementation code: > > http://paste.debian.net/1091037/ > > lib/checksum.c: > ... > [ 206.084537] ____bpf_csum_diff from_size: 1 to_size: 0 > [ 206.085274] ____bpf_csum_diff from[0]: 28 > [ 206.085276] ____bpf_csum_diff diff[0]: 4294967267 > [ 206.085277] ____bpf_csum_diff diff_size: 4 seed: 0 > > After csum_partial() call: > > [ 206.086059] ____bpf_csum_diff csum: 65507 - 0xffe3 > > arch/x86/lib/csum-partial_64.c > ... > [ 40.467308] ____bpf_csum_diff from_size: 1 to_size: 0 > [ 40.468141] ____bpf_csum_diff from[0]: 28 > [ 40.468143] ____bpf_csum_diff diff[0]: 4294967267 > [ 40.468144] ____bpf_csum_diff diff_size: 4 seed: 0 > > After csum_partial() call: > > [ 40.468937] ____bpf_csum_diff csum: -29 - 0xffffffe3 > > One thing that i noticed, x86-64 csum-partial_64.c::do_csum() doesn't reduce the > calculated checksum to 16bit before returning it (unless the input value is > odd - *): > > arch/x86/lib/csum-partial_64.c::do_csum() > ... > if (unlikely(odd)) { > result = from32to16(result); > result = ((result >> 8) & 0xff) | ((result & 0xff) << 8); > } > return result; > } > > contrary to all the other do_csum() implementations (that i could understand): > > lib/checksum.c::do_csum() > arch/alpha/lib/checksum.c::do_csum() > arch/parisc/lib/checksum.c::do_csum() > > Apparently even ia64 does the folding (arch/ia64/lib/do_csum.S see a comment right > before .do_csum_exit:), and arch/c6x/lib/csum_64plus.S too (see > arch/c6x/lib/csum_64plus.S). > > Funnily enough, if i change do_csum() for x86-64, folding the > checksum to 16 bit (following all the other implementations): > > --- a/arch/x86/lib/csum-partial_64.c > +++ b/arch/x86/lib/csum-partial_64.c > @@ -112,8 +112,8 @@ static unsigned do_csum(const unsigned char *buff, unsigned > len) > if (len & 1) > result += *buff; > result = add32_with_carry(result>>32, result & 0xffffffff); > + result = from32to16(result); > if (unlikely(odd)) { > - result = from32to16(result); > result = ((result >> 8) & 0xff) | ((result & 0xff) << 8); > } > return result; > > then, the x86-64 result match the others: 65507 or 0xffe3. > > As a last attempt, i tried running the bpf test_verifier on an armhf platform, > and i got a completely different number: > > [ 57.667999] ____bpf_csum_diff from_size: 1 to_size: 0 > [ 57.668016] ____bpf_csum_diff from[0]: 28 > [ 57.668028] ____bpf_csum_diff diff[0]: 4294967267 > [ 57.668039] ____bpf_csum_diff diff_size: 4 seed: 0 > > After csum_partial() call: > > [ 57.668052] ____bpf_csum_diff::2002 csum: 131042 - 0x0001ffe2 > > Not sure what to make of these number, but i have a question: whats is the > correct checksum of the memory chunk passed to csum_partial()? Is it really -29? > > Because, at least 2 other implementations i tested (the arm assembly code and > the c implementation in lib/checksum.c) computes a different value, so either > there's a bug in checksum calcution (2 out of 3???), or we are interpreting the > returned value from csum_partial() somehow wrongly. > > *: originally, the x86-64 did the 16bit folding, but the logic was changed to > what we have today during a big rewrite - search for: > > commit 3ef076bb685a461bbaff37a1f06010fc4d7ce733 > Author: Andi Kleen <ak@suse.de> > Date: Fri Jun 13 04:27:34 2003 -0700 > > [PATCH] x86-64 merge > > in this historic repo: > > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git > -- > bye, > p. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND] test_verifier #13 fails on arm64: "retval 65507 != -29" 2019-07-10 22:52 ` Andrii Nakryiko @ 2019-07-10 22:54 ` Andrii Nakryiko 2019-07-10 23:14 ` Andi Kleen 0 siblings, 1 reply; 8+ messages in thread From: Andrii Nakryiko @ 2019-07-10 22:54 UTC (permalink / raw) To: Paolo Pisati Cc: Yonghong Song, Daniel Borkmann, Alexei Starovoitov, Martin Lau, Song Liu, bpf, Networking, ak On Wed, Jul 10, 2019 at 3:52 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > cc Andi Kleen re: refactoring, do you have any insight here? OK, let's try again... > > On Wed, Jul 10, 2019 at 3:12 AM Paolo Pisati <p.pisati@gmail.com> wrote: > > > > On Mon, Jul 01, 2019 at 09:51:25PM +0000, Yonghong Song wrote: > > > > > > Below is the test case. > > > { > > > "valid read map access into a read-only array 2", > > > .insns = { > > > BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), > > > BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), > > > BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), > > > BPF_LD_MAP_FD(BPF_REG_1, 0), > > > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, > > > BPF_FUNC_map_lookup_elem), > > > BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6), > > > > > > BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), > > > BPF_MOV64_IMM(BPF_REG_2, 4), > > > BPF_MOV64_IMM(BPF_REG_3, 0), > > > BPF_MOV64_IMM(BPF_REG_4, 0), > > > BPF_MOV64_IMM(BPF_REG_5, 0), > > > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, > > > BPF_FUNC_csum_diff), > > > BPF_EXIT_INSN(), > > > }, > > > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > > > .fixup_map_array_ro = { 3 }, > > > .result = ACCEPT, > > > .retval = -29, > > > }, > > > > > > The issue may be with helper bpf_csum_diff(). > > > Maybe you can check bpf_csum_diff() helper return value > > > to confirm and take a further look at bpf_csum_diff implementations > > > between x64 and amd64. > > > > Indeed, the different result comes from csum_partial() or, more precisely, > > do_csum(). > > I assume this is checksum used for ipv4 header checksum ([0]), right? > It's defined as "16-bit one's complement of the one's complement sum > of all 16-bit words", so at the end it has to be folded into 16-bit > anyways. > > Reading csum_partial/csum_fold, seems like after calculation of > checksum (so-called unfolded checksum), it is supposed to be passed > into csum_fold() to convert it into 16-bit one and invert. > > So maybe that's what is missing from bpf_csum_diff()? Though I've > never used it and don't even know exactly what is its purpose, so > might be (and probably am) totally wrong here (e.g., it might be that > BPF app is supposed to do that or something). > > [0] https://en.wikipedia.org/wiki/IPv4_header_checksum > > > > > x86-64 uses an asm optimized version residing in arch/x86/lib/csum-partial_64.c, > > while the generic version is in lib/checksum.c. > > > > I replaced the x86-64 csum_partial() / do_csum() code, with the one in > > lib/checksum.c and by doing so i reproduced the same error on x86-64 (thus, it's > > not an arch dependent issue). > > > > I added some debugging to bpf_csum_diff(), and here are the results with different > > checksum implementation code: > > > > http://paste.debian.net/1091037/ > > > > lib/checksum.c: > > ... > > [ 206.084537] ____bpf_csum_diff from_size: 1 to_size: 0 > > [ 206.085274] ____bpf_csum_diff from[0]: 28 > > [ 206.085276] ____bpf_csum_diff diff[0]: 4294967267 > > [ 206.085277] ____bpf_csum_diff diff_size: 4 seed: 0 > > > > After csum_partial() call: > > > > [ 206.086059] ____bpf_csum_diff csum: 65507 - 0xffe3 > > > > arch/x86/lib/csum-partial_64.c > > ... > > [ 40.467308] ____bpf_csum_diff from_size: 1 to_size: 0 > > [ 40.468141] ____bpf_csum_diff from[0]: 28 > > [ 40.468143] ____bpf_csum_diff diff[0]: 4294967267 > > [ 40.468144] ____bpf_csum_diff diff_size: 4 seed: 0 > > > > After csum_partial() call: > > > > [ 40.468937] ____bpf_csum_diff csum: -29 - 0xffffffe3 > > > > One thing that i noticed, x86-64 csum-partial_64.c::do_csum() doesn't reduce the > > calculated checksum to 16bit before returning it (unless the input value is > > odd - *): > > > > arch/x86/lib/csum-partial_64.c::do_csum() > > ... > > if (unlikely(odd)) { > > result = from32to16(result); > > result = ((result >> 8) & 0xff) | ((result & 0xff) << 8); > > } > > return result; > > } > > > > contrary to all the other do_csum() implementations (that i could understand): > > > > lib/checksum.c::do_csum() > > arch/alpha/lib/checksum.c::do_csum() > > arch/parisc/lib/checksum.c::do_csum() > > > > Apparently even ia64 does the folding (arch/ia64/lib/do_csum.S see a comment right > > before .do_csum_exit:), and arch/c6x/lib/csum_64plus.S too (see > > arch/c6x/lib/csum_64plus.S). > > > > Funnily enough, if i change do_csum() for x86-64, folding the > > checksum to 16 bit (following all the other implementations): > > > > --- a/arch/x86/lib/csum-partial_64.c > > +++ b/arch/x86/lib/csum-partial_64.c > > @@ -112,8 +112,8 @@ static unsigned do_csum(const unsigned char *buff, unsigned > > len) > > if (len & 1) > > result += *buff; > > result = add32_with_carry(result>>32, result & 0xffffffff); > > + result = from32to16(result); > > if (unlikely(odd)) { > > - result = from32to16(result); > > result = ((result >> 8) & 0xff) | ((result & 0xff) << 8); > > } > > return result; > > > > then, the x86-64 result match the others: 65507 or 0xffe3. > > > > As a last attempt, i tried running the bpf test_verifier on an armhf platform, > > and i got a completely different number: > > > > [ 57.667999] ____bpf_csum_diff from_size: 1 to_size: 0 > > [ 57.668016] ____bpf_csum_diff from[0]: 28 > > [ 57.668028] ____bpf_csum_diff diff[0]: 4294967267 > > [ 57.668039] ____bpf_csum_diff diff_size: 4 seed: 0 > > > > After csum_partial() call: > > > > [ 57.668052] ____bpf_csum_diff::2002 csum: 131042 - 0x0001ffe2 > > > > Not sure what to make of these number, but i have a question: whats is the > > correct checksum of the memory chunk passed to csum_partial()? Is it really -29? > > > > Because, at least 2 other implementations i tested (the arm assembly code and > > the c implementation in lib/checksum.c) computes a different value, so either > > there's a bug in checksum calcution (2 out of 3???), or we are interpreting the > > returned value from csum_partial() somehow wrongly. > > > > *: originally, the x86-64 did the 16bit folding, but the logic was changed to > > what we have today during a big rewrite - search for: > > > > commit 3ef076bb685a461bbaff37a1f06010fc4d7ce733 > > Author: Andi Kleen <ak@suse.de> > > Date: Fri Jun 13 04:27:34 2003 -0700 > > > > [PATCH] x86-64 merge > > > > in this historic repo: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git > > -- > > bye, > > p. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND] test_verifier #13 fails on arm64: "retval 65507 != -29" 2019-07-10 22:54 ` Andrii Nakryiko @ 2019-07-10 23:14 ` Andi Kleen 2019-07-11 9:40 ` [PATCH 0/2] [RESEND] Fold checksum at the end of bpf_csum_diff and fix Paolo Pisati 0 siblings, 1 reply; 8+ messages in thread From: Andi Kleen @ 2019-07-10 23:14 UTC (permalink / raw) To: Andrii Nakryiko Cc: Paolo Pisati, Yonghong Song, Daniel Borkmann, Alexei Starovoitov, Martin Lau, Song Liu, bpf, Networking > > Reading csum_partial/csum_fold, seems like after calculation of > > checksum (so-called unfolded checksum), it is supposed to be passed > > into csum_fold() to convert it into 16-bit one and invert. Yes, you always need to fold at the end. The low level code does fold sometimes, but not always. -Andi ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] [RESEND] Fold checksum at the end of bpf_csum_diff and fix 2019-07-10 23:14 ` Andi Kleen @ 2019-07-11 9:40 ` Paolo Pisati 2019-07-11 9:40 ` [PATCH 1/2] bpf: bpf_csum_diff: fold the checksum before returning the value Paolo Pisati 2019-07-11 9:40 ` [PATCH 2/2] bpf, selftest: fix checksum value for test #13 Paolo Pisati 0 siblings, 2 replies; 8+ messages in thread From: Paolo Pisati @ 2019-07-11 9:40 UTC (permalink / raw) To: --to=Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, David S . Miller, Shuah Khan, Jakub Kicinski, Jiong Wang Cc: netdev, bpf, linux-kselftest, linux-kernel From: Paolo Pisati <paolo.pisati@canonical.com> After applying patch 0001, all checksum implementations i could test (x86-64, arm64 and arm), now agree on the return value. Patch 0002 fix the expected return value for test #13: i did the calculation manually, and it correspond. Unfortunately, after applying patch 0001, other test cases now fail in test_verifier: $ sudo ./tools/testing/selftests/bpf/test_verifier ... #417/p helper access to variable memory: size = 0 allowed on NULL (ARG_PTR_TO_MEM_OR_NULL) FAIL retval 65535 != 0 #419/p helper access to variable memory: size = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL) FAIL retval 65535 != 0 #423/p helper access to variable memory: size possible = 0 allowed on != NULL packet pointer (ARG_PTR_TO_MEM_OR_NULL) FAIL retval 65535 != 0 ... Summary: 1500 PASSED, 0 SKIPPED, 3 FAILED And there are probably other fallouts in other selftests - someone familiar should take a look before applying these patches. Paolo Pisati (2): bpf: bpf_csum_diff: fold the checksum before returning the value bpf, selftest: fix checksum value for test #13 net/core/filter.c | 2 +- tools/testing/selftests/bpf/verifier/array_access.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] bpf: bpf_csum_diff: fold the checksum before returning the value 2019-07-11 9:40 ` [PATCH 0/2] [RESEND] Fold checksum at the end of bpf_csum_diff and fix Paolo Pisati @ 2019-07-11 9:40 ` Paolo Pisati 2019-07-11 9:40 ` [PATCH 2/2] bpf, selftest: fix checksum value for test #13 Paolo Pisati 1 sibling, 0 replies; 8+ messages in thread From: Paolo Pisati @ 2019-07-11 9:40 UTC (permalink / raw) To: --to=Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, David S . Miller, Shuah Khan, Jakub Kicinski, Jiong Wang Cc: netdev, bpf, linux-kselftest, linux-kernel From: Paolo Pisati <paolo.pisati@canonical.com> With this change, bpf_csum_diff behave homogeneously among different checksum calculation code / csum_partial() (tested on x86-64, arm64 and arm). Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> --- net/core/filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/filter.c b/net/core/filter.c index f615e42cf4ef..8db7f58f1ea1 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1990,7 +1990,7 @@ BPF_CALL_5(bpf_csum_diff, __be32 *, from, u32, from_size, for (i = 0; i < to_size / sizeof(__be32); i++, j++) sp->diff[j] = to[i]; - return csum_partial(sp->diff, diff_size, seed); + return csum_fold(csum_partial(sp->diff, diff_size, seed)); } static const struct bpf_func_proto bpf_csum_diff_proto = { -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] bpf, selftest: fix checksum value for test #13 2019-07-11 9:40 ` [PATCH 0/2] [RESEND] Fold checksum at the end of bpf_csum_diff and fix Paolo Pisati 2019-07-11 9:40 ` [PATCH 1/2] bpf: bpf_csum_diff: fold the checksum before returning the value Paolo Pisati @ 2019-07-11 9:40 ` Paolo Pisati 2019-07-11 23:47 ` Andrii Nakryiko 1 sibling, 1 reply; 8+ messages in thread From: Paolo Pisati @ 2019-07-11 9:40 UTC (permalink / raw) To: --to=Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, David S . Miller, Shuah Khan, Jakub Kicinski, Jiong Wang Cc: netdev, bpf, linux-kselftest, linux-kernel From: Paolo Pisati <paolo.pisati@canonical.com> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> --- tools/testing/selftests/bpf/verifier/array_access.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/verifier/array_access.c b/tools/testing/selftests/bpf/verifier/array_access.c index bcb83196e459..4698f560d756 100644 --- a/tools/testing/selftests/bpf/verifier/array_access.c +++ b/tools/testing/selftests/bpf/verifier/array_access.c @@ -255,7 +255,7 @@ .prog_type = BPF_PROG_TYPE_SCHED_CLS, .fixup_map_array_ro = { 3 }, .result = ACCEPT, - .retval = -29, + .retval = 28, }, { "invalid write map access into a read-only array 1", -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] bpf, selftest: fix checksum value for test #13 2019-07-11 9:40 ` [PATCH 2/2] bpf, selftest: fix checksum value for test #13 Paolo Pisati @ 2019-07-11 23:47 ` Andrii Nakryiko 0 siblings, 0 replies; 8+ messages in thread From: Andrii Nakryiko @ 2019-07-11 23:47 UTC (permalink / raw) To: Paolo Pisati Cc: --to=Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, David S . Miller, Shuah Khan, Jakub Kicinski, Jiong Wang, Networking, bpf, open list:KERNEL SELFTEST FRAMEWORK, open list On Thu, Jul 11, 2019 at 2:41 AM Paolo Pisati <p.pisati@gmail.com> wrote: > > From: Paolo Pisati <paolo.pisati@canonical.com> > Please include description, in addition to subject. Also, when submitting patches, please add bpf or bpf-next (e.g., [PATCH bpf 2/2] to indicate which tree it's supposed to go into). For this one it's probably bpf. > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> > --- > tools/testing/selftests/bpf/verifier/array_access.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/verifier/array_access.c b/tools/testing/selftests/bpf/verifier/array_access.c > index bcb83196e459..4698f560d756 100644 > --- a/tools/testing/selftests/bpf/verifier/array_access.c > +++ b/tools/testing/selftests/bpf/verifier/array_access.c > @@ -255,7 +255,7 @@ > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > .fixup_map_array_ro = { 3 }, > .result = ACCEPT, > - .retval = -29, > + .retval = 28, > }, > { > "invalid write map access into a read-only array 1", > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-11 23:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20190701115414.GA4452@harukaze> [not found] ` <68248069-bcf6-69dd-b0a9-f4ec11e50092@fb.com> 2019-07-10 10:02 ` [RESEND] test_verifier #13 fails on arm64: "retval 65507 != -29" Paolo Pisati 2019-07-10 22:52 ` Andrii Nakryiko 2019-07-10 22:54 ` Andrii Nakryiko 2019-07-10 23:14 ` Andi Kleen 2019-07-11 9:40 ` [PATCH 0/2] [RESEND] Fold checksum at the end of bpf_csum_diff and fix Paolo Pisati 2019-07-11 9:40 ` [PATCH 1/2] bpf: bpf_csum_diff: fold the checksum before returning the value Paolo Pisati 2019-07-11 9:40 ` [PATCH 2/2] bpf, selftest: fix checksum value for test #13 Paolo Pisati 2019-07-11 23:47 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).