On Wed, Jun 1, 2022 at 12:02 PM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote:
>
> Implement bpf_prog_query_opts as a more expendable version of
> bpf_prog_query. Expose new prog_attach_flags and attach_btf_func_id as
> well:
>
> * prog_attach_flags is a per-program attach_type; relevant only for
> lsm cgroup program which might have different attach_flags
> per attach_btf_id
> * attach_btf_func_id is a new field expose for prog_query which
> specifies real btf function id for lsm cgroup attachments
>
> Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
> ---
> tools/include/uapi/linux/bpf.h | 3 +++
> tools/lib/bpf/bpf.c | 40 +++++++++++++++++++++++++++-------
> tools/lib/bpf/bpf.h | 15 +++++++++++++
> tools/lib/bpf/libbpf.map | 2 +-
> 4 files changed, 51 insertions(+), 9 deletions(-)
>
Few consistency nits, but otherwise:
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index fa64b0b612fd..4271ef3c2afb 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1432,6 +1432,7 @@ union bpf_attr {
> __u32 attach_flags;
> __aligned_u64 prog_ids;
> __u32 prog_cnt;
> + __aligned_u64 prog_attach_flags; /* output: per-program attach_flags */
> } query;
>
> struct { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
> @@ -5996,6 +5997,8 @@ struct bpf_prog_info {
> __u64 run_cnt;
> __u64 recursion_misses;
> __u32 verified_insns;
> + __u32 attach_btf_obj_id;
> + __u32 attach_btf_id;
> } __attribute__((aligned(8)));
>
> struct bpf_map_info {
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 240186aac8e6..c7af7db53725 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -888,28 +888,52 @@ int bpf_iter_create(int link_fd)
> return libbpf_err_errno(fd);
> }
>
> -int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
> - __u32 *attach_flags, __u32 *prog_ids, __u32 *prog_cnt)
> +int bpf_prog_query_opts(int target_fd,
> + enum bpf_attach_type type,
> + struct bpf_prog_query_opts *opts)
> {
> union bpf_attr attr;
> int ret;
>
> memset(&attr, 0, sizeof(attr));
> +
> + if (!OPTS_VALID(opts, bpf_prog_query_opts))
> + return libbpf_err(-EINVAL);
> +
nit: check input args before you do work (memset), but it's very minor
> attr.query.target_fd = target_fd;
> attr.query.attach_type = type;
> - attr.query.query_flags = query_flags;
> - attr.query.prog_cnt = *prog_cnt;
> - attr.query.prog_ids = ptr_to_u64(prog_ids);
> + attr.query.query_flags = OPTS_GET(opts, query_flags, 0);
> + attr.query.prog_cnt = OPTS_GET(opts, prog_cnt, 0);
> + attr.query.prog_ids = ptr_to_u64(OPTS_GET(opts, prog_ids, NULL));
> + attr.query.prog_attach_flags = ptr_to_u64(OPTS_GET(opts, prog_attach_flags, NULL));
>
> ret = sys_bpf(BPF_PROG_QUERY, &attr, sizeof(attr));
>
> - if (attach_flags)
> - *attach_flags = attr.query.attach_flags;
> - *prog_cnt = attr.query.prog_cnt;
> + OPTS_SET(opts, attach_flags, attr.query.attach_flags);
> + OPTS_SET(opts, prog_cnt, attr.query.prog_cnt);
>
> return libbpf_err_errno(ret);
> }
>
> +int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
> + __u32 *attach_flags, __u32 *prog_ids, __u32 *prog_cnt)
> +{
> + LIBBPF_OPTS(bpf_prog_query_opts, p);
nit: for consistency it would be good to call variable "opts" as we do
pretty much everywhere else?
> + int ret;
> +
> + p.query_flags = query_flags;
> + p.prog_ids = prog_ids;
> + p.prog_cnt = *prog_cnt;
> +
> + ret = bpf_prog_query_opts(target_fd, type, &p);
> +
> + if (attach_flags)
> + *attach_flags = p.attach_flags;
> + *prog_cnt = p.prog_cnt;
> +
> + return ret;
maybe use libbpf_err() here for consistency and just in case we add
something that can clobber errno
> +}
> +
> int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
> void *data_out, __u32 *size_out, __u32 *retval,
> __u32 *duration)
[...]
