Re: [PATCH bpf-next] libbpf: export bpf_object__reuse_map() to libbpf api

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Sep 29, 2020 at 2:42 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Mon, Sep 28, 2020 at 08:30:42PM -0700, Andrii Nakryiko wrote:
> > > @@ -431,6 +431,7 @@ bpf_map__prev(const struct bpf_map *map, const struct bpf_object *obj);
> > >  /* get/set map FD */
> > >  LIBBPF_API int bpf_map__fd(const struct bpf_map *map);
> > >  LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
> > > +LIBBPF_API int bpf_object__reuse_map(struct bpf_map *map);
> >
> > It's internal function, which doesn't check that map->pin_path is set,
>
> How about add a path check in bpf_object__reuse_map()?
>
> And off course users who use it should call bpf_map__set_pin_path() first.
>
> > for one thing. It shouldn't be exposed. libbpf exposes
> > bpf_map__set_pin_path() to set pin_path for any map, and then during
> > load time libbpf with "reuse map", if pin_path is not NULL. Doesn't
> > that work for you?
>
> Long story...
>
> When trying to add iproute2 libbpf support that I'm working on. We need to
> create iproute2 legacy map-in-map manually before libbpf load objects, because
> libbpf only support BTF type map-in-map(unless I missed something.).
>
> After creating legacy map-in-map and reuse the fd, if the map has legacy
> pining defined, only set the pin path would not enough as libbpf will skip
> pinning map if map->fd > 0 in bpf_object__create_maps(). See the following
> code bellow.
>
> bpf_map__set_pin_path()
> bpf_create_map_in_map()    <- create inner or outer map
> bpf_map__reuse_fd(map, inner/outer_fd)
> bpf_object__load(obj)
>   - bpf_object__load_xattr()
>     - bpf_object__create_maps()
>       - if (map->fd >= 0)
>           continue      <- this will skip pinning map

so maybe that's the part that needs to be fixed?..

>
> So when handle legacy map-in-map + pin map, we need to create the map
> and pin maps manually at the same time. The code would looks like
> (err handler skipped).
>
> map_fd = bpf_obj_get(pathname);
> if (map_fd > 0) {
>         bpf_map__set_pin_path(map, pathname);
>         return bpf_object__reuse_map(map);   <- here we need the reuse_map
> }
> bpf_create_map_in_map()
> bpf_map__reuse_fd(map, map_fd);
> bpf_map__pin(map, pathname);
>
> So I think this function is needed, what do you think?

I'm still not sure. And to be honest your examples are still a bit too
succinct for me to follow where the problem is exactly. Can you please
elaborate a bit more?

It might very well be that map pinning and FD reuse have buggy and
convoluted logic, but let's try to fix that first, before we expose
new APIs.

>
> Thanks
> Hangbin