BPF Archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 0/2] Add 12-argument support for RV64 bpf trampoline
@ 2024-04-03  7:28 Pu Lehui
  2024-04-03  7:28 ` [PATCH bpf-next v3 1/2] riscv, bpf: " Pu Lehui
  2024-04-03  7:28 ` [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct Pu Lehui
  0 siblings, 2 replies; 6+ messages in thread
From: Pu Lehui @ 2024-04-03  7:28 UTC (permalink / raw
  To: bpf, linux-riscv, netdev
  Cc: Björn Töpel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Palmer Dabbelt, Pu Lehui, Pu Lehui

This patch adds 12 function arguments support for riscv64 bpf
trampoline. The current bpf trampoline supports <= sizeof(u64) bytes
scalar arguments [0] and <= 16 bytes struct arguments [1]. Therefore, we
focus on the situation where scalars are at most XLEN bits and
aggregates whose total size does not exceed 2×XLEN bits in the riscv
calling convention [2].

Link: https://elixir.bootlin.com/linux/v6.8/source/kernel/bpf/btf.c#L6184 [0]
Link: https://elixir.bootlin.com/linux/v6.8/source/kernel/bpf/btf.c#L6769 [1]
Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/download/draft-20230929-e5c800e661a53efe3c2678d71a306323b60eb13b/riscv-abi.pdf [2]

v3:
- Variable and macro name alignment:
  nr_reg_args: number of args in reg
  nr_stack_args: number of args on stack
  RV_MAX_REG_ARGS: macro for riscv max args in reg

v2: https://lore.kernel.org/all/20240403041710.1416369-1-pulehui@huaweicloud.com/
- Add tracing_struct to DENYLIST.aarch64 while aarch64 does not yet support
  bpf trampoline with more than 8 args.
- Change the macro RV_MAX_ARG_REGS to RV_MAX_ARGS_REG to synchronize with
  the variable definition below.
- Add some comments for stk_arg_off and magic number of skip slots for loading
  args on stack.

v1: https://lore.kernel.org/all/20240331092405.822571-1-pulehui@huaweicloud.com/

Pu Lehui (2):
  riscv, bpf: Add 12-argument support for RV64 bpf trampoline
  selftests/bpf: Add testcase where 7th argment is struct

 arch/riscv/net/bpf_jit_comp64.c               | 65 +++++++++++++------
 tools/testing/selftests/bpf/DENYLIST.aarch64  |  1 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 19 ++++++
 .../selftests/bpf/prog_tests/tracing_struct.c | 13 ++++
 .../selftests/bpf/progs/tracing_struct.c      | 35 ++++++++++
 5 files changed, 114 insertions(+), 19 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v3 1/2] riscv, bpf: Add 12-argument support for RV64 bpf trampoline
  2024-04-03  7:28 [PATCH bpf-next v3 0/2] Add 12-argument support for RV64 bpf trampoline Pu Lehui
@ 2024-04-03  7:28 ` Pu Lehui
  2024-04-03  7:28 ` [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct Pu Lehui
  1 sibling, 0 replies; 6+ messages in thread
From: Pu Lehui @ 2024-04-03  7:28 UTC (permalink / raw
  To: bpf, linux-riscv, netdev
  Cc: Björn Töpel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Palmer Dabbelt, Pu Lehui, Pu Lehui

From: Pu Lehui <pulehui@huawei.com>

This patch adds 12 function arguments support for riscv64 bpf
trampoline. The current bpf trampoline supports <= sizeof(u64) bytes
scalar arguments [0] and <= 16 bytes struct arguments [1]. Therefore, we
focus on the situation where scalars are at most XLEN bits and
aggregates whose total size does not exceed 2×XLEN bits in the riscv
calling convention [2].

Link: https://elixir.bootlin.com/linux/v6.8/source/kernel/bpf/btf.c#L6184 [0]
Link: https://elixir.bootlin.com/linux/v6.8/source/kernel/bpf/btf.c#L6769 [1]
Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/download/draft-20230929-e5c800e661a53efe3c2678d71a306323b60eb13b/riscv-abi.pdf [2]
Signed-off-by: Pu Lehui <pulehui@huawei.com>
Acked-by: Björn Töpel <bjorn@kernel.org>
Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
---
 arch/riscv/net/bpf_jit_comp64.c | 65 +++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 19 deletions(-)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 77ea306452d4..00723ac6cd1a 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -14,6 +14,7 @@
 #include <asm/cfi.h>
 #include "bpf_jit.h"
 
+#define RV_MAX_REG_ARGS 8
 #define RV_FENTRY_NINSNS 2
 
 #define RV_REG_TCC RV_REG_A6
@@ -688,26 +689,44 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
 	return ret;
 }
 
-static void store_args(int nregs, int args_off, struct rv_jit_context *ctx)
+static void store_args(int nr_arg_slots, int args_off, struct rv_jit_context *ctx)
 {
 	int i;
 
-	for (i = 0; i < nregs; i++) {
-		emit_sd(RV_REG_FP, -args_off, RV_REG_A0 + i, ctx);
+	for (i = 0; i < nr_arg_slots; i++) {
+		if (i < RV_MAX_REG_ARGS) {
+			emit_sd(RV_REG_FP, -args_off, RV_REG_A0 + i, ctx);
+		} else {
+			/* skip slots for T0 and FP of traced function */
+			emit_ld(RV_REG_T1, 16 + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);
+			emit_sd(RV_REG_FP, -args_off, RV_REG_T1, ctx);
+		}
 		args_off -= 8;
 	}
 }
 
-static void restore_args(int nregs, int args_off, struct rv_jit_context *ctx)
+static void restore_args(int nr_reg_args, int args_off, struct rv_jit_context *ctx)
 {
 	int i;
 
-	for (i = 0; i < nregs; i++) {
+	for (i = 0; i < nr_reg_args; i++) {
 		emit_ld(RV_REG_A0 + i, -args_off, RV_REG_FP, ctx);
 		args_off -= 8;
 	}
 }
 
+static void restore_stack_args(int nr_stack_args, int args_off, int stk_arg_off, struct rv_jit_context *ctx)
+{
+	int i;
+
+	for (i = 0; i < nr_stack_args; i++) {
+		emit_ld(RV_REG_T1, -(args_off - RV_MAX_REG_ARGS * 8), RV_REG_FP, ctx);
+		emit_sd(RV_REG_FP, -stk_arg_off, RV_REG_T1, ctx);
+		args_off -= 8;
+		stk_arg_off -= 8;
+	}
+}
+
 static int invoke_bpf_prog(struct bpf_tramp_link *l, int args_off, int retval_off,
 			   int run_ctx_off, bool save_ret, struct rv_jit_context *ctx)
 {
@@ -780,8 +799,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 {
 	int i, ret, offset;
 	int *branches_off = NULL;
-	int stack_size = 0, nregs = m->nr_args;
-	int retval_off, args_off, nregs_off, ip_off, run_ctx_off, sreg_off;
+	int stack_size = 0, nr_arg_slots = 0;
+	int retval_off, args_off, nregs_off, ip_off, run_ctx_off, sreg_off, stk_arg_off;
 	struct bpf_tramp_links *fentry = &tlinks[BPF_TRAMP_FENTRY];
 	struct bpf_tramp_links *fexit = &tlinks[BPF_TRAMP_FEXIT];
 	struct bpf_tramp_links *fmod_ret = &tlinks[BPF_TRAMP_MODIFY_RETURN];
@@ -827,20 +846,21 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 	 * FP - sreg_off    [ callee saved reg	]
 	 *
 	 *		    [ pads              ] pads for 16 bytes alignment
+	 *
+	 *		    [ stack_argN        ]
+	 *		    [ ...               ]
+	 * FP - stk_arg_off [ stack_arg1        ] BPF_TRAMP_F_CALL_ORIG
 	 */
 
 	if (flags & (BPF_TRAMP_F_ORIG_STACK | BPF_TRAMP_F_SHARE_IPMODIFY))
 		return -ENOTSUPP;
 
-	/* extra regiters for struct arguments */
-	for (i = 0; i < m->nr_args; i++)
-		if (m->arg_flags[i] & BTF_FMODEL_STRUCT_ARG)
-			nregs += round_up(m->arg_size[i], 8) / 8 - 1;
-
-	/* 8 arguments passed by registers */
-	if (nregs > 8)
+	if (m->nr_args > MAX_BPF_FUNC_ARGS)
 		return -ENOTSUPP;
 
+	for (i = 0; i < m->nr_args; i++)
+		nr_arg_slots += round_up(m->arg_size[i], 8) / 8;
+
 	/* room of trampoline frame to store return address and frame pointer */
 	stack_size += 16;
 
@@ -850,7 +870,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 		retval_off = stack_size;
 	}
 
-	stack_size += nregs * 8;
+	stack_size += nr_arg_slots * 8;
 	args_off = stack_size;
 
 	stack_size += 8;
@@ -867,8 +887,14 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 	stack_size += 8;
 	sreg_off = stack_size;
 
+	if (nr_arg_slots - RV_MAX_REG_ARGS > 0)
+		stack_size += (nr_arg_slots - RV_MAX_REG_ARGS) * 8;
+
 	stack_size = round_up(stack_size, 16);
 
+	/* room for args on stack must be at the top of stack */
+	stk_arg_off = stack_size;
+
 	if (!is_struct_ops) {
 		/* For the trampoline called from function entry,
 		 * the frame of traced function and the frame of
@@ -904,10 +930,10 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 		emit_sd(RV_REG_FP, -ip_off, RV_REG_T1, ctx);
 	}
 
-	emit_li(RV_REG_T1, nregs, ctx);
+	emit_li(RV_REG_T1, nr_arg_slots, ctx);
 	emit_sd(RV_REG_FP, -nregs_off, RV_REG_T1, ctx);
 
-	store_args(nregs, args_off, ctx);
+	store_args(nr_arg_slots, args_off, ctx);
 
 	/* skip to actual body of traced function */
 	if (flags & BPF_TRAMP_F_SKIP_FRAME)
@@ -947,7 +973,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 	}
 
 	if (flags & BPF_TRAMP_F_CALL_ORIG) {
-		restore_args(nregs, args_off, ctx);
+		restore_args(min_t(int, nr_arg_slots, RV_MAX_REG_ARGS), args_off, ctx);
+		restore_stack_args(nr_arg_slots - RV_MAX_REG_ARGS, args_off, stk_arg_off, ctx);
 		ret = emit_call((const u64)orig_call, true, ctx);
 		if (ret)
 			goto out;
@@ -982,7 +1009,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 	}
 
 	if (flags & BPF_TRAMP_F_RESTORE_REGS)
-		restore_args(nregs, args_off, ctx);
+		restore_args(min_t(int, nr_arg_slots, RV_MAX_REG_ARGS), args_off, ctx);
 
 	if (save_ret) {
 		emit_ld(RV_REG_A0, -retval_off, RV_REG_FP, ctx);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct
  2024-04-03  7:28 [PATCH bpf-next v3 0/2] Add 12-argument support for RV64 bpf trampoline Pu Lehui
  2024-04-03  7:28 ` [PATCH bpf-next v3 1/2] riscv, bpf: " Pu Lehui
@ 2024-04-03  7:28 ` Pu Lehui
  2024-04-03 14:40   ` Daniel Borkmann
  1 sibling, 1 reply; 6+ messages in thread
From: Pu Lehui @ 2024-04-03  7:28 UTC (permalink / raw
  To: bpf, linux-riscv, netdev
  Cc: Björn Töpel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Palmer Dabbelt, Pu Lehui, Pu Lehui

From: Pu Lehui <pulehui@huawei.com>

Add testcase where 7th argument is struct for architectures with 8
argument registers, and increase the complexity of the struct.

Signed-off-by: Pu Lehui <pulehui@huawei.com>
Acked-by: Björn Töpel <bjorn@kernel.org>
Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
---
 tools/testing/selftests/bpf/DENYLIST.aarch64  |  1 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 19 ++++++++++
 .../selftests/bpf/prog_tests/tracing_struct.c | 13 +++++++
 .../selftests/bpf/progs/tracing_struct.c      | 35 +++++++++++++++++++
 4 files changed, 68 insertions(+)

diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
index d8ade15e2789..639ee3f5bc74 100644
--- a/tools/testing/selftests/bpf/DENYLIST.aarch64
+++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
@@ -6,6 +6,7 @@ kprobe_multi_test                                # needs CONFIG_FPROBE
 module_attach                                    # prog 'kprobe_multi': failed to auto-attach: -95
 fentry_test/fentry_many_args                     # fentry_many_args:FAIL:fentry_many_args_attach unexpected error: -524
 fexit_test/fexit_many_args                       # fexit_many_args:FAIL:fexit_many_args_attach unexpected error: -524
+tracing_struct                                   # test_fentry:FAIL:tracing_struct__attach unexpected error: -524
 fill_link_info/kprobe_multi_link_info            # bpf_program__attach_kprobe_multi_opts unexpected error: -95
 fill_link_info/kretprobe_multi_link_info         # bpf_program__attach_kprobe_multi_opts unexpected error: -95
 fill_link_info/kprobe_multi_invalid_ubuff        # bpf_program__attach_kprobe_multi_opts unexpected error: -95
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 39ad96a18123..4973d9de10af 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -41,6 +41,13 @@ struct bpf_testmod_struct_arg_4 {
 	int b;
 };
 
+struct bpf_testmod_struct_arg_5 {
+	char a;
+	short b;
+	int c;
+	long d;
+};
+
 __bpf_hook_start();
 
 noinline int
@@ -98,6 +105,15 @@ bpf_testmod_test_struct_arg_8(u64 a, void *b, short c, int d, void *e,
 	return bpf_testmod_test_struct_arg_result;
 }
 
+noinline int
+bpf_testmod_test_struct_arg_9(u64 a, void *b, short c, int d, void *e, char f,
+			      short g, struct bpf_testmod_struct_arg_5 h, long i)
+{
+	bpf_testmod_test_struct_arg_result = a + (long)b + c + d + (long)e +
+		f + g + h.a + h.b + h.c + h.d + i;
+	return bpf_testmod_test_struct_arg_result;
+}
+
 noinline int
 bpf_testmod_test_arg_ptr_to_struct(struct bpf_testmod_struct_arg_1 *a) {
 	bpf_testmod_test_struct_arg_result = a->a;
@@ -254,6 +270,7 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 	struct bpf_testmod_struct_arg_2 struct_arg2 = {2, 3};
 	struct bpf_testmod_struct_arg_3 *struct_arg3;
 	struct bpf_testmod_struct_arg_4 struct_arg4 = {21, 22};
+	struct bpf_testmod_struct_arg_5 struct_arg5 = {23, 24, 25, 26};
 	int i = 1;
 
 	while (bpf_testmod_return_ptr(i))
@@ -268,6 +285,8 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 					    (void *)20, struct_arg4);
 	(void)bpf_testmod_test_struct_arg_8(16, (void *)17, 18, 19,
 					    (void *)20, struct_arg4, 23);
+	(void)bpf_testmod_test_struct_arg_9(16, (void *)17, 18, 19, (void *)20,
+					    21, 22, struct_arg5, 27);
 
 	(void)bpf_testmod_test_arg_ptr_to_struct(&struct_arg1_2);
 
diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
index fe0fb0c9849a..5a8eeb07a6ba 100644
--- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
+++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
@@ -74,6 +74,19 @@ static void test_fentry(void)
 	ASSERT_EQ(skel->bss->t8_g, 23, "t8:g");
 	ASSERT_EQ(skel->bss->t8_ret, 156, "t8 ret");
 
+	ASSERT_EQ(skel->bss->t9_a, 16, "t9:a");
+	ASSERT_EQ(skel->bss->t9_b, 17, "t9:b");
+	ASSERT_EQ(skel->bss->t9_c, 18, "t9:c");
+	ASSERT_EQ(skel->bss->t9_d, 19, "t9:d");
+	ASSERT_EQ(skel->bss->t9_e, 20, "t9:e");
+	ASSERT_EQ(skel->bss->t9_f, 21, "t9:f");
+	ASSERT_EQ(skel->bss->t9_g, 22, "t9:f");
+	ASSERT_EQ(skel->bss->t9_h_a, 23, "t9:h.a");
+	ASSERT_EQ(skel->bss->t9_h_b, 24, "t9:h.b");
+	ASSERT_EQ(skel->bss->t9_h_c, 25, "t9:h.c");
+	ASSERT_EQ(skel->bss->t9_h_d, 26, "t9:h.d");
+	ASSERT_EQ(skel->bss->t9_i, 27, "t9:i");
+	ASSERT_EQ(skel->bss->t9_ret, 258, "t9 ret");
 	tracing_struct__detach(skel);
 destroy_skel:
 	tracing_struct__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/tracing_struct.c b/tools/testing/selftests/bpf/progs/tracing_struct.c
index 515daef3c84b..bfe96bab94c0 100644
--- a/tools/testing/selftests/bpf/progs/tracing_struct.c
+++ b/tools/testing/selftests/bpf/progs/tracing_struct.c
@@ -23,6 +23,13 @@ struct bpf_testmod_struct_arg_4 {
 	int b;
 };
 
+struct bpf_testmod_struct_arg_5 {
+	char a;
+	short b;
+	int c;
+	long d;
+};
+
 long t1_a_a, t1_a_b, t1_b, t1_c, t1_ret, t1_nregs;
 __u64 t1_reg0, t1_reg1, t1_reg2, t1_reg3;
 long t2_a, t2_b_a, t2_b_b, t2_c, t2_ret;
@@ -32,6 +39,7 @@ long t5_ret;
 int t6;
 long t7_a, t7_b, t7_c, t7_d, t7_e, t7_f_a, t7_f_b, t7_ret;
 long t8_a, t8_b, t8_c, t8_d, t8_e, t8_f_a, t8_f_b, t8_g, t8_ret;
+long t9_a, t9_b, t9_c, t9_d, t9_e, t9_f, t9_g, t9_h_a, t9_h_b, t9_h_c, t9_h_d, t9_i, t9_ret;
 
 
 SEC("fentry/bpf_testmod_test_struct_arg_1")
@@ -184,4 +192,31 @@ int BPF_PROG2(test_struct_arg_15, __u64, a, void *, b, short, c, int, d,
 	return 0;
 }
 
+SEC("fentry/bpf_testmod_test_struct_arg_9")
+int BPF_PROG2(test_struct_arg_16, __u64, a, void *, b, short, c, int, d, void *, e,
+	      char, f, short, g, struct bpf_testmod_struct_arg_5, h, long, i)
+{
+	t9_a = a;
+	t9_b = (long)b;
+	t9_c = c;
+	t9_d = d;
+	t9_e = (long)e;
+	t9_f = f;
+	t9_g = g;
+	t9_h_a = h.a;
+	t9_h_b = h.b;
+	t9_h_c = h.c;
+	t9_h_d = h.d;
+	t9_i = i;
+	return 0;
+}
+
+SEC("fexit/bpf_testmod_test_struct_arg_9")
+int BPF_PROG2(test_struct_arg_17, __u64, a, void *, b, short, c, int, d, void *, e,
+	      char, f, short, g, struct bpf_testmod_struct_arg_5, h, long, i, int, ret)
+{
+	t9_ret = ret;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct
  2024-04-03  7:28 ` [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct Pu Lehui
@ 2024-04-03 14:40   ` Daniel Borkmann
  2024-04-03 15:50     ` Pu Lehui
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2024-04-03 14:40 UTC (permalink / raw
  To: Pu Lehui, bpf, linux-riscv, netdev
  Cc: Björn Töpel, Alexei Starovoitov, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Palmer Dabbelt, Pu Lehui

On 4/3/24 9:28 AM, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
> 
> Add testcase where 7th argument is struct for architectures with 8
> argument registers, and increase the complexity of the struct.
> 
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> Acked-by: Björn Töpel <bjorn@kernel.org>
> Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
> ---
>   tools/testing/selftests/bpf/DENYLIST.aarch64  |  1 +
>   .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 19 ++++++++++
>   .../selftests/bpf/prog_tests/tracing_struct.c | 13 +++++++
>   .../selftests/bpf/progs/tracing_struct.c      | 35 +++++++++++++++++++
>   4 files changed, 68 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
> index d8ade15e2789..639ee3f5bc74 100644
> --- a/tools/testing/selftests/bpf/DENYLIST.aarch64
> +++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
> @@ -6,6 +6,7 @@ kprobe_multi_test                                # needs CONFIG_FPROBE
>   module_attach                                    # prog 'kprobe_multi': failed to auto-attach: -95
>   fentry_test/fentry_many_args                     # fentry_many_args:FAIL:fentry_many_args_attach unexpected error: -524
>   fexit_test/fexit_many_args                       # fexit_many_args:FAIL:fexit_many_args_attach unexpected error: -524
> +tracing_struct                                   # test_fentry:FAIL:tracing_struct__attach unexpected error: -524

Do we need to blacklist the whole test given it had coverage on arm64
before.. perhaps this test here could be done as a new subtest and only
that one is listed for arm64?

>   fill_link_info/kprobe_multi_link_info            # bpf_program__attach_kprobe_multi_opts unexpected error: -95
>   fill_link_info/kretprobe_multi_link_info         # bpf_program__attach_kprobe_multi_opts unexpected error: -95
>   fill_link_info/kprobe_multi_invalid_ubuff        # bpf_program__attach_kprobe_multi_opts unexpected error: -95

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct
  2024-04-03 14:40   ` Daniel Borkmann
@ 2024-04-03 15:50     ` Pu Lehui
  2024-04-08 13:58       ` Pu Lehui
  0 siblings, 1 reply; 6+ messages in thread
From: Pu Lehui @ 2024-04-03 15:50 UTC (permalink / raw
  To: Daniel Borkmann, bpf, linux-riscv, netdev
  Cc: Björn Töpel, Alexei Starovoitov, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Palmer Dabbelt, Pu Lehui



On 2024/4/3 22:40, Daniel Borkmann wrote:
> On 4/3/24 9:28 AM, Pu Lehui wrote:
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> Add testcase where 7th argument is struct for architectures with 8
>> argument registers, and increase the complexity of the struct.
>>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>> Acked-by: Björn Töpel <bjorn@kernel.org>
>> Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
>> ---
>>   tools/testing/selftests/bpf/DENYLIST.aarch64  |  1 +
>>   .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 19 ++++++++++
>>   .../selftests/bpf/prog_tests/tracing_struct.c | 13 +++++++
>>   .../selftests/bpf/progs/tracing_struct.c      | 35 +++++++++++++++++++
>>   4 files changed, 68 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 
>> b/tools/testing/selftests/bpf/DENYLIST.aarch64
>> index d8ade15e2789..639ee3f5bc74 100644
>> --- a/tools/testing/selftests/bpf/DENYLIST.aarch64
>> +++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
>> @@ -6,6 +6,7 @@ kprobe_multi_test                                # 
>> needs CONFIG_FPROBE
>>   module_attach                                    # prog 
>> 'kprobe_multi': failed to auto-attach: -95
>>   fentry_test/fentry_many_args                     # 
>> fentry_many_args:FAIL:fentry_many_args_attach unexpected error: -524
>>   fexit_test/fexit_many_args                       # 
>> fexit_many_args:FAIL:fexit_many_args_attach unexpected error: -524
>> +tracing_struct                                   # 
>> test_fentry:FAIL:tracing_struct__attach unexpected error: -524
> 
> Do we need to blacklist the whole test given it had coverage on arm64
> before.. perhaps this test here could be done as a new subtest and only
> that one is listed for arm64?

Yeah, I thought so at first, just like fexit_many_args of fentry/fexit, 
but I found that the things struct_tracing does are all in the same 
series, but the number or type of parameters are different, and the new 
use case I added is the same in this way. And I found that the execution 
logic of stract_tracing is relatively simple and clear, triggering all 
hook points, executing all bpf programs, and asserting all parameters.
Shall we need to slice them up?

> 
>>   fill_link_info/kprobe_multi_link_info            # 
>> bpf_program__attach_kprobe_multi_opts unexpected error: -95
>>   fill_link_info/kretprobe_multi_link_info         # 
>> bpf_program__attach_kprobe_multi_opts unexpected error: -95
>>   fill_link_info/kprobe_multi_invalid_ubuff        # 
>> bpf_program__attach_kprobe_multi_opts unexpected error: -95
> 
> Thanks,
> Daniel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct
  2024-04-03 15:50     ` Pu Lehui
@ 2024-04-08 13:58       ` Pu Lehui
  0 siblings, 0 replies; 6+ messages in thread
From: Pu Lehui @ 2024-04-08 13:58 UTC (permalink / raw
  To: Daniel Borkmann, bpf, linux-riscv, netdev
  Cc: Björn Töpel, Alexei Starovoitov, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Palmer Dabbelt, Pu Lehui


On 2024/4/3 23:50, Pu Lehui wrote:
> 
> 
> On 2024/4/3 22:40, Daniel Borkmann wrote:
>> On 4/3/24 9:28 AM, Pu Lehui wrote:
>>> From: Pu Lehui <pulehui@huawei.com>
>>>
>>> Add testcase where 7th argument is struct for architectures with 8
>>> argument registers, and increase the complexity of the struct.
>>>
>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>> Acked-by: Björn Töpel <bjorn@kernel.org>
>>> Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
>>> ---
>>>   tools/testing/selftests/bpf/DENYLIST.aarch64  |  1 +
>>>   .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 19 ++++++++++
>>>   .../selftests/bpf/prog_tests/tracing_struct.c | 13 +++++++
>>>   .../selftests/bpf/progs/tracing_struct.c      | 35 +++++++++++++++++++
>>>   4 files changed, 68 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 
>>> b/tools/testing/selftests/bpf/DENYLIST.aarch64
>>> index d8ade15e2789..639ee3f5bc74 100644
>>> --- a/tools/testing/selftests/bpf/DENYLIST.aarch64
>>> +++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
>>> @@ -6,6 +6,7 @@ kprobe_multi_test                                # 
>>> needs CONFIG_FPROBE
>>>   module_attach                                    # prog 
>>> 'kprobe_multi': failed to auto-attach: -95
>>>   fentry_test/fentry_many_args                     # 
>>> fentry_many_args:FAIL:fentry_many_args_attach unexpected error: -524
>>>   fexit_test/fexit_many_args                       # 
>>> fexit_many_args:FAIL:fexit_many_args_attach unexpected error: -524
>>> +tracing_struct                                   # 
>>> test_fentry:FAIL:tracing_struct__attach unexpected error: -524
>>
>> Do we need to blacklist the whole test given it had coverage on arm64
>> before.. perhaps this test here could be done as a new subtest and only
>> that one is listed for arm64?
> 
> Yeah, I thought so at first, just like fexit_many_args of fentry/fexit, 
> but I found that the things struct_tracing does are all in the same 
> series, but the number or type of parameters are different, and the new 
> use case I added is the same in this way. And I found that the execution 
> logic of stract_tracing is relatively simple and clear, triggering all 
> hook points, executing all bpf programs, and asserting all parameters.
> Shall we need to slice them up?

ping~ Daniel, shall we need to do that?

> 
>>
>>>   fill_link_info/kprobe_multi_link_info            # 
>>> bpf_program__attach_kprobe_multi_opts unexpected error: -95
>>>   fill_link_info/kretprobe_multi_link_info         # 
>>> bpf_program__attach_kprobe_multi_opts unexpected error: -95
>>>   fill_link_info/kprobe_multi_invalid_ubuff        # 
>>> bpf_program__attach_kprobe_multi_opts unexpected error: -95
>>
>> Thanks,
>> Daniel
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-04-08 13:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-03  7:28 [PATCH bpf-next v3 0/2] Add 12-argument support for RV64 bpf trampoline Pu Lehui
2024-04-03  7:28 ` [PATCH bpf-next v3 1/2] riscv, bpf: " Pu Lehui
2024-04-03  7:28 ` [PATCH bpf-next v3 2/2] selftests/bpf: Add testcase where 7th argment is struct Pu Lehui
2024-04-03 14:40   ` Daniel Borkmann
2024-04-03 15:50     ` Pu Lehui
2024-04-08 13:58       ` Pu Lehui

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 read-only IMAP folder(s) and NNTP newsgroup(s).