From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Song Liu <liu.song.a23@gmail.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
Networking <netdev@vger.kernel.org>,
Alexei Starovoitov <ast@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [RFC PATCH bpf-next 3/8] libbpf: refactor map initialization
Date: Mon, 17 Jun 2019 11:01:55 -0700 [thread overview]
Message-ID: <CAEf4BzbTf6o9yqij7M1rwAOx_hq2Yc=qvNX_o1Trooesxja2ig@mail.gmail.com> (raw)
In-Reply-To: <CAPhsuW7bowxNMr22UkCvTkq8VHYrNiEJtQSdZjausj_8d4oYUQ@mail.gmail.com>
On Sat, Jun 15, 2019 at 2:03 PM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Mon, Jun 10, 2019 at 9:35 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > User and global data maps initialization has gotten pretty complicated
> > and unnecessarily convoluted. This patch splits out the logic for global
> > data map and user-defined map initialization. It also removes the
> > restriction of pre-calculating how many maps will be initialized,
> > instead allowing to keep adding new maps as they are discovered, which
> > will be used later for BTF-defined map definitions.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 244 ++++++++++++++++++++++-------------------
> > 1 file changed, 134 insertions(+), 110 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 9e39a0a33aeb..c931ee7e1fd2 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -234,6 +234,7 @@ struct bpf_object {
> > size_t nr_programs;
> > struct bpf_map *maps;
> > size_t nr_maps;
> > + size_t maps_cap;
> > struct bpf_secdata sections;
> >
> > bool loaded;
> > @@ -763,12 +764,38 @@ int bpf_object__variable_offset(const struct bpf_object *obj, const char *name,
> > return -ENOENT;
> > }
> >
> > -static bool bpf_object__has_maps(const struct bpf_object *obj)
> > +static struct bpf_map *bpf_object__add_map(struct bpf_object *obj)
> > {
> > - return obj->efile.maps_shndx >= 0 ||
> > - obj->efile.data_shndx >= 0 ||
> > - obj->efile.rodata_shndx >= 0 ||
> > - obj->efile.bss_shndx >= 0;
> > + struct bpf_map *new_maps;
> > + size_t new_cap;
> > + int i;
> > +
> > + if (obj->nr_maps + 1 <= obj->maps_cap)
> nit: how about if (obj->nr_maps < obj->maps_cap)
yep, will do
>
> > + return &obj->maps[obj->nr_maps++];
> > +
> > + new_cap = max(4ul, obj->maps_cap * 3 / 2);
> > + new_maps = realloc(obj->maps, new_cap * sizeof(*obj->maps));
> > + if (!new_maps) {
> > + pr_warning("alloc maps for object failed\n");
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +
> > + obj->maps_cap = new_cap;
> > + obj->maps = new_maps;
> > +
> > + /* zero out new maps */
> > + memset(obj->maps + obj->nr_maps, 0,
> > + (obj->maps_cap - obj->nr_maps) * sizeof(*obj->maps));
> > + /*
> > + * fill all fd with -1 so won't close incorrect fd (fd=0 is stdin)
> > + * when failure (zclose won't close negative fd)).
> > + */
> > + for (i = obj->nr_maps; i < obj->maps_cap; i++) {
> > + obj->maps[i].fd = -1;
> > + obj->maps[i].inner_map_fd = -1;
> > + }
> > +
> > + return &obj->maps[obj->nr_maps++];
> > }
> >
> > static int
> > @@ -808,29 +835,68 @@ bpf_object__init_internal_map(struct bpf_object *obj, struct bpf_map *map,
> > return 0;
> > }
> >
> > -static int bpf_object__init_maps(struct bpf_object *obj, int flags)
> > +static int bpf_object__init_global_data_maps(struct bpf_object *obj)
> > +{
> > + struct bpf_map *map;
> > + int err;
> > +
> > + if (!obj->caps.global_data)
> > + return 0;
> > + /*
> > + * Populate obj->maps with libbpf internal maps.
> > + */
> > + if (obj->efile.data_shndx >= 0) {
> > + map = bpf_object__add_map(obj);
> > + if (IS_ERR(map))
> > + return PTR_ERR(map);
> > + err = bpf_object__init_internal_map(obj, map, LIBBPF_MAP_DATA,
> > + obj->efile.data,
> > + &obj->sections.data);
> > + if (err)
> > + return err;
> > + }
> > + if (obj->efile.rodata_shndx >= 0) {
> > + map = bpf_object__add_map(obj);
> > + if (IS_ERR(map))
> > + return PTR_ERR(map);
> > + err = bpf_object__init_internal_map(obj, map, LIBBPF_MAP_RODATA,
> > + obj->efile.rodata,
> > + &obj->sections.rodata);
> > + if (err)
> > + return err;
> > + }
> > + if (obj->efile.bss_shndx >= 0) {
> > + map = bpf_object__add_map(obj);
> > + if (IS_ERR(map))
> > + return PTR_ERR(map);
> > + err = bpf_object__init_internal_map(obj, map, LIBBPF_MAP_BSS,
> > + obj->efile.bss, NULL);
> > + if (err)
> > + return err;
> > + }
>
> These 3 if clause are a little too complicated. How about we call
> bpf_obj_add_map(obj) within bpf_object__init_internal_map()?
Yeah, totally, not sure why I did it this way :)
>
> > + return 0;
> > +}
> > +
> > +static int bpf_object__init_user_maps(struct bpf_object *obj, bool strict)
> > {
<snip>
next prev parent reply other threads:[~2019-06-17 18:02 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-11 4:34 [RFC PATCH bpf-next 0/8] BTF-defined BPF map definitions Andrii Nakryiko
2019-06-11 4:34 ` [RFC PATCH bpf-next 1/8] libbpf: add common min/max macro to libbpf_internal.h Andrii Nakryiko
2019-06-11 4:34 ` [RFC PATCH bpf-next 2/8] libbpf: extract BTF loading and simplify ELF parsing logic Andrii Nakryiko
2019-06-11 4:35 ` [RFC PATCH bpf-next 3/8] libbpf: refactor map initialization Andrii Nakryiko
2019-06-15 21:02 ` Song Liu
2019-06-15 21:32 ` Song Liu
2019-06-17 18:01 ` Andrii Nakryiko [this message]
2019-06-11 4:35 ` [RFC PATCH bpf-next 4/8] libbpf: identify maps by section index in addition to offset Andrii Nakryiko
2019-06-15 21:07 ` Song Liu
2019-06-17 18:06 ` Andrii Nakryiko
2019-06-17 18:15 ` Song Liu
2019-06-11 4:35 ` [RFC PATCH bpf-next 5/8] libbpf: split initialization and loading of BTF Andrii Nakryiko
2019-06-11 4:35 ` [RFC PATCH bpf-next 6/8] libbpf: allow specifying map definitions using BTF Andrii Nakryiko
2019-06-11 4:35 ` [RFC PATCH bpf-next 7/8] selftests/bpf: add test for BTF-defined maps Andrii Nakryiko
2019-06-11 4:35 ` [RFC PATCH bpf-next 8/8] selftests/bpf: switch tests to BTF-defined map definitions Andrii Nakryiko
2019-06-11 4:36 ` [RFC PATCH bpf-next 0/8] BTF-defined BPF " Andrii Nakryiko
-- strict thread matches above, loose matches on Subject: below --
2019-05-31 20:21 Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 3/8] libbpf: refactor map initialization Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEf4BzbTf6o9yqij7M1rwAOx_hq2Yc=qvNX_o1Trooesxja2ig@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=liu.song.a23@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).