netdev - Re: [PATCH bpf-next] libbpf: export bpf_object__reuse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzZy9=x0neCOdat-CWO4nM3QYgWOKaZpN31Ce5Uz9m_qfg@mail.gmail.com>
Date:   Tue, 29 Sep 2020 16:03:45 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Hangbin Liu <liuhangbin@...il.com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Jiri Benc <jbenc@...hat.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>
Subject: Re: [PATCH bpf-next] libbpf: export bpf_object__reuse_map() to libbpf api

On Tue, Sep 29, 2020 at 2:42 AM Hangbin Liu <liuhangbin@...il.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