Re: [PATCH bpf-next 2/4] libbpf: Add helpers for pinning bpf prog through bpf object skeleton

[Daniel Borkmann <daniel@xxxxxxxxxxxxx>]

On 4/23/22 4:00 PM, Yafang Shao wrote:
> Currently there're helpers for allowing to open/load/attach BPF object
> through BPF object skeleton. Let's also add helpers for pinning through
> BPF object skeleton. It could simplify BPF userspace code which wants to
> pin the progs into bpffs.

Please elaborate some more on your use case/rationale for the commit message,
do you have orchestration code that will rely on these specifically?

> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
>   tools/lib/bpf/libbpf.c   | 59 ++++++++++++++++++++++++++++++++++++++++
>   tools/lib/bpf/libbpf.h   |  4 +++
>   tools/lib/bpf/libbpf.map |  2 ++
>   3 files changed, 65 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 13fcf91e9e0e..e7ed6c53c525 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -12726,6 +12726,65 @@ void bpf_object__detach_skeleton(struct bpf_object_skeleton *s)
>   	}
>   }
>   
> +int bpf_object__pin_skeleton_prog(struct bpf_object_skeleton *s,
> +				  const char *path)

These pin the link, not the prog itself, so the func name is a bit misleading? Also,
what if is this needs to be more customized in future? It doesn't seem very generic.

> +{
> +	struct bpf_link *link;
> +	int err;
> +	int i;
> +
> +	if (!s->prog_cnt)
> +		return libbpf_err(-EINVAL);
> +
> +	if (!path)
> +		path = DEFAULT_BPFFS;
> +
> +	for (i = 0; i < s->prog_cnt; i++) {
> +		char buf[PATH_MAX];
> +		int len;
> +
> +		len = snprintf(buf, PATH_MAX, "%s/%s", path, s->progs[i].name);
> +		if (len < 0) {
> +			err = -EINVAL;
> +			goto err_unpin_prog;
> +		} else if (len >= PATH_MAX) {
> +			err = -ENAMETOOLONG;
> +			goto err_unpin_prog;
> +		}
> +
> +		link = *s->progs[i].link;
> +		if (!link) {
> +			err = -EINVAL;
> +			goto err_unpin_prog;
> +		}
> +
> +		err = bpf_link__pin(link, buf);
> +		if (err)
> +			goto err_unpin_prog;
> +	}
> +
> +	return 0;
> +
> +err_unpin_prog:
> +	bpf_object__unpin_skeleton_prog(s);
> +
> +	return libbpf_err(err);
> +}
> +
> +void bpf_object__unpin_skeleton_prog(struct bpf_object_skeleton *s)
> +{
> +	struct bpf_link *link;
> +	int i;
> +
> +	for (i = 0; i < s->prog_cnt; i++) {
> +		link = *s->progs[i].link;
> +		if (!link || !link->pin_path)
> +			continue;
> +
> +		bpf_link__unpin(link);
> +	}
> +}
> +
>   void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s)
>   {
>   	if (!s)
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 3784867811a4..af44b0968cca 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -1427,6 +1427,10 @@ bpf_object__open_skeleton(struct bpf_object_skeleton *s,
>   LIBBPF_API int bpf_object__load_skeleton(struct bpf_object_skeleton *s);
>   LIBBPF_API int bpf_object__attach_skeleton(struct bpf_object_skeleton *s);
>   LIBBPF_API void bpf_object__detach_skeleton(struct bpf_object_skeleton *s);
> +LIBBPF_API int
> +bpf_object__pin_skeleton_prog(struct bpf_object_skeleton *s, const char *path);
> +LIBBPF_API void
> +bpf_object__unpin_skeleton_prog(struct bpf_object_skeleton *s);
>   LIBBPF_API void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s);

Please also add API documentation.

>   struct bpf_var_skeleton {
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 82f6d62176dd..4e3e37b84b3a 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -55,6 +55,8 @@ LIBBPF_0.0.1 {
>   		bpf_object__unload;
>   		bpf_object__unpin_maps;
>   		bpf_object__unpin_programs;
> +		bpf_object__pin_skeleton_prog;
> +		bpf_object__unpin_skeleton_prog;

This would have to go under LIBBPF_0.8.0 if so.

>   		bpf_perf_event_read_simple;
>   		bpf_prog_attach;
>   		bpf_prog_detach;