netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
@ 2023-01-05  3:06 tong
  2023-01-05  9:36 ` Björn Töpel
  2023-01-05 17:53 ` Christophe Leroy
  0 siblings, 2 replies; 23+ messages in thread
From: tong @ 2023-01-05  3:06 UTC (permalink / raw)
  To: bpf, netdev, linux-arm-kernel, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux
  Cc: Tonghao Zhang, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Hou Tao

From: Tonghao Zhang <tong@infragraf.org>

The x86_64 can't dump the valid insn in this way. A test BPF prog
which include subprog:

$ llvm-objdump -d subprog.o
Disassembly of section .text:
0000000000000000 <subprog>:
       0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1 = 29114459903653235 ll
       2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
       3:       bf a1 00 00 00 00 00 00 r1 = r10
       4:       07 01 00 00 f8 ff ff ff r1 += -8
       5:       b7 02 00 00 08 00 00 00 r2 = 8
       6:       85 00 00 00 06 00 00 00 call 6
       7:       95 00 00 00 00 00 00 00 exit
Disassembly of section raw_tp/sys_enter:
0000000000000000 <entry>:
       0:       85 10 00 00 ff ff ff ff call -1
       1:       b7 00 00 00 00 00 00 00 r0 = 0
       2:       95 00 00 00 00 00 00 00 exit

kernel print message:
[  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c from=kprobe-load pid=1643
[  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
[  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
[  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
[  580.782568] JIT code: 00000030: cc cc cc

$ bpf_jit_disasm
51 bytes emitted from JIT compiler (pass:3, flen:8)
ffffffffa000c20c + <x>:
   0:   int3
   1:   int3
   2:   int3
   3:   int3
   4:   int3
   5:   int3
   ...

Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to header
and then image/insn is valid. BTW, we can use the "bpftool prog dump" JITed instructions.

Signed-off-by: Tonghao Zhang <tong@infragraf.org>
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Martin KaFai Lau <martin.lau@linux.dev>
Cc: Song Liu <song@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Stanislav Fomichev <sdf@google.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Hou Tao <houtao1@huawei.com>
---
 Documentation/admin-guide/sysctl/net.rst |   1 +
 Documentation/networking/filter.rst      |  98 +------
 arch/arm/net/bpf_jit_32.c                |   4 -
 arch/arm64/net/bpf_jit_comp.c            |   4 -
 arch/loongarch/net/bpf_jit.c             |   4 -
 arch/mips/net/bpf_jit_comp.c             |   3 -
 arch/powerpc/net/bpf_jit_comp.c          |  11 -
 arch/riscv/net/bpf_jit_core.c            |   3 -
 arch/s390/net/bpf_jit_comp.c             |   4 -
 arch/sparc/net/bpf_jit_comp_32.c         |   3 -
 arch/sparc/net/bpf_jit_comp_64.c         |  13 -
 arch/x86/net/bpf_jit_comp.c              |   3 -
 arch/x86/net/bpf_jit_comp32.c            |   3 -
 net/core/sysctl_net_core.c               |  12 +-
 tools/bpf/.gitignore                     |   1 -
 tools/bpf/Makefile                       |  10 +-
 tools/bpf/bpf_jit_disasm.c               | 332 -----------------------
 17 files changed, 9 insertions(+), 500 deletions(-)
 delete mode 100644 tools/bpf/bpf_jit_disasm.c

diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
index 6394f5dc2303..82ca05ca6ed0 100644
--- a/Documentation/admin-guide/sysctl/net.rst
+++ b/Documentation/admin-guide/sysctl/net.rst
@@ -88,6 +88,7 @@ Values:
 	- 0 - disable the JIT (default value)
 	- 1 - enable the JIT
 	- 2 - enable the JIT and ask the compiler to emit traces on kernel log.
+              (deprecated since v6.3, use ``bpftool prog dump jited id <id>`` instead)
 
 bpf_jit_harden
 --------------
diff --git a/Documentation/networking/filter.rst b/Documentation/networking/filter.rst
index f69da5074860..5f51c050e88f 100644
--- a/Documentation/networking/filter.rst
+++ b/Documentation/networking/filter.rst
@@ -520,102 +520,8 @@ been previously enabled by root::
 
   echo 1 > /proc/sys/net/core/bpf_jit_enable
 
-For JIT developers, doing audits etc, each compile run can output the generated
-opcode image into the kernel log via::
-
-  echo 2 > /proc/sys/net/core/bpf_jit_enable
-
-Example output from dmesg::
-
-    [ 3389.935842] flen=6 proglen=70 pass=3 image=ffffffffa0069c8f
-    [ 3389.935847] JIT code: 00000000: 55 48 89 e5 48 83 ec 60 48 89 5d f8 44 8b 4f 68
-    [ 3389.935849] JIT code: 00000010: 44 2b 4f 6c 4c 8b 87 d8 00 00 00 be 0c 00 00 00
-    [ 3389.935850] JIT code: 00000020: e8 1d 94 ff e0 3d 00 08 00 00 75 16 be 17 00 00
-    [ 3389.935851] JIT code: 00000030: 00 e8 28 94 ff e0 83 f8 01 75 07 b8 ff ff 00 00
-    [ 3389.935852] JIT code: 00000040: eb 02 31 c0 c9 c3
-
-When CONFIG_BPF_JIT_ALWAYS_ON is enabled, bpf_jit_enable is permanently set to 1 and
-setting any other value than that will return in failure. This is even the case for
-setting bpf_jit_enable to 2, since dumping the final JIT image into the kernel log
-is discouraged and introspection through bpftool (under tools/bpf/bpftool/) is the
-generally recommended approach instead.
-
-In the kernel source tree under tools/bpf/, there's bpf_jit_disasm for
-generating disassembly out of the kernel log's hexdump::
-
-	# ./bpf_jit_disasm
-	70 bytes emitted from JIT compiler (pass:3, flen:6)
-	ffffffffa0069c8f + <x>:
-	0:	push   %rbp
-	1:	mov    %rsp,%rbp
-	4:	sub    $0x60,%rsp
-	8:	mov    %rbx,-0x8(%rbp)
-	c:	mov    0x68(%rdi),%r9d
-	10:	sub    0x6c(%rdi),%r9d
-	14:	mov    0xd8(%rdi),%r8
-	1b:	mov    $0xc,%esi
-	20:	callq  0xffffffffe0ff9442
-	25:	cmp    $0x800,%eax
-	2a:	jne    0x0000000000000042
-	2c:	mov    $0x17,%esi
-	31:	callq  0xffffffffe0ff945e
-	36:	cmp    $0x1,%eax
-	39:	jne    0x0000000000000042
-	3b:	mov    $0xffff,%eax
-	40:	jmp    0x0000000000000044
-	42:	xor    %eax,%eax
-	44:	leaveq
-	45:	retq
-
-	Issuing option `-o` will "annotate" opcodes to resulting assembler
-	instructions, which can be very useful for JIT developers:
-
-	# ./bpf_jit_disasm -o
-	70 bytes emitted from JIT compiler (pass:3, flen:6)
-	ffffffffa0069c8f + <x>:
-	0:	push   %rbp
-		55
-	1:	mov    %rsp,%rbp
-		48 89 e5
-	4:	sub    $0x60,%rsp
-		48 83 ec 60
-	8:	mov    %rbx,-0x8(%rbp)
-		48 89 5d f8
-	c:	mov    0x68(%rdi),%r9d
-		44 8b 4f 68
-	10:	sub    0x6c(%rdi),%r9d
-		44 2b 4f 6c
-	14:	mov    0xd8(%rdi),%r8
-		4c 8b 87 d8 00 00 00
-	1b:	mov    $0xc,%esi
-		be 0c 00 00 00
-	20:	callq  0xffffffffe0ff9442
-		e8 1d 94 ff e0
-	25:	cmp    $0x800,%eax
-		3d 00 08 00 00
-	2a:	jne    0x0000000000000042
-		75 16
-	2c:	mov    $0x17,%esi
-		be 17 00 00 00
-	31:	callq  0xffffffffe0ff945e
-		e8 28 94 ff e0
-	36:	cmp    $0x1,%eax
-		83 f8 01
-	39:	jne    0x0000000000000042
-		75 07
-	3b:	mov    $0xffff,%eax
-		b8 ff ff 00 00
-	40:	jmp    0x0000000000000044
-		eb 02
-	42:	xor    %eax,%eax
-		31 c0
-	44:	leaveq
-		c9
-	45:	retq
-		c3
-
-For BPF JIT developers, bpf_jit_disasm, bpf_asm and bpf_dbg provides a useful
-toolchain for developing and testing the kernel's JIT compiler.
+For JIT developers, doing audits etc, should use `bpftool prog dump` to
+veiw the JIT generated opcode image.
 
 BPF kernel internals
 --------------------
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 6a1c9fca5260..39301d59b537 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1999,10 +1999,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	}
 	flush_icache_range((u32)header, (u32)(ctx.target + ctx.idx));
 
-	if (bpf_jit_enable > 1)
-		/* there are 2 passes here */
-		bpf_jit_dump(prog->len, image_size, 2, ctx.target);
-
 	bpf_jit_binary_lock_ro(header);
 	prog->bpf_func = (void *)ctx.target;
 	prog->jited = 1;
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 62f805f427b7..59c35b4d77b7 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1567,10 +1567,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		goto out_off;
 	}
 
-	/* And we're done. */
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(prog->len, prog_size, 2, ctx.image);
-
 	bpf_flush_icache(header, ctx.image + ctx.idx);
 
 	if (!prog->is_func || extra_pass) {
diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index bdcd0c7719a9..ea37f52faa6e 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -1123,10 +1123,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		goto out_offset;
 	}
 
-	/* And we're done */
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(prog->len, image_size, 2, ctx.image);
-
 	/* Update the icache */
 	flush_icache_range((unsigned long)header, (unsigned long)(ctx.image + ctx.idx));
 
diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c
index b17130d510d4..ccbb7c231cb0 100644
--- a/arch/mips/net/bpf_jit_comp.c
+++ b/arch/mips/net/bpf_jit_comp.c
@@ -1012,9 +1012,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	flush_icache_range((unsigned long)header,
 			   (unsigned long)&ctx.target[ctx.jit_index]);
 
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(prog->len, image_size, 2, ctx.target);
-
 	prog->bpf_func = (void *)ctx.target;
 	prog->jited = 1;
 	prog->jited_len = image_size;
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 43e634126514..f0f7d8ff2022 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -262,20 +262,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 			goto out_addrs;
 		}
 		bpf_jit_build_epilogue(code_base, &cgctx);
-
-		if (bpf_jit_enable > 1)
-			pr_info("Pass %d: shrink = %d, seen = 0x%x\n", pass,
-				proglen - (cgctx.idx * 4), cgctx.seen);
 	}
 
 skip_codegen_passes:
-	if (bpf_jit_enable > 1)
-		/*
-		 * Note that we output the base address of the code_base
-		 * rather than image, since opcodes are in code_base.
-		 */
-		bpf_jit_dump(flen, proglen, pass, code_base);
-
 #ifdef CONFIG_PPC64_ELF_ABI_V1
 	/* Function descriptor nastiness: Address + TOC */
 	((u64 *)image)[0] = (u64)code_base;
diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
index 737baf8715da..ff168c50d46a 100644
--- a/arch/riscv/net/bpf_jit_core.c
+++ b/arch/riscv/net/bpf_jit_core.c
@@ -151,9 +151,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	}
 	bpf_jit_build_epilogue(ctx);
 
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(prog->len, prog_size, pass, ctx->insns);
-
 	prog->bpf_func = (void *)ctx->insns;
 	prog->jited = 1;
 	prog->jited_len = prog_size;
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index af35052d06ed..13d996e27602 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -1831,10 +1831,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		fp = orig_fp;
 		goto free_addrs;
 	}
-	if (bpf_jit_enable > 1) {
-		bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
-		print_fn_code(jit.prg_buf, jit.size_prg);
-	}
 	if (!fp->is_func || extra_pass) {
 		bpf_jit_binary_lock_ro(header);
 	} else {
diff --git a/arch/sparc/net/bpf_jit_comp_32.c b/arch/sparc/net/bpf_jit_comp_32.c
index a74e5004c6c8..08de0ed84831 100644
--- a/arch/sparc/net/bpf_jit_comp_32.c
+++ b/arch/sparc/net/bpf_jit_comp_32.c
@@ -743,9 +743,6 @@ cond_branch:			f_offset = addrs[i + filter[i].jf];
 		oldproglen = proglen;
 	}
 
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(flen, proglen, pass + 1, image);
-
 	if (image) {
 		fp->bpf_func = (void *)image;
 		fp->jited = 1;
diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
index fa0759bfe498..14c9e5ce4100 100644
--- a/arch/sparc/net/bpf_jit_comp_64.c
+++ b/arch/sparc/net/bpf_jit_comp_64.c
@@ -1549,16 +1549,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		}
 		build_epilogue(&ctx);
 
-		if (bpf_jit_enable > 1)
-			pr_info("Pass %d: size = %u, seen = [%c%c%c%c%c%c]\n", pass,
-				ctx.idx * 4,
-				ctx.tmp_1_used ? '1' : ' ',
-				ctx.tmp_2_used ? '2' : ' ',
-				ctx.tmp_3_used ? '3' : ' ',
-				ctx.saw_frame_pointer ? 'F' : ' ',
-				ctx.saw_call ? 'C' : ' ',
-				ctx.saw_tail_call ? 'T' : ' ');
-
 		if (ctx.idx * 4 == prev_image_size)
 			break;
 		prev_image_size = ctx.idx * 4;
@@ -1596,9 +1586,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		goto out_off;
 	}
 
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(prog->len, image_size, pass, ctx.image);
-
 	bpf_flush_icache(header, (u8 *)header + header->size);
 
 	if (!prog->is_func || extra_pass) {
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index e3e2b57e4e13..197ff8651a56 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2551,9 +2551,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		cond_resched();
 	}
 
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(prog->len, proglen, pass + 1, image);
-
 	if (image) {
 		if (!prog->is_func || extra_pass) {
 			/*
diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index 429a89c5468b..ca53f20aca73 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -2597,9 +2597,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		cond_resched();
 	}
 
-	if (bpf_jit_enable > 1)
-		bpf_jit_dump(prog->len, proglen, pass + 1, image);
-
 	if (image) {
 		bpf_jit_binary_lock_ro(header);
 		prog->bpf_func = (void *)image;
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 5b1ce656baa1..562ace48e1c9 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -276,14 +276,10 @@ static int proc_dointvec_minmax_bpf_enable(struct ctl_table *table, int write,
 	tmp.data = &jit_enable;
 	ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
 	if (write && !ret) {
-		if (jit_enable < 2 ||
-		    (jit_enable == 2 && bpf_dump_raw_ok(current_cred()))) {
-			*(int *)table->data = jit_enable;
-			if (jit_enable == 2)
-				pr_warn("bpf_jit_enable = 2 was set! NEVER use this in production, only for JIT debugging!\n");
-		} else {
-			ret = -EPERM;
-		}
+		*(int *)table->data = jit_enable;
+
+		if (jit_enable == 2)
+			pr_warn_once("bpf_jit_enable == 2 was deprecated! Use bpftool prog dump instead.\n");
 	}
 
 	if (write && ret && min == max)
diff --git a/tools/bpf/.gitignore b/tools/bpf/.gitignore
index cf53342175e7..5c70cfb9092e 100644
--- a/tools/bpf/.gitignore
+++ b/tools/bpf/.gitignore
@@ -4,4 +4,3 @@ feature
 bpf_asm
 bpf_dbg
 bpf_exp.yacc.*
-bpf_jit_disasm
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 243b79f2b451..9264d7b0edf6 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -74,14 +74,10 @@ $(OUTPUT)%.yacc.o: $(OUTPUT)%.yacc.c
 $(OUTPUT)%.lex.o: $(OUTPUT)%.lex.c
 	$(QUIET_CC)$(CC) $(CFLAGS) -c -o $@ $<
 
-PROGS = $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
+PROGS = $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
 
 all: $(PROGS) bpftool runqslower
 
-$(OUTPUT)bpf_jit_disasm: CFLAGS += -DPACKAGE='bpf_jit_disasm'
-$(OUTPUT)bpf_jit_disasm: $(OUTPUT)bpf_jit_disasm.o
-	$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lopcodes -lbfd -ldl
-
 $(OUTPUT)bpf_dbg: $(OUTPUT)bpf_dbg.o
 	$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lreadline
 
@@ -94,16 +90,14 @@ $(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.lex.c
 
 clean: bpftool_clean runqslower_clean resolve_btfids_clean
 	$(call QUIET_CLEAN, bpf-progs)
-	$(Q)$(RM) -r -- $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
+	$(Q)$(RM) -r -- $(OUTPUT)*.o $(OUTPUT)bpf_dbg \
 	       $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
 	$(call QUIET_CLEAN, core-gen)
 	$(Q)$(RM) -- $(OUTPUT)FEATURE-DUMP.bpf
 	$(Q)$(RM) -r -- $(OUTPUT)feature
 
 install: $(PROGS) bpftool_install
-	$(call QUIET_INSTALL, bpf_jit_disasm)
 	$(Q)$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/bin
-	$(Q)$(INSTALL) $(OUTPUT)bpf_jit_disasm $(DESTDIR)$(prefix)/bin/bpf_jit_disasm
 	$(call QUIET_INSTALL, bpf_dbg)
 	$(Q)$(INSTALL) $(OUTPUT)bpf_dbg $(DESTDIR)$(prefix)/bin/bpf_dbg
 	$(call QUIET_INSTALL, bpf_asm)
diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
deleted file mode 100644
index a90a5d110f92..000000000000
--- a/tools/bpf/bpf_jit_disasm.c
+++ /dev/null
@@ -1,332 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Minimal BPF JIT image disassembler
- *
- * Disassembles BPF JIT compiler emitted opcodes back to asm insn's for
- * debugging or verification purposes.
- *
- * To get the disassembly of the JIT code, do the following:
- *
- *  1) `echo 2 > /proc/sys/net/core/bpf_jit_enable`
- *  2) Load a BPF filter (e.g. `tcpdump -p -n -s 0 -i eth1 host 192.168.20.0/24`)
- *  3) Run e.g. `bpf_jit_disasm -o` to read out the last JIT code
- *
- * Copyright 2013 Daniel Borkmann <borkmann@redhat.com>
- */
-
-#include <stdint.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <assert.h>
-#include <unistd.h>
-#include <string.h>
-#include <bfd.h>
-#include <dis-asm.h>
-#include <regex.h>
-#include <fcntl.h>
-#include <sys/klog.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <limits.h>
-#include <tools/dis-asm-compat.h>
-
-#define CMD_ACTION_SIZE_BUFFER		10
-#define CMD_ACTION_READ_ALL		3
-
-static void get_exec_path(char *tpath, size_t size)
-{
-	char *path;
-	ssize_t len;
-
-	snprintf(tpath, size, "/proc/%d/exe", (int) getpid());
-	tpath[size - 1] = 0;
-
-	path = strdup(tpath);
-	assert(path);
-
-	len = readlink(path, tpath, size);
-	tpath[len] = 0;
-
-	free(path);
-}
-
-static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
-{
-	int count, i, pc = 0;
-	char tpath[PATH_MAX];
-	struct disassemble_info info;
-	disassembler_ftype disassemble;
-	bfd *bfdf;
-
-	memset(tpath, 0, sizeof(tpath));
-	get_exec_path(tpath, sizeof(tpath));
-
-	bfdf = bfd_openr(tpath, NULL);
-	assert(bfdf);
-	assert(bfd_check_format(bfdf, bfd_object));
-
-	init_disassemble_info_compat(&info, stdout,
-				     (fprintf_ftype) fprintf,
-				     fprintf_styled);
-	info.arch = bfd_get_arch(bfdf);
-	info.mach = bfd_get_mach(bfdf);
-	info.buffer = image;
-	info.buffer_length = len;
-
-	disassemble_init_for_target(&info);
-
-#ifdef DISASM_FOUR_ARGS_SIGNATURE
-	disassemble = disassembler(info.arch,
-				   bfd_big_endian(bfdf),
-				   info.mach,
-				   bfdf);
-#else
-	disassemble = disassembler(bfdf);
-#endif
-	assert(disassemble);
-
-	do {
-		printf("%4x:\t", pc);
-
-		count = disassemble(pc, &info);
-
-		if (opcodes) {
-			printf("\n\t");
-			for (i = 0; i < count; ++i)
-				printf("%02x ", (uint8_t) image[pc + i]);
-		}
-		printf("\n");
-
-		pc += count;
-	} while(count > 0 && pc < len);
-
-	bfd_close(bfdf);
-}
-
-static char *get_klog_buff(unsigned int *klen)
-{
-	int ret, len;
-	char *buff;
-
-	len = klogctl(CMD_ACTION_SIZE_BUFFER, NULL, 0);
-	if (len < 0)
-		return NULL;
-
-	buff = malloc(len);
-	if (!buff)
-		return NULL;
-
-	ret = klogctl(CMD_ACTION_READ_ALL, buff, len);
-	if (ret < 0) {
-		free(buff);
-		return NULL;
-	}
-
-	*klen = ret;
-	return buff;
-}
-
-static char *get_flog_buff(const char *file, unsigned int *klen)
-{
-	int fd, ret, len;
-	struct stat fi;
-	char *buff;
-
-	fd = open(file, O_RDONLY);
-	if (fd < 0)
-		return NULL;
-
-	ret = fstat(fd, &fi);
-	if (ret < 0 || !S_ISREG(fi.st_mode))
-		goto out;
-
-	len = fi.st_size + 1;
-	buff = malloc(len);
-	if (!buff)
-		goto out;
-
-	memset(buff, 0, len);
-	ret = read(fd, buff, len - 1);
-	if (ret <= 0)
-		goto out_free;
-
-	close(fd);
-	*klen = ret;
-	return buff;
-out_free:
-	free(buff);
-out:
-	close(fd);
-	return NULL;
-}
-
-static char *get_log_buff(const char *file, unsigned int *klen)
-{
-	return file ? get_flog_buff(file, klen) : get_klog_buff(klen);
-}
-
-static void put_log_buff(char *buff)
-{
-	free(buff);
-}
-
-static uint8_t *get_last_jit_image(char *haystack, size_t hlen,
-				   unsigned int *ilen)
-{
-	char *ptr, *pptr, *tmp;
-	off_t off = 0;
-	unsigned int proglen;
-	int ret, flen, pass, ulen = 0;
-	regmatch_t pmatch[1];
-	unsigned long base;
-	regex_t regex;
-	uint8_t *image;
-
-	if (hlen == 0)
-		return NULL;
-
-	ret = regcomp(&regex, "flen=[[:alnum:]]+ proglen=[[:digit:]]+ "
-		      "pass=[[:digit:]]+ image=[[:xdigit:]]+", REG_EXTENDED);
-	assert(ret == 0);
-
-	ptr = haystack;
-	memset(pmatch, 0, sizeof(pmatch));
-
-	while (1) {
-		ret = regexec(&regex, ptr, 1, pmatch, 0);
-		if (ret == 0) {
-			ptr += pmatch[0].rm_eo;
-			off += pmatch[0].rm_eo;
-			assert(off < hlen);
-		} else
-			break;
-	}
-
-	ptr = haystack + off - (pmatch[0].rm_eo - pmatch[0].rm_so);
-	ret = sscanf(ptr, "flen=%d proglen=%u pass=%d image=%lx",
-		     &flen, &proglen, &pass, &base);
-	if (ret != 4) {
-		regfree(&regex);
-		return NULL;
-	}
-	if (proglen > 1000000) {
-		printf("proglen of %d too big, stopping\n", proglen);
-		return NULL;
-	}
-
-	image = malloc(proglen);
-	if (!image) {
-		printf("Out of memory\n");
-		return NULL;
-	}
-	memset(image, 0, proglen);
-
-	tmp = ptr = haystack + off;
-	while ((ptr = strtok(tmp, "\n")) != NULL && ulen < proglen) {
-		tmp = NULL;
-		if (!strstr(ptr, "JIT code"))
-			continue;
-		pptr = ptr;
-		while ((ptr = strstr(pptr, ":")))
-			pptr = ptr + 1;
-		ptr = pptr;
-		do {
-			image[ulen++] = (uint8_t) strtoul(pptr, &pptr, 16);
-			if (ptr == pptr) {
-				ulen--;
-				break;
-			}
-			if (ulen >= proglen)
-				break;
-			ptr = pptr;
-		} while (1);
-	}
-
-	assert(ulen == proglen);
-	printf("%u bytes emitted from JIT compiler (pass:%d, flen:%d)\n",
-	       proglen, pass, flen);
-	printf("%lx + <x>:\n", base);
-
-	regfree(&regex);
-	*ilen = ulen;
-	return image;
-}
-
-static void usage(void)
-{
-	printf("Usage: bpf_jit_disasm [...]\n");
-	printf("       -o          Also display related opcodes (default: off).\n");
-	printf("       -O <file>   Write binary image of code to file, don't disassemble to stdout.\n");
-	printf("       -f <file>   Read last image dump from file or stdin (default: klog).\n");
-	printf("       -h          Display this help.\n");
-}
-
-int main(int argc, char **argv)
-{
-	unsigned int len, klen, opt, opcodes = 0;
-	char *kbuff, *file = NULL;
-	char *ofile = NULL;
-	int ofd;
-	ssize_t nr;
-	uint8_t *pos;
-	uint8_t *image = NULL;
-
-	while ((opt = getopt(argc, argv, "of:O:")) != -1) {
-		switch (opt) {
-		case 'o':
-			opcodes = 1;
-			break;
-		case 'O':
-			ofile = optarg;
-			break;
-		case 'f':
-			file = optarg;
-			break;
-		default:
-			usage();
-			return -1;
-		}
-	}
-
-	bfd_init();
-
-	kbuff = get_log_buff(file, &klen);
-	if (!kbuff) {
-		fprintf(stderr, "Could not retrieve log buffer!\n");
-		return -1;
-	}
-
-	image = get_last_jit_image(kbuff, klen, &len);
-	if (!image) {
-		fprintf(stderr, "No JIT image found!\n");
-		goto done;
-	}
-	if (!ofile) {
-		get_asm_insns(image, len, opcodes);
-		goto done;
-	}
-
-	ofd = open(ofile, O_WRONLY | O_CREAT | O_TRUNC, DEFFILEMODE);
-	if (ofd < 0) {
-		fprintf(stderr, "Could not open file %s for writing: ", ofile);
-		perror(NULL);
-		goto done;
-	}
-	pos = image;
-	do {
-		nr = write(ofd, pos, len);
-		if (nr < 0) {
-			fprintf(stderr, "Could not write data to %s: ", ofile);
-			perror(NULL);
-			goto done;
-		}
-		len -= nr;
-		pos += nr;
-	} while (len);
-	close(ofd);
-
-done:
-	put_log_buff(kbuff);
-	free(image);
-	return 0;
-}
-- 
2.27.0


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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-05  3:06 [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2 tong
@ 2023-01-05  9:36 ` Björn Töpel
  2023-01-05 17:53 ` Christophe Leroy
  1 sibling, 0 replies; 23+ messages in thread
From: Björn Töpel @ 2023-01-05  9:36 UTC (permalink / raw)
  To: tong, bpf, netdev, linux-arm-kernel, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux
  Cc: Tonghao Zhang, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Hou Tao

tong@infragraf.org writes:

> diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
> index 737baf8715da..ff168c50d46a 100644
> --- a/arch/riscv/net/bpf_jit_core.c
> +++ b/arch/riscv/net/bpf_jit_core.c
> @@ -151,9 +151,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>  	}
>  	bpf_jit_build_epilogue(ctx);
>  
> -	if (bpf_jit_enable > 1)
> -		bpf_jit_dump(prog->len, prog_size, pass, ctx->insns);
> -
>  	prog->bpf_func = (void *)ctx->insns;
>  	prog->jited = 1;
>  	prog->jited_len = prog_size;

Acked-by: Björn Töpel <bjorn@kernel.org> # RISC-V

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-05  3:06 [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2 tong
  2023-01-05  9:36 ` Björn Töpel
@ 2023-01-05 17:53 ` Christophe Leroy
  2023-01-06 13:22   ` Tonghao Zhang
  2023-01-06 15:37   ` Daniel Borkmann
  1 sibling, 2 replies; 23+ messages in thread
From: Christophe Leroy @ 2023-01-05 17:53 UTC (permalink / raw)
  To: tong, bpf, netdev, linux-arm-kernel, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux
  Cc: Hao Luo, Daniel Borkmann, John Fastabend, Alexei Starovoitov,
	Andrii Nakryiko, Song Liu, Stanislav Fomichev, Jiri Olsa,
	Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau



Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
> From: Tonghao Zhang <tong@infragraf.org>
> 
> The x86_64 can't dump the valid insn in this way. A test BPF prog
> which include subprog:
> 
> $ llvm-objdump -d subprog.o
> Disassembly of section .text:
> 0000000000000000 <subprog>:
>         0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1 = 29114459903653235 ll
>         2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>         3:       bf a1 00 00 00 00 00 00 r1 = r10
>         4:       07 01 00 00 f8 ff ff ff r1 += -8
>         5:       b7 02 00 00 08 00 00 00 r2 = 8
>         6:       85 00 00 00 06 00 00 00 call 6
>         7:       95 00 00 00 00 00 00 00 exit
> Disassembly of section raw_tp/sys_enter:
> 0000000000000000 <entry>:
>         0:       85 10 00 00 ff ff ff ff call -1
>         1:       b7 00 00 00 00 00 00 00 r0 = 0
>         2:       95 00 00 00 00 00 00 00 exit
> 
> kernel print message:
> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c from=kprobe-load pid=1643
> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
> [  580.782568] JIT code: 00000030: cc cc cc
> 
> $ bpf_jit_disasm
> 51 bytes emitted from JIT compiler (pass:3, flen:8)
> ffffffffa000c20c + <x>:
>     0:   int3
>     1:   int3
>     2:   int3
>     3:   int3
>     4:   int3
>     5:   int3
>     ...
> 
> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to header
> and then image/insn is valid. BTW, we can use the "bpftool prog dump" JITed instructions.

NACK.

Because the feature is buggy on x86_64, you remove it for all 
architectures ?

On powerpc bpf_jit_enable == 2 works and is very usefull.

Last time I tried to use bpftool on powerpc/32 it didn't work. I don't 
remember the details, I think it was an issue with endianess. Maybe it 
is fixed now, but it needs to be verified.

So please, before removing a working and usefull feature, make sure 
there is an alternative available to it for all architectures in all 
configurations.

Also, I don't think bpftool is usable to dump kernel BPF selftests. 
That's vital when a selftest fails if you want to have a chance to 
understand why it fails.

Thanks
Christophe

> 
> Signed-off-by: Tonghao Zhang <tong@infragraf.org>
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Martin KaFai Lau <martin.lau@linux.dev>
> Cc: Song Liu <song@kernel.org>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: KP Singh <kpsingh@kernel.org>
> Cc: Stanislav Fomichev <sdf@google.com>
> Cc: Hao Luo <haoluo@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Hou Tao <houtao1@huawei.com>
> ---
>   Documentation/admin-guide/sysctl/net.rst |   1 +
>   Documentation/networking/filter.rst      |  98 +------
>   arch/arm/net/bpf_jit_32.c                |   4 -
>   arch/arm64/net/bpf_jit_comp.c            |   4 -
>   arch/loongarch/net/bpf_jit.c             |   4 -
>   arch/mips/net/bpf_jit_comp.c             |   3 -
>   arch/powerpc/net/bpf_jit_comp.c          |  11 -
>   arch/riscv/net/bpf_jit_core.c            |   3 -
>   arch/s390/net/bpf_jit_comp.c             |   4 -
>   arch/sparc/net/bpf_jit_comp_32.c         |   3 -
>   arch/sparc/net/bpf_jit_comp_64.c         |  13 -
>   arch/x86/net/bpf_jit_comp.c              |   3 -
>   arch/x86/net/bpf_jit_comp32.c            |   3 -
>   net/core/sysctl_net_core.c               |  12 +-
>   tools/bpf/.gitignore                     |   1 -
>   tools/bpf/Makefile                       |  10 +-
>   tools/bpf/bpf_jit_disasm.c               | 332 -----------------------
>   17 files changed, 9 insertions(+), 500 deletions(-)
>   delete mode 100644 tools/bpf/bpf_jit_disasm.c
> 
> diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
> index 6394f5dc2303..82ca05ca6ed0 100644
> --- a/Documentation/admin-guide/sysctl/net.rst
> +++ b/Documentation/admin-guide/sysctl/net.rst
> @@ -88,6 +88,7 @@ Values:
>          - 0 - disable the JIT (default value)
>          - 1 - enable the JIT
>          - 2 - enable the JIT and ask the compiler to emit traces on kernel log.
> +              (deprecated since v6.3, use ``bpftool prog dump jited id <id>`` instead)
> 
>   bpf_jit_harden
>   --------------
> diff --git a/Documentation/networking/filter.rst b/Documentation/networking/filter.rst
> index f69da5074860..5f51c050e88f 100644
> --- a/Documentation/networking/filter.rst
> +++ b/Documentation/networking/filter.rst
> @@ -520,102 +520,8 @@ been previously enabled by root::
> 
>     echo 1 > /proc/sys/net/core/bpf_jit_enable
> 
> -For JIT developers, doing audits etc, each compile run can output the generated
> -opcode image into the kernel log via::
> -
> -  echo 2 > /proc/sys/net/core/bpf_jit_enable
> -
> -Example output from dmesg::
> -
> -    [ 3389.935842] flen=6 proglen=70 pass=3 image=ffffffffa0069c8f
> -    [ 3389.935847] JIT code: 00000000: 55 48 89 e5 48 83 ec 60 48 89 5d f8 44 8b 4f 68
> -    [ 3389.935849] JIT code: 00000010: 44 2b 4f 6c 4c 8b 87 d8 00 00 00 be 0c 00 00 00
> -    [ 3389.935850] JIT code: 00000020: e8 1d 94 ff e0 3d 00 08 00 00 75 16 be 17 00 00
> -    [ 3389.935851] JIT code: 00000030: 00 e8 28 94 ff e0 83 f8 01 75 07 b8 ff ff 00 00
> -    [ 3389.935852] JIT code: 00000040: eb 02 31 c0 c9 c3
> -
> -When CONFIG_BPF_JIT_ALWAYS_ON is enabled, bpf_jit_enable is permanently set to 1 and
> -setting any other value than that will return in failure. This is even the case for
> -setting bpf_jit_enable to 2, since dumping the final JIT image into the kernel log
> -is discouraged and introspection through bpftool (under tools/bpf/bpftool/) is the
> -generally recommended approach instead.
> -
> -In the kernel source tree under tools/bpf/, there's bpf_jit_disasm for
> -generating disassembly out of the kernel log's hexdump::
> -
> -       # ./bpf_jit_disasm
> -       70 bytes emitted from JIT compiler (pass:3, flen:6)
> -       ffffffffa0069c8f + <x>:
> -       0:      push   %rbp
> -       1:      mov    %rsp,%rbp
> -       4:      sub    $0x60,%rsp
> -       8:      mov    %rbx,-0x8(%rbp)
> -       c:      mov    0x68(%rdi),%r9d
> -       10:     sub    0x6c(%rdi),%r9d
> -       14:     mov    0xd8(%rdi),%r8
> -       1b:     mov    $0xc,%esi
> -       20:     callq  0xffffffffe0ff9442
> -       25:     cmp    $0x800,%eax
> -       2a:     jne    0x0000000000000042
> -       2c:     mov    $0x17,%esi
> -       31:     callq  0xffffffffe0ff945e
> -       36:     cmp    $0x1,%eax
> -       39:     jne    0x0000000000000042
> -       3b:     mov    $0xffff,%eax
> -       40:     jmp    0x0000000000000044
> -       42:     xor    %eax,%eax
> -       44:     leaveq
> -       45:     retq
> -
> -       Issuing option `-o` will "annotate" opcodes to resulting assembler
> -       instructions, which can be very useful for JIT developers:
> -
> -       # ./bpf_jit_disasm -o
> -       70 bytes emitted from JIT compiler (pass:3, flen:6)
> -       ffffffffa0069c8f + <x>:
> -       0:      push   %rbp
> -               55
> -       1:      mov    %rsp,%rbp
> -               48 89 e5
> -       4:      sub    $0x60,%rsp
> -               48 83 ec 60
> -       8:      mov    %rbx,-0x8(%rbp)
> -               48 89 5d f8
> -       c:      mov    0x68(%rdi),%r9d
> -               44 8b 4f 68
> -       10:     sub    0x6c(%rdi),%r9d
> -               44 2b 4f 6c
> -       14:     mov    0xd8(%rdi),%r8
> -               4c 8b 87 d8 00 00 00
> -       1b:     mov    $0xc,%esi
> -               be 0c 00 00 00
> -       20:     callq  0xffffffffe0ff9442
> -               e8 1d 94 ff e0
> -       25:     cmp    $0x800,%eax
> -               3d 00 08 00 00
> -       2a:     jne    0x0000000000000042
> -               75 16
> -       2c:     mov    $0x17,%esi
> -               be 17 00 00 00
> -       31:     callq  0xffffffffe0ff945e
> -               e8 28 94 ff e0
> -       36:     cmp    $0x1,%eax
> -               83 f8 01
> -       39:     jne    0x0000000000000042
> -               75 07
> -       3b:     mov    $0xffff,%eax
> -               b8 ff ff 00 00
> -       40:     jmp    0x0000000000000044
> -               eb 02
> -       42:     xor    %eax,%eax
> -               31 c0
> -       44:     leaveq
> -               c9
> -       45:     retq
> -               c3
> -
> -For BPF JIT developers, bpf_jit_disasm, bpf_asm and bpf_dbg provides a useful
> -toolchain for developing and testing the kernel's JIT compiler.
> +For JIT developers, doing audits etc, should use `bpftool prog dump` to
> +veiw the JIT generated opcode image.
> 
>   BPF kernel internals
>   --------------------
> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
> index 6a1c9fca5260..39301d59b537 100644
> --- a/arch/arm/net/bpf_jit_32.c
> +++ b/arch/arm/net/bpf_jit_32.c
> @@ -1999,10 +1999,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>          }
>          flush_icache_range((u32)header, (u32)(ctx.target + ctx.idx));
> 
> -       if (bpf_jit_enable > 1)
> -               /* there are 2 passes here */
> -               bpf_jit_dump(prog->len, image_size, 2, ctx.target);
> -
>          bpf_jit_binary_lock_ro(header);
>          prog->bpf_func = (void *)ctx.target;
>          prog->jited = 1;
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 62f805f427b7..59c35b4d77b7 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -1567,10 +1567,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>                  goto out_off;
>          }
> 
> -       /* And we're done. */
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(prog->len, prog_size, 2, ctx.image);
> -
>          bpf_flush_icache(header, ctx.image + ctx.idx);
> 
>          if (!prog->is_func || extra_pass) {
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index bdcd0c7719a9..ea37f52faa6e 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -1123,10 +1123,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>                  goto out_offset;
>          }
> 
> -       /* And we're done */
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(prog->len, image_size, 2, ctx.image);
> -
>          /* Update the icache */
>          flush_icache_range((unsigned long)header, (unsigned long)(ctx.image + ctx.idx));
> 
> diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c
> index b17130d510d4..ccbb7c231cb0 100644
> --- a/arch/mips/net/bpf_jit_comp.c
> +++ b/arch/mips/net/bpf_jit_comp.c
> @@ -1012,9 +1012,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>          flush_icache_range((unsigned long)header,
>                             (unsigned long)&ctx.target[ctx.jit_index]);
> 
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(prog->len, image_size, 2, ctx.target);
> -
>          prog->bpf_func = (void *)ctx.target;
>          prog->jited = 1;
>          prog->jited_len = image_size;
> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index 43e634126514..f0f7d8ff2022 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -262,20 +262,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>                          goto out_addrs;
>                  }
>                  bpf_jit_build_epilogue(code_base, &cgctx);
> -
> -               if (bpf_jit_enable > 1)
> -                       pr_info("Pass %d: shrink = %d, seen = 0x%x\n", pass,
> -                               proglen - (cgctx.idx * 4), cgctx.seen);
>          }
> 
>   skip_codegen_passes:
> -       if (bpf_jit_enable > 1)
> -               /*
> -                * Note that we output the base address of the code_base
> -                * rather than image, since opcodes are in code_base.
> -                */
> -               bpf_jit_dump(flen, proglen, pass, code_base);
> -
>   #ifdef CONFIG_PPC64_ELF_ABI_V1
>          /* Function descriptor nastiness: Address + TOC */
>          ((u64 *)image)[0] = (u64)code_base;
> diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
> index 737baf8715da..ff168c50d46a 100644
> --- a/arch/riscv/net/bpf_jit_core.c
> +++ b/arch/riscv/net/bpf_jit_core.c
> @@ -151,9 +151,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>          }
>          bpf_jit_build_epilogue(ctx);
> 
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(prog->len, prog_size, pass, ctx->insns);
> -
>          prog->bpf_func = (void *)ctx->insns;
>          prog->jited = 1;
>          prog->jited_len = prog_size;
> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index af35052d06ed..13d996e27602 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -1831,10 +1831,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>                  fp = orig_fp;
>                  goto free_addrs;
>          }
> -       if (bpf_jit_enable > 1) {
> -               bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
> -               print_fn_code(jit.prg_buf, jit.size_prg);
> -       }
>          if (!fp->is_func || extra_pass) {
>                  bpf_jit_binary_lock_ro(header);
>          } else {
> diff --git a/arch/sparc/net/bpf_jit_comp_32.c b/arch/sparc/net/bpf_jit_comp_32.c
> index a74e5004c6c8..08de0ed84831 100644
> --- a/arch/sparc/net/bpf_jit_comp_32.c
> +++ b/arch/sparc/net/bpf_jit_comp_32.c
> @@ -743,9 +743,6 @@ cond_branch:                        f_offset = addrs[i + filter[i].jf];
>                  oldproglen = proglen;
>          }
> 
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(flen, proglen, pass + 1, image);
> -
>          if (image) {
>                  fp->bpf_func = (void *)image;
>                  fp->jited = 1;
> diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
> index fa0759bfe498..14c9e5ce4100 100644
> --- a/arch/sparc/net/bpf_jit_comp_64.c
> +++ b/arch/sparc/net/bpf_jit_comp_64.c
> @@ -1549,16 +1549,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>                  }
>                  build_epilogue(&ctx);
> 
> -               if (bpf_jit_enable > 1)
> -                       pr_info("Pass %d: size = %u, seen = [%c%c%c%c%c%c]\n", pass,
> -                               ctx.idx * 4,
> -                               ctx.tmp_1_used ? '1' : ' ',
> -                               ctx.tmp_2_used ? '2' : ' ',
> -                               ctx.tmp_3_used ? '3' : ' ',
> -                               ctx.saw_frame_pointer ? 'F' : ' ',
> -                               ctx.saw_call ? 'C' : ' ',
> -                               ctx.saw_tail_call ? 'T' : ' ');
> -
>                  if (ctx.idx * 4 == prev_image_size)
>                          break;
>                  prev_image_size = ctx.idx * 4;
> @@ -1596,9 +1586,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>                  goto out_off;
>          }
> 
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(prog->len, image_size, pass, ctx.image);
> -
>          bpf_flush_icache(header, (u8 *)header + header->size);
> 
>          if (!prog->is_func || extra_pass) {
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index e3e2b57e4e13..197ff8651a56 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2551,9 +2551,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>                  cond_resched();
>          }
> 
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(prog->len, proglen, pass + 1, image);
> -
>          if (image) {
>                  if (!prog->is_func || extra_pass) {
>                          /*
> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index 429a89c5468b..ca53f20aca73 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c
> @@ -2597,9 +2597,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>                  cond_resched();
>          }
> 
> -       if (bpf_jit_enable > 1)
> -               bpf_jit_dump(prog->len, proglen, pass + 1, image);
> -
>          if (image) {
>                  bpf_jit_binary_lock_ro(header);
>                  prog->bpf_func = (void *)image;
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 5b1ce656baa1..562ace48e1c9 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -276,14 +276,10 @@ static int proc_dointvec_minmax_bpf_enable(struct ctl_table *table, int write,
>          tmp.data = &jit_enable;
>          ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
>          if (write && !ret) {
> -               if (jit_enable < 2 ||
> -                   (jit_enable == 2 && bpf_dump_raw_ok(current_cred()))) {
> -                       *(int *)table->data = jit_enable;
> -                       if (jit_enable == 2)
> -                               pr_warn("bpf_jit_enable = 2 was set! NEVER use this in production, only for JIT debugging!\n");
> -               } else {
> -                       ret = -EPERM;
> -               }
> +               *(int *)table->data = jit_enable;
> +
> +               if (jit_enable == 2)
> +                       pr_warn_once("bpf_jit_enable == 2 was deprecated! Use bpftool prog dump instead.\n");
>          }
> 
>          if (write && ret && min == max)
> diff --git a/tools/bpf/.gitignore b/tools/bpf/.gitignore
> index cf53342175e7..5c70cfb9092e 100644
> --- a/tools/bpf/.gitignore
> +++ b/tools/bpf/.gitignore
> @@ -4,4 +4,3 @@ feature
>   bpf_asm
>   bpf_dbg
>   bpf_exp.yacc.*
> -bpf_jit_disasm
> diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> index 243b79f2b451..9264d7b0edf6 100644
> --- a/tools/bpf/Makefile
> +++ b/tools/bpf/Makefile
> @@ -74,14 +74,10 @@ $(OUTPUT)%.yacc.o: $(OUTPUT)%.yacc.c
>   $(OUTPUT)%.lex.o: $(OUTPUT)%.lex.c
>          $(QUIET_CC)$(CC) $(CFLAGS) -c -o $@ $<
> 
> -PROGS = $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
> +PROGS = $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
> 
>   all: $(PROGS) bpftool runqslower
> 
> -$(OUTPUT)bpf_jit_disasm: CFLAGS += -DPACKAGE='bpf_jit_disasm'
> -$(OUTPUT)bpf_jit_disasm: $(OUTPUT)bpf_jit_disasm.o
> -       $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lopcodes -lbfd -ldl
> -
>   $(OUTPUT)bpf_dbg: $(OUTPUT)bpf_dbg.o
>          $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lreadline
> 
> @@ -94,16 +90,14 @@ $(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.lex.c
> 
>   clean: bpftool_clean runqslower_clean resolve_btfids_clean
>          $(call QUIET_CLEAN, bpf-progs)
> -       $(Q)$(RM) -r -- $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
> +       $(Q)$(RM) -r -- $(OUTPUT)*.o $(OUTPUT)bpf_dbg \
>                 $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
>          $(call QUIET_CLEAN, core-gen)
>          $(Q)$(RM) -- $(OUTPUT)FEATURE-DUMP.bpf
>          $(Q)$(RM) -r -- $(OUTPUT)feature
> 
>   install: $(PROGS) bpftool_install
> -       $(call QUIET_INSTALL, bpf_jit_disasm)
>          $(Q)$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/bin
> -       $(Q)$(INSTALL) $(OUTPUT)bpf_jit_disasm $(DESTDIR)$(prefix)/bin/bpf_jit_disasm
>          $(call QUIET_INSTALL, bpf_dbg)
>          $(Q)$(INSTALL) $(OUTPUT)bpf_dbg $(DESTDIR)$(prefix)/bin/bpf_dbg
>          $(call QUIET_INSTALL, bpf_asm)
> diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
> deleted file mode 100644
> index a90a5d110f92..000000000000
> --- a/tools/bpf/bpf_jit_disasm.c
> +++ /dev/null
> @@ -1,332 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Minimal BPF JIT image disassembler
> - *
> - * Disassembles BPF JIT compiler emitted opcodes back to asm insn's for
> - * debugging or verification purposes.
> - *
> - * To get the disassembly of the JIT code, do the following:
> - *
> - *  1) `echo 2 > /proc/sys/net/core/bpf_jit_enable`
> - *  2) Load a BPF filter (e.g. `tcpdump -p -n -s 0 -i eth1 host 192.168.20.0/24`)
> - *  3) Run e.g. `bpf_jit_disasm -o` to read out the last JIT code
> - *
> - * Copyright 2013 Daniel Borkmann <borkmann@redhat.com>
> - */
> -
> -#include <stdint.h>
> -#include <stdio.h>
> -#include <stdlib.h>
> -#include <assert.h>
> -#include <unistd.h>
> -#include <string.h>
> -#include <bfd.h>
> -#include <dis-asm.h>
> -#include <regex.h>
> -#include <fcntl.h>
> -#include <sys/klog.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <limits.h>
> -#include <tools/dis-asm-compat.h>
> -
> -#define CMD_ACTION_SIZE_BUFFER         10
> -#define CMD_ACTION_READ_ALL            3
> -
> -static void get_exec_path(char *tpath, size_t size)
> -{
> -       char *path;
> -       ssize_t len;
> -
> -       snprintf(tpath, size, "/proc/%d/exe", (int) getpid());
> -       tpath[size - 1] = 0;
> -
> -       path = strdup(tpath);
> -       assert(path);
> -
> -       len = readlink(path, tpath, size);
> -       tpath[len] = 0;
> -
> -       free(path);
> -}
> -
> -static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
> -{
> -       int count, i, pc = 0;
> -       char tpath[PATH_MAX];
> -       struct disassemble_info info;
> -       disassembler_ftype disassemble;
> -       bfd *bfdf;
> -
> -       memset(tpath, 0, sizeof(tpath));
> -       get_exec_path(tpath, sizeof(tpath));
> -
> -       bfdf = bfd_openr(tpath, NULL);
> -       assert(bfdf);
> -       assert(bfd_check_format(bfdf, bfd_object));
> -
> -       init_disassemble_info_compat(&info, stdout,
> -                                    (fprintf_ftype) fprintf,
> -                                    fprintf_styled);
> -       info.arch = bfd_get_arch(bfdf);
> -       info.mach = bfd_get_mach(bfdf);
> -       info.buffer = image;
> -       info.buffer_length = len;
> -
> -       disassemble_init_for_target(&info);
> -
> -#ifdef DISASM_FOUR_ARGS_SIGNATURE
> -       disassemble = disassembler(info.arch,
> -                                  bfd_big_endian(bfdf),
> -                                  info.mach,
> -                                  bfdf);
> -#else
> -       disassemble = disassembler(bfdf);
> -#endif
> -       assert(disassemble);
> -
> -       do {
> -               printf("%4x:\t", pc);
> -
> -               count = disassemble(pc, &info);
> -
> -               if (opcodes) {
> -                       printf("\n\t");
> -                       for (i = 0; i < count; ++i)
> -                               printf("%02x ", (uint8_t) image[pc + i]);
> -               }
> -               printf("\n");
> -
> -               pc += count;
> -       } while(count > 0 && pc < len);
> -
> -       bfd_close(bfdf);
> -}
> -
> -static char *get_klog_buff(unsigned int *klen)
> -{
> -       int ret, len;
> -       char *buff;
> -
> -       len = klogctl(CMD_ACTION_SIZE_BUFFER, NULL, 0);
> -       if (len < 0)
> -               return NULL;
> -
> -       buff = malloc(len);
> -       if (!buff)
> -               return NULL;
> -
> -       ret = klogctl(CMD_ACTION_READ_ALL, buff, len);
> -       if (ret < 0) {
> -               free(buff);
> -               return NULL;
> -       }
> -
> -       *klen = ret;
> -       return buff;
> -}
> -
> -static char *get_flog_buff(const char *file, unsigned int *klen)
> -{
> -       int fd, ret, len;
> -       struct stat fi;
> -       char *buff;
> -
> -       fd = open(file, O_RDONLY);
> -       if (fd < 0)
> -               return NULL;
> -
> -       ret = fstat(fd, &fi);
> -       if (ret < 0 || !S_ISREG(fi.st_mode))
> -               goto out;
> -
> -       len = fi.st_size + 1;
> -       buff = malloc(len);
> -       if (!buff)
> -               goto out;
> -
> -       memset(buff, 0, len);
> -       ret = read(fd, buff, len - 1);
> -       if (ret <= 0)
> -               goto out_free;
> -
> -       close(fd);
> -       *klen = ret;
> -       return buff;
> -out_free:
> -       free(buff);
> -out:
> -       close(fd);
> -       return NULL;
> -}
> -
> -static char *get_log_buff(const char *file, unsigned int *klen)
> -{
> -       return file ? get_flog_buff(file, klen) : get_klog_buff(klen);
> -}
> -
> -static void put_log_buff(char *buff)
> -{
> -       free(buff);
> -}
> -
> -static uint8_t *get_last_jit_image(char *haystack, size_t hlen,
> -                                  unsigned int *ilen)
> -{
> -       char *ptr, *pptr, *tmp;
> -       off_t off = 0;
> -       unsigned int proglen;
> -       int ret, flen, pass, ulen = 0;
> -       regmatch_t pmatch[1];
> -       unsigned long base;
> -       regex_t regex;
> -       uint8_t *image;
> -
> -       if (hlen == 0)
> -               return NULL;
> -
> -       ret = regcomp(&regex, "flen=[[:alnum:]]+ proglen=[[:digit:]]+ "
> -                     "pass=[[:digit:]]+ image=[[:xdigit:]]+", REG_EXTENDED);
> -       assert(ret == 0);
> -
> -       ptr = haystack;
> -       memset(pmatch, 0, sizeof(pmatch));
> -
> -       while (1) {
> -               ret = regexec(&regex, ptr, 1, pmatch, 0);
> -               if (ret == 0) {
> -                       ptr += pmatch[0].rm_eo;
> -                       off += pmatch[0].rm_eo;
> -                       assert(off < hlen);
> -               } else
> -                       break;
> -       }
> -
> -       ptr = haystack + off - (pmatch[0].rm_eo - pmatch[0].rm_so);
> -       ret = sscanf(ptr, "flen=%d proglen=%u pass=%d image=%lx",
> -                    &flen, &proglen, &pass, &base);
> -       if (ret != 4) {
> -               regfree(&regex);
> -               return NULL;
> -       }
> -       if (proglen > 1000000) {
> -               printf("proglen of %d too big, stopping\n", proglen);
> -               return NULL;
> -       }
> -
> -       image = malloc(proglen);
> -       if (!image) {
> -               printf("Out of memory\n");
> -               return NULL;
> -       }
> -       memset(image, 0, proglen);
> -
> -       tmp = ptr = haystack + off;
> -       while ((ptr = strtok(tmp, "\n")) != NULL && ulen < proglen) {
> -               tmp = NULL;
> -               if (!strstr(ptr, "JIT code"))
> -                       continue;
> -               pptr = ptr;
> -               while ((ptr = strstr(pptr, ":")))
> -                       pptr = ptr + 1;
> -               ptr = pptr;
> -               do {
> -                       image[ulen++] = (uint8_t) strtoul(pptr, &pptr, 16);
> -                       if (ptr == pptr) {
> -                               ulen--;
> -                               break;
> -                       }
> -                       if (ulen >= proglen)
> -                               break;
> -                       ptr = pptr;
> -               } while (1);
> -       }
> -
> -       assert(ulen == proglen);
> -       printf("%u bytes emitted from JIT compiler (pass:%d, flen:%d)\n",
> -              proglen, pass, flen);
> -       printf("%lx + <x>:\n", base);
> -
> -       regfree(&regex);
> -       *ilen = ulen;
> -       return image;
> -}
> -
> -static void usage(void)
> -{
> -       printf("Usage: bpf_jit_disasm [...]\n");
> -       printf("       -o          Also display related opcodes (default: off).\n");
> -       printf("       -O <file>   Write binary image of code to file, don't disassemble to stdout.\n");
> -       printf("       -f <file>   Read last image dump from file or stdin (default: klog).\n");
> -       printf("       -h          Display this help.\n");
> -}
> -
> -int main(int argc, char **argv)
> -{
> -       unsigned int len, klen, opt, opcodes = 0;
> -       char *kbuff, *file = NULL;
> -       char *ofile = NULL;
> -       int ofd;
> -       ssize_t nr;
> -       uint8_t *pos;
> -       uint8_t *image = NULL;
> -
> -       while ((opt = getopt(argc, argv, "of:O:")) != -1) {
> -               switch (opt) {
> -               case 'o':
> -                       opcodes = 1;
> -                       break;
> -               case 'O':
> -                       ofile = optarg;
> -                       break;
> -               case 'f':
> -                       file = optarg;
> -                       break;
> -               default:
> -                       usage();
> -                       return -1;
> -               }
> -       }
> -
> -       bfd_init();
> -
> -       kbuff = get_log_buff(file, &klen);
> -       if (!kbuff) {
> -               fprintf(stderr, "Could not retrieve log buffer!\n");
> -               return -1;
> -       }
> -
> -       image = get_last_jit_image(kbuff, klen, &len);
> -       if (!image) {
> -               fprintf(stderr, "No JIT image found!\n");
> -               goto done;
> -       }
> -       if (!ofile) {
> -               get_asm_insns(image, len, opcodes);
> -               goto done;
> -       }
> -
> -       ofd = open(ofile, O_WRONLY | O_CREAT | O_TRUNC, DEFFILEMODE);
> -       if (ofd < 0) {
> -               fprintf(stderr, "Could not open file %s for writing: ", ofile);
> -               perror(NULL);
> -               goto done;
> -       }
> -       pos = image;
> -       do {
> -               nr = write(ofd, pos, len);
> -               if (nr < 0) {
> -                       fprintf(stderr, "Could not write data to %s: ", ofile);
> -                       perror(NULL);
> -                       goto done;
> -               }
> -               len -= nr;
> -               pos += nr;
> -       } while (len);
> -       close(ofd);
> -
> -done:
> -       put_log_buff(kbuff);
> -       free(image);
> -       return 0;
> -}
> --
> 2.27.0
> 

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-05 17:53 ` Christophe Leroy
@ 2023-01-06 13:22   ` Tonghao Zhang
  2023-01-06 15:37   ` Daniel Borkmann
  1 sibling, 0 replies; 23+ messages in thread
From: Tonghao Zhang @ 2023-01-06 13:22 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: bpf, netdev, linux-arm-kernel, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, Hao Luo,
	Daniel Borkmann, John Fastabend, Alexei Starovoitov,
	Andrii Nakryiko, Song Liu, Stanislav Fomichev, Jiri Olsa,
	Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau



> On Jan 6, 2023, at 1:53 AM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>> From: Tonghao Zhang <tong@infragraf.org>
>> 
>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>> which include subprog:
>> 
>> $ llvm-objdump -d subprog.o
>> Disassembly of section .text:
>> 0000000000000000 <subprog>:
>>        0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1 = 29114459903653235 ll
>>        2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>        3:       bf a1 00 00 00 00 00 00 r1 = r10
>>        4:       07 01 00 00 f8 ff ff ff r1 += -8
>>        5:       b7 02 00 00 08 00 00 00 r2 = 8
>>        6:       85 00 00 00 06 00 00 00 call 6
>>        7:       95 00 00 00 00 00 00 00 exit
>> Disassembly of section raw_tp/sys_enter:
>> 0000000000000000 <entry>:
>>        0:       85 10 00 00 ff ff ff ff call -1
>>        1:       b7 00 00 00 00 00 00 00 r0 = 0
>>        2:       95 00 00 00 00 00 00 00 exit
>> 
>> kernel print message:
>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c from=kprobe-load pid=1643
>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
>> [  580.782568] JIT code: 00000030: cc cc cc
>> 
>> $ bpf_jit_disasm
>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>> ffffffffa000c20c + <x>:
>>    0:   int3
>>    1:   int3
>>    2:   int3
>>    3:   int3
>>    4:   int3
>>    5:   int3
>>    ...
>> 
>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to header
>> and then image/insn is valid. BTW, we can use the "bpftool prog dump" JITed instructions.
> 
> NACK.
> 
> Because the feature is buggy on x86_64, you remove it for all 
> architectures ?
> 
> On powerpc bpf_jit_enable == 2 works and is very usefull.
> 
> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't 
> remember the details, I think it was an issue with endianess. Maybe it 
> is fixed now, but it needs to be verified.
> 
[so sorry, email is always rejected]
Hi
I think bpftool may dump the jited insn too.

> So please, before removing a working and usefull feature, make sure 
> there is an alternative available to it for all architectures in all 
> configurations.
> 
> Also, I don't think bpftool is usable to dump kernel BPF selftests. 
> That's vital when a selftest fails if you want to have a chance to 
> understand why it fails.
Why bpftool does’t work for you ? I think this is a core function for bpf. If you can dump the jited insn using bpf_jit_enable == 2, you should
dump the inns tool, while in selftest case.

Alexei any thoughts?

> Thanks
> Christophe
> 
>> 
>> Signed-off-by: Tonghao Zhang <tong@infragraf.org>
>> Suggested-by: Alexei Starovoitov <ast@kernel.org>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Andrii Nakryiko <andrii@kernel.org>
>> Cc: Martin KaFai Lau <martin.lau@linux.dev>
>> Cc: Song Liu <song@kernel.org>
>> Cc: Yonghong Song <yhs@fb.com>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Cc: KP Singh <kpsingh@kernel.org>
>> Cc: Stanislav Fomichev <sdf@google.com>
>> Cc: Hao Luo <haoluo@google.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Hou Tao <houtao1@huawei.com>
>> ---
>>  Documentation/admin-guide/sysctl/net.rst |   1 +
>>  Documentation/networking/filter.rst      |  98 +------
>>  arch/arm/net/bpf_jit_32.c                |   4 -
>>  arch/arm64/net/bpf_jit_comp.c            |   4 -
>>  arch/loongarch/net/bpf_jit.c             |   4 -
>>  arch/mips/net/bpf_jit_comp.c             |   3 -
>>  arch/powerpc/net/bpf_jit_comp.c          |  11 -
>>  arch/riscv/net/bpf_jit_core.c            |   3 -
>>  arch/s390/net/bpf_jit_comp.c             |   4 -
>>  arch/sparc/net/bpf_jit_comp_32.c         |   3 -
>>  arch/sparc/net/bpf_jit_comp_64.c         |  13 -
>>  arch/x86/net/bpf_jit_comp.c              |   3 -
>>  arch/x86/net/bpf_jit_comp32.c            |   3 -
>>  net/core/sysctl_net_core.c               |  12 +-
>>  tools/bpf/.gitignore                     |   1 -
>>  tools/bpf/Makefile                       |  10 +-
>>  tools/bpf/bpf_jit_disasm.c               | 332 -----------------------
>>  17 files changed, 9 insertions(+), 500 deletions(-)
>>  delete mode 100644 tools/bpf/bpf_jit_disasm.c
>> 
>> diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
>> index 6394f5dc2303..82ca05ca6ed0 100644
>> --- a/Documentation/admin-guide/sysctl/net.rst
>> +++ b/Documentation/admin-guide/sysctl/net.rst
>> @@ -88,6 +88,7 @@ Values:
>>         - 0 - disable the JIT (default value)
>>         - 1 - enable the JIT
>>         - 2 - enable the JIT and ask the compiler to emit traces on kernel log.
>> +              (deprecated since v6.3, use ``bpftool prog dump jited id <id>`` instead)
>> 
>>  bpf_jit_harden
>>  --------------
>> diff --git a/Documentation/networking/filter.rst b/Documentation/networking/filter.rst
>> index f69da5074860..5f51c050e88f 100644
>> --- a/Documentation/networking/filter.rst
>> +++ b/Documentation/networking/filter.rst
>> @@ -520,102 +520,8 @@ been previously enabled by root::
>> 
>>    echo 1 > /proc/sys/net/core/bpf_jit_enable
>> 
>> -For JIT developers, doing audits etc, each compile run can output the generated
>> -opcode image into the kernel log via::
>> -
>> -  echo 2 > /proc/sys/net/core/bpf_jit_enable
>> -
>> -Example output from dmesg::
>> -
>> -    [ 3389.935842] flen=6 proglen=70 pass=3 image=ffffffffa0069c8f
>> -    [ 3389.935847] JIT code: 00000000: 55 48 89 e5 48 83 ec 60 48 89 5d f8 44 8b 4f 68
>> -    [ 3389.935849] JIT code: 00000010: 44 2b 4f 6c 4c 8b 87 d8 00 00 00 be 0c 00 00 00
>> -    [ 3389.935850] JIT code: 00000020: e8 1d 94 ff e0 3d 00 08 00 00 75 16 be 17 00 00
>> -    [ 3389.935851] JIT code: 00000030: 00 e8 28 94 ff e0 83 f8 01 75 07 b8 ff ff 00 00
>> -    [ 3389.935852] JIT code: 00000040: eb 02 31 c0 c9 c3
>> -
>> -When CONFIG_BPF_JIT_ALWAYS_ON is enabled, bpf_jit_enable is permanently set to 1 and
>> -setting any other value than that will return in failure. This is even the case for
>> -setting bpf_jit_enable to 2, since dumping the final JIT image into the kernel log
>> -is discouraged and introspection through bpftool (under tools/bpf/bpftool/) is the
>> -generally recommended approach instead.
>> -
>> -In the kernel source tree under tools/bpf/, there's bpf_jit_disasm for
>> -generating disassembly out of the kernel log's hexdump::
>> -
>> -       # ./bpf_jit_disasm
>> -       70 bytes emitted from JIT compiler (pass:3, flen:6)
>> -       ffffffffa0069c8f + <x>:
>> -       0:      push   %rbp
>> -       1:      mov    %rsp,%rbp
>> -       4:      sub    $0x60,%rsp
>> -       8:      mov    %rbx,-0x8(%rbp)
>> -       c:      mov    0x68(%rdi),%r9d
>> -       10:     sub    0x6c(%rdi),%r9d
>> -       14:     mov    0xd8(%rdi),%r8
>> -       1b:     mov    $0xc,%esi
>> -       20:     callq  0xffffffffe0ff9442
>> -       25:     cmp    $0x800,%eax
>> -       2a:     jne    0x0000000000000042
>> -       2c:     mov    $0x17,%esi
>> -       31:     callq  0xffffffffe0ff945e
>> -       36:     cmp    $0x1,%eax
>> -       39:     jne    0x0000000000000042
>> -       3b:     mov    $0xffff,%eax
>> -       40:     jmp    0x0000000000000044
>> -       42:     xor    %eax,%eax
>> -       44:     leaveq
>> -       45:     retq
>> -
>> -       Issuing option `-o` will "annotate" opcodes to resulting assembler
>> -       instructions, which can be very useful for JIT developers:
>> -
>> -       # ./bpf_jit_disasm -o
>> -       70 bytes emitted from JIT compiler (pass:3, flen:6)
>> -       ffffffffa0069c8f + <x>:
>> -       0:      push   %rbp
>> -               55
>> -       1:      mov    %rsp,%rbp
>> -               48 89 e5
>> -       4:      sub    $0x60,%rsp
>> -               48 83 ec 60
>> -       8:      mov    %rbx,-0x8(%rbp)
>> -               48 89 5d f8
>> -       c:      mov    0x68(%rdi),%r9d
>> -               44 8b 4f 68
>> -       10:     sub    0x6c(%rdi),%r9d
>> -               44 2b 4f 6c
>> -       14:     mov    0xd8(%rdi),%r8
>> -               4c 8b 87 d8 00 00 00
>> -       1b:     mov    $0xc,%esi
>> -               be 0c 00 00 00
>> -       20:     callq  0xffffffffe0ff9442
>> -               e8 1d 94 ff e0
>> -       25:     cmp    $0x800,%eax
>> -               3d 00 08 00 00
>> -       2a:     jne    0x0000000000000042
>> -               75 16
>> -       2c:     mov    $0x17,%esi
>> -               be 17 00 00 00
>> -       31:     callq  0xffffffffe0ff945e
>> -               e8 28 94 ff e0
>> -       36:     cmp    $0x1,%eax
>> -               83 f8 01
>> -       39:     jne    0x0000000000000042
>> -               75 07
>> -       3b:     mov    $0xffff,%eax
>> -               b8 ff ff 00 00
>> -       40:     jmp    0x0000000000000044
>> -               eb 02
>> -       42:     xor    %eax,%eax
>> -               31 c0
>> -       44:     leaveq
>> -               c9
>> -       45:     retq
>> -               c3
>> -
>> -For BPF JIT developers, bpf_jit_disasm, bpf_asm and bpf_dbg provides a useful
>> -toolchain for developing and testing the kernel's JIT compiler.
>> +For JIT developers, doing audits etc, should use `bpftool prog dump` to
>> +veiw the JIT generated opcode image.
>> 
>>  BPF kernel internals
>>  --------------------
>> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
>> index 6a1c9fca5260..39301d59b537 100644
>> --- a/arch/arm/net/bpf_jit_32.c
>> +++ b/arch/arm/net/bpf_jit_32.c
>> @@ -1999,10 +1999,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>         }
>>         flush_icache_range((u32)header, (u32)(ctx.target + ctx.idx));
>> 
>> -       if (bpf_jit_enable > 1)
>> -               /* there are 2 passes here */
>> -               bpf_jit_dump(prog->len, image_size, 2, ctx.target);
>> -
>>         bpf_jit_binary_lock_ro(header);
>>         prog->bpf_func = (void *)ctx.target;
>>         prog->jited = 1;
>> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
>> index 62f805f427b7..59c35b4d77b7 100644
>> --- a/arch/arm64/net/bpf_jit_comp.c
>> +++ b/arch/arm64/net/bpf_jit_comp.c
>> @@ -1567,10 +1567,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>                 goto out_off;
>>         }
>> 
>> -       /* And we're done. */
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(prog->len, prog_size, 2, ctx.image);
>> -
>>         bpf_flush_icache(header, ctx.image + ctx.idx);
>> 
>>         if (!prog->is_func || extra_pass) {
>> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
>> index bdcd0c7719a9..ea37f52faa6e 100644
>> --- a/arch/loongarch/net/bpf_jit.c
>> +++ b/arch/loongarch/net/bpf_jit.c
>> @@ -1123,10 +1123,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>                 goto out_offset;
>>         }
>> 
>> -       /* And we're done */
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(prog->len, image_size, 2, ctx.image);
>> -
>>         /* Update the icache */
>>         flush_icache_range((unsigned long)header, (unsigned long)(ctx.image + ctx.idx));
>> 
>> diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c
>> index b17130d510d4..ccbb7c231cb0 100644
>> --- a/arch/mips/net/bpf_jit_comp.c
>> +++ b/arch/mips/net/bpf_jit_comp.c
>> @@ -1012,9 +1012,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>         flush_icache_range((unsigned long)header,
>>                            (unsigned long)&ctx.target[ctx.jit_index]);
>> 
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(prog->len, image_size, 2, ctx.target);
>> -
>>         prog->bpf_func = (void *)ctx.target;
>>         prog->jited = 1;
>>         prog->jited_len = image_size;
>> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
>> index 43e634126514..f0f7d8ff2022 100644
>> --- a/arch/powerpc/net/bpf_jit_comp.c
>> +++ b/arch/powerpc/net/bpf_jit_comp.c
>> @@ -262,20 +262,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>>                         goto out_addrs;
>>                 }
>>                 bpf_jit_build_epilogue(code_base, &cgctx);
>> -
>> -               if (bpf_jit_enable > 1)
>> -                       pr_info("Pass %d: shrink = %d, seen = 0x%x\n", pass,
>> -                               proglen - (cgctx.idx * 4), cgctx.seen);
>>         }
>> 
>>  skip_codegen_passes:
>> -       if (bpf_jit_enable > 1)
>> -               /*
>> -                * Note that we output the base address of the code_base
>> -                * rather than image, since opcodes are in code_base.
>> -                */
>> -               bpf_jit_dump(flen, proglen, pass, code_base);
>> -
>>  #ifdef CONFIG_PPC64_ELF_ABI_V1
>>         /* Function descriptor nastiness: Address + TOC */
>>         ((u64 *)image)[0] = (u64)code_base;
>> diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
>> index 737baf8715da..ff168c50d46a 100644
>> --- a/arch/riscv/net/bpf_jit_core.c
>> +++ b/arch/riscv/net/bpf_jit_core.c
>> @@ -151,9 +151,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>         }
>>         bpf_jit_build_epilogue(ctx);
>> 
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(prog->len, prog_size, pass, ctx->insns);
>> -
>>         prog->bpf_func = (void *)ctx->insns;
>>         prog->jited = 1;
>>         prog->jited_len = prog_size;
>> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
>> index af35052d06ed..13d996e27602 100644
>> --- a/arch/s390/net/bpf_jit_comp.c
>> +++ b/arch/s390/net/bpf_jit_comp.c
>> @@ -1831,10 +1831,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>>                 fp = orig_fp;
>>                 goto free_addrs;
>>         }
>> -       if (bpf_jit_enable > 1) {
>> -               bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
>> -               print_fn_code(jit.prg_buf, jit.size_prg);
>> -       }
>>         if (!fp->is_func || extra_pass) {
>>                 bpf_jit_binary_lock_ro(header);
>>         } else {
>> diff --git a/arch/sparc/net/bpf_jit_comp_32.c b/arch/sparc/net/bpf_jit_comp_32.c
>> index a74e5004c6c8..08de0ed84831 100644
>> --- a/arch/sparc/net/bpf_jit_comp_32.c
>> +++ b/arch/sparc/net/bpf_jit_comp_32.c
>> @@ -743,9 +743,6 @@ cond_branch:                        f_offset = addrs[i + filter[i].jf];
>>                 oldproglen = proglen;
>>         }
>> 
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(flen, proglen, pass + 1, image);
>> -
>>         if (image) {
>>                 fp->bpf_func = (void *)image;
>>                 fp->jited = 1;
>> diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
>> index fa0759bfe498..14c9e5ce4100 100644
>> --- a/arch/sparc/net/bpf_jit_comp_64.c
>> +++ b/arch/sparc/net/bpf_jit_comp_64.c
>> @@ -1549,16 +1549,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>                 }
>>                 build_epilogue(&ctx);
>> 
>> -               if (bpf_jit_enable > 1)
>> -                       pr_info("Pass %d: size = %u, seen = [%c%c%c%c%c%c]\n", pass,
>> -                               ctx.idx * 4,
>> -                               ctx.tmp_1_used ? '1' : ' ',
>> -                               ctx.tmp_2_used ? '2' : ' ',
>> -                               ctx.tmp_3_used ? '3' : ' ',
>> -                               ctx.saw_frame_pointer ? 'F' : ' ',
>> -                               ctx.saw_call ? 'C' : ' ',
>> -                               ctx.saw_tail_call ? 'T' : ' ');
>> -
>>                 if (ctx.idx * 4 == prev_image_size)
>>                         break;
>>                 prev_image_size = ctx.idx * 4;
>> @@ -1596,9 +1586,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>                 goto out_off;
>>         }
>> 
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(prog->len, image_size, pass, ctx.image);
>> -
>>         bpf_flush_icache(header, (u8 *)header + header->size);
>> 
>>         if (!prog->is_func || extra_pass) {
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index e3e2b57e4e13..197ff8651a56 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -2551,9 +2551,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>                 cond_resched();
>>         }
>> 
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(prog->len, proglen, pass + 1, image);
>> -
>>         if (image) {
>>                 if (!prog->is_func || extra_pass) {
>>                         /*
>> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
>> index 429a89c5468b..ca53f20aca73 100644
>> --- a/arch/x86/net/bpf_jit_comp32.c
>> +++ b/arch/x86/net/bpf_jit_comp32.c
>> @@ -2597,9 +2597,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>                 cond_resched();
>>         }
>> 
>> -       if (bpf_jit_enable > 1)
>> -               bpf_jit_dump(prog->len, proglen, pass + 1, image);
>> -
>>         if (image) {
>>                 bpf_jit_binary_lock_ro(header);
>>                 prog->bpf_func = (void *)image;
>> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
>> index 5b1ce656baa1..562ace48e1c9 100644
>> --- a/net/core/sysctl_net_core.c
>> +++ b/net/core/sysctl_net_core.c
>> @@ -276,14 +276,10 @@ static int proc_dointvec_minmax_bpf_enable(struct ctl_table *table, int write,
>>         tmp.data = &jit_enable;
>>         ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
>>         if (write && !ret) {
>> -               if (jit_enable < 2 ||
>> -                   (jit_enable == 2 && bpf_dump_raw_ok(current_cred()))) {
>> -                       *(int *)table->data = jit_enable;
>> -                       if (jit_enable == 2)
>> -                               pr_warn("bpf_jit_enable = 2 was set! NEVER use this in production, only for JIT debugging!\n");
>> -               } else {
>> -                       ret = -EPERM;
>> -               }
>> +               *(int *)table->data = jit_enable;
>> +
>> +               if (jit_enable == 2)
>> +                       pr_warn_once("bpf_jit_enable == 2 was deprecated! Use bpftool prog dump instead.\n");
>>         }
>> 
>>         if (write && ret && min == max)
>> diff --git a/tools/bpf/.gitignore b/tools/bpf/.gitignore
>> index cf53342175e7..5c70cfb9092e 100644
>> --- a/tools/bpf/.gitignore
>> +++ b/tools/bpf/.gitignore
>> @@ -4,4 +4,3 @@ feature
>>  bpf_asm
>>  bpf_dbg
>>  bpf_exp.yacc.*
>> -bpf_jit_disasm
>> diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
>> index 243b79f2b451..9264d7b0edf6 100644
>> --- a/tools/bpf/Makefile
>> +++ b/tools/bpf/Makefile
>> @@ -74,14 +74,10 @@ $(OUTPUT)%.yacc.o: $(OUTPUT)%.yacc.c
>>  $(OUTPUT)%.lex.o: $(OUTPUT)%.lex.c
>>         $(QUIET_CC)$(CC) $(CFLAGS) -c -o $@ $<
>> 
>> -PROGS = $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
>> +PROGS = $(OUTPUT)bpf_dbg $(OUTPUT)bpf_asm
>> 
>>  all: $(PROGS) bpftool runqslower
>> 
>> -$(OUTPUT)bpf_jit_disasm: CFLAGS += -DPACKAGE='bpf_jit_disasm'
>> -$(OUTPUT)bpf_jit_disasm: $(OUTPUT)bpf_jit_disasm.o
>> -       $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lopcodes -lbfd -ldl
>> -
>>  $(OUTPUT)bpf_dbg: $(OUTPUT)bpf_dbg.o
>>         $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ -lreadline
>> 
>> @@ -94,16 +90,14 @@ $(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.lex.c
>> 
>>  clean: bpftool_clean runqslower_clean resolve_btfids_clean
>>         $(call QUIET_CLEAN, bpf-progs)
>> -       $(Q)$(RM) -r -- $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
>> +       $(Q)$(RM) -r -- $(OUTPUT)*.o $(OUTPUT)bpf_dbg \
>>                $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
>>         $(call QUIET_CLEAN, core-gen)
>>         $(Q)$(RM) -- $(OUTPUT)FEATURE-DUMP.bpf
>>         $(Q)$(RM) -r -- $(OUTPUT)feature
>> 
>>  install: $(PROGS) bpftool_install
>> -       $(call QUIET_INSTALL, bpf_jit_disasm)
>>         $(Q)$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/bin
>> -       $(Q)$(INSTALL) $(OUTPUT)bpf_jit_disasm $(DESTDIR)$(prefix)/bin/bpf_jit_disasm
>>         $(call QUIET_INSTALL, bpf_dbg)
>>         $(Q)$(INSTALL) $(OUTPUT)bpf_dbg $(DESTDIR)$(prefix)/bin/bpf_dbg
>>         $(call QUIET_INSTALL, bpf_asm)
>> diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
>> deleted file mode 100644
>> index a90a5d110f92..000000000000
>> --- a/tools/bpf/bpf_jit_disasm.c
>> +++ /dev/null
>> @@ -1,332 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0-only
>> -/*
>> - * Minimal BPF JIT image disassembler
>> - *
>> - * Disassembles BPF JIT compiler emitted opcodes back to asm insn's for
>> - * debugging or verification purposes.
>> - *
>> - * To get the disassembly of the JIT code, do the following:
>> - *
>> - *  1) `echo 2 > /proc/sys/net/core/bpf_jit_enable`
>> - *  2) Load a BPF filter (e.g. `tcpdump -p -n -s 0 -i eth1 host 192.168.20.0/24`)
>> - *  3) Run e.g. `bpf_jit_disasm -o` to read out the last JIT code
>> - *
>> - * Copyright 2013 Daniel Borkmann <borkmann@redhat.com>
>> - */
>> -
>> -#include <stdint.h>
>> -#include <stdio.h>
>> -#include <stdlib.h>
>> -#include <assert.h>
>> -#include <unistd.h>
>> -#include <string.h>
>> -#include <bfd.h>
>> -#include <dis-asm.h>
>> -#include <regex.h>
>> -#include <fcntl.h>
>> -#include <sys/klog.h>
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <limits.h>
>> -#include <tools/dis-asm-compat.h>
>> -
>> -#define CMD_ACTION_SIZE_BUFFER         10
>> -#define CMD_ACTION_READ_ALL            3
>> -
>> -static void get_exec_path(char *tpath, size_t size)
>> -{
>> -       char *path;
>> -       ssize_t len;
>> -
>> -       snprintf(tpath, size, "/proc/%d/exe", (int) getpid());
>> -       tpath[size - 1] = 0;
>> -
>> -       path = strdup(tpath);
>> -       assert(path);
>> -
>> -       len = readlink(path, tpath, size);
>> -       tpath[len] = 0;
>> -
>> -       free(path);
>> -}
>> -
>> -static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
>> -{
>> -       int count, i, pc = 0;
>> -       char tpath[PATH_MAX];
>> -       struct disassemble_info info;
>> -       disassembler_ftype disassemble;
>> -       bfd *bfdf;
>> -
>> -       memset(tpath, 0, sizeof(tpath));
>> -       get_exec_path(tpath, sizeof(tpath));
>> -
>> -       bfdf = bfd_openr(tpath, NULL);
>> -       assert(bfdf);
>> -       assert(bfd_check_format(bfdf, bfd_object));
>> -
>> -       init_disassemble_info_compat(&info, stdout,
>> -                                    (fprintf_ftype) fprintf,
>> -                                    fprintf_styled);
>> -       info.arch = bfd_get_arch(bfdf);
>> -       info.mach = bfd_get_mach(bfdf);
>> -       info.buffer = image;
>> -       info.buffer_length = len;
>> -
>> -       disassemble_init_for_target(&info);
>> -
>> -#ifdef DISASM_FOUR_ARGS_SIGNATURE
>> -       disassemble = disassembler(info.arch,
>> -                                  bfd_big_endian(bfdf),
>> -                                  info.mach,
>> -                                  bfdf);
>> -#else
>> -       disassemble = disassembler(bfdf);
>> -#endif
>> -       assert(disassemble);
>> -
>> -       do {
>> -               printf("%4x:\t", pc);
>> -
>> -               count = disassemble(pc, &info);
>> -
>> -               if (opcodes) {
>> -                       printf("\n\t");
>> -                       for (i = 0; i < count; ++i)
>> -                               printf("%02x ", (uint8_t) image[pc + i]);
>> -               }
>> -               printf("\n");
>> -
>> -               pc += count;
>> -       } while(count > 0 && pc < len);
>> -
>> -       bfd_close(bfdf);
>> -}
>> -
>> -static char *get_klog_buff(unsigned int *klen)
>> -{
>> -       int ret, len;
>> -       char *buff;
>> -
>> -       len = klogctl(CMD_ACTION_SIZE_BUFFER, NULL, 0);
>> -       if (len < 0)
>> -               return NULL;
>> -
>> -       buff = malloc(len);
>> -       if (!buff)
>> -               return NULL;
>> -
>> -       ret = klogctl(CMD_ACTION_READ_ALL, buff, len);
>> -       if (ret < 0) {
>> -               free(buff);
>> -               return NULL;
>> -       }
>> -
>> -       *klen = ret;
>> -       return buff;
>> -}
>> -
>> -static char *get_flog_buff(const char *file, unsigned int *klen)
>> -{
>> -       int fd, ret, len;
>> -       struct stat fi;
>> -       char *buff;
>> -
>> -       fd = open(file, O_RDONLY);
>> -       if (fd < 0)
>> -               return NULL;
>> -
>> -       ret = fstat(fd, &fi);
>> -       if (ret < 0 || !S_ISREG(fi.st_mode))
>> -               goto out;
>> -
>> -       len = fi.st_size + 1;
>> -       buff = malloc(len);
>> -       if (!buff)
>> -               goto out;
>> -
>> -       memset(buff, 0, len);
>> -       ret = read(fd, buff, len - 1);
>> -       if (ret <= 0)
>> -               goto out_free;
>> -
>> -       close(fd);
>> -       *klen = ret;
>> -       return buff;
>> -out_free:
>> -       free(buff);
>> -out:
>> -       close(fd);
>> -       return NULL;
>> -}
>> -
>> -static char *get_log_buff(const char *file, unsigned int *klen)
>> -{
>> -       return file ? get_flog_buff(file, klen) : get_klog_buff(klen);
>> -}
>> -
>> -static void put_log_buff(char *buff)
>> -{
>> -       free(buff);
>> -}
>> -
>> -static uint8_t *get_last_jit_image(char *haystack, size_t hlen,
>> -                                  unsigned int *ilen)
>> -{
>> -       char *ptr, *pptr, *tmp;
>> -       off_t off = 0;
>> -       unsigned int proglen;
>> -       int ret, flen, pass, ulen = 0;
>> -       regmatch_t pmatch[1];
>> -       unsigned long base;
>> -       regex_t regex;
>> -       uint8_t *image;
>> -
>> -       if (hlen == 0)
>> -               return NULL;
>> -
>> -       ret = regcomp(&regex, "flen=[[:alnum:]]+ proglen=[[:digit:]]+ "
>> -                     "pass=[[:digit:]]+ image=[[:xdigit:]]+", REG_EXTENDED);
>> -       assert(ret == 0);
>> -
>> -       ptr = haystack;
>> -       memset(pmatch, 0, sizeof(pmatch));
>> -
>> -       while (1) {
>> -               ret = regexec(&regex, ptr, 1, pmatch, 0);
>> -               if (ret == 0) {
>> -                       ptr += pmatch[0].rm_eo;
>> -                       off += pmatch[0].rm_eo;
>> -                       assert(off < hlen);
>> -               } else
>> -                       break;
>> -       }
>> -
>> -       ptr = haystack + off - (pmatch[0].rm_eo - pmatch[0].rm_so);
>> -       ret = sscanf(ptr, "flen=%d proglen=%u pass=%d image=%lx",
>> -                    &flen, &proglen, &pass, &base);
>> -       if (ret != 4) {
>> -               regfree(&regex);
>> -               return NULL;
>> -       }
>> -       if (proglen > 1000000) {
>> -               printf("proglen of %d too big, stopping\n", proglen);
>> -               return NULL;
>> -       }
>> -
>> -       image = malloc(proglen);
>> -       if (!image) {
>> -               printf("Out of memory\n");
>> -               return NULL;
>> -       }
>> -       memset(image, 0, proglen);
>> -
>> -       tmp = ptr = haystack + off;
>> -       while ((ptr = strtok(tmp, "\n")) != NULL && ulen < proglen) {
>> -               tmp = NULL;
>> -               if (!strstr(ptr, "JIT code"))
>> -                       continue;
>> -               pptr = ptr;
>> -               while ((ptr = strstr(pptr, ":")))
>> -                       pptr = ptr + 1;
>> -               ptr = pptr;
>> -               do {
>> -                       image[ulen++] = (uint8_t) strtoul(pptr, &pptr, 16);
>> -                       if (ptr == pptr) {
>> -                               ulen--;
>> -                               break;
>> -                       }
>> -                       if (ulen >= proglen)
>> -                               break;
>> -                       ptr = pptr;
>> -               } while (1);
>> -       }
>> -
>> -       assert(ulen == proglen);
>> -       printf("%u bytes emitted from JIT compiler (pass:%d, flen:%d)\n",
>> -              proglen, pass, flen);
>> -       printf("%lx + <x>:\n", base);
>> -
>> -       regfree(&regex);
>> -       *ilen = ulen;
>> -       return image;
>> -}
>> -
>> -static void usage(void)
>> -{
>> -       printf("Usage: bpf_jit_disasm [...]\n");
>> -       printf("       -o          Also display related opcodes (default: off).\n");
>> -       printf("       -O <file>   Write binary image of code to file, don't disassemble to stdout.\n");
>> -       printf("       -f <file>   Read last image dump from file or stdin (default: klog).\n");
>> -       printf("       -h          Display this help.\n");
>> -}
>> -
>> -int main(int argc, char **argv)
>> -{
>> -       unsigned int len, klen, opt, opcodes = 0;
>> -       char *kbuff, *file = NULL;
>> -       char *ofile = NULL;
>> -       int ofd;
>> -       ssize_t nr;
>> -       uint8_t *pos;
>> -       uint8_t *image = NULL;
>> -
>> -       while ((opt = getopt(argc, argv, "of:O:")) != -1) {
>> -               switch (opt) {
>> -               case 'o':
>> -                       opcodes = 1;
>> -                       break;
>> -               case 'O':
>> -                       ofile = optarg;
>> -                       break;
>> -               case 'f':
>> -                       file = optarg;
>> -                       break;
>> -               default:
>> -                       usage();
>> -                       return -1;
>> -               }
>> -       }
>> -
>> -       bfd_init();
>> -
>> -       kbuff = get_log_buff(file, &klen);
>> -       if (!kbuff) {
>> -               fprintf(stderr, "Could not retrieve log buffer!\n");
>> -               return -1;
>> -       }
>> -
>> -       image = get_last_jit_image(kbuff, klen, &len);
>> -       if (!image) {
>> -               fprintf(stderr, "No JIT image found!\n");
>> -               goto done;
>> -       }
>> -       if (!ofile) {
>> -               get_asm_insns(image, len, opcodes);
>> -               goto done;
>> -       }
>> -
>> -       ofd = open(ofile, O_WRONLY | O_CREAT | O_TRUNC, DEFFILEMODE);
>> -       if (ofd < 0) {
>> -               fprintf(stderr, "Could not open file %s for writing: ", ofile);
>> -               perror(NULL);
>> -               goto done;
>> -       }
>> -       pos = image;
>> -       do {
>> -               nr = write(ofd, pos, len);
>> -               if (nr < 0) {
>> -                       fprintf(stderr, "Could not write data to %s: ", ofile);
>> -                       perror(NULL);
>> -                       goto done;
>> -               }
>> -               len -= nr;
>> -               pos += nr;
>> -       } while (len);
>> -       close(ofd);
>> -
>> -done:
>> -       put_log_buff(kbuff);
>> -       free(image);
>> -       return 0;
>> -}
>> --
>> 2.27.0


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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-05 17:53 ` Christophe Leroy
  2023-01-06 13:22   ` Tonghao Zhang
@ 2023-01-06 15:37   ` Daniel Borkmann
  2023-01-09  8:15     ` Christophe Leroy
  1 sibling, 1 reply; 23+ messages in thread
From: Daniel Borkmann @ 2023-01-06 15:37 UTC (permalink / raw)
  To: Christophe Leroy, tong, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux
  Cc: Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe

On 1/5/23 6:53 PM, Christophe Leroy wrote:
> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>> From: Tonghao Zhang <tong@infragraf.org>
>>
>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>> which include subprog:
>>
>> $ llvm-objdump -d subprog.o
>> Disassembly of section .text:
>> 0000000000000000 <subprog>:
>>          0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1 = 29114459903653235 ll
>>          2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>          3:       bf a1 00 00 00 00 00 00 r1 = r10
>>          4:       07 01 00 00 f8 ff ff ff r1 += -8
>>          5:       b7 02 00 00 08 00 00 00 r2 = 8
>>          6:       85 00 00 00 06 00 00 00 call 6
>>          7:       95 00 00 00 00 00 00 00 exit
>> Disassembly of section raw_tp/sys_enter:
>> 0000000000000000 <entry>:
>>          0:       85 10 00 00 ff ff ff ff call -1
>>          1:       b7 00 00 00 00 00 00 00 r0 = 0
>>          2:       95 00 00 00 00 00 00 00 exit
>>
>> kernel print message:
>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c from=kprobe-load pid=1643
>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
>> [  580.782568] JIT code: 00000030: cc cc cc
>>
>> $ bpf_jit_disasm
>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>> ffffffffa000c20c + <x>:
>>      0:   int3
>>      1:   int3
>>      2:   int3
>>      3:   int3
>>      4:   int3
>>      5:   int3
>>      ...
>>
>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to header
>> and then image/insn is valid. BTW, we can use the "bpftool prog dump" JITed instructions.
> 
> NACK.
> 
> Because the feature is buggy on x86_64, you remove it for all
> architectures ?
> 
> On powerpc bpf_jit_enable == 2 works and is very usefull.
> 
> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
> remember the details, I think it was an issue with endianess. Maybe it
> is fixed now, but it needs to be verified.
> 
> So please, before removing a working and usefull feature, make sure
> there is an alternative available to it for all architectures in all
> configurations.
> 
> Also, I don't think bpftool is usable to dump kernel BPF selftests.
> That's vital when a selftest fails if you want to have a chance to
> understand why it fails.

If this is actively used by JIT developers and considered useful, I'd be
ok to leave it for the time being. Overall goal is to reach feature parity
among (at least major arch) JITs and not just have most functionality only
available on x86-64 JIT. Could you however check what is not working with
bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
but details would be useful otherwise 'it didn't work' is too fuzzy.

Also, with regards to the last statement that bpftool is not usable to
dump kernel BPF selftests. Could you elaborate some more? I haven't used
bpf_jit_enable == 2 in a long time and for debugging always relied on
bpftool to dump xlated insns or JIT. Or do you mean by BPF selftests
the test_bpf.ko module? Given it has a big batch with kernel-only tests,
there I can see it's probably still useful.

Cheers,
Daniel

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-06 15:37   ` Daniel Borkmann
@ 2023-01-09  8:15     ` Christophe Leroy
  2023-01-17  5:30       ` Tonghao Zhang
  0 siblings, 1 reply; 23+ messages in thread
From: Christophe Leroy @ 2023-01-09  8:15 UTC (permalink / raw)
  To: Daniel Borkmann, tong, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux
  Cc: Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>> From: Tonghao Zhang <tong@infragraf.org>
>>>
>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>> which include subprog:
>>>
>>> $ llvm-objdump -d subprog.o
>>> Disassembly of section .text:
>>> 0000000000000000 <subprog>:
>>>          0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1 
>>> = 29114459903653235 ll
>>>          2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>          3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>          4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>          5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>          6:       85 00 00 00 06 00 00 00 call 6
>>>          7:       95 00 00 00 00 00 00 00 exit
>>> Disassembly of section raw_tp/sys_enter:
>>> 0000000000000000 <entry>:
>>>          0:       85 10 00 00 ff ff ff ff call -1
>>>          1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>          2:       95 00 00 00 00 00 00 00 exit
>>>
>>> kernel print message:
>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c 
>>> from=kprobe-load pid=1643
>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc 
>>> cc cc cc cc cc
>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc 
>>> cc cc cc cc cc
>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc 
>>> cc cc cc cc cc
>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>
>>> $ bpf_jit_disasm
>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>> ffffffffa000c20c + <x>:
>>>      0:   int3
>>>      1:   int3
>>>      2:   int3
>>>      3:   int3
>>>      4:   int3
>>>      5:   int3
>>>      ...
>>>
>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to 
>>> header
>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump" 
>>> JITed instructions.
>>
>> NACK.
>>
>> Because the feature is buggy on x86_64, you remove it for all
>> architectures ?
>>
>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>
>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>> remember the details, I think it was an issue with endianess. Maybe it
>> is fixed now, but it needs to be verified.
>>
>> So please, before removing a working and usefull feature, make sure
>> there is an alternative available to it for all architectures in all
>> configurations.
>>
>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>> That's vital when a selftest fails if you want to have a chance to
>> understand why it fails.
> 
> If this is actively used by JIT developers and considered useful, I'd be
> ok to leave it for the time being. Overall goal is to reach feature parity
> among (at least major arch) JITs and not just have most functionality only
> available on x86-64 JIT. Could you however check what is not working with
> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
> but details would be useful otherwise 'it didn't work' is too fuzzy.

Sure I will try to test bpftool again in the coming days.

Previous discussion about that subject is here: 
https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847


> 
> Also, with regards to the last statement that bpftool is not usable to
> dump kernel BPF selftests. Could you elaborate some more? I haven't used
> bpf_jit_enable == 2 in a long time and for debugging always relied on
> bpftool to dump xlated insns or JIT. Or do you mean by BPF selftests
> the test_bpf.ko module? Given it has a big batch with kernel-only tests,
> there I can see it's probably still useful.

Yes I mean test_bpf.ko

I used it as the test basis when I implemented eBPF for powerpc/32. And 
not so long ago it helped decover and fix a bug, see 
https://github.com/torvalds/linux/commit/89d21e259a94f7d5582ec675aa445f5a79f347e4

> 
> Cheers,
> Daniel

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-09  8:15     ` Christophe Leroy
@ 2023-01-17  5:30       ` Tonghao Zhang
  2023-01-17  7:30         ` Christophe Leroy
  0 siblings, 1 reply; 23+ messages in thread
From: Tonghao Zhang @ 2023-01-17  5:30 UTC (permalink / raw)
  To: Christophe Leroy, Daniel Borkmann
  Cc: Daniel Borkmann, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>> 
>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>> which include subprog:
>>>> 
>>>> $ llvm-objdump -d subprog.o
>>>> Disassembly of section .text:
>>>> 0000000000000000 <subprog>:
>>>>          0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1 
>>>> = 29114459903653235 ll
>>>>          2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>          3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>          4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>          5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>          6:       85 00 00 00 06 00 00 00 call 6
>>>>          7:       95 00 00 00 00 00 00 00 exit
>>>> Disassembly of section raw_tp/sys_enter:
>>>> 0000000000000000 <entry>:
>>>>          0:       85 10 00 00 ff ff ff ff call -1
>>>>          1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>          2:       95 00 00 00 00 00 00 00 exit
>>>> 
>>>> kernel print message:
>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c 
>>>> from=kprobe-load pid=1643
>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc 
>>>> cc cc cc cc cc
>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc 
>>>> cc cc cc cc cc
>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc 
>>>> cc cc cc cc cc
>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>> 
>>>> $ bpf_jit_disasm
>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>> ffffffffa000c20c + <x>:
>>>>      0:   int3
>>>>      1:   int3
>>>>      2:   int3
>>>>      3:   int3
>>>>      4:   int3
>>>>      5:   int3
>>>>      ...
>>>> 
>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to 
>>>> header
>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump" 
>>>> JITed instructions.
>>> 
>>> NACK.
>>> 
>>> Because the feature is buggy on x86_64, you remove it for all
>>> architectures ?
>>> 
>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>> 
>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>> remember the details, I think it was an issue with endianess. Maybe it
>>> is fixed now, but it needs to be verified.
>>> 
>>> So please, before removing a working and usefull feature, make sure
>>> there is an alternative available to it for all architectures in all
>>> configurations.
>>> 
>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>> That's vital when a selftest fails if you want to have a chance to
>>> understand why it fails.
>> 
>> If this is actively used by JIT developers and considered useful, I'd be
>> ok to leave it for the time being. Overall goal is to reach feature parity
>> among (at least major arch) JITs and not just have most functionality only
>> available on x86-64 JIT. Could you however check what is not working with
>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>> but details would be useful otherwise 'it didn't work' is too fuzzy.
> 
> Sure I will try to test bpftool again in the coming days.
> 
> Previous discussion about that subject is here: 
> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
Hi Christophe
Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
Now can we fix this issue? 
> 
>> 
>> Also, with regards to the last statement that bpftool is not usable to
>> dump kernel BPF selftests. Could you elaborate some more? I haven't used
>> bpf_jit_enable == 2 in a long time and for debugging always relied on
>> bpftool to dump xlated insns or JIT. Or do you mean by BPF selftests
>> the test_bpf.ko module? Given it has a big batch with kernel-only tests,
>> there I can see it's probably still useful.
> 
> Yes I mean test_bpf.ko
> 
> I used it as the test basis when I implemented eBPF for powerpc/32. And 
> not so long ago it helped decover and fix a bug, see 
> https://github.com/torvalds/linux/commit/89d21e259a94f7d5582ec675aa445f5a79f347e4
> 
>> 
>> Cheers,
>> Daniel


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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17  5:30       ` Tonghao Zhang
@ 2023-01-17  7:30         ` Christophe Leroy
  2023-01-17 11:36           ` Christophe Leroy
  2023-01-17 14:22           ` Tonghao Zhang
  0 siblings, 2 replies; 23+ messages in thread
From: Christophe Leroy @ 2023-01-17  7:30 UTC (permalink / raw)
  To: Tonghao Zhang, Daniel Borkmann
  Cc: bpf, netdev, linux-arm-kernel, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, Hao Luo,
	John Fastabend, Alexei Starovoitov, Andrii Nakryiko, Song Liu,
	Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh, Yonghong Song,
	Martin KaFai Lau, naveen.n.rao, mpe



Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
> 
> 
>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>
>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>> which include subprog:
>>>>>
>>>>> $ llvm-objdump -d subprog.o
>>>>> Disassembly of section .text:
>>>>> 0000000000000000 <subprog>:
>>>>>           0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>> = 29114459903653235 ll
>>>>>           2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>           3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>           4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>           5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>           6:       85 00 00 00 06 00 00 00 call 6
>>>>>           7:       95 00 00 00 00 00 00 00 exit
>>>>> Disassembly of section raw_tp/sys_enter:
>>>>> 0000000000000000 <entry>:
>>>>>           0:       85 10 00 00 ff ff ff ff call -1
>>>>>           1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>           2:       95 00 00 00 00 00 00 00 exit
>>>>>
>>>>> kernel print message:
>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>> from=kprobe-load pid=1643
>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>> cc cc cc cc cc
>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>> cc cc cc cc cc
>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>> cc cc cc cc cc
>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>
>>>>> $ bpf_jit_disasm
>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>> ffffffffa000c20c + <x>:
>>>>>       0:   int3
>>>>>       1:   int3
>>>>>       2:   int3
>>>>>       3:   int3
>>>>>       4:   int3
>>>>>       5:   int3
>>>>>       ...
>>>>>
>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>> header
>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>> JITed instructions.
>>>>
>>>> NACK.
>>>>
>>>> Because the feature is buggy on x86_64, you remove it for all
>>>> architectures ?
>>>>
>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>
>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>> is fixed now, but it needs to be verified.
>>>>
>>>> So please, before removing a working and usefull feature, make sure
>>>> there is an alternative available to it for all architectures in all
>>>> configurations.
>>>>
>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>> That's vital when a selftest fails if you want to have a chance to
>>>> understand why it fails.
>>>
>>> If this is actively used by JIT developers and considered useful, I'd be
>>> ok to leave it for the time being. Overall goal is to reach feature parity
>>> among (at least major arch) JITs and not just have most functionality only
>>> available on x86-64 JIT. Could you however check what is not working with
>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>
>> Sure I will try to test bpftool again in the coming days.
>>
>> Previous discussion about that subject is here:
>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
> Hi Christophe
> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
> Now can we fix this issue?

Hi Tong,

I have started to look at it but I don't have any fruitfull feedback yet.

In the meantime, were you able to confirm that bpftool can also be used 
to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you 
tell me how to proceed ?

Thanks
Christophe

>>
>>>
>>> Also, with regards to the last statement that bpftool is not usable to
>>> dump kernel BPF selftests. Could you elaborate some more? I haven't used
>>> bpf_jit_enable == 2 in a long time and for debugging always relied on
>>> bpftool to dump xlated insns or JIT. Or do you mean by BPF selftests
>>> the test_bpf.ko module? Given it has a big batch with kernel-only tests,
>>> there I can see it's probably still useful.
>>
>> Yes I mean test_bpf.ko
>>
>> I used it as the test basis when I implemented eBPF for powerpc/32. And
>> not so long ago it helped decover and fix a bug, see
>> https://github.com/torvalds/linux/commit/89d21e259a94f7d5582ec675aa445f5a79f347e4
>>
>>>
>>> Cheers,
>>> Daniel
> 

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17  7:30         ` Christophe Leroy
@ 2023-01-17 11:36           ` Christophe Leroy
  2023-01-17 14:18             ` Tonghao Zhang
  2023-01-17 14:22           ` Tonghao Zhang
  1 sibling, 1 reply; 23+ messages in thread
From: Christophe Leroy @ 2023-01-17 11:36 UTC (permalink / raw)
  To: Tonghao Zhang, Daniel Borkmann
  Cc: bpf, netdev, linux-arm-kernel, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, Hao Luo,
	John Fastabend, Alexei Starovoitov, Andrii Nakryiko, Song Liu,
	Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh, Yonghong Song,
	Martin KaFai Lau, naveen.n.rao, mpe



Le 17/01/2023 à 08:30, Christophe Leroy a écrit :
> 
> 
> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>
>>
>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy 
>>> <christophe.leroy@csgroup.eu> wrote:
>>>
>>>
>>>
>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>>
>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>> which include subprog:
>>>>>>
>>>>>> $ llvm-objdump -d subprog.o
>>>>>> Disassembly of section .text:
>>>>>> 0000000000000000 <subprog>:
>>>>>>           0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>> = 29114459903653235 ll
>>>>>>           2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>           3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>           4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>           5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>           6:       85 00 00 00 06 00 00 00 call 6
>>>>>>           7:       95 00 00 00 00 00 00 00 exit
>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>> 0000000000000000 <entry>:
>>>>>>           0:       85 10 00 00 ff ff ff ff call -1
>>>>>>           1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>           2:       95 00 00 00 00 00 00 00 exit
>>>>>>
>>>>>> kernel print message:
>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>> from=kprobe-load pid=1643
>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>> cc cc cc cc cc
>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>> cc cc cc cc cc
>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>> cc cc cc cc cc
>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>>
>>>>>> $ bpf_jit_disasm
>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>> ffffffffa000c20c + <x>:
>>>>>>       0:   int3
>>>>>>       1:   int3
>>>>>>       2:   int3
>>>>>>       3:   int3
>>>>>>       4:   int3
>>>>>>       5:   int3
>>>>>>       ...
>>>>>>
>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>> header
>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>> JITed instructions.
>>>>>
>>>>> NACK.
>>>>>
>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>> architectures ?
>>>>>
>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>>
>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>> is fixed now, but it needs to be verified.
>>>>>
>>>>> So please, before removing a working and usefull feature, make sure
>>>>> there is an alternative available to it for all architectures in all
>>>>> configurations.
>>>>>
>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>> understand why it fails.
>>>>
>>>> If this is actively used by JIT developers and considered useful, 
>>>> I'd be
>>>> ok to leave it for the time being. Overall goal is to reach feature 
>>>> parity
>>>> among (at least major arch) JITs and not just have most 
>>>> functionality only
>>>> available on x86-64 JIT. Could you however check what is not working 
>>>> with
>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>>
>>> Sure I will try to test bpftool again in the coming days.
>>>
>>> Previous discussion about that subject is here:
>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>> Hi Christophe
>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, 
>> but bpftool can not run on powerpc.
>> Now can we fix this issue?
> 
> Hi Tong,
> 
> I have started to look at it but I don't have any fruitfull feedback yet.

Hi Again,

I tested again, the problem is still the same as one year ago:

root@vgoip:~# ./bpftool prog
libbpf: elf: endianness mismatch in pid_iter_bpf.
libbpf: failed to initialize skeleton BPF object 'pid_iter_bpf': -4003
Error: failed to open PID iterator skeleton

root@vgoip:~# uname -a
Linux vgoip 6.2.0-rc3-02596-g1c2c9c13e256 #242 PREEMPT Tue Jan 17 
09:36:08 CET 2023 ppc GNU/Linux


> 
> In the meantime, were you able to confirm that bpftool can also be used 
> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you 
> tell me how to proceed ?
> 
> Thanks
> Christophe
> 


Christophe

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 11:36           ` Christophe Leroy
@ 2023-01-17 14:18             ` Tonghao Zhang
  2023-01-17 14:25               ` Christophe Leroy
  0 siblings, 1 reply; 23+ messages in thread
From: Tonghao Zhang @ 2023-01-17 14:18 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Daniel Borkmann, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



> On Jan 17, 2023, at 7:36 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 17/01/2023 à 08:30, Christophe Leroy a écrit :
>> 
>> 
>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>> 
>>> 
>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy 
>>>> <christophe.leroy@csgroup.eu> wrote:
>>>> 
>>>> 
>>>> 
>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>>> 
>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>>> which include subprog:
>>>>>>> 
>>>>>>> $ llvm-objdump -d subprog.o
>>>>>>> Disassembly of section .text:
>>>>>>> 0000000000000000 <subprog>:
>>>>>>>           0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>>> = 29114459903653235 ll
>>>>>>>           2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>>           3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>>           4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>>           5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>>           6:       85 00 00 00 06 00 00 00 call 6
>>>>>>>           7:       95 00 00 00 00 00 00 00 exit
>>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>>> 0000000000000000 <entry>:
>>>>>>>           0:       85 10 00 00 ff ff ff ff call -1
>>>>>>>           1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>>           2:       95 00 00 00 00 00 00 00 exit
>>>>>>> 
>>>>>>> kernel print message:
>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>>> from=kprobe-load pid=1643
>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>> cc cc cc cc cc
>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>> cc cc cc cc cc
>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>> cc cc cc cc cc
>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>>> 
>>>>>>> $ bpf_jit_disasm
>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>>> ffffffffa000c20c + <x>:
>>>>>>>       0:   int3
>>>>>>>       1:   int3
>>>>>>>       2:   int3
>>>>>>>       3:   int3
>>>>>>>       4:   int3
>>>>>>>       5:   int3
>>>>>>>       ...
>>>>>>> 
>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>>> header
>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>>> JITed instructions.
>>>>>> 
>>>>>> NACK.
>>>>>> 
>>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>>> architectures ?
>>>>>> 
>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>>> 
>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>>> is fixed now, but it needs to be verified.
>>>>>> 
>>>>>> So please, before removing a working and usefull feature, make sure
>>>>>> there is an alternative available to it for all architectures in all
>>>>>> configurations.
>>>>>> 
>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>>> understand why it fails.
>>>>> 
>>>>> If this is actively used by JIT developers and considered useful, 
>>>>> I'd be
>>>>> ok to leave it for the time being. Overall goal is to reach feature 
>>>>> parity
>>>>> among (at least major arch) JITs and not just have most 
>>>>> functionality only
>>>>> available on x86-64 JIT. Could you however check what is not working 
>>>>> with
>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>>> 
>>>> Sure I will try to test bpftool again in the coming days.
>>>> 
>>>> Previous discussion about that subject is here:
>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>>> Hi Christophe
>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, 
>>> but bpftool can not run on powerpc.
>>> Now can we fix this issue?
>> 
>> Hi Tong,
>> 
>> I have started to look at it but I don't have any fruitfull feedback yet.
> 
> Hi Again,
> 
> I tested again, the problem is still the same as one year ago:
> 
> root@vgoip:~# ./bpftool prog
> libbpf: elf: endianness mismatch in pid_iter_bpf.
It seem to be not right ehdr->e_ident[EI_DATA]. Do we can print the real value?
/*
 * e_ident[EI_DATA]
 */
#define ELFDATANONE     0
#define ELFDATA2LSB     1
#define ELFDATA2MSB     2
#define ELFDATANUM      3

bpf_object__elf_init:
obj->efile.ehdr = ehdr = elf64_getehdr(elf);

> libbpf: failed to initialize skeleton BPF object 'pid_iter_bpf': -4003
> Error: failed to open PID iterator skeleton
> 
> root@vgoip:~# uname -a
> Linux vgoip 6.2.0-rc3-02596-g1c2c9c13e256 #242 PREEMPT Tue Jan 17 
> 09:36:08 CET 2023 ppc GNU/Linux
On my pc, elf is little endian.
# readelf -h tools/bpf/bpftool/pid_iter.bpf.o
ELF Header:
  Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
  Class:                             ELF64
  Data:                              2's complement, little endian # x86_64
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              REL (Relocatable file)
  Machine:                           Linux BPF
  Version:                           0x1
  Entry point address:               0x0
  Start of program headers:          0 (bytes into file)
  Start of section headers:          64832 (bytes into file)
  Flags:                             0x0
  Size of this header:               64 (bytes)
  Size of program headers:           0 (bytes)
  Number of program headers:         0
  Size of section headers:           64 (bytes)
  Number of section headers:         13
  Section header string table index: 1

> 
>> 
>> In the meantime, were you able to confirm that bpftool can also be used 
>> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you 
>> tell me how to proceed ?
>> 
>> Thanks
>> Christophe
>> 
> 
> 
> Christophe


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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17  7:30         ` Christophe Leroy
  2023-01-17 11:36           ` Christophe Leroy
@ 2023-01-17 14:22           ` Tonghao Zhang
  2023-01-17 15:59             ` Daniel Borkmann
  1 sibling, 1 reply; 23+ messages in thread
From: Tonghao Zhang @ 2023-01-17 14:22 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Daniel Borkmann, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



> On Jan 17, 2023, at 3:30 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>> 
>> 
>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>> 
>>> 
>>> 
>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>> 
>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>> which include subprog:
>>>>>> 
>>>>>> $ llvm-objdump -d subprog.o
>>>>>> Disassembly of section .text:
>>>>>> 0000000000000000 <subprog>:
>>>>>>          0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>> = 29114459903653235 ll
>>>>>>          2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>          3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>          4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>          5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>          6:       85 00 00 00 06 00 00 00 call 6
>>>>>>          7:       95 00 00 00 00 00 00 00 exit
>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>> 0000000000000000 <entry>:
>>>>>>          0:       85 10 00 00 ff ff ff ff call -1
>>>>>>          1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>          2:       95 00 00 00 00 00 00 00 exit
>>>>>> 
>>>>>> kernel print message:
>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>> from=kprobe-load pid=1643
>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>> cc cc cc cc cc
>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>> cc cc cc cc cc
>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>> cc cc cc cc cc
>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>> 
>>>>>> $ bpf_jit_disasm
>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>> ffffffffa000c20c + <x>:
>>>>>>      0:   int3
>>>>>>      1:   int3
>>>>>>      2:   int3
>>>>>>      3:   int3
>>>>>>      4:   int3
>>>>>>      5:   int3
>>>>>>      ...
>>>>>> 
>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>> header
>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>> JITed instructions.
>>>>> 
>>>>> NACK.
>>>>> 
>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>> architectures ?
>>>>> 
>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>> 
>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>> is fixed now, but it needs to be verified.
>>>>> 
>>>>> So please, before removing a working and usefull feature, make sure
>>>>> there is an alternative available to it for all architectures in all
>>>>> configurations.
>>>>> 
>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>> understand why it fails.
>>>> 
>>>> If this is actively used by JIT developers and considered useful, I'd be
>>>> ok to leave it for the time being. Overall goal is to reach feature parity
>>>> among (at least major arch) JITs and not just have most functionality only
>>>> available on x86-64 JIT. Could you however check what is not working with
>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>> 
>>> Sure I will try to test bpftool again in the coming days.
>>> 
>>> Previous discussion about that subject is here:
>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>> Hi Christophe
>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
>> Now can we fix this issue?
> 
> Hi Tong,
> 
> I have started to look at it but I don't have any fruitfull feedback yet.
> 
> In the meantime, were you able to confirm that bpftool can also be used 
> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you 
> tell me how to proceed ?
Now I do not test, but we can dump the insn after bpf_prog_select_runtime in test_bpf.ko. bpf_map_get_info_by_fd can copy the insn to userspace, but we can
dump them in test_bpf.ko in the same way.

> Thanks
> Christophe
> 
>>> 
>>>> 
>>>> Also, with regards to the last statement that bpftool is not usable to
>>>> dump kernel BPF selftests. Could you elaborate some more? I haven't used
>>>> bpf_jit_enable == 2 in a long time and for debugging always relied on
>>>> bpftool to dump xlated insns or JIT. Or do you mean by BPF selftests
>>>> the test_bpf.ko module? Given it has a big batch with kernel-only tests,
>>>> there I can see it's probably still useful.
>>> 
>>> Yes I mean test_bpf.ko
>>> 
>>> I used it as the test basis when I implemented eBPF for powerpc/32. And
>>> not so long ago it helped decover and fix a bug, see
>>> https://github.com/torvalds/linux/commit/89d21e259a94f7d5582ec675aa445f5a79f347e4
>>> 
>>>> 
>>>> Cheers,
>>>> Daniel


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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 14:18             ` Tonghao Zhang
@ 2023-01-17 14:25               ` Christophe Leroy
  2023-01-17 14:41                 ` Quentin Monnet
  0 siblings, 1 reply; 23+ messages in thread
From: Christophe Leroy @ 2023-01-17 14:25 UTC (permalink / raw)
  To: Tonghao Zhang
  Cc: Daniel Borkmann, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



Le 17/01/2023 à 15:18, Tonghao Zhang a écrit :
> 
> 
>> On Jan 17, 2023, at 7:36 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 17/01/2023 à 08:30, Christophe Leroy a écrit :
>>>
>>>
>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>>
>>>>
>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>
>>>>>
>>>>>
>>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>>>>
>>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>>>> which include subprog:
>>>>>>>>
>>>>>>>> $ llvm-objdump -d subprog.o
>>>>>>>> Disassembly of section .text:
>>>>>>>> 0000000000000000 <subprog>:
>>>>>>>>            0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>>>> = 29114459903653235 ll
>>>>>>>>            2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>>>            3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>>>            4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>>>            5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>>>            6:       85 00 00 00 06 00 00 00 call 6
>>>>>>>>            7:       95 00 00 00 00 00 00 00 exit
>>>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>>>> 0000000000000000 <entry>:
>>>>>>>>            0:       85 10 00 00 ff ff ff ff call -1
>>>>>>>>            1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>>>            2:       95 00 00 00 00 00 00 00 exit
>>>>>>>>
>>>>>>>> kernel print message:
>>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>>>> from=kprobe-load pid=1643
>>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>> cc cc cc cc cc
>>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>> cc cc cc cc cc
>>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>> cc cc cc cc cc
>>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>>>>
>>>>>>>> $ bpf_jit_disasm
>>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>>>> ffffffffa000c20c + <x>:
>>>>>>>>        0:   int3
>>>>>>>>        1:   int3
>>>>>>>>        2:   int3
>>>>>>>>        3:   int3
>>>>>>>>        4:   int3
>>>>>>>>        5:   int3
>>>>>>>>        ...
>>>>>>>>
>>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>>>> header
>>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>>>> JITed instructions.
>>>>>>>
>>>>>>> NACK.
>>>>>>>
>>>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>>>> architectures ?
>>>>>>>
>>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>>>>
>>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>>>> is fixed now, but it needs to be verified.
>>>>>>>
>>>>>>> So please, before removing a working and usefull feature, make sure
>>>>>>> there is an alternative available to it for all architectures in all
>>>>>>> configurations.
>>>>>>>
>>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>>>> understand why it fails.
>>>>>>
>>>>>> If this is actively used by JIT developers and considered useful,
>>>>>> I'd be
>>>>>> ok to leave it for the time being. Overall goal is to reach feature
>>>>>> parity
>>>>>> among (at least major arch) JITs and not just have most
>>>>>> functionality only
>>>>>> available on x86-64 JIT. Could you however check what is not working
>>>>>> with
>>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>>>>
>>>>> Sure I will try to test bpftool again in the coming days.
>>>>>
>>>>> Previous discussion about that subject is here:
>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>>>> Hi Christophe
>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021,
>>>> but bpftool can not run on powerpc.
>>>> Now can we fix this issue?
>>>
>>> Hi Tong,
>>>
>>> I have started to look at it but I don't have any fruitfull feedback yet.
>>
>> Hi Again,
>>
>> I tested again, the problem is still the same as one year ago:
>>
>> root@vgoip:~# ./bpftool prog
>> libbpf: elf: endianness mismatch in pid_iter_bpf.
> It seem to be not right ehdr->e_ident[EI_DATA]. Do we can print the real value?
> /*
>   * e_ident[EI_DATA]
>   */
> #define ELFDATANONE     0
> #define ELFDATA2LSB     1
> #define ELFDATA2MSB     2
> #define ELFDATANUM      3
> 
> bpf_object__elf_init:
> obj->efile.ehdr = ehdr = elf64_getehdr(elf);
> 
>> libbpf: failed to initialize skeleton BPF object 'pid_iter_bpf': -4003
>> Error: failed to open PID iterator skeleton
>>
>> root@vgoip:~# uname -a
>> Linux vgoip 6.2.0-rc3-02596-g1c2c9c13e256 #242 PREEMPT Tue Jan 17
>> 09:36:08 CET 2023 ppc GNU/Linux
> On my pc, elf is little endian.
> # readelf -h tools/bpf/bpftool/pid_iter.bpf.o
> ELF Header:
>    Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>    Class:                             ELF64
>    Data:                              2's complement, little endian # x86_64
>    Version:                           1 (current)
>    OS/ABI:                            UNIX - System V
>    ABI Version:                       0
>    Type:                              REL (Relocatable file)
>    Machine:                           Linux BPF
>    Version:                           0x1
>    Entry point address:               0x0
>    Start of program headers:          0 (bytes into file)
>    Start of section headers:          64832 (bytes into file)
>    Flags:                             0x0
>    Size of this header:               64 (bytes)
>    Size of program headers:           0 (bytes)
>    Number of program headers:         0
>    Size of section headers:           64 (bytes)
>    Number of section headers:         13
>    Section header string table index: 1
> 

Yes, must be something wrong with the build, I get same as you :

$ LANG= readelf -h pid_iter.bpf.o
ELF Header:
   Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
   Class:                             ELF64
   Data:                              2's complement, little endian
   Version:                           1 (current)
   OS/ABI:                            UNIX - System V
   ABI Version:                       0
   Type:                              REL (Relocatable file)
   Machine:                           Linux BPF
   Version:                           0x1
   Entry point address:               0x0
   Start of program headers:          0 (bytes into file)
   Start of section headers:          34704 (bytes into file)
   Flags:                             0x0
   Size of this header:               64 (bytes)
   Size of program headers:           0 (bytes)
   Number of program headers:         0
   Size of section headers:           64 (bytes)
   Number of section headers:         13
   Section header string table index: 1


Whereas I expect the same as bpftool I suppose, which is :

$ LANG= readelf -h bpftool
ELF Header:
   Magic:   7f 45 4c 46 01 02 01 00 00 00 00 00 00 00 00 00
   Class:                             ELF32
   Data:                              2's complement, big endian
   Version:                           1 (current)
   OS/ABI:                            UNIX - System V
   ABI Version:                       0
   Type:                              EXEC (Executable file)
   Machine:                           PowerPC
   Version:                           0x1
   Entry point address:               0x100027d0
   Start of program headers:          52 (bytes into file)
   Start of section headers:          1842896 (bytes into file)
   Flags:                             0x0
   Size of this header:               52 (bytes)
   Size of program headers:           32 (bytes)
   Number of program headers:         9
   Size of section headers:           40 (bytes)
   Number of section headers:         39
   Section header string table index: 38


Thanks
Christophe

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 14:25               ` Christophe Leroy
@ 2023-01-17 14:41                 ` Quentin Monnet
  2023-01-17 14:55                   ` Christophe Leroy
  0 siblings, 1 reply; 23+ messages in thread
From: Quentin Monnet @ 2023-01-17 14:41 UTC (permalink / raw)
  To: Christophe Leroy, Tonghao Zhang
  Cc: Daniel Borkmann, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe

2023-01-17 14:25 UTC+0000 ~ Christophe Leroy <christophe.leroy@csgroup.eu>
> 
> 
> Le 17/01/2023 à 15:18, Tonghao Zhang a écrit :
>>
>>
>>> On Jan 17, 2023, at 7:36 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>
>>>
>>>
>>> Le 17/01/2023 à 08:30, Christophe Leroy a écrit :
>>>>
>>>>
>>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>>>
>>>>>
>>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy
>>>>>> <christophe.leroy@csgroup.eu> wrote:

[...]

>>>>>> Sure I will try to test bpftool again in the coming days.
>>>>>>
>>>>>> Previous discussion about that subject is here:
>>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=

Christophe, apologies from dropping the discussion the last time, it
seems your last message on that thread didn't make it to my inbox at the
time :/. Thanks a lot for looking into that again!

>>>>> Hi Christophe
>>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021,
>>>>> but bpftool can not run on powerpc.
>>>>> Now can we fix this issue?
>>>>
>>>> Hi Tong,
>>>>
>>>> I have started to look at it but I don't have any fruitfull feedback yet.
>>>
>>> Hi Again,
>>>
>>> I tested again, the problem is still the same as one year ago:
>>>
>>> root@vgoip:~# ./bpftool prog
>>> libbpf: elf: endianness mismatch in pid_iter_bpf.
>> It seem to be not right ehdr->e_ident[EI_DATA]. Do we can print the real value?
>> /*
>>   * e_ident[EI_DATA]
>>   */
>> #define ELFDATANONE     0
>> #define ELFDATA2LSB     1
>> #define ELFDATA2MSB     2
>> #define ELFDATANUM      3
>>
>> bpf_object__elf_init:
>> obj->efile.ehdr = ehdr = elf64_getehdr(elf);
>>
>>> libbpf: failed to initialize skeleton BPF object 'pid_iter_bpf': -4003
>>> Error: failed to open PID iterator skeleton
>>>
>>> root@vgoip:~# uname -a
>>> Linux vgoip 6.2.0-rc3-02596-g1c2c9c13e256 #242 PREEMPT Tue Jan 17
>>> 09:36:08 CET 2023 ppc GNU/Linux
>> On my pc, elf is little endian.
>> # readelf -h tools/bpf/bpftool/pid_iter.bpf.o
>> ELF Header:
>>    Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>>    Class:                             ELF64
>>    Data:                              2's complement, little endian # x86_64
>>    Version:                           1 (current)
>>    OS/ABI:                            UNIX - System V
>>    ABI Version:                       0
>>    Type:                              REL (Relocatable file)
>>    Machine:                           Linux BPF
>>    Version:                           0x1
>>    Entry point address:               0x0
>>    Start of program headers:          0 (bytes into file)
>>    Start of section headers:          64832 (bytes into file)
>>    Flags:                             0x0
>>    Size of this header:               64 (bytes)
>>    Size of program headers:           0 (bytes)
>>    Number of program headers:         0
>>    Size of section headers:           64 (bytes)
>>    Number of section headers:         13
>>    Section header string table index: 1
>>
> 
> Yes, must be something wrong with the build, I get same as you :
> 
> $ LANG= readelf -h pid_iter.bpf.o
> ELF Header:
>    Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>    Class:                             ELF64
>    Data:                              2's complement, little endian
>    Version:                           1 (current)
>    OS/ABI:                            UNIX - System V
>    ABI Version:                       0
>    Type:                              REL (Relocatable file)
>    Machine:                           Linux BPF
>    Version:                           0x1
>    Entry point address:               0x0
>    Start of program headers:          0 (bytes into file)
>    Start of section headers:          34704 (bytes into file)
>    Flags:                             0x0
>    Size of this header:               64 (bytes)
>    Size of program headers:           0 (bytes)
>    Number of program headers:         0
>    Size of section headers:           64 (bytes)
>    Number of section headers:         13
>    Section header string table index: 1
> 
> 
> Whereas I expect the same as bpftool I suppose, which is :
> 
> $ LANG= readelf -h bpftool
> ELF Header:
>    Magic:   7f 45 4c 46 01 02 01 00 00 00 00 00 00 00 00 00
>    Class:                             ELF32
>    Data:                              2's complement, big endian
>    Version:                           1 (current)
>    OS/ABI:                            UNIX - System V
>    ABI Version:                       0
>    Type:                              EXEC (Executable file)
>    Machine:                           PowerPC
>    Version:                           0x1
>    Entry point address:               0x100027d0
>    Start of program headers:          52 (bytes into file)
>    Start of section headers:          1842896 (bytes into file)
>    Flags:                             0x0
>    Size of this header:               52 (bytes)
>    Size of program headers:           32 (bytes)
>    Number of program headers:         9
>    Size of section headers:           40 (bytes)
>    Number of section headers:         39
>    Section header string table index: 38
> 

pid_iter.bpf.o should be generated from that command in bpftool's Makefile:

	$(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h \
			$(LIBBPF_BOOTSTRAP)
		$(QUIET_CLANG)$(CLANG) \
			-I$(or $(OUTPUT),.) \
			-I$(srctree)/tools/include/uapi/ \
			-I$(LIBBPF_BOOTSTRAP_INCLUDE) \
			-g -O2 -Wall -fno-stack-protector \
			-target bpf -c $< -o $@

My understanding is that "-target bpf" is supposed to pick the
endianness for the host (see "llc --version | grep bpf". If that's not
the case, could you please try to turn that into '-target bpfeb' in the
Makefile instead? I'd be curious to see if it helps.

Quentin

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 14:41                 ` Quentin Monnet
@ 2023-01-17 14:55                   ` Christophe Leroy
  2023-01-17 15:42                     ` Quentin Monnet
  0 siblings, 1 reply; 23+ messages in thread
From: Christophe Leroy @ 2023-01-17 14:55 UTC (permalink / raw)
  To: Quentin Monnet, Tonghao Zhang
  Cc: Daniel Borkmann, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



Le 17/01/2023 à 15:41, Quentin Monnet a écrit :
> [Vous ne recevez pas souvent de courriers de quentin@isovalent.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> 2023-01-17 14:25 UTC+0000 ~ Christophe Leroy <christophe.leroy@csgroup.eu>
>>
>>
>> Le 17/01/2023 à 15:18, Tonghao Zhang a écrit :
>>>
>>>
>>>> On Jan 17, 2023, at 7:36 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>
>>>>
>>>>
>>>> Le 17/01/2023 à 08:30, Christophe Leroy a écrit :
>>>>>
>>>>>
>>>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>>>>
>>>>>>
>>>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy
>>>>>>> <christophe.leroy@csgroup.eu> wrote:
> 
> [...]
> 
>>>>>>> Sure I will try to test bpftool again in the coming days.
>>>>>>>
>>>>>>> Previous discussion about that subject is here:
>>>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
> 
> Christophe, apologies from dropping the discussion the last time, it
> seems your last message on that thread didn't make it to my inbox at the
> time :/. Thanks a lot for looking into that again!
> 
>>>>>> Hi Christophe
>>>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021,
>>>>>> but bpftool can not run on powerpc.
>>>>>> Now can we fix this issue?
>>>>>
>>>>> Hi Tong,
>>>>>
>>>>> I have started to look at it but I don't have any fruitfull feedback yet.
>>>>
>>>> Hi Again,
>>>>
>>>> I tested again, the problem is still the same as one year ago:
>>>>
>>>> root@vgoip:~# ./bpftool prog
>>>> libbpf: elf: endianness mismatch in pid_iter_bpf.
>>> It seem to be not right ehdr->e_ident[EI_DATA]. Do we can print the real value?
>>> /*
>>>    * e_ident[EI_DATA]
>>>    */
>>> #define ELFDATANONE     0
>>> #define ELFDATA2LSB     1
>>> #define ELFDATA2MSB     2
>>> #define ELFDATANUM      3
>>>
>>> bpf_object__elf_init:
>>> obj->efile.ehdr = ehdr = elf64_getehdr(elf);
>>>
>>>> libbpf: failed to initialize skeleton BPF object 'pid_iter_bpf': -4003
>>>> Error: failed to open PID iterator skeleton
>>>>
>>>> root@vgoip:~# uname -a
>>>> Linux vgoip 6.2.0-rc3-02596-g1c2c9c13e256 #242 PREEMPT Tue Jan 17
>>>> 09:36:08 CET 2023 ppc GNU/Linux
>>> On my pc, elf is little endian.
>>> # readelf -h tools/bpf/bpftool/pid_iter.bpf.o
>>> ELF Header:
>>>     Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>>>     Class:                             ELF64
>>>     Data:                              2's complement, little endian # x86_64
>>>     Version:                           1 (current)
>>>     OS/ABI:                            UNIX - System V
>>>     ABI Version:                       0
>>>     Type:                              REL (Relocatable file)
>>>     Machine:                           Linux BPF
>>>     Version:                           0x1
>>>     Entry point address:               0x0
>>>     Start of program headers:          0 (bytes into file)
>>>     Start of section headers:          64832 (bytes into file)
>>>     Flags:                             0x0
>>>     Size of this header:               64 (bytes)
>>>     Size of program headers:           0 (bytes)
>>>     Number of program headers:         0
>>>     Size of section headers:           64 (bytes)
>>>     Number of section headers:         13
>>>     Section header string table index: 1
>>>
>>
>> Yes, must be something wrong with the build, I get same as you :
>>
>> $ LANG= readelf -h pid_iter.bpf.o
>> ELF Header:
>>     Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>>     Class:                             ELF64
>>     Data:                              2's complement, little endian
>>     Version:                           1 (current)
>>     OS/ABI:                            UNIX - System V
>>     ABI Version:                       0
>>     Type:                              REL (Relocatable file)
>>     Machine:                           Linux BPF
>>     Version:                           0x1
>>     Entry point address:               0x0
>>     Start of program headers:          0 (bytes into file)
>>     Start of section headers:          34704 (bytes into file)
>>     Flags:                             0x0
>>     Size of this header:               64 (bytes)
>>     Size of program headers:           0 (bytes)
>>     Number of program headers:         0
>>     Size of section headers:           64 (bytes)
>>     Number of section headers:         13
>>     Section header string table index: 1
>>
>>
>> Whereas I expect the same as bpftool I suppose, which is :
>>
>> $ LANG= readelf -h bpftool
>> ELF Header:
>>     Magic:   7f 45 4c 46 01 02 01 00 00 00 00 00 00 00 00 00
>>     Class:                             ELF32
>>     Data:                              2's complement, big endian
>>     Version:                           1 (current)
>>     OS/ABI:                            UNIX - System V
>>     ABI Version:                       0
>>     Type:                              EXEC (Executable file)
>>     Machine:                           PowerPC
>>     Version:                           0x1
>>     Entry point address:               0x100027d0
>>     Start of program headers:          52 (bytes into file)
>>     Start of section headers:          1842896 (bytes into file)
>>     Flags:                             0x0
>>     Size of this header:               52 (bytes)
>>     Size of program headers:           32 (bytes)
>>     Number of program headers:         9
>>     Size of section headers:           40 (bytes)
>>     Number of section headers:         39
>>     Section header string table index: 38
>>
> 
> pid_iter.bpf.o should be generated from that command in bpftool's Makefile:
> 
>          $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h \
>                          $(LIBBPF_BOOTSTRAP)
>                  $(QUIET_CLANG)$(CLANG) \
>                          -I$(or $(OUTPUT),.) \
>                          -I$(srctree)/tools/include/uapi/ \
>                          -I$(LIBBPF_BOOTSTRAP_INCLUDE) \
>                          -g -O2 -Wall -fno-stack-protector \
>                          -target bpf -c $< -o $@
> 
> My understanding is that "-target bpf" is supposed to pick the
> endianness for the host (see "llc --version | grep bpf". If that's not
> the case, could you please try to turn that into '-target bpfeb' in the
> Makefile instead? I'd be curious to see if it helps.
> 

I guess it cannot work if it picks the endianness for the build host. It 
should pick the endianness for the target host.

That's worse it seems with bpfeb : it fails at build :

   LINK    /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/bpftool
   GEN     vmlinux.h
   CLANG   pid_iter.bpf.o
   GEN     pid_iter.skel.h
libbpf: elf: endianness mismatch in pid_iter_bpf.
Error: failed to open BPF object file: Endian mismatch
make: *** [Makefile:222: pid_iter.skel.h] Error 93


Complete build in case it helps:

$ LANG= make CROSS_COMPILE=ppc-linux- ARCH=powerpc

Auto-detecting system features:
...                         clang-bpf-co-re: [ ^[[32mon^[[m  ]
...                                    llvm: [ ^[[31mOFF^[[m ]
...                                  libcap: [ ^[[31mOFF^[[m ]
...                                  libbfd: [ ^[[31mOFF^[[m ]

   MKDIR   /home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/
make[1]: Entering directory '/home/chleroy/linux-powerpc/tools/lib/bpf'
   GEN 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/bpf_helper_defs.h
   MKDIR   /home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/libbpf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/bpf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/nlattr.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/btf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/libbpf_errno.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/str_error.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/netlink.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/bpf_prog_linfo.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/libbpf_probes.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/hashmap.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/btf_dump.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/ringbuf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/strset.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/linker.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/gen_loader.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/relo_core.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/usdt.o
   LD 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/staticobjs/libbpf-in.o
   LINK    /home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/libbpf.a
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/bpf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/libbpf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/btf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/libbpf_common.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/libbpf_legacy.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/bpf_helpers.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/bpf_tracing.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/bpf_endian.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/bpf_core_read.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/skel_internal.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/libbpf_version.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/usdt.bpf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/bpf_helper_defs.h
   INSTALL libbpf_headers
make[1]: Leaving directory '/home/chleroy/linux-powerpc/tools/lib/bpf'
   MKDIR   /home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/hashmap.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/nlattr.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/relo_core.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/libbpf/include/bpf/libbpf_internal.h
   CC      btf.o
   CC      btf_dumper.o
   CC      cfg.o
   CC      cgroup.o
   CC      common.o
   CC      feature.o
   CC      gen.o
   CC      iter.o
   CC      json_writer.o
   CC      link.o
   CC      main.o
   CC      map.o
   CC      map_perf_ring.o
   CC      net.o
   CC      netlink_dumper.o
   CC      perf.o
   MKDIR 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/hashmap.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/relo_core.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/libbpf_internal.h
   MKDIR   /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/
   MKDIR   /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/
make[1]: Entering directory '/home/chleroy/linux-powerpc/tools/lib/bpf'
   GEN 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/bpf_helper_defs.h
   MKDIR 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/libbpf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/bpf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/nlattr.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/btf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/libbpf_errno.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/str_error.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/netlink.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/bpf_prog_linfo.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/libbpf_probes.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/hashmap.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/btf_dump.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/ringbuf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/strset.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/linker.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/gen_loader.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/relo_core.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/usdt.o
   LD 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/staticobjs/libbpf-in.o
   LINK 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/libbpf.a
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/libbpf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/btf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/libbpf_common.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/libbpf_legacy.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_tracing.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_endian.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_core_read.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/skel_internal.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/libbpf_version.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/usdt.bpf.h
   INSTALL 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helper_defs.h
   INSTALL libbpf_headers
make[1]: Leaving directory '/home/chleroy/linux-powerpc/tools/lib/bpf'
   CC      /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/main.o
   CC      /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/common.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/json_writer.o
   CC      /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/gen.o
   CC      /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/btf.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/xlated_dumper.o
   CC 
/home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/btf_dumper.o
   CC      /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/disasm.o
   LINK    /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/bpftool
   GEN     vmlinux.h
   CLANG   pid_iter.bpf.o
   GEN     pid_iter.skel.h
libbpf: elf: endianness mismatch in pid_iter_bpf.
Error: failed to open BPF object file: Endian mismatch
make: *** [Makefile:222: pid_iter.skel.h] Error 93

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 14:55                   ` Christophe Leroy
@ 2023-01-17 15:42                     ` Quentin Monnet
  2023-01-23  7:57                       ` Christophe Leroy
  0 siblings, 1 reply; 23+ messages in thread
From: Quentin Monnet @ 2023-01-17 15:42 UTC (permalink / raw)
  To: Christophe Leroy, Tonghao Zhang, Andrii Nakryiko
  Cc: Daniel Borkmann, bpf, netdev, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, Hao Luo,
	John Fastabend, Alexei Starovoitov, Song Liu, Stanislav Fomichev,
	Jiri Olsa, Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau,
	naveen.n.rao, mpe

2023-01-17 14:55 UTC+0000 ~ Christophe Leroy <christophe.leroy@csgroup.eu>
> 
> 
> Le 17/01/2023 à 15:41, Quentin Monnet a écrit :
>> [Vous ne recevez pas souvent de courriers de quentin@isovalent.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>>
>> 2023-01-17 14:25 UTC+0000 ~ Christophe Leroy <christophe.leroy@csgroup.eu>
>>>
>>>
>>> Le 17/01/2023 à 15:18, Tonghao Zhang a écrit :
>>>>
>>>>
>>>>> On Jan 17, 2023, at 7:36 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>>
>>>>>
>>>>>
>>>>> Le 17/01/2023 à 08:30, Christophe Leroy a écrit :
>>>>>>
>>>>>>
>>>>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>>>>>
>>>>>>>
>>>>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy
>>>>>>>> <christophe.leroy@csgroup.eu> wrote:
>>
>> [...]
>>
>>>>>>>> Sure I will try to test bpftool again in the coming days.
>>>>>>>>
>>>>>>>> Previous discussion about that subject is here:
>>>>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>>
>> Christophe, apologies from dropping the discussion the last time, it
>> seems your last message on that thread didn't make it to my inbox at the
>> time :/. Thanks a lot for looking into that again!
>>
>>>>>>> Hi Christophe
>>>>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021,
>>>>>>> but bpftool can not run on powerpc.
>>>>>>> Now can we fix this issue?
>>>>>>
>>>>>> Hi Tong,
>>>>>>
>>>>>> I have started to look at it but I don't have any fruitfull feedback yet.
>>>>>
>>>>> Hi Again,
>>>>>
>>>>> I tested again, the problem is still the same as one year ago:
>>>>>
>>>>> root@vgoip:~# ./bpftool prog
>>>>> libbpf: elf: endianness mismatch in pid_iter_bpf.
>>>> It seem to be not right ehdr->e_ident[EI_DATA]. Do we can print the real value?
>>>> /*
>>>>    * e_ident[EI_DATA]
>>>>    */
>>>> #define ELFDATANONE     0
>>>> #define ELFDATA2LSB     1
>>>> #define ELFDATA2MSB     2
>>>> #define ELFDATANUM      3
>>>>
>>>> bpf_object__elf_init:
>>>> obj->efile.ehdr = ehdr = elf64_getehdr(elf);
>>>>
>>>>> libbpf: failed to initialize skeleton BPF object 'pid_iter_bpf': -4003
>>>>> Error: failed to open PID iterator skeleton
>>>>>
>>>>> root@vgoip:~# uname -a
>>>>> Linux vgoip 6.2.0-rc3-02596-g1c2c9c13e256 #242 PREEMPT Tue Jan 17
>>>>> 09:36:08 CET 2023 ppc GNU/Linux
>>>> On my pc, elf is little endian.
>>>> # readelf -h tools/bpf/bpftool/pid_iter.bpf.o
>>>> ELF Header:
>>>>     Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>>>>     Class:                             ELF64
>>>>     Data:                              2's complement, little endian # x86_64
>>>>     Version:                           1 (current)
>>>>     OS/ABI:                            UNIX - System V
>>>>     ABI Version:                       0
>>>>     Type:                              REL (Relocatable file)
>>>>     Machine:                           Linux BPF
>>>>     Version:                           0x1
>>>>     Entry point address:               0x0
>>>>     Start of program headers:          0 (bytes into file)
>>>>     Start of section headers:          64832 (bytes into file)
>>>>     Flags:                             0x0
>>>>     Size of this header:               64 (bytes)
>>>>     Size of program headers:           0 (bytes)
>>>>     Number of program headers:         0
>>>>     Size of section headers:           64 (bytes)
>>>>     Number of section headers:         13
>>>>     Section header string table index: 1
>>>>
>>>
>>> Yes, must be something wrong with the build, I get same as you :
>>>
>>> $ LANG= readelf -h pid_iter.bpf.o
>>> ELF Header:
>>>     Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>>>     Class:                             ELF64
>>>     Data:                              2's complement, little endian
>>>     Version:                           1 (current)
>>>     OS/ABI:                            UNIX - System V
>>>     ABI Version:                       0
>>>     Type:                              REL (Relocatable file)
>>>     Machine:                           Linux BPF
>>>     Version:                           0x1
>>>     Entry point address:               0x0
>>>     Start of program headers:          0 (bytes into file)
>>>     Start of section headers:          34704 (bytes into file)
>>>     Flags:                             0x0
>>>     Size of this header:               64 (bytes)
>>>     Size of program headers:           0 (bytes)
>>>     Number of program headers:         0
>>>     Size of section headers:           64 (bytes)
>>>     Number of section headers:         13
>>>     Section header string table index: 1
>>>
>>>
>>> Whereas I expect the same as bpftool I suppose, which is :
>>>
>>> $ LANG= readelf -h bpftool
>>> ELF Header:
>>>     Magic:   7f 45 4c 46 01 02 01 00 00 00 00 00 00 00 00 00
>>>     Class:                             ELF32
>>>     Data:                              2's complement, big endian
>>>     Version:                           1 (current)
>>>     OS/ABI:                            UNIX - System V
>>>     ABI Version:                       0
>>>     Type:                              EXEC (Executable file)
>>>     Machine:                           PowerPC
>>>     Version:                           0x1
>>>     Entry point address:               0x100027d0
>>>     Start of program headers:          52 (bytes into file)
>>>     Start of section headers:          1842896 (bytes into file)
>>>     Flags:                             0x0
>>>     Size of this header:               52 (bytes)
>>>     Size of program headers:           32 (bytes)
>>>     Number of program headers:         9
>>>     Size of section headers:           40 (bytes)
>>>     Number of section headers:         39
>>>     Section header string table index: 38
>>>
>>
>> pid_iter.bpf.o should be generated from that command in bpftool's Makefile:
>>
>>          $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h \
>>                          $(LIBBPF_BOOTSTRAP)
>>                  $(QUIET_CLANG)$(CLANG) \
>>                          -I$(or $(OUTPUT),.) \
>>                          -I$(srctree)/tools/include/uapi/ \
>>                          -I$(LIBBPF_BOOTSTRAP_INCLUDE) \
>>                          -g -O2 -Wall -fno-stack-protector \
>>                          -target bpf -c $< -o $@
>>
>> My understanding is that "-target bpf" is supposed to pick the
>> endianness for the host (see "llc --version | grep bpf". If that's not
>> the case, could you please try to turn that into '-target bpfeb' in the
>> Makefile instead? I'd be curious to see if it helps.
>>
> 
> I guess it cannot work if it picks the endianness for the build host. It 
> should pick the endianness for the target host.
> 
> That's worse it seems with bpfeb : it fails at build :
> 
>    LINK    /home/chleroy/linux-powerpc/tools/bpf/bpftool/bootstrap/bpftool
>    GEN     vmlinux.h
>    CLANG   pid_iter.bpf.o
>    GEN     pid_iter.skel.h
> libbpf: elf: endianness mismatch in pid_iter_bpf.
> Error: failed to open BPF object file: Endian mismatch
> make: *** [Makefile:222: pid_iter.skel.h] Error 93
> 
> 
> Complete build in case it helps:
> 
> $ LANG= make CROSS_COMPILE=ppc-linux- ARCH=powerpc
> 
> Auto-detecting system features:
> ...                         clang-bpf-co-re: [ ^[[32mon^[[m  ]
> ...                                    llvm: [ ^[[31mOFF^[[m ]
> ...                                  libcap: [ ^[[31mOFF^[[m ]
> ...                                  libbfd: [ ^[[31mOFF^[[m ]

[...]

>    CLANG   pid_iter.bpf.o
>    GEN     pid_iter.skel.h
> libbpf: elf: endianness mismatch in pid_iter_bpf.
> Error: failed to open BPF object file: Endian mismatch
> make: *** [Makefile:222: pid_iter.skel.h] Error 93


Right sorry, I forgot you were cross-compiling. As far as I understand,
it seems that bpftool's Makefile picks up the host endianness when
building the skeleton header file and embedding the BPF program in it,
so loading it on the target host doesn't work. We would like it to build
the skeleton with the target endianness instead, but it seems that
libbpf does not support that (referring to
bpf_object__check_endianness() in tools/lib/bpf/libbpf.c).

I don't know if there's a way to make libbpf work here, or plans to add
it (Andrii do you know?).

In the meantime, you could disable the use of skeletons in bpftool, by
removing "clang-bpf-co-re" from FEATURE_TESTS from the Makefile. You
should get a functional binary, which would only miss a few features
(namely, printing the pids of programs holding references to BPF
programs, and the "bpftool prog profile" command).

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 14:22           ` Tonghao Zhang
@ 2023-01-17 15:59             ` Daniel Borkmann
  2023-01-18  2:13               ` Tonghao Zhang
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Borkmann @ 2023-01-17 15:59 UTC (permalink / raw)
  To: Tonghao Zhang, Christophe Leroy
  Cc: bpf, netdev, linux-arm-kernel, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, Hao Luo,
	John Fastabend, Alexei Starovoitov, Andrii Nakryiko, Song Liu,
	Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh, Yonghong Song,
	Martin KaFai Lau, naveen.n.rao, mpe

On 1/17/23 3:22 PM, Tonghao Zhang wrote:
> 
> 
>> On Jan 17, 2023, at 3:30 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>
>>>
>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>
>>>>
>>>>
>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>>>
>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>>> which include subprog:
>>>>>>>
>>>>>>> $ llvm-objdump -d subprog.o
>>>>>>> Disassembly of section .text:
>>>>>>> 0000000000000000 <subprog>:
>>>>>>>           0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>>> = 29114459903653235 ll
>>>>>>>           2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>>           3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>>           4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>>           5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>>           6:       85 00 00 00 06 00 00 00 call 6
>>>>>>>           7:       95 00 00 00 00 00 00 00 exit
>>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>>> 0000000000000000 <entry>:
>>>>>>>           0:       85 10 00 00 ff ff ff ff call -1
>>>>>>>           1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>>           2:       95 00 00 00 00 00 00 00 exit
>>>>>>>
>>>>>>> kernel print message:
>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>>> from=kprobe-load pid=1643
>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>> cc cc cc cc cc
>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>> cc cc cc cc cc
>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>> cc cc cc cc cc
>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>>>
>>>>>>> $ bpf_jit_disasm
>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>>> ffffffffa000c20c + <x>:
>>>>>>>       0:   int3
>>>>>>>       1:   int3
>>>>>>>       2:   int3
>>>>>>>       3:   int3
>>>>>>>       4:   int3
>>>>>>>       5:   int3
>>>>>>>       ...
>>>>>>>
>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>>> header
>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>>> JITed instructions.
>>>>>>
>>>>>> NACK.
>>>>>>
>>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>>> architectures ?
>>>>>>
>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>>>
>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>>> is fixed now, but it needs to be verified.
>>>>>>
>>>>>> So please, before removing a working and usefull feature, make sure
>>>>>> there is an alternative available to it for all architectures in all
>>>>>> configurations.
>>>>>>
>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>>> understand why it fails.
>>>>>
>>>>> If this is actively used by JIT developers and considered useful, I'd be
>>>>> ok to leave it for the time being. Overall goal is to reach feature parity
>>>>> among (at least major arch) JITs and not just have most functionality only
>>>>> available on x86-64 JIT. Could you however check what is not working with
>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>>>
>>>> Sure I will try to test bpftool again in the coming days.
>>>>
>>>> Previous discussion about that subject is here:
>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>>> Hi Christophe
>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
>>> Now can we fix this issue?
>>
>> Hi Tong,
>>
>> I have started to look at it but I don't have any fruitfull feedback yet.
>>
>> In the meantime, were you able to confirm that bpftool can also be used
>> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you
>> tell me how to proceed ?
> Now I do not test, but we can dump the insn after bpf_prog_select_runtime in test_bpf.ko. bpf_map_get_info_by_fd can copy the insn to userspace, but we can
> dump them in test_bpf.ko in the same way.

Issue is that these progs are not consumable from userspace (and therefore not bpftool).
it's just simple bpf_prog_alloc + copy of test insns + bpf_prog_select_runtime() to test
JITs (see generate_filter()). Some of them could be converted over to test_verifier, but
not all might actually pass verifier, iirc. Don't think it's a good idea to allow exposing
them via fd tbh.

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 15:59             ` Daniel Borkmann
@ 2023-01-18  2:13               ` Tonghao Zhang
  2023-01-18  2:21                 ` Alexei Starovoitov
  0 siblings, 1 reply; 23+ messages in thread
From: Tonghao Zhang @ 2023-01-18  2:13 UTC (permalink / raw)
  To: Daniel Borkmann, Christophe Leroy
  Cc: Christophe Leroy, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



> On Jan 17, 2023, at 11:59 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> 
> On 1/17/23 3:22 PM, Tonghao Zhang wrote:
>>> On Jan 17, 2023, at 3:30 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>> 
>>> 
>>> 
>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>> 
>>>> 
>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>>>> 
>>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>>>> which include subprog:
>>>>>>>> 
>>>>>>>> $ llvm-objdump -d subprog.o
>>>>>>>> Disassembly of section .text:
>>>>>>>> 0000000000000000 <subprog>:
>>>>>>>>          0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>>>> = 29114459903653235 ll
>>>>>>>>          2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>>>          3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>>>          4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>>>          5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>>>          6:       85 00 00 00 06 00 00 00 call 6
>>>>>>>>          7:       95 00 00 00 00 00 00 00 exit
>>>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>>>> 0000000000000000 <entry>:
>>>>>>>>          0:       85 10 00 00 ff ff ff ff call -1
>>>>>>>>          1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>>>          2:       95 00 00 00 00 00 00 00 exit
>>>>>>>> 
>>>>>>>> kernel print message:
>>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>>>> from=kprobe-load pid=1643
>>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>> cc cc cc cc cc
>>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>> cc cc cc cc cc
>>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>> cc cc cc cc cc
>>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>>>> 
>>>>>>>> $ bpf_jit_disasm
>>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>>>> ffffffffa000c20c + <x>:
>>>>>>>>      0:   int3
>>>>>>>>      1:   int3
>>>>>>>>      2:   int3
>>>>>>>>      3:   int3
>>>>>>>>      4:   int3
>>>>>>>>      5:   int3
>>>>>>>>      ...
>>>>>>>> 
>>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>>>> header
>>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>>>> JITed instructions.
>>>>>>> 
>>>>>>> NACK.
>>>>>>> 
>>>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>>>> architectures ?
>>>>>>> 
>>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>>>> 
>>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>>>> is fixed now, but it needs to be verified.
>>>>>>> 
>>>>>>> So please, before removing a working and usefull feature, make sure
>>>>>>> there is an alternative available to it for all architectures in all
>>>>>>> configurations.
>>>>>>> 
>>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>>>> understand why it fails.
>>>>>> 
>>>>>> If this is actively used by JIT developers and considered useful, I'd be
>>>>>> ok to leave it for the time being. Overall goal is to reach feature parity
>>>>>> among (at least major arch) JITs and not just have most functionality only
>>>>>> available on x86-64 JIT. Could you however check what is not working with
>>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>>>> 
>>>>> Sure I will try to test bpftool again in the coming days.
>>>>> 
>>>>> Previous discussion about that subject is here:
>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>>>> Hi Christophe
>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
>>>> Now can we fix this issue?
>>> 
>>> Hi Tong,
>>> 
>>> I have started to look at it but I don't have any fruitfull feedback yet.
>>> 
>>> In the meantime, were you able to confirm that bpftool can also be used
>>> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you
>>> tell me how to proceed ?
>> Now I do not test, but we can dump the insn after bpf_prog_select_runtime in test_bpf.ko. bpf_map_get_info_by_fd can copy the insn to userspace, but we can
>> dump them in test_bpf.ko in the same way.
> 
> Issue is that these progs are not consumable from userspace (and therefore not bpftool).
> it's just simple bpf_prog_alloc + copy of test insns + bpf_prog_select_runtime() to test
> JITs (see generate_filter()). Some of them could be converted over to test_verifier, but
> not all might actually pass verifier, iirc. Don't think it's a good idea to allow exposing
> them via fd tbh.
Hi
I mean that, can we invoke the bpf_jit_dump in test_bpf.ko directly ?. bpf_prog_get_info_by_fd copy the insn to userspace, but we only dump insn in test_bpf.ko

                if (bpf_dump_raw_ok(file->f_cred)) {// code copied from bpf_prog_get_info_by_fd, not tested

                        /* for multi-function programs, copy the JITed
                         * instructions for all the functions
                         */
                        if (prog->aux->func_cnt) {
                                for (i = 0; i < prog->aux->func_cnt; i++) {
                                        len = prog->aux->func[i]->jited_len;
                                        img = (u8 *) prog->aux->func[i]->bpf_func;
                                        bpf_jit_dump(1, len, 1, img);
                                }
                        } else {
                                bpf_jit_dump(1, ulen, 1, prog->bpf_func);
                        }
                }


 


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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-18  2:13               ` Tonghao Zhang
@ 2023-01-18  2:21                 ` Alexei Starovoitov
  2023-01-18  7:35                   ` Christophe Leroy
  0 siblings, 1 reply; 23+ messages in thread
From: Alexei Starovoitov @ 2023-01-18  2:21 UTC (permalink / raw)
  To: Tonghao Zhang
  Cc: Daniel Borkmann, Christophe Leroy, bpf, netdev, linux-arm-kernel,
	loongarch, linux-mips, linuxppc-dev, linux-riscv, linux-s390,
	sparclinux, Hao Luo, John Fastabend, Alexei Starovoitov,
	Andrii Nakryiko, Song Liu, Stanislav Fomichev, Jiri Olsa,
	Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau, naveen.n.rao,
	mpe

On Tue, Jan 17, 2023 at 6:13 PM Tonghao Zhang <tong@infragraf.org> wrote:
>
>
>
> > On Jan 17, 2023, at 11:59 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 1/17/23 3:22 PM, Tonghao Zhang wrote:
> >>> On Jan 17, 2023, at 3:30 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> >>>
> >>>
> >>>
> >>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
> >>>>
> >>>>
> >>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
> >>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
> >>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
> >>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
> >>>>>>>>
> >>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
> >>>>>>>> which include subprog:
> >>>>>>>>
> >>>>>>>> $ llvm-objdump -d subprog.o
> >>>>>>>> Disassembly of section .text:
> >>>>>>>> 0000000000000000 <subprog>:
> >>>>>>>>          0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
> >>>>>>>> = 29114459903653235 ll
> >>>>>>>>          2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> >>>>>>>>          3:       bf a1 00 00 00 00 00 00 r1 = r10
> >>>>>>>>          4:       07 01 00 00 f8 ff ff ff r1 += -8
> >>>>>>>>          5:       b7 02 00 00 08 00 00 00 r2 = 8
> >>>>>>>>          6:       85 00 00 00 06 00 00 00 call 6
> >>>>>>>>          7:       95 00 00 00 00 00 00 00 exit
> >>>>>>>> Disassembly of section raw_tp/sys_enter:
> >>>>>>>> 0000000000000000 <entry>:
> >>>>>>>>          0:       85 10 00 00 ff ff ff ff call -1
> >>>>>>>>          1:       b7 00 00 00 00 00 00 00 r0 = 0
> >>>>>>>>          2:       95 00 00 00 00 00 00 00 exit
> >>>>>>>>
> >>>>>>>> kernel print message:
> >>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
> >>>>>>>> from=kprobe-load pid=1643
> >>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
> >>>>>>>> cc cc cc cc cc
> >>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
> >>>>>>>> cc cc cc cc cc
> >>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
> >>>>>>>> cc cc cc cc cc
> >>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
> >>>>>>>>
> >>>>>>>> $ bpf_jit_disasm
> >>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
> >>>>>>>> ffffffffa000c20c + <x>:
> >>>>>>>>      0:   int3
> >>>>>>>>      1:   int3
> >>>>>>>>      2:   int3
> >>>>>>>>      3:   int3
> >>>>>>>>      4:   int3
> >>>>>>>>      5:   int3
> >>>>>>>>      ...
> >>>>>>>>
> >>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
> >>>>>>>> header
> >>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
> >>>>>>>> JITed instructions.
> >>>>>>>
> >>>>>>> NACK.
> >>>>>>>
> >>>>>>> Because the feature is buggy on x86_64, you remove it for all
> >>>>>>> architectures ?
> >>>>>>>
> >>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
> >>>>>>>
> >>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
> >>>>>>> remember the details, I think it was an issue with endianess. Maybe it
> >>>>>>> is fixed now, but it needs to be verified.
> >>>>>>>
> >>>>>>> So please, before removing a working and usefull feature, make sure
> >>>>>>> there is an alternative available to it for all architectures in all
> >>>>>>> configurations.
> >>>>>>>
> >>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
> >>>>>>> That's vital when a selftest fails if you want to have a chance to
> >>>>>>> understand why it fails.
> >>>>>>
> >>>>>> If this is actively used by JIT developers and considered useful, I'd be
> >>>>>> ok to leave it for the time being. Overall goal is to reach feature parity
> >>>>>> among (at least major arch) JITs and not just have most functionality only
> >>>>>> available on x86-64 JIT. Could you however check what is not working with
> >>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
> >>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
> >>>>>
> >>>>> Sure I will try to test bpftool again in the coming days.
> >>>>>
> >>>>> Previous discussion about that subject is here:
> >>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
> >>>> Hi Christophe
> >>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
> >>>> Now can we fix this issue?
> >>>
> >>> Hi Tong,
> >>>
> >>> I have started to look at it but I don't have any fruitfull feedback yet.
> >>>
> >>> In the meantime, were you able to confirm that bpftool can also be used
> >>> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you
> >>> tell me how to proceed ?
> >> Now I do not test, but we can dump the insn after bpf_prog_select_runtime in test_bpf.ko. bpf_map_get_info_by_fd can copy the insn to userspace, but we can
> >> dump them in test_bpf.ko in the same way.
> >
> > Issue is that these progs are not consumable from userspace (and therefore not bpftool).
> > it's just simple bpf_prog_alloc + copy of test insns + bpf_prog_select_runtime() to test
> > JITs (see generate_filter()). Some of them could be converted over to test_verifier, but
> > not all might actually pass verifier, iirc. Don't think it's a good idea to allow exposing
> > them via fd tbh.
> Hi
> I mean that, can we invoke the bpf_jit_dump in test_bpf.ko directly ?. bpf_prog_get_info_by_fd copy the insn to userspace, but we only dump insn in test_bpf.ko
>
>                 if (bpf_dump_raw_ok(file->f_cred)) {// code copied from bpf_prog_get_info_by_fd, not tested
>
>                         /* for multi-function programs, copy the JITed
>                          * instructions for all the functions
>                          */
>                         if (prog->aux->func_cnt) {
>                                 for (i = 0; i < prog->aux->func_cnt; i++) {
>                                         len = prog->aux->func[i]->jited_len;
>                                         img = (u8 *) prog->aux->func[i]->bpf_func;
>                                         bpf_jit_dump(1, len, 1, img);
>                                 }
>                         } else {
>                                 bpf_jit_dump(1, ulen, 1, prog->bpf_func);
>                         }
>                 }

Let's not reinvent the wheel.
bpftool prog dump jited
is our supported command.
ppc issue with bpftool is related to endianness of embedded skeleton.
which means that none of the bpftool prog commands work on ppc.
It's a bigger issue to address with cross compilation of bpftool.

bpftool supports gnu and llvm disassembler. It retrieves and
prints BTF, line info and source code along with asm.
The user experience is at different level comparing to bpf_jit_dump.

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-18  2:21                 ` Alexei Starovoitov
@ 2023-01-18  7:35                   ` Christophe Leroy
  2023-01-18 17:42                     ` Alexei Starovoitov
  0 siblings, 1 reply; 23+ messages in thread
From: Christophe Leroy @ 2023-01-18  7:35 UTC (permalink / raw)
  To: Alexei Starovoitov, Tonghao Zhang
  Cc: Daniel Borkmann, bpf, netdev, linux-arm-kernel, loongarch,
	linux-mips, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
	Hao Luo, John Fastabend, Alexei Starovoitov, Andrii Nakryiko,
	Song Liu, Stanislav Fomichev, Jiri Olsa, Hou Tao, KP Singh,
	Yonghong Song, Martin KaFai Lau, naveen.n.rao, mpe



Le 18/01/2023 à 03:21, Alexei Starovoitov a écrit :
> On Tue, Jan 17, 2023 at 6:13 PM Tonghao Zhang <tong@infragraf.org> wrote:
>>
>>
>>
>>> On Jan 17, 2023, at 11:59 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>>>
>>> On 1/17/23 3:22 PM, Tonghao Zhang wrote:
>>>>> On Jan 17, 2023, at 3:30 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>>
>>>>>
>>>>>
>>>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>>>>
>>>>>>
>>>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>>>>>>
>>>>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>>>>>> which include subprog:
>>>>>>>>>>
>>>>>>>>>> $ llvm-objdump -d subprog.o
>>>>>>>>>> Disassembly of section .text:
>>>>>>>>>> 0000000000000000 <subprog>:
>>>>>>>>>>           0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>>>>>> = 29114459903653235 ll
>>>>>>>>>>           2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>>>>>           3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>>>>>           4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>>>>>           5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>>>>>           6:       85 00 00 00 06 00 00 00 call 6
>>>>>>>>>>           7:       95 00 00 00 00 00 00 00 exit
>>>>>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>>>>>> 0000000000000000 <entry>:
>>>>>>>>>>           0:       85 10 00 00 ff ff ff ff call -1
>>>>>>>>>>           1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>>>>>           2:       95 00 00 00 00 00 00 00 exit
>>>>>>>>>>
>>>>>>>>>> kernel print message:
>>>>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>>>>>> from=kprobe-load pid=1643
>>>>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>>>> cc cc cc cc cc
>>>>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>>>> cc cc cc cc cc
>>>>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>>>> cc cc cc cc cc
>>>>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>>>>>>
>>>>>>>>>> $ bpf_jit_disasm
>>>>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>>>>>> ffffffffa000c20c + <x>:
>>>>>>>>>>       0:   int3
>>>>>>>>>>       1:   int3
>>>>>>>>>>       2:   int3
>>>>>>>>>>       3:   int3
>>>>>>>>>>       4:   int3
>>>>>>>>>>       5:   int3
>>>>>>>>>>       ...
>>>>>>>>>>
>>>>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>>>>>> header
>>>>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>>>>>> JITed instructions.
>>>>>>>>>
>>>>>>>>> NACK.
>>>>>>>>>
>>>>>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>>>>>> architectures ?
>>>>>>>>>
>>>>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>>>>>>
>>>>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>>>>>> is fixed now, but it needs to be verified.
>>>>>>>>>
>>>>>>>>> So please, before removing a working and usefull feature, make sure
>>>>>>>>> there is an alternative available to it for all architectures in all
>>>>>>>>> configurations.
>>>>>>>>>
>>>>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>>>>>> understand why it fails.
>>>>>>>>
>>>>>>>> If this is actively used by JIT developers and considered useful, I'd be
>>>>>>>> ok to leave it for the time being. Overall goal is to reach feature parity
>>>>>>>> among (at least major arch) JITs and not just have most functionality only
>>>>>>>> available on x86-64 JIT. Could you however check what is not working with
>>>>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>>>>>>
>>>>>>> Sure I will try to test bpftool again in the coming days.
>>>>>>>
>>>>>>> Previous discussion about that subject is here:
>>>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>>>>>> Hi Christophe
>>>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
>>>>>> Now can we fix this issue?
>>>>>
>>>>> Hi Tong,
>>>>>
>>>>> I have started to look at it but I don't have any fruitfull feedback yet.
>>>>>
>>>>> In the meantime, were you able to confirm that bpftool can also be used
>>>>> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you
>>>>> tell me how to proceed ?
>>>> Now I do not test, but we can dump the insn after bpf_prog_select_runtime in test_bpf.ko. bpf_map_get_info_by_fd can copy the insn to userspace, but we can
>>>> dump them in test_bpf.ko in the same way.
>>>
>>> Issue is that these progs are not consumable from userspace (and therefore not bpftool).
>>> it's just simple bpf_prog_alloc + copy of test insns + bpf_prog_select_runtime() to test
>>> JITs (see generate_filter()). Some of them could be converted over to test_verifier, but
>>> not all might actually pass verifier, iirc. Don't think it's a good idea to allow exposing
>>> them via fd tbh.
>> Hi
>> I mean that, can we invoke the bpf_jit_dump in test_bpf.ko directly ?. bpf_prog_get_info_by_fd copy the insn to userspace, but we only dump insn in test_bpf.ko
>>
>>                  if (bpf_dump_raw_ok(file->f_cred)) {// code copied from bpf_prog_get_info_by_fd, not tested
>>
>>                          /* for multi-function programs, copy the JITed
>>                           * instructions for all the functions
>>                           */
>>                          if (prog->aux->func_cnt) {
>>                                  for (i = 0; i < prog->aux->func_cnt; i++) {
>>                                          len = prog->aux->func[i]->jited_len;
>>                                          img = (u8 *) prog->aux->func[i]->bpf_func;
>>                                          bpf_jit_dump(1, len, 1, img);
>>                                  }
>>                          } else {
>>                                  bpf_jit_dump(1, ulen, 1, prog->bpf_func);
>>                          }
>>                  }
> 
> Let's not reinvent the wheel.
> bpftool prog dump jited
> is our supported command.
> ppc issue with bpftool is related to endianness of embedded skeleton.
> which means that none of the bpftool prog commands work on ppc.
> It's a bigger issue to address with cross compilation of bpftool.
> 
> bpftool supports gnu and llvm disassembler. It retrieves and
> prints BTF, line info and source code along with asm.
> The user experience is at different level comparing to bpf_jit_dump.

Hi Alexei,

Fair enough, we are going to try and fix bpftool.

But for test_bpf.ko module, how do you use bpftool to dump the BPF tests 
? Even on x86 I have not been able to use bpftool for that until now. 
Can you tell me how you do ?

Thanks
Christophe

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-18  7:35                   ` Christophe Leroy
@ 2023-01-18 17:42                     ` Alexei Starovoitov
  2023-01-23  8:00                       ` Christophe Leroy
  0 siblings, 1 reply; 23+ messages in thread
From: Alexei Starovoitov @ 2023-01-18 17:42 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Tonghao Zhang, Daniel Borkmann, bpf, netdev, linux-arm-kernel,
	loongarch, linux-mips, linuxppc-dev, linux-riscv, linux-s390,
	sparclinux, Hao Luo, John Fastabend, Alexei Starovoitov,
	Andrii Nakryiko, Song Liu, Stanislav Fomichev, Jiri Olsa,
	Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau, naveen.n.rao,
	mpe

On Tue, Jan 17, 2023 at 11:36 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 18/01/2023 à 03:21, Alexei Starovoitov a écrit :
> > On Tue, Jan 17, 2023 at 6:13 PM Tonghao Zhang <tong@infragraf.org> wrote:
> >>
> >>
> >>
> >>> On Jan 17, 2023, at 11:59 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>>
> >>> On 1/17/23 3:22 PM, Tonghao Zhang wrote:
> >>>>> On Jan 17, 2023, at 3:30 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
> >>>>>>
> >>>>>>
> >>>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
> >>>>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
> >>>>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
> >>>>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
> >>>>>>>>>>
> >>>>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
> >>>>>>>>>> which include subprog:
> >>>>>>>>>>
> >>>>>>>>>> $ llvm-objdump -d subprog.o
> >>>>>>>>>> Disassembly of section .text:
> >>>>>>>>>> 0000000000000000 <subprog>:
> >>>>>>>>>>           0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
> >>>>>>>>>> = 29114459903653235 ll
> >>>>>>>>>>           2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
> >>>>>>>>>>           3:       bf a1 00 00 00 00 00 00 r1 = r10
> >>>>>>>>>>           4:       07 01 00 00 f8 ff ff ff r1 += -8
> >>>>>>>>>>           5:       b7 02 00 00 08 00 00 00 r2 = 8
> >>>>>>>>>>           6:       85 00 00 00 06 00 00 00 call 6
> >>>>>>>>>>           7:       95 00 00 00 00 00 00 00 exit
> >>>>>>>>>> Disassembly of section raw_tp/sys_enter:
> >>>>>>>>>> 0000000000000000 <entry>:
> >>>>>>>>>>           0:       85 10 00 00 ff ff ff ff call -1
> >>>>>>>>>>           1:       b7 00 00 00 00 00 00 00 r0 = 0
> >>>>>>>>>>           2:       95 00 00 00 00 00 00 00 exit
> >>>>>>>>>>
> >>>>>>>>>> kernel print message:
> >>>>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
> >>>>>>>>>> from=kprobe-load pid=1643
> >>>>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
> >>>>>>>>>> cc cc cc cc cc
> >>>>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
> >>>>>>>>>> cc cc cc cc cc
> >>>>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
> >>>>>>>>>> cc cc cc cc cc
> >>>>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
> >>>>>>>>>>
> >>>>>>>>>> $ bpf_jit_disasm
> >>>>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
> >>>>>>>>>> ffffffffa000c20c + <x>:
> >>>>>>>>>>       0:   int3
> >>>>>>>>>>       1:   int3
> >>>>>>>>>>       2:   int3
> >>>>>>>>>>       3:   int3
> >>>>>>>>>>       4:   int3
> >>>>>>>>>>       5:   int3
> >>>>>>>>>>       ...
> >>>>>>>>>>
> >>>>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
> >>>>>>>>>> header
> >>>>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
> >>>>>>>>>> JITed instructions.
> >>>>>>>>>
> >>>>>>>>> NACK.
> >>>>>>>>>
> >>>>>>>>> Because the feature is buggy on x86_64, you remove it for all
> >>>>>>>>> architectures ?
> >>>>>>>>>
> >>>>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
> >>>>>>>>>
> >>>>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
> >>>>>>>>> remember the details, I think it was an issue with endianess. Maybe it
> >>>>>>>>> is fixed now, but it needs to be verified.
> >>>>>>>>>
> >>>>>>>>> So please, before removing a working and usefull feature, make sure
> >>>>>>>>> there is an alternative available to it for all architectures in all
> >>>>>>>>> configurations.
> >>>>>>>>>
> >>>>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
> >>>>>>>>> That's vital when a selftest fails if you want to have a chance to
> >>>>>>>>> understand why it fails.
> >>>>>>>>
> >>>>>>>> If this is actively used by JIT developers and considered useful, I'd be
> >>>>>>>> ok to leave it for the time being. Overall goal is to reach feature parity
> >>>>>>>> among (at least major arch) JITs and not just have most functionality only
> >>>>>>>> available on x86-64 JIT. Could you however check what is not working with
> >>>>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
> >>>>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
> >>>>>>>
> >>>>>>> Sure I will try to test bpftool again in the coming days.
> >>>>>>>
> >>>>>>> Previous discussion about that subject is here:
> >>>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
> >>>>>> Hi Christophe
> >>>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
> >>>>>> Now can we fix this issue?
> >>>>>
> >>>>> Hi Tong,
> >>>>>
> >>>>> I have started to look at it but I don't have any fruitfull feedback yet.
> >>>>>
> >>>>> In the meantime, were you able to confirm that bpftool can also be used
> >>>>> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you
> >>>>> tell me how to proceed ?
> >>>> Now I do not test, but we can dump the insn after bpf_prog_select_runtime in test_bpf.ko. bpf_map_get_info_by_fd can copy the insn to userspace, but we can
> >>>> dump them in test_bpf.ko in the same way.
> >>>
> >>> Issue is that these progs are not consumable from userspace (and therefore not bpftool).
> >>> it's just simple bpf_prog_alloc + copy of test insns + bpf_prog_select_runtime() to test
> >>> JITs (see generate_filter()). Some of them could be converted over to test_verifier, but
> >>> not all might actually pass verifier, iirc. Don't think it's a good idea to allow exposing
> >>> them via fd tbh.
> >> Hi
> >> I mean that, can we invoke the bpf_jit_dump in test_bpf.ko directly ?. bpf_prog_get_info_by_fd copy the insn to userspace, but we only dump insn in test_bpf.ko
> >>
> >>                  if (bpf_dump_raw_ok(file->f_cred)) {// code copied from bpf_prog_get_info_by_fd, not tested
> >>
> >>                          /* for multi-function programs, copy the JITed
> >>                           * instructions for all the functions
> >>                           */
> >>                          if (prog->aux->func_cnt) {
> >>                                  for (i = 0; i < prog->aux->func_cnt; i++) {
> >>                                          len = prog->aux->func[i]->jited_len;
> >>                                          img = (u8 *) prog->aux->func[i]->bpf_func;
> >>                                          bpf_jit_dump(1, len, 1, img);
> >>                                  }
> >>                          } else {
> >>                                  bpf_jit_dump(1, ulen, 1, prog->bpf_func);
> >>                          }
> >>                  }
> >
> > Let's not reinvent the wheel.
> > bpftool prog dump jited
> > is our supported command.
> > ppc issue with bpftool is related to endianness of embedded skeleton.
> > which means that none of the bpftool prog commands work on ppc.
> > It's a bigger issue to address with cross compilation of bpftool.
> >
> > bpftool supports gnu and llvm disassembler. It retrieves and
> > prints BTF, line info and source code along with asm.
> > The user experience is at different level comparing to bpf_jit_dump.
>
> Hi Alexei,
>
> Fair enough, we are going to try and fix bpftool.
>
> But for test_bpf.ko module, how do you use bpftool to dump the BPF tests
> ? Even on x86 I have not been able to use bpftool for that until now.
> Can you tell me how you do ?

test_bpf.ko is useful to JIT developers when they're starting
to work on it, but its test coverage is inadequate for real
world bpf usage comparing to selftests/bpf.
Johan Almbladh did some great additions to test_bpf.ko back in 2021.
Since then there wasn't much.

Here it's important to distinguish the target user.
Is it a kernel JIT developer or user space bpf prog developer?
When it's a kernel developer they can easily
add print_hex_dump() in the right places.
That's what I did when I was developing bpf trampoline.
bpf is more than just JIT. There are trampoline, kfuncs, dispatch.
The kernel devs should not add a debug code.
Long ago bpf_jit_enable=2 was useful to user space bpf developers.
They wanted to see how JITed code look like to optimize it and what not.
Now 'perf record' captures bpf asm and annotates it in 'perf report',
so performance analysis problem is solved that way.
bpftool prog dump jit addressed the needs of users and admins who
want to understand what bpf progs are loaded and what are they doing.
Both 'dump jited' and 'dump xlated' are useful for this case.
So bpf_jit_enable=2 remained useful to kernel developers only and
in that sense it become a kernel debug feature for a narrow set of
JIT developers. On x86 bpf_jit_dump() was neglected and broken.
I suspect the other archs will follow the same fate. If not already.
Having a sysctl for kernel developers is not something the kernel
developers should have around. Hence the cleanup of this patch.

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-17 15:42                     ` Quentin Monnet
@ 2023-01-23  7:57                       ` Christophe Leroy
  2023-01-23 10:57                         ` Quentin Monnet
  0 siblings, 1 reply; 23+ messages in thread
From: Christophe Leroy @ 2023-01-23  7:57 UTC (permalink / raw)
  To: Quentin Monnet, Tonghao Zhang, Andrii Nakryiko
  Cc: Daniel Borkmann, bpf, netdev, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, Hao Luo,
	John Fastabend, Alexei Starovoitov, Song Liu, Stanislav Fomichev,
	Jiri Olsa, Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau,
	naveen.n.rao, mpe



Le 17/01/2023 à 16:42, Quentin Monnet a écrit :
> 
> In the meantime, you could disable the use of skeletons in bpftool, by
> removing "clang-bpf-co-re" from FEATURE_TESTS from the Makefile. You
> should get a functional binary, which would only miss a few features
> (namely, printing the pids of programs holding references to BPF
> programs, and the "bpftool prog profile" command).

Ok, with "clang-bpf-co-re" removed, bpftool doesn't complain.

However, does it work at all ?

I started a 'tcpdump', I confirmed with ' bpf_jit_enable == 2' that a 
BPF jitted program is created by tcpdump.

'bptool prog show' and 'bpftool prog list' returns no result.

Christophe

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-18 17:42                     ` Alexei Starovoitov
@ 2023-01-23  8:00                       ` Christophe Leroy
  0 siblings, 0 replies; 23+ messages in thread
From: Christophe Leroy @ 2023-01-23  8:00 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Tonghao Zhang, Daniel Borkmann, bpf, netdev, linux-arm-kernel,
	loongarch, linux-mips, linuxppc-dev, linux-riscv, linux-s390,
	sparclinux, Hao Luo, John Fastabend, Alexei Starovoitov,
	Andrii Nakryiko, Song Liu, Stanislav Fomichev, Jiri Olsa,
	Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau, naveen.n.rao,
	mpe



Le 18/01/2023 à 18:42, Alexei Starovoitov a écrit :
> On Tue, Jan 17, 2023 at 11:36 PM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 18/01/2023 à 03:21, Alexei Starovoitov a écrit :
>>> On Tue, Jan 17, 2023 at 6:13 PM Tonghao Zhang <tong@infragraf.org> wrote:
>>>>
>>>>
>>>>
>>>>> On Jan 17, 2023, at 11:59 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>>>>>
>>>>> On 1/17/23 3:22 PM, Tonghao Zhang wrote:
>>>>>>> On Jan 17, 2023, at 3:30 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le 17/01/2023 à 06:30, Tonghao Zhang a écrit :
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Jan 9, 2023, at 4:15 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le 06/01/2023 à 16:37, Daniel Borkmann a écrit :
>>>>>>>>>> On 1/5/23 6:53 PM, Christophe Leroy wrote:
>>>>>>>>>>> Le 05/01/2023 à 04:06, tong@infragraf.org a écrit :
>>>>>>>>>>>> From: Tonghao Zhang <tong@infragraf.org>
>>>>>>>>>>>>
>>>>>>>>>>>> The x86_64 can't dump the valid insn in this way. A test BPF prog
>>>>>>>>>>>> which include subprog:
>>>>>>>>>>>>
>>>>>>>>>>>> $ llvm-objdump -d subprog.o
>>>>>>>>>>>> Disassembly of section .text:
>>>>>>>>>>>> 0000000000000000 <subprog>:
>>>>>>>>>>>>            0:       18 01 00 00 73 75 62 70 00 00 00 00 72 6f 67 00 r1
>>>>>>>>>>>> = 29114459903653235 ll
>>>>>>>>>>>>            2:       7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
>>>>>>>>>>>>            3:       bf a1 00 00 00 00 00 00 r1 = r10
>>>>>>>>>>>>            4:       07 01 00 00 f8 ff ff ff r1 += -8
>>>>>>>>>>>>            5:       b7 02 00 00 08 00 00 00 r2 = 8
>>>>>>>>>>>>            6:       85 00 00 00 06 00 00 00 call 6
>>>>>>>>>>>>            7:       95 00 00 00 00 00 00 00 exit
>>>>>>>>>>>> Disassembly of section raw_tp/sys_enter:
>>>>>>>>>>>> 0000000000000000 <entry>:
>>>>>>>>>>>>            0:       85 10 00 00 ff ff ff ff call -1
>>>>>>>>>>>>            1:       b7 00 00 00 00 00 00 00 r0 = 0
>>>>>>>>>>>>            2:       95 00 00 00 00 00 00 00 exit
>>>>>>>>>>>>
>>>>>>>>>>>> kernel print message:
>>>>>>>>>>>> [  580.775387] flen=8 proglen=51 pass=3 image=ffffffffa000c20c
>>>>>>>>>>>> from=kprobe-load pid=1643
>>>>>>>>>>>> [  580.777236] JIT code: 00000000: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>>>>>> cc cc cc cc cc
>>>>>>>>>>>> [  580.779037] JIT code: 00000010: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>>>>>> cc cc cc cc cc
>>>>>>>>>>>> [  580.780767] JIT code: 00000020: cc cc cc cc cc cc cc cc cc cc cc
>>>>>>>>>>>> cc cc cc cc cc
>>>>>>>>>>>> [  580.782568] JIT code: 00000030: cc cc cc
>>>>>>>>>>>>
>>>>>>>>>>>> $ bpf_jit_disasm
>>>>>>>>>>>> 51 bytes emitted from JIT compiler (pass:3, flen:8)
>>>>>>>>>>>> ffffffffa000c20c + <x>:
>>>>>>>>>>>>        0:   int3
>>>>>>>>>>>>        1:   int3
>>>>>>>>>>>>        2:   int3
>>>>>>>>>>>>        3:   int3
>>>>>>>>>>>>        4:   int3
>>>>>>>>>>>>        5:   int3
>>>>>>>>>>>>        ...
>>>>>>>>>>>>
>>>>>>>>>>>> Until bpf_jit_binary_pack_finalize is invoked, we copy rw_header to
>>>>>>>>>>>> header
>>>>>>>>>>>> and then image/insn is valid. BTW, we can use the "bpftool prog dump"
>>>>>>>>>>>> JITed instructions.
>>>>>>>>>>>
>>>>>>>>>>> NACK.
>>>>>>>>>>>
>>>>>>>>>>> Because the feature is buggy on x86_64, you remove it for all
>>>>>>>>>>> architectures ?
>>>>>>>>>>>
>>>>>>>>>>> On powerpc bpf_jit_enable == 2 works and is very usefull.
>>>>>>>>>>>
>>>>>>>>>>> Last time I tried to use bpftool on powerpc/32 it didn't work. I don't
>>>>>>>>>>> remember the details, I think it was an issue with endianess. Maybe it
>>>>>>>>>>> is fixed now, but it needs to be verified.
>>>>>>>>>>>
>>>>>>>>>>> So please, before removing a working and usefull feature, make sure
>>>>>>>>>>> there is an alternative available to it for all architectures in all
>>>>>>>>>>> configurations.
>>>>>>>>>>>
>>>>>>>>>>> Also, I don't think bpftool is usable to dump kernel BPF selftests.
>>>>>>>>>>> That's vital when a selftest fails if you want to have a chance to
>>>>>>>>>>> understand why it fails.
>>>>>>>>>>
>>>>>>>>>> If this is actively used by JIT developers and considered useful, I'd be
>>>>>>>>>> ok to leave it for the time being. Overall goal is to reach feature parity
>>>>>>>>>> among (at least major arch) JITs and not just have most functionality only
>>>>>>>>>> available on x86-64 JIT. Could you however check what is not working with
>>>>>>>>>> bpftool on powerpc/32? Perhaps it's not too much effort to just fix it,
>>>>>>>>>> but details would be useful otherwise 'it didn't work' is too fuzzy.
>>>>>>>>>
>>>>>>>>> Sure I will try to test bpftool again in the coming days.
>>>>>>>>>
>>>>>>>>> Previous discussion about that subject is here:
>>>>>>>>> https://patchwork.kernel.org/project/linux-riscv/patch/20210415093250.3391257-1-Jianlin.Lv@arm.com/#24176847=
>>>>>>>> Hi Christophe
>>>>>>>> Any progress? We discuss to deprecate the bpf_jit_enable == 2 in 2021, but bpftool can not run on powerpc.
>>>>>>>> Now can we fix this issue?
>>>>>>>
>>>>>>> Hi Tong,
>>>>>>>
>>>>>>> I have started to look at it but I don't have any fruitfull feedback yet.
>>>>>>>
>>>>>>> In the meantime, were you able to confirm that bpftool can also be used
>>>>>>> to dump jitted tests from test_bpf.ko module on x86_64 ? In that can you
>>>>>>> tell me how to proceed ?
>>>>>> Now I do not test, but we can dump the insn after bpf_prog_select_runtime in test_bpf.ko. bpf_map_get_info_by_fd can copy the insn to userspace, but we can
>>>>>> dump them in test_bpf.ko in the same way.
>>>>>
>>>>> Issue is that these progs are not consumable from userspace (and therefore not bpftool).
>>>>> it's just simple bpf_prog_alloc + copy of test insns + bpf_prog_select_runtime() to test
>>>>> JITs (see generate_filter()). Some of them could be converted over to test_verifier, but
>>>>> not all might actually pass verifier, iirc. Don't think it's a good idea to allow exposing
>>>>> them via fd tbh.
>>>> Hi
>>>> I mean that, can we invoke the bpf_jit_dump in test_bpf.ko directly ?. bpf_prog_get_info_by_fd copy the insn to userspace, but we only dump insn in test_bpf.ko
>>>>
>>>>                   if (bpf_dump_raw_ok(file->f_cred)) {// code copied from bpf_prog_get_info_by_fd, not tested
>>>>
>>>>                           /* for multi-function programs, copy the JITed
>>>>                            * instructions for all the functions
>>>>                            */
>>>>                           if (prog->aux->func_cnt) {
>>>>                                   for (i = 0; i < prog->aux->func_cnt; i++) {
>>>>                                           len = prog->aux->func[i]->jited_len;
>>>>                                           img = (u8 *) prog->aux->func[i]->bpf_func;
>>>>                                           bpf_jit_dump(1, len, 1, img);
>>>>                                   }
>>>>                           } else {
>>>>                                   bpf_jit_dump(1, ulen, 1, prog->bpf_func);
>>>>                           }
>>>>                   }
>>>
>>> Let's not reinvent the wheel.
>>> bpftool prog dump jited
>>> is our supported command.
>>> ppc issue with bpftool is related to endianness of embedded skeleton.
>>> which means that none of the bpftool prog commands work on ppc.
>>> It's a bigger issue to address with cross compilation of bpftool.
>>>
>>> bpftool supports gnu and llvm disassembler. It retrieves and
>>> prints BTF, line info and source code along with asm.
>>> The user experience is at different level comparing to bpf_jit_dump.
>>
>> Hi Alexei,
>>
>> Fair enough, we are going to try and fix bpftool.
>>
>> But for test_bpf.ko module, how do you use bpftool to dump the BPF tests
>> ? Even on x86 I have not been able to use bpftool for that until now.
>> Can you tell me how you do ?
> 
> test_bpf.ko is useful to JIT developers when they're starting
> to work on it, but its test coverage is inadequate for real
> world bpf usage comparing to selftests/bpf.
> Johan Almbladh did some great additions to test_bpf.ko back in 2021.
> Since then there wasn't much.
> 
> Here it's important to distinguish the target user.
> Is it a kernel JIT developer or user space bpf prog developer?
> When it's a kernel developer they can easily
> add print_hex_dump() in the right places.
> That's what I did when I was developing bpf trampoline.
> bpf is more than just JIT. There are trampoline, kfuncs, dispatch.
> The kernel devs should not add a debug code.
> Long ago bpf_jit_enable=2 was useful to user space bpf developers.
> They wanted to see how JITed code look like to optimize it and what not.
> Now 'perf record' captures bpf asm and annotates it in 'perf report',
> so performance analysis problem is solved that way.
> bpftool prog dump jit addressed the needs of users and admins who
> want to understand what bpf progs are loaded and what are they doing.
> Both 'dump jited' and 'dump xlated' are useful for this case.
> So bpf_jit_enable=2 remained useful to kernel developers only and
> in that sense it become a kernel debug feature for a narrow set of
> JIT developers. On x86 bpf_jit_dump() was neglected and broken.
> I suspect the other archs will follow the same fate. If not already.
> Having a sysctl for kernel developers is not something the kernel
> developers should have around. Hence the cleanup of this patch.


Ok, I understand. Then it means that ' bpf_jit_enable == 2' can be 
removed once 'bpftool' properly works for dumping userspace BPF progs.

But for the time being it doesn't seem to work, see my other answer.

Christophe

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

* Re: [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2
  2023-01-23  7:57                       ` Christophe Leroy
@ 2023-01-23 10:57                         ` Quentin Monnet
  0 siblings, 0 replies; 23+ messages in thread
From: Quentin Monnet @ 2023-01-23 10:57 UTC (permalink / raw)
  To: Christophe Leroy, Tonghao Zhang, Andrii Nakryiko
  Cc: Daniel Borkmann, bpf, netdev, loongarch, linux-mips,
	linuxppc-dev, linux-riscv, linux-s390, sparclinux, Hao Luo,
	John Fastabend, Alexei Starovoitov, Song Liu, Stanislav Fomichev,
	Jiri Olsa, Hou Tao, KP Singh, Yonghong Song, Martin KaFai Lau,
	naveen.n.rao, mpe

2023-01-23 07:57 UTC+0000 ~ Christophe Leroy <christophe.leroy@csgroup.eu>
> 
> 
> Le 17/01/2023 à 16:42, Quentin Monnet a écrit :
>>
>> In the meantime, you could disable the use of skeletons in bpftool, by
>> removing "clang-bpf-co-re" from FEATURE_TESTS from the Makefile. You
>> should get a functional binary, which would only miss a few features
>> (namely, printing the pids of programs holding references to BPF
>> programs, and the "bpftool prog profile" command).
> 
> Ok, with "clang-bpf-co-re" removed, bpftool doesn't complain.
> 
> However, does it work at all ?

Yes it does.

> 
> I started a 'tcpdump', I confirmed with ' bpf_jit_enable == 2' that a 
> BPF jitted program is created by tcpdump.
> 
> 'bptool prog show' and 'bpftool prog list' returns no result.

Bpftool works with eBPF, not with the older "classic" BPF (cBPF) used by
tcpdump. You should see programs listed if you load anything eBPF, for
example by using BCC tools, bpftrace, or load an eBPF program any other
way from user space:

	$ echo "int main(void) {return 0;}" | \
		clang -O2 -target bpf -c -o foo.o -x c -
	# bpftool prog load foo.o /sys/fs/bpf/foo type xdp
	# bpftool prog list
	# bpftool prog dump jited name main
	# rm /sys/fs/bpf/foo

I know tcpdump itself can show the cBPF bytecode for its programs, but I
don't know of another way to dump the JIT-ed image for cBPF programs.
Drgn could probably do it, with kernel debug symbols.

Quentin

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

end of thread, other threads:[~2023-01-23 10:58 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-05  3:06 [bpf-next v2] bpf: drop deprecated bpf_jit_enable == 2 tong
2023-01-05  9:36 ` Björn Töpel
2023-01-05 17:53 ` Christophe Leroy
2023-01-06 13:22   ` Tonghao Zhang
2023-01-06 15:37   ` Daniel Borkmann
2023-01-09  8:15     ` Christophe Leroy
2023-01-17  5:30       ` Tonghao Zhang
2023-01-17  7:30         ` Christophe Leroy
2023-01-17 11:36           ` Christophe Leroy
2023-01-17 14:18             ` Tonghao Zhang
2023-01-17 14:25               ` Christophe Leroy
2023-01-17 14:41                 ` Quentin Monnet
2023-01-17 14:55                   ` Christophe Leroy
2023-01-17 15:42                     ` Quentin Monnet
2023-01-23  7:57                       ` Christophe Leroy
2023-01-23 10:57                         ` Quentin Monnet
2023-01-17 14:22           ` Tonghao Zhang
2023-01-17 15:59             ` Daniel Borkmann
2023-01-18  2:13               ` Tonghao Zhang
2023-01-18  2:21                 ` Alexei Starovoitov
2023-01-18  7:35                   ` Christophe Leroy
2023-01-18 17:42                     ` Alexei Starovoitov
2023-01-23  8:00                       ` Christophe Leroy

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).