On Tue, Mar 7, 2023 at 3:33 PM Kui-Feng Lee <kuifeng@xxxxxxxx> wrote:
>
> bpf_map__attach_struct_ops() was creating a dummy bpf_link as a
> placeholder, but now it is constructing an authentic one by calling
> bpf_link_create() if the map has the BPF_F_LINK flag.
>
> You can flag a struct_ops map with BPF_F_LINK by calling
> bpf_map__set_map_flags().
>
> Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 84 +++++++++++++++++++++++++++++++-----------
> 1 file changed, 62 insertions(+), 22 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 35a698eb825d..a67efc3b3763 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -115,6 +115,7 @@ static const char * const attach_type_name[] = {
> [BPF_SK_REUSEPORT_SELECT_OR_MIGRATE] = "sk_reuseport_select_or_migrate",
> [BPF_PERF_EVENT] = "perf_event",
> [BPF_TRACE_KPROBE_MULTI] = "trace_kprobe_multi",
> + [BPF_STRUCT_OPS] = "struct_ops",
> };
>
> static const char * const link_type_name[] = {
> @@ -7677,6 +7678,26 @@ static int bpf_object__resolve_externs(struct bpf_object *obj,
> return 0;
> }
>
> +static void bpf_map_prepare_vdata(const struct bpf_map *map)
> +{
> + struct bpf_struct_ops *st_ops;
> + __u32 i;
> +
> + st_ops = map->st_ops;
> + for (i = 0; i < btf_vlen(st_ops->type); i++) {
> + struct bpf_program *prog = st_ops->progs[i];
> + void *kern_data;
> + int prog_fd;
> +
> + if (!prog)
> + continue;
> +
> + prog_fd = bpf_program__fd(prog);
> + kern_data = st_ops->kern_vdata + st_ops->kern_func_off[i];
> + *(unsigned long *)kern_data = prog_fd;
> + }
> +}
> +
> static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const char *target_btf_path)
> {
> int err, i;
> @@ -7728,6 +7749,10 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch
> btf__free(obj->btf_vmlinux);
> obj->btf_vmlinux = NULL;
>
> + for (i = 0; i < obj->nr_maps; i++)
> + if (bpf_map__is_struct_ops(&obj->maps[i]))
> + bpf_map_prepare_vdata(&obj->maps[i]);
This is similar in spirit to what bpf_object_init_prog_arrays() is
doing, let's add this as a separate step.
How about bpf_object_prepare_struct_ops()?
> +
> obj->loaded = true; /* doesn't matter if successfully or not */
>
> if (err)
> @@ -11429,22 +11454,34 @@ struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
> return link;
> }
>
> +struct bpf_link_struct_ops {
> + struct bpf_link link;
> + int map_fd;
> +};
> +
> static int bpf_link__detach_struct_ops(struct bpf_link *link)
> {
> + struct bpf_link_struct_ops *st_link;
> __u32 zero = 0;
>
> - if (bpf_map_delete_elem(link->fd, &zero))
> - return -errno;
> + st_link = container_of(link, struct bpf_link_struct_ops, link);
>
> - return 0;
> + if (st_link->map_fd < 0) {
> + /* Fake bpf_link */
> + if (bpf_map_delete_elem(link->fd, &zero))
> + return -errno;
> + return 0;
just `return bpf_map_delete_elem(...)`, it will return actual error
(libbpf 1.0 simplification)
> + }
> +
> + /* Doesn't support detaching. */
> + return -EOPNOTSUPP;
> }
>
> struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
> {
> - struct bpf_struct_ops *st_ops;
> - struct bpf_link *link;
> - __u32 i, zero = 0;
> - int err;
> + struct bpf_link_struct_ops *link;
> + __u32 zero = 0;
> + int err, fd;
>
> if (!bpf_map__is_struct_ops(map) || map->fd == -1)
> return libbpf_err_ptr(-EINVAL);
> @@ -11453,31 +11490,34 @@ struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
> if (!link)
> return libbpf_err_ptr(-EINVAL);
>
> - st_ops = map->st_ops;
> - for (i = 0; i < btf_vlen(st_ops->type); i++) {
> - struct bpf_program *prog = st_ops->progs[i];
> - void *kern_data;
> - int prog_fd;
> + /* kern_vdata should be prepared during the loading phase. */
> + err = bpf_map_update_elem(map->fd, &zero, map->st_ops->kern_vdata, 0);
> + if (err) {
> + err = -errno;
no need to deal with -errno, err is already the error you need
> + free(link);
> + return libbpf_err_ptr(err);
> + }
>
> - if (!prog)
> - continue;
>
> - prog_fd = bpf_program__fd(prog);
> - kern_data = st_ops->kern_vdata + st_ops->kern_func_off[i];
> - *(unsigned long *)kern_data = prog_fd;
> + if (!(map->def.map_flags & BPF_F_LINK)) {
> + /* Fake bpf_link */
> + link->link.fd = map->fd;
> + link->map_fd = -1;
> + link->link.detach = bpf_link__detach_struct_ops;
> + return &link->link;
> }
>
> - err = bpf_map_update_elem(map->fd, &zero, st_ops->kern_vdata, 0);
> - if (err) {
> + fd = bpf_link_create(map->fd, -1, BPF_STRUCT_OPS, NULL);
> + if (fd < 0) {
> err = -errno;
same, fd is an error, it's true for all low-level libbpf APIs
> free(link);
> return libbpf_err_ptr(err);
> }
>
> - link->detach = bpf_link__detach_struct_ops;
> - link->fd = map->fd;
> + link->link.fd = fd;
> + link->map_fd = map->fd;
>
> - return link;
> + return &link->link;
> }
>
> typedef enum bpf_perf_event_ret (*bpf_perf_event_print_t)(struct perf_event_header *hdr,
> --
> 2.34.1
>
