From: "Daniel T. Lee" <danieltimlee@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
Alexei Starovoitov <ast@kernel.org>, Yonghong Song <yhs@fb.com>,
Martin KaFai Lau <kafai@fb.com>, Andrii Nakryiko <andriin@fb.com>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH 3/3] samples: bpf: refactor BPF map in map test with libbpf
Date: Sat, 27 Jun 2020 07:14:14 +0900 [thread overview]
Message-ID: <CAEKGpziJWYDhnq=DWvcFdSAA-jnGk=Vrci2A-9ktY6g5_4Ki8Q@mail.gmail.com> (raw)
In-Reply-To: <CAEf4BzbGk2xSGAkLEXKSg3NhrL28o+cmW9jTq2=EhggJEYT=5Q@mail.gmail.com>
On Sat, Jun 27, 2020 at 5:30 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jun 26, 2020 at 1:18 AM Daniel T. Lee <danieltimlee@gmail.com> wrote:
> >
> > From commit 646f02ffdd49 ("libbpf: Add BTF-defined map-in-map
> > support"), a way to define internal map in BTF-defined map has been
> > added.
> >
> > Instead of using previous 'inner_map_idx' definition, the structure to
> > be used for the inner map can be directly defined using array directive.
> >
> > __array(values, struct inner_map)
> >
> > This commit refactors map in map test program with libbpf by explicitly
> > defining inner map with BTF-defined format.
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> > ---
>
> Thanks for the clean up, looks good except that prog NULL check.
>
I'll fix this NULL check as well too.
> It also seems like this is the last use of bpf_map_def_legacy, do you
> mind removing it as well?
>
Actually, there is one more place that uses bpf_map_def_legacy.
map_perf_test_kern.c is the one, and I'm currently working on it, but
I'm having difficulty with refactoring this file at the moment.
It has a hash_map map definition named inner_lru_hash_map with
BPF_F_NUMA_NODE flag and '.numa_node = 0'.
The bpf_map_def in libbpf has the attribute name map_flags but
it does not have the numa_node attribute. Because the numa node
for bpf_map_def cannot be explicitly specified, this means that there
is no way to set the numa node where the map will be placed at the
time of bpf_object__load.
The only approach currently available is not to use libbbpf to handle
everything (bpf_object_load), but instead to create a map directly with
specifying numa node (bpf_load approach).
bpf_create_map_in_map_node
bpf_create_map_node
I'm trying to stick with the libbpf implementation only, and I'm wondering
If I have to create bpf maps manually at _user.c program.
Any advice and suggestions will be greatly appreciated.
Thanks for your time and effort for the review.
Daniel.
>
> > samples/bpf/Makefile | 2 +-
> > samples/bpf/test_map_in_map_kern.c | 85 +++++++++++++++---------------
> > samples/bpf/test_map_in_map_user.c | 53 +++++++++++++++++--
> > 3 files changed, 91 insertions(+), 49 deletions(-)
> >
>
> [...]
>
> >
> > snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
> > + obj = bpf_object__open_file(filename, NULL);
> > + if (libbpf_get_error(obj)) {
>
> this is right, but...
>
> > + fprintf(stderr, "ERROR: opening BPF object file failed\n");
> > + return 0;
> > + }
> >
> > - if (load_bpf_file(filename)) {
> > - printf("%s", bpf_log_buf);
> > - return 1;
> > + prog = bpf_object__find_program_by_name(obj, "trace_sys_connect");
> > + if (libbpf_get_error(prog)) {
>
> this is wrong. Just NULL check. libbpf APIs are not very consistent
> with what they return, unfortunately.
>
> > + printf("finding a prog in obj file failed\n");
> > + goto cleanup;
> > + }
> > +
>
> [...]
next prev parent reply other threads:[~2020-06-26 22:14 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-26 8:17 [PATCH 1/3] samples: bpf: fix bpf programs with kprobe/sys_connect event Daniel T. Lee
2020-06-26 8:17 ` [PATCH 2/3] samples: bpf: cleanup pointer error check with libbpf Daniel T. Lee
2020-06-26 20:24 ` Andrii Nakryiko
2020-06-26 21:28 ` Daniel T. Lee
2020-06-26 8:17 ` [PATCH 3/3] samples: bpf: refactor BPF map in map test " Daniel T. Lee
2020-06-26 20:30 ` Andrii Nakryiko
2020-06-26 22:14 ` Daniel T. Lee [this message]
2020-06-26 22:19 ` Andrii Nakryiko
2020-06-26 22:25 ` Daniel T. Lee
2020-06-26 20:31 ` [PATCH 1/3] samples: bpf: fix bpf programs with kprobe/sys_connect event 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='CAEKGpziJWYDhnq=DWvcFdSAA-jnGk=Vrci2A-9ktY6g5_4Ki8Q@mail.gmail.com' \
--to=danieltimlee@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kafai@fb.com \
--cc=netdev@vger.kernel.org \
--cc=yhs@fb.com \
/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).