* [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
* [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
* [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 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
* 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 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
* 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
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).