Re: [PATCH v2 16/23] libbpf: introduce bpf_map__attach_struct_ops_opts()

bot+bpf-ci
2025-10-27T23:48:12Z

[-- Attachment #1: Type: text/plain, Size: 3308 bytes --]

```
commit b58d54ed56054e3a550e02c324982feb20a2671e
Author: Roman Gushchin <roman.gushchin@linux.dev>

libbpf: introduce bpf_map__attach_struct_ops_opts()

This commit introduces a new API function that extends the existing
bpf_map__attach_struct_ops() to accept additional options, specifically
allowing attachment of struct_ops maps to cgroups via a relative_fd.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b90574f39..be56a5dee 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -13196,12 +13196,19 @@ static int bpf_link__detach_struct_ops(struct bpf_link *link)
> return close(link->fd);
> }
>
> -struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
> +struct bpf_link *bpf_map__attach_struct_ops_opts(const struct bpf_map *map,
> + const struct bpf_struct_ops_opts *opts)
> {
> + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, link_opts);
> struct bpf_link_struct_ops *link;
> __u32 zero = 0;
> int err, fd;
>
> + if (!OPTS_VALID(opts, bpf_struct_ops_opts)) {
> + pr_warn("map '%s': invalid opts\n", map->name);
> + return libbpf_err_ptr(-EINVAL);
> + }
> +
> if (!bpf_map__is_struct_ops(map)) {
> pr_warn("map '%s': can't attach non-struct_ops map\n", map->name);
> return libbpf_err_ptr(-EINVAL);

[ ... ]

> link = calloc(1, sizeof(*link));
> if (!link)
> return libbpf_err_ptr(-EINVAL);

Should this return -ENOMEM instead of -EINVAL? All other calloc failures
in libbpf.c return -ENOMEM (see lines 11006, 11802, 12288, 12746, 12829,
12891, 13089, 13129 in the same file).

[ ... ]

> - fd = bpf_link_create(map->fd, 0, BPF_STRUCT_OPS, NULL);
> + link_opts.cgroup.relative_fd = OPTS_GET(opts, relative_fd, 0);
> +
> + fd = bpf_link_create(map->fd, 0, BPF_STRUCT_OPS, &link_opts);

Is the expected_revision field being propagated correctly? The new
bpf_struct_ops_opts structure includes an expected_revision field
(see libbpf.h), and bpf_link_create() with BPF_STRUCT_OPS reads
opts->cgroup.expected_revision (see bpf.c line 889), but this code only
sets relative_fd in link_opts. Should it also include:

link_opts.cgroup.expected_revision = OPTS_GET(opts, expected_revision, 0);

> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5118d0a90..dc8489871 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -922,6 +922,20 @@ bpf_program__attach_cgroup_opts(const struct bpf_program *prog, int cgroup_fd,
> struct bpf_map;
>
> LIBBPF_API struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map);
> +
> +struct bpf_struct_ops_opts {
> + /* size of this struct, for forward/backward compatibility */
> + size_t sz;
> + __u32 flags;
> + __u32 relative_fd;
> + __u64 expected_revision;
> + size_t :0;
> +};
> +#define bpf_struct_ops_opts__last_field expected_revision
> +
> +LIBBPF_API struct bpf_link *
> +bpf_map__attach_struct_ops_opts(const struct bpf_map *map,
> + const struct bpf_struct_ops_opts *opts);

[ ... ]

```

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: `libbpf: introduce bpf_map__attach_struct_ops_opts()`
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18859027430