| Message ID | 20220416042940.656344-5-kuifeng@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Attach a cookie to a tracing program. | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for Kernel LATEST on ubuntu-latest + selftests |
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for Kernel LATEST on z15 + selftests |
| netdev/tree_selection | success | Not a local patch, async |
On Fri, Apr 15, 2022 at 9:30 PM Kui-Feng Lee <kuifeng@fb.com> 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@fb.com> > --- 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. > 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? 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. > 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 >
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@fb.com> 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@fb.com> > > --- > > 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 > >
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; 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;
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@fb.com> --- include/uapi/linux/bpf.h | 7 +++++++ kernel/bpf/syscall.c | 9 +++++++++ tools/include/uapi/linux/bpf.h | 7 +++++++ 3 files changed, 23 insertions(+)