On Tue, Dec 10, 2019 at 6:35 PM Andrey Ignatov <rdna@xxxxxx> wrote:
>
> Introduce a new bpf_prog_attach_xattr function that accepts an
> extendable structure and supports passing a new attribute to
> BPF_PROG_ATTACH command: replace_prog_fd that is fd of previously
> attached cgroup-bpf program to replace if recently introduced
> BPF_F_REPLACE flag is used.
>
> The new function is named to be consistent with other xattr-functions
> (bpf_prog_test_run_xattr, bpf_create_map_xattr, bpf_load_program_xattr).
>
> NOTE: DECLARE_LIBBPF_OPTS macro is not used here because it's available
> in libbpf.h, and unavailable in bpf.h. Please let me know if the macro
> should be shared in a common place and used here instead of declaring
> struct bpf_prog_attach_attr directly.
>
I think doing opts is a better way to go forward. With current
approach, next time we need another extra field, we'd need to add yet
another function and/or symbol version existing one. BTW, with
xxx_opts approach, we tried to keep mandatory arguments that are going
to be always specified as first few arguments of a function, and other
stuff that's optional (e.g., flags or replace_prog_fd seem to be good
candidates), would go under opts. This differs from xattr way, which
is why I'm pointing this out.
> Signed-off-by: Andrey Ignatov <rdna@xxxxxx>
> ---
> tools/lib/bpf/bpf.c | 22 ++++++++++++++++++----
> tools/lib/bpf/bpf.h | 10 ++++++++++
> tools/lib/bpf/libbpf.map | 5 +++++
> 3 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 98596e15390f..5a2830fac227 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -466,14 +466,28 @@ int bpf_obj_get(const char *pathname)
>
> int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type,
> unsigned int flags)
> +{
> + struct bpf_prog_attach_attr attach_attr;
> +
> + memset(&attach_attr, 0, sizeof(attach_attr));
> + attach_attr.target_fd = target_fd;
> + attach_attr.prog_fd = prog_fd;
> + attach_attr.type = type;
> + attach_attr.flags = flags;
> +
> + return bpf_prog_attach_xattr(&attach_attr);
> +}
> +
> +int bpf_prog_attach_xattr(const struct bpf_prog_attach_attr *attach_attr)
> {
> union bpf_attr attr;
>
> memset(&attr, 0, sizeof(attr));
> - attr.target_fd = target_fd;
> - attr.attach_bpf_fd = prog_fd;
> - attr.attach_type = type;
> - attr.attach_flags = flags;
> + attr.target_fd = attach_attr->target_fd;
> + attr.attach_bpf_fd = attach_attr->prog_fd;
> + attr.attach_type = attach_attr->type;
> + attr.attach_flags = attach_attr->flags;
> + attr.replace_bpf_fd = attach_attr->replace_prog_fd;
>
> return sys_bpf(BPF_PROG_ATTACH, &attr, sizeof(attr));
> }
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 3c791fa8e68e..4b7269d3bae7 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -128,8 +128,18 @@ LIBBPF_API int bpf_map_get_next_key(int fd, const void *key, void *next_key);
> LIBBPF_API int bpf_map_freeze(int fd);
> LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
> LIBBPF_API int bpf_obj_get(const char *pathname);
> +
> +struct bpf_prog_attach_attr {
> + int target_fd;
> + int prog_fd;
> + enum bpf_attach_type type;
> + unsigned int flags;
> + int replace_prog_fd;
> +};
> +
> LIBBPF_API int bpf_prog_attach(int prog_fd, int attachable_fd,
> enum bpf_attach_type type, unsigned int flags);
> +LIBBPF_API int bpf_prog_attach_xattr(const struct bpf_prog_attach_attr *attr);
> LIBBPF_API int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
> LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd,
> enum bpf_attach_type type);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 8ddc2c40e482..42b065454031 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -208,3 +208,8 @@ LIBBPF_0.0.6 {
> btf__find_by_name_kind;
> libbpf_find_vmlinux_btf_id;
> } LIBBPF_0.0.5;
> +
> +LIBBPF_0.0.7 {
> + global:
> + bpf_prog_attach_xattr;
> +} LIBBPF_0.0.6;
> --
> 2.17.1
>
