Re: [PATCH bpf-next v2 08/12] tools: libbpf: add extended attributes version of bpf_object__open()

[Andrey Ignatov <rdna@fb.com>]

Jakub Kicinski <jakub.kicinski@netronome.com> [Mon, 2018-07-09 11:01 -0700]:
> Similarly to bpf_prog_load() users of bpf_object__open() may need
> to specify the expected program type.  Program type is needed at
> open to avoid the kernel version check for program types which don't
> require it.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
>  tools/lib/bpf/libbpf.c | 21 +++++++++++++++++----
>  tools/lib/bpf/libbpf.h |  6 ++++++
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index edc3b0b3737d..5b0e84fbcf71 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1520,7 +1520,8 @@ __bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz,
>  	return ERR_PTR(err);
>  }
>  
> -struct bpf_object *bpf_object__open(const char *path)
> +struct bpf_object *bpf_object__open_xattr(const char *path,
> +					  struct bpf_object_open_attr *attr)
>  {
>  	/* param validation */
>  	if (!path)
> @@ -1528,7 +1529,17 @@ struct bpf_object *bpf_object__open(const char *path)
>  
>  	pr_debug("loading %s\n", path);
>  
> -	return __bpf_object__open(path, NULL, 0, true);
> +	return __bpf_object__open(path, NULL, 0,
> +				  bpf_prog_type__needs_kver(attr->prog_type));
> +}
> +
> +struct bpf_object *bpf_object__open(const char *path)
> +{
> +	struct bpf_object_open_attr attr = {
> +		.prog_type	= BPF_PROG_TYPE_UNSPEC,
> +	};
> +
> +	return bpf_object__open_xattr(path, &attr);
>  }
>  
>  struct bpf_object *bpf_object__open_buffer(void *obj_buf,
> @@ -2238,6 +2249,9 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
>  int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>  			struct bpf_object **pobj, int *prog_fd)
>  {
> +	struct bpf_object_open_attr open_attr = {
> +		.prog_type	= attr->prog_type,
> +	};
>  	struct bpf_program *prog, *first_prog = NULL;
>  	enum bpf_attach_type expected_attach_type;
>  	enum bpf_prog_type prog_type;
> @@ -2250,8 +2264,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>  	if (!attr->file)
>  		return -EINVAL;
>  
> -	obj = __bpf_object__open(attr->file, NULL, 0,
> -				 bpf_prog_type__needs_kver(attr->prog_type));
> +	obj = bpf_object__open_xattr(attr->file, &open_attr);
>  	if (IS_ERR_OR_NULL(obj))
>  		return -ENOENT;
>  
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 3122d74f2643..60593ac44700 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -66,7 +66,13 @@ void libbpf_set_print(libbpf_print_fn_t warn,
>  /* Hide internal to user */
>  struct bpf_object;
>  
> +struct bpf_object_open_attr {
> +	enum bpf_prog_type prog_type;
> +};
> +
>  struct bpf_object *bpf_object__open(const char *path);
> +struct bpf_object *bpf_object__open_xattr(const char *path,
> +					  struct bpf_object_open_attr *attr);

Should the new bpf_object__open_xattr() API have _only_ attr argument?
Path, in turn, can become a member of attr.

That way it can be reused e.g. to load object from buffer (like
bpf_object__open_buffer() below), where path is not needed.

Otherwise, if bpf_object__open_buffer() has to be extended in the
future, another _xattr function will be needed (or caller would need to
pass NULL to path, what would make API less convenient).


>  struct bpf_object *bpf_object__open_buffer(void *obj_buf,
>  					   size_t obj_buf_sz,
>  					   const char *name);
> -- 
> 2.17.1
> 

-- 
Andrey Ignatov