linux-kernel - Re: [PATCH bpf-next v1] tools/bpftool: create map of maps

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] [day] [month] [year] [list]

Message-ID: <CADZs7q77wnbQSOT-mgX9KRC5ZbBxwt1_GaPDqeQ3jijFa09nMA@mail.gmail.com>
Date:   Thu, 7 Mar 2019 10:14:14 +0100
From:   Alban Crequy <alban@...volk.io>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Alban Crequy <alban.crequy@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Iago López Galeiras <iago@...volk.io>,
        Thilo Fromm <thilo@...volk.io>
Subject: Re: [PATCH bpf-next v1] tools/bpftool: create map of maps

Thanks for the reviews, Jakub and Quentin! I will address it and
resend a new version when bpf-next opens again.
I'm also preparing some other patches on "bpftool map" about pinning
and passing file descriptors to help applications that don't support
map pinning directly.

On Tue, Mar 5, 2019 at 6:32 PM Jakub Kicinski
<jakub.kicinski@...ronome.com> wrote:
>
> On Tue,  5 Mar 2019 17:38:03 +0100, Alban Crequy wrote:
> > From: Alban Crequy <alban@...volk.io>
> >
> > Before this patch, there was no way to fill attr.inner_map_fd, necessary
> > for array_of_maps or hash_of_maps.
> >
> > This patch adds keyword 'innermap' to pass the innermap, either as an id
> > or as a pinned map.
> >
> > Example of commands:
> >
> > $ sudo bpftool map create /sys/fs/bpf/innermap type hash \
> >         key 8 value 8 entries 64 name innermap flags 1
> > $ sudo bpftool map create /sys/fs/bpf/outermap type hash_of_maps \
> >         innermap pinned /sys/fs/bpf/innermap key 64 value 4 \
> >         entries 64 name myoutermap flags 1
> > $ sudo bpftool map show pinned /sys/fs/bpf/outermap
> > 47: hash_of_maps  name myoutermap  flags 0x1
> >       key 64B  value 4B  max_entries 64  memlock 12288B
> >
> > Documentation and bash completion updated as well.
> >
> > Signed-off-by: Alban Crequy <alban@...volk.io>
>
> bpf-next is closed let's continue reviewing, but you'll probably have
> to repost after the merge window :(
>
> > diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> > index e0c650d91784..7d8ce903a471 100644
> > --- a/tools/bpf/bpftool/map.c
> > +++ b/tools/bpf/bpftool/map.c
> > @@ -1151,6 +1151,9 @@ static int do_create(int argc, char **argv)
> >                               return -1;
> >                       }
> >                       NEXT_ARG();
> > +             } else if (is_prefix(*argv, "innermap")) {
> > +                     NEXT_ARG();
> > +                     attr.inner_map_fd = map_parse_fd(&argc, &argv);
>
> You need to check if the return value is not -1, and also close this
> file descriptor (a) when done, (b) when error happens.
>
> >               }
> >       }
> >