* [PATCH bpf-next 0/3] Fix three endianness issues in test_progs
@ 2020-09-09 23:24 Ilya Leoshkevich
2020-09-09 23:24 ` [PATCH bpf-next 1/3] selftests/bpf: Fix endianness issue in test_sockopt_sk Ilya Leoshkevich
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Ilya Leoshkevich @ 2020-09-09 23:24 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich
This series fixes three test_progs failures on s390, all of which occur
because of endianness issues.
Ilya Leoshkevich (3):
selftests/bpf: Fix endianness issue in test_sockopt_sk
selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access
selftests/bpf: Fix endianness issue in sk_assign
.../selftests/bpf/prog_tests/sk_assign.c | 2 +-
.../selftests/bpf/prog_tests/sockopt_sk.c | 2 +-
.../selftests/bpf/progs/test_sk_lookup.c | 264 ++++++++++--------
3 files changed, 151 insertions(+), 117 deletions(-)
--
2.25.4
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH bpf-next 1/3] selftests/bpf: Fix endianness issue in test_sockopt_sk 2020-09-09 23:24 [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Ilya Leoshkevich @ 2020-09-09 23:24 ` Ilya Leoshkevich 2020-09-10 16:49 ` Andrii Nakryiko 2020-09-09 23:24 ` [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access Ilya Leoshkevich ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Ilya Leoshkevich @ 2020-09-09 23:24 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich getsetsockopt() calls getsockopt() with optlen == 1, but then checks the resulting int. It is ok on little endian, but not on big endian. Fix by checking char instead. Fixes: 8a027dc0 ("selftests/bpf: add sockopt test that exercises sk helpers") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/prog_tests/sockopt_sk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c index 5f54c6aec7f0..ba4da50987d6 100644 --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c @@ -45,7 +45,7 @@ static int getsetsockopt(void) goto err; } - if (*(int *)big_buf != 0x08) { + if (*big_buf != 0x08) { log_err("Unexpected getsockopt(IP_TOS) optval 0x%x != 0x08", *(int *)big_buf); goto err; -- 2.25.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] selftests/bpf: Fix endianness issue in test_sockopt_sk 2020-09-09 23:24 ` [PATCH bpf-next 1/3] selftests/bpf: Fix endianness issue in test_sockopt_sk Ilya Leoshkevich @ 2020-09-10 16:49 ` Andrii Nakryiko 0 siblings, 0 replies; 12+ messages in thread From: Andrii Nakryiko @ 2020-09-10 16:49 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Wed, Sep 9, 2020 at 7:52 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > getsetsockopt() calls getsockopt() with optlen == 1, but then checks > the resulting int. It is ok on little endian, but not on big endian. > > Fix by checking char instead. > > Fixes: 8a027dc0 ("selftests/bpf: add sockopt test that exercises sk helpers") > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/testing/selftests/bpf/prog_tests/sockopt_sk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > index 5f54c6aec7f0..ba4da50987d6 100644 > --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c > @@ -45,7 +45,7 @@ static int getsetsockopt(void) > goto err; > } > > - if (*(int *)big_buf != 0x08) { > + if (*big_buf != 0x08) { > log_err("Unexpected getsockopt(IP_TOS) optval 0x%x != 0x08", > *(int *)big_buf); (int)*big_buf here? > goto err; > -- > 2.25.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access 2020-09-09 23:24 [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Ilya Leoshkevich 2020-09-09 23:24 ` [PATCH bpf-next 1/3] selftests/bpf: Fix endianness issue in test_sockopt_sk Ilya Leoshkevich @ 2020-09-09 23:24 ` Ilya Leoshkevich 2020-09-10 16:55 ` Andrii Nakryiko 2020-09-24 9:24 ` Jakub Sitnicki 2020-09-09 23:24 ` [PATCH bpf-next 3/3] selftests/bpf: Fix endianness issue in sk_assign Ilya Leoshkevich 2020-09-10 16:58 ` [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Andrii Nakryiko 3 siblings, 2 replies; 12+ messages in thread From: Ilya Leoshkevich @ 2020-09-09 23:24 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich This test makes a lot of narrow load checks while assuming little endian architecture, and therefore fails on s390. Fix by introducing LSB and LSW macros and using them to perform narrow loads. Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- .../selftests/bpf/progs/test_sk_lookup.c | 264 ++++++++++-------- 1 file changed, 149 insertions(+), 115 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/test_sk_lookup.c b/tools/testing/selftests/bpf/progs/test_sk_lookup.c index bbf8296f4d66..94e6d370967b 100644 --- a/tools/testing/selftests/bpf/progs/test_sk_lookup.c +++ b/tools/testing/selftests/bpf/progs/test_sk_lookup.c @@ -19,6 +19,17 @@ #define IP6(aaaa, bbbb, cccc, dddd) \ { bpf_htonl(aaaa), bpf_htonl(bbbb), bpf_htonl(cccc), bpf_htonl(dddd) } +/* Macros for least-significant byte and word accesses. */ +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ +#define LSE_INDEX(index, size) (index) +#else +#define LSE_INDEX(index, size) ((size) - (index) - 1) +#endif +#define LSB(value, index) \ + (((__u8 *)&(value))[LSE_INDEX((index), sizeof(value))]) +#define LSW(value, index) \ + (((__u16 *)&(value))[LSE_INDEX((index), sizeof(value) / 2)]) + #define MAX_SOCKS 32 struct { @@ -369,171 +380,194 @@ int ctx_narrow_access(struct bpf_sk_lookup *ctx) { struct bpf_sock *sk; int err, family; - __u16 *half; - __u8 *byte; bool v4; v4 = (ctx->family == AF_INET); /* Narrow loads from family field */ - byte = (__u8 *)&ctx->family; - half = (__u16 *)&ctx->family; - if (byte[0] != (v4 ? AF_INET : AF_INET6) || - byte[1] != 0 || byte[2] != 0 || byte[3] != 0) + if (LSB(ctx->family, 0) != (v4 ? AF_INET : AF_INET6) || + LSB(ctx->family, 1) != 0 || LSB(ctx->family, 2) != 0 || + LSB(ctx->family, 3) != 0) return SK_DROP; - if (half[0] != (v4 ? AF_INET : AF_INET6)) + if (LSW(ctx->family, 0) != (v4 ? AF_INET : AF_INET6)) return SK_DROP; - byte = (__u8 *)&ctx->protocol; - if (byte[0] != IPPROTO_TCP || - byte[1] != 0 || byte[2] != 0 || byte[3] != 0) + /* Narrow loads from protocol field */ + if (LSB(ctx->protocol, 0) != IPPROTO_TCP || + LSB(ctx->protocol, 1) != 0 || LSB(ctx->protocol, 2) != 0 || + LSB(ctx->protocol, 3) != 0) return SK_DROP; - half = (__u16 *)&ctx->protocol; - if (half[0] != IPPROTO_TCP) + if (LSW(ctx->protocol, 0) != IPPROTO_TCP) return SK_DROP; /* Narrow loads from remote_port field. Expect non-0 value. */ - byte = (__u8 *)&ctx->remote_port; - if (byte[0] == 0 && byte[1] == 0 && byte[2] == 0 && byte[3] == 0) + if (LSB(ctx->remote_port, 0) == 0 && LSB(ctx->remote_port, 1) == 0 && + LSB(ctx->remote_port, 2) == 0 && LSB(ctx->remote_port, 3) == 0) return SK_DROP; - half = (__u16 *)&ctx->remote_port; - if (half[0] == 0) + if (LSW(ctx->remote_port, 0) == 0) return SK_DROP; /* Narrow loads from local_port field. Expect DST_PORT. */ - byte = (__u8 *)&ctx->local_port; - if (byte[0] != ((DST_PORT >> 0) & 0xff) || - byte[1] != ((DST_PORT >> 8) & 0xff) || - byte[2] != 0 || byte[3] != 0) + if (LSB(ctx->local_port, 0) != ((DST_PORT >> 0) & 0xff) || + LSB(ctx->local_port, 1) != ((DST_PORT >> 8) & 0xff) || + LSB(ctx->local_port, 2) != 0 || LSB(ctx->local_port, 3) != 0) return SK_DROP; - half = (__u16 *)&ctx->local_port; - if (half[0] != DST_PORT) + if (LSW(ctx->local_port, 0) != DST_PORT) return SK_DROP; /* Narrow loads from IPv4 fields */ if (v4) { /* Expect non-0.0.0.0 in remote_ip4 */ - byte = (__u8 *)&ctx->remote_ip4; - if (byte[0] == 0 && byte[1] == 0 && - byte[2] == 0 && byte[3] == 0) + if (LSB(ctx->remote_ip4, 0) == 0 && + LSB(ctx->remote_ip4, 1) == 0 && + LSB(ctx->remote_ip4, 2) == 0 && + LSB(ctx->remote_ip4, 3) == 0) return SK_DROP; - half = (__u16 *)&ctx->remote_ip4; - if (half[0] == 0 && half[1] == 0) + if (LSW(ctx->remote_ip4, 0) == 0 && + LSW(ctx->remote_ip4, 1) == 0) return SK_DROP; /* Expect DST_IP4 in local_ip4 */ - byte = (__u8 *)&ctx->local_ip4; - if (byte[0] != ((DST_IP4 >> 0) & 0xff) || - byte[1] != ((DST_IP4 >> 8) & 0xff) || - byte[2] != ((DST_IP4 >> 16) & 0xff) || - byte[3] != ((DST_IP4 >> 24) & 0xff)) + if (LSB(ctx->local_ip4, 0) != ((DST_IP4 >> 0) & 0xff) || + LSB(ctx->local_ip4, 1) != ((DST_IP4 >> 8) & 0xff) || + LSB(ctx->local_ip4, 2) != ((DST_IP4 >> 16) & 0xff) || + LSB(ctx->local_ip4, 3) != ((DST_IP4 >> 24) & 0xff)) return SK_DROP; - half = (__u16 *)&ctx->local_ip4; - if (half[0] != ((DST_IP4 >> 0) & 0xffff) || - half[1] != ((DST_IP4 >> 16) & 0xffff)) + if (LSW(ctx->local_ip4, 0) != ((DST_IP4 >> 0) & 0xffff) || + LSW(ctx->local_ip4, 1) != ((DST_IP4 >> 16) & 0xffff)) return SK_DROP; } else { /* Expect 0.0.0.0 IPs when family != AF_INET */ - byte = (__u8 *)&ctx->remote_ip4; - if (byte[0] != 0 || byte[1] != 0 && - byte[2] != 0 || byte[3] != 0) + if (LSB(ctx->remote_ip4, 0) != 0 || + LSB(ctx->remote_ip4, 1) != 0 || + LSB(ctx->remote_ip4, 2) != 0 || + LSB(ctx->remote_ip4, 3) != 0) return SK_DROP; - half = (__u16 *)&ctx->remote_ip4; - if (half[0] != 0 || half[1] != 0) + if (LSW(ctx->remote_ip4, 0) != 0 || + LSW(ctx->remote_ip4, 1) != 0) return SK_DROP; - byte = (__u8 *)&ctx->local_ip4; - if (byte[0] != 0 || byte[1] != 0 && - byte[2] != 0 || byte[3] != 0) + if (LSB(ctx->local_ip4, 0) != 0 || + LSB(ctx->local_ip4, 1) != 0 || + LSB(ctx->local_ip4, 2) != 0 || LSB(ctx->local_ip4, 3) != 0) return SK_DROP; - half = (__u16 *)&ctx->local_ip4; - if (half[0] != 0 || half[1] != 0) + if (LSW(ctx->local_ip4, 0) != 0 || LSW(ctx->local_ip4, 1) != 0) return SK_DROP; } /* Narrow loads from IPv6 fields */ if (!v4) { - /* Expenct non-:: IP in remote_ip6 */ - byte = (__u8 *)&ctx->remote_ip6; - if (byte[0] == 0 && byte[1] == 0 && - byte[2] == 0 && byte[3] == 0 && - byte[4] == 0 && byte[5] == 0 && - byte[6] == 0 && byte[7] == 0 && - byte[8] == 0 && byte[9] == 0 && - byte[10] == 0 && byte[11] == 0 && - byte[12] == 0 && byte[13] == 0 && - byte[14] == 0 && byte[15] == 0) + /* Expect non-:: IP in remote_ip6 */ + if (LSB(ctx->remote_ip6[0], 0) == 0 && + LSB(ctx->remote_ip6[0], 1) == 0 && + LSB(ctx->remote_ip6[0], 2) == 0 && + LSB(ctx->remote_ip6[0], 3) == 0 && + LSB(ctx->remote_ip6[1], 0) == 0 && + LSB(ctx->remote_ip6[1], 1) == 0 && + LSB(ctx->remote_ip6[1], 2) == 0 && + LSB(ctx->remote_ip6[1], 3) == 0 && + LSB(ctx->remote_ip6[2], 0) == 0 && + LSB(ctx->remote_ip6[2], 1) == 0 && + LSB(ctx->remote_ip6[2], 2) == 0 && + LSB(ctx->remote_ip6[2], 3) == 0 && + LSB(ctx->remote_ip6[3], 0) == 0 && + LSB(ctx->remote_ip6[3], 1) == 0 && + LSB(ctx->remote_ip6[3], 2) == 0 && + LSB(ctx->remote_ip6[3], 3) == 0) return SK_DROP; - half = (__u16 *)&ctx->remote_ip6; - if (half[0] == 0 && half[1] == 0 && - half[2] == 0 && half[3] == 0 && - half[4] == 0 && half[5] == 0 && - half[6] == 0 && half[7] == 0) + if (LSW(ctx->remote_ip6[0], 0) == 0 && + LSW(ctx->remote_ip6[0], 1) == 0 && + LSW(ctx->remote_ip6[1], 0) == 0 && + LSW(ctx->remote_ip6[1], 1) == 0 && + LSW(ctx->remote_ip6[2], 0) == 0 && + LSW(ctx->remote_ip6[2], 1) == 0 && + LSW(ctx->remote_ip6[3], 0) == 0 && + LSW(ctx->remote_ip6[3], 1) == 0) return SK_DROP; - /* Expect DST_IP6 in local_ip6 */ - byte = (__u8 *)&ctx->local_ip6; - if (byte[0] != ((DST_IP6[0] >> 0) & 0xff) || - byte[1] != ((DST_IP6[0] >> 8) & 0xff) || - byte[2] != ((DST_IP6[0] >> 16) & 0xff) || - byte[3] != ((DST_IP6[0] >> 24) & 0xff) || - byte[4] != ((DST_IP6[1] >> 0) & 0xff) || - byte[5] != ((DST_IP6[1] >> 8) & 0xff) || - byte[6] != ((DST_IP6[1] >> 16) & 0xff) || - byte[7] != ((DST_IP6[1] >> 24) & 0xff) || - byte[8] != ((DST_IP6[2] >> 0) & 0xff) || - byte[9] != ((DST_IP6[2] >> 8) & 0xff) || - byte[10] != ((DST_IP6[2] >> 16) & 0xff) || - byte[11] != ((DST_IP6[2] >> 24) & 0xff) || - byte[12] != ((DST_IP6[3] >> 0) & 0xff) || - byte[13] != ((DST_IP6[3] >> 8) & 0xff) || - byte[14] != ((DST_IP6[3] >> 16) & 0xff) || - byte[15] != ((DST_IP6[3] >> 24) & 0xff)) + if (LSB(ctx->local_ip6[0], 0) != ((DST_IP6[0] >> 0) & 0xff) || + LSB(ctx->local_ip6[0], 1) != ((DST_IP6[0] >> 8) & 0xff) || + LSB(ctx->local_ip6[0], 2) != ((DST_IP6[0] >> 16) & 0xff) || + LSB(ctx->local_ip6[0], 3) != ((DST_IP6[0] >> 24) & 0xff) || + LSB(ctx->local_ip6[1], 0) != ((DST_IP6[1] >> 0) & 0xff) || + LSB(ctx->local_ip6[1], 1) != ((DST_IP6[1] >> 8) & 0xff) || + LSB(ctx->local_ip6[1], 2) != ((DST_IP6[1] >> 16) & 0xff) || + LSB(ctx->local_ip6[1], 3) != ((DST_IP6[1] >> 24) & 0xff) || + LSB(ctx->local_ip6[2], 0) != ((DST_IP6[2] >> 0) & 0xff) || + LSB(ctx->local_ip6[2], 1) != ((DST_IP6[2] >> 8) & 0xff) || + LSB(ctx->local_ip6[2], 2) != ((DST_IP6[2] >> 16) & 0xff) || + LSB(ctx->local_ip6[2], 3) != ((DST_IP6[2] >> 24) & 0xff) || + LSB(ctx->local_ip6[3], 0) != ((DST_IP6[3] >> 0) & 0xff) || + LSB(ctx->local_ip6[3], 1) != ((DST_IP6[3] >> 8) & 0xff) || + LSB(ctx->local_ip6[3], 2) != ((DST_IP6[3] >> 16) & 0xff) || + LSB(ctx->local_ip6[3], 3) != ((DST_IP6[3] >> 24) & 0xff)) return SK_DROP; - half = (__u16 *)&ctx->local_ip6; - if (half[0] != ((DST_IP6[0] >> 0) & 0xffff) || - half[1] != ((DST_IP6[0] >> 16) & 0xffff) || - half[2] != ((DST_IP6[1] >> 0) & 0xffff) || - half[3] != ((DST_IP6[1] >> 16) & 0xffff) || - half[4] != ((DST_IP6[2] >> 0) & 0xffff) || - half[5] != ((DST_IP6[2] >> 16) & 0xffff) || - half[6] != ((DST_IP6[3] >> 0) & 0xffff) || - half[7] != ((DST_IP6[3] >> 16) & 0xffff)) + if (LSW(ctx->local_ip6[0], 0) != ((DST_IP6[0] >> 0) & 0xffff) || + LSW(ctx->local_ip6[0], 1) != + ((DST_IP6[0] >> 16) & 0xffff) || + LSW(ctx->local_ip6[1], 0) != ((DST_IP6[1] >> 0) & 0xffff) || + LSW(ctx->local_ip6[1], 1) != + ((DST_IP6[1] >> 16) & 0xffff) || + LSW(ctx->local_ip6[2], 0) != ((DST_IP6[2] >> 0) & 0xffff) || + LSW(ctx->local_ip6[2], 1) != + ((DST_IP6[2] >> 16) & 0xffff) || + LSW(ctx->local_ip6[3], 0) != ((DST_IP6[3] >> 0) & 0xffff) || + LSW(ctx->local_ip6[3], 1) != ((DST_IP6[3] >> 16) & 0xffff)) return SK_DROP; } else { /* Expect :: IPs when family != AF_INET6 */ - byte = (__u8 *)&ctx->remote_ip6; - if (byte[0] != 0 || byte[1] != 0 || - byte[2] != 0 || byte[3] != 0 || - byte[4] != 0 || byte[5] != 0 || - byte[6] != 0 || byte[7] != 0 || - byte[8] != 0 || byte[9] != 0 || - byte[10] != 0 || byte[11] != 0 || - byte[12] != 0 || byte[13] != 0 || - byte[14] != 0 || byte[15] != 0) + if (LSB(ctx->remote_ip6[0], 0) != 0 || + LSB(ctx->remote_ip6[0], 1) != 0 || + LSB(ctx->remote_ip6[0], 2) != 0 || + LSB(ctx->remote_ip6[0], 3) != 0 || + LSB(ctx->remote_ip6[1], 0) != 0 || + LSB(ctx->remote_ip6[1], 1) != 0 || + LSB(ctx->remote_ip6[1], 2) != 0 || + LSB(ctx->remote_ip6[1], 3) != 0 || + LSB(ctx->remote_ip6[2], 0) != 0 || + LSB(ctx->remote_ip6[2], 1) != 0 || + LSB(ctx->remote_ip6[2], 2) != 0 || + LSB(ctx->remote_ip6[2], 3) != 0 || + LSB(ctx->remote_ip6[3], 0) != 0 || + LSB(ctx->remote_ip6[3], 1) != 0 || + LSB(ctx->remote_ip6[3], 2) != 0 || + LSB(ctx->remote_ip6[3], 3) != 0) return SK_DROP; - half = (__u16 *)&ctx->remote_ip6; - if (half[0] != 0 || half[1] != 0 || - half[2] != 0 || half[3] != 0 || - half[4] != 0 || half[5] != 0 || - half[6] != 0 || half[7] != 0) + if (LSW(ctx->remote_ip6[0], 0) != 0 || + LSW(ctx->remote_ip6[0], 1) != 0 || + LSW(ctx->remote_ip6[1], 0) != 0 || + LSW(ctx->remote_ip6[1], 1) != 0 || + LSW(ctx->remote_ip6[2], 0) != 0 || + LSW(ctx->remote_ip6[2], 1) != 0 || + LSW(ctx->remote_ip6[3], 0) != 0 || + LSW(ctx->remote_ip6[3], 1) != 0) return SK_DROP; - byte = (__u8 *)&ctx->local_ip6; - if (byte[0] != 0 || byte[1] != 0 || - byte[2] != 0 || byte[3] != 0 || - byte[4] != 0 || byte[5] != 0 || - byte[6] != 0 || byte[7] != 0 || - byte[8] != 0 || byte[9] != 0 || - byte[10] != 0 || byte[11] != 0 || - byte[12] != 0 || byte[13] != 0 || - byte[14] != 0 || byte[15] != 0) + if (LSB(ctx->local_ip6[0], 0) != 0 || + LSB(ctx->local_ip6[0], 1) != 0 || + LSB(ctx->local_ip6[0], 2) != 0 || + LSB(ctx->local_ip6[0], 3) != 0 || + LSB(ctx->local_ip6[1], 0) != 0 || + LSB(ctx->local_ip6[1], 1) != 0 || + LSB(ctx->local_ip6[1], 2) != 0 || + LSB(ctx->local_ip6[1], 3) != 0 || + LSB(ctx->local_ip6[2], 0) != 0 || + LSB(ctx->local_ip6[2], 1) != 0 || + LSB(ctx->local_ip6[2], 2) != 0 || + LSB(ctx->local_ip6[2], 3) != 0 || + LSB(ctx->local_ip6[3], 0) != 0 || + LSB(ctx->local_ip6[3], 1) != 0 || + LSB(ctx->local_ip6[3], 2) != 0 || + LSB(ctx->local_ip6[3], 3) != 0) return SK_DROP; - half = (__u16 *)&ctx->local_ip6; - if (half[0] != 0 || half[1] != 0 || - half[2] != 0 || half[3] != 0 || - half[4] != 0 || half[5] != 0 || - half[6] != 0 || half[7] != 0) + if (LSW(ctx->remote_ip6[0], 0) != 0 || + LSW(ctx->remote_ip6[0], 1) != 0 || + LSW(ctx->remote_ip6[1], 0) != 0 || + LSW(ctx->remote_ip6[1], 1) != 0 || + LSW(ctx->remote_ip6[2], 0) != 0 || + LSW(ctx->remote_ip6[2], 1) != 0 || + LSW(ctx->remote_ip6[3], 0) != 0 || + LSW(ctx->remote_ip6[3], 1) != 0) return SK_DROP; } -- 2.25.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access 2020-09-09 23:24 ` [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access Ilya Leoshkevich @ 2020-09-10 16:55 ` Andrii Nakryiko 2020-09-23 21:12 ` Ilya Leoshkevich 2020-09-24 9:24 ` Jakub Sitnicki 1 sibling, 1 reply; 12+ messages in thread From: Andrii Nakryiko @ 2020-09-10 16:55 UTC (permalink / raw) To: Ilya Leoshkevich, Jakub Sitnicki Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Wed, Sep 9, 2020 at 6:59 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > This test makes a lot of narrow load checks while assuming little > endian architecture, and therefore fails on s390. > > Fix by introducing LSB and LSW macros and using them to perform narrow > loads. > > Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point") > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- Jakub, Can you please help review this to make sure no error accidentally slipped in? > .../selftests/bpf/progs/test_sk_lookup.c | 264 ++++++++++-------- > 1 file changed, 149 insertions(+), 115 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/test_sk_lookup.c b/tools/testing/selftests/bpf/progs/test_sk_lookup.c > index bbf8296f4d66..94e6d370967b 100644 > --- a/tools/testing/selftests/bpf/progs/test_sk_lookup.c > +++ b/tools/testing/selftests/bpf/progs/test_sk_lookup.c > @@ -19,6 +19,17 @@ > #define IP6(aaaa, bbbb, cccc, dddd) \ > { bpf_htonl(aaaa), bpf_htonl(bbbb), bpf_htonl(cccc), bpf_htonl(dddd) } > > +/* Macros for least-significant byte and word accesses. */ > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ > +#define LSE_INDEX(index, size) (index) > +#else > +#define LSE_INDEX(index, size) ((size) - (index) - 1) > +#endif > +#define LSB(value, index) \ > + (((__u8 *)&(value))[LSE_INDEX((index), sizeof(value))]) > +#define LSW(value, index) \ > + (((__u16 *)&(value))[LSE_INDEX((index), sizeof(value) / 2)]) > + > #define MAX_SOCKS 32 > > struct { > @@ -369,171 +380,194 @@ int ctx_narrow_access(struct bpf_sk_lookup *ctx) > { > struct bpf_sock *sk; > int err, family; > - __u16 *half; > - __u8 *byte; > bool v4; > > v4 = (ctx->family == AF_INET); > > /* Narrow loads from family field */ > - byte = (__u8 *)&ctx->family; > - half = (__u16 *)&ctx->family; > - if (byte[0] != (v4 ? AF_INET : AF_INET6) || > - byte[1] != 0 || byte[2] != 0 || byte[3] != 0) > + if (LSB(ctx->family, 0) != (v4 ? AF_INET : AF_INET6) || > + LSB(ctx->family, 1) != 0 || LSB(ctx->family, 2) != 0 || > + LSB(ctx->family, 3) != 0) > return SK_DROP; > - if (half[0] != (v4 ? AF_INET : AF_INET6)) > + if (LSW(ctx->family, 0) != (v4 ? AF_INET : AF_INET6)) > return SK_DROP; > > - byte = (__u8 *)&ctx->protocol; > - if (byte[0] != IPPROTO_TCP || > - byte[1] != 0 || byte[2] != 0 || byte[3] != 0) > + /* Narrow loads from protocol field */ > + if (LSB(ctx->protocol, 0) != IPPROTO_TCP || > + LSB(ctx->protocol, 1) != 0 || LSB(ctx->protocol, 2) != 0 || > + LSB(ctx->protocol, 3) != 0) > return SK_DROP; > - half = (__u16 *)&ctx->protocol; > - if (half[0] != IPPROTO_TCP) > + if (LSW(ctx->protocol, 0) != IPPROTO_TCP) > return SK_DROP; > > /* Narrow loads from remote_port field. Expect non-0 value. */ > - byte = (__u8 *)&ctx->remote_port; > - if (byte[0] == 0 && byte[1] == 0 && byte[2] == 0 && byte[3] == 0) > + if (LSB(ctx->remote_port, 0) == 0 && LSB(ctx->remote_port, 1) == 0 && > + LSB(ctx->remote_port, 2) == 0 && LSB(ctx->remote_port, 3) == 0) > return SK_DROP; > - half = (__u16 *)&ctx->remote_port; > - if (half[0] == 0) > + if (LSW(ctx->remote_port, 0) == 0) > return SK_DROP; > > /* Narrow loads from local_port field. Expect DST_PORT. */ > - byte = (__u8 *)&ctx->local_port; > - if (byte[0] != ((DST_PORT >> 0) & 0xff) || > - byte[1] != ((DST_PORT >> 8) & 0xff) || > - byte[2] != 0 || byte[3] != 0) > + if (LSB(ctx->local_port, 0) != ((DST_PORT >> 0) & 0xff) || > + LSB(ctx->local_port, 1) != ((DST_PORT >> 8) & 0xff) || > + LSB(ctx->local_port, 2) != 0 || LSB(ctx->local_port, 3) != 0) > return SK_DROP; > - half = (__u16 *)&ctx->local_port; > - if (half[0] != DST_PORT) > + if (LSW(ctx->local_port, 0) != DST_PORT) > return SK_DROP; > > /* Narrow loads from IPv4 fields */ > if (v4) { > /* Expect non-0.0.0.0 in remote_ip4 */ > - byte = (__u8 *)&ctx->remote_ip4; > - if (byte[0] == 0 && byte[1] == 0 && > - byte[2] == 0 && byte[3] == 0) > + if (LSB(ctx->remote_ip4, 0) == 0 && > + LSB(ctx->remote_ip4, 1) == 0 && > + LSB(ctx->remote_ip4, 2) == 0 && > + LSB(ctx->remote_ip4, 3) == 0) > return SK_DROP; > - half = (__u16 *)&ctx->remote_ip4; > - if (half[0] == 0 && half[1] == 0) > + if (LSW(ctx->remote_ip4, 0) == 0 && > + LSW(ctx->remote_ip4, 1) == 0) > return SK_DROP; > > /* Expect DST_IP4 in local_ip4 */ > - byte = (__u8 *)&ctx->local_ip4; > - if (byte[0] != ((DST_IP4 >> 0) & 0xff) || > - byte[1] != ((DST_IP4 >> 8) & 0xff) || > - byte[2] != ((DST_IP4 >> 16) & 0xff) || > - byte[3] != ((DST_IP4 >> 24) & 0xff)) > + if (LSB(ctx->local_ip4, 0) != ((DST_IP4 >> 0) & 0xff) || > + LSB(ctx->local_ip4, 1) != ((DST_IP4 >> 8) & 0xff) || > + LSB(ctx->local_ip4, 2) != ((DST_IP4 >> 16) & 0xff) || > + LSB(ctx->local_ip4, 3) != ((DST_IP4 >> 24) & 0xff)) > return SK_DROP; > - half = (__u16 *)&ctx->local_ip4; > - if (half[0] != ((DST_IP4 >> 0) & 0xffff) || > - half[1] != ((DST_IP4 >> 16) & 0xffff)) > + if (LSW(ctx->local_ip4, 0) != ((DST_IP4 >> 0) & 0xffff) || > + LSW(ctx->local_ip4, 1) != ((DST_IP4 >> 16) & 0xffff)) > return SK_DROP; > } else { > /* Expect 0.0.0.0 IPs when family != AF_INET */ > - byte = (__u8 *)&ctx->remote_ip4; > - if (byte[0] != 0 || byte[1] != 0 && > - byte[2] != 0 || byte[3] != 0) > + if (LSB(ctx->remote_ip4, 0) != 0 || > + LSB(ctx->remote_ip4, 1) != 0 || > + LSB(ctx->remote_ip4, 2) != 0 || > + LSB(ctx->remote_ip4, 3) != 0) > return SK_DROP; > - half = (__u16 *)&ctx->remote_ip4; > - if (half[0] != 0 || half[1] != 0) > + if (LSW(ctx->remote_ip4, 0) != 0 || > + LSW(ctx->remote_ip4, 1) != 0) > return SK_DROP; > > - byte = (__u8 *)&ctx->local_ip4; > - if (byte[0] != 0 || byte[1] != 0 && > - byte[2] != 0 || byte[3] != 0) > + if (LSB(ctx->local_ip4, 0) != 0 || > + LSB(ctx->local_ip4, 1) != 0 || > + LSB(ctx->local_ip4, 2) != 0 || LSB(ctx->local_ip4, 3) != 0) > return SK_DROP; > - half = (__u16 *)&ctx->local_ip4; > - if (half[0] != 0 || half[1] != 0) > + if (LSW(ctx->local_ip4, 0) != 0 || LSW(ctx->local_ip4, 1) != 0) > return SK_DROP; > } > > /* Narrow loads from IPv6 fields */ > if (!v4) { > - /* Expenct non-:: IP in remote_ip6 */ > - byte = (__u8 *)&ctx->remote_ip6; > - if (byte[0] == 0 && byte[1] == 0 && > - byte[2] == 0 && byte[3] == 0 && > - byte[4] == 0 && byte[5] == 0 && > - byte[6] == 0 && byte[7] == 0 && > - byte[8] == 0 && byte[9] == 0 && > - byte[10] == 0 && byte[11] == 0 && > - byte[12] == 0 && byte[13] == 0 && > - byte[14] == 0 && byte[15] == 0) > + /* Expect non-:: IP in remote_ip6 */ > + if (LSB(ctx->remote_ip6[0], 0) == 0 && > + LSB(ctx->remote_ip6[0], 1) == 0 && > + LSB(ctx->remote_ip6[0], 2) == 0 && > + LSB(ctx->remote_ip6[0], 3) == 0 && > + LSB(ctx->remote_ip6[1], 0) == 0 && > + LSB(ctx->remote_ip6[1], 1) == 0 && > + LSB(ctx->remote_ip6[1], 2) == 0 && > + LSB(ctx->remote_ip6[1], 3) == 0 && > + LSB(ctx->remote_ip6[2], 0) == 0 && > + LSB(ctx->remote_ip6[2], 1) == 0 && > + LSB(ctx->remote_ip6[2], 2) == 0 && > + LSB(ctx->remote_ip6[2], 3) == 0 && > + LSB(ctx->remote_ip6[3], 0) == 0 && > + LSB(ctx->remote_ip6[3], 1) == 0 && > + LSB(ctx->remote_ip6[3], 2) == 0 && > + LSB(ctx->remote_ip6[3], 3) == 0) > return SK_DROP; > - half = (__u16 *)&ctx->remote_ip6; > - if (half[0] == 0 && half[1] == 0 && > - half[2] == 0 && half[3] == 0 && > - half[4] == 0 && half[5] == 0 && > - half[6] == 0 && half[7] == 0) > + if (LSW(ctx->remote_ip6[0], 0) == 0 && > + LSW(ctx->remote_ip6[0], 1) == 0 && > + LSW(ctx->remote_ip6[1], 0) == 0 && > + LSW(ctx->remote_ip6[1], 1) == 0 && > + LSW(ctx->remote_ip6[2], 0) == 0 && > + LSW(ctx->remote_ip6[2], 1) == 0 && > + LSW(ctx->remote_ip6[3], 0) == 0 && > + LSW(ctx->remote_ip6[3], 1) == 0) > return SK_DROP; > - > /* Expect DST_IP6 in local_ip6 */ > - byte = (__u8 *)&ctx->local_ip6; > - if (byte[0] != ((DST_IP6[0] >> 0) & 0xff) || > - byte[1] != ((DST_IP6[0] >> 8) & 0xff) || > - byte[2] != ((DST_IP6[0] >> 16) & 0xff) || > - byte[3] != ((DST_IP6[0] >> 24) & 0xff) || > - byte[4] != ((DST_IP6[1] >> 0) & 0xff) || > - byte[5] != ((DST_IP6[1] >> 8) & 0xff) || > - byte[6] != ((DST_IP6[1] >> 16) & 0xff) || > - byte[7] != ((DST_IP6[1] >> 24) & 0xff) || > - byte[8] != ((DST_IP6[2] >> 0) & 0xff) || > - byte[9] != ((DST_IP6[2] >> 8) & 0xff) || > - byte[10] != ((DST_IP6[2] >> 16) & 0xff) || > - byte[11] != ((DST_IP6[2] >> 24) & 0xff) || > - byte[12] != ((DST_IP6[3] >> 0) & 0xff) || > - byte[13] != ((DST_IP6[3] >> 8) & 0xff) || > - byte[14] != ((DST_IP6[3] >> 16) & 0xff) || > - byte[15] != ((DST_IP6[3] >> 24) & 0xff)) > + if (LSB(ctx->local_ip6[0], 0) != ((DST_IP6[0] >> 0) & 0xff) || > + LSB(ctx->local_ip6[0], 1) != ((DST_IP6[0] >> 8) & 0xff) || > + LSB(ctx->local_ip6[0], 2) != ((DST_IP6[0] >> 16) & 0xff) || > + LSB(ctx->local_ip6[0], 3) != ((DST_IP6[0] >> 24) & 0xff) || > + LSB(ctx->local_ip6[1], 0) != ((DST_IP6[1] >> 0) & 0xff) || > + LSB(ctx->local_ip6[1], 1) != ((DST_IP6[1] >> 8) & 0xff) || > + LSB(ctx->local_ip6[1], 2) != ((DST_IP6[1] >> 16) & 0xff) || > + LSB(ctx->local_ip6[1], 3) != ((DST_IP6[1] >> 24) & 0xff) || > + LSB(ctx->local_ip6[2], 0) != ((DST_IP6[2] >> 0) & 0xff) || > + LSB(ctx->local_ip6[2], 1) != ((DST_IP6[2] >> 8) & 0xff) || > + LSB(ctx->local_ip6[2], 2) != ((DST_IP6[2] >> 16) & 0xff) || > + LSB(ctx->local_ip6[2], 3) != ((DST_IP6[2] >> 24) & 0xff) || > + LSB(ctx->local_ip6[3], 0) != ((DST_IP6[3] >> 0) & 0xff) || > + LSB(ctx->local_ip6[3], 1) != ((DST_IP6[3] >> 8) & 0xff) || > + LSB(ctx->local_ip6[3], 2) != ((DST_IP6[3] >> 16) & 0xff) || > + LSB(ctx->local_ip6[3], 3) != ((DST_IP6[3] >> 24) & 0xff)) > return SK_DROP; > - half = (__u16 *)&ctx->local_ip6; > - if (half[0] != ((DST_IP6[0] >> 0) & 0xffff) || > - half[1] != ((DST_IP6[0] >> 16) & 0xffff) || > - half[2] != ((DST_IP6[1] >> 0) & 0xffff) || > - half[3] != ((DST_IP6[1] >> 16) & 0xffff) || > - half[4] != ((DST_IP6[2] >> 0) & 0xffff) || > - half[5] != ((DST_IP6[2] >> 16) & 0xffff) || > - half[6] != ((DST_IP6[3] >> 0) & 0xffff) || > - half[7] != ((DST_IP6[3] >> 16) & 0xffff)) > + if (LSW(ctx->local_ip6[0], 0) != ((DST_IP6[0] >> 0) & 0xffff) || > + LSW(ctx->local_ip6[0], 1) != > + ((DST_IP6[0] >> 16) & 0xffff) || > + LSW(ctx->local_ip6[1], 0) != ((DST_IP6[1] >> 0) & 0xffff) || > + LSW(ctx->local_ip6[1], 1) != > + ((DST_IP6[1] >> 16) & 0xffff) || > + LSW(ctx->local_ip6[2], 0) != ((DST_IP6[2] >> 0) & 0xffff) || > + LSW(ctx->local_ip6[2], 1) != > + ((DST_IP6[2] >> 16) & 0xffff) || > + LSW(ctx->local_ip6[3], 0) != ((DST_IP6[3] >> 0) & 0xffff) || > + LSW(ctx->local_ip6[3], 1) != ((DST_IP6[3] >> 16) & 0xffff)) > return SK_DROP; > } else { > /* Expect :: IPs when family != AF_INET6 */ > - byte = (__u8 *)&ctx->remote_ip6; > - if (byte[0] != 0 || byte[1] != 0 || > - byte[2] != 0 || byte[3] != 0 || > - byte[4] != 0 || byte[5] != 0 || > - byte[6] != 0 || byte[7] != 0 || > - byte[8] != 0 || byte[9] != 0 || > - byte[10] != 0 || byte[11] != 0 || > - byte[12] != 0 || byte[13] != 0 || > - byte[14] != 0 || byte[15] != 0) > + if (LSB(ctx->remote_ip6[0], 0) != 0 || > + LSB(ctx->remote_ip6[0], 1) != 0 || > + LSB(ctx->remote_ip6[0], 2) != 0 || > + LSB(ctx->remote_ip6[0], 3) != 0 || > + LSB(ctx->remote_ip6[1], 0) != 0 || > + LSB(ctx->remote_ip6[1], 1) != 0 || > + LSB(ctx->remote_ip6[1], 2) != 0 || > + LSB(ctx->remote_ip6[1], 3) != 0 || > + LSB(ctx->remote_ip6[2], 0) != 0 || > + LSB(ctx->remote_ip6[2], 1) != 0 || > + LSB(ctx->remote_ip6[2], 2) != 0 || > + LSB(ctx->remote_ip6[2], 3) != 0 || > + LSB(ctx->remote_ip6[3], 0) != 0 || > + LSB(ctx->remote_ip6[3], 1) != 0 || > + LSB(ctx->remote_ip6[3], 2) != 0 || > + LSB(ctx->remote_ip6[3], 3) != 0) > return SK_DROP; > - half = (__u16 *)&ctx->remote_ip6; > - if (half[0] != 0 || half[1] != 0 || > - half[2] != 0 || half[3] != 0 || > - half[4] != 0 || half[5] != 0 || > - half[6] != 0 || half[7] != 0) > + if (LSW(ctx->remote_ip6[0], 0) != 0 || > + LSW(ctx->remote_ip6[0], 1) != 0 || > + LSW(ctx->remote_ip6[1], 0) != 0 || > + LSW(ctx->remote_ip6[1], 1) != 0 || > + LSW(ctx->remote_ip6[2], 0) != 0 || > + LSW(ctx->remote_ip6[2], 1) != 0 || > + LSW(ctx->remote_ip6[3], 0) != 0 || > + LSW(ctx->remote_ip6[3], 1) != 0) > return SK_DROP; > > - byte = (__u8 *)&ctx->local_ip6; > - if (byte[0] != 0 || byte[1] != 0 || > - byte[2] != 0 || byte[3] != 0 || > - byte[4] != 0 || byte[5] != 0 || > - byte[6] != 0 || byte[7] != 0 || > - byte[8] != 0 || byte[9] != 0 || > - byte[10] != 0 || byte[11] != 0 || > - byte[12] != 0 || byte[13] != 0 || > - byte[14] != 0 || byte[15] != 0) > + if (LSB(ctx->local_ip6[0], 0) != 0 || > + LSB(ctx->local_ip6[0], 1) != 0 || > + LSB(ctx->local_ip6[0], 2) != 0 || > + LSB(ctx->local_ip6[0], 3) != 0 || > + LSB(ctx->local_ip6[1], 0) != 0 || > + LSB(ctx->local_ip6[1], 1) != 0 || > + LSB(ctx->local_ip6[1], 2) != 0 || > + LSB(ctx->local_ip6[1], 3) != 0 || > + LSB(ctx->local_ip6[2], 0) != 0 || > + LSB(ctx->local_ip6[2], 1) != 0 || > + LSB(ctx->local_ip6[2], 2) != 0 || > + LSB(ctx->local_ip6[2], 3) != 0 || > + LSB(ctx->local_ip6[3], 0) != 0 || > + LSB(ctx->local_ip6[3], 1) != 0 || > + LSB(ctx->local_ip6[3], 2) != 0 || > + LSB(ctx->local_ip6[3], 3) != 0) > return SK_DROP; > - half = (__u16 *)&ctx->local_ip6; > - if (half[0] != 0 || half[1] != 0 || > - half[2] != 0 || half[3] != 0 || > - half[4] != 0 || half[5] != 0 || > - half[6] != 0 || half[7] != 0) > + if (LSW(ctx->remote_ip6[0], 0) != 0 || > + LSW(ctx->remote_ip6[0], 1) != 0 || > + LSW(ctx->remote_ip6[1], 0) != 0 || > + LSW(ctx->remote_ip6[1], 1) != 0 || > + LSW(ctx->remote_ip6[2], 0) != 0 || > + LSW(ctx->remote_ip6[2], 1) != 0 || > + LSW(ctx->remote_ip6[3], 0) != 0 || > + LSW(ctx->remote_ip6[3], 1) != 0) > return SK_DROP; > } > > -- > 2.25.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access 2020-09-10 16:55 ` Andrii Nakryiko @ 2020-09-23 21:12 ` Ilya Leoshkevich 2020-09-24 7:49 ` Jakub Sitnicki 0 siblings, 1 reply; 12+ messages in thread From: Ilya Leoshkevich @ 2020-09-23 21:12 UTC (permalink / raw) To: Andrii Nakryiko, Jakub Sitnicki Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Thu, 2020-09-10 at 09:55 -0700, Andrii Nakryiko wrote: > On Wed, Sep 9, 2020 at 6:59 PM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > This test makes a lot of narrow load checks while assuming little > > endian architecture, and therefore fails on s390. > > > > Fix by introducing LSB and LSW macros and using them to perform > > narrow > > loads. > > > > Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach > > point") > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > --- > > Jakub, > > Can you please help review this to make sure no error accidentally > slipped in? Gentle ping. > > > .../selftests/bpf/progs/test_sk_lookup.c | 264 ++++++++++-- > > ------ > > 1 file changed, 149 insertions(+), 115 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/progs/test_sk_lookup.c > > b/tools/testing/selftests/bpf/progs/test_sk_lookup.c > > index bbf8296f4d66..94e6d370967b 100644 > > --- a/tools/testing/selftests/bpf/progs/test_sk_lookup.c > > +++ b/tools/testing/selftests/bpf/progs/test_sk_lookup.c > > @@ -19,6 +19,17 @@ > > #define IP6(aaaa, bbbb, cccc, dddd) \ > > { bpf_htonl(aaaa), bpf_htonl(bbbb), bpf_htonl(cccc), > > bpf_htonl(dddd) } > > > > +/* Macros for least-significant byte and word accesses. */ > > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ > > +#define LSE_INDEX(index, size) (index) > > +#else > > +#define LSE_INDEX(index, size) ((size) - (index) - 1) > > +#endif > > +#define LSB(value, index) \ > > + (((__u8 *)&(value))[LSE_INDEX((index), sizeof(value))]) > > +#define LSW(value, index) \ > > + (((__u16 *)&(value))[LSE_INDEX((index), sizeof(value) / > > 2)]) > > + > > #define MAX_SOCKS 32 > > > > struct { > > @@ -369,171 +380,194 @@ int ctx_narrow_access(struct bpf_sk_lookup > > *ctx) > > { > > struct bpf_sock *sk; > > int err, family; > > - __u16 *half; > > - __u8 *byte; > > bool v4; > > > > v4 = (ctx->family == AF_INET); > > > > /* Narrow loads from family field */ > > - byte = (__u8 *)&ctx->family; > > - half = (__u16 *)&ctx->family; > > - if (byte[0] != (v4 ? AF_INET : AF_INET6) || > > - byte[1] != 0 || byte[2] != 0 || byte[3] != 0) > > + if (LSB(ctx->family, 0) != (v4 ? AF_INET : AF_INET6) || > > + LSB(ctx->family, 1) != 0 || LSB(ctx->family, 2) != 0 || > > + LSB(ctx->family, 3) != 0) > > return SK_DROP; > > - if (half[0] != (v4 ? AF_INET : AF_INET6)) > > + if (LSW(ctx->family, 0) != (v4 ? AF_INET : AF_INET6)) > > return SK_DROP; > > > > - byte = (__u8 *)&ctx->protocol; > > - if (byte[0] != IPPROTO_TCP || > > - byte[1] != 0 || byte[2] != 0 || byte[3] != 0) > > + /* Narrow loads from protocol field */ > > + if (LSB(ctx->protocol, 0) != IPPROTO_TCP || > > + LSB(ctx->protocol, 1) != 0 || LSB(ctx->protocol, 2) != > > 0 || > > + LSB(ctx->protocol, 3) != 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->protocol; > > - if (half[0] != IPPROTO_TCP) > > + if (LSW(ctx->protocol, 0) != IPPROTO_TCP) > > return SK_DROP; > > > > /* Narrow loads from remote_port field. Expect non-0 value. > > */ > > - byte = (__u8 *)&ctx->remote_port; > > - if (byte[0] == 0 && byte[1] == 0 && byte[2] == 0 && byte[3] > > == 0) > > + if (LSB(ctx->remote_port, 0) == 0 && LSB(ctx->remote_port, > > 1) == 0 && > > + LSB(ctx->remote_port, 2) == 0 && LSB(ctx->remote_port, > > 3) == 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->remote_port; > > - if (half[0] == 0) > > + if (LSW(ctx->remote_port, 0) == 0) > > return SK_DROP; > > > > /* Narrow loads from local_port field. Expect DST_PORT. */ > > - byte = (__u8 *)&ctx->local_port; > > - if (byte[0] != ((DST_PORT >> 0) & 0xff) || > > - byte[1] != ((DST_PORT >> 8) & 0xff) || > > - byte[2] != 0 || byte[3] != 0) > > + if (LSB(ctx->local_port, 0) != ((DST_PORT >> 0) & 0xff) || > > + LSB(ctx->local_port, 1) != ((DST_PORT >> 8) & 0xff) || > > + LSB(ctx->local_port, 2) != 0 || LSB(ctx->local_port, 3) > > != 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->local_port; > > - if (half[0] != DST_PORT) > > + if (LSW(ctx->local_port, 0) != DST_PORT) > > return SK_DROP; > > > > /* Narrow loads from IPv4 fields */ > > if (v4) { > > /* Expect non-0.0.0.0 in remote_ip4 */ > > - byte = (__u8 *)&ctx->remote_ip4; > > - if (byte[0] == 0 && byte[1] == 0 && > > - byte[2] == 0 && byte[3] == 0) > > + if (LSB(ctx->remote_ip4, 0) == 0 && > > + LSB(ctx->remote_ip4, 1) == 0 && > > + LSB(ctx->remote_ip4, 2) == 0 && > > + LSB(ctx->remote_ip4, 3) == 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->remote_ip4; > > - if (half[0] == 0 && half[1] == 0) > > + if (LSW(ctx->remote_ip4, 0) == 0 && > > + LSW(ctx->remote_ip4, 1) == 0) > > return SK_DROP; > > > > /* Expect DST_IP4 in local_ip4 */ > > - byte = (__u8 *)&ctx->local_ip4; > > - if (byte[0] != ((DST_IP4 >> 0) & 0xff) || > > - byte[1] != ((DST_IP4 >> 8) & 0xff) || > > - byte[2] != ((DST_IP4 >> 16) & 0xff) || > > - byte[3] != ((DST_IP4 >> 24) & 0xff)) > > + if (LSB(ctx->local_ip4, 0) != ((DST_IP4 >> 0) & > > 0xff) || > > + LSB(ctx->local_ip4, 1) != ((DST_IP4 >> 8) & > > 0xff) || > > + LSB(ctx->local_ip4, 2) != ((DST_IP4 >> 16) & > > 0xff) || > > + LSB(ctx->local_ip4, 3) != ((DST_IP4 >> 24) & > > 0xff)) > > return SK_DROP; > > - half = (__u16 *)&ctx->local_ip4; > > - if (half[0] != ((DST_IP4 >> 0) & 0xffff) || > > - half[1] != ((DST_IP4 >> 16) & 0xffff)) > > + if (LSW(ctx->local_ip4, 0) != ((DST_IP4 >> 0) & > > 0xffff) || > > + LSW(ctx->local_ip4, 1) != ((DST_IP4 >> 16) & > > 0xffff)) > > return SK_DROP; > > } else { > > /* Expect 0.0.0.0 IPs when family != AF_INET */ > > - byte = (__u8 *)&ctx->remote_ip4; > > - if (byte[0] != 0 || byte[1] != 0 && > > - byte[2] != 0 || byte[3] != 0) > > + if (LSB(ctx->remote_ip4, 0) != 0 || > > + LSB(ctx->remote_ip4, 1) != 0 || > > + LSB(ctx->remote_ip4, 2) != 0 || > > + LSB(ctx->remote_ip4, 3) != 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->remote_ip4; > > - if (half[0] != 0 || half[1] != 0) > > + if (LSW(ctx->remote_ip4, 0) != 0 || > > + LSW(ctx->remote_ip4, 1) != 0) > > return SK_DROP; > > > > - byte = (__u8 *)&ctx->local_ip4; > > - if (byte[0] != 0 || byte[1] != 0 && > > - byte[2] != 0 || byte[3] != 0) > > + if (LSB(ctx->local_ip4, 0) != 0 || > > + LSB(ctx->local_ip4, 1) != 0 || > > + LSB(ctx->local_ip4, 2) != 0 || LSB(ctx- > > >local_ip4, 3) != 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->local_ip4; > > - if (half[0] != 0 || half[1] != 0) > > + if (LSW(ctx->local_ip4, 0) != 0 || LSW(ctx- > > >local_ip4, 1) != 0) > > return SK_DROP; > > } > > > > /* Narrow loads from IPv6 fields */ > > if (!v4) { > > - /* Expenct non-:: IP in remote_ip6 */ > > - byte = (__u8 *)&ctx->remote_ip6; > > - if (byte[0] == 0 && byte[1] == 0 && > > - byte[2] == 0 && byte[3] == 0 && > > - byte[4] == 0 && byte[5] == 0 && > > - byte[6] == 0 && byte[7] == 0 && > > - byte[8] == 0 && byte[9] == 0 && > > - byte[10] == 0 && byte[11] == 0 && > > - byte[12] == 0 && byte[13] == 0 && > > - byte[14] == 0 && byte[15] == 0) > > + /* Expect non-:: IP in remote_ip6 */ > > + if (LSB(ctx->remote_ip6[0], 0) == 0 && > > + LSB(ctx->remote_ip6[0], 1) == 0 && > > + LSB(ctx->remote_ip6[0], 2) == 0 && > > + LSB(ctx->remote_ip6[0], 3) == 0 && > > + LSB(ctx->remote_ip6[1], 0) == 0 && > > + LSB(ctx->remote_ip6[1], 1) == 0 && > > + LSB(ctx->remote_ip6[1], 2) == 0 && > > + LSB(ctx->remote_ip6[1], 3) == 0 && > > + LSB(ctx->remote_ip6[2], 0) == 0 && > > + LSB(ctx->remote_ip6[2], 1) == 0 && > > + LSB(ctx->remote_ip6[2], 2) == 0 && > > + LSB(ctx->remote_ip6[2], 3) == 0 && > > + LSB(ctx->remote_ip6[3], 0) == 0 && > > + LSB(ctx->remote_ip6[3], 1) == 0 && > > + LSB(ctx->remote_ip6[3], 2) == 0 && > > + LSB(ctx->remote_ip6[3], 3) == 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->remote_ip6; > > - if (half[0] == 0 && half[1] == 0 && > > - half[2] == 0 && half[3] == 0 && > > - half[4] == 0 && half[5] == 0 && > > - half[6] == 0 && half[7] == 0) > > + if (LSW(ctx->remote_ip6[0], 0) == 0 && > > + LSW(ctx->remote_ip6[0], 1) == 0 && > > + LSW(ctx->remote_ip6[1], 0) == 0 && > > + LSW(ctx->remote_ip6[1], 1) == 0 && > > + LSW(ctx->remote_ip6[2], 0) == 0 && > > + LSW(ctx->remote_ip6[2], 1) == 0 && > > + LSW(ctx->remote_ip6[3], 0) == 0 && > > + LSW(ctx->remote_ip6[3], 1) == 0) > > return SK_DROP; > > - > > /* Expect DST_IP6 in local_ip6 */ > > - byte = (__u8 *)&ctx->local_ip6; > > - if (byte[0] != ((DST_IP6[0] >> 0) & 0xff) || > > - byte[1] != ((DST_IP6[0] >> 8) & 0xff) || > > - byte[2] != ((DST_IP6[0] >> 16) & 0xff) || > > - byte[3] != ((DST_IP6[0] >> 24) & 0xff) || > > - byte[4] != ((DST_IP6[1] >> 0) & 0xff) || > > - byte[5] != ((DST_IP6[1] >> 8) & 0xff) || > > - byte[6] != ((DST_IP6[1] >> 16) & 0xff) || > > - byte[7] != ((DST_IP6[1] >> 24) & 0xff) || > > - byte[8] != ((DST_IP6[2] >> 0) & 0xff) || > > - byte[9] != ((DST_IP6[2] >> 8) & 0xff) || > > - byte[10] != ((DST_IP6[2] >> 16) & 0xff) || > > - byte[11] != ((DST_IP6[2] >> 24) & 0xff) || > > - byte[12] != ((DST_IP6[3] >> 0) & 0xff) || > > - byte[13] != ((DST_IP6[3] >> 8) & 0xff) || > > - byte[14] != ((DST_IP6[3] >> 16) & 0xff) || > > - byte[15] != ((DST_IP6[3] >> 24) & 0xff)) > > + if (LSB(ctx->local_ip6[0], 0) != ((DST_IP6[0] >> 0) > > & 0xff) || > > + LSB(ctx->local_ip6[0], 1) != ((DST_IP6[0] >> 8) > > & 0xff) || > > + LSB(ctx->local_ip6[0], 2) != ((DST_IP6[0] >> > > 16) & 0xff) || > > + LSB(ctx->local_ip6[0], 3) != ((DST_IP6[0] >> > > 24) & 0xff) || > > + LSB(ctx->local_ip6[1], 0) != ((DST_IP6[1] >> 0) > > & 0xff) || > > + LSB(ctx->local_ip6[1], 1) != ((DST_IP6[1] >> 8) > > & 0xff) || > > + LSB(ctx->local_ip6[1], 2) != ((DST_IP6[1] >> > > 16) & 0xff) || > > + LSB(ctx->local_ip6[1], 3) != ((DST_IP6[1] >> > > 24) & 0xff) || > > + LSB(ctx->local_ip6[2], 0) != ((DST_IP6[2] >> 0) > > & 0xff) || > > + LSB(ctx->local_ip6[2], 1) != ((DST_IP6[2] >> 8) > > & 0xff) || > > + LSB(ctx->local_ip6[2], 2) != ((DST_IP6[2] >> > > 16) & 0xff) || > > + LSB(ctx->local_ip6[2], 3) != ((DST_IP6[2] >> > > 24) & 0xff) || > > + LSB(ctx->local_ip6[3], 0) != ((DST_IP6[3] >> 0) > > & 0xff) || > > + LSB(ctx->local_ip6[3], 1) != ((DST_IP6[3] >> 8) > > & 0xff) || > > + LSB(ctx->local_ip6[3], 2) != ((DST_IP6[3] >> > > 16) & 0xff) || > > + LSB(ctx->local_ip6[3], 3) != ((DST_IP6[3] >> > > 24) & 0xff)) > > return SK_DROP; > > - half = (__u16 *)&ctx->local_ip6; > > - if (half[0] != ((DST_IP6[0] >> 0) & 0xffff) || > > - half[1] != ((DST_IP6[0] >> 16) & 0xffff) || > > - half[2] != ((DST_IP6[1] >> 0) & 0xffff) || > > - half[3] != ((DST_IP6[1] >> 16) & 0xffff) || > > - half[4] != ((DST_IP6[2] >> 0) & 0xffff) || > > - half[5] != ((DST_IP6[2] >> 16) & 0xffff) || > > - half[6] != ((DST_IP6[3] >> 0) & 0xffff) || > > - half[7] != ((DST_IP6[3] >> 16) & 0xffff)) > > + if (LSW(ctx->local_ip6[0], 0) != ((DST_IP6[0] >> 0) > > & 0xffff) || > > + LSW(ctx->local_ip6[0], 1) != > > + ((DST_IP6[0] >> 16) & 0xffff) || > > + LSW(ctx->local_ip6[1], 0) != ((DST_IP6[1] >> 0) > > & 0xffff) || > > + LSW(ctx->local_ip6[1], 1) != > > + ((DST_IP6[1] >> 16) & 0xffff) || > > + LSW(ctx->local_ip6[2], 0) != ((DST_IP6[2] >> 0) > > & 0xffff) || > > + LSW(ctx->local_ip6[2], 1) != > > + ((DST_IP6[2] >> 16) & 0xffff) || > > + LSW(ctx->local_ip6[3], 0) != ((DST_IP6[3] >> 0) > > & 0xffff) || > > + LSW(ctx->local_ip6[3], 1) != ((DST_IP6[3] >> > > 16) & 0xffff)) > > return SK_DROP; > > } else { > > /* Expect :: IPs when family != AF_INET6 */ > > - byte = (__u8 *)&ctx->remote_ip6; > > - if (byte[0] != 0 || byte[1] != 0 || > > - byte[2] != 0 || byte[3] != 0 || > > - byte[4] != 0 || byte[5] != 0 || > > - byte[6] != 0 || byte[7] != 0 || > > - byte[8] != 0 || byte[9] != 0 || > > - byte[10] != 0 || byte[11] != 0 || > > - byte[12] != 0 || byte[13] != 0 || > > - byte[14] != 0 || byte[15] != 0) > > + if (LSB(ctx->remote_ip6[0], 0) != 0 || > > + LSB(ctx->remote_ip6[0], 1) != 0 || > > + LSB(ctx->remote_ip6[0], 2) != 0 || > > + LSB(ctx->remote_ip6[0], 3) != 0 || > > + LSB(ctx->remote_ip6[1], 0) != 0 || > > + LSB(ctx->remote_ip6[1], 1) != 0 || > > + LSB(ctx->remote_ip6[1], 2) != 0 || > > + LSB(ctx->remote_ip6[1], 3) != 0 || > > + LSB(ctx->remote_ip6[2], 0) != 0 || > > + LSB(ctx->remote_ip6[2], 1) != 0 || > > + LSB(ctx->remote_ip6[2], 2) != 0 || > > + LSB(ctx->remote_ip6[2], 3) != 0 || > > + LSB(ctx->remote_ip6[3], 0) != 0 || > > + LSB(ctx->remote_ip6[3], 1) != 0 || > > + LSB(ctx->remote_ip6[3], 2) != 0 || > > + LSB(ctx->remote_ip6[3], 3) != 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->remote_ip6; > > - if (half[0] != 0 || half[1] != 0 || > > - half[2] != 0 || half[3] != 0 || > > - half[4] != 0 || half[5] != 0 || > > - half[6] != 0 || half[7] != 0) > > + if (LSW(ctx->remote_ip6[0], 0) != 0 || > > + LSW(ctx->remote_ip6[0], 1) != 0 || > > + LSW(ctx->remote_ip6[1], 0) != 0 || > > + LSW(ctx->remote_ip6[1], 1) != 0 || > > + LSW(ctx->remote_ip6[2], 0) != 0 || > > + LSW(ctx->remote_ip6[2], 1) != 0 || > > + LSW(ctx->remote_ip6[3], 0) != 0 || > > + LSW(ctx->remote_ip6[3], 1) != 0) > > return SK_DROP; > > > > - byte = (__u8 *)&ctx->local_ip6; > > - if (byte[0] != 0 || byte[1] != 0 || > > - byte[2] != 0 || byte[3] != 0 || > > - byte[4] != 0 || byte[5] != 0 || > > - byte[6] != 0 || byte[7] != 0 || > > - byte[8] != 0 || byte[9] != 0 || > > - byte[10] != 0 || byte[11] != 0 || > > - byte[12] != 0 || byte[13] != 0 || > > - byte[14] != 0 || byte[15] != 0) > > + if (LSB(ctx->local_ip6[0], 0) != 0 || > > + LSB(ctx->local_ip6[0], 1) != 0 || > > + LSB(ctx->local_ip6[0], 2) != 0 || > > + LSB(ctx->local_ip6[0], 3) != 0 || > > + LSB(ctx->local_ip6[1], 0) != 0 || > > + LSB(ctx->local_ip6[1], 1) != 0 || > > + LSB(ctx->local_ip6[1], 2) != 0 || > > + LSB(ctx->local_ip6[1], 3) != 0 || > > + LSB(ctx->local_ip6[2], 0) != 0 || > > + LSB(ctx->local_ip6[2], 1) != 0 || > > + LSB(ctx->local_ip6[2], 2) != 0 || > > + LSB(ctx->local_ip6[2], 3) != 0 || > > + LSB(ctx->local_ip6[3], 0) != 0 || > > + LSB(ctx->local_ip6[3], 1) != 0 || > > + LSB(ctx->local_ip6[3], 2) != 0 || > > + LSB(ctx->local_ip6[3], 3) != 0) > > return SK_DROP; > > - half = (__u16 *)&ctx->local_ip6; > > - if (half[0] != 0 || half[1] != 0 || > > - half[2] != 0 || half[3] != 0 || > > - half[4] != 0 || half[5] != 0 || > > - half[6] != 0 || half[7] != 0) > > + if (LSW(ctx->remote_ip6[0], 0) != 0 || > > + LSW(ctx->remote_ip6[0], 1) != 0 || > > + LSW(ctx->remote_ip6[1], 0) != 0 || > > + LSW(ctx->remote_ip6[1], 1) != 0 || > > + LSW(ctx->remote_ip6[2], 0) != 0 || > > + LSW(ctx->remote_ip6[2], 1) != 0 || > > + LSW(ctx->remote_ip6[3], 0) != 0 || > > + LSW(ctx->remote_ip6[3], 1) != 0) > > return SK_DROP; > > } > > > > -- > > 2.25.4 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access 2020-09-23 21:12 ` Ilya Leoshkevich @ 2020-09-24 7:49 ` Jakub Sitnicki 0 siblings, 0 replies; 12+ messages in thread From: Jakub Sitnicki @ 2020-09-24 7:49 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Wed, Sep 23, 2020 at 11:12 PM CEST, Ilya Leoshkevich wrote: > On Thu, 2020-09-10 at 09:55 -0700, Andrii Nakryiko wrote: >> On Wed, Sep 9, 2020 at 6:59 PM Ilya Leoshkevich <iii@linux.ibm.com> >> wrote: >> > This test makes a lot of narrow load checks while assuming little >> > endian architecture, and therefore fails on s390. >> > >> > Fix by introducing LSB and LSW macros and using them to perform >> > narrow >> > loads. >> > >> > Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach >> > point") >> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> >> > --- >> >> Jakub, >> >> Can you please help review this to make sure no error accidentally >> slipped in? > > Gentle ping. Sorry for being unresponsive. I've been off for a couple of weeks. I'm on it. [...] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access 2020-09-09 23:24 ` [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access Ilya Leoshkevich 2020-09-10 16:55 ` Andrii Nakryiko @ 2020-09-24 9:24 ` Jakub Sitnicki 1 sibling, 0 replies; 12+ messages in thread From: Jakub Sitnicki @ 2020-09-24 9:24 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Thu, Sep 10, 2020 at 01:24 AM CEST, Ilya Leoshkevich wrote: > This test makes a lot of narrow load checks while assuming little > endian architecture, and therefore fails on s390. > > Fix by introducing LSB and LSW macros and using them to perform narrow > loads. > > Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point") > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- Keeping some lines > 80 chars would make it a bit more readable IMO, but otherwise LGTM. Thank you for fixing it. Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com> [...] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: Fix endianness issue in sk_assign 2020-09-09 23:24 [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Ilya Leoshkevich 2020-09-09 23:24 ` [PATCH bpf-next 1/3] selftests/bpf: Fix endianness issue in test_sockopt_sk Ilya Leoshkevich 2020-09-09 23:24 ` [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access Ilya Leoshkevich @ 2020-09-09 23:24 ` Ilya Leoshkevich 2020-09-10 16:56 ` Andrii Nakryiko 2020-09-10 16:58 ` [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Andrii Nakryiko 3 siblings, 1 reply; 12+ messages in thread From: Ilya Leoshkevich @ 2020-09-09 23:24 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich server_map's value size is 8, but the test tries to put an int there. This sort of works on x86 (unless followed by non-0), but hard fails on s390. Fix by using long instead of int. Fixes: 2d7824ffd25c ("selftests: bpf: Add test for sk_assign") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/prog_tests/sk_assign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/sk_assign.c b/tools/testing/selftests/bpf/prog_tests/sk_assign.c index a49a26f95a8b..402d0da8e05a 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_assign.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_assign.c @@ -265,7 +265,7 @@ void test_sk_assign(void) TEST("ipv6 udp port redir", AF_INET6, SOCK_DGRAM, false), TEST("ipv6 udp addr redir", AF_INET6, SOCK_DGRAM, true), }; - int server = -1; + long server = -1; int server_map; int self_net; int i; -- 2.25.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 3/3] selftests/bpf: Fix endianness issue in sk_assign 2020-09-09 23:24 ` [PATCH bpf-next 3/3] selftests/bpf: Fix endianness issue in sk_assign Ilya Leoshkevich @ 2020-09-10 16:56 ` Andrii Nakryiko 0 siblings, 0 replies; 12+ messages in thread From: Andrii Nakryiko @ 2020-09-10 16:56 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Wed, Sep 9, 2020 at 6:59 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > server_map's value size is 8, but the test tries to put an int there. > This sort of works on x86 (unless followed by non-0), but hard fails on > s390. > > Fix by using long instead of int. > > Fixes: 2d7824ffd25c ("selftests: bpf: Add test for sk_assign") > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/testing/selftests/bpf/prog_tests/sk_assign.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/sk_assign.c b/tools/testing/selftests/bpf/prog_tests/sk_assign.c > index a49a26f95a8b..402d0da8e05a 100644 > --- a/tools/testing/selftests/bpf/prog_tests/sk_assign.c > +++ b/tools/testing/selftests/bpf/prog_tests/sk_assign.c > @@ -265,7 +265,7 @@ void test_sk_assign(void) > TEST("ipv6 udp port redir", AF_INET6, SOCK_DGRAM, false), > TEST("ipv6 udp addr redir", AF_INET6, SOCK_DGRAM, true), > }; > - int server = -1; > + long server = -1; this would still fail on 32-bit arches, so maybe __s64 instead? > int server_map; > int self_net; > int i; > -- > 2.25.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/3] Fix three endianness issues in test_progs 2020-09-09 23:24 [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Ilya Leoshkevich ` (2 preceding siblings ...) 2020-09-09 23:24 ` [PATCH bpf-next 3/3] selftests/bpf: Fix endianness issue in sk_assign Ilya Leoshkevich @ 2020-09-10 16:58 ` Andrii Nakryiko 2020-09-10 21:55 ` Ilya Leoshkevich 3 siblings, 1 reply; 12+ messages in thread From: Andrii Nakryiko @ 2020-09-10 16:58 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Wed, Sep 9, 2020 at 6:59 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > This series fixes three test_progs failures on s390, all of which occur > because of endianness issues. > > Ilya Leoshkevich (3): > selftests/bpf: Fix endianness issue in test_sockopt_sk > selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access > selftests/bpf: Fix endianness issue in sk_assign > > .../selftests/bpf/prog_tests/sk_assign.c | 2 +- > .../selftests/bpf/prog_tests/sockopt_sk.c | 2 +- > .../selftests/bpf/progs/test_sk_lookup.c | 264 ++++++++++-------- > 3 files changed, 151 insertions(+), 117 deletions(-) > > -- > 2.25.4 > Ilya, libbpf Travis CI setup does only a build of libbpf for s390x right now. But we additionally have a test that builds the latest kernel and selftests and runs them in qemu. All this is implemented for x86-64 right now. But if you'd like to spend some time to set this up for s390x as well, please let me know. You'll be able to detect issues like this much earlier. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 0/3] Fix three endianness issues in test_progs 2020-09-10 16:58 ` [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Andrii Nakryiko @ 2020-09-10 21:55 ` Ilya Leoshkevich 0 siblings, 0 replies; 12+ messages in thread From: Ilya Leoshkevich @ 2020-09-10 21:55 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Thu, 2020-09-10 at 09:58 -0700, Andrii Nakryiko wrote: > On Wed, Sep 9, 2020 at 6:59 PM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > This series fixes three test_progs failures on s390, all of which > > occur > > because of endianness issues. > > > > Ilya Leoshkevich (3): > > selftests/bpf: Fix endianness issue in test_sockopt_sk > > selftests/bpf: Fix endianness issues in > > sk_lookup/ctx_narrow_access > > selftests/bpf: Fix endianness issue in sk_assign > > > > .../selftests/bpf/prog_tests/sk_assign.c | 2 +- > > .../selftests/bpf/prog_tests/sockopt_sk.c | 2 +- > > .../selftests/bpf/progs/test_sk_lookup.c | 264 ++++++++++-- > > ------ > > 3 files changed, 151 insertions(+), 117 deletions(-) > > > > -- > > 2.25.4 > > > > Ilya, > > libbpf Travis CI setup does only a build of libbpf for s390x right > now. But we additionally have a test that builds the latest kernel > and > selftests and runs them in qemu. All this is implemented for x86-64 > right now. But if you'd like to spend some time to set this up for > s390x as well, please let me know. You'll be able to detect issues > like this much earlier. That sounds interesting. I will try to adjust your scripts to create a s390x rootfs, cross-compile s390x kernel and selftests, and run them on my laptop; when this works, I will send a PR. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-09-24 9:24 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-09 23:24 [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Ilya Leoshkevich 2020-09-09 23:24 ` [PATCH bpf-next 1/3] selftests/bpf: Fix endianness issue in test_sockopt_sk Ilya Leoshkevich 2020-09-10 16:49 ` Andrii Nakryiko 2020-09-09 23:24 ` [PATCH bpf-next 2/3] selftests/bpf: Fix endianness issues in sk_lookup/ctx_narrow_access Ilya Leoshkevich 2020-09-10 16:55 ` Andrii Nakryiko 2020-09-23 21:12 ` Ilya Leoshkevich 2020-09-24 7:49 ` Jakub Sitnicki 2020-09-24 9:24 ` Jakub Sitnicki 2020-09-09 23:24 ` [PATCH bpf-next 3/3] selftests/bpf: Fix endianness issue in sk_assign Ilya Leoshkevich 2020-09-10 16:56 ` Andrii Nakryiko 2020-09-10 16:58 ` [PATCH bpf-next 0/3] Fix three endianness issues in test_progs Andrii Nakryiko 2020-09-10 21:55 ` Ilya Leoshkevich
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).