Re: [PATCH 3/3] samples: bpf: refactor BPF map in map test with libbpf

On Sat, Jun 27, 2020 at 5:30 AM Andrii Nakryiko
[off-list ref] wrote:

On Fri, Jun 26, 2020 at 1:18 AM Daniel T. Lee [off-list ref] wrote:

quoted

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 <redacted>
---

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.

quoted

 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(-)

[...]

quoted

        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...

quoted

+               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.

quoted

+               printf("finding a prog in obj file failed\n");
+               goto cleanup;
+       }
+

[...]