diff mbox series

LoongArch: BPF: Use move_addr() for BPF_PSEUDO_FUNC

Message ID 20250317015755.2760716-1-hengqi.chen@gmail.com (mailing list archive)
State New
Delegated to: BPF
Headers show
Series LoongArch: BPF: Use move_addr() for BPF_PSEUDO_FUNC | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-11 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-19 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-18 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-20 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-43 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-44 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-45 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-49 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-50 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-51 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / GCC BPF / GCC BPF
bpf/vmtest-bpf-next-VM_Test-46 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-47 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-48 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc

Commit Message

Hengqi Chen March 17, 2025, 1:57 a.m. UTC 
Vincent reported that running XDP synproxy program on LoongArch
results in the following error:
    JIT doesn't support bpf-to-bpf calls
With dmesg:
    multi-func JIT bug 1391 != 1390

The root cause is that verifier will refill the imm with the
correct addresses of bpf_calls for BPF_PSEUDO_FUNC instructions
and then run the last pass of JIT. So we generate different JIT
code for the same instruction in two passes (one for placeholder
and one for real address). Let's use move_addr() instead.

See commit 64f50f657572 ("LoongArch, bpf: Use 4 instructions for
 function address in JIT") for a similar fix.

Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
Fixes: bb035ef0cc91 ("LoongArch: BPF: Support mixing bpf2bpf and tailcalls")
Reported-by: Vincent Li <vincent.mc.li@gmail.com>
Closes: https://lore.kernel.org/loongarch/CAK3+h2yfM9FTNiXvEQBkvtuoJrvzmN4c_NZsFXqEk4Cj1tsBNA@mail.gmail.com/T/#u
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 arch/loongarch/net/bpf_jit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Vincent Li March 17, 2025, 2:41 a.m. UTC | #1 
On Sun, Mar 16, 2025 at 6:58 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Vincent reported that running XDP synproxy program on LoongArch
> results in the following error:
>     JIT doesn't support bpf-to-bpf calls
> With dmesg:
>     multi-func JIT bug 1391 != 1390
>
> The root cause is that verifier will refill the imm with the
> correct addresses of bpf_calls for BPF_PSEUDO_FUNC instructions
> and then run the last pass of JIT. So we generate different JIT
> code for the same instruction in two passes (one for placeholder
> and one for real address). Let's use move_addr() instead.
>
> See commit 64f50f657572 ("LoongArch, bpf: Use 4 instructions for
>  function address in JIT") for a similar fix.
>
> Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
> Fixes: bb035ef0cc91 ("LoongArch: BPF: Support mixing bpf2bpf and tailcalls")
> Reported-by: Vincent Li <vincent.mc.li@gmail.com>
> Closes: https://lore.kernel.org/loongarch/CAK3+h2yfM9FTNiXvEQBkvtuoJrvzmN4c_NZsFXqEk4Cj1tsBNA@mail.gmail.com/T/#u
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>  arch/loongarch/net/bpf_jit.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index ea357a3edc09..b25b0bb43428 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -930,7 +930,10 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext
>         {
>                 const u64 imm64 = (u64)(insn + 1)->imm << 32 | (u32)insn->imm;
>
> -               move_imm(ctx, dst, imm64, is32);
> +               if (bpf_pseudo_func(insn))
> +                       move_addr(ctx, dst, imm64);
> +               else
> +                       move_imm(ctx, dst, imm64, is32);
>                 return 1;
>         }
>
> --
> 2.43.5
>

Thanks Hengqi for the quick fix! tested and verified working now.

[root@fedora xdp-tools]# uname -a
Linux fedora 6.14.0-rc5 #2 SMP PREEMPT_DYNAMIC Sun Mar 16 17:16:21 PDT
2025 loongarch64 GNU/Linux

[root@fedora xdp-tools]# ./xdp-loader/xdp-loader load  -vvv lo -m skb
-P 80 -p /sys/fs/bpf/xdp-synproxy-tailcall -n synproxy_tailcall
./xdp-synproxy-tailcall/xdp_synproxy_tailcall.bpf.o
Current rlimit 8388608 already >= minimum 1048576
Loading 1 files on interface 'lo'.
  libbpf: loading object from
./xdp-synproxy-tailcall/xdp_synproxy_tailcall.bpf.o
...
  libbpf: map 'tail_call_tbl': created successfully, fd=4
  libbpf: pinned map '/sys/fs/bpf/xdp-synproxy-tailcall/tail_call_tbl'
  libbpf: found no pinned map to reuse at
'/sys/fs/bpf/xdp-synproxy-tailcall/values'
  libbpf: map 'values': created successfully, fd=5
  libbpf: pinned map '/sys/fs/bpf/xdp-synproxy-tailcall/values'
  libbpf: found no pinned map to reuse at
'/sys/fs/bpf/xdp-synproxy-tailcall/allowed_ports'
  libbpf: map 'allowed_ports': created successfully, fd=6
  libbpf: pinned map '/sys/fs/bpf/xdp-synproxy-tailcall/allowed_ports'
  libbpf: map 'xdp_synp.rodata': created successfully, fd=7
  libbpf: map 'tail_call_tbl': slot [0] set to prog 'syncookie_xdp' fd=65
 libxdp: Loaded XDP program synproxy_tailcall, got fd 66
 libxdp: Duplicated fd 66 to 3 for prog synproxy_tailcall
 libxdp: Replacing XDP fd -1 with 3 on ifindex 1

[root@fedora xdp-tools]# bpftool prog
...
55: xdp  name syncookie_xdp  tag 1426f5e6593da050  gpl
loaded_at 2025-03-16T19:38:49-0700  uid 0
xlated 8392B  jited 6412B  memlock 16384B  map_ids 11,10,12
btf_id 75

56: xdp  name synproxy_tailcall  tag 0433e599459b925f  gpl
loaded_at 2025-03-16T19:38:49-0700  uid 0
xlated 192B  jited 328B  memlock 16384B  map_ids 9,12
btf_id 75
Vincent Li March 17, 2025, 2:58 a.m. UTC | #2 
On Sun, Mar 16, 2025 at 7:41 PM Vincent Li <vincent.mc.li@gmail.com> wrote:
>
> On Sun, Mar 16, 2025 at 6:58 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
> >
> > Vincent reported that running XDP synproxy program on LoongArch
> > results in the following error:
> >     JIT doesn't support bpf-to-bpf calls
> > With dmesg:
> >     multi-func JIT bug 1391 != 1390
> >
> > The root cause is that verifier will refill the imm with the
> > correct addresses of bpf_calls for BPF_PSEUDO_FUNC instructions
> > and then run the last pass of JIT. So we generate different JIT
> > code for the same instruction in two passes (one for placeholder
> > and one for real address). Let's use move_addr() instead.
> >
> > See commit 64f50f657572 ("LoongArch, bpf: Use 4 instructions for
> >  function address in JIT") for a similar fix.
> >
> > Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
> > Fixes: bb035ef0cc91 ("LoongArch: BPF: Support mixing bpf2bpf and tailcalls")
> > Reported-by: Vincent Li <vincent.mc.li@gmail.com>
> > Closes: https://lore.kernel.org/loongarch/CAK3+h2yfM9FTNiXvEQBkvtuoJrvzmN4c_NZsFXqEk4Cj1tsBNA@mail.gmail.com/T/#u
> > Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> > ---
> >  arch/loongarch/net/bpf_jit.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> > index ea357a3edc09..b25b0bb43428 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -930,7 +930,10 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext
> >         {
> >                 const u64 imm64 = (u64)(insn + 1)->imm << 32 | (u32)insn->imm;
> >
> > -               move_imm(ctx, dst, imm64, is32);
> > +               if (bpf_pseudo_func(insn))
> > +                       move_addr(ctx, dst, imm64);
> > +               else
> > +                       move_imm(ctx, dst, imm64, is32);
> >                 return 1;
> >         }
> >
> > --
> > 2.43.5
> >
>
> Thanks Hengqi for the quick fix! tested and verified working now.
>
> [root@fedora xdp-tools]# uname -a
> Linux fedora 6.14.0-rc5 #2 SMP PREEMPT_DYNAMIC Sun Mar 16 17:16:21 PDT
> 2025 loongarch64 GNU/Linux
>
> [root@fedora xdp-tools]# ./xdp-loader/xdp-loader load  -vvv lo -m skb
> -P 80 -p /sys/fs/bpf/xdp-synproxy-tailcall -n synproxy_tailcall
> ./xdp-synproxy-tailcall/xdp_synproxy_tailcall.bpf.o
> Current rlimit 8388608 already >= minimum 1048576
> Loading 1 files on interface 'lo'.
>   libbpf: loading object from
> ./xdp-synproxy-tailcall/xdp_synproxy_tailcall.bpf.o
> ...
>   libbpf: map 'tail_call_tbl': created successfully, fd=4
>   libbpf: pinned map '/sys/fs/bpf/xdp-synproxy-tailcall/tail_call_tbl'
>   libbpf: found no pinned map to reuse at
> '/sys/fs/bpf/xdp-synproxy-tailcall/values'
>   libbpf: map 'values': created successfully, fd=5
>   libbpf: pinned map '/sys/fs/bpf/xdp-synproxy-tailcall/values'
>   libbpf: found no pinned map to reuse at
> '/sys/fs/bpf/xdp-synproxy-tailcall/allowed_ports'
>   libbpf: map 'allowed_ports': created successfully, fd=6
>   libbpf: pinned map '/sys/fs/bpf/xdp-synproxy-tailcall/allowed_ports'
>   libbpf: map 'xdp_synp.rodata': created successfully, fd=7
>   libbpf: map 'tail_call_tbl': slot [0] set to prog 'syncookie_xdp' fd=65
>  libxdp: Loaded XDP program synproxy_tailcall, got fd 66
>  libxdp: Duplicated fd 66 to 3 for prog synproxy_tailcall
>  libxdp: Replacing XDP fd -1 with 3 on ifindex 1
>
> [root@fedora xdp-tools]# bpftool prog
> ...
> 55: xdp  name syncookie_xdp  tag 1426f5e6593da050  gpl
> loaded_at 2025-03-16T19:38:49-0700  uid 0
> xlated 8392B  jited 6412B  memlock 16384B  map_ids 11,10,12
> btf_id 75
>
> 56: xdp  name synproxy_tailcall  tag 0433e599459b925f  gpl
> loaded_at 2025-03-16T19:38:49-0700  uid 0
> xlated 192B  jited 328B  memlock 16384B  map_ids 9,12
> btf_id 75

Sorry missed the Tested-by,  you can add:

Tested-by: Vincent Li <vincent.mc.li@gmail.com>
diff mbox series

Patch

diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index ea357a3edc09..b25b0bb43428 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -930,7 +930,10 @@  static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext
 	{
 		const u64 imm64 = (u64)(insn + 1)->imm << 32 | (u32)insn->imm;
 
-		move_imm(ctx, dst, imm64, is32);
+		if (bpf_pseudo_func(insn))
+			move_addr(ctx, dst, imm64);
+		else
+			move_imm(ctx, dst, imm64, is32);
 		return 1;
 	}