selftests/bpf: fix endianness issues in test_sysctl
Subsystems (3)
v3 cover selftests/bpf: fix endianness issues in test_sysctl
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(-)
Signature · 1 line
2.21.0
PATCH v3 1/2 selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu
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.
--- 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(-)
tools/testing/selftests/bpf/bpf_endian.h +14 -0
@@ -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__ */
tools/testing/selftests/bpf/progs/test_lwt_seg6local.c +6 -10
@@ -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)
tools/testing/selftests/bpf/progs/test_seg6_loop.c +2 -6
@@ -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)
Signature · 1 line
2.21.0
Re: [PATCH bpf v3 1/2] selftests/bpf: introduce bpf_cpu_to_be64 and bpf_be64_to_cpu
Quoted 7 lines · Ilya Leoshkevich
test_lwt_seg6localandtest_seg6_loopuse custom 64-bit endianness conversion macros. Centralize their definitions inbpf_endian.hin order to reduce code duplication. This will also be useful whenbpf_endian.his promoted to an offical libbpf header.Signed-off-by Ilya Leoshkevich <[email protected]>
PATCH v3 2/2 selftests/bpf: fix endianness issues in test_sysctl
In reply to Ilya LeoshkevichA 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".
--- tools/testing/selftests/bpf/test_sysctl.c | 130 ++++++++++++++-------- 1 file changed, 85 insertions(+), 45 deletions(-)
tools/testing/selftests/bpf/test_sysctl.c +85 -45
@@ -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; }
Signature · 1 line
2.21.0
Quoted 35 lines · Ilya Leoshkevich
A lot of
test_sysctlsub-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_ntohland the newbpf_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 callingsysctl()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 <[email protected]>--- tools/testing/selftests/bpf/test_sysctl.c | 130 ++++++++++++++-------- 1 file changed, 85 insertions(+), 45 deletions(-)tools/testing/selftests/bpf/test_sysctl.c +1
-0@@ -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"
tools/testing/selftests/bpf/test_sysctl.c static struct sysctl_test tests[] = { · line 104
Quoted 5 lines
tools/testing/selftests/bpf/test_sysctl.c +1
-1@@ -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?
Quoted 298 lines
tools/testing/selftests/bpf/test_sysctl.c +70
-36offsetof(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),
tools/testing/selftests/bpf/test_sysctl.c static size_t probe_prog_length(const struct bpf_insn *fp) · line 1385
Quoted 8 lines
tools/testing/selftests/bpf/test_sysctl.c +4
-1@@ -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.
Quoted 43 lines
tools/testing/selftests/bpf/test_sysctl.c +9
-8uint8_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
Quoted 13 lines
Am 29.08.2019 um 23:53 schrieb Song Liu <[email protected]>:
Quoted 10 lines · Ilya Leoshkevich
A lot of
test_sysctlsub-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_ntohland the newbpf_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 callingsysctl()and replace the message with "Unexpected success".
...
Quoted 4 lines
Quoted 4 lines
tools/testing/selftests/bpf/test_sysctl.c +0
-0@@ -13,6 +13,7 @@#include <bpf/bpf.h> #include <bpf/libbpf.h>
Quoted 13 lines
tools/testing/selftests/bpf/test_sysctl.c static struct sysctl_test tests[] = { · line 104
Quoted 10 lines
tools/testing/selftests/bpf/test_sysctl.c +2
-1+#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.
...
Quoted 11 lines
tools/testing/selftests/bpf/test_sysctl.c static size_t probe_prog_length(const struct bpf_insn *fp) · line 1385
Quoted 9 lines
tools/testing/selftests/bpf/test_sysctl.c +4
-1@@ -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.