* [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
@ 2019-07-11 14:29 Ilya Leoshkevich
2019-07-11 14:29 ` [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH) Ilya Leoshkevich
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Ilya Leoshkevich @ 2019-07-11 14:29 UTC (permalink / raw)
To: bpf, netdev; +Cc: ys114321, daniel, sdf, davem, ast, Ilya Leoshkevich
Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
This patch series consists of three preparatory commits, which make it
possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
> > Will this also work for 32-bit x86?
> Thanks, this is a good catch: this builds, but makes 64-bit accesses, as
> if it used the 64-bit variant of pt_regs. I will fix this.
I found four problems in this area:
1. Selftest tracing progs are built with -target bpf, leading to struct
pt_regs and friends being interpreted incorrectly.
2. When the Makefile is adjusted to build them without -target bpf, it
still lacks -m32/-m64, leading to a similar issue.
3. There is no __i386__ define, leading to incorrect userspace struct
pt_regs variant being chosen for x86.
4. Finally, there is an issue in my patch: when 1-3 are fixed, it fails
to build, since i386 defines yet another set of field names.
I will send fixes for problems 1-3 separately, I believe for this patch
series to be correct, it's enough to fix #4 (which I did by adding
another #ifdef).
I've also changed ARCH to SRCARCH in patch #1, since while ARCH can be
e.g. "i386", SRCARCH always corresponds to directory names under arch/.
v1->v2: Split into multiple patches.
v2->v3: Added arm64 support.
v3->v4: Added i386 support, use SRCARCH instead of ARCH.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH)
2019-07-11 14:29 [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
@ 2019-07-11 14:29 ` Ilya Leoshkevich
2019-07-12 0:53 ` Andrii Nakryiko
2019-07-11 14:29 ` [PATCH v4 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo Ilya Leoshkevich
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2019-07-11 14:29 UTC (permalink / raw)
To: bpf, netdev; +Cc: ys114321, daniel, sdf, davem, ast, Ilya Leoshkevich
This opens up the possibility of accessing registers in an
arch-independent way.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/Makefile | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 2620406a53ec..ad84450e4ab8 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
+include ../../../scripts/Makefile.arch
LIBDIR := ../../../lib
BPFDIR := $(LIBDIR)/bpf
@@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
$(CLANG_SYS_INCLUDES) \
- -Wno-compare-distinct-pointer-types
+ -Wno-compare-distinct-pointer-types \
+ -D__TARGET_ARCH_$(SRCARCH)
$(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline
$(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo
2019-07-11 14:29 [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-11 14:29 ` [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH) Ilya Leoshkevich
@ 2019-07-11 14:29 ` Ilya Leoshkevich
2019-07-12 0:50 ` Andrii Nakryiko
2019-07-11 14:29 ` [PATCH v4 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2019-07-11 14:29 UTC (permalink / raw)
To: bpf, netdev; +Cc: ys114321, daniel, sdf, davem, ast, Ilya Leoshkevich
Also check for __s390__ instead of __s390x__, just in case bpf_helpers.h
is ever used by 32-bit userspace.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/bpf_helpers.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 5a3d92c8bec8..73071a94769a 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -315,8 +315,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#if defined(__TARGET_ARCH_x86)
#define bpf_target_x86
#define bpf_target_defined
-#elif defined(__TARGET_ARCH_s930x)
- #define bpf_target_s930x
+#elif defined(__TARGET_ARCH_s390)
+ #define bpf_target_s390
#define bpf_target_defined
#elif defined(__TARGET_ARCH_arm)
#define bpf_target_arm
@@ -341,8 +341,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#ifndef bpf_target_defined
#if defined(__x86_64__)
#define bpf_target_x86
-#elif defined(__s390x__)
- #define bpf_target_s930x
+#elif defined(__s390__)
+ #define bpf_target_s390
#elif defined(__arm__)
#define bpf_target_arm
#elif defined(__aarch64__)
@@ -369,7 +369,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#define PT_REGS_SP(x) ((x)->sp)
#define PT_REGS_IP(x) ((x)->ip)
-#elif defined(bpf_target_s390x)
+#elif defined(bpf_target_s390)
#define PT_REGS_PARM1(x) ((x)->gprs[2])
#define PT_REGS_PARM2(x) ((x)->gprs[3])
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace
2019-07-11 14:29 [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-11 14:29 ` [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH) Ilya Leoshkevich
2019-07-11 14:29 ` [PATCH v4 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo Ilya Leoshkevich
@ 2019-07-11 14:29 ` Ilya Leoshkevich
2019-07-12 0:49 ` Andrii Nakryiko
2019-07-11 14:29 ` [PATCH v4 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-11 20:35 ` [PATCH v4 bpf-next 0/4] " Stanislav Fomichev
4 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2019-07-11 14:29 UTC (permalink / raw)
To: bpf, netdev; +Cc: ys114321, daniel, sdf, davem, ast, Ilya Leoshkevich
Right now, on certain architectures, these macros are usable only with
kernel headers. This patch makes it possible to use them with userspace
headers and, as a consequence, not only in BPF samples, but also in BPF
selftests.
On s390, provide the forward declaration of struct pt_regs and cast it
to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
of the full struct pt_regs, s390 exposes only its first member
user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
(in selftests) and kernel (in samples) headers. It was added in commit
466698e654e8 ("s390/bpf: correct broken uapi for
BPF_PROG_TYPE_PERF_EVENT program type").
Ditto on arm64.
On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and
arm64, x86 provides struct pt_regs to both userspace and kernel, however,
with different member names.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/bpf_helpers.h | 75 +++++++++++++++++------
1 file changed, 55 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 73071a94769a..27090d94afb6 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -358,6 +358,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#if defined(bpf_target_x86)
+#ifdef __KERNEL__
#define PT_REGS_PARM1(x) ((x)->di)
#define PT_REGS_PARM2(x) ((x)->si)
#define PT_REGS_PARM3(x) ((x)->dx)
@@ -368,19 +369,49 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#define PT_REGS_RC(x) ((x)->ax)
#define PT_REGS_SP(x) ((x)->sp)
#define PT_REGS_IP(x) ((x)->ip)
+#else
+#ifdef __i386__
+/* i386 kernel is built with -mregparm=3 */
+#define PT_REGS_PARM1(x) ((x)->eax)
+#define PT_REGS_PARM2(x) ((x)->edx)
+#define PT_REGS_PARM3(x) ((x)->ecx)
+#define PT_REGS_PARM4(x) 0
+#define PT_REGS_PARM5(x) 0
+#define PT_REGS_RET(x) ((x)->esp)
+#define PT_REGS_FP(x) ((x)->ebp)
+#define PT_REGS_RC(x) ((x)->eax)
+#define PT_REGS_SP(x) ((x)->esp)
+#define PT_REGS_IP(x) ((x)->eip)
+#else
+#define PT_REGS_PARM1(x) ((x)->rdi)
+#define PT_REGS_PARM2(x) ((x)->rsi)
+#define PT_REGS_PARM3(x) ((x)->rdx)
+#define PT_REGS_PARM4(x) ((x)->rcx)
+#define PT_REGS_PARM5(x) ((x)->r8)
+#define PT_REGS_RET(x) ((x)->rsp)
+#define PT_REGS_FP(x) ((x)->rbp)
+#define PT_REGS_RC(x) ((x)->rax)
+#define PT_REGS_SP(x) ((x)->rsp)
+#define PT_REGS_IP(x) ((x)->rip)
+#endif
+#endif
#elif defined(bpf_target_s390)
-#define PT_REGS_PARM1(x) ((x)->gprs[2])
-#define PT_REGS_PARM2(x) ((x)->gprs[3])
-#define PT_REGS_PARM3(x) ((x)->gprs[4])
-#define PT_REGS_PARM4(x) ((x)->gprs[5])
-#define PT_REGS_PARM5(x) ((x)->gprs[6])
-#define PT_REGS_RET(x) ((x)->gprs[14])
-#define PT_REGS_FP(x) ((x)->gprs[11]) /* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_RC(x) ((x)->gprs[2])
-#define PT_REGS_SP(x) ((x)->gprs[15])
-#define PT_REGS_IP(x) ((x)->psw.addr)
+/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
+struct pt_regs;
+#define PT_REGS_S390 const volatile user_pt_regs
+#define PT_REGS_PARM1(x) (((PT_REGS_S390 *)(x))->gprs[2])
+#define PT_REGS_PARM2(x) (((PT_REGS_S390 *)(x))->gprs[3])
+#define PT_REGS_PARM3(x) (((PT_REGS_S390 *)(x))->gprs[4])
+#define PT_REGS_PARM4(x) (((PT_REGS_S390 *)(x))->gprs[5])
+#define PT_REGS_PARM5(x) (((PT_REGS_S390 *)(x))->gprs[6])
+#define PT_REGS_RET(x) (((PT_REGS_S390 *)(x))->gprs[14])
+/* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_FP(x) (((PT_REGS_S390 *)(x))->gprs[11])
+#define PT_REGS_RC(x) (((PT_REGS_S390 *)(x))->gprs[2])
+#define PT_REGS_SP(x) (((PT_REGS_S390 *)(x))->gprs[15])
+#define PT_REGS_IP(x) (((PT_REGS_S390 *)(x))->psw.addr)
#elif defined(bpf_target_arm)
@@ -397,16 +428,20 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#elif defined(bpf_target_arm64)
-#define PT_REGS_PARM1(x) ((x)->regs[0])
-#define PT_REGS_PARM2(x) ((x)->regs[1])
-#define PT_REGS_PARM3(x) ((x)->regs[2])
-#define PT_REGS_PARM4(x) ((x)->regs[3])
-#define PT_REGS_PARM5(x) ((x)->regs[4])
-#define PT_REGS_RET(x) ((x)->regs[30])
-#define PT_REGS_FP(x) ((x)->regs[29]) /* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_RC(x) ((x)->regs[0])
-#define PT_REGS_SP(x) ((x)->sp)
-#define PT_REGS_IP(x) ((x)->pc)
+/* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
+struct pt_regs;
+#define PT_REGS_ARM64 const volatile struct user_pt_regs
+#define PT_REGS_PARM1(x) (((PT_REGS_ARM64 *)(x))->regs[0])
+#define PT_REGS_PARM2(x) (((PT_REGS_ARM64 *)(x))->regs[1])
+#define PT_REGS_PARM3(x) (((PT_REGS_ARM64 *)(x))->regs[2])
+#define PT_REGS_PARM4(x) (((PT_REGS_ARM64 *)(x))->regs[3])
+#define PT_REGS_PARM5(x) (((PT_REGS_ARM64 *)(x))->regs[4])
+#define PT_REGS_RET(x) (((PT_REGS_ARM64 *)(x))->regs[30])
+/* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_FP(x) (((PT_REGS_ARM64 *)(x))->regs[29])
+#define PT_REGS_RC(x) (((PT_REGS_ARM64 *)(x))->regs[0])
+#define PT_REGS_SP(x) (((PT_REGS_ARM64 *)(x))->sp)
+#define PT_REGS_IP(x) (((PT_REGS_ARM64 *)(x))->pc)
#elif defined(bpf_target_mips)
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
2019-07-11 14:29 [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
` (2 preceding siblings ...)
2019-07-11 14:29 ` [PATCH v4 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
@ 2019-07-11 14:29 ` Ilya Leoshkevich
2019-07-11 20:35 ` [PATCH v4 bpf-next 0/4] " Stanislav Fomichev
4 siblings, 0 replies; 13+ messages in thread
From: Ilya Leoshkevich @ 2019-07-11 14:29 UTC (permalink / raw)
To: bpf, netdev; +Cc: ys114321, daniel, sdf, davem, ast, Ilya Leoshkevich
Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/progs/loop1.c | 2 +-
tools/testing/selftests/bpf/progs/loop2.c | 2 +-
tools/testing/selftests/bpf/progs/loop3.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/loop1.c b/tools/testing/selftests/bpf/progs/loop1.c
index dea395af9ea9..7cdb7f878310 100644
--- a/tools/testing/selftests/bpf/progs/loop1.c
+++ b/tools/testing/selftests/bpf/progs/loop1.c
@@ -18,7 +18,7 @@ int nested_loops(volatile struct pt_regs* ctx)
for (j = 0; j < 300; j++)
for (i = 0; i < j; i++) {
if (j & 1)
- m = ctx->rax;
+ m = PT_REGS_RC(ctx);
else
m = j;
sum += i * m;
diff --git a/tools/testing/selftests/bpf/progs/loop2.c b/tools/testing/selftests/bpf/progs/loop2.c
index 0637bd8e8bcf..9b2f808a2863 100644
--- a/tools/testing/selftests/bpf/progs/loop2.c
+++ b/tools/testing/selftests/bpf/progs/loop2.c
@@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx)
int i = 0;
while (true) {
- if (ctx->rax & 1)
+ if (PT_REGS_RC(ctx) & 1)
i += 3;
else
i += 7;
diff --git a/tools/testing/selftests/bpf/progs/loop3.c b/tools/testing/selftests/bpf/progs/loop3.c
index 30a0f6cba080..d727657d51e2 100644
--- a/tools/testing/selftests/bpf/progs/loop3.c
+++ b/tools/testing/selftests/bpf/progs/loop3.c
@@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx)
__u64 i = 0, sum = 0;
do {
i++;
- sum += ctx->rax;
+ sum += PT_REGS_RC(ctx);
} while (i < 0x100000000ULL);
return sum;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
2019-07-11 14:29 [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
` (3 preceding siblings ...)
2019-07-11 14:29 ` [PATCH v4 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
@ 2019-07-11 20:35 ` Stanislav Fomichev
2019-07-12 8:55 ` Ilya Leoshkevich
4 siblings, 1 reply; 13+ messages in thread
From: Stanislav Fomichev @ 2019-07-11 20:35 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, netdev, ys114321, daniel, davem, ast
On 07/11, Ilya Leoshkevich wrote:
> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
>
> This patch series consists of three preparatory commits, which make it
> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
>
> > > Will this also work for 32-bit x86?
> > Thanks, this is a good catch: this builds, but makes 64-bit accesses, as
> > if it used the 64-bit variant of pt_regs. I will fix this.
> I found four problems in this area:
>
> 1. Selftest tracing progs are built with -target bpf, leading to struct
> pt_regs and friends being interpreted incorrectly.
> 2. When the Makefile is adjusted to build them without -target bpf, it
> still lacks -m32/-m64, leading to a similar issue.
> 3. There is no __i386__ define, leading to incorrect userspace struct
> pt_regs variant being chosen for x86.
> 4. Finally, there is an issue in my patch: when 1-3 are fixed, it fails
> to build, since i386 defines yet another set of field names.
>
> I will send fixes for problems 1-3 separately, I believe for this patch
> series to be correct, it's enough to fix #4 (which I did by adding
> another #ifdef).
>
> I've also changed ARCH to SRCARCH in patch #1, since while ARCH can be
> e.g. "i386", SRCARCH always corresponds to directory names under arch/.
>
> v1->v2: Split into multiple patches.
> v2->v3: Added arm64 support.
> v3->v4: Added i386 support, use SRCARCH instead of ARCH.
Still looks good to me, thanks!
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Again, should probably go via bpf to fix the existing tests, not bpf-next
(but I see bpf tree is not synced with net tree yet).
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace
2019-07-11 14:29 ` [PATCH v4 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
@ 2019-07-12 0:49 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-07-12 0:49 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: bpf, Networking, Y Song, Daniel Borkmann, Stanislav Fomichev,
David S. Miller, Alexei Starovoitov
On Thu, Jul 11, 2019 at 7:31 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Right now, on certain architectures, these macros are usable only with
> kernel headers. This patch makes it possible to use them with userspace
> headers and, as a consequence, not only in BPF samples, but also in BPF
> selftests.
>
> On s390, provide the forward declaration of struct pt_regs and cast it
> to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
> of the full struct pt_regs, s390 exposes only its first member
> user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
> (in selftests) and kernel (in samples) headers. It was added in commit
> 466698e654e8 ("s390/bpf: correct broken uapi for
> BPF_PROG_TYPE_PERF_EVENT program type").
>
> Ditto on arm64.
>
> On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and
> arm64, x86 provides struct pt_regs to both userspace and kernel, however,
> with different member names.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
This is great, thanks!
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/testing/selftests/bpf/bpf_helpers.h | 75 +++++++++++++++++------
> 1 file changed, 55 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 73071a94769a..27090d94afb6 100644
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo
2019-07-11 14:29 ` [PATCH v4 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo Ilya Leoshkevich
@ 2019-07-12 0:50 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-07-12 0:50 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: bpf, Networking, Y Song, Daniel Borkmann, Stanislav Fomichev,
David S. Miller, Alexei Starovoitov
On Thu, Jul 11, 2019 at 7:31 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Also check for __s390__ instead of __s390x__, just in case bpf_helpers.h
> is ever used by 32-bit userspace.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/testing/selftests/bpf/bpf_helpers.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 5a3d92c8bec8..73071a94769a 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -315,8 +315,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> #if defined(__TARGET_ARCH_x86)
> #define bpf_target_x86
> #define bpf_target_defined
> -#elif defined(__TARGET_ARCH_s930x)
> - #define bpf_target_s930x
> +#elif defined(__TARGET_ARCH_s390)
> + #define bpf_target_s390
> #define bpf_target_defined
> #elif defined(__TARGET_ARCH_arm)
> #define bpf_target_arm
> @@ -341,8 +341,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> #ifndef bpf_target_defined
> #if defined(__x86_64__)
> #define bpf_target_x86
> -#elif defined(__s390x__)
> - #define bpf_target_s930x
> +#elif defined(__s390__)
> + #define bpf_target_s390
> #elif defined(__arm__)
> #define bpf_target_arm
> #elif defined(__aarch64__)
> @@ -369,7 +369,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> #define PT_REGS_SP(x) ((x)->sp)
> #define PT_REGS_IP(x) ((x)->ip)
>
> -#elif defined(bpf_target_s390x)
> +#elif defined(bpf_target_s390)
>
> #define PT_REGS_PARM1(x) ((x)->gprs[2])
> #define PT_REGS_PARM2(x) ((x)->gprs[3])
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH)
2019-07-11 14:29 ` [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH) Ilya Leoshkevich
@ 2019-07-12 0:53 ` Andrii Nakryiko
2019-07-12 8:59 ` Ilya Leoshkevich
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2019-07-12 0:53 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: bpf, Networking, Y Song, Daniel Borkmann, Stanislav Fomichev,
David S. Miller, Alexei Starovoitov
On Thu, Jul 11, 2019 at 7:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> This opens up the possibility of accessing registers in an
> arch-independent way.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/testing/selftests/bpf/Makefile | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 2620406a53ec..ad84450e4ab8 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> +include ../../../scripts/Makefile.arch
>
> LIBDIR := ../../../lib
> BPFDIR := $(LIBDIR)/bpf
> @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
>
> CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> $(CLANG_SYS_INCLUDES) \
> - -Wno-compare-distinct-pointer-types
> + -Wno-compare-distinct-pointer-types \
> + -D__TARGET_ARCH_$(SRCARCH)
samples/bpf/Makefile uses $(ARCH), why does it work for samples?
Should we update samples/bpf/Makefile as well?
>
> $(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline
> $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
2019-07-11 20:35 ` [PATCH v4 bpf-next 0/4] " Stanislav Fomichev
@ 2019-07-12 8:55 ` Ilya Leoshkevich
2019-07-12 13:44 ` Daniel Borkmann
0 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2019-07-12 8:55 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: bpf, Networking, Y Song, daniel, davem, ast
> Am 11.07.2019 um 22:35 schrieb Stanislav Fomichev <sdf@fomichev.me>:
>
> On 07/11, Ilya Leoshkevich wrote:
>> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
>>
>> This patch series consists of three preparatory commits, which make it
>> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
>>
> Still looks good to me, thanks!
>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
>
> Again, should probably go via bpf to fix the existing tests, not bpf-next
> (but I see bpf tree is not synced with net tree yet).
Sorry, I missed your comment the last time. You are right - that’s the
reason I’ve been sending this to bpf-next so far — loop*.c don’t even
exist in the bpf tree.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH)
2019-07-12 0:53 ` Andrii Nakryiko
@ 2019-07-12 8:59 ` Ilya Leoshkevich
2019-07-12 17:28 ` Andrii Nakryiko
0 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2019-07-12 8:59 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Y Song, Daniel Borkmann, Stanislav Fomichev,
David S. Miller, Alexei Starovoitov
> Am 12.07.2019 um 02:53 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
>
> On Thu, Jul 11, 2019 at 7:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> This opens up the possibility of accessing registers in an
>> arch-independent way.
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>> tools/testing/selftests/bpf/Makefile | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 2620406a53ec..ad84450e4ab8 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -1,4 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0
>> +include ../../../scripts/Makefile.arch
>>
>> LIBDIR := ../../../lib
>> BPFDIR := $(LIBDIR)/bpf
>> @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
>>
>> CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
>> $(CLANG_SYS_INCLUDES) \
>> - -Wno-compare-distinct-pointer-types
>> + -Wno-compare-distinct-pointer-types \
>> + -D__TARGET_ARCH_$(SRCARCH)
>
> samples/bpf/Makefile uses $(ARCH), why does it work for samples?
> Should we update samples/bpf/Makefile as well?
I believe that in common cases both are okay, but judging by
linux:Makefile and linux:tools/scripts/Makefile.arch, one could use e.g.
ARCH=i686, and that would be converted to SRCARCH=x86. So IMHO SRCARCH
is safer, and we should change bpf/samples/Makefile. I could send a
patch separately.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
2019-07-12 8:55 ` Ilya Leoshkevich
@ 2019-07-12 13:44 ` Daniel Borkmann
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Borkmann @ 2019-07-12 13:44 UTC (permalink / raw)
To: Ilya Leoshkevich, Stanislav Fomichev; +Cc: bpf, Networking, Y Song, davem, ast
On 07/12/2019 10:55 AM, Ilya Leoshkevich wrote:
>> Am 11.07.2019 um 22:35 schrieb Stanislav Fomichev <sdf@fomichev.me>:
>>
>> On 07/11, Ilya Leoshkevich wrote:
>>> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
>>>
>>> This patch series consists of three preparatory commits, which make it
>>> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
>>>
>> Still looks good to me, thanks!
>>
>> Reviewed-by: Stanislav Fomichev <sdf@google.com>
>>
>> Again, should probably go via bpf to fix the existing tests, not bpf-next
>> (but I see bpf tree is not synced with net tree yet).
>
> Sorry, I missed your comment the last time. You are right - that’s the
> reason I’ve been sending this to bpf-next so far — loop*.c don’t even
> exist in the bpf tree.
Applied to bpf tree (and also added Stanislav's Tested-by to the last
one), thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH)
2019-07-12 8:59 ` Ilya Leoshkevich
@ 2019-07-12 17:28 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-07-12 17:28 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: bpf, Networking, Y Song, Daniel Borkmann, Stanislav Fomichev,
David S. Miller, Alexei Starovoitov
On Fri, Jul 12, 2019 at 2:00 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> > Am 12.07.2019 um 02:53 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
> >
> > On Thu, Jul 11, 2019 at 7:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >>
> >> This opens up the possibility of accessing registers in an
> >> arch-independent way.
> >>
> >> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> >> ---
> >> tools/testing/selftests/bpf/Makefile | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >> index 2620406a53ec..ad84450e4ab8 100644
> >> --- a/tools/testing/selftests/bpf/Makefile
> >> +++ b/tools/testing/selftests/bpf/Makefile
> >> @@ -1,4 +1,5 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> +include ../../../scripts/Makefile.arch
> >>
> >> LIBDIR := ../../../lib
> >> BPFDIR := $(LIBDIR)/bpf
> >> @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
> >>
> >> CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >> $(CLANG_SYS_INCLUDES) \
> >> - -Wno-compare-distinct-pointer-types
> >> + -Wno-compare-distinct-pointer-types \
> >> + -D__TARGET_ARCH_$(SRCARCH)
> >
> > samples/bpf/Makefile uses $(ARCH), why does it work for samples?
> > Should we update samples/bpf/Makefile as well?
>
> I believe that in common cases both are okay, but judging by
> linux:Makefile and linux:tools/scripts/Makefile.arch, one could use e.g.
> ARCH=i686, and that would be converted to SRCARCH=x86. So IMHO SRCARCH
> is safer, and we should change bpf/samples/Makefile. I could send a
> patch separately.
Yeah, let's do that then, thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-07-12 17:28 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-11 14:29 [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-11 14:29 ` [PATCH v4 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(SRCARCH) Ilya Leoshkevich
2019-07-12 0:53 ` Andrii Nakryiko
2019-07-12 8:59 ` Ilya Leoshkevich
2019-07-12 17:28 ` Andrii Nakryiko
2019-07-11 14:29 ` [PATCH v4 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo Ilya Leoshkevich
2019-07-12 0:50 ` Andrii Nakryiko
2019-07-11 14:29 ` [PATCH v4 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
2019-07-12 0:49 ` Andrii Nakryiko
2019-07-11 14:29 ` [PATCH v4 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-11 20:35 ` [PATCH v4 bpf-next 0/4] " Stanislav Fomichev
2019-07-12 8:55 ` Ilya Leoshkevich
2019-07-12 13:44 ` Daniel Borkmann
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).