Re: [PATCH bpf-next v2 3/4] bpf, x86: Support BPF cookie for fentry/fexit/fmod_ret.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 15, 2022 at 5:44 PM Kui-Feng Lee <kuifeng@xxxxxx> wrote:
>
> Add a bpf_cookie field to attach a cookie to an instance of struct
> bpf_link.  The cookie of a bpf_link will be installed when calling the
> associated program to make it available to the program.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxx>
> ---
>  arch/x86/net/bpf_jit_comp.c    |  4 ++--
>  include/linux/bpf.h            |  1 +
>  include/uapi/linux/bpf.h       |  1 +
>  kernel/bpf/syscall.c           | 11 +++++++----
>  kernel/trace/bpf_trace.c       | 17 +++++++++++++++++
>  tools/include/uapi/linux/bpf.h |  1 +
>  tools/lib/bpf/bpf.c            | 14 ++++++++++++++
>  tools/lib/bpf/bpf.h            |  1 +
>  tools/lib/bpf/libbpf.map       |  1 +
>  9 files changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 29775a475513..5fab8530e909 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -1753,8 +1753,8 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
>
>         EMIT1(0x52);             /* push rdx */
>
> -       /* mov rdi, 0 */
> -       emit_mov_imm64(&prog, BPF_REG_1, 0, 0);
> +       /* mov rdi, cookie */
> +       emit_mov_imm64(&prog, BPF_REG_1, (long) l->cookie >> 32, (u32) (long) l->cookie);

why __u64 to long casting? I don't think you need to cast anything at
all, but if you want to make that more explicit than just casting to
(u32) should be fine, no?

>
>         /* Prepare struct bpf_trace_run_ctx.
>          * sub rsp, sizeof(struct bpf_trace_run_ctx)
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index d20a23953696..9469f9264b4f 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1040,6 +1040,7 @@ struct bpf_link {
>         struct bpf_prog *prog;
>         struct work_struct work;
>         struct hlist_node tramp_hlist;
> +       u64 cookie;

I was a bit hesitant about adding tramp_hlist into generic struct
bpf_link, but now with also cookie there I'm even more convinced that
it's not the right thing to do... Some BPF links won't have cookie,
some (like multi-kprobe) will have lots of them.

Should we create struct bpf_tramp_link {} which will have tramp_hlist
and cookie? As for tramp_hlist, we can probably also keep it back in
bpf_prog_aux and just fetch it through link->prog->aux->tramp_hlist in
trampoline code. This might reduce amount of code churn in patch 1.

Thoughts?

>  };
>
>  struct bpf_link_ops {

[...]



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux