On Mon, 2022-03-21 at 16:18 -0700, Andrii Nakryiko wrote:
> 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.
Do you mean a struct likes like?
struct bpf_tramp_link {
struct bpf_link link;
struct hlist_node tramp_hlist;
u64 cookie;
};
I like this idea since we don't use cookie for every bpf_link.
But, could you give me an example that we don't want a cookie?
