* [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
@ 2021-12-14 13:55 Pu Lehui
2021-12-14 20:01 ` Daniel Borkmann
2021-12-16 4:06 ` Andrii Nakryiko
0 siblings, 2 replies; 13+ messages in thread
From: Pu Lehui @ 2021-12-14 13:55 UTC (permalink / raw)
To: ast, daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
kpsingh, paul.walmsley, palmer, aou, shuah
Cc: linux-kselftest, netdev, bpf, linux-kernel, pulehui
When building bpf selftests on arm64, the following error will occur:
progs/loop2.c:20:7: error: incomplete definition of type 'struct
user_pt_regs'
Some archs, like arm64 and riscv, use userspace pt_regs in
bpf_tracing.h, which causes build failure when bpf prog use
macro in bpf_tracing.h. So let's use vmlinux.h directly.
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
tools/testing/selftests/bpf/progs/loop6.c | 20 ++++++-------------
.../selftests/bpf/progs/test_overhead.c | 8 ++------
.../selftests/bpf/progs/test_probe_user.c | 6 +-----
6 files changed, 15 insertions(+), 43 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/loop1.c b/tools/testing/selftests/bpf/progs/loop1.c
index 50e66772c046..ea04c035719c 100644
--- a/tools/testing/selftests/bpf/progs/loop1.c
+++ b/tools/testing/selftests/bpf/progs/loop1.c
@@ -1,11 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2019 Facebook
-#include <linux/sched.h>
-#include <linux/ptrace.h>
-#include <stdint.h>
-#include <stddef.h>
-#include <stdbool.h>
-#include <linux/bpf.h>
+
+#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
diff --git a/tools/testing/selftests/bpf/progs/loop2.c b/tools/testing/selftests/bpf/progs/loop2.c
index 947bb7e988c2..edf07b1d310e 100644
--- a/tools/testing/selftests/bpf/progs/loop2.c
+++ b/tools/testing/selftests/bpf/progs/loop2.c
@@ -1,11 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2019 Facebook
-#include <linux/sched.h>
-#include <linux/ptrace.h>
-#include <stdint.h>
-#include <stddef.h>
-#include <stdbool.h>
-#include <linux/bpf.h>
+
+#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
diff --git a/tools/testing/selftests/bpf/progs/loop3.c b/tools/testing/selftests/bpf/progs/loop3.c
index 76e93b31c14b..c8d2f2c55547 100644
--- a/tools/testing/selftests/bpf/progs/loop3.c
+++ b/tools/testing/selftests/bpf/progs/loop3.c
@@ -1,11 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2019 Facebook
-#include <linux/sched.h>
-#include <linux/ptrace.h>
-#include <stdint.h>
-#include <stddef.h>
-#include <stdbool.h>
-#include <linux/bpf.h>
+
+#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
diff --git a/tools/testing/selftests/bpf/progs/loop6.c b/tools/testing/selftests/bpf/progs/loop6.c
index 38de0331e6b4..17ac4da5664d 100644
--- a/tools/testing/selftests/bpf/progs/loop6.c
+++ b/tools/testing/selftests/bpf/progs/loop6.c
@@ -1,8 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-#include <linux/ptrace.h>
-#include <stddef.h>
-#include <linux/bpf.h>
+#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
@@ -25,12 +23,6 @@ char _license[] SEC("license") = "GPL";
#define SG_CHAIN 0x01UL
#define SG_END 0x02UL
-struct scatterlist {
- unsigned long page_link;
- unsigned int offset;
- unsigned int length;
-};
-
#define sg_is_chain(sg) ((sg)->page_link & SG_CHAIN)
#define sg_is_last(sg) ((sg)->page_link & SG_END)
#define sg_chain_ptr(sg) \
@@ -61,8 +53,8 @@ static inline struct scatterlist *get_sgp(struct scatterlist **sgs, int i)
return sgp;
}
-int config = 0;
-int result = 0;
+int g_config = 0;
+int g_result = 0;
SEC("kprobe/virtqueue_add_sgs")
int BPF_KPROBE(trace_virtqueue_add_sgs, void *unused, struct scatterlist **sgs,
@@ -72,7 +64,7 @@ int BPF_KPROBE(trace_virtqueue_add_sgs, void *unused, struct scatterlist **sgs,
__u64 length1 = 0, length2 = 0;
unsigned int i, n, len;
- if (config != 0)
+ if (g_config != 0)
return 0;
for (i = 0; (i < VIRTIO_MAX_SGS) && (i < out_sgs); i++) {
@@ -93,7 +85,7 @@ int BPF_KPROBE(trace_virtqueue_add_sgs, void *unused, struct scatterlist **sgs,
}
}
- config = 1;
- result = length2 - length1;
+ g_config = 1;
+ g_result = length2 - length1;
return 0;
}
diff --git a/tools/testing/selftests/bpf/progs/test_overhead.c b/tools/testing/selftests/bpf/progs/test_overhead.c
index abb7344b531f..df035e6a3143 100644
--- a/tools/testing/selftests/bpf/progs/test_overhead.c
+++ b/tools/testing/selftests/bpf/progs/test_overhead.c
@@ -1,14 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2019 Facebook */
-#include <stdbool.h>
-#include <stddef.h>
-#include <linux/bpf.h>
-#include <linux/ptrace.h>
+
+#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
-struct task_struct;
-
SEC("kprobe/__set_task_comm")
int BPF_KPROBE(prog1, struct task_struct *tsk, const char *buf, bool exec)
{
diff --git a/tools/testing/selftests/bpf/progs/test_probe_user.c b/tools/testing/selftests/bpf/progs/test_probe_user.c
index 8812a90da4eb..bf6c0b864ace 100644
--- a/tools/testing/selftests/bpf/progs/test_probe_user.c
+++ b/tools/testing/selftests/bpf/progs/test_probe_user.c
@@ -1,10 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-#include <linux/ptrace.h>
-#include <linux/bpf.h>
-
-#include <netinet/in.h>
-
+#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-14 13:55 [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs Pu Lehui
@ 2021-12-14 20:01 ` Daniel Borkmann
2021-12-15 1:20 ` Pu Lehui
2021-12-16 4:06 ` Andrii Nakryiko
1 sibling, 1 reply; 13+ messages in thread
From: Daniel Borkmann @ 2021-12-14 20:01 UTC (permalink / raw)
To: Pu Lehui, ast, andrii, kafai, songliubraving, yhs,
john.fastabend, kpsingh, paul.walmsley, palmer, aou, shuah
Cc: linux-kselftest, netdev, bpf, linux-kernel
On 12/14/21 2:55 PM, Pu Lehui wrote:
> When building bpf selftests on arm64, the following error will occur:
>
> progs/loop2.c:20:7: error: incomplete definition of type 'struct
> user_pt_regs'
>
> Some archs, like arm64 and riscv, use userspace pt_regs in
> bpf_tracing.h, which causes build failure when bpf prog use
> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
Looks like this lets CI fail, did you run the selftests also with vmtest.sh to
double check?
https://github.com/kernel-patches/bpf/runs/4521708490?check_suite_focus=true :
[...]
#189 verif_scale_loop6:FAIL
libbpf: prog 'trace_virtqueue_add_sgs': BPF program load failed: Argument list too long
libbpf: prog 'trace_virtqueue_add_sgs': -- BEGIN PROG LOAD LOG --
R1 type=ctx expected=fp
BPF program is too large. Processed 1000001 insn
verification time 12250995 usec
stack depth 88
processed 1000001 insns (limit 1000000) max_states_per_insn 107 total_states 21739 peak_states 2271 mark_read 6
-- END PROG LOAD LOG --
libbpf: failed to load program 'trace_virtqueue_add_sgs'
libbpf: failed to load object 'loop6.o'
scale_test:FAIL:expect_success unexpected error: -7 (errno 7)
Summary: 221/986 PASSED, 8 SKIPPED, 1 FAILED
[...]
Please take a look and fix in your patch, thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-14 20:01 ` Daniel Borkmann
@ 2021-12-15 1:20 ` Pu Lehui
2021-12-17 2:19 ` Pu Lehui
0 siblings, 1 reply; 13+ messages in thread
From: Pu Lehui @ 2021-12-15 1:20 UTC (permalink / raw)
To: Daniel Borkmann, ast, andrii, kafai, songliubraving, yhs,
john.fastabend, kpsingh, paul.walmsley, palmer, aou, shuah
Cc: linux-kselftest, netdev, bpf, linux-kernel
On 2021/12/15 4:01, Daniel Borkmann wrote:
> On 12/14/21 2:55 PM, Pu Lehui wrote:
>> When building bpf selftests on arm64, the following error will occur:
>>
>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
>> user_pt_regs'
>>
>> Some archs, like arm64 and riscv, use userspace pt_regs in
>> bpf_tracing.h, which causes build failure when bpf prog use
>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>
> Looks like this lets CI fail, did you run the selftests also with
> vmtest.sh to
> double check?
>
> https://github.com/kernel-patches/bpf/runs/4521708490?check_suite_focus=true
> :
>
> [...]
> #189 verif_scale_loop6:FAIL
> libbpf: prog 'trace_virtqueue_add_sgs': BPF program load failed:
> Argument list too long
> libbpf: prog 'trace_virtqueue_add_sgs': -- BEGIN PROG LOAD LOG --
> R1 type=ctx expected=fp
> BPF program is too large. Processed 1000001 insn
> verification time 12250995 usec
> stack depth 88
> processed 1000001 insns (limit 1000000) max_states_per_insn 107
> total_states 21739 peak_states 2271 mark_read 6
> -- END PROG LOAD LOG --
> libbpf: failed to load program 'trace_virtqueue_add_sgs'
> libbpf: failed to load object 'loop6.o'
> scale_test:FAIL:expect_success unexpected error: -7 (errno 7)
> Summary: 221/986 PASSED, 8 SKIPPED, 1 FAILED
> [...]
>
> Please take a look and fix in your patch, thanks!
> .
Sorry for my negligence, I'll take a look and fix it.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-15 1:20 ` Pu Lehui
@ 2021-12-17 2:19 ` Pu Lehui
0 siblings, 0 replies; 13+ messages in thread
From: Pu Lehui @ 2021-12-17 2:19 UTC (permalink / raw)
To: Daniel Borkmann, ast, andrii, kafai, songliubraving, yhs,
john.fastabend, kpsingh, paul.walmsley, palmer, aou, shuah
Cc: linux-kselftest, netdev, bpf, linux-kernel
On 2021/12/15 9:20, Pu Lehui wrote:
> On 2021/12/15 4:01, Daniel Borkmann wrote:
>> On 12/14/21 2:55 PM, Pu Lehui wrote:
>>> When building bpf selftests on arm64, the following error will occur:
>>>
>>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
>>> user_pt_regs'
>>>
>>> Some archs, like arm64 and riscv, use userspace pt_regs in
>>> bpf_tracing.h, which causes build failure when bpf prog use
>>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>>>
>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>
>> Looks like this lets CI fail, did you run the selftests also with
>> vmtest.sh to
>> double check?
>>
>> https://github.com/kernel-patches/bpf/runs/4521708490?check_suite_focus=true
>> :
>>
>> [...]
>> #189 verif_scale_loop6:FAIL
>> libbpf: prog 'trace_virtqueue_add_sgs': BPF program load failed:
>> Argument list too long
>> libbpf: prog 'trace_virtqueue_add_sgs': -- BEGIN PROG LOAD LOG --
>> R1 type=ctx expected=fp
>> BPF program is too large. Processed 1000001 insn
>> verification time 12250995 usec
>> stack depth 88
>> processed 1000001 insns (limit 1000000) max_states_per_insn 107
>> total_states 21739 peak_states 2271 mark_read 6
>> -- END PROG LOAD LOG --
>> libbpf: failed to load program 'trace_virtqueue_add_sgs'
>> libbpf: failed to load object 'loop6.o'
>> scale_test:FAIL:expect_success unexpected error: -7 (errno 7)
>> Summary: 221/986 PASSED, 8 SKIPPED, 1 FAILED
>> [...]
>>
>> Please take a look and fix in your patch, thanks!
>> .
> Sorry for my negligence, I'll take a look and fix it.
> .
It seems strange that verifier think the loop can execute up to u64_max
while I just replace the header file.
This looks very similar to the previous llvm issue,
https://github.com/iovisor/bcc/pull/3270, but I have no idea how to locate.
Back to arm64 bpf selftest compiling problem, we can use header file
directory generated by "make headers_install" to fix it.
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -294,7 +294,8 @@ MENDIAN=$(if
$(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
-I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
- -I$(abspath $(OUTPUT)/../usr/include)
+ -I$(abspath $(OUTPUT)/../usr/include) \
+ -I../../../../usr/include
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-14 13:55 [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs Pu Lehui
2021-12-14 20:01 ` Daniel Borkmann
@ 2021-12-16 4:06 ` Andrii Nakryiko
2021-12-17 2:25 ` Pu Lehui
1 sibling, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-12-16 4:06 UTC (permalink / raw)
To: Pu Lehui
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
>
> When building bpf selftests on arm64, the following error will occur:
>
> progs/loop2.c:20:7: error: incomplete definition of type 'struct
> user_pt_regs'
>
> Some archs, like arm64 and riscv, use userspace pt_regs in
> bpf_tracing.h, which causes build failure when bpf prog use
> macro in bpf_tracing.h. So let's use vmlinux.h directly.
We could probably also extend bpf_tracing.h to work with
kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
__VMLINUX_H__ checks). It's more work, but will benefit other end
users, not just selftests.
>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
> tools/testing/selftests/bpf/progs/loop6.c | 20 ++++++-------------
> .../selftests/bpf/progs/test_overhead.c | 8 ++------
> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
> 6 files changed, 15 insertions(+), 43 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-16 4:06 ` Andrii Nakryiko
@ 2021-12-17 2:25 ` Pu Lehui
2021-12-17 16:45 ` Andrii Nakryiko
0 siblings, 1 reply; 13+ messages in thread
From: Pu Lehui @ 2021-12-17 2:25 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On 2021/12/16 12:06, Andrii Nakryiko wrote:
> On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
>>
>> When building bpf selftests on arm64, the following error will occur:
>>
>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
>> user_pt_regs'
>>
>> Some archs, like arm64 and riscv, use userspace pt_regs in
>> bpf_tracing.h, which causes build failure when bpf prog use
>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>
> We could probably also extend bpf_tracing.h to work with
> kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
> __VMLINUX_H__ checks). It's more work, but will benefit other end
> users, not just selftests.
>
It might change a lot. We can use header file directory generated by
"make headers_install" to fix it.
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -294,7 +294,8 @@ MENDIAN=$(if
$(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
-I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
- -I$(abspath $(OUTPUT)/../usr/include)
+ -I$(abspath $(OUTPUT)/../usr/include) \
+ -I../../../../usr/include
>>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>> ---
>> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
>> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
>> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
>> tools/testing/selftests/bpf/progs/loop6.c | 20 ++++++-------------
>> .../selftests/bpf/progs/test_overhead.c | 8 ++------
>> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
>> 6 files changed, 15 insertions(+), 43 deletions(-)
>>
>
> [...]
> .
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-17 2:25 ` Pu Lehui
@ 2021-12-17 16:45 ` Andrii Nakryiko
2021-12-20 14:02 ` Pu Lehui
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-12-17 16:45 UTC (permalink / raw)
To: Pu Lehui
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On Thu, Dec 16, 2021 at 6:25 PM Pu Lehui <pulehui@huawei.com> wrote:
>
>
>
> On 2021/12/16 12:06, Andrii Nakryiko wrote:
> > On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
> >>
> >> When building bpf selftests on arm64, the following error will occur:
> >>
> >> progs/loop2.c:20:7: error: incomplete definition of type 'struct
> >> user_pt_regs'
> >>
> >> Some archs, like arm64 and riscv, use userspace pt_regs in
> >> bpf_tracing.h, which causes build failure when bpf prog use
> >> macro in bpf_tracing.h. So let's use vmlinux.h directly.
> >
> > We could probably also extend bpf_tracing.h to work with
> > kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
> > __VMLINUX_H__ checks). It's more work, but will benefit other end
> > users, not just selftests.
> >
> It might change a lot. We can use header file directory generated by
> "make headers_install" to fix it.
We don't have dependency on "make headers_install" and I'd rather not add it.
What do you mean by "change a lot"?
>
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -294,7 +294,8 @@ MENDIAN=$(if
> $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
> CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
> BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
> -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
> - -I$(abspath $(OUTPUT)/../usr/include)
> + -I$(abspath $(OUTPUT)/../usr/include) \
> + -I../../../../usr/include
> >>
> >> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> >> ---
> >> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
> >> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
> >> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
> >> tools/testing/selftests/bpf/progs/loop6.c | 20 ++++++-------------
> >> .../selftests/bpf/progs/test_overhead.c | 8 ++------
> >> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
> >> 6 files changed, 15 insertions(+), 43 deletions(-)
> >>
> >
> > [...]
> > .
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-17 16:45 ` Andrii Nakryiko
@ 2021-12-20 14:02 ` Pu Lehui
2021-12-21 0:58 ` Pu Lehui
0 siblings, 1 reply; 13+ messages in thread
From: Pu Lehui @ 2021-12-20 14:02 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On 2021/12/18 0:45, Andrii Nakryiko wrote:
> On Thu, Dec 16, 2021 at 6:25 PM Pu Lehui <pulehui@huawei.com> wrote:
>>
>>
>>
>> On 2021/12/16 12:06, Andrii Nakryiko wrote:
>>> On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
>>>>
>>>> When building bpf selftests on arm64, the following error will occur:
>>>>
>>>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
>>>> user_pt_regs'
>>>>
>>>> Some archs, like arm64 and riscv, use userspace pt_regs in
>>>> bpf_tracing.h, which causes build failure when bpf prog use
>>>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>>>
>>> We could probably also extend bpf_tracing.h to work with
>>> kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
>>> __VMLINUX_H__ checks). It's more work, but will benefit other end
>>> users, not just selftests.
>>>
>> It might change a lot. We can use header file directory generated by
>> "make headers_install" to fix it.
>
> We don't have dependency on "make headers_install" and I'd rather not add it.
>
> What do you mean by "change a lot"?
>
Maybe I misunderstood your advice. Your suggestion might be to extend
bpf_tracing.h to kernel-space pt_regs, while some archs, like arm64,
only support user-space. So the patch might be like this:
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index db05a5937105..2c3cb8e9ae92 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -195,9 +195,13 @@ struct pt_regs;
#elif defined(bpf_target_arm64)
-struct pt_regs;
+#if defined(__KERNEL__)
+#define PT_REGS_ARM64 const volatile struct pt_regs
+#else
/* arm64 provides struct user_pt_regs instead of struct pt_regs to
userspace */
#define PT_REGS_ARM64 const volatile struct user_pt_regs
+#endif
+
#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])
>>
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -294,7 +294,8 @@ MENDIAN=$(if
>> $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
>> CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
>> BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
>> -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
>> - -I$(abspath $(OUTPUT)/../usr/include)
>> + -I$(abspath $(OUTPUT)/../usr/include) \
>> + -I../../../../usr/include
>>>>
>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>> ---
>>>> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
>>>> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
>>>> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
>>>> tools/testing/selftests/bpf/progs/loop6.c | 20 ++++++-------------
>>>> .../selftests/bpf/progs/test_overhead.c | 8 ++------
>>>> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
>>>> 6 files changed, 15 insertions(+), 43 deletions(-)
>>>>
>>>
>>> [...]
>>> .
>>>
> .
>
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-20 14:02 ` Pu Lehui
@ 2021-12-21 0:58 ` Pu Lehui
2021-12-21 23:52 ` Andrii Nakryiko
0 siblings, 1 reply; 13+ messages in thread
From: Pu Lehui @ 2021-12-21 0:58 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On 2021/12/20 22:02, Pu Lehui wrote:
>
>
> On 2021/12/18 0:45, Andrii Nakryiko wrote:
>> On Thu, Dec 16, 2021 at 6:25 PM Pu Lehui <pulehui@huawei.com> wrote:
>>>
>>>
>>>
>>> On 2021/12/16 12:06, Andrii Nakryiko wrote:
>>>> On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
>>>>>
>>>>> When building bpf selftests on arm64, the following error will occur:
>>>>>
>>>>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
>>>>> user_pt_regs'
>>>>>
>>>>> Some archs, like arm64 and riscv, use userspace pt_regs in
>>>>> bpf_tracing.h, which causes build failure when bpf prog use
>>>>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>>>>
>>>> We could probably also extend bpf_tracing.h to work with
>>>> kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
>>>> __VMLINUX_H__ checks). It's more work, but will benefit other end
>>>> users, not just selftests.
>>>>
>>> It might change a lot. We can use header file directory generated by
>>> "make headers_install" to fix it.
>>
>> We don't have dependency on "make headers_install" and I'd rather not
>> add it.
>>
>> What do you mean by "change a lot"?
>>
> Maybe I misunderstood your advice. Your suggestion might be to extend
> bpf_tracing.h to kernel-space pt_regs, while some archs, like arm64,
> only support user-space. So the patch might be like this:
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index db05a5937105..2c3cb8e9ae92 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -195,9 +195,13 @@ struct pt_regs;
>
> #elif defined(bpf_target_arm64)
>
> -struct pt_regs;
> +#if defined(__KERNEL__)
> +#define PT_REGS_ARM64 const volatile struct pt_regs
> +#else
> /* arm64 provides struct user_pt_regs instead of struct pt_regs to
> userspace */
> #define PT_REGS_ARM64 const volatile struct user_pt_regs
> +#endif
> +
> #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])
>
Please ignore the last reply. User-space pt_regs of arm64/s390 is the
first part of the kernel-space's, it should has covered both kernel and
userspace.
>>>
>>> --- a/tools/testing/selftests/bpf/Makefile
>>> +++ b/tools/testing/selftests/bpf/Makefile
>>> @@ -294,7 +294,8 @@ MENDIAN=$(if
>>> $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
>>> CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
>>> BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
>>> -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
>>> - -I$(abspath $(OUTPUT)/../usr/include)
>>> + -I$(abspath $(OUTPUT)/../usr/include) \
>>> + -I../../../../usr/include
>>>>>
>>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>>> ---
>>>>> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
>>>>> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
>>>>> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
>>>>> tools/testing/selftests/bpf/progs/loop6.c | 20
>>>>> ++++++-------------
>>>>> .../selftests/bpf/progs/test_overhead.c | 8 ++------
>>>>> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
>>>>> 6 files changed, 15 insertions(+), 43 deletions(-)
>>>>>
>>>>
>>>> [...]
>>>> .
>>>>
>> .
>>
> .
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-21 0:58 ` Pu Lehui
@ 2021-12-21 23:52 ` Andrii Nakryiko
2021-12-22 1:33 ` Pu Lehui
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-12-21 23:52 UTC (permalink / raw)
To: Pu Lehui
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On Mon, Dec 20, 2021 at 4:58 PM Pu Lehui <pulehui@huawei.com> wrote:
>
>
>
> On 2021/12/20 22:02, Pu Lehui wrote:
> >
> >
> > On 2021/12/18 0:45, Andrii Nakryiko wrote:
> >> On Thu, Dec 16, 2021 at 6:25 PM Pu Lehui <pulehui@huawei.com> wrote:
> >>>
> >>>
> >>>
> >>> On 2021/12/16 12:06, Andrii Nakryiko wrote:
> >>>> On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
> >>>>>
> >>>>> When building bpf selftests on arm64, the following error will occur:
> >>>>>
> >>>>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
> >>>>> user_pt_regs'
> >>>>>
> >>>>> Some archs, like arm64 and riscv, use userspace pt_regs in
> >>>>> bpf_tracing.h, which causes build failure when bpf prog use
> >>>>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
> >>>>
> >>>> We could probably also extend bpf_tracing.h to work with
> >>>> kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
> >>>> __VMLINUX_H__ checks). It's more work, but will benefit other end
> >>>> users, not just selftests.
> >>>>
> >>> It might change a lot. We can use header file directory generated by
> >>> "make headers_install" to fix it.
> >>
> >> We don't have dependency on "make headers_install" and I'd rather not
> >> add it.
> >>
> >> What do you mean by "change a lot"?
> >>
> > Maybe I misunderstood your advice. Your suggestion might be to extend
> > bpf_tracing.h to kernel-space pt_regs, while some archs, like arm64,
yes
> > only support user-space. So the patch might be like this:
> >
> > diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> > index db05a5937105..2c3cb8e9ae92 100644
> > --- a/tools/lib/bpf/bpf_tracing.h
> > +++ b/tools/lib/bpf/bpf_tracing.h
> > @@ -195,9 +195,13 @@ struct pt_regs;
> >
> > #elif defined(bpf_target_arm64)
> >
> > -struct pt_regs;
> > +#if defined(__KERNEL__)
> > +#define PT_REGS_ARM64 const volatile struct pt_regs
> > +#else
> > /* arm64 provides struct user_pt_regs instead of struct pt_regs to
> > userspace */
> > #define PT_REGS_ARM64 const volatile struct user_pt_regs
> > +#endif
> > +
> > #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])
> >
> Please ignore the last reply. User-space pt_regs of arm64/s390 is the
> first part of the kernel-space's, it should has covered both kernel and
> userspace.
Alright, so is there still a problem or not? Looking at the definition
of struct pt_regs for arm64, just casting struct pt_regs to struct
user_pt_regs will indeed just work. So in that case, what was your
original issue?
> >>>
> >>> --- a/tools/testing/selftests/bpf/Makefile
> >>> +++ b/tools/testing/selftests/bpf/Makefile
> >>> @@ -294,7 +294,8 @@ MENDIAN=$(if
> >>> $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
> >>> CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
> >>> BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
> >>> -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
> >>> - -I$(abspath $(OUTPUT)/../usr/include)
> >>> + -I$(abspath $(OUTPUT)/../usr/include) \
> >>> + -I../../../../usr/include
> >>>>>
> >>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> >>>>> ---
> >>>>> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
> >>>>> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
> >>>>> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
> >>>>> tools/testing/selftests/bpf/progs/loop6.c | 20
> >>>>> ++++++-------------
> >>>>> .../selftests/bpf/progs/test_overhead.c | 8 ++------
> >>>>> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
> >>>>> 6 files changed, 15 insertions(+), 43 deletions(-)
> >>>>>
> >>>>
> >>>> [...]
> >>>> .
> >>>>
> >> .
> >>
> > .
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-21 23:52 ` Andrii Nakryiko
@ 2021-12-22 1:33 ` Pu Lehui
2021-12-22 23:17 ` Andrii Nakryiko
0 siblings, 1 reply; 13+ messages in thread
From: Pu Lehui @ 2021-12-22 1:33 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On 2021/12/22 7:52, Andrii Nakryiko wrote:
> On Mon, Dec 20, 2021 at 4:58 PM Pu Lehui <pulehui@huawei.com> wrote:
>>
>>
>>
>> On 2021/12/20 22:02, Pu Lehui wrote:
>>>
>>>
>>> On 2021/12/18 0:45, Andrii Nakryiko wrote:
>>>> On Thu, Dec 16, 2021 at 6:25 PM Pu Lehui <pulehui@huawei.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 2021/12/16 12:06, Andrii Nakryiko wrote:
>>>>>> On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
>>>>>>>
>>>>>>> When building bpf selftests on arm64, the following error will occur:
>>>>>>>
>>>>>>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
>>>>>>> user_pt_regs'
>>>>>>>
>>>>>>> Some archs, like arm64 and riscv, use userspace pt_regs in
>>>>>>> bpf_tracing.h, which causes build failure when bpf prog use
>>>>>>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>>>>>>
>>>>>> We could probably also extend bpf_tracing.h to work with
>>>>>> kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
>>>>>> __VMLINUX_H__ checks). It's more work, but will benefit other end
>>>>>> users, not just selftests.
>>>>>>
>>>>> It might change a lot. We can use header file directory generated by
>>>>> "make headers_install" to fix it.
>>>>
>>>> We don't have dependency on "make headers_install" and I'd rather not
>>>> add it.
>>>>
>>>> What do you mean by "change a lot"?
>>>>
>>> Maybe I misunderstood your advice. Your suggestion might be to extend
>>> bpf_tracing.h to kernel-space pt_regs, while some archs, like arm64,
>
> yes
>
>>> only support user-space. So the patch might be like this:
>>>
>>> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
>>> index db05a5937105..2c3cb8e9ae92 100644
>>> --- a/tools/lib/bpf/bpf_tracing.h
>>> +++ b/tools/lib/bpf/bpf_tracing.h
>>> @@ -195,9 +195,13 @@ struct pt_regs;
>>>
>>> #elif defined(bpf_target_arm64)
>>>
>>> -struct pt_regs;
>>> +#if defined(__KERNEL__)
>>> +#define PT_REGS_ARM64 const volatile struct pt_regs
>>> +#else
>>> /* arm64 provides struct user_pt_regs instead of struct pt_regs to
>>> userspace */
>>> #define PT_REGS_ARM64 const volatile struct user_pt_regs
>>> +#endif
>>> +
>>> #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])
>>>
>> Please ignore the last reply. User-space pt_regs of arm64/s390 is the
>> first part of the kernel-space's, it should has covered both kernel and
>> userspace.
>
> Alright, so is there still a problem or not? Looking at the definition
> of struct pt_regs for arm64, just casting struct pt_regs to struct
> user_pt_regs will indeed just work. So in that case, what was your
> original issue?
>
Thanks for your reply. The original issue is, when arm64 bpf selftests
cross compiling in x86_64 host, clang cannot find the arch specific uapi
ptrace.h, and then the above error occur. Of course it works when
compiling in arm64 host for it owns the corresponding uapi ptrace.h. So
my suggestion is to add arch specific use header file directory
generated by "make headers_install" for the cross compiling issue.
>>>>>
>>>>> --- a/tools/testing/selftests/bpf/Makefile
>>>>> +++ b/tools/testing/selftests/bpf/Makefile
>>>>> @@ -294,7 +294,8 @@ MENDIAN=$(if
>>>>> $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
>>>>> CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
>>>>> BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
>>>>> -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
>>>>> - -I$(abspath $(OUTPUT)/../usr/include)
>>>>> + -I$(abspath $(OUTPUT)/../usr/include) \
>>>>> + -I../../../../usr/include
>>>>>>>
>>>>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>>>>> ---
>>>>>>> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
>>>>>>> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
>>>>>>> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
>>>>>>> tools/testing/selftests/bpf/progs/loop6.c | 20
>>>>>>> ++++++-------------
>>>>>>> .../selftests/bpf/progs/test_overhead.c | 8 ++------
>>>>>>> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
>>>>>>> 6 files changed, 15 insertions(+), 43 deletions(-)
>>>>>>>
>>>>>>
>>>>>> [...]
>>>>>> .
>>>>>>
>>>> .
>>>>
>>> .
> .
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-22 1:33 ` Pu Lehui
@ 2021-12-22 23:17 ` Andrii Nakryiko
2021-12-23 4:52 ` Pu Lehui
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-12-22 23:17 UTC (permalink / raw)
To: Pu Lehui
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On Tue, Dec 21, 2021 at 5:33 PM Pu Lehui <pulehui@huawei.com> wrote:
>
>
>
> On 2021/12/22 7:52, Andrii Nakryiko wrote:
> > On Mon, Dec 20, 2021 at 4:58 PM Pu Lehui <pulehui@huawei.com> wrote:
> >>
> >>
> >>
> >> On 2021/12/20 22:02, Pu Lehui wrote:
> >>>
> >>>
> >>> On 2021/12/18 0:45, Andrii Nakryiko wrote:
> >>>> On Thu, Dec 16, 2021 at 6:25 PM Pu Lehui <pulehui@huawei.com> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 2021/12/16 12:06, Andrii Nakryiko wrote:
> >>>>>> On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
> >>>>>>>
> >>>>>>> When building bpf selftests on arm64, the following error will occur:
> >>>>>>>
> >>>>>>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
> >>>>>>> user_pt_regs'
> >>>>>>>
> >>>>>>> Some archs, like arm64 and riscv, use userspace pt_regs in
> >>>>>>> bpf_tracing.h, which causes build failure when bpf prog use
> >>>>>>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
> >>>>>>
> >>>>>> We could probably also extend bpf_tracing.h to work with
> >>>>>> kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
> >>>>>> __VMLINUX_H__ checks). It's more work, but will benefit other end
> >>>>>> users, not just selftests.
> >>>>>>
> >>>>> It might change a lot. We can use header file directory generated by
> >>>>> "make headers_install" to fix it.
> >>>>
> >>>> We don't have dependency on "make headers_install" and I'd rather not
> >>>> add it.
> >>>>
> >>>> What do you mean by "change a lot"?
> >>>>
> >>> Maybe I misunderstood your advice. Your suggestion might be to extend
> >>> bpf_tracing.h to kernel-space pt_regs, while some archs, like arm64,
> >
> > yes
> >
> >>> only support user-space. So the patch might be like this:
> >>>
> >>> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> >>> index db05a5937105..2c3cb8e9ae92 100644
> >>> --- a/tools/lib/bpf/bpf_tracing.h
> >>> +++ b/tools/lib/bpf/bpf_tracing.h
> >>> @@ -195,9 +195,13 @@ struct pt_regs;
> >>>
> >>> #elif defined(bpf_target_arm64)
> >>>
> >>> -struct pt_regs;
> >>> +#if defined(__KERNEL__)
> >>> +#define PT_REGS_ARM64 const volatile struct pt_regs
> >>> +#else
> >>> /* arm64 provides struct user_pt_regs instead of struct pt_regs to
> >>> userspace */
> >>> #define PT_REGS_ARM64 const volatile struct user_pt_regs
> >>> +#endif
> >>> +
> >>> #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])
> >>>
> >> Please ignore the last reply. User-space pt_regs of arm64/s390 is the
> >> first part of the kernel-space's, it should has covered both kernel and
> >> userspace.
> >
> > Alright, so is there still a problem or not? Looking at the definition
> > of struct pt_regs for arm64, just casting struct pt_regs to struct
> > user_pt_regs will indeed just work. So in that case, what was your
> > original issue?
> >
> Thanks for your reply. The original issue is, when arm64 bpf selftests
> cross compiling in x86_64 host, clang cannot find the arch specific uapi
> ptrace.h, and then the above error occur. Of course it works when
> compiling in arm64 host for it owns the corresponding uapi ptrace.h. So
> my suggestion is to add arch specific use header file directory
> generated by "make headers_install" for the cross compiling issue.
I see. Can you try adding something like:
ARCH_APIDIR := $(abspath ../../../../arch/$(SRCARCH)/include/uapi)
and then add -I$(ARCH_APIDIR) to BPF_CFLAGS?
Please let me know if that works for your cross-compilation case.
> >>>>>
> >>>>> --- a/tools/testing/selftests/bpf/Makefile
> >>>>> +++ b/tools/testing/selftests/bpf/Makefile
> >>>>> @@ -294,7 +294,8 @@ MENDIAN=$(if
> >>>>> $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
> >>>>> CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
> >>>>> BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
> >>>>> -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
> >>>>> - -I$(abspath $(OUTPUT)/../usr/include)
> >>>>> + -I$(abspath $(OUTPUT)/../usr/include) \
> >>>>> + -I../../../../usr/include
> >>>>>>>
> >>>>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> >>>>>>> ---
> >>>>>>> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
> >>>>>>> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
> >>>>>>> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
> >>>>>>> tools/testing/selftests/bpf/progs/loop6.c | 20
> >>>>>>> ++++++-------------
> >>>>>>> .../selftests/bpf/progs/test_overhead.c | 8 ++------
> >>>>>>> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
> >>>>>>> 6 files changed, 15 insertions(+), 43 deletions(-)
> >>>>>>>
> >>>>>>
> >>>>>> [...]
> >>>>>> .
> >>>>>>
> >>>> .
> >>>>
> >>> .
> > .
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs
2021-12-22 23:17 ` Andrii Nakryiko
@ 2021-12-23 4:52 ` Pu Lehui
0 siblings, 0 replies; 13+ messages in thread
From: Pu Lehui @ 2021-12-23 4:52 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, Networking, bpf, open list
On 2021/12/23 7:17, Andrii Nakryiko wrote:
> On Tue, Dec 21, 2021 at 5:33 PM Pu Lehui <pulehui@huawei.com> wrote:
>>
>>
>>
>> On 2021/12/22 7:52, Andrii Nakryiko wrote:
>>> On Mon, Dec 20, 2021 at 4:58 PM Pu Lehui <pulehui@huawei.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2021/12/20 22:02, Pu Lehui wrote:
>>>>>
>>>>>
>>>>> On 2021/12/18 0:45, Andrii Nakryiko wrote:
>>>>>> On Thu, Dec 16, 2021 at 6:25 PM Pu Lehui <pulehui@huawei.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 2021/12/16 12:06, Andrii Nakryiko wrote:
>>>>>>>> On Tue, Dec 14, 2021 at 5:54 AM Pu Lehui <pulehui@huawei.com> wrote:
>>>>>>>>>
>>>>>>>>> When building bpf selftests on arm64, the following error will occur:
>>>>>>>>>
>>>>>>>>> progs/loop2.c:20:7: error: incomplete definition of type 'struct
>>>>>>>>> user_pt_regs'
>>>>>>>>>
>>>>>>>>> Some archs, like arm64 and riscv, use userspace pt_regs in
>>>>>>>>> bpf_tracing.h, which causes build failure when bpf prog use
>>>>>>>>> macro in bpf_tracing.h. So let's use vmlinux.h directly.
>>>>>>>>
>>>>>>>> We could probably also extend bpf_tracing.h to work with
>>>>>>>> kernel-defined pt_regs, just like we do for x86 (see __KERNEL__ and
>>>>>>>> __VMLINUX_H__ checks). It's more work, but will benefit other end
>>>>>>>> users, not just selftests.
>>>>>>>>
>>>>>>> It might change a lot. We can use header file directory generated by
>>>>>>> "make headers_install" to fix it.
>>>>>>
>>>>>> We don't have dependency on "make headers_install" and I'd rather not
>>>>>> add it.
>>>>>>
>>>>>> What do you mean by "change a lot"?
>>>>>>
>>>>> Maybe I misunderstood your advice. Your suggestion might be to extend
>>>>> bpf_tracing.h to kernel-space pt_regs, while some archs, like arm64,
>>>
>>> yes
>>>
>>>>> only support user-space. So the patch might be like this:
>>>>>
>>>>> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
>>>>> index db05a5937105..2c3cb8e9ae92 100644
>>>>> --- a/tools/lib/bpf/bpf_tracing.h
>>>>> +++ b/tools/lib/bpf/bpf_tracing.h
>>>>> @@ -195,9 +195,13 @@ struct pt_regs;
>>>>>
>>>>> #elif defined(bpf_target_arm64)
>>>>>
>>>>> -struct pt_regs;
>>>>> +#if defined(__KERNEL__)
>>>>> +#define PT_REGS_ARM64 const volatile struct pt_regs
>>>>> +#else
>>>>> /* arm64 provides struct user_pt_regs instead of struct pt_regs to
>>>>> userspace */
>>>>> #define PT_REGS_ARM64 const volatile struct user_pt_regs
>>>>> +#endif
>>>>> +
>>>>> #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])
>>>>>
>>>> Please ignore the last reply. User-space pt_regs of arm64/s390 is the
>>>> first part of the kernel-space's, it should has covered both kernel and
>>>> userspace.
>>>
>>> Alright, so is there still a problem or not? Looking at the definition
>>> of struct pt_regs for arm64, just casting struct pt_regs to struct
>>> user_pt_regs will indeed just work. So in that case, what was your
>>> original issue?
>>>
>> Thanks for your reply. The original issue is, when arm64 bpf selftests
>> cross compiling in x86_64 host, clang cannot find the arch specific uapi
>> ptrace.h, and then the above error occur. Of course it works when
>> compiling in arm64 host for it owns the corresponding uapi ptrace.h. So
>> my suggestion is to add arch specific use header file directory
>> generated by "make headers_install" for the cross compiling issue.
>
> I see. Can you try adding something like:
>
> ARCH_APIDIR := $(abspath ../../../../arch/$(SRCARCH)/include/uapi)
>
> and then add -I$(ARCH_APIDIR) to BPF_CFLAGS?
>
> Please let me know if that works for your cross-compilation case.
>
It works, thanks. I will add it to v2.
>>>>>>>
>>>>>>> --- a/tools/testing/selftests/bpf/Makefile
>>>>>>> +++ b/tools/testing/selftests/bpf/Makefile
>>>>>>> @@ -294,7 +294,8 @@ MENDIAN=$(if
>>>>>>> $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
>>>>>>> CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
>>>>>>> BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
>>>>>>> -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
>>>>>>> - -I$(abspath $(OUTPUT)/../usr/include)
>>>>>>> + -I$(abspath $(OUTPUT)/../usr/include) \
>>>>>>> + -I../../../../usr/include
>>>>>>>>>
>>>>>>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>>>>>>> ---
>>>>>>>>> tools/testing/selftests/bpf/progs/loop1.c | 8 ++------
>>>>>>>>> tools/testing/selftests/bpf/progs/loop2.c | 8 ++------
>>>>>>>>> tools/testing/selftests/bpf/progs/loop3.c | 8 ++------
>>>>>>>>> tools/testing/selftests/bpf/progs/loop6.c | 20
>>>>>>>>> ++++++-------------
>>>>>>>>> .../selftests/bpf/progs/test_overhead.c | 8 ++------
>>>>>>>>> .../selftests/bpf/progs/test_probe_user.c | 6 +-----
>>>>>>>>> 6 files changed, 15 insertions(+), 43 deletions(-)
>>>>>>>>>
>>>>>>>>
>>>>>>>> [...]
>>>>>>>> .
>>>>>>>>
>>>>>> .
>>>>>>
>>>>> .
>>> .
>>>
> .
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-12-23 4:52 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-14 13:55 [PATCH bpf-next] selftests/bpf: Fix building error when using userspace pt_regs Pu Lehui
2021-12-14 20:01 ` Daniel Borkmann
2021-12-15 1:20 ` Pu Lehui
2021-12-17 2:19 ` Pu Lehui
2021-12-16 4:06 ` Andrii Nakryiko
2021-12-17 2:25 ` Pu Lehui
2021-12-17 16:45 ` Andrii Nakryiko
2021-12-20 14:02 ` Pu Lehui
2021-12-21 0:58 ` Pu Lehui
2021-12-21 23:52 ` Andrii Nakryiko
2021-12-22 1:33 ` Pu Lehui
2021-12-22 23:17 ` Andrii Nakryiko
2021-12-23 4:52 ` Pu Lehui
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.