netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps
@ 2022-02-04 22:58 Jiri Olsa
  2022-02-04 22:58 ` [PATCH bpf-next 2/3] selftests/bpf/test_offload.py: Add more base maps names Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jiri Olsa @ 2022-02-04 22:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Toke Høiland-Jørgensen,
	Jakub Kicinski, Jiri Pirko

Adding names for maps that bpftool uses for various detections.
These maps can appear in final map show output (due to deferred
removal in kernel) so some tests (like test_offload.py) needs
to filter them out.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 904cdf83002b..38294ce935d6 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4412,7 +4412,7 @@ static int probe_kern_global_data(void)
 	};
 	int ret, map, insn_cnt = ARRAY_SIZE(insns);
 
-	map = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), 32, 1, NULL);
+	map = bpf_map_create(BPF_MAP_TYPE_ARRAY, "global_data", sizeof(int), 32, 1, NULL);
 	if (map < 0) {
 		ret = -errno;
 		cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
@@ -4545,7 +4545,7 @@ static int probe_kern_array_mmap(void)
 	LIBBPF_OPTS(bpf_map_create_opts, opts, .map_flags = BPF_F_MMAPABLE);
 	int fd;
 
-	fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), sizeof(int), 1, &opts);
+	fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "array_mmap", sizeof(int), sizeof(int), 1, &opts);
 	return probe_fd(fd);
 }
 
@@ -4592,7 +4592,7 @@ static int probe_prog_bind_map(void)
 	};
 	int ret, map, prog, insn_cnt = ARRAY_SIZE(insns);
 
-	map = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), 32, 1, NULL);
+	map = bpf_map_create(BPF_MAP_TYPE_ARRAY, "bind_map_detect", sizeof(int), 32, 1, NULL);
 	if (map < 0) {
 		ret = -errno;
 		cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/3] selftests/bpf/test_offload.py: Add more base maps names
  2022-02-04 22:58 [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps Jiri Olsa
@ 2022-02-04 22:58 ` Jiri Olsa
  2022-02-04 22:58 ` [PATCH bpf-next 3/3] bpftool: Fix pretty print dump for maps without BTF loaded Jiri Olsa
  2022-02-04 23:04 ` [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps Andrii Nakryiko
  2 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2022-02-04 22:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Toke Høiland-Jørgensen,
	Jakub Kicinski, Jiri Pirko

Adding more base maps that can show in bpftool map output,
so we can properly filter them out.

This fixes for me following test_offload.py failure:

  Test bpftool bound info reporting (own ns)...
  FAIL: 3 BPF maps loaded, expected 2
    File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 1177, in <module>
      check_dev_info(False, "")
    File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 645, in check_dev_info
      maps = bpftool_map_list(expected=2, ns=ns)
    File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 190, in bpftool_map_list
      fail(True, "%d BPF maps loaded, expected %d" %
    File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 86, in fail
      tb = "".join(traceback.extract_stack().format())

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/testing/selftests/bpf/test_offload.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index edaffd43da83..0cf93d246804 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -769,7 +769,11 @@ skip(ret != 0, "bpftool not installed")
 base_progs = progs
 _, base_maps = bpftool("map")
 base_map_names = [
-    'pid_iter.rodata' # created on each bpftool invocation
+    # created on each bpftool invocation
+    'pid_iter.rodata',
+    'bind_map_detect',
+    'global_data',
+    'array_mmap',
 ]
 
 # Check netdevsim
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 3/3] bpftool: Fix pretty print dump for maps without BTF loaded
  2022-02-04 22:58 [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps Jiri Olsa
  2022-02-04 22:58 ` [PATCH bpf-next 2/3] selftests/bpf/test_offload.py: Add more base maps names Jiri Olsa
@ 2022-02-04 22:58 ` Jiri Olsa
  2022-02-04 23:08   ` Andrii Nakryiko
  2022-02-04 23:04 ` [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps Andrii Nakryiko
  2 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2022-02-04 22:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Toke Høiland-Jørgensen,
	Jakub Kicinski, Jiri Pirko

The commit e5043894b21f ("bpftool: Use libbpf_get_error() to check
error") forced map dump with pretty print enabled to has BTF loaded,
which is not necessarily needed.

Keeping the libbpf_get_error call, but setting errno to 0 because
get_map_kv_btf does nothing for this case.

This fixes test_offload.py for me, which failed because of the
pretty print fails with:

   Test map dump...
   Traceback (most recent call last):
     File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 1251, in <module>
       _, entries = bpftool("map dump id %d" % (m["id"]))
     File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 169, in bpftool
       return tool("bpftool", args, {"json":"-p"}, JSON=JSON, ns=ns,
     File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 155, in tool
       ret, stdout = cmd(ns + name + " " + params + args,
     File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 109, in cmd
       return cmd_result(proc, include_stderr=include_stderr, fail=fail)
     File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 131, in cmd_result
       raise Exception("Command failed: %s\n%s" % (proc.args, stderr))
   Exception: Command failed: bpftool -p map dump id 4325

Fixes: e5043894b21f ("bpftool: Use libbpf_get_error() to check error")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/map.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index c66a3c979b7a..2ccf85042e75 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -862,6 +862,7 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
 	prev_key = NULL;
 
 	if (wtr) {
+		errno = 0;
 		btf = get_map_kv_btf(info);
 		err = libbpf_get_error(btf);
 		if (err) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps
  2022-02-04 22:58 [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps Jiri Olsa
  2022-02-04 22:58 ` [PATCH bpf-next 2/3] selftests/bpf/test_offload.py: Add more base maps names Jiri Olsa
  2022-02-04 22:58 ` [PATCH bpf-next 3/3] bpftool: Fix pretty print dump for maps without BTF loaded Jiri Olsa
@ 2022-02-04 23:04 ` Andrii Nakryiko
  2 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 23:04 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
	bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Toke Høiland-Jørgensen, Jakub Kicinski,
	Jiri Pirko

On Fri, Feb 4, 2022 at 2:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> Adding names for maps that bpftool uses for various detections.
> These maps can appear in final map show output (due to deferred
> removal in kernel) so some tests (like test_offload.py) needs
> to filter them out.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 904cdf83002b..38294ce935d6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4412,7 +4412,7 @@ static int probe_kern_global_data(void)
>         };
>         int ret, map, insn_cnt = ARRAY_SIZE(insns);
>
> -       map = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), 32, 1, NULL);
> +       map = bpf_map_create(BPF_MAP_TYPE_ARRAY, "global_data", sizeof(int), 32, 1, NULL);

some old kernel versions don't support map names, so you can't just
blindly specify them and log error

I'd rather fix test_offload.py instead of "fixing" libbpf.


>         if (map < 0) {
>                 ret = -errno;
>                 cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
> @@ -4545,7 +4545,7 @@ static int probe_kern_array_mmap(void)
>         LIBBPF_OPTS(bpf_map_create_opts, opts, .map_flags = BPF_F_MMAPABLE);
>         int fd;
>
> -       fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), sizeof(int), 1, &opts);
> +       fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "array_mmap", sizeof(int), sizeof(int), 1, &opts);
>         return probe_fd(fd);
>  }
>
> @@ -4592,7 +4592,7 @@ static int probe_prog_bind_map(void)
>         };
>         int ret, map, prog, insn_cnt = ARRAY_SIZE(insns);
>
> -       map = bpf_map_create(BPF_MAP_TYPE_ARRAY, NULL, sizeof(int), 32, 1, NULL);
> +       map = bpf_map_create(BPF_MAP_TYPE_ARRAY, "bind_map_detect", sizeof(int), 32, 1, NULL);
>         if (map < 0) {
>                 ret = -errno;
>                 cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 3/3] bpftool: Fix pretty print dump for maps without BTF loaded
  2022-02-04 22:58 ` [PATCH bpf-next 3/3] bpftool: Fix pretty print dump for maps without BTF loaded Jiri Olsa
@ 2022-02-04 23:08   ` Andrii Nakryiko
  0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 23:08 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
	bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Toke Høiland-Jørgensen, Jakub Kicinski,
	Jiri Pirko

On Fri, Feb 4, 2022 at 2:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> The commit e5043894b21f ("bpftool: Use libbpf_get_error() to check
> error") forced map dump with pretty print enabled to has BTF loaded,
> which is not necessarily needed.
>
> Keeping the libbpf_get_error call, but setting errno to 0 because
> get_map_kv_btf does nothing for this case.
>
> This fixes test_offload.py for me, which failed because of the
> pretty print fails with:
>
>    Test map dump...
>    Traceback (most recent call last):
>      File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 1251, in <module>
>        _, entries = bpftool("map dump id %d" % (m["id"]))
>      File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 169, in bpftool
>        return tool("bpftool", args, {"json":"-p"}, JSON=JSON, ns=ns,
>      File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 155, in tool
>        ret, stdout = cmd(ns + name + " " + params + args,
>      File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 109, in cmd
>        return cmd_result(proc, include_stderr=include_stderr, fail=fail)
>      File "/root/bpf-next/tools/testing/selftests/bpf/./test_offload.py", line 131, in cmd_result
>        raise Exception("Command failed: %s\n%s" % (proc.args, stderr))
>    Exception: Command failed: bpftool -p map dump id 4325
>
> Fixes: e5043894b21f ("bpftool: Use libbpf_get_error() to check error")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/bpf/bpftool/map.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index c66a3c979b7a..2ccf85042e75 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -862,6 +862,7 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
>         prev_key = NULL;
>
>         if (wtr) {
> +               errno = 0;

I don't think that's right. errno can be modified by something inside
get_map_kv_btf() so this is unreliable approach. It's better to change
get_map_kv_btf() to return an error explicitly and a btf pointer
separate from error. Because btf == NULL isn't necessarily due to an
error anymore.

>                 btf = get_map_kv_btf(info);
>                 err = libbpf_get_error(btf);
>                 if (err) {
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-02-04 23:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-04 22:58 [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps Jiri Olsa
2022-02-04 22:58 ` [PATCH bpf-next 2/3] selftests/bpf/test_offload.py: Add more base maps names Jiri Olsa
2022-02-04 22:58 ` [PATCH bpf-next 3/3] bpftool: Fix pretty print dump for maps without BTF loaded Jiri Olsa
2022-02-04 23:08   ` Andrii Nakryiko
2022-02-04 23:04 ` [PATCH bpf-next 1/3] libbpf: Add names for auxiliary maps Andrii Nakryiko

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