Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751958AbdHDTGR (ORCPT ); Fri, 4 Aug 2017 15:06:17 -0400 Received: from www62.your-server.de ([213.133.104.62]:34798 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751281AbdHDTGQ (ORCPT ); Fri, 4 Aug 2017 15:06:16 -0400 Message-ID: <5984C5A4.8040301@iogearbox.net> Date: Fri, 04 Aug 2017 21:06:12 +0200 From: Daniel Borkmann User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Thomas Richter , ast@kernel.org, linux-kernel@vger.kernel.org CC: schwidefsky@de.ibm.com, brueckner@linux.vnet.ibm.com Subject: Re: [PATCH] bpf: fix selftest/bpf/test_pkt_md_access on s390x References: <20170804064027.69086-1-tmricht@linux.vnet.ibm.com> In-Reply-To: <20170804064027.69086-1-tmricht@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3988 Lines: 107 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 > --- > 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) >