* [PATCH bpf v3 0/2] selftests/bpf: fix endianness issues in test_sysctl
@ 2019-08-28 13:22 Ilya Leoshkevich
2019-08-28 13:22 ` [PATCH bpf v3 1/2] selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu Ilya Leoshkevich
2019-08-28 13:22 ` [PATCH bpf v3 2/2] selftests/bpf: fix endianness issues in test_sysctl Ilya Leoshkevich
0 siblings, 2 replies; 6+ messages in thread
From: Ilya Leoshkevich @ 2019-08-28 13:22 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov
Cc: bpf, Yonghong Song, Andrey Ignatov, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich
The first patch is a preparatory commit, which introduces 64-bit
endianness conversion functions.
The second patch is an actual fix.
v1->v2: Use bpf_ntohl and bpf_be64_to_cpu, drop __bpf_le64_to_cpu.
v2->v3: Split bpf_be64_to_cpu introduction into a separate patch.
Use the new functions in test_lwt_seg6local.c and
test_seg6_loop.c.
Ilya Leoshkevich (2):
selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu
selftests/bpf: fix endianness issues in test_sysctl
tools/testing/selftests/bpf/bpf_endian.h | 14 ++
.../selftests/bpf/progs/test_lwt_seg6local.c | 16 +--
.../selftests/bpf/progs/test_seg6_loop.c | 8 +-
tools/testing/selftests/bpf/test_sysctl.c | 130 ++++++++++++------
4 files changed, 107 insertions(+), 61 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf v3 1/2] selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu
2019-08-28 13:22 [PATCH bpf v3 0/2] selftests/bpf: fix endianness issues in test_sysctl Ilya Leoshkevich
@ 2019-08-28 13:22 ` Ilya Leoshkevich
2019-08-29 21:46 ` Song Liu
2019-08-28 13:22 ` [PATCH bpf v3 2/2] selftests/bpf: fix endianness issues in test_sysctl Ilya Leoshkevich
1 sibling, 1 reply; 6+ messages in thread
From: Ilya Leoshkevich @ 2019-08-28 13:22 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov
Cc: bpf, Yonghong Song, Andrey Ignatov, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich
test_lwt_seg6local and test_seg6_loop use custom 64-bit endianness
conversion macros. Centralize their definitions in bpf_endian.h in order
to reduce code duplication. This will also be useful when bpf_endian.h
is promoted to an offical libbpf header.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/bpf_endian.h | 14 ++++++++++++++
.../selftests/bpf/progs/test_lwt_seg6local.c | 16 ++++++----------
.../testing/selftests/bpf/progs/test_seg6_loop.c | 8 ++------
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_endian.h b/tools/testing/selftests/bpf/bpf_endian.h
index 05f036df8a4c..f92f29c00d50 100644
--- a/tools/testing/selftests/bpf/bpf_endian.h
+++ b/tools/testing/selftests/bpf/bpf_endian.h
@@ -29,6 +29,10 @@
# define __bpf_htonl(x) __builtin_bswap32(x)
# define __bpf_constant_ntohl(x) ___constant_swab32(x)
# define __bpf_constant_htonl(x) ___constant_swab32(x)
+# define __bpf_be64_to_cpu(x) __builtin_bswap64(x)
+# define __bpf_cpu_to_be64(x) __builtin_bswap64(x)
+# define __bpf_constant_be64_to_cpu(x) ___constant_swab64(x)
+# define __bpf_constant_cpu_to_be64(x) ___constant_swab64(x)
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
# define __bpf_ntohs(x) (x)
# define __bpf_htons(x) (x)
@@ -38,6 +42,10 @@
# define __bpf_htonl(x) (x)
# define __bpf_constant_ntohl(x) (x)
# define __bpf_constant_htonl(x) (x)
+# define __bpf_be64_to_cpu(x) (x)
+# define __bpf_cpu_to_be64(x) (x)
+# define __bpf_constant_be64_to_cpu(x) (x)
+# define __bpf_constant_cpu_to_be64(x) (x)
#else
# error "Fix your compiler's __BYTE_ORDER__?!"
#endif
@@ -54,5 +62,11 @@
#define bpf_ntohl(x) \
(__builtin_constant_p(x) ? \
__bpf_constant_ntohl(x) : __bpf_ntohl(x))
+#define bpf_cpu_to_be64(x) \
+ (__builtin_constant_p(x) ? \
+ __bpf_constant_cpu_to_be64(x) : __bpf_cpu_to_be64(x))
+#define bpf_be64_to_cpu(x) \
+ (__builtin_constant_p(x) ? \
+ __bpf_constant_be64_to_cpu(x) : __bpf_be64_to_cpu(x))
#endif /* __BPF_ENDIAN__ */
diff --git a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
index a334a0e882e4..5e86afb8fb31 100644
--- a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
+++ b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
@@ -12,10 +12,6 @@
#define SR6_FLAG_ALERT (1 << 4)
-#define htonll(x) ((bpf_htonl(1)) == 1 ? (x) : ((uint64_t)bpf_htonl((x) & \
- 0xFFFFFFFF) << 32) | bpf_htonl((x) >> 32))
-#define ntohll(x) ((bpf_ntohl(1)) == 1 ? (x) : ((uint64_t)bpf_ntohl((x) & \
- 0xFFFFFFFF) << 32) | bpf_ntohl((x) >> 32))
#define BPF_PACKET_HEADER __attribute__((packed))
struct ip6_t {
@@ -276,8 +272,8 @@ int has_egr_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh)
return 0;
// check if egress TLV value is correct
- if (ntohll(egr_addr.hi) == 0xfd00000000000000 &&
- ntohll(egr_addr.lo) == 0x4)
+ if (bpf_be64_to_cpu(egr_addr.hi) == 0xfd00000000000000 &&
+ bpf_be64_to_cpu(egr_addr.lo) == 0x4)
return 1;
}
@@ -308,8 +304,8 @@ int __encap_srh(struct __sk_buff *skb)
#pragma clang loop unroll(full)
for (unsigned long long lo = 0; lo < 4; lo++) {
- seg->lo = htonll(4 - lo);
- seg->hi = htonll(hi);
+ seg->lo = bpf_cpu_to_be64(4 - lo);
+ seg->hi = bpf_cpu_to_be64(hi);
seg = (struct ip6_addr_t *)((char *)seg + sizeof(*seg));
}
@@ -349,8 +345,8 @@ int __add_egr_x(struct __sk_buff *skb)
if (err)
return BPF_DROP;
- addr.lo = htonll(lo);
- addr.hi = htonll(hi);
+ addr.lo = bpf_cpu_to_be64(lo);
+ addr.hi = bpf_cpu_to_be64(hi);
err = bpf_lwt_seg6_action(skb, SEG6_LOCAL_ACTION_END_X,
(void *)&addr, sizeof(addr));
if (err)
diff --git a/tools/testing/selftests/bpf/progs/test_seg6_loop.c b/tools/testing/selftests/bpf/progs/test_seg6_loop.c
index 1dbe1d4d467e..c4d104428643 100644
--- a/tools/testing/selftests/bpf/progs/test_seg6_loop.c
+++ b/tools/testing/selftests/bpf/progs/test_seg6_loop.c
@@ -12,10 +12,6 @@
#define SR6_FLAG_ALERT (1 << 4)
-#define htonll(x) ((bpf_htonl(1)) == 1 ? (x) : ((uint64_t)bpf_htonl((x) & \
- 0xFFFFFFFF) << 32) | bpf_htonl((x) >> 32))
-#define ntohll(x) ((bpf_ntohl(1)) == 1 ? (x) : ((uint64_t)bpf_ntohl((x) & \
- 0xFFFFFFFF) << 32) | bpf_ntohl((x) >> 32))
#define BPF_PACKET_HEADER __attribute__((packed))
struct ip6_t {
@@ -251,8 +247,8 @@ int __add_egr_x(struct __sk_buff *skb)
if (err)
return BPF_DROP;
- addr.lo = htonll(lo);
- addr.hi = htonll(hi);
+ addr.lo = bpf_cpu_to_be64(lo);
+ addr.hi = bpf_cpu_to_be64(hi);
err = bpf_lwt_seg6_action(skb, SEG6_LOCAL_ACTION_END_X,
(void *)&addr, sizeof(addr));
if (err)
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf v3 2/2] selftests/bpf: fix endianness issues in test_sysctl
2019-08-28 13:22 [PATCH bpf v3 0/2] selftests/bpf: fix endianness issues in test_sysctl Ilya Leoshkevich
2019-08-28 13:22 ` [PATCH bpf v3 1/2] selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu Ilya Leoshkevich
@ 2019-08-28 13:22 ` Ilya Leoshkevich
2019-08-29 21:53 ` Song Liu
1 sibling, 1 reply; 6+ messages in thread
From: Ilya Leoshkevich @ 2019-08-28 13:22 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov
Cc: bpf, Yonghong Song, Andrey Ignatov, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich
A lot of test_sysctl sub-tests fail due to handling strings as a bunch
of immediate values in a little-endian-specific manner.
Fix by wrapping all immediates in bpf_ntohl and the new bpf_be64_to_cpu.
Also, sometimes tests fail because sysctl() unexpectedly succeeds with
an inappropriate "Unexpected failure" message and a random errno. Zero
out errno before calling sysctl() and replace the message with
"Unexpected success".
Fixes: 1f5fa9ab6e2e ("selftests/bpf: Test BPF_CGROUP_SYSCTL")
Fixes: 9a1027e52535 ("selftests/bpf: Test file_pos field in bpf_sysctl ctx")
Fixes: 6041c67f28d8 ("selftests/bpf: Test bpf_sysctl_get_name helper")
Fixes: 11ff34f74e32 ("selftests/bpf: Test sysctl_get_current_value helper")
Fixes: 786047dd08de ("selftests/bpf: Test bpf_sysctl_{get,set}_new_value helpers")
Fixes: 8549ddc832d6 ("selftests/bpf: Test bpf_strtol and bpf_strtoul helpers")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/test_sysctl.c | 130 ++++++++++++++--------
1 file changed, 85 insertions(+), 45 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sysctl.c b/tools/testing/selftests/bpf/test_sysctl.c
index a3bebd7c68dd..b8c6e31645cb 100644
--- a/tools/testing/selftests/bpf/test_sysctl.c
+++ b/tools/testing/selftests/bpf/test_sysctl.c
@@ -13,6 +13,7 @@
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
+#include "bpf_endian.h"
#include "bpf_rlimit.h"
#include "bpf_util.h"
#include "cgroup_helpers.h"
@@ -100,7 +101,7 @@ static struct sysctl_test tests[] = {
.descr = "ctx:write sysctl:write read ok",
.insns = {
/* If (write) */
- BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_1,
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
offsetof(struct bpf_sysctl, write)),
BPF_JMP_IMM(BPF_JNE, BPF_REG_7, 1, 2),
@@ -214,7 +215,8 @@ static struct sysctl_test tests[] = {
/* if (ret == expected && */
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, sizeof("tcp_mem") - 1, 6),
/* buf == "tcp_mem\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x006d656d5f706374ULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x7463705f6d656d00ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -255,7 +257,8 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 6),
/* buf[0:7] == "tcp_me\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x00656d5f706374ULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x7463705f6d650000ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -298,12 +301,14 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 16, 14),
/* buf[0:8] == "net/ipv4" && */
- BPF_LD_IMM64(BPF_REG_8, 0x347670692f74656eULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x6e65742f69707634ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 10),
/* buf[8:16] == "/tcp_mem" && */
- BPF_LD_IMM64(BPF_REG_8, 0x6d656d5f7063742fULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x2f7463705f6d656dULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 8),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 6),
@@ -350,12 +355,14 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 10),
/* buf[0:8] == "net/ipv4" && */
- BPF_LD_IMM64(BPF_REG_8, 0x347670692f74656eULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x6e65742f69707634ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 6),
/* buf[8:16] == "/tcp_me\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x00656d5f7063742fULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x2f7463705f6d6500ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 8),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -396,7 +403,8 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 6),
/* buf[0:8] == "net/ip\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x000070692f74656eULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x6e65742f69700000ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -431,7 +439,8 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 6, 6),
/* buf[0:6] == "Linux\n\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x000a78756e694cULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x4c696e75780a0000ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -469,7 +478,8 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 6, 6),
/* buf[0:6] == "Linux\n\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x000a78756e694cULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x4c696e75780a0000ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -507,7 +517,8 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 6),
/* buf[0:6] == "Linux\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x000078756e694cULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x4c696e7578000000ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -650,7 +661,8 @@ static struct sysctl_test tests[] = {
/* buf[0:4] == "606\0") */
BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_7, 0),
- BPF_JMP_IMM(BPF_JNE, BPF_REG_9, 0x00363036, 2),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_9,
+ bpf_ntohl(0x36303600), 2),
/* return DENY; */
BPF_MOV64_IMM(BPF_REG_0, 0),
@@ -685,17 +697,20 @@ static struct sysctl_test tests[] = {
BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 23, 14),
/* buf[0:8] == "3000000 " && */
- BPF_LD_IMM64(BPF_REG_8, 0x2030303030303033ULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x3330303030303020ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 10),
/* buf[8:16] == "4000000 " && */
- BPF_LD_IMM64(BPF_REG_8, 0x2030303030303034ULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x3430303030303020ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 8),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 6),
/* buf[16:24] == "6000000\0") */
- BPF_LD_IMM64(BPF_REG_8, 0x0030303030303036ULL),
+ BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
+ 0x3630303030303000ULL)),
BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 16),
BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
@@ -735,7 +750,8 @@ static struct sysctl_test tests[] = {
/* buf[0:3] == "60\0") */
BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_7, 0),
- BPF_JMP_IMM(BPF_JNE, BPF_REG_9, 0x003036, 2),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_9,
+ bpf_ntohl(0x36300000), 2),
/* return DENY; */
BPF_MOV64_IMM(BPF_REG_0, 0),
@@ -757,7 +773,8 @@ static struct sysctl_test tests[] = {
/* sysctl_set_new_value arg2 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x36303000)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_2, BPF_REG_7),
@@ -791,7 +808,7 @@ static struct sysctl_test tests[] = {
/* sysctl_set_new_value arg2 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, FIXUP_SYSCTL_VALUE),
+ BPF_LD_IMM64(BPF_REG_0, FIXUP_SYSCTL_VALUE),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_2, BPF_REG_7),
@@ -825,8 +842,9 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
- BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x36303000)),
+ BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -869,7 +887,8 @@ static struct sysctl_test tests[] = {
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
/* "600 602\0" */
- BPF_LD_IMM64(BPF_REG_0, 0x0032303620303036ULL),
+ BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
+ 0x3630302036303200ULL)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -937,7 +956,8 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x36303000)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -969,8 +989,9 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x00373730),
- BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x30373700)),
+ BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1012,7 +1033,8 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x36303000)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1052,7 +1074,8 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x090a0c0d),
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x0d0c0a09)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1092,7 +1115,9 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x00362d0a), /* " -6\0" */
+ /* " -6\0" */
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x0a2d3600)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1132,8 +1157,10 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x00362d0a), /* " -6\0" */
- BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
+ /* " -6\0" */
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x0a2d3600)),
+ BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1175,8 +1202,10 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
- BPF_MOV64_IMM(BPF_REG_0, 0x65667830), /* "0xfe" */
- BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
+ /* "0xfe" */
+ BPF_MOV64_IMM(BPF_REG_0,
+ bpf_ntohl(0x30786665)),
+ BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1218,11 +1247,14 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) 9223372036854775807 */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -24),
- BPF_LD_IMM64(BPF_REG_0, 0x3032373333323239ULL),
+ BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
+ 0x3932323333373230ULL)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
- BPF_LD_IMM64(BPF_REG_0, 0x3537373435383633ULL),
+ BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
+ 0x3336383534373735ULL)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 8),
- BPF_LD_IMM64(BPF_REG_0, 0x0000000000373038ULL),
+ BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
+ 0x3830370000000000ULL)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 16),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1266,11 +1298,14 @@ static struct sysctl_test tests[] = {
/* arg1 (buf) 9223372036854775808 */
BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -24),
- BPF_LD_IMM64(BPF_REG_0, 0x3032373333323239ULL),
+ BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
+ 0x3932323333373230ULL)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
- BPF_LD_IMM64(BPF_REG_0, 0x3537373435383633ULL),
+ BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
+ 0x3336383534373735ULL)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 8),
- BPF_LD_IMM64(BPF_REG_0, 0x0000000000383038ULL),
+ BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
+ 0x3830380000000000ULL)),
BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 16),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
@@ -1344,20 +1379,24 @@ static size_t probe_prog_length(const struct bpf_insn *fp)
static int fixup_sysctl_value(const char *buf, size_t buf_len,
struct bpf_insn *prog, size_t insn_num)
{
- uint32_t value_num = 0;
+ union {
+ uint8_t raw[sizeof(uint64_t)];
+ uint64_t num;
+ } value = {};
uint8_t c, i;
- if (buf_len > sizeof(value_num)) {
+ if (buf_len > sizeof(value)) {
log_err("Value is too big (%zd) to use in fixup", buf_len);
return -1;
}
-
- for (i = 0; i < buf_len; ++i) {
- c = buf[i];
- value_num |= (c << i * 8);
+ if (prog[insn_num].code != (BPF_LD | BPF_DW | BPF_IMM)) {
+ log_err("Can fixup only BPF_LD_IMM64 insns");
+ return -1;
}
- prog[insn_num].imm = value_num;
+ memcpy(value.raw, buf, buf_len);
+ prog[insn_num].imm = (uint32_t)value.num;
+ prog[insn_num + 1].imm = (uint32_t)(value.num >> 32);
return 0;
}
@@ -1499,6 +1538,7 @@ static int run_test_case(int cgfd, struct sysctl_test *test)
goto err;
}
+ errno = 0;
if (access_sysctl(sysctl_path, test) == -1) {
if (test->result == OP_EPERM && errno == EPERM)
goto out;
@@ -1507,7 +1547,7 @@ static int run_test_case(int cgfd, struct sysctl_test *test)
}
if (test->result != SUCCESS) {
- log_err("Unexpected failure");
+ log_err("Unexpected success");
goto err;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v3 1/2] selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu
2019-08-28 13:22 ` [PATCH bpf v3 1/2] selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu Ilya Leoshkevich
@ 2019-08-29 21:46 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2019-08-29 21:46 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Daniel Borkmann, Alexei Starovoitov, bpf, Yonghong Song,
Andrey Ignatov, Heiko Carstens, Vasily Gorbik
On Wed, Aug 28, 2019 at 6:23 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> test_lwt_seg6local and test_seg6_loop use custom 64-bit endianness
> conversion macros. Centralize their definitions in bpf_endian.h in order
> to reduce code duplication. This will also be useful when bpf_endian.h
> is promoted to an offical libbpf header.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v3 2/2] selftests/bpf: fix endianness issues in test_sysctl
2019-08-28 13:22 ` [PATCH bpf v3 2/2] selftests/bpf: fix endianness issues in test_sysctl Ilya Leoshkevich
@ 2019-08-29 21:53 ` Song Liu
2019-08-30 10:22 ` Ilya Leoshkevich
0 siblings, 1 reply; 6+ messages in thread
From: Song Liu @ 2019-08-29 21:53 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Daniel Borkmann, Alexei Starovoitov, bpf, Yonghong Song,
Andrey Ignatov, Heiko Carstens, Vasily Gorbik
On Wed, Aug 28, 2019 at 6:22 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> A lot of test_sysctl sub-tests fail due to handling strings as a bunch
> of immediate values in a little-endian-specific manner.
>
> Fix by wrapping all immediates in bpf_ntohl and the new bpf_be64_to_cpu.
>
> Also, sometimes tests fail because sysctl() unexpectedly succeeds with
> an inappropriate "Unexpected failure" message and a random errno. Zero
> out errno before calling sysctl() and replace the message with
> "Unexpected success".
>
> Fixes: 1f5fa9ab6e2e ("selftests/bpf: Test BPF_CGROUP_SYSCTL")
> Fixes: 9a1027e52535 ("selftests/bpf: Test file_pos field in bpf_sysctl ctx")
> Fixes: 6041c67f28d8 ("selftests/bpf: Test bpf_sysctl_get_name helper")
> Fixes: 11ff34f74e32 ("selftests/bpf: Test sysctl_get_current_value helper")
> Fixes: 786047dd08de ("selftests/bpf: Test bpf_sysctl_{get,set}_new_value helpers")
> Fixes: 8549ddc832d6 ("selftests/bpf: Test bpf_strtol and bpf_strtoul helpers")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/testing/selftests/bpf/test_sysctl.c | 130 ++++++++++++++--------
> 1 file changed, 85 insertions(+), 45 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_sysctl.c b/tools/testing/selftests/bpf/test_sysctl.c
> index a3bebd7c68dd..b8c6e31645cb 100644
> --- a/tools/testing/selftests/bpf/test_sysctl.c
> +++ b/tools/testing/selftests/bpf/test_sysctl.c
> @@ -13,6 +13,7 @@
> #include <bpf/bpf.h>
> #include <bpf/libbpf.h>
>
> +#include "bpf_endian.h"
> #include "bpf_rlimit.h"
> #include "bpf_util.h"
> #include "cgroup_helpers.h"
> @@ -100,7 +101,7 @@ static struct sysctl_test tests[] = {
> .descr = "ctx:write sysctl:write read ok",
> .insns = {
> /* If (write) */
> - BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_1,
> + BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
I didn't find explanation for this change in the commit log. Is this a typo, or
a real fix?
> offsetof(struct bpf_sysctl, write)),
> BPF_JMP_IMM(BPF_JNE, BPF_REG_7, 1, 2),
>
> @@ -214,7 +215,8 @@ static struct sysctl_test tests[] = {
> /* if (ret == expected && */
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, sizeof("tcp_mem") - 1, 6),
> /* buf == "tcp_mem\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x006d656d5f706374ULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x7463705f6d656d00ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -255,7 +257,8 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 6),
>
> /* buf[0:7] == "tcp_me\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x00656d5f706374ULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x7463705f6d650000ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -298,12 +301,14 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 16, 14),
>
> /* buf[0:8] == "net/ipv4" && */
> - BPF_LD_IMM64(BPF_REG_8, 0x347670692f74656eULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x6e65742f69707634ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 10),
>
> /* buf[8:16] == "/tcp_mem" && */
> - BPF_LD_IMM64(BPF_REG_8, 0x6d656d5f7063742fULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x2f7463705f6d656dULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 8),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 6),
>
> @@ -350,12 +355,14 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 10),
>
> /* buf[0:8] == "net/ipv4" && */
> - BPF_LD_IMM64(BPF_REG_8, 0x347670692f74656eULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x6e65742f69707634ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 6),
>
> /* buf[8:16] == "/tcp_me\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x00656d5f7063742fULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x2f7463705f6d6500ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 8),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -396,7 +403,8 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 6),
>
> /* buf[0:8] == "net/ip\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x000070692f74656eULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x6e65742f69700000ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -431,7 +439,8 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 6, 6),
>
> /* buf[0:6] == "Linux\n\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x000a78756e694cULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x4c696e75780a0000ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -469,7 +478,8 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 6, 6),
>
> /* buf[0:6] == "Linux\n\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x000a78756e694cULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x4c696e75780a0000ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -507,7 +517,8 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, -E2BIG, 6),
>
> /* buf[0:6] == "Linux\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x000078756e694cULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x4c696e7578000000ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -650,7 +661,8 @@ static struct sysctl_test tests[] = {
>
> /* buf[0:4] == "606\0") */
> BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_7, 0),
> - BPF_JMP_IMM(BPF_JNE, BPF_REG_9, 0x00363036, 2),
> + BPF_JMP_IMM(BPF_JNE, BPF_REG_9,
> + bpf_ntohl(0x36303600), 2),
>
> /* return DENY; */
> BPF_MOV64_IMM(BPF_REG_0, 0),
> @@ -685,17 +697,20 @@ static struct sysctl_test tests[] = {
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 23, 14),
>
> /* buf[0:8] == "3000000 " && */
> - BPF_LD_IMM64(BPF_REG_8, 0x2030303030303033ULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x3330303030303020ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 0),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 10),
>
> /* buf[8:16] == "4000000 " && */
> - BPF_LD_IMM64(BPF_REG_8, 0x2030303030303034ULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x3430303030303020ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 8),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 6),
>
> /* buf[16:24] == "6000000\0") */
> - BPF_LD_IMM64(BPF_REG_8, 0x0030303030303036ULL),
> + BPF_LD_IMM64(BPF_REG_8, bpf_be64_to_cpu(
> + 0x3630303030303000ULL)),
> BPF_LDX_MEM(BPF_DW, BPF_REG_9, BPF_REG_7, 16),
> BPF_JMP_REG(BPF_JNE, BPF_REG_8, BPF_REG_9, 2),
>
> @@ -735,7 +750,8 @@ static struct sysctl_test tests[] = {
>
> /* buf[0:3] == "60\0") */
> BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_7, 0),
> - BPF_JMP_IMM(BPF_JNE, BPF_REG_9, 0x003036, 2),
> + BPF_JMP_IMM(BPF_JNE, BPF_REG_9,
> + bpf_ntohl(0x36300000), 2),
>
> /* return DENY; */
> BPF_MOV64_IMM(BPF_REG_0, 0),
> @@ -757,7 +773,8 @@ static struct sysctl_test tests[] = {
> /* sysctl_set_new_value arg2 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x36303000)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_2, BPF_REG_7),
> @@ -791,7 +808,7 @@ static struct sysctl_test tests[] = {
> /* sysctl_set_new_value arg2 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, FIXUP_SYSCTL_VALUE),
> + BPF_LD_IMM64(BPF_REG_0, FIXUP_SYSCTL_VALUE),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_2, BPF_REG_7),
> @@ -825,8 +842,9 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
> - BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x36303000)),
> + BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
>
> @@ -869,7 +887,8 @@ static struct sysctl_test tests[] = {
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> /* "600 602\0" */
> - BPF_LD_IMM64(BPF_REG_0, 0x0032303620303036ULL),
> + BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
> + 0x3630302036303200ULL)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
>
> @@ -937,7 +956,8 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x36303000)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
> @@ -969,8 +989,9 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x00373730),
> - BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x30373700)),
> + BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
>
> @@ -1012,7 +1033,8 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x00303036),
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x36303000)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
> @@ -1052,7 +1074,8 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x090a0c0d),
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x0d0c0a09)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
> @@ -1092,7 +1115,9 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x00362d0a), /* " -6\0" */
> + /* " -6\0" */
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x0a2d3600)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
> @@ -1132,8 +1157,10 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x00362d0a), /* " -6\0" */
> - BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
> + /* " -6\0" */
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x0a2d3600)),
> + BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
>
> @@ -1175,8 +1202,10 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -8),
> - BPF_MOV64_IMM(BPF_REG_0, 0x65667830), /* "0xfe" */
> - BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
> + /* "0xfe" */
> + BPF_MOV64_IMM(BPF_REG_0,
> + bpf_ntohl(0x30786665)),
> + BPF_STX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, 0),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
>
> @@ -1218,11 +1247,14 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) 9223372036854775807 */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -24),
> - BPF_LD_IMM64(BPF_REG_0, 0x3032373333323239ULL),
> + BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
> + 0x3932323333373230ULL)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
> - BPF_LD_IMM64(BPF_REG_0, 0x3537373435383633ULL),
> + BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
> + 0x3336383534373735ULL)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 8),
> - BPF_LD_IMM64(BPF_REG_0, 0x0000000000373038ULL),
> + BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
> + 0x3830370000000000ULL)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 16),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
> @@ -1266,11 +1298,14 @@ static struct sysctl_test tests[] = {
> /* arg1 (buf) 9223372036854775808 */
> BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
> BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, -24),
> - BPF_LD_IMM64(BPF_REG_0, 0x3032373333323239ULL),
> + BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
> + 0x3932323333373230ULL)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 0),
> - BPF_LD_IMM64(BPF_REG_0, 0x3537373435383633ULL),
> + BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
> + 0x3336383534373735ULL)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 8),
> - BPF_LD_IMM64(BPF_REG_0, 0x0000000000383038ULL),
> + BPF_LD_IMM64(BPF_REG_0, bpf_be64_to_cpu(
> + 0x3830380000000000ULL)),
> BPF_STX_MEM(BPF_DW, BPF_REG_7, BPF_REG_0, 16),
>
> BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
> @@ -1344,20 +1379,24 @@ static size_t probe_prog_length(const struct bpf_insn *fp)
> static int fixup_sysctl_value(const char *buf, size_t buf_len,
> struct bpf_insn *prog, size_t insn_num)
> {
> - uint32_t value_num = 0;
> + union {
> + uint8_t raw[sizeof(uint64_t)];
> + uint64_t num;
> + } value = {};
This change doesn't match the description in the changelog.
> uint8_t c, i;
>
> - if (buf_len > sizeof(value_num)) {
> + if (buf_len > sizeof(value)) {
> log_err("Value is too big (%zd) to use in fixup", buf_len);
> return -1;
> }
> -
> - for (i = 0; i < buf_len; ++i) {
> - c = buf[i];
> - value_num |= (c << i * 8);
> + if (prog[insn_num].code != (BPF_LD | BPF_DW | BPF_IMM)) {
> + log_err("Can fixup only BPF_LD_IMM64 insns");
> + return -1;
> }
>
> - prog[insn_num].imm = value_num;
> + memcpy(value.raw, buf, buf_len);
> + prog[insn_num].imm = (uint32_t)value.num;
> + prog[insn_num + 1].imm = (uint32_t)(value.num >> 32);
>
> return 0;
> }
> @@ -1499,6 +1538,7 @@ static int run_test_case(int cgfd, struct sysctl_test *test)
> goto err;
> }
>
> + errno = 0;
> if (access_sysctl(sysctl_path, test) == -1) {
> if (test->result == OP_EPERM && errno == EPERM)
> goto out;
> @@ -1507,7 +1547,7 @@ static int run_test_case(int cgfd, struct sysctl_test *test)
> }
>
> if (test->result != SUCCESS) {
> - log_err("Unexpected failure");
> + log_err("Unexpected success");
> goto err;
> }
>
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v3 2/2] selftests/bpf: fix endianness issues in test_sysctl
2019-08-29 21:53 ` Song Liu
@ 2019-08-30 10:22 ` Ilya Leoshkevich
0 siblings, 0 replies; 6+ messages in thread
From: Ilya Leoshkevich @ 2019-08-30 10:22 UTC (permalink / raw)
To: Song Liu
Cc: Daniel Borkmann, Alexei Starovoitov, bpf, Yonghong Song,
Andrey Ignatov, Heiko Carstens, Vasily Gorbik
> Am 29.08.2019 um 23:53 schrieb Song Liu <liu.song.a23@gmail.com>:
>
> On Wed, Aug 28, 2019 at 6:22 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> A lot of test_sysctl sub-tests fail due to handling strings as a bunch
>> of immediate values in a little-endian-specific manner.
>>
>> Fix by wrapping all immediates in bpf_ntohl and the new bpf_be64_to_cpu.
>>
>> Also, sometimes tests fail because sysctl() unexpectedly succeeds with
>> an inappropriate "Unexpected failure" message and a random errno. Zero
>> out errno before calling sysctl() and replace the message with
>> "Unexpected success".
...
>> @@ -13,6 +13,7 @@
>> #include <bpf/bpf.h>
>> #include <bpf/libbpf.h>
>>
>> +#include "bpf_endian.h"
>> #include "bpf_rlimit.h"
>> #include "bpf_util.h"
>> #include "cgroup_helpers.h"
>> @@ -100,7 +101,7 @@ static struct sysctl_test tests[] = {
>> .descr = "ctx:write sysctl:write read ok",
>> .insns = {
>> /* If (write) */
>> - BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_1,
>> + BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
>
> I didn't find explanation for this change in the commit log. Is this a typo, or
> a real fix?
This is a real fix, but I forgot to explain it. We read the first byte of an
int assuming it's the least-significant one, which is not the case on BE arches.
Two fixes are possible: use a different offset on BE arches or just read the
whole int. Since we are not testing narrow accesses here (there is e.g.
"ctx:file_pos sysctl:read read ok narrow" for that), I went with the simplest
solution.
...
>> @@ -1344,20 +1379,24 @@ static size_t probe_prog_length(const struct bpf_insn *fp)
>> static int fixup_sysctl_value(const char *buf, size_t buf_len,
>> struct bpf_insn *prog, size_t insn_num)
>> {
>> - uint32_t value_num = 0;
>> + union {
>> + uint8_t raw[sizeof(uint64_t)];
>> + uint64_t num;
>> + } value = {};
>
> This change doesn't match the description in the changelog.
This one I also forgot to explain - writing an immediate into things like
BPF_LD_IMM64(BPF_REG_0, FIXUP_SYSCTL_VALUE) should be endianness-aware. We can
also do bpf_be64_to_cpu or similar here, but a better trick (suggested by
Yonghong) is to simply memcpy the raw user-provided value, since testcase
endianness and bpf program endianness match.
Let me send a v4 with an improved commit message.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-30 10:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-28 13:22 [PATCH bpf v3 0/2] selftests/bpf: fix endianness issues in test_sysctl Ilya Leoshkevich
2019-08-28 13:22 ` [PATCH bpf v3 1/2] selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu Ilya Leoshkevich
2019-08-29 21:46 ` Song Liu
2019-08-28 13:22 ` [PATCH bpf v3 2/2] selftests/bpf: fix endianness issues in test_sysctl Ilya Leoshkevich
2019-08-29 21:53 ` Song Liu
2019-08-30 10:22 ` Ilya Leoshkevich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox