Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752348AbdHDUeZ (ORCPT ); Fri, 4 Aug 2017 16:34:25 -0400 Received: from www62.your-server.de ([213.133.104.62]:48363 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752104AbdHDUeY (ORCPT ); Fri, 4 Aug 2017 16:34:24 -0400 Message-ID: <5984DA4D.1000204@iogearbox.net> Date: Fri, 04 Aug 2017 22:34:21 +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> <5984C5A4.8040301@iogearbox.net> In-Reply-To: <5984C5A4.8040301@iogearbox.net> 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: 4679 Lines: 120 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 >> --- >> 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)