Re: [RFC PATCH bpf-next 2/5] libbpf: Refactor bpf_program_attach_fd()

[Yonghong Song <yonghong.song@linux.dev>]

On 3/8/24 5:02 PM, Andrii Nakryiko wrote:
> On Tue, Mar 5, 2024 at 12:22 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Refactor function bpf_program_attach_fd() to provide a helper function
>> which has attach_type as one of input parameters. This will make later
>> libbpf change easier to understand.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   tools/lib/bpf/libbpf.c | 18 +++++++++++++-----
>>   1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 6c2979f1b471..97b573516675 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -12151,11 +12151,10 @@ static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_li
>>   }
>>
>>   static struct bpf_link *
>> -bpf_program_attach_fd(const struct bpf_program *prog,
>> -                     int target_fd, const char *target_name,
>> -                     const struct bpf_link_create_opts *opts)
>> +__bpf_program_attach_fd(const struct bpf_program *prog, int target_fd,
> I hope we won't need this patch at all (see my comment on later
> patch), but if we do need to do this refactoring then a) let's not
> used underscore-prefixed names, this is not a common practice in
> libbpf code base and b) I think we can just update all callers of
> bpf_program_attach_fd() to pass attach_type directly, there are just 6
> of them.

I thought about this 'update all callers of bpf_program_attach_fd
' but did not do that since it needs to update all callers. But since
you suggest this I certainly can do this.

>
>> +                       enum bpf_attach_type attach_type, const char *target_name,
>> +                       const struct bpf_link_create_opts *opts)
>>   {
>> -       enum bpf_attach_type attach_type;
>>          char errmsg[STRERR_BUFSIZE];
>>          struct bpf_link *link;
>>          int prog_fd, link_fd;
>> @@ -12171,7 +12170,6 @@ bpf_program_attach_fd(const struct bpf_program *prog,
>>                  return libbpf_err_ptr(-ENOMEM);
>>          link->detach = &bpf_link__detach_fd;
>>
>> -       attach_type = bpf_program__expected_attach_type(prog);
>>          link_fd = bpf_link_create(prog_fd, target_fd, attach_type, opts);
>>          if (link_fd < 0) {
>>                  link_fd = -errno;
>> @@ -12185,6 +12183,16 @@ bpf_program_attach_fd(const struct bpf_program *prog,
>>          return link;
>>   }
>>
>> +static struct bpf_link *
>> +bpf_program_attach_fd(const struct bpf_program *prog,
>> +                     int target_fd, const char *target_name,
>> +                     const struct bpf_link_create_opts *opts)
>> +{
>> +       return __bpf_program_attach_fd(prog, target_fd,
>> +                                      bpf_program__expected_attach_type(prog),
>> +                                      target_name, opts);
>> +}
>> +
>>   struct bpf_link *
>>   bpf_program__attach_cgroup(const struct bpf_program *prog, int cgroup_fd)
>>   {
>> --
>> 2.43.0
>>