On Mon, 2022-05-09 at 11:58 -0700, Andrii Nakryiko wrote:
> On Sat, May 7, 2022 at 8:21 PM Kui-Feng Lee <kuifeng@xxxxxx> wrote:
> >
> > Pass a cookie along with BPF_LINK_CREATE requests.
> >
> > Add a bpf_cookie field to struct bpf_tracing_link to attach a
> > cookie.
> > The cookie of a bpf_tracing_link is available by calling
> > bpf_get_attach_cookie when running the BPF program of the attached
> > link.
> >
> > The value of a cookie will be set at bpf_tramp_run_ctx by the
> > trampoline of the link.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxx>
> > ---
> > arch/x86/net/bpf_jit_comp.c | 12 ++++++++++--
> > include/linux/bpf.h | 1 +
> > include/uapi/linux/bpf.h | 9 +++++++++
> > kernel/bpf/bpf_lsm.c | 17 +++++++++++++++++
> > kernel/bpf/syscall.c | 12 ++++++++----
> > kernel/bpf/trampoline.c | 7 +++++--
> > kernel/trace/bpf_trace.c | 17 +++++++++++++++++
> > tools/include/uapi/linux/bpf.h | 9 +++++++++
> > 8 files changed, 76 insertions(+), 8 deletions(-)
> >
>
> LGTM with a suggestion for some follow up clean up.
>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> > diff --git a/arch/x86/net/bpf_jit_comp.c
> > b/arch/x86/net/bpf_jit_comp.c
> > index bf4576a6938c..52a5eba2d5e8 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -1764,13 +1764,21 @@ static int invoke_bpf_prog(const struct
> > btf_func_model *m, u8 **pprog,
> > struct bpf_tramp_link *l, int
> > stack_size,
> > bool save_ret)
> > {
> > + u64 cookie = 0;
> > u8 *prog = *pprog;
> > u8 *jmp_insn;
> > int ctx_cookie_off = offsetof(struct bpf_tramp_run_ctx,
> > bpf_cookie);
> > struct bpf_prog *p = l->link.prog;
> >
> > - /* mov rdi, 0 */
> > - emit_mov_imm64(&prog, BPF_REG_1, 0, 0);
> > + if (l->link.type == BPF_LINK_TYPE_TRACING) {
>
> It would probably be nicer to put cookie field into struct
> bpf_tramp_link instead so that the JIT compiler doesn't have to do
> this special handling. It also makes sense that struct bpf_trampoline
> *trampoline is moved into struct bpf_tramp_link itself (given
> trampoline is always there for bpf_tramp_link).
It will increase the size of bpf_tramp_link a little bit, but they are
not used by bpf_struct_ops.
>
> > + struct bpf_tracing_link *tr_link =
> > + container_of(l, struct bpf_tracing_link,
> > link);
> > +
> > + cookie = tr_link->cookie;
> > + }
> > +
> > + /* mov rdi, cookie */
> > + emit_mov_imm64(&prog, BPF_REG_1, (long) cookie >> 32, (u32)
> > (long) cookie);
> >
> > /* Prepare struct bpf_tramp_run_ctx.
> > *
>
> [...]
