* [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390
@ 2019-07-04 8:52 Ilya Leoshkevich
2019-07-04 17:28 ` Y Song
2019-07-15 22:13 ` Alexei Starovoitov
0 siblings, 2 replies; 4+ messages in thread
From: Ilya Leoshkevich @ 2019-07-04 8:52 UTC (permalink / raw)
To: bpf, netdev, ys114321; +Cc: Ilya Leoshkevich
BPF_LDX_MEM is used to load the least significant byte of the retrieved
test_val.index, however, on big-endian machines it ends up retrieving
the most significant byte.
Use the correct least significant byte offset on big-endian machines.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
v1->v2:
- use __BYTE_ORDER instead of __BYTE_ORDER__.
tools/testing/selftests/bpf/verifier/value_ptr_arith.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
index c3de1a2c9dc5..e5940c4e8b8f 100644
--- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
+++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
@@ -183,7 +183,11 @@
BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
BPF_EXIT_INSN(),
+#if __BYTE_ORDER == __LITTLE_ENDIAN
BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, 0),
+#else
+ BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, sizeof(int) - 1),
+#endif
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 3),
BPF_MOV64_IMM(BPF_REG_2, 0),
BPF_MOV64_IMM(BPF_REG_3, 0x100000),
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390
2019-07-04 8:52 [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390 Ilya Leoshkevich
@ 2019-07-04 17:28 ` Y Song
2019-07-15 22:13 ` Alexei Starovoitov
1 sibling, 0 replies; 4+ messages in thread
From: Y Song @ 2019-07-04 17:28 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, netdev
On Thu, Jul 4, 2019 at 1:52 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> BPF_LDX_MEM is used to load the least significant byte of the retrieved
> test_val.index, however, on big-endian machines it ends up retrieving
> the most significant byte.
>
> Use the correct least significant byte offset on big-endian machines.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
>
> v1->v2:
> - use __BYTE_ORDER instead of __BYTE_ORDER__.
>
> tools/testing/selftests/bpf/verifier/value_ptr_arith.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> index c3de1a2c9dc5..e5940c4e8b8f 100644
> --- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> +++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> @@ -183,7 +183,11 @@
> BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
> BPF_EXIT_INSN(),
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, 0),
> +#else
> + BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, sizeof(int) - 1),
> +#endif
> BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 3),
> BPF_MOV64_IMM(BPF_REG_2, 0),
> BPF_MOV64_IMM(BPF_REG_3, 0x100000),
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390
2019-07-04 8:52 [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390 Ilya Leoshkevich
2019-07-04 17:28 ` Y Song
@ 2019-07-15 22:13 ` Alexei Starovoitov
2019-07-15 22:16 ` Daniel Borkmann
1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2019-07-15 22:13 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, Network Development, Y Song
On Thu, Jul 4, 2019 at 1:53 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> BPF_LDX_MEM is used to load the least significant byte of the retrieved
> test_val.index, however, on big-endian machines it ends up retrieving
> the most significant byte.
>
> Use the correct least significant byte offset on big-endian machines.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>
> v1->v2:
> - use __BYTE_ORDER instead of __BYTE_ORDER__.
>
> tools/testing/selftests/bpf/verifier/value_ptr_arith.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> index c3de1a2c9dc5..e5940c4e8b8f 100644
> --- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> +++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> @@ -183,7 +183,11 @@
> BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
> BPF_EXIT_INSN(),
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, 0),
> +#else
> + BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, sizeof(int) - 1),
> +#endif
I think tests should be arch and endian independent where possible.
In this case test_val.index is 4 byte and 4 byte load should work just as well.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390
2019-07-15 22:13 ` Alexei Starovoitov
@ 2019-07-15 22:16 ` Daniel Borkmann
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2019-07-15 22:16 UTC (permalink / raw)
To: Alexei Starovoitov, Ilya Leoshkevich; +Cc: bpf, Network Development, Y Song
On 7/16/19 12:13 AM, Alexei Starovoitov wrote:
> On Thu, Jul 4, 2019 at 1:53 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> BPF_LDX_MEM is used to load the least significant byte of the retrieved
>> test_val.index, however, on big-endian machines it ends up retrieving
>> the most significant byte.
>>
>> Use the correct least significant byte offset on big-endian machines.
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>>
>> v1->v2:
>> - use __BYTE_ORDER instead of __BYTE_ORDER__.
>>
>> tools/testing/selftests/bpf/verifier/value_ptr_arith.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
>> index c3de1a2c9dc5..e5940c4e8b8f 100644
>> --- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
>> +++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
>> @@ -183,7 +183,11 @@
>> BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
>> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
>> BPF_EXIT_INSN(),
>> +#if __BYTE_ORDER == __LITTLE_ENDIAN
>> BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, 0),
>> +#else
>> + BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, sizeof(int) - 1),
>> +#endif
>
> I think tests should be arch and endian independent where possible.
> In this case test_val.index is 4 byte and 4 byte load should work just as well.
Yes, agree, this should be fixed with BPF_W as load.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-15 22:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-04 8:52 [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390 Ilya Leoshkevich
2019-07-04 17:28 ` Y Song
2019-07-15 22:13 ` Alexei Starovoitov
2019-07-15 22:16 ` Daniel Borkmann
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).