Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1703722 > unrolled thread
| Started by | Thomas Richter <tmricht@linux.vnet.ibm.com> |
|---|---|
| First post | 2017-08-04 08:50 +0200 |
| Last post | 2017-08-04 22:40 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] bpf: fix selftest/bpf/test_pkt_md_access on s390x Thomas Richter <tmricht@linux.vnet.ibm.com> - 2017-08-04 08:50 +0200
Re: [PATCH] bpf: fix selftest/bpf/test_pkt_md_access on s390x Daniel Borkmann <daniel@iogearbox.net> - 2017-08-04 21:10 +0200
Re: [PATCH] bpf: fix selftest/bpf/test_pkt_md_access on s390x Daniel Borkmann <daniel@iogearbox.net> - 2017-08-04 22:40 +0200
| From | Thomas Richter <tmricht@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-04 08:50 +0200 |
| Subject | [PATCH] bpf: fix selftest/bpf/test_pkt_md_access on s390x |
| Message-ID | <uaEAO-3uH-19@gated-at.bofh.it> |
Commit 18f3d6be6be1 ("selftests/bpf: Add test cases to test narrower ctx field loads")
introduced new eBPF test cases. One of them (test_pkt_md_access.c)
fails on s390x. The BPF verifier error message is:
[root@s8360046 bpf]# ./test_progs
test_pkt_access:PASS:ipv4 349 nsec
test_pkt_access:PASS:ipv6 212 nsec
[....]
libbpf: load bpf program failed: Permission denied
libbpf: -- BEGIN DUMP LOG ---
libbpf:
0: (71) r2 = *(u8 *)(r1 +0)
invalid bpf_context access off=0 size=1
libbpf: -- END LOG --
libbpf: failed to load program 'test1'
libbpf: failed to load object './test_pkt_md_access.o'
Summary: 29 PASSED, 1 FAILED
[root@s8360046 bpf]#
This is caused by a byte endianess issue. S390x is a big endian
architecture. Pointer access to the lowest byte or halfword of a
four byte value need to add an offset.
On little endian architectures this offset is not needed.
Fix this and use the same approach as the originator used for other files
(for example test_verifier.c) in his original commit.
With this fix the test program test_progs succeeds on s390x:
[root@s8360046 bpf]# ./test_progs
test_pkt_access:PASS:ipv4 236 nsec
test_pkt_access:PASS:ipv6 217 nsec
test_xdp:PASS:ipv4 3624 nsec
test_xdp:PASS:ipv6 1722 nsec
test_l4lb:PASS:ipv4 926 nsec
test_l4lb:PASS:ipv6 1322 nsec
test_tcp_estats:PASS: 0 nsec
test_bpf_obj_id:PASS:get-fd-by-notexist-prog-id 0 nsec
test_bpf_obj_id:PASS:get-fd-by-notexist-map-id 0 nsec
test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
test_bpf_obj_id:PASS:check total prog id found by get_next_id 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
test_bpf_obj_id:PASS:check total map id found by get_next_id 0 nsec
test_pkt_md_access:PASS: 277 nsec
Summary: 30 PASSED, 0 FAILED
[root@s8360046 bpf]#
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
---
tools/testing/selftests/bpf/test_pkt_md_access.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_pkt_md_access.c b/tools/testing/selftests/bpf/test_pkt_md_access.c
index 71729d4..8393ced 100644
--- a/tools/testing/selftests/bpf/test_pkt_md_access.c
+++ b/tools/testing/selftests/bpf/test_pkt_md_access.c
@@ -12,12 +12,23 @@
int _version SEC("version") = 1;
+#ifdef __LITTLE_ENDIAN
#define TEST_FIELD(TYPE, FIELD, MASK) \
{ \
TYPE tmp = *(volatile TYPE *)&skb->FIELD; \
if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK)) \
return TC_ACT_SHOT; \
}
+#else
+#define TEST_FIELD_OFFSET(a, b) ((sizeof(a) - sizeof(b)) / sizeof(b))
+#define TEST_FIELD(TYPE, FIELD, MASK) \
+ { \
+ TYPE tmp = *((volatile TYPE *)&skb->FIELD + \
+ TEST_FIELD_OFFSET(skb->FIELD, TYPE)); \
+ if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK)) \
+ return TC_ACT_SHOT; \
+ }
+#endif
SEC("test1")
int process(struct __sk_buff *skb)
--
2.9.3
[toc] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-08-04 21:10 +0200 |
| Message-ID | <uaQ8W-2CK-29@gated-at.bofh.it> |
| In reply to | #1703722 |
Hi Thomas,
On 08/04/2017 08:40 AM, Thomas Richter wrote:
> Commit 18f3d6be6be1 ("selftests/bpf: Add test cases to test narrower ctx field loads")
> introduced new eBPF test cases. One of them (test_pkt_md_access.c)
> fails on s390x. The BPF verifier error message is:
Could you submit the fix to: netdev@vger.kernel.org
(BPF patches are handled/accepted there.)
Thanks a lot,
Daniel
> [root@s8360046 bpf]# ./test_progs
> test_pkt_access:PASS:ipv4 349 nsec
> test_pkt_access:PASS:ipv6 212 nsec
> [....]
> libbpf: load bpf program failed: Permission denied
> libbpf: -- BEGIN DUMP LOG ---
> libbpf:
> 0: (71) r2 = *(u8 *)(r1 +0)
> invalid bpf_context access off=0 size=1
>
> libbpf: -- END LOG --
> libbpf: failed to load program 'test1'
> libbpf: failed to load object './test_pkt_md_access.o'
> Summary: 29 PASSED, 1 FAILED
> [root@s8360046 bpf]#
>
> This is caused by a byte endianess issue. S390x is a big endian
> architecture. Pointer access to the lowest byte or halfword of a
> four byte value need to add an offset.
> On little endian architectures this offset is not needed.
>
> Fix this and use the same approach as the originator used for other files
> (for example test_verifier.c) in his original commit.
>
> With this fix the test program test_progs succeeds on s390x:
> [root@s8360046 bpf]# ./test_progs
> test_pkt_access:PASS:ipv4 236 nsec
> test_pkt_access:PASS:ipv6 217 nsec
> test_xdp:PASS:ipv4 3624 nsec
> test_xdp:PASS:ipv6 1722 nsec
> test_l4lb:PASS:ipv4 926 nsec
> test_l4lb:PASS:ipv6 1322 nsec
> test_tcp_estats:PASS: 0 nsec
> test_bpf_obj_id:PASS:get-fd-by-notexist-prog-id 0 nsec
> test_bpf_obj_id:PASS:get-fd-by-notexist-map-id 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:check total prog id found by get_next_id 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:check total map id found by get_next_id 0 nsec
> test_pkt_md_access:PASS: 277 nsec
> Summary: 30 PASSED, 0 FAILED
> [root@s8360046 bpf]#
>
> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
> ---
> tools/testing/selftests/bpf/test_pkt_md_access.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/test_pkt_md_access.c b/tools/testing/selftests/bpf/test_pkt_md_access.c
> index 71729d4..8393ced 100644
> --- a/tools/testing/selftests/bpf/test_pkt_md_access.c
> +++ b/tools/testing/selftests/bpf/test_pkt_md_access.c
> @@ -12,12 +12,23 @@
>
> int _version SEC("version") = 1;
>
> +#ifdef __LITTLE_ENDIAN
> #define TEST_FIELD(TYPE, FIELD, MASK) \
> { \
> TYPE tmp = *(volatile TYPE *)&skb->FIELD; \
> if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK)) \
> return TC_ACT_SHOT; \
> }
> +#else
> +#define TEST_FIELD_OFFSET(a, b) ((sizeof(a) - sizeof(b)) / sizeof(b))
> +#define TEST_FIELD(TYPE, FIELD, MASK) \
> + { \
> + TYPE tmp = *((volatile TYPE *)&skb->FIELD + \
> + TEST_FIELD_OFFSET(skb->FIELD, TYPE)); \
> + if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK)) \
> + return TC_ACT_SHOT; \
> + }
> +#endif
>
> SEC("test1")
> int process(struct __sk_buff *skb)
>
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-08-04 22:40 +0200 |
| Message-ID | <uaRy2-3oq-17@gated-at.bofh.it> |
| In reply to | #1704175 |
On 08/04/2017 09:06 PM, Daniel Borkmann wrote:
> Hi Thomas,
>
> On 08/04/2017 08:40 AM, Thomas Richter wrote:
>> Commit 18f3d6be6be1 ("selftests/bpf: Add test cases to test narrower ctx field loads")
>> introduced new eBPF test cases. One of them (test_pkt_md_access.c)
>> fails on s390x. The BPF verifier error message is:
>
> Could you submit the fix to: netdev@vger.kernel.org
>
> (BPF patches are handled/accepted there.)
>
> Thanks a lot,
> Daniel
>
>> [root@s8360046 bpf]# ./test_progs
>> test_pkt_access:PASS:ipv4 349 nsec
>> test_pkt_access:PASS:ipv6 212 nsec
>> [....]
>> libbpf: load bpf program failed: Permission denied
>> libbpf: -- BEGIN DUMP LOG ---
>> libbpf:
>> 0: (71) r2 = *(u8 *)(r1 +0)
>> invalid bpf_context access off=0 size=1
>>
>> libbpf: -- END LOG --
>> libbpf: failed to load program 'test1'
>> libbpf: failed to load object './test_pkt_md_access.o'
>> Summary: 29 PASSED, 1 FAILED
>> [root@s8360046 bpf]#
>>
>> This is caused by a byte endianess issue. S390x is a big endian
>> architecture. Pointer access to the lowest byte or halfword of a
>> four byte value need to add an offset.
>> On little endian architectures this offset is not needed.
>>
>> Fix this and use the same approach as the originator used for other files
>> (for example test_verifier.c) in his original commit.
>>
>> With this fix the test program test_progs succeeds on s390x:
>> [root@s8360046 bpf]# ./test_progs
>> test_pkt_access:PASS:ipv4 236 nsec
>> test_pkt_access:PASS:ipv6 217 nsec
>> test_xdp:PASS:ipv4 3624 nsec
>> test_xdp:PASS:ipv6 1722 nsec
>> test_l4lb:PASS:ipv4 926 nsec
>> test_l4lb:PASS:ipv6 1322 nsec
>> test_tcp_estats:PASS: 0 nsec
>> test_bpf_obj_id:PASS:get-fd-by-notexist-prog-id 0 nsec
>> test_bpf_obj_id:PASS:get-fd-by-notexist-map-id 0 nsec
>> test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
>> test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
>> test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
>> test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
>> test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
>> test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
>> test_bpf_obj_id:PASS:check total prog id found by get_next_id 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
>> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
>> test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
>> test_bpf_obj_id:PASS:check total map id found by get_next_id 0 nsec
>> test_pkt_md_access:PASS: 277 nsec
>> Summary: 30 PASSED, 0 FAILED
>> [root@s8360046 bpf]#
>>
>> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
>> ---
>> tools/testing/selftests/bpf/test_pkt_md_access.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/test_pkt_md_access.c b/tools/testing/selftests/bpf/test_pkt_md_access.c
>> index 71729d4..8393ced 100644
>> --- a/tools/testing/selftests/bpf/test_pkt_md_access.c
>> +++ b/tools/testing/selftests/bpf/test_pkt_md_access.c
>> @@ -12,12 +12,23 @@
>>
>> int _version SEC("version") = 1;
>>
>> +#ifdef __LITTLE_ENDIAN
One more thing I noticed, could you do above check with:
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
This is basically the version that clang exposes, so that
this could then properly be compiled with both, bpfel/bpfeb
targets, see also 78a5a93c1eeb ("bpf, tests: fix endianness
selection") for some more context.
Thanks again,
Daniel
>> #define TEST_FIELD(TYPE, FIELD, MASK) \
>> { \
>> TYPE tmp = *(volatile TYPE *)&skb->FIELD; \
>> if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK)) \
>> return TC_ACT_SHOT; \
>> }
>> +#else
>> +#define TEST_FIELD_OFFSET(a, b) ((sizeof(a) - sizeof(b)) / sizeof(b))
>> +#define TEST_FIELD(TYPE, FIELD, MASK) \
>> + { \
>> + TYPE tmp = *((volatile TYPE *)&skb->FIELD + \
>> + TEST_FIELD_OFFSET(skb->FIELD, TYPE)); \
>> + if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK)) \
>> + return TC_ACT_SHOT; \
>> + }
>> +#endif
>>
>> SEC("test1")
>> int process(struct __sk_buff *skb)
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web