[bpf-next,v1,0/4] Support bpf trampoline for RV64

Message

Pu Lehui Feb. 15, 2023, 1:52 p.m. UTC

BPF trampoline is the critical infrastructure of the bpf
subsystem, acting as a mediator between kernel functions
and BPF programs. Numerous important features, such as
using ebpf program for zero overhead kernel introspection,
rely on this key component. We can't wait to support bpf
trampoline on RV64. Since RV64 does not support ftrace
direct call yet, the current RV64 bpf trampoline is only
used in bpf context.

As most of riscv cpu support unaligned memory accesses,
we temporarily use patch [1] to facilitate testing. The
test results are as follow, and test_verifier with no
new failure ceses.

- fexit_bpf2bpf:OK
- dummy_st_ops:OK
- xdp_bpf2bpf:OK

[1] https://lore.kernel.org/linux-riscv/[e-mail]/

v1:
- Remove the logic of bpf_arch_text_poke supported for
  kernel functions. (Kuohai and Björn)
- Extend patch_text for multiple instructions. (Björn)
- Fix OOB issue when image too big. (Björn)

RFC:
https://lore.kernel.org/bpf/[e-mail]/

Pu Lehui (4):
  riscv: Extend patch_text for multiple instructions
  riscv, bpf: Factor out emit_call for kernel and bpf context
  riscv, bpf: Add bpf_arch_text_poke support for RV64
  riscv, bpf: Add bpf trampoline support for RV64

 arch/riscv/include/asm/patch.h     |   2 +-
 arch/riscv/kernel/patch.c          |  19 +-
 arch/riscv/kernel/probes/kprobes.c |  15 +-
 arch/riscv/net/bpf_jit.h           |   5 +
 arch/riscv/net/bpf_jit_comp64.c    | 437 +++++++++++++++++++++++++++--
 5 files changed, 444 insertions(+), 34 deletions(-)

Comments

Björn Töpel Feb. 15, 2023, 2:45 p.m. UTC | #1

Hi Lehui,

Pu Lehui <[e-mail]> writes:

> BPF trampoline is the critical infrastructure of the bpf
> subsystem, acting as a mediator between kernel functions
> and BPF programs. Numerous important features, such as
> using ebpf program for zero overhead kernel introspection,
> rely on this key component. We can't wait to support bpf
> trampoline on RV64. Since RV64 does not support ftrace
> direct call yet, the current RV64 bpf trampoline is only
> used in bpf context.

Thanks a lot for continuing this work. I agree with you that it's
valuable to have BPF trampoline support, even without proper direct call
support (we'll get there soon). The trampoline enables kfunc calls. On
that note; I don't see that you enable "bpf_jit_supports_kfunc_call()"
anywhere in the series.  With BPF trampoline support, the RISC-V BPF
finally can support kfunc calls!

I'd add the following to bpf_jit_comp64.c:

bool bpf_jit_supports_kfunc_call(void)
{
        return true;
}

:-)

I'll do a review ASAP.


Björn

Pu Lehui Feb. 16, 2023, 1:23 a.m. UTC | #2

On 2023/2/15 22:45, Björn Töpel wrote:
> Hi Lehui,
> 
> Pu Lehui <[e-mail]> writes:
> 
>> BPF trampoline is the critical infrastructure of the bpf
>> subsystem, acting as a mediator between kernel functions
>> and BPF programs. Numerous important features, such as
>> using ebpf program for zero overhead kernel introspection,
>> rely on this key component. We can't wait to support bpf
>> trampoline on RV64. Since RV64 does not support ftrace
>> direct call yet, the current RV64 bpf trampoline is only
>> used in bpf context.
> 
> Thanks a lot for continuing this work. I agree with you that it's
> valuable to have BPF trampoline support, even without proper direct call
> support (we'll get there soon). The trampoline enables kfunc calls. On
> that note; I don't see that you enable "bpf_jit_supports_kfunc_call()"
> anywhere in the series.  With BPF trampoline support, the RISC-V BPF
> finally can support kfunc calls!
> 
> I'd add the following to bpf_jit_comp64.c:
> 

happy to hear that，let's make it more completeable.

> bool bpf_jit_supports_kfunc_call(void)
> {
>          return true;
> }
> 
> :-)
> 
> I'll do a review ASAP.
> 
> 
> Björn

Björn Töpel Feb. 16, 2023, 9:56 a.m. UTC | #3

Pu Lehui <[e-mail]> writes:

> BPF trampoline is the critical infrastructure of the bpf
> subsystem, acting as a mediator between kernel functions
> and BPF programs. Numerous important features, such as
> using ebpf program for zero overhead kernel introspection,
> rely on this key component. We can't wait to support bpf
> trampoline on RV64. Since RV64 does not support ftrace
> direct call yet, the current RV64 bpf trampoline is only
> used in bpf context.
>
> As most of riscv cpu support unaligned memory accesses,
> we temporarily use patch [1] to facilitate testing. The
> test results are as follow, and test_verifier with no
> new failure ceses.
>
> - fexit_bpf2bpf:OK
> - dummy_st_ops:OK
> - xdp_bpf2bpf:OK
>
> [1] https://lore.kernel.org/linux-riscv/[e-mail]/
>
> v1:
> - Remove the logic of bpf_arch_text_poke supported for
>   kernel functions. (Kuohai and Björn)
> - Extend patch_text for multiple instructions. (Björn)
> - Fix OOB issue when image too big. (Björn)

This series is ready to go in as is.

@Palmer I'd like to take this series via the bpf-next tree (as usual),
but note that there are some non-BPF changes as well, related to text
poking.

@Lehui I'd like to see two follow-up patches:

1. Enable kfunc for RV64, by adding:
 | bool bpf_jit_supports_kfunc_call(void)
 | {
 |         return true;
 | }

2. Remove the checkpatch warning on patch 4:
 | WARNING: kfree(NULL) is safe and this check is probably not required
 | #313: FILE: arch/riscv/net/bpf_jit_comp64.c:984:
 | +	if (branches_off)
 | +		kfree(branches_off);


For the series:

Tested-by: Björn Töpel <[e-mail]>
Acked-by: Björn Töpel <[e-mail]>

Daniel Borkmann Feb. 17, 2023, 8:49 p.m. UTC | #4

On 2/16/23 10:56 AM, Björn Töpel wrote:
> Pu Lehui <[e-mail]> writes:
> 
>> BPF trampoline is the critical infrastructure of the bpf
>> subsystem, acting as a mediator between kernel functions
>> and BPF programs. Numerous important features, such as
>> using ebpf program for zero overhead kernel introspection,
>> rely on this key component. We can't wait to support bpf
>> trampoline on RV64. Since RV64 does not support ftrace
>> direct call yet, the current RV64 bpf trampoline is only
>> used in bpf context.
>>
>> As most of riscv cpu support unaligned memory accesses,
>> we temporarily use patch [1] to facilitate testing. The
>> test results are as follow, and test_verifier with no
>> new failure ceses.
>>
>> - fexit_bpf2bpf:OK
>> - dummy_st_ops:OK
>> - xdp_bpf2bpf:OK
>>
>> [1] https://lore.kernel.org/linux-riscv/[e-mail]/
>>
>> v1:
>> - Remove the logic of bpf_arch_text_poke supported for
>>    kernel functions. (Kuohai and Björn)
>> - Extend patch_text for multiple instructions. (Björn)
>> - Fix OOB issue when image too big. (Björn)
> 
> This series is ready to go in as is.

Ok.

> @Palmer I'd like to take this series via the bpf-next tree (as usual),
> but note that there are some non-BPF changes as well, related to text
> poking.
> 
> @Lehui I'd like to see two follow-up patches:
> 
> 1. Enable kfunc for RV64, by adding:
>   | bool bpf_jit_supports_kfunc_call(void)
>   | {
>   |         return true;
>   | }
> 
> 2. Remove the checkpatch warning on patch 4:
>   | WARNING: kfree(NULL) is safe and this check is probably not required
>   | #313: FILE: arch/riscv/net/bpf_jit_comp64.c:984:
>   | +	if (branches_off)
>   | +		kfree(branches_off);
> 
> 
> For the series:
> 
> Tested-by: Björn Töpel <[e-mail]>
> Acked-by: Björn Töpel <[e-mail]>

Thanks, I fixed up issue 2 and cleaned up the commit msgs while applying.
For issue 1, pls send a follow-up.

Pu Lehui Feb. 18, 2023, 1:30 a.m. UTC | #5

On 2023/2/18 4:49, Daniel Borkmann wrote:
> On 2/16/23 10:56 AM, Björn Töpel wrote:
>> Pu Lehui <[e-mail]> writes:
>>
>>> BPF trampoline is the critical infrastructure of the bpf
>>> subsystem, acting as a mediator between kernel functions
>>> and BPF programs. Numerous important features, such as
>>> using ebpf program for zero overhead kernel introspection,
>>> rely on this key component. We can't wait to support bpf
>>> trampoline on RV64. Since RV64 does not support ftrace
>>> direct call yet, the current RV64 bpf trampoline is only
>>> used in bpf context.
>>>
>>> As most of riscv cpu support unaligned memory accesses,
>>> we temporarily use patch [1] to facilitate testing. The
>>> test results are as follow, and test_verifier with no
>>> new failure ceses.
>>>
>>> - fexit_bpf2bpf:OK
>>> - dummy_st_ops:OK
>>> - xdp_bpf2bpf:OK
>>>
>>> [1] 
>>> https://lore.kernel.org/linux-riscv/[e-mail]/
>>>
>>> v1:
>>> - Remove the logic of bpf_arch_text_poke supported for
>>>    kernel functions. (Kuohai and Björn)
>>> - Extend patch_text for multiple instructions. (Björn)
>>> - Fix OOB issue when image too big. (Björn)
>>
>> This series is ready to go in as is.
> 
> Ok.
> 
>> @Palmer I'd like to take this series via the bpf-next tree (as usual),
>> but note that there are some non-BPF changes as well, related to text
>> poking.
>>
>> @Lehui I'd like to see two follow-up patches:
>>
>> 1. Enable kfunc for RV64, by adding:
>>   | bool bpf_jit_supports_kfunc_call(void)
>>   | {
>>   |         return true;
>>   | }
>>
>> 2. Remove the checkpatch warning on patch 4:
>>   | WARNING: kfree(NULL) is safe and this check is probably not required
>>   | #313: FILE: arch/riscv/net/bpf_jit_comp64.c:984:
>>   | +    if (branches_off)
>>   | +        kfree(branches_off);
>>
>>
>> For the series:
>>
>> Tested-by: Björn Töpel <[e-mail]>
>> Acked-by: Björn Töpel <[e-mail]>
> 
> Thanks, I fixed up issue 2 and cleaned up the commit msgs while applying.
> For issue 1, pls send a follow-up.

Thank you both, will handle this.