On Wed, 2022-04-20 at 10:49 -0700, Andrii Nakryiko wrote:
> On Fri, Apr 15, 2022 at 9:30 PM Kui-Feng Lee <kuifeng@xxxxxx> wrote:
> >
> > Make fentry/fexit/fmod_ret as valid attach-types for
> > BPF_LINK_CREATE.
> > Pass a cookie along with BPF_LINK_CREATE requests.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxx>
> > ---
>
> I think logically this patch should be #3 and current patch #3 adding
> cookie to UAPI should go after this. So it would make sense to swap
> them.
This patch modifies UAPI. The current patch #3 set a cookie for
fentry/fexit/fmod_ret, but the value is always 0 (empty).
This patch provides a way to set cookies from the userland.
>
> > include/uapi/linux/bpf.h | 7 +++++++
> > kernel/bpf/syscall.c | 9 +++++++++
> > tools/include/uapi/linux/bpf.h | 7 +++++++
> > 3 files changed, 23 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index a4f557338af7..780be5a8ae39 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1490,6 +1490,13 @@ union bpf_attr {
> > __aligned_u64 addrs;
> > __aligned_u64 cookies;
> > } kprobe_multi;
> > + struct {
> > + /* black box user-provided value
> > passed through
> > + * to BPF program at the execution
> > time and
> > + * accessible through
> > bpf_get_attach_cookie() BPF helper
> > + */
> > + __u64 cookie;
> > + } tracing;
> > };
> > } link_create;
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 966f2d40ae55..ca14b0a2e222 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3189,6 +3189,10 @@ attach_type_to_prog_type(enum
> > bpf_attach_type attach_type)
> > return BPF_PROG_TYPE_SK_LOOKUP;
> > case BPF_XDP:
> > return BPF_PROG_TYPE_XDP;
> > + case BPF_TRACE_FENTRY:
> > + case BPF_TRACE_FEXIT:
> > + case BPF_MODIFY_RETURN:
> > + return BPF_PROG_TYPE_TRACING;
>
> seems like
>
> case BPF_LSM_MAC:
> return BPF_PROG_TYPE_LSM;
>
> is missing?
Should I add cases for all attach types?
I thought it is intentionally to have cases only for supported attach
types. For example, link_create() & bpf_prog_attach() returns earlier
if the returned type of this function is BPF_PROG_TYPE_UNSPEC.
>
>
> Looking at my experiment for cleaning up RAW_TRACEPOINT_OPEN and
> LINK_CREATE, I think I also got rid of tracing_bpf_link_attach()
> altogether and there was extra case for BPF_PROG_TYPE_EXT.
>
> How about this. Given I have an almost ready kernel code and I'd like
> libbpf to use LINK_CREATE if possible in all cases, let me add the
> feature-probing on libbpf side and post it as a separate small patch
> set that you can base your cookie-specific changes on top. That will
> let you concentrate on BPF cookie side and I'll handle the libbpf
> intricacies that are not directly related to your changes?
>
> I'll try to post patches today or tomorrow, so it should not delay
> you much.
Sure! I will send you my branch.
>
>
> > default:
> > return BPF_PROG_TYPE_UNSPEC;
> > }
> > @@ -4254,6 +4258,11 @@ static int tracing_bpf_link_attach(const
> > union bpf_attr *attr, bpfptr_t uattr,
> > attr-
> > >link_create.target_fd,
> > attr-
> > >link_create.target_btf_id,
> > 0);
> > + else if (prog->type == BPF_PROG_TYPE_TRACING)
> > + return bpf_tracing_prog_attach(prog,
> > + 0,
> > + 0,
> > + attr-
> > >link_create.tracing.cookie);
> >
> > return -EINVAL;
> > }
> > diff --git a/tools/include/uapi/linux/bpf.h
> > b/tools/include/uapi/linux/bpf.h
> > index a4f557338af7..780be5a8ae39 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -1490,6 +1490,13 @@ union bpf_attr {
> > __aligned_u64 addrs;
> > __aligned_u64 cookies;
> > } kprobe_multi;
> > + struct {
> > + /* black box user-provided value
> > passed through
> > + * to BPF program at the execution
> > time and
> > + * accessible through
> > bpf_get_attach_cookie() BPF helper
> > + */
> > + __u64 cookie;
> > + } tracing;
> > };
> > } link_create;
> >
> > --
> > 2.30.2
> >
