diff mbox series

[15/19] libbpf: Add support to link multi func tracing program

Message ID 20210605111034.1810858-16-jolsa@kernel.org (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series x86/ftrace/bpf: Add batch support for direct/tracing attach | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Jiri Olsa June 5, 2021, 11:10 a.m. UTC
Adding support to link multi func tracing program
through link_create interface.

Adding special types for multi func programs:

  fentry.multi
  fexit.multi

so you can define multi func programs like:

  SEC("fentry.multi/bpf_fentry_test*")
  int BPF_PROG(test1, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f)

that defines test1 to be attached to bpf_fentry_test* functions,
and able to attach ip and 6 arguments.

If functions are not specified the program needs to be attached
manually.

Adding new btf id related fields to bpf_link_create_opts and
bpf_link_create to use them.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/bpf.c    | 11 ++++++-
 tools/lib/bpf/bpf.h    |  4 ++-
 tools/lib/bpf/libbpf.c | 72 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 2 deletions(-)

Comments

Yonghong Song June 7, 2021, 5:49 a.m. UTC | #1
On 6/5/21 4:10 AM, Jiri Olsa wrote:
> Adding support to link multi func tracing program
> through link_create interface.
> 
> Adding special types for multi func programs:
> 
>    fentry.multi
>    fexit.multi
> 
> so you can define multi func programs like:
> 
>    SEC("fentry.multi/bpf_fentry_test*")
>    int BPF_PROG(test1, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f)
> 
> that defines test1 to be attached to bpf_fentry_test* functions,
> and able to attach ip and 6 arguments.
> 
> If functions are not specified the program needs to be attached
> manually.
> 
> Adding new btf id related fields to bpf_link_create_opts and
> bpf_link_create to use them.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>   tools/lib/bpf/bpf.c    | 11 ++++++-
>   tools/lib/bpf/bpf.h    |  4 ++-
>   tools/lib/bpf/libbpf.c | 72 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 86dcac44f32f..da892737b522 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -674,7 +674,8 @@ int bpf_link_create(int prog_fd, int target_fd,
>   		    enum bpf_attach_type attach_type,
>   		    const struct bpf_link_create_opts *opts)
>   {
> -	__u32 target_btf_id, iter_info_len;
> +	__u32 target_btf_id, iter_info_len, multi_btf_ids_cnt;
> +	__s32 *multi_btf_ids;
>   	union bpf_attr attr;
>   	int fd;
>   
[...]
> @@ -9584,6 +9597,9 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
>   	if (!name)
>   		return -EINVAL;
>   
> +	if (prog->prog_flags & BPF_F_MULTI_FUNC)
> +		return 0;
> +
>   	for (i = 0; i < ARRAY_SIZE(section_defs); i++) {
>   		if (!section_defs[i].is_attach_btf)
>   			continue;
> @@ -10537,6 +10553,62 @@ static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
>   	return (struct bpf_link *)link;
>   }
>   
> +static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
> +{
> +	char *pattern = prog->sec_name + prog->sec_def->len;
> +	DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> +	enum bpf_attach_type attach_type;
> +	int prog_fd, link_fd, cnt, err;
> +	struct bpf_link *link = NULL;
> +	__s32 *ids = NULL;
> +
> +	prog_fd = bpf_program__fd(prog);
> +	if (prog_fd < 0) {
> +		pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	err = bpf_object__load_vmlinux_btf(prog->obj, true);
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
> +					BTF_KIND_FUNC, &ids);
> +	if (cnt <= 0)
> +		return ERR_PTR(-EINVAL);

In kernel, looks like we support cnt = 0, here we error out.
Should we also error out in the kernel if cnt == 0?

> +
> +	link = calloc(1, sizeof(*link));
> +	if (!link) {
> +		err = -ENOMEM;
> +		goto out_err;
> +	}
> +	link->detach = &bpf_link__detach_fd;
> +
> +	opts.multi_btf_ids = ids;
> +	opts.multi_btf_ids_cnt = cnt;
> +
> +	attach_type = bpf_program__get_expected_attach_type(prog);
> +	link_fd = bpf_link_create(prog_fd, 0, attach_type, &opts);
> +	if (link_fd < 0) {
> +		err = -errno;
> +		goto out_err;
> +	}
> +	link->fd = link_fd;
> +	free(ids);
> +	return link;
> +
> +out_err:
> +	free(link);
> +	free(ids);
> +	return ERR_PTR(err);
> +}
> +
[...]
Jiri Olsa June 7, 2021, 6:28 p.m. UTC | #2
On Sun, Jun 06, 2021 at 10:49:16PM -0700, Yonghong Song wrote:
> 
> 
> On 6/5/21 4:10 AM, Jiri Olsa wrote:
> > Adding support to link multi func tracing program
> > through link_create interface.
> > 
> > Adding special types for multi func programs:
> > 
> >    fentry.multi
> >    fexit.multi
> > 
> > so you can define multi func programs like:
> > 
> >    SEC("fentry.multi/bpf_fentry_test*")
> >    int BPF_PROG(test1, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f)
> > 
> > that defines test1 to be attached to bpf_fentry_test* functions,
> > and able to attach ip and 6 arguments.
> > 
> > If functions are not specified the program needs to be attached
> > manually.
> > 
> > Adding new btf id related fields to bpf_link_create_opts and
> > bpf_link_create to use them.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >   tools/lib/bpf/bpf.c    | 11 ++++++-
> >   tools/lib/bpf/bpf.h    |  4 ++-
> >   tools/lib/bpf/libbpf.c | 72 ++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 85 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index 86dcac44f32f..da892737b522 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -674,7 +674,8 @@ int bpf_link_create(int prog_fd, int target_fd,
> >   		    enum bpf_attach_type attach_type,
> >   		    const struct bpf_link_create_opts *opts)
> >   {
> > -	__u32 target_btf_id, iter_info_len;
> > +	__u32 target_btf_id, iter_info_len, multi_btf_ids_cnt;
> > +	__s32 *multi_btf_ids;
> >   	union bpf_attr attr;
> >   	int fd;
> [...]
> > @@ -9584,6 +9597,9 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
> >   	if (!name)
> >   		return -EINVAL;
> > +	if (prog->prog_flags & BPF_F_MULTI_FUNC)
> > +		return 0;
> > +
> >   	for (i = 0; i < ARRAY_SIZE(section_defs); i++) {
> >   		if (!section_defs[i].is_attach_btf)
> >   			continue;
> > @@ -10537,6 +10553,62 @@ static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
> >   	return (struct bpf_link *)link;
> >   }
> > +static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
> > +{
> > +	char *pattern = prog->sec_name + prog->sec_def->len;
> > +	DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> > +	enum bpf_attach_type attach_type;
> > +	int prog_fd, link_fd, cnt, err;
> > +	struct bpf_link *link = NULL;
> > +	__s32 *ids = NULL;
> > +
> > +	prog_fd = bpf_program__fd(prog);
> > +	if (prog_fd < 0) {
> > +		pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> > +		return ERR_PTR(-EINVAL);
> > +	}
> > +
> > +	err = bpf_object__load_vmlinux_btf(prog->obj, true);
> > +	if (err)
> > +		return ERR_PTR(err);
> > +
> > +	cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
> > +					BTF_KIND_FUNC, &ids);
> > +	if (cnt <= 0)
> > +		return ERR_PTR(-EINVAL);
> 
> In kernel, looks like we support cnt = 0, here we error out.
> Should we also error out in the kernel if cnt == 0?

hum, I'm not what you mean.. what kernel code are you referring to?

thanks,
jirka
Yonghong Song June 7, 2021, 7:42 p.m. UTC | #3
On 6/7/21 11:28 AM, Jiri Olsa wrote:
> On Sun, Jun 06, 2021 at 10:49:16PM -0700, Yonghong Song wrote:
>>
>>
>> On 6/5/21 4:10 AM, Jiri Olsa wrote:
>>> Adding support to link multi func tracing program
>>> through link_create interface.
>>>
>>> Adding special types for multi func programs:
>>>
>>>     fentry.multi
>>>     fexit.multi
>>>
>>> so you can define multi func programs like:
>>>
>>>     SEC("fentry.multi/bpf_fentry_test*")
>>>     int BPF_PROG(test1, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f)
>>>
>>> that defines test1 to be attached to bpf_fentry_test* functions,
>>> and able to attach ip and 6 arguments.
>>>
>>> If functions are not specified the program needs to be attached
>>> manually.
>>>
>>> Adding new btf id related fields to bpf_link_create_opts and
>>> bpf_link_create to use them.
>>>
>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>>> ---
>>>    tools/lib/bpf/bpf.c    | 11 ++++++-
>>>    tools/lib/bpf/bpf.h    |  4 ++-
>>>    tools/lib/bpf/libbpf.c | 72 ++++++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 85 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>>> index 86dcac44f32f..da892737b522 100644
>>> --- a/tools/lib/bpf/bpf.c
>>> +++ b/tools/lib/bpf/bpf.c
>>> @@ -674,7 +674,8 @@ int bpf_link_create(int prog_fd, int target_fd,
>>>    		    enum bpf_attach_type attach_type,
>>>    		    const struct bpf_link_create_opts *opts)
>>>    {
>>> -	__u32 target_btf_id, iter_info_len;
>>> +	__u32 target_btf_id, iter_info_len, multi_btf_ids_cnt;
>>> +	__s32 *multi_btf_ids;
>>>    	union bpf_attr attr;
>>>    	int fd;
>> [...]
>>> @@ -9584,6 +9597,9 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
>>>    	if (!name)
>>>    		return -EINVAL;
>>> +	if (prog->prog_flags & BPF_F_MULTI_FUNC)
>>> +		return 0;
>>> +
>>>    	for (i = 0; i < ARRAY_SIZE(section_defs); i++) {
>>>    		if (!section_defs[i].is_attach_btf)
>>>    			continue;
>>> @@ -10537,6 +10553,62 @@ static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
>>>    	return (struct bpf_link *)link;
>>>    }
>>> +static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
>>> +{
>>> +	char *pattern = prog->sec_name + prog->sec_def->len;
>>> +	DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
>>> +	enum bpf_attach_type attach_type;
>>> +	int prog_fd, link_fd, cnt, err;
>>> +	struct bpf_link *link = NULL;
>>> +	__s32 *ids = NULL;
>>> +
>>> +	prog_fd = bpf_program__fd(prog);
>>> +	if (prog_fd < 0) {
>>> +		pr_warn("prog '%s': can't attach before loaded\n", prog->name);
>>> +		return ERR_PTR(-EINVAL);
>>> +	}
>>> +
>>> +	err = bpf_object__load_vmlinux_btf(prog->obj, true);
>>> +	if (err)
>>> +		return ERR_PTR(err);
>>> +
>>> +	cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
>>> +					BTF_KIND_FUNC, &ids);
>>> +	if (cnt <= 0)
>>> +		return ERR_PTR(-EINVAL);
>>
>> In kernel, looks like we support cnt = 0, here we error out.
>> Should we also error out in the kernel if cnt == 0?
> 
> hum, I'm not what you mean.. what kernel code are you referring to?

I am referring to the following kernel code:

+static int bpf_tracing_multi_attach(struct bpf_prog *prog,
+				    const union bpf_attr *attr)
+{
+	void __user *ubtf_ids = u64_to_user_ptr(attr->link_create.multi_btf_ids);
+	u32 size, i, cnt = attr->link_create.multi_btf_ids_cnt;
+	struct bpf_tracing_multi_link *link = NULL;
+	struct bpf_link_primer link_primer;
+	struct bpf_trampoline *tr = NULL;
+	int err = -EINVAL;
+	u8 nr_args = 0;
+	u32 *btf_ids;
+
+	if (check_multi_prog_type(prog))
+		return -EINVAL;
+
+	size = cnt * sizeof(*btf_ids);
+	btf_ids = kmalloc(size, GFP_USER | __GFP_NOWARN);
+	if (!btf_ids)
+		return -ENOMEM;
+
+	err = -EFAULT;
+	if (ubtf_ids && copy_from_user(btf_ids, ubtf_ids, size))
+		goto out_free;
+
+	link = kzalloc(sizeof(*link), GFP_USER);
+	if (!link)
+		goto out_free;
+
+	for (i = 0; i < cnt; i++) {
+		struct bpf_attach_target_info tgt_info = {};
+
+		err = bpf_check_attach_target(NULL, prog, NULL, btf_ids[i],
+					      &tgt_info);
+		if (err)
+			goto out_free;
+
+		if (ftrace_set_filter_ip(&link->ops, tgt_info.tgt_addr, 0, 0))
+			goto out_free;
+
+		if (nr_args < tgt_info.fmodel.nr_args)
+			nr_args = tgt_info.fmodel.nr_args;
+	}
+
+	tr = bpf_trampoline_multi_alloc();
+	if (!tr)
+		goto out_free;
+
+	bpf_func_model_nargs(&tr->func.model, nr_args);
+
+	err = bpf_trampoline_link_prog(prog, tr);
+	if (err)
+		goto out_free;
+
+	err = register_ftrace_direct_multi(&link->ops, (unsigned long) 
tr->cur_image->image);
+	if (err)
+		goto out_free;
+
+	bpf_link_init(&link->link, BPF_LINK_TYPE_TRACING_MULTI,
+		      &bpf_tracing_multi_link_lops, prog);
+	link->attach_type = prog->expected_attach_type;
+
+	err = bpf_link_prime(&link->link, &link_primer);
+	if (err)
+		goto out_unlink;
+
+	link->tr = tr;
+	/* Take extra ref so we are even with progs added by link_update. */
+	bpf_prog_inc(prog);
+	return bpf_link_settle(&link_primer);
+
+out_unlink:
+	unregister_ftrace_direct_multi(&link->ops);
+out_free:
+	kfree(tr);
+	kfree(btf_ids);
+	kfree(link);
+	return err;
+}
+

Looks like cnt = 0 is okay in bpf_tracing_multi_attach().

> 
> thanks,
> jirka
>
Jiri Olsa June 7, 2021, 8:11 p.m. UTC | #4
On Mon, Jun 07, 2021 at 12:42:51PM -0700, Yonghong Song wrote:

SNIP

> 
> +static int bpf_tracing_multi_attach(struct bpf_prog *prog,
> +				    const union bpf_attr *attr)
> +{
> +	void __user *ubtf_ids = u64_to_user_ptr(attr->link_create.multi_btf_ids);
> +	u32 size, i, cnt = attr->link_create.multi_btf_ids_cnt;
> +	struct bpf_tracing_multi_link *link = NULL;
> +	struct bpf_link_primer link_primer;
> +	struct bpf_trampoline *tr = NULL;
> +	int err = -EINVAL;
> +	u8 nr_args = 0;
> +	u32 *btf_ids;
> +
> +	if (check_multi_prog_type(prog))
> +		return -EINVAL;
> +
> +	size = cnt * sizeof(*btf_ids);
> +	btf_ids = kmalloc(size, GFP_USER | __GFP_NOWARN);
> +	if (!btf_ids)
> +		return -ENOMEM;
> +
> +	err = -EFAULT;
> +	if (ubtf_ids && copy_from_user(btf_ids, ubtf_ids, size))
> +		goto out_free;
> +
> +	link = kzalloc(sizeof(*link), GFP_USER);
> +	if (!link)
> +		goto out_free;
> +
> +	for (i = 0; i < cnt; i++) {
> +		struct bpf_attach_target_info tgt_info = {};
> +
> +		err = bpf_check_attach_target(NULL, prog, NULL, btf_ids[i],
> +					      &tgt_info);
> +		if (err)
> +			goto out_free;
> +
> +		if (ftrace_set_filter_ip(&link->ops, tgt_info.tgt_addr, 0, 0))
> +			goto out_free;
> +
> +		if (nr_args < tgt_info.fmodel.nr_args)
> +			nr_args = tgt_info.fmodel.nr_args;
> +	}
> +
> +	tr = bpf_trampoline_multi_alloc();
> +	if (!tr)
> +		goto out_free;
> +
> +	bpf_func_model_nargs(&tr->func.model, nr_args);
> +
> +	err = bpf_trampoline_link_prog(prog, tr);
> +	if (err)
> +		goto out_free;
> +
> +	err = register_ftrace_direct_multi(&link->ops, (unsigned long)
> tr->cur_image->image);
> +	if (err)
> +		goto out_free;
> +
> +	bpf_link_init(&link->link, BPF_LINK_TYPE_TRACING_MULTI,
> +		      &bpf_tracing_multi_link_lops, prog);
> +	link->attach_type = prog->expected_attach_type;
> +
> +	err = bpf_link_prime(&link->link, &link_primer);
> +	if (err)
> +		goto out_unlink;
> +
> +	link->tr = tr;
> +	/* Take extra ref so we are even with progs added by link_update. */
> +	bpf_prog_inc(prog);
> +	return bpf_link_settle(&link_primer);
> +
> +out_unlink:
> +	unregister_ftrace_direct_multi(&link->ops);
> +out_free:
> +	kfree(tr);
> +	kfree(btf_ids);
> +	kfree(link);
> +	return err;
> +}
> +
> 
> Looks like cnt = 0 is okay in bpf_tracing_multi_attach().

right, we should fail for that with EINVAL, also the
maximum with prog->aux->attach_btf->nr_types at least

thanks,
jirka
Andrii Nakryiko June 9, 2021, 5:34 a.m. UTC | #5
On Sat, Jun 5, 2021 at 4:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support to link multi func tracing program
> through link_create interface.
>
> Adding special types for multi func programs:
>
>   fentry.multi
>   fexit.multi
>
> so you can define multi func programs like:
>
>   SEC("fentry.multi/bpf_fentry_test*")
>   int BPF_PROG(test1, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f)
>
> that defines test1 to be attached to bpf_fentry_test* functions,
> and able to attach ip and 6 arguments.
>
> If functions are not specified the program needs to be attached
> manually.
>
> Adding new btf id related fields to bpf_link_create_opts and
> bpf_link_create to use them.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/bpf.c    | 11 ++++++-
>  tools/lib/bpf/bpf.h    |  4 ++-
>  tools/lib/bpf/libbpf.c | 72 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 86dcac44f32f..da892737b522 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -674,7 +674,8 @@ int bpf_link_create(int prog_fd, int target_fd,
>                     enum bpf_attach_type attach_type,
>                     const struct bpf_link_create_opts *opts)
>  {
> -       __u32 target_btf_id, iter_info_len;
> +       __u32 target_btf_id, iter_info_len, multi_btf_ids_cnt;
> +       __s32 *multi_btf_ids;
>         union bpf_attr attr;
>         int fd;
>
> @@ -687,6 +688,9 @@ int bpf_link_create(int prog_fd, int target_fd,
>         if (iter_info_len && target_btf_id)

here we check that mutually exclusive options are not specified, we
should do the same for multi stuff

>                 return libbpf_err(-EINVAL);
>
> +       multi_btf_ids = OPTS_GET(opts, multi_btf_ids, 0);
> +       multi_btf_ids_cnt = OPTS_GET(opts, multi_btf_ids_cnt, 0);
> +
>         memset(&attr, 0, sizeof(attr));
>         attr.link_create.prog_fd = prog_fd;
>         attr.link_create.target_fd = target_fd;
> @@ -701,6 +705,11 @@ int bpf_link_create(int prog_fd, int target_fd,
>                 attr.link_create.target_btf_id = target_btf_id;
>         }
>
> +       if (multi_btf_ids && multi_btf_ids_cnt) {
> +               attr.link_create.multi_btf_ids = (__u64) multi_btf_ids;
> +               attr.link_create.multi_btf_ids_cnt = multi_btf_ids_cnt;
> +       }
> +
>         fd = sys_bpf(BPF_LINK_CREATE, &attr, sizeof(attr));
>         return libbpf_err_errno(fd);
>  }
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 4f758f8f50cd..2f78b6c34765 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -177,8 +177,10 @@ struct bpf_link_create_opts {
>         union bpf_iter_link_info *iter_info;
>         __u32 iter_info_len;
>         __u32 target_btf_id;
> +       __s32 *multi_btf_ids;

why ids are __s32?..

> +       __u32 multi_btf_ids_cnt;
>  };
> -#define bpf_link_create_opts__last_field target_btf_id
> +#define bpf_link_create_opts__last_field multi_btf_ids_cnt
>
>  LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
>                                enum bpf_attach_type attach_type,
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 65f87cc1220c..bd31de3b6a85 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -228,6 +228,7 @@ struct bpf_sec_def {
>         bool is_attachable;
>         bool is_attach_btf;
>         bool is_sleepable;
> +       bool is_multi_func;
>         attach_fn_t attach_fn;
>  };
>
> @@ -7609,6 +7610,8 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
>
>                 if (prog->sec_def->is_sleepable)
>                         prog->prog_flags |= BPF_F_SLEEPABLE;
> +               if (prog->sec_def->is_multi_func)
> +                       prog->prog_flags |= BPF_F_MULTI_FUNC;
>                 bpf_program__set_type(prog, prog->sec_def->prog_type);
>                 bpf_program__set_expected_attach_type(prog,
>                                 prog->sec_def->expected_attach_type);
> @@ -9070,6 +9073,8 @@ static struct bpf_link *attach_raw_tp(const struct bpf_sec_def *sec,
>                                       struct bpf_program *prog);
>  static struct bpf_link *attach_trace(const struct bpf_sec_def *sec,
>                                      struct bpf_program *prog);
> +static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
> +                                          struct bpf_program *prog);
>  static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec,
>                                    struct bpf_program *prog);
>  static struct bpf_link *attach_iter(const struct bpf_sec_def *sec,
> @@ -9143,6 +9148,14 @@ static const struct bpf_sec_def section_defs[] = {
>                 .attach_fn = attach_iter),
>         SEC_DEF("syscall", SYSCALL,
>                 .is_sleepable = true),
> +       SEC_DEF("fentry.multi/", TRACING,
> +               .expected_attach_type = BPF_TRACE_FENTRY,

BPF_TRACE_MULTI_FENTRY instead of is_multi stuff everywhere?.. Or a
new type of BPF program altogether?

> +               .is_multi_func = true,
> +               .attach_fn = attach_trace_multi),
> +       SEC_DEF("fexit.multi/", TRACING,
> +               .expected_attach_type = BPF_TRACE_FEXIT,
> +               .is_multi_func = true,
> +               .attach_fn = attach_trace_multi),
>         BPF_EAPROG_SEC("xdp_devmap/",           BPF_PROG_TYPE_XDP,
>                                                 BPF_XDP_DEVMAP),
>         BPF_EAPROG_SEC("xdp_cpumap/",           BPF_PROG_TYPE_XDP,
> @@ -9584,6 +9597,9 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
>         if (!name)
>                 return -EINVAL;
>
> +       if (prog->prog_flags & BPF_F_MULTI_FUNC)
> +               return 0;
> +
>         for (i = 0; i < ARRAY_SIZE(section_defs); i++) {
>                 if (!section_defs[i].is_attach_btf)
>                         continue;
> @@ -10537,6 +10553,62 @@ static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
>         return (struct bpf_link *)link;
>  }
>
> +static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
> +{
> +       char *pattern = prog->sec_name + prog->sec_def->len;
> +       DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> +       enum bpf_attach_type attach_type;
> +       int prog_fd, link_fd, cnt, err;
> +       struct bpf_link *link = NULL;
> +       __s32 *ids = NULL;
> +
> +       prog_fd = bpf_program__fd(prog);
> +       if (prog_fd < 0) {
> +               pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       err = bpf_object__load_vmlinux_btf(prog->obj, true);
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
> +                                       BTF_KIND_FUNC, &ids);

I wonder if it would be better to just support a simplified glob
patterns like "prefix*", "*suffix", "exactmatch", and "*substring*"?
That should be sufficient for majority of cases. For the cases where
user needs something more nuanced, they can just construct BTF ID list
with custom code and do manual attach.

> +       if (cnt <= 0)
> +               return ERR_PTR(-EINVAL);
> +
> +       link = calloc(1, sizeof(*link));
> +       if (!link) {
> +               err = -ENOMEM;
> +               goto out_err;
> +       }
> +       link->detach = &bpf_link__detach_fd;
> +
> +       opts.multi_btf_ids = ids;
> +       opts.multi_btf_ids_cnt = cnt;
> +
> +       attach_type = bpf_program__get_expected_attach_type(prog);
> +       link_fd = bpf_link_create(prog_fd, 0, attach_type, &opts);
> +       if (link_fd < 0) {
> +               err = -errno;
> +               goto out_err;
> +       }
> +       link->fd = link_fd;
> +       free(ids);
> +       return link;
> +
> +out_err:
> +       free(link);
> +       free(ids);
> +       return ERR_PTR(err);
> +}
> +
> +static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
> +                                          struct bpf_program *prog)
> +{
> +       return bpf_program__attach_multi(prog);
> +}
> +
>  struct bpf_link *bpf_program__attach_trace(struct bpf_program *prog)
>  {
>         return bpf_program__attach_btf_id(prog);
> --
> 2.31.1
>
Jiri Olsa June 9, 2021, 2:17 p.m. UTC | #6
On Tue, Jun 08, 2021 at 10:34:11PM -0700, Andrii Nakryiko wrote:
> On Sat, Jun 5, 2021 at 4:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support to link multi func tracing program
> > through link_create interface.
> >
> > Adding special types for multi func programs:
> >
> >   fentry.multi
> >   fexit.multi
> >
> > so you can define multi func programs like:
> >
> >   SEC("fentry.multi/bpf_fentry_test*")
> >   int BPF_PROG(test1, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f)
> >
> > that defines test1 to be attached to bpf_fentry_test* functions,
> > and able to attach ip and 6 arguments.
> >
> > If functions are not specified the program needs to be attached
> > manually.
> >
> > Adding new btf id related fields to bpf_link_create_opts and
> > bpf_link_create to use them.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/bpf/bpf.c    | 11 ++++++-
> >  tools/lib/bpf/bpf.h    |  4 ++-
> >  tools/lib/bpf/libbpf.c | 72 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 85 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index 86dcac44f32f..da892737b522 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -674,7 +674,8 @@ int bpf_link_create(int prog_fd, int target_fd,
> >                     enum bpf_attach_type attach_type,
> >                     const struct bpf_link_create_opts *opts)
> >  {
> > -       __u32 target_btf_id, iter_info_len;
> > +       __u32 target_btf_id, iter_info_len, multi_btf_ids_cnt;
> > +       __s32 *multi_btf_ids;
> >         union bpf_attr attr;
> >         int fd;
> >
> > @@ -687,6 +688,9 @@ int bpf_link_create(int prog_fd, int target_fd,
> >         if (iter_info_len && target_btf_id)
> 
> here we check that mutually exclusive options are not specified, we
> should do the same for multi stuff

right, ok

> 
> >                 return libbpf_err(-EINVAL);
> >
> > +       multi_btf_ids = OPTS_GET(opts, multi_btf_ids, 0);
> > +       multi_btf_ids_cnt = OPTS_GET(opts, multi_btf_ids_cnt, 0);
> > +
> >         memset(&attr, 0, sizeof(attr));
> >         attr.link_create.prog_fd = prog_fd;
> >         attr.link_create.target_fd = target_fd;
> > @@ -701,6 +705,11 @@ int bpf_link_create(int prog_fd, int target_fd,
> >                 attr.link_create.target_btf_id = target_btf_id;
> >         }
> >
> > +       if (multi_btf_ids && multi_btf_ids_cnt) {
> > +               attr.link_create.multi_btf_ids = (__u64) multi_btf_ids;
> > +               attr.link_create.multi_btf_ids_cnt = multi_btf_ids_cnt;
> > +       }
> > +
> >         fd = sys_bpf(BPF_LINK_CREATE, &attr, sizeof(attr));
> >         return libbpf_err_errno(fd);
> >  }
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index 4f758f8f50cd..2f78b6c34765 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -177,8 +177,10 @@ struct bpf_link_create_opts {
> >         union bpf_iter_link_info *iter_info;
> >         __u32 iter_info_len;
> >         __u32 target_btf_id;
> > +       __s32 *multi_btf_ids;
> 
> why ids are __s32?..

hum not sure why I did that.. __u32 then

> 
> > +       __u32 multi_btf_ids_cnt;
> >  };
> > -#define bpf_link_create_opts__last_field target_btf_id
> > +#define bpf_link_create_opts__last_field multi_btf_ids_cnt
> >
> >  LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
> >                                enum bpf_attach_type attach_type,
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 65f87cc1220c..bd31de3b6a85 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -228,6 +228,7 @@ struct bpf_sec_def {
> >         bool is_attachable;
> >         bool is_attach_btf;
> >         bool is_sleepable;
> > +       bool is_multi_func;
> >         attach_fn_t attach_fn;
> >  };
> >
> > @@ -7609,6 +7610,8 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
> >
> >                 if (prog->sec_def->is_sleepable)
> >                         prog->prog_flags |= BPF_F_SLEEPABLE;
> > +               if (prog->sec_def->is_multi_func)
> > +                       prog->prog_flags |= BPF_F_MULTI_FUNC;
> >                 bpf_program__set_type(prog, prog->sec_def->prog_type);
> >                 bpf_program__set_expected_attach_type(prog,
> >                                 prog->sec_def->expected_attach_type);
> > @@ -9070,6 +9073,8 @@ static struct bpf_link *attach_raw_tp(const struct bpf_sec_def *sec,
> >                                       struct bpf_program *prog);
> >  static struct bpf_link *attach_trace(const struct bpf_sec_def *sec,
> >                                      struct bpf_program *prog);
> > +static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
> > +                                          struct bpf_program *prog);
> >  static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec,
> >                                    struct bpf_program *prog);
> >  static struct bpf_link *attach_iter(const struct bpf_sec_def *sec,
> > @@ -9143,6 +9148,14 @@ static const struct bpf_sec_def section_defs[] = {
> >                 .attach_fn = attach_iter),
> >         SEC_DEF("syscall", SYSCALL,
> >                 .is_sleepable = true),
> > +       SEC_DEF("fentry.multi/", TRACING,
> > +               .expected_attach_type = BPF_TRACE_FENTRY,
> 
> BPF_TRACE_MULTI_FENTRY instead of is_multi stuff everywhere?.. Or a
> new type of BPF program altogether?
> 
> > +               .is_multi_func = true,
> > +               .attach_fn = attach_trace_multi),
> > +       SEC_DEF("fexit.multi/", TRACING,
> > +               .expected_attach_type = BPF_TRACE_FEXIT,
> > +               .is_multi_func = true,
> > +               .attach_fn = attach_trace_multi),
> >         BPF_EAPROG_SEC("xdp_devmap/",           BPF_PROG_TYPE_XDP,
> >                                                 BPF_XDP_DEVMAP),
> >         BPF_EAPROG_SEC("xdp_cpumap/",           BPF_PROG_TYPE_XDP,
> > @@ -9584,6 +9597,9 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
> >         if (!name)
> >                 return -EINVAL;
> >
> > +       if (prog->prog_flags & BPF_F_MULTI_FUNC)
> > +               return 0;
> > +
> >         for (i = 0; i < ARRAY_SIZE(section_defs); i++) {
> >                 if (!section_defs[i].is_attach_btf)
> >                         continue;
> > @@ -10537,6 +10553,62 @@ static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
> >         return (struct bpf_link *)link;
> >  }
> >
> > +static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
> > +{
> > +       char *pattern = prog->sec_name + prog->sec_def->len;
> > +       DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> > +       enum bpf_attach_type attach_type;
> > +       int prog_fd, link_fd, cnt, err;
> > +       struct bpf_link *link = NULL;
> > +       __s32 *ids = NULL;
> > +
> > +       prog_fd = bpf_program__fd(prog);
> > +       if (prog_fd < 0) {
> > +               pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> > +               return ERR_PTR(-EINVAL);
> > +       }
> > +
> > +       err = bpf_object__load_vmlinux_btf(prog->obj, true);
> > +       if (err)
> > +               return ERR_PTR(err);
> > +
> > +       cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
> > +                                       BTF_KIND_FUNC, &ids);
> 
> I wonder if it would be better to just support a simplified glob
> patterns like "prefix*", "*suffix", "exactmatch", and "*substring*"?
> That should be sufficient for majority of cases. For the cases where
> user needs something more nuanced, they can just construct BTF ID list
> with custom code and do manual attach.

as I wrote earlier the function is just for the purpose of the test,
and we can always do the manual attach

I don't mind adding that simplified matching you described

jirka

> 
> > +       if (cnt <= 0)
> > +               return ERR_PTR(-EINVAL);
> > +
> > +       link = calloc(1, sizeof(*link));
> > +       if (!link) {
> > +               err = -ENOMEM;
> > +               goto out_err;
> > +       }
> > +       link->detach = &bpf_link__detach_fd;
> > +
> > +       opts.multi_btf_ids = ids;
> > +       opts.multi_btf_ids_cnt = cnt;
> > +
> > +       attach_type = bpf_program__get_expected_attach_type(prog);
> > +       link_fd = bpf_link_create(prog_fd, 0, attach_type, &opts);
> > +       if (link_fd < 0) {
> > +               err = -errno;
> > +               goto out_err;
> > +       }
> > +       link->fd = link_fd;
> > +       free(ids);
> > +       return link;
> > +
> > +out_err:
> > +       free(link);
> > +       free(ids);
> > +       return ERR_PTR(err);
> > +}
> > +
> > +static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
> > +                                          struct bpf_program *prog)
> > +{
> > +       return bpf_program__attach_multi(prog);
> > +}
> > +
> >  struct bpf_link *bpf_program__attach_trace(struct bpf_program *prog)
> >  {
> >         return bpf_program__attach_btf_id(prog);
> > --
> > 2.31.1
> >
>
Andrii Nakryiko June 10, 2021, 5:05 p.m. UTC | #7
On Wed, Jun 9, 2021 at 7:17 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Jun 08, 2021 at 10:34:11PM -0700, Andrii Nakryiko wrote:
> > On Sat, Jun 5, 2021 at 4:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding support to link multi func tracing program
> > > through link_create interface.
> > >
> > > Adding special types for multi func programs:
> > >
> > >   fentry.multi
> > >   fexit.multi
> > >
> > > so you can define multi func programs like:
> > >
> > >   SEC("fentry.multi/bpf_fentry_test*")
> > >   int BPF_PROG(test1, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f)
> > >
> > > that defines test1 to be attached to bpf_fentry_test* functions,
> > > and able to attach ip and 6 arguments.
> > >
> > > If functions are not specified the program needs to be attached
> > > manually.
> > >
> > > Adding new btf id related fields to bpf_link_create_opts and
> > > bpf_link_create to use them.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/lib/bpf/bpf.c    | 11 ++++++-
> > >  tools/lib/bpf/bpf.h    |  4 ++-
> > >  tools/lib/bpf/libbpf.c | 72 ++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 85 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > > index 86dcac44f32f..da892737b522 100644
> > > --- a/tools/lib/bpf/bpf.c
> > > +++ b/tools/lib/bpf/bpf.c
> > > @@ -674,7 +674,8 @@ int bpf_link_create(int prog_fd, int target_fd,
> > >                     enum bpf_attach_type attach_type,
> > >                     const struct bpf_link_create_opts *opts)
> > >  {
> > > -       __u32 target_btf_id, iter_info_len;
> > > +       __u32 target_btf_id, iter_info_len, multi_btf_ids_cnt;
> > > +       __s32 *multi_btf_ids;
> > >         union bpf_attr attr;
> > >         int fd;
> > >
> > > @@ -687,6 +688,9 @@ int bpf_link_create(int prog_fd, int target_fd,
> > >         if (iter_info_len && target_btf_id)
> >
> > here we check that mutually exclusive options are not specified, we
> > should do the same for multi stuff
>
> right, ok
>
> >
> > >                 return libbpf_err(-EINVAL);
> > >
> > > +       multi_btf_ids = OPTS_GET(opts, multi_btf_ids, 0);
> > > +       multi_btf_ids_cnt = OPTS_GET(opts, multi_btf_ids_cnt, 0);
> > > +
> > >         memset(&attr, 0, sizeof(attr));
> > >         attr.link_create.prog_fd = prog_fd;
> > >         attr.link_create.target_fd = target_fd;
> > > @@ -701,6 +705,11 @@ int bpf_link_create(int prog_fd, int target_fd,
> > >                 attr.link_create.target_btf_id = target_btf_id;
> > >         }
> > >
> > > +       if (multi_btf_ids && multi_btf_ids_cnt) {
> > > +               attr.link_create.multi_btf_ids = (__u64) multi_btf_ids;
> > > +               attr.link_create.multi_btf_ids_cnt = multi_btf_ids_cnt;
> > > +       }
> > > +
> > >         fd = sys_bpf(BPF_LINK_CREATE, &attr, sizeof(attr));
> > >         return libbpf_err_errno(fd);
> > >  }
> > > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > > index 4f758f8f50cd..2f78b6c34765 100644
> > > --- a/tools/lib/bpf/bpf.h
> > > +++ b/tools/lib/bpf/bpf.h
> > > @@ -177,8 +177,10 @@ struct bpf_link_create_opts {
> > >         union bpf_iter_link_info *iter_info;
> > >         __u32 iter_info_len;
> > >         __u32 target_btf_id;
> > > +       __s32 *multi_btf_ids;
> >
> > why ids are __s32?..
>
> hum not sure why I did that.. __u32 then
>
> >
> > > +       __u32 multi_btf_ids_cnt;
> > >  };
> > > -#define bpf_link_create_opts__last_field target_btf_id
> > > +#define bpf_link_create_opts__last_field multi_btf_ids_cnt
> > >
> > >  LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
> > >                                enum bpf_attach_type attach_type,
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 65f87cc1220c..bd31de3b6a85 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -228,6 +228,7 @@ struct bpf_sec_def {
> > >         bool is_attachable;
> > >         bool is_attach_btf;
> > >         bool is_sleepable;
> > > +       bool is_multi_func;
> > >         attach_fn_t attach_fn;
> > >  };
> > >
> > > @@ -7609,6 +7610,8 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
> > >
> > >                 if (prog->sec_def->is_sleepable)
> > >                         prog->prog_flags |= BPF_F_SLEEPABLE;
> > > +               if (prog->sec_def->is_multi_func)
> > > +                       prog->prog_flags |= BPF_F_MULTI_FUNC;
> > >                 bpf_program__set_type(prog, prog->sec_def->prog_type);
> > >                 bpf_program__set_expected_attach_type(prog,
> > >                                 prog->sec_def->expected_attach_type);
> > > @@ -9070,6 +9073,8 @@ static struct bpf_link *attach_raw_tp(const struct bpf_sec_def *sec,
> > >                                       struct bpf_program *prog);
> > >  static struct bpf_link *attach_trace(const struct bpf_sec_def *sec,
> > >                                      struct bpf_program *prog);
> > > +static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
> > > +                                          struct bpf_program *prog);
> > >  static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec,
> > >                                    struct bpf_program *prog);
> > >  static struct bpf_link *attach_iter(const struct bpf_sec_def *sec,
> > > @@ -9143,6 +9148,14 @@ static const struct bpf_sec_def section_defs[] = {
> > >                 .attach_fn = attach_iter),
> > >         SEC_DEF("syscall", SYSCALL,
> > >                 .is_sleepable = true),
> > > +       SEC_DEF("fentry.multi/", TRACING,
> > > +               .expected_attach_type = BPF_TRACE_FENTRY,
> >
> > BPF_TRACE_MULTI_FENTRY instead of is_multi stuff everywhere?.. Or a
> > new type of BPF program altogether?
> >
> > > +               .is_multi_func = true,
> > > +               .attach_fn = attach_trace_multi),
> > > +       SEC_DEF("fexit.multi/", TRACING,
> > > +               .expected_attach_type = BPF_TRACE_FEXIT,
> > > +               .is_multi_func = true,
> > > +               .attach_fn = attach_trace_multi),
> > >         BPF_EAPROG_SEC("xdp_devmap/",           BPF_PROG_TYPE_XDP,
> > >                                                 BPF_XDP_DEVMAP),
> > >         BPF_EAPROG_SEC("xdp_cpumap/",           BPF_PROG_TYPE_XDP,
> > > @@ -9584,6 +9597,9 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
> > >         if (!name)
> > >                 return -EINVAL;
> > >
> > > +       if (prog->prog_flags & BPF_F_MULTI_FUNC)
> > > +               return 0;
> > > +
> > >         for (i = 0; i < ARRAY_SIZE(section_defs); i++) {
> > >                 if (!section_defs[i].is_attach_btf)
> > >                         continue;
> > > @@ -10537,6 +10553,62 @@ static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
> > >         return (struct bpf_link *)link;
> > >  }
> > >
> > > +static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
> > > +{
> > > +       char *pattern = prog->sec_name + prog->sec_def->len;
> > > +       DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> > > +       enum bpf_attach_type attach_type;
> > > +       int prog_fd, link_fd, cnt, err;
> > > +       struct bpf_link *link = NULL;
> > > +       __s32 *ids = NULL;
> > > +
> > > +       prog_fd = bpf_program__fd(prog);
> > > +       if (prog_fd < 0) {
> > > +               pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> > > +               return ERR_PTR(-EINVAL);
> > > +       }
> > > +
> > > +       err = bpf_object__load_vmlinux_btf(prog->obj, true);
> > > +       if (err)
> > > +               return ERR_PTR(err);
> > > +
> > > +       cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
> > > +                                       BTF_KIND_FUNC, &ids);
> >
> > I wonder if it would be better to just support a simplified glob
> > patterns like "prefix*", "*suffix", "exactmatch", and "*substring*"?
> > That should be sufficient for majority of cases. For the cases where
> > user needs something more nuanced, they can just construct BTF ID list
> > with custom code and do manual attach.
>
> as I wrote earlier the function is just for the purpose of the test,
> and we can always do the manual attach
>
> I don't mind adding that simplified matching you described

I use that in retsnoop and that seems to be simple but flexible enough
for all the purposes, so far. It matches typical file globbing rules
(with extra limitations, of course), so it's also intuitive.

But I still am not sure about making it a public API, because in a lot
of cases you'll want a list of patterns (both allowing and denying
different patterns), so it should be generalized to something like

btf__find_by_glob_kind(btf, allow_patterns, deny_patterns, ids)

which gets pretty unwieldy. I'd start with telling users to just
iterate BTF on their own and apply whatever custom filtering they
need. For simple cases libbpf will just initially support a simple and
single glob filter declaratively (e.g, SEC("fentry.multi/bpf_*")).


>
> jirka
>
> >
> > > +       if (cnt <= 0)
> > > +               return ERR_PTR(-EINVAL);
> > > +
> > > +       link = calloc(1, sizeof(*link));
> > > +       if (!link) {
> > > +               err = -ENOMEM;
> > > +               goto out_err;
> > > +       }
> > > +       link->detach = &bpf_link__detach_fd;
> > > +
> > > +       opts.multi_btf_ids = ids;
> > > +       opts.multi_btf_ids_cnt = cnt;
> > > +
> > > +       attach_type = bpf_program__get_expected_attach_type(prog);
> > > +       link_fd = bpf_link_create(prog_fd, 0, attach_type, &opts);
> > > +       if (link_fd < 0) {
> > > +               err = -errno;
> > > +               goto out_err;
> > > +       }
> > > +       link->fd = link_fd;
> > > +       free(ids);
> > > +       return link;
> > > +
> > > +out_err:
> > > +       free(link);
> > > +       free(ids);
> > > +       return ERR_PTR(err);
> > > +}
> > > +
> > > +static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
> > > +                                          struct bpf_program *prog)
> > > +{
> > > +       return bpf_program__attach_multi(prog);
> > > +}
> > > +
> > >  struct bpf_link *bpf_program__attach_trace(struct bpf_program *prog)
> > >  {
> > >         return bpf_program__attach_btf_id(prog);
> > > --
> > > 2.31.1
> > >
> >
>
Jiri Olsa June 10, 2021, 8:35 p.m. UTC | #8
On Thu, Jun 10, 2021 at 10:05:39AM -0700, Andrii Nakryiko wrote:

SNIP

> > > > +static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
> > > > +{
> > > > +       char *pattern = prog->sec_name + prog->sec_def->len;
> > > > +       DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
> > > > +       enum bpf_attach_type attach_type;
> > > > +       int prog_fd, link_fd, cnt, err;
> > > > +       struct bpf_link *link = NULL;
> > > > +       __s32 *ids = NULL;
> > > > +
> > > > +       prog_fd = bpf_program__fd(prog);
> > > > +       if (prog_fd < 0) {
> > > > +               pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> > > > +               return ERR_PTR(-EINVAL);
> > > > +       }
> > > > +
> > > > +       err = bpf_object__load_vmlinux_btf(prog->obj, true);
> > > > +       if (err)
> > > > +               return ERR_PTR(err);
> > > > +
> > > > +       cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
> > > > +                                       BTF_KIND_FUNC, &ids);
> > >
> > > I wonder if it would be better to just support a simplified glob
> > > patterns like "prefix*", "*suffix", "exactmatch", and "*substring*"?
> > > That should be sufficient for majority of cases. For the cases where
> > > user needs something more nuanced, they can just construct BTF ID list
> > > with custom code and do manual attach.
> >
> > as I wrote earlier the function is just for the purpose of the test,
> > and we can always do the manual attach
> >
> > I don't mind adding that simplified matching you described
> 
> I use that in retsnoop and that seems to be simple but flexible enough
> for all the purposes, so far. It matches typical file globbing rules
> (with extra limitations, of course), so it's also intuitive.
> 
> But I still am not sure about making it a public API, because in a lot
> of cases you'll want a list of patterns (both allowing and denying
> different patterns), so it should be generalized to something like
> 
> btf__find_by_glob_kind(btf, allow_patterns, deny_patterns, ids)
> 
> which gets pretty unwieldy. I'd start with telling users to just
> iterate BTF on their own and apply whatever custom filtering they
> need. For simple cases libbpf will just initially support a simple and
> single glob filter declaratively (e.g, SEC("fentry.multi/bpf_*")).

ok, I'll scan retsnoop and see what I can steal ;-)

jirka
diff mbox series

Patch

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 86dcac44f32f..da892737b522 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -674,7 +674,8 @@  int bpf_link_create(int prog_fd, int target_fd,
 		    enum bpf_attach_type attach_type,
 		    const struct bpf_link_create_opts *opts)
 {
-	__u32 target_btf_id, iter_info_len;
+	__u32 target_btf_id, iter_info_len, multi_btf_ids_cnt;
+	__s32 *multi_btf_ids;
 	union bpf_attr attr;
 	int fd;
 
@@ -687,6 +688,9 @@  int bpf_link_create(int prog_fd, int target_fd,
 	if (iter_info_len && target_btf_id)
 		return libbpf_err(-EINVAL);
 
+	multi_btf_ids = OPTS_GET(opts, multi_btf_ids, 0);
+	multi_btf_ids_cnt = OPTS_GET(opts, multi_btf_ids_cnt, 0);
+
 	memset(&attr, 0, sizeof(attr));
 	attr.link_create.prog_fd = prog_fd;
 	attr.link_create.target_fd = target_fd;
@@ -701,6 +705,11 @@  int bpf_link_create(int prog_fd, int target_fd,
 		attr.link_create.target_btf_id = target_btf_id;
 	}
 
+	if (multi_btf_ids && multi_btf_ids_cnt) {
+		attr.link_create.multi_btf_ids = (__u64) multi_btf_ids;
+		attr.link_create.multi_btf_ids_cnt = multi_btf_ids_cnt;
+	}
+
 	fd = sys_bpf(BPF_LINK_CREATE, &attr, sizeof(attr));
 	return libbpf_err_errno(fd);
 }
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 4f758f8f50cd..2f78b6c34765 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -177,8 +177,10 @@  struct bpf_link_create_opts {
 	union bpf_iter_link_info *iter_info;
 	__u32 iter_info_len;
 	__u32 target_btf_id;
+	__s32 *multi_btf_ids;
+	__u32 multi_btf_ids_cnt;
 };
-#define bpf_link_create_opts__last_field target_btf_id
+#define bpf_link_create_opts__last_field multi_btf_ids_cnt
 
 LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
 			       enum bpf_attach_type attach_type,
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 65f87cc1220c..bd31de3b6a85 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -228,6 +228,7 @@  struct bpf_sec_def {
 	bool is_attachable;
 	bool is_attach_btf;
 	bool is_sleepable;
+	bool is_multi_func;
 	attach_fn_t attach_fn;
 };
 
@@ -7609,6 +7610,8 @@  __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 
 		if (prog->sec_def->is_sleepable)
 			prog->prog_flags |= BPF_F_SLEEPABLE;
+		if (prog->sec_def->is_multi_func)
+			prog->prog_flags |= BPF_F_MULTI_FUNC;
 		bpf_program__set_type(prog, prog->sec_def->prog_type);
 		bpf_program__set_expected_attach_type(prog,
 				prog->sec_def->expected_attach_type);
@@ -9070,6 +9073,8 @@  static struct bpf_link *attach_raw_tp(const struct bpf_sec_def *sec,
 				      struct bpf_program *prog);
 static struct bpf_link *attach_trace(const struct bpf_sec_def *sec,
 				     struct bpf_program *prog);
+static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
+					   struct bpf_program *prog);
 static struct bpf_link *attach_lsm(const struct bpf_sec_def *sec,
 				   struct bpf_program *prog);
 static struct bpf_link *attach_iter(const struct bpf_sec_def *sec,
@@ -9143,6 +9148,14 @@  static const struct bpf_sec_def section_defs[] = {
 		.attach_fn = attach_iter),
 	SEC_DEF("syscall", SYSCALL,
 		.is_sleepable = true),
+	SEC_DEF("fentry.multi/", TRACING,
+		.expected_attach_type = BPF_TRACE_FENTRY,
+		.is_multi_func = true,
+		.attach_fn = attach_trace_multi),
+	SEC_DEF("fexit.multi/", TRACING,
+		.expected_attach_type = BPF_TRACE_FEXIT,
+		.is_multi_func = true,
+		.attach_fn = attach_trace_multi),
 	BPF_EAPROG_SEC("xdp_devmap/",		BPF_PROG_TYPE_XDP,
 						BPF_XDP_DEVMAP),
 	BPF_EAPROG_SEC("xdp_cpumap/",		BPF_PROG_TYPE_XDP,
@@ -9584,6 +9597,9 @@  static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
 	if (!name)
 		return -EINVAL;
 
+	if (prog->prog_flags & BPF_F_MULTI_FUNC)
+		return 0;
+
 	for (i = 0; i < ARRAY_SIZE(section_defs); i++) {
 		if (!section_defs[i].is_attach_btf)
 			continue;
@@ -10537,6 +10553,62 @@  static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
 	return (struct bpf_link *)link;
 }
 
+static struct bpf_link *bpf_program__attach_multi(struct bpf_program *prog)
+{
+	char *pattern = prog->sec_name + prog->sec_def->len;
+	DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
+	enum bpf_attach_type attach_type;
+	int prog_fd, link_fd, cnt, err;
+	struct bpf_link *link = NULL;
+	__s32 *ids = NULL;
+
+	prog_fd = bpf_program__fd(prog);
+	if (prog_fd < 0) {
+		pr_warn("prog '%s': can't attach before loaded\n", prog->name);
+		return ERR_PTR(-EINVAL);
+	}
+
+	err = bpf_object__load_vmlinux_btf(prog->obj, true);
+	if (err)
+		return ERR_PTR(err);
+
+	cnt = btf__find_by_pattern_kind(prog->obj->btf_vmlinux, pattern,
+					BTF_KIND_FUNC, &ids);
+	if (cnt <= 0)
+		return ERR_PTR(-EINVAL);
+
+	link = calloc(1, sizeof(*link));
+	if (!link) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+	link->detach = &bpf_link__detach_fd;
+
+	opts.multi_btf_ids = ids;
+	opts.multi_btf_ids_cnt = cnt;
+
+	attach_type = bpf_program__get_expected_attach_type(prog);
+	link_fd = bpf_link_create(prog_fd, 0, attach_type, &opts);
+	if (link_fd < 0) {
+		err = -errno;
+		goto out_err;
+	}
+	link->fd = link_fd;
+	free(ids);
+	return link;
+
+out_err:
+	free(link);
+	free(ids);
+	return ERR_PTR(err);
+}
+
+static struct bpf_link *attach_trace_multi(const struct bpf_sec_def *sec,
+					   struct bpf_program *prog)
+{
+	return bpf_program__attach_multi(prog);
+}
+
 struct bpf_link *bpf_program__attach_trace(struct bpf_program *prog)
 {
 	return bpf_program__attach_btf_id(prog);