[PATCH bpf-next v3 0/8] bpf/selftests: page size fixes

bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH bpf-next v3 0/8] bpf/selftests: page size fixes
@ 2021-03-31 16:44 Yauheni Kaliuta
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
  0 siblings, 1 reply; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:44 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

A set of fixes for selftests to make them working on systems with PAGE_SIZE > 4K

--
v2->v3:
 - reorder: move version removing patch first to keep main patches in
   one group;
 - rename "selftests/bpf: pass page size from userspace in sockopt_sk"
   as suggested;
 - convert sockopt_sk test to use ASSERT macros;
 - set page size from userspace
 - split patches to pairs userspace/bpf. It's easier to check that
   every conversion works as expected;

v1->v2:

- add missed 'selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton'

Yauheni Kaliuta (8):
  selftests/bpf: test_progs/sockopt_sk: remove version
  selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton
  selftests/bpf: pass page size from userspace in sockopt_sk
  selftests/bpf: pass page size from userspace in map_ptr
  selftests/bpf: mmap: use runtime page size
  selftests/bpf: ringbuf: use runtime page size
  libbpf: add bpf_map__inner_map API
  selftests/bpf: ringbuf_multi: use runtime page size

 tools/lib/bpf/libbpf.c                        | 10 +++
 tools/lib/bpf/libbpf.h                        |  1 +
 tools/lib/bpf/libbpf.map                      |  1 +
 .../selftests/bpf/prog_tests/map_ptr.c        | 15 ++++-
 tools/testing/selftests/bpf/prog_tests/mmap.c | 24 +++++--
 .../selftests/bpf/prog_tests/ringbuf.c        | 17 +++--
 .../selftests/bpf/prog_tests/ringbuf_multi.c  | 23 ++++++-
 .../selftests/bpf/prog_tests/sockopt_sk.c     | 65 +++++--------------
 .../selftests/bpf/progs/map_ptr_kern.c        |  4 +-
 .../testing/selftests/bpf/progs/sockopt_sk.c  | 11 ++--
 tools/testing/selftests/bpf/progs/test_mmap.c |  2 -
 .../selftests/bpf/progs/test_ringbuf.c        |  1 -
 .../selftests/bpf/progs/test_ringbuf_multi.c  |  1 -
 13 files changed, 101 insertions(+), 74 deletions(-)

-- 
2.31.1


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

* [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version
  2021-03-31 16:44 [PATCH bpf-next v3 0/8] bpf/selftests: page size fixes Yauheni Kaliuta
@ 2021-03-31 16:44 ` Yauheni Kaliuta
  2021-03-31 16:44   ` [PATCH bpf-next v3 2/8] selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton Yauheni Kaliuta
                     ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:44 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

As pointed by Andrii Nakryiko, _version is useless now, remove it.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/testing/selftests/bpf/progs/sockopt_sk.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c b/tools/testing/selftests/bpf/progs/sockopt_sk.c
index d3597f81e6e9..978a68005966 100644
--- a/tools/testing/selftests/bpf/progs/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c
@@ -6,7 +6,6 @@
 #include <bpf/bpf_helpers.h>
 
 char _license[] SEC("license") = "GPL";
-__u32 _version SEC("version") = 1;
 
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 4096
-- 
2.31.1


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

* [PATCH bpf-next v3 2/8] selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
@ 2021-03-31 16:44   ` Yauheni Kaliuta
  2021-03-31 16:44   ` [PATCH bpf-next v3 3/8] selftests/bpf: pass page size from userspace in sockopt_sk Yauheni Kaliuta
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:44 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

Switch the test to use BPF skeleton to save some boilerplate and
make it easy to access bpf program bss segment.

The latter will be used to pass PAGE_SIZE from userspace since there
is no convenient way for bpf program to get it from inside of the
kernel.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 .../selftests/bpf/prog_tests/sockopt_sk.c     | 65 +++++--------------
 1 file changed, 17 insertions(+), 48 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
index d5b44b135c00..7274b12abe17 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
@@ -3,6 +3,7 @@
 #include "cgroup_helpers.h"
 
 #include <linux/tcp.h>
+#include "sockopt_sk.skel.h"
 
 #ifndef SOL_TCP
 #define SOL_TCP IPPROTO_TCP
@@ -191,60 +192,28 @@ static int getsetsockopt(void)
 	return -1;
 }
 
-static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
-{
-	enum bpf_attach_type attach_type;
-	enum bpf_prog_type prog_type;
-	struct bpf_program *prog;
-	int err;
-
-	err = libbpf_prog_type_by_name(title, &prog_type, &attach_type);
-	if (err) {
-		log_err("Failed to deduct types for %s BPF program", title);
-		return -1;
-	}
-
-	prog = bpf_object__find_program_by_title(obj, title);
-	if (!prog) {
-		log_err("Failed to find %s BPF program", title);
-		return -1;
-	}
-
-	err = bpf_prog_attach(bpf_program__fd(prog), cgroup_fd,
-			      attach_type, 0);
-	if (err) {
-		log_err("Failed to attach %s BPF program", title);
-		return -1;
-	}
-
-	return 0;
-}
-
 static void run_test(int cgroup_fd)
 {
-	struct bpf_prog_load_attr attr = {
-		.file = "./sockopt_sk.o",
-	};
-	struct bpf_object *obj;
-	int ignored;
-	int err;
-
-	err = bpf_prog_load_xattr(&attr, &obj, &ignored);
-	if (CHECK_FAIL(err))
-		return;
+	struct sockopt_sk *skel;
+
+	skel = sockopt_sk__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_load"))
+		goto cleanup;
 
-	err = prog_attach(obj, cgroup_fd, "cgroup/getsockopt");
-	if (CHECK_FAIL(err))
-		goto close_bpf_object;
+	skel->links._setsockopt =
+		bpf_program__attach_cgroup(skel->progs._setsockopt, cgroup_fd);
+	if (!ASSERT_OK_PTR(skel->links._setsockopt, "setsockopt_link"))
+		goto cleanup;
 
-	err = prog_attach(obj, cgroup_fd, "cgroup/setsockopt");
-	if (CHECK_FAIL(err))
-		goto close_bpf_object;
+	skel->links._getsockopt =
+		bpf_program__attach_cgroup(skel->progs._getsockopt, cgroup_fd);
+	if (!ASSERT_OK_PTR(skel->links._getsockopt, "getsockopt_link"))
+		goto cleanup;
 
-	CHECK_FAIL(getsetsockopt());
+	ASSERT_OK(getsetsockopt(), "getsetsockopt");
 
-close_bpf_object:
-	bpf_object__close(obj);
+cleanup:
+	sockopt_sk__destroy(skel);
 }
 
 void test_sockopt_sk(void)
-- 
2.31.1


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

* [PATCH bpf-next v3 3/8] selftests/bpf: pass page size from userspace in sockopt_sk
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
  2021-03-31 16:44   ` [PATCH bpf-next v3 2/8] selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton Yauheni Kaliuta
@ 2021-03-31 16:44   ` Yauheni Kaliuta
  2021-03-31 18:44     ` Andrii Nakryiko
  2021-03-31 16:45   ` [PATCH bpf-next v3 4/8] selftests/bpf: pass page size from userspace in map_ptr Yauheni Kaliuta
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:44 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

Since there is no convenient way for bpf program to get PAGE_SIZE
from inside of the kernel, pass the value from userspace.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/testing/selftests/bpf/prog_tests/sockopt_sk.c |  2 ++
 tools/testing/selftests/bpf/progs/sockopt_sk.c      | 10 ++++------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
index 7274b12abe17..4b937e5dbaca 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
@@ -200,6 +200,8 @@ static void run_test(int cgroup_fd)
 	if (!ASSERT_OK_PTR(skel, "skel_load"))
 		goto cleanup;
 
+	skel->bss->page_size = getpagesize();
+
 	skel->links._setsockopt =
 		bpf_program__attach_cgroup(skel->progs._setsockopt, cgroup_fd);
 	if (!ASSERT_OK_PTR(skel->links._setsockopt, "setsockopt_link"))
diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c b/tools/testing/selftests/bpf/progs/sockopt_sk.c
index 978a68005966..d6d03f64e2e4 100644
--- a/tools/testing/selftests/bpf/progs/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c
@@ -7,9 +7,7 @@
 
 char _license[] SEC("license") = "GPL";
 
-#ifndef PAGE_SIZE
-#define PAGE_SIZE 4096
-#endif
+int page_size; /* userspace should set it */
 
 #ifndef SOL_TCP
 #define SOL_TCP IPPROTO_TCP
@@ -89,7 +87,7 @@ int _getsockopt(struct bpf_sockopt *ctx)
 		 * program can only see the first PAGE_SIZE
 		 * bytes of data.
 		 */
-		if (optval_end - optval != PAGE_SIZE)
+		if (optval_end - optval != page_size)
 			return 0; /* EPERM, unexpected data size */
 
 		return 1;
@@ -160,7 +158,7 @@ int _setsockopt(struct bpf_sockopt *ctx)
 
 	if (ctx->level == SOL_IP && ctx->optname == IP_FREEBIND) {
 		/* Original optlen is larger than PAGE_SIZE. */
-		if (ctx->optlen != PAGE_SIZE * 2)
+		if (ctx->optlen != page_size * 2)
 			return 0; /* EPERM, unexpected data size */
 
 		if (optval + 1 > optval_end)
@@ -174,7 +172,7 @@ int _setsockopt(struct bpf_sockopt *ctx)
 		 * program can only see the first PAGE_SIZE
 		 * bytes of data.
 		 */
-		if (optval_end - optval != PAGE_SIZE)
+		if (optval_end - optval != page_size)
 			return 0; /* EPERM, unexpected data size */
 
 		return 1;
-- 
2.31.1


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

* Re: [PATCH bpf-next v3 3/8] selftests/bpf: pass page size from userspace in sockopt_sk
  2021-03-31 16:44   ` [PATCH bpf-next v3 3/8] selftests/bpf: pass page size from userspace in sockopt_sk Yauheni Kaliuta
@ 2021-03-31 18:44     ` Andrii Nakryiko
  0 siblings, 0 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2021-03-31 18:44 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, Andrii Nakryiko, Jiri Olsa

On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Since there is no convenient way for bpf program to get PAGE_SIZE
> from inside of the kernel, pass the value from userspace.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/sockopt_sk.c |  2 ++
>  tools/testing/selftests/bpf/progs/sockopt_sk.c      | 10 ++++------
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> index 7274b12abe17..4b937e5dbaca 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> @@ -200,6 +200,8 @@ static void run_test(int cgroup_fd)
>         if (!ASSERT_OK_PTR(skel, "skel_load"))
>                 goto cleanup;
>
> +       skel->bss->page_size = getpagesize();
> +
>         skel->links._setsockopt =
>                 bpf_program__attach_cgroup(skel->progs._setsockopt, cgroup_fd);
>         if (!ASSERT_OK_PTR(skel->links._setsockopt, "setsockopt_link"))
> diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c b/tools/testing/selftests/bpf/progs/sockopt_sk.c
> index 978a68005966..d6d03f64e2e4 100644
> --- a/tools/testing/selftests/bpf/progs/sockopt_sk.c
> +++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c
> @@ -7,9 +7,7 @@
>
>  char _license[] SEC("license") = "GPL";
>
> -#ifndef PAGE_SIZE
> -#define PAGE_SIZE 4096
> -#endif
> +int page_size; /* userspace should set it */

please zero-initialize this, otherwise it will cause problems on some
versions of Clang

>
>  #ifndef SOL_TCP
>  #define SOL_TCP IPPROTO_TCP
> @@ -89,7 +87,7 @@ int _getsockopt(struct bpf_sockopt *ctx)
>                  * program can only see the first PAGE_SIZE
>                  * bytes of data.
>                  */
> -               if (optval_end - optval != PAGE_SIZE)
> +               if (optval_end - optval != page_size)
>                         return 0; /* EPERM, unexpected data size */
>
>                 return 1;
> @@ -160,7 +158,7 @@ int _setsockopt(struct bpf_sockopt *ctx)
>
>         if (ctx->level == SOL_IP && ctx->optname == IP_FREEBIND) {
>                 /* Original optlen is larger than PAGE_SIZE. */
> -               if (ctx->optlen != PAGE_SIZE * 2)
> +               if (ctx->optlen != page_size * 2)
>                         return 0; /* EPERM, unexpected data size */
>
>                 if (optval + 1 > optval_end)
> @@ -174,7 +172,7 @@ int _setsockopt(struct bpf_sockopt *ctx)
>                  * program can only see the first PAGE_SIZE
>                  * bytes of data.
>                  */
> -               if (optval_end - optval != PAGE_SIZE)
> +               if (optval_end - optval != page_size)
>                         return 0; /* EPERM, unexpected data size */
>
>                 return 1;
> --
> 2.31.1
>

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

* [PATCH bpf-next v3 4/8] selftests/bpf: pass page size from userspace in map_ptr
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
  2021-03-31 16:44   ` [PATCH bpf-next v3 2/8] selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton Yauheni Kaliuta
  2021-03-31 16:44   ` [PATCH bpf-next v3 3/8] selftests/bpf: pass page size from userspace in sockopt_sk Yauheni Kaliuta
@ 2021-03-31 16:45   ` Yauheni Kaliuta
  2021-03-31 16:45   ` [PATCH bpf-next v3 5/8] selftests/bpf: mmap: use runtime page size Yauheni Kaliuta
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:45 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

Use ASSERT to check result but keep CHECK where format was used to
report error.

Use bpf_map__set_max_entries() to set map size dynamically from
userspace according to page size.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/testing/selftests/bpf/prog_tests/map_ptr.c | 15 +++++++++++++--
 tools/testing/selftests/bpf/progs/map_ptr_kern.c |  4 ++--
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/map_ptr.c b/tools/testing/selftests/bpf/prog_tests/map_ptr.c
index c230a573c373..4972f92205c7 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_ptr.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_ptr.c
@@ -12,11 +12,22 @@ void test_map_ptr(void)
 	__u32 duration = 0, retval;
 	char buf[128];
 	int err;
+	int page_size = getpagesize();
 
-	skel = map_ptr_kern__open_and_load();
-	if (CHECK(!skel, "skel_open_load", "open_load failed\n"))
+	skel = map_ptr_kern__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
 		return;
 
+	err = bpf_map__set_max_entries(skel->maps.m_ringbuf, page_size);
+	if (!ASSERT_OK(err, "bpf_map__set_max_entries"))
+		goto cleanup;
+
+	err = map_ptr_kern__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto cleanup;
+
+	skel->bss->page_size = page_size;
+
 	err = bpf_prog_test_run(bpf_program__fd(skel->progs.cg_skb), 1, &pkt_v4,
 				sizeof(pkt_v4), buf, NULL, &retval, NULL);
 
diff --git a/tools/testing/selftests/bpf/progs/map_ptr_kern.c b/tools/testing/selftests/bpf/progs/map_ptr_kern.c
index d8850bc6a9f1..0e06789ad4d2 100644
--- a/tools/testing/selftests/bpf/progs/map_ptr_kern.c
+++ b/tools/testing/selftests/bpf/progs/map_ptr_kern.c
@@ -12,6 +12,7 @@ _Static_assert(MAX_ENTRIES < LOOP_BOUND, "MAX_ENTRIES must be < LOOP_BOUND");
 
 enum bpf_map_type g_map_type = BPF_MAP_TYPE_UNSPEC;
 __u32 g_line = 0;
+int page_size; /* userspace should set it */
 
 #define VERIFY_TYPE(type, func) ({	\
 	g_map_type = type;		\
@@ -635,7 +636,6 @@ struct bpf_ringbuf_map {
 
 struct {
 	__uint(type, BPF_MAP_TYPE_RINGBUF);
-	__uint(max_entries, 1 << 12);
 } m_ringbuf SEC(".maps");
 
 static inline int check_ringbuf(void)
@@ -643,7 +643,7 @@ static inline int check_ringbuf(void)
 	struct bpf_ringbuf_map *ringbuf = (struct bpf_ringbuf_map *)&m_ringbuf;
 	struct bpf_map *map = (struct bpf_map *)&m_ringbuf;
 
-	VERIFY(check(&ringbuf->map, map, 0, 0, 1 << 12));
+	VERIFY(check(&ringbuf->map, map, 0, 0, page_size));
 
 	return 1;
 }
-- 
2.31.1


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

* [PATCH bpf-next v3 5/8] selftests/bpf: mmap: use runtime page size
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
                     ` (2 preceding siblings ...)
  2021-03-31 16:45   ` [PATCH bpf-next v3 4/8] selftests/bpf: pass page size from userspace in map_ptr Yauheni Kaliuta
@ 2021-03-31 16:45   ` Yauheni Kaliuta
  2021-03-31 16:45   ` [PATCH bpf-next v3 6/8] selftests/bpf: ringbuf: " Yauheni Kaliuta
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:45 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

Replace hardcoded 4096 with runtime value in the userspace part of
the test and set bpf table sizes dynamically according to the value.

Do not switch to ASSERT macros, keep CHECK, for consistency with the
rest of the test. Can be a separate cleanup patch.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/testing/selftests/bpf/prog_tests/mmap.c | 24 +++++++++++++++----
 tools/testing/selftests/bpf/progs/test_mmap.c |  2 --
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/mmap.c b/tools/testing/selftests/bpf/prog_tests/mmap.c
index 9c3c5c0f068f..691b54b1c7cd 100644
--- a/tools/testing/selftests/bpf/prog_tests/mmap.c
+++ b/tools/testing/selftests/bpf/prog_tests/mmap.c
@@ -29,22 +29,36 @@ void test_mmap(void)
 	struct test_mmap *skel;
 	__u64 val = 0;
 
-	skel = test_mmap__open_and_load();
-	if (CHECK(!skel, "skel_open_and_load", "skeleton open/load failed\n"))
+	skel = test_mmap__open();
+	if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
 		return;
 
+	err = bpf_map__set_max_entries(skel->maps.rdonly_map, page_size);
+	if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
+		goto cleanup;
+
+	/* at least 4 pages of data */
+	err = bpf_map__set_max_entries(skel->maps.data_map,
+				       4 * (page_size / sizeof (u64)));
+	if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
+		goto cleanup;
+
+	err = test_mmap__load(skel);
+	if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
+		goto cleanup;
+
 	bss_map = skel->maps.bss;
 	data_map = skel->maps.data_map;
 	data_map_fd = bpf_map__fd(data_map);
 
 	rdmap_fd = bpf_map__fd(skel->maps.rdonly_map);
-	tmp1 = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, rdmap_fd, 0);
+	tmp1 = mmap(NULL, page_size, PROT_READ | PROT_WRITE, MAP_SHARED, rdmap_fd, 0);
 	if (CHECK(tmp1 != MAP_FAILED, "rdonly_write_mmap", "unexpected success\n")) {
-		munmap(tmp1, 4096);
+		munmap(tmp1, page_size);
 		goto cleanup;
 	}
 	/* now double-check if it's mmap()'able at all */
-	tmp1 = mmap(NULL, 4096, PROT_READ, MAP_SHARED, rdmap_fd, 0);
+	tmp1 = mmap(NULL, page_size, PROT_READ, MAP_SHARED, rdmap_fd, 0);
 	if (CHECK(tmp1 == MAP_FAILED, "rdonly_read_mmap", "failed: %d\n", errno))
 		goto cleanup;
 
diff --git a/tools/testing/selftests/bpf/progs/test_mmap.c b/tools/testing/selftests/bpf/progs/test_mmap.c
index 4eb42cff5fe9..5a5cc19a15bf 100644
--- a/tools/testing/selftests/bpf/progs/test_mmap.c
+++ b/tools/testing/selftests/bpf/progs/test_mmap.c
@@ -9,7 +9,6 @@ char _license[] SEC("license") = "GPL";
 
 struct {
 	__uint(type, BPF_MAP_TYPE_ARRAY);
-	__uint(max_entries, 4096);
 	__uint(map_flags, BPF_F_MMAPABLE | BPF_F_RDONLY_PROG);
 	__type(key, __u32);
 	__type(value, char);
@@ -17,7 +16,6 @@ struct {
 
 struct {
 	__uint(type, BPF_MAP_TYPE_ARRAY);
-	__uint(max_entries, 512 * 4); /* at least 4 pages of data */
 	__uint(map_flags, BPF_F_MMAPABLE);
 	__type(key, __u32);
 	__type(value, __u64);
-- 
2.31.1


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

* [PATCH bpf-next v3 6/8] selftests/bpf: ringbuf: use runtime page size
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
                     ` (3 preceding siblings ...)
  2021-03-31 16:45   ` [PATCH bpf-next v3 5/8] selftests/bpf: mmap: use runtime page size Yauheni Kaliuta
@ 2021-03-31 16:45   ` Yauheni Kaliuta
  2021-03-31 16:45   ` [PATCH bpf-next v3 7/8] libbpf: add bpf_map__inner_map API Yauheni Kaliuta
  2021-03-31 16:45   ` [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size Yauheni Kaliuta
  6 siblings, 0 replies; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:45 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

Replace hardcoded 4096 with runtime value in the userspace part of
the test and set bpf table sizes dynamically according to the value.

Do not switch to ASSERT macros, keep CHECK, for consistency with the
rest of the test. Can be a separate cleanup patch.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 .../testing/selftests/bpf/prog_tests/ringbuf.c  | 17 +++++++++++++----
 .../testing/selftests/bpf/progs/test_ringbuf.c  |  1 -
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf.c b/tools/testing/selftests/bpf/prog_tests/ringbuf.c
index fddbc5db5d6a..8c9f71a816f6 100644
--- a/tools/testing/selftests/bpf/prog_tests/ringbuf.c
+++ b/tools/testing/selftests/bpf/prog_tests/ringbuf.c
@@ -87,11 +87,20 @@ void test_ringbuf(void)
 	pthread_t thread;
 	long bg_ret = -1;
 	int err, cnt;
+	int page_size = getpagesize();
 
-	skel = test_ringbuf__open_and_load();
-	if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
+	skel = test_ringbuf__open();
+	if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
 		return;
 
+	err = bpf_map__set_max_entries(skel->maps.ringbuf, page_size);
+	if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
+               goto cleanup;
+
+	err = test_ringbuf__load(skel);
+	if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
+		goto cleanup;
+
 	/* only trigger BPF program for current process */
 	skel->bss->pid = getpid();
 
@@ -110,9 +119,9 @@ void test_ringbuf(void)
 	CHECK(skel->bss->avail_data != 3 * rec_sz,
 	      "err_avail_size", "exp %ld, got %ld\n",
 	      3L * rec_sz, skel->bss->avail_data);
-	CHECK(skel->bss->ring_size != 4096,
+	CHECK(skel->bss->ring_size != page_size,
 	      "err_ring_size", "exp %ld, got %ld\n",
-	      4096L, skel->bss->ring_size);
+	      (long)page_size, skel->bss->ring_size);
 	CHECK(skel->bss->cons_pos != 0,
 	      "err_cons_pos", "exp %ld, got %ld\n",
 	      0L, skel->bss->cons_pos);
diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf.c b/tools/testing/selftests/bpf/progs/test_ringbuf.c
index 8ba9959b036b..6b3f288b7c63 100644
--- a/tools/testing/selftests/bpf/progs/test_ringbuf.c
+++ b/tools/testing/selftests/bpf/progs/test_ringbuf.c
@@ -15,7 +15,6 @@ struct sample {
 
 struct {
 	__uint(type, BPF_MAP_TYPE_RINGBUF);
-	__uint(max_entries, 1 << 12);
 } ringbuf SEC(".maps");
 
 /* inputs */
-- 
2.31.1


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

* [PATCH bpf-next v3 7/8] libbpf: add bpf_map__inner_map API
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
                     ` (4 preceding siblings ...)
  2021-03-31 16:45   ` [PATCH bpf-next v3 6/8] selftests/bpf: ringbuf: " Yauheni Kaliuta
@ 2021-03-31 16:45   ` Yauheni Kaliuta
  2021-03-31 18:48     ` Andrii Nakryiko
  2021-03-31 16:45   ` [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size Yauheni Kaliuta
  6 siblings, 1 reply; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:45 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>

The API gives access to inner map for map in map types (array or
hash of map). It will be used to dynamically set max_entries in it.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 tools/lib/bpf/libbpf.c   | 10 ++++++++++
 tools/lib/bpf/libbpf.h   |  1 +
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 12 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7aad78dbb4b4..b48dc380059d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2194,6 +2194,7 @@ static int parse_btf_map_def(struct bpf_object *obj,
 			map->inner_map = calloc(1, sizeof(*map->inner_map));
 			if (!map->inner_map)
 				return -ENOMEM;
+			map->inner_map->fd = -1;
 			map->inner_map->sec_idx = obj->efile.btf_maps_shndx;
 			map->inner_map->name = malloc(strlen(map->name) +
 						      sizeof(".inner") + 1);
@@ -3845,6 +3846,14 @@ __u32 bpf_map__max_entries(const struct bpf_map *map)
 	return map->def.max_entries;
 }
 
+struct bpf_map *bpf_map__inner_map(struct bpf_map *map)
+{
+    if (!bpf_map_type__is_map_in_map(map->def.type))
+        return NULL;
+
+    return map->inner_map;
+}
+
 int bpf_map__set_max_entries(struct bpf_map *map, __u32 max_entries)
 {
 	if (map->fd >= 0)
@@ -9476,6 +9485,7 @@ int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd)
 		pr_warn("error: inner_map_fd already specified\n");
 		return -EINVAL;
 	}
+	zfree(&map->inner_map);
 	map->inner_map_fd = fd;
 	return 0;
 }
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index f500621d28e5..bec4e6a6e31d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -480,6 +480,7 @@ LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
 LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
 
 LIBBPF_API int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd);
+LIBBPF_API struct bpf_map *bpf_map__inner_map(struct bpf_map *map);
 
 LIBBPF_API long libbpf_get_error(const void *ptr);
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index f5990f7208ce..eeb6d5ebd1cc 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -360,4 +360,5 @@ LIBBPF_0.4.0 {
 		bpf_linker__free;
 		bpf_linker__new;
 		bpf_object__set_kversion;
+		bpf_map__inner_map;
 } LIBBPF_0.3.0;
-- 
2.31.1


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

* Re: [PATCH bpf-next v3 7/8] libbpf: add bpf_map__inner_map API
  2021-03-31 16:45   ` [PATCH bpf-next v3 7/8] libbpf: add bpf_map__inner_map API Yauheni Kaliuta
@ 2021-03-31 18:48     ` Andrii Nakryiko
  0 siblings, 0 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2021-03-31 18:48 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, Andrii Nakryiko, Jiri Olsa

On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>
> The API gives access to inner map for map in map types (array or
> hash of map). It will be used to dynamically set max_entries in it.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---
>  tools/lib/bpf/libbpf.c   | 10 ++++++++++
>  tools/lib/bpf/libbpf.h   |  1 +
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 12 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 7aad78dbb4b4..b48dc380059d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2194,6 +2194,7 @@ static int parse_btf_map_def(struct bpf_object *obj,
>                         map->inner_map = calloc(1, sizeof(*map->inner_map));
>                         if (!map->inner_map)
>                                 return -ENOMEM;
> +                       map->inner_map->fd = -1;
>                         map->inner_map->sec_idx = obj->efile.btf_maps_shndx;
>                         map->inner_map->name = malloc(strlen(map->name) +
>                                                       sizeof(".inner") + 1);
> @@ -3845,6 +3846,14 @@ __u32 bpf_map__max_entries(const struct bpf_map *map)
>         return map->def.max_entries;
>  }
>
> +struct bpf_map *bpf_map__inner_map(struct bpf_map *map)
> +{
> +    if (!bpf_map_type__is_map_in_map(map->def.type))
> +        return NULL;
> +
> +    return map->inner_map;
> +}
> +
>  int bpf_map__set_max_entries(struct bpf_map *map, __u32 max_entries)
>  {
>         if (map->fd >= 0)
> @@ -9476,6 +9485,7 @@ int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd)
>                 pr_warn("error: inner_map_fd already specified\n");
>                 return -EINVAL;
>         }
> +       zfree(&map->inner_map);
>         map->inner_map_fd = fd;
>         return 0;
>  }
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index f500621d28e5..bec4e6a6e31d 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -480,6 +480,7 @@ LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
>  LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
>
>  LIBBPF_API int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd);
> +LIBBPF_API struct bpf_map *bpf_map__inner_map(struct bpf_map *map);
>
>  LIBBPF_API long libbpf_get_error(const void *ptr);
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index f5990f7208ce..eeb6d5ebd1cc 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -360,4 +360,5 @@ LIBBPF_0.4.0 {
>                 bpf_linker__free;
>                 bpf_linker__new;
>                 bpf_object__set_kversion;
> +               bpf_map__inner_map;

please keep the list sorted alphabetically

>  } LIBBPF_0.3.0;
> --
> 2.31.1
>

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

* [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size
  2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
                     ` (5 preceding siblings ...)
  2021-03-31 16:45   ` [PATCH bpf-next v3 7/8] libbpf: add bpf_map__inner_map API Yauheni Kaliuta
@ 2021-03-31 16:45   ` Yauheni Kaliuta
  2021-03-31 18:52     ` Andrii Nakryiko
  6 siblings, 1 reply; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-03-31 16:45 UTC (permalink / raw)
  To: bpf; +Cc: andrii, jolsa, Yauheni Kaliuta

Set bpf table sizes dynamically according to the runtime page size
value.

Do not switch to ASSERT macros, keep CHECK, for consistency with the
rest of the test. Can be a separate cleanup patch.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 .../selftests/bpf/prog_tests/ringbuf_multi.c  | 23 ++++++++++++++++---
 .../selftests/bpf/progs/test_ringbuf_multi.c  |  1 -
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
index d37161e59bb2..159de99621c7 100644
--- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
@@ -41,13 +41,30 @@ static int process_sample(void *ctx, void *data, size_t len)
 void test_ringbuf_multi(void)
 {
 	struct test_ringbuf_multi *skel;
-	struct ring_buffer *ringbuf;
+	struct ring_buffer *ringbuf = NULL;
 	int err;
+	int page_size = getpagesize();
 
-	skel = test_ringbuf_multi__open_and_load();
-	if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
+	skel = test_ringbuf_multi__open();
+	if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
 		return;
 
+	err = bpf_map__set_max_entries(skel->maps.ringbuf1, page_size);
+	if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
+		goto cleanup;
+
+	err = bpf_map__set_max_entries(skel->maps.ringbuf2, page_size);
+	if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
+		goto cleanup;
+
+	err = bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);
+	if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
+		goto cleanup;
+
+	err = test_ringbuf_multi__load(skel);
+	if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
+		goto cleanup;
+
 	/* only trigger BPF program for current process */
 	skel->bss->pid = getpid();
 
diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
index edf3b6953533..055c10b2ff80 100644
--- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
@@ -15,7 +15,6 @@ struct sample {
 
 struct ringbuf_map {
 	__uint(type, BPF_MAP_TYPE_RINGBUF);
-	__uint(max_entries, 1 << 12);
 } ringbuf1 SEC(".maps"),
   ringbuf2 SEC(".maps");
 
-- 
2.31.1


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

* Re: [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size
  2021-03-31 16:45   ` [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size Yauheni Kaliuta
@ 2021-03-31 18:52     ` Andrii Nakryiko
  2021-04-07 16:36       ` Yauheni Kaliuta
  0 siblings, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2021-03-31 18:52 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, Andrii Nakryiko, Jiri Olsa

On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Set bpf table sizes dynamically according to the runtime page size
> value.
>
> Do not switch to ASSERT macros, keep CHECK, for consistency with the
> rest of the test. Can be a separate cleanup patch.
>
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---
>  .../selftests/bpf/prog_tests/ringbuf_multi.c  | 23 ++++++++++++++++---
>  .../selftests/bpf/progs/test_ringbuf_multi.c  |  1 -
>  2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> index d37161e59bb2..159de99621c7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> @@ -41,13 +41,30 @@ static int process_sample(void *ctx, void *data, size_t len)
>  void test_ringbuf_multi(void)
>  {
>         struct test_ringbuf_multi *skel;
> -       struct ring_buffer *ringbuf;
> +       struct ring_buffer *ringbuf = NULL;
>         int err;
> +       int page_size = getpagesize();
>
> -       skel = test_ringbuf_multi__open_and_load();
> -       if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
> +       skel = test_ringbuf_multi__open();
> +       if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
>                 return;
>
> +       err = bpf_map__set_max_entries(skel->maps.ringbuf1, page_size);
> +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> +               goto cleanup;
> +
> +       err = bpf_map__set_max_entries(skel->maps.ringbuf2, page_size);
> +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> +               goto cleanup;
> +
> +       err = bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);
> +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> +               goto cleanup;
> +
> +       err = test_ringbuf_multi__load(skel);
> +       if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> +               goto cleanup;
> +

To test bpf_map__set_inner_map_fd() interaction with map-in-map
initialization, can you extend the test to have another map-in-map
(could be HASHMAP, just for fun), which is initialized with either
ringbuf1 or ringbuf2, but then from user-space use a different way to
override inner map definition:

int proto_fd = bpf_create_map(... RINGBUF of page_size ...);
bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
close(proto_fd);

/* perform load, it should succeed */

Important is to use a different map-in-map from ringbuf_arr, so that
load fails, unless set_inner_map_fd() properly updates internals of a
map.

>         /* only trigger BPF program for current process */
>         skel->bss->pid = getpid();
>
> diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> index edf3b6953533..055c10b2ff80 100644
> --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> @@ -15,7 +15,6 @@ struct sample {
>
>  struct ringbuf_map {
>         __uint(type, BPF_MAP_TYPE_RINGBUF);
> -       __uint(max_entries, 1 << 12);
>  } ringbuf1 SEC(".maps"),
>    ringbuf2 SEC(".maps");
>
> --
> 2.31.1
>

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

* Re: [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size
  2021-03-31 18:52     ` Andrii Nakryiko
@ 2021-04-07 16:36       ` Yauheni Kaliuta
  2021-04-07 18:05         ` Andrii Nakryiko
  0 siblings, 1 reply; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-04-07 16:36 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, Andrii Nakryiko, Jiri Olsa

Hi, Andrii!

On Wed, Mar 31, 2021 at 9:52 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
> <yauheni.kaliuta@redhat.com> wrote:
> >
> > Set bpf table sizes dynamically according to the runtime page size
> > value.
> >
> > Do not switch to ASSERT macros, keep CHECK, for consistency with the
> > rest of the test. Can be a separate cleanup patch.
> >
> > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> > ---
> >  .../selftests/bpf/prog_tests/ringbuf_multi.c  | 23 ++++++++++++++++---
> >  .../selftests/bpf/progs/test_ringbuf_multi.c  |  1 -
> >  2 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > index d37161e59bb2..159de99621c7 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > @@ -41,13 +41,30 @@ static int process_sample(void *ctx, void *data, size_t len)
> >  void test_ringbuf_multi(void)
> >  {
> >         struct test_ringbuf_multi *skel;
> > -       struct ring_buffer *ringbuf;
> > +       struct ring_buffer *ringbuf = NULL;
> >         int err;
> > +       int page_size = getpagesize();
> >
> > -       skel = test_ringbuf_multi__open_and_load();
> > -       if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
> > +       skel = test_ringbuf_multi__open();
> > +       if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> >                 return;
> >
> > +       err = bpf_map__set_max_entries(skel->maps.ringbuf1, page_size);
> > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > +               goto cleanup;
> > +
> > +       err = bpf_map__set_max_entries(skel->maps.ringbuf2, page_size);
> > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > +               goto cleanup;
> > +
> > +       err = bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);
> > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > +               goto cleanup;
> > +
> > +       err = test_ringbuf_multi__load(skel);
> > +       if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> > +               goto cleanup;
> > +
>
> To test bpf_map__set_inner_map_fd() interaction with map-in-map
> initialization, can you extend the test to have another map-in-map
> (could be HASHMAP, just for fun), which is initialized with either
> ringbuf1 or ringbuf2, but then from user-space use a different way to
> override inner map definition:
>
> int proto_fd = bpf_create_map(... RINGBUF of page_size ...);
> bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> close(proto_fd);
>
> /* perform load, it should succeed */
>
> Important is to use a different map-in-map from ringbuf_arr, so that
> load fails, unless set_inner_map_fd() properly updates internals of a
> map.

Is that what you mean?
https://github.com/ykaliuta/linux/commit/59fedd3b00678023d04b74bac61581a04896602e

diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
index 159de99621c7..5794317d05dd 100644
--- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
@@ -44,6 +44,7 @@ void test_ringbuf_multi(void)
        struct ring_buffer *ringbuf = NULL;
        int err;
        int page_size = getpagesize();
+       int proto_fd;

        skel = test_ringbuf_multi__open();
        if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
@@ -61,6 +62,16 @@ void test_ringbuf_multi(void)
        if (CHECK(err != 0, "bpf_map__set_max_entries",
"bpf_map__set_max_entries failed\n"))
                goto cleanup;

+       proto_fd = bpf_create_map(BPF_MAP_TYPE_RINGBUF, 0, 0, page_size, 0);
+       if (CHECK(proto_fd == -1, "bpf_create_map", "bpf_create_map failed\n"))
+               goto cleanup;
+
+       err = bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
+       if (CHECK(err != 0, "bpf_map__set_inner_map_fd",
"bpf_map__set_inner_map_fd failed\n"))
+               goto cleanup;
+
+       close(proto_fd);
+
        err = test_ringbuf_multi__load(skel);
        if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
                goto cleanup;
diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
index 055c10b2ff80..197b86546dca 100644
--- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
@@ -30,6 +30,17 @@ struct {
        },
 };

+struct {
+       __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
+       __uint(max_entries, 1);
+       __type(key, int);
+       __array(values, struct ringbuf_map);
+} ringbuf_hash SEC(".maps") = {
+       .values = {
+               [0] = &ringbuf1,
+       },
+};
+


I get with it:

test_ringbuf_multi:PASS:bpf_map__set_inner_map_fd 0 nsec
libbpf: Error in bpf_create_map_xattr(ringbuf_arr):Invalid
argument(-22). Retrying without BTF.
libbpf: Error in bpf_create_map_xattr(ringbuf_hash):Invalid
argument(-22). Retrying without BTF.
libbpf: map 'ringbuf_hash': failed to create: Invalid argument(-22)
libbpf: failed to load object 'test_ringbuf_multi'
libbpf: failed to load BPF skeleton 'test_ringbuf_multi': -22
test_ringbuf_multi:FAIL:skel_load skeleton load failed
#90 ringbuf_multi:FAIL


>
> >         /* only trigger BPF program for current process */
> >         skel->bss->pid = getpid();
> >
> > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > index edf3b6953533..055c10b2ff80 100644
> > --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > @@ -15,7 +15,6 @@ struct sample {
> >
> >  struct ringbuf_map {
> >         __uint(type, BPF_MAP_TYPE_RINGBUF);
> > -       __uint(max_entries, 1 << 12);
> >  } ringbuf1 SEC(".maps"),
> >    ringbuf2 SEC(".maps");
> >
> > --
> > 2.31.1
> >
>


-- 
WBR, Yauheni


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

* Re: [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size
  2021-04-07 16:36       ` Yauheni Kaliuta
@ 2021-04-07 18:05         ` Andrii Nakryiko
  2021-04-07 18:17           ` Andrii Nakryiko
  0 siblings, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2021-04-07 18:05 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, Andrii Nakryiko, Jiri Olsa

On Wed, Apr 7, 2021 at 9:36 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Hi, Andrii!
>
> On Wed, Mar 31, 2021 at 9:52 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
> > <yauheni.kaliuta@redhat.com> wrote:
> > >
> > > Set bpf table sizes dynamically according to the runtime page size
> > > value.
> > >
> > > Do not switch to ASSERT macros, keep CHECK, for consistency with the
> > > rest of the test. Can be a separate cleanup patch.
> > >
> > > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> > > ---
> > >  .../selftests/bpf/prog_tests/ringbuf_multi.c  | 23 ++++++++++++++++---
> > >  .../selftests/bpf/progs/test_ringbuf_multi.c  |  1 -
> > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > index d37161e59bb2..159de99621c7 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > @@ -41,13 +41,30 @@ static int process_sample(void *ctx, void *data, size_t len)
> > >  void test_ringbuf_multi(void)
> > >  {
> > >         struct test_ringbuf_multi *skel;
> > > -       struct ring_buffer *ringbuf;
> > > +       struct ring_buffer *ringbuf = NULL;
> > >         int err;
> > > +       int page_size = getpagesize();
> > >
> > > -       skel = test_ringbuf_multi__open_and_load();
> > > -       if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
> > > +       skel = test_ringbuf_multi__open();
> > > +       if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> > >                 return;
> > >
> > > +       err = bpf_map__set_max_entries(skel->maps.ringbuf1, page_size);
> > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > +               goto cleanup;
> > > +
> > > +       err = bpf_map__set_max_entries(skel->maps.ringbuf2, page_size);
> > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > +               goto cleanup;
> > > +
> > > +       err = bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);
> > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > +               goto cleanup;
> > > +
> > > +       err = test_ringbuf_multi__load(skel);
> > > +       if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> > > +               goto cleanup;
> > > +
> >
> > To test bpf_map__set_inner_map_fd() interaction with map-in-map
> > initialization, can you extend the test to have another map-in-map
> > (could be HASHMAP, just for fun), which is initialized with either
> > ringbuf1 or ringbuf2, but then from user-space use a different way to
> > override inner map definition:
> >
> > int proto_fd = bpf_create_map(... RINGBUF of page_size ...);
> > bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> > close(proto_fd);
> >
> > /* perform load, it should succeed */
> >
> > Important is to use a different map-in-map from ringbuf_arr, so that
> > load fails, unless set_inner_map_fd() properly updates internals of a
> > map.
>
> Is that what you mean?

yes


> https://github.com/ykaliuta/linux/commit/59fedd3b00678023d04b74bac61581a04896602e
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> index 159de99621c7..5794317d05dd 100644
> --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> @@ -44,6 +44,7 @@ void test_ringbuf_multi(void)
>         struct ring_buffer *ringbuf = NULL;
>         int err;
>         int page_size = getpagesize();
> +       int proto_fd;
>
>         skel = test_ringbuf_multi__open();
>         if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> @@ -61,6 +62,16 @@ void test_ringbuf_multi(void)
>         if (CHECK(err != 0, "bpf_map__set_max_entries",
> "bpf_map__set_max_entries failed\n"))
>                 goto cleanup;
>
> +       proto_fd = bpf_create_map(BPF_MAP_TYPE_RINGBUF, 0, 0, page_size, 0);
> +       if (CHECK(proto_fd == -1, "bpf_create_map", "bpf_create_map failed\n"))
> +               goto cleanup;
> +
> +       err = bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> +       if (CHECK(err != 0, "bpf_map__set_inner_map_fd",
> "bpf_map__set_inner_map_fd failed\n"))
> +               goto cleanup;
> +
> +       close(proto_fd);
> +
>         err = test_ringbuf_multi__load(skel);
>         if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
>                 goto cleanup;
> diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> index 055c10b2ff80..197b86546dca 100644
> --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> @@ -30,6 +30,17 @@ struct {
>         },
>  };
>
> +struct {
> +       __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
> +       __uint(max_entries, 1);
> +       __type(key, int);
> +       __array(values, struct ringbuf_map);
> +} ringbuf_hash SEC(".maps") = {
> +       .values = {
> +               [0] = &ringbuf1,
> +       },
> +};
> +
>
>
> I get with it:
>
> test_ringbuf_multi:PASS:bpf_map__set_inner_map_fd 0 nsec
> libbpf: Error in bpf_create_map_xattr(ringbuf_arr):Invalid
> argument(-22). Retrying without BTF.
> libbpf: Error in bpf_create_map_xattr(ringbuf_hash):Invalid
> argument(-22). Retrying without BTF.
> libbpf: map 'ringbuf_hash': failed to create: Invalid argument(-22)
> libbpf: failed to load object 'test_ringbuf_multi'
> libbpf: failed to load BPF skeleton 'test_ringbuf_multi': -22
> test_ringbuf_multi:FAIL:skel_load skeleton load failed
> #90 ringbuf_multi:FAIL

Did you try to investigate why it doesn't work?

>
>
> >
> > >         /* only trigger BPF program for current process */
> > >         skel->bss->pid = getpid();
> > >
> > > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > index edf3b6953533..055c10b2ff80 100644
> > > --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > @@ -15,7 +15,6 @@ struct sample {
> > >
> > >  struct ringbuf_map {
> > >         __uint(type, BPF_MAP_TYPE_RINGBUF);
> > > -       __uint(max_entries, 1 << 12);
> > >  } ringbuf1 SEC(".maps"),
> > >    ringbuf2 SEC(".maps");
> > >
> > > --
> > > 2.31.1
> > >
> >
>
>
> --
> WBR, Yauheni
>

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

* Re: [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size
  2021-04-07 18:05         ` Andrii Nakryiko
@ 2021-04-07 18:17           ` Andrii Nakryiko
  2021-04-08  5:56             ` Yauheni Kaliuta
  0 siblings, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2021-04-07 18:17 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, Andrii Nakryiko, Jiri Olsa

On Wed, Apr 7, 2021 at 11:05 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Apr 7, 2021 at 9:36 AM Yauheni Kaliuta
> <yauheni.kaliuta@redhat.com> wrote:
> >
> > Hi, Andrii!
> >
> > On Wed, Mar 31, 2021 at 9:52 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
> > > <yauheni.kaliuta@redhat.com> wrote:
> > > >
> > > > Set bpf table sizes dynamically according to the runtime page size
> > > > value.
> > > >
> > > > Do not switch to ASSERT macros, keep CHECK, for consistency with the
> > > > rest of the test. Can be a separate cleanup patch.
> > > >
> > > > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> > > > ---
> > > >  .../selftests/bpf/prog_tests/ringbuf_multi.c  | 23 ++++++++++++++++---
> > > >  .../selftests/bpf/progs/test_ringbuf_multi.c  |  1 -
> > > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > index d37161e59bb2..159de99621c7 100644
> > > > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > @@ -41,13 +41,30 @@ static int process_sample(void *ctx, void *data, size_t len)
> > > >  void test_ringbuf_multi(void)
> > > >  {
> > > >         struct test_ringbuf_multi *skel;
> > > > -       struct ring_buffer *ringbuf;
> > > > +       struct ring_buffer *ringbuf = NULL;
> > > >         int err;
> > > > +       int page_size = getpagesize();
> > > >
> > > > -       skel = test_ringbuf_multi__open_and_load();
> > > > -       if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
> > > > +       skel = test_ringbuf_multi__open();
> > > > +       if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> > > >                 return;
> > > >
> > > > +       err = bpf_map__set_max_entries(skel->maps.ringbuf1, page_size);
> > > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > +               goto cleanup;
> > > > +
> > > > +       err = bpf_map__set_max_entries(skel->maps.ringbuf2, page_size);
> > > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > +               goto cleanup;
> > > > +
> > > > +       err = bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);
> > > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > +               goto cleanup;
> > > > +
> > > > +       err = test_ringbuf_multi__load(skel);
> > > > +       if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> > > > +               goto cleanup;
> > > > +
> > >
> > > To test bpf_map__set_inner_map_fd() interaction with map-in-map
> > > initialization, can you extend the test to have another map-in-map
> > > (could be HASHMAP, just for fun), which is initialized with either
> > > ringbuf1 or ringbuf2, but then from user-space use a different way to
> > > override inner map definition:
> > >
> > > int proto_fd = bpf_create_map(... RINGBUF of page_size ...);
> > > bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> > > close(proto_fd);
> > >
> > > /* perform load, it should succeed */
> > >
> > > Important is to use a different map-in-map from ringbuf_arr, so that
> > > load fails, unless set_inner_map_fd() properly updates internals of a
> > > map.
> >
> > Is that what you mean?
>
> yes
>
>
> > https://github.com/ykaliuta/linux/commit/59fedd3b00678023d04b74bac61581a04896602e
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > index 159de99621c7..5794317d05dd 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > @@ -44,6 +44,7 @@ void test_ringbuf_multi(void)
> >         struct ring_buffer *ringbuf = NULL;
> >         int err;
> >         int page_size = getpagesize();
> > +       int proto_fd;
> >
> >         skel = test_ringbuf_multi__open();
> >         if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> > @@ -61,6 +62,16 @@ void test_ringbuf_multi(void)
> >         if (CHECK(err != 0, "bpf_map__set_max_entries",
> > "bpf_map__set_max_entries failed\n"))
> >                 goto cleanup;
> >
> > +       proto_fd = bpf_create_map(BPF_MAP_TYPE_RINGBUF, 0, 0, page_size, 0);
> > +       if (CHECK(proto_fd == -1, "bpf_create_map", "bpf_create_map failed\n"))
> > +               goto cleanup;
> > +
> > +       err = bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> > +       if (CHECK(err != 0, "bpf_map__set_inner_map_fd",
> > "bpf_map__set_inner_map_fd failed\n"))
> > +               goto cleanup;
> > +
> > +       close(proto_fd);

I think the problem is this, you are deleting that inner map too soon,
before the kernel can get it. Try moving close(proto_fd) after the
load step.

> > +
> >         err = test_ringbuf_multi__load(skel);
> >         if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> >                 goto cleanup;
> > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > index 055c10b2ff80..197b86546dca 100644
> > --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > @@ -30,6 +30,17 @@ struct {
> >         },
> >  };
> >
> > +struct {
> > +       __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
> > +       __uint(max_entries, 1);
> > +       __type(key, int);
> > +       __array(values, struct ringbuf_map);
> > +} ringbuf_hash SEC(".maps") = {
> > +       .values = {
> > +               [0] = &ringbuf1,
> > +       },
> > +};
> > +
> >
> >
> > I get with it:
> >
> > test_ringbuf_multi:PASS:bpf_map__set_inner_map_fd 0 nsec
> > libbpf: Error in bpf_create_map_xattr(ringbuf_arr):Invalid
> > argument(-22). Retrying without BTF.
> > libbpf: Error in bpf_create_map_xattr(ringbuf_hash):Invalid
> > argument(-22). Retrying without BTF.
> > libbpf: map 'ringbuf_hash': failed to create: Invalid argument(-22)
> > libbpf: failed to load object 'test_ringbuf_multi'
> > libbpf: failed to load BPF skeleton 'test_ringbuf_multi': -22
> > test_ringbuf_multi:FAIL:skel_load skeleton load failed
> > #90 ringbuf_multi:FAIL
>
> Did you try to investigate why it doesn't work?
>
> >
> >
> > >
> > > >         /* only trigger BPF program for current process */
> > > >         skel->bss->pid = getpid();
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > > index edf3b6953533..055c10b2ff80 100644
> > > > --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > > @@ -15,7 +15,6 @@ struct sample {
> > > >
> > > >  struct ringbuf_map {
> > > >         __uint(type, BPF_MAP_TYPE_RINGBUF);
> > > > -       __uint(max_entries, 1 << 12);
> > > >  } ringbuf1 SEC(".maps"),
> > > >    ringbuf2 SEC(".maps");
> > > >
> > > > --
> > > > 2.31.1
> > > >
> > >
> >
> >
> > --
> > WBR, Yauheni
> >

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

* Re: [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size
  2021-04-07 18:17           ` Andrii Nakryiko
@ 2021-04-08  5:56             ` Yauheni Kaliuta
  0 siblings, 0 replies; 16+ messages in thread
From: Yauheni Kaliuta @ 2021-04-08  5:56 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, Andrii Nakryiko, Jiri Olsa

On Wed, Apr 7, 2021 at 9:17 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Apr 7, 2021 at 11:05 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Apr 7, 2021 at 9:36 AM Yauheni Kaliuta
> > <yauheni.kaliuta@redhat.com> wrote:
> > >
> > > Hi, Andrii!
> > >
> > > On Wed, Mar 31, 2021 at 9:52 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
> > > > <yauheni.kaliuta@redhat.com> wrote:
> > > > >
> > > > > Set bpf table sizes dynamically according to the runtime page size
> > > > > value.
> > > > >
> > > > > Do not switch to ASSERT macros, keep CHECK, for consistency with the
> > > > > rest of the test. Can be a separate cleanup patch.
> > > > >
> > > > > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> > > > > ---
> > > > >  .../selftests/bpf/prog_tests/ringbuf_multi.c  | 23 ++++++++++++++++---
> > > > >  .../selftests/bpf/progs/test_ringbuf_multi.c  |  1 -
> > > > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > > index d37161e59bb2..159de99621c7 100644
> > > > > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > > @@ -41,13 +41,30 @@ static int process_sample(void *ctx, void *data, size_t len)
> > > > >  void test_ringbuf_multi(void)
> > > > >  {
> > > > >         struct test_ringbuf_multi *skel;
> > > > > -       struct ring_buffer *ringbuf;
> > > > > +       struct ring_buffer *ringbuf = NULL;
> > > > >         int err;
> > > > > +       int page_size = getpagesize();
> > > > >
> > > > > -       skel = test_ringbuf_multi__open_and_load();
> > > > > -       if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
> > > > > +       skel = test_ringbuf_multi__open();
> > > > > +       if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> > > > >                 return;
> > > > >
> > > > > +       err = bpf_map__set_max_entries(skel->maps.ringbuf1, page_size);
> > > > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > > +               goto cleanup;
> > > > > +
> > > > > +       err = bpf_map__set_max_entries(skel->maps.ringbuf2, page_size);
> > > > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > > +               goto cleanup;
> > > > > +
> > > > > +       err = bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);
> > > > > +       if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > > +               goto cleanup;
> > > > > +
> > > > > +       err = test_ringbuf_multi__load(skel);
> > > > > +       if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> > > > > +               goto cleanup;
> > > > > +
> > > >
> > > > To test bpf_map__set_inner_map_fd() interaction with map-in-map
> > > > initialization, can you extend the test to have another map-in-map
> > > > (could be HASHMAP, just for fun), which is initialized with either
> > > > ringbuf1 or ringbuf2, but then from user-space use a different way to
> > > > override inner map definition:
> > > >
> > > > int proto_fd = bpf_create_map(... RINGBUF of page_size ...);
> > > > bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> > > > close(proto_fd);
> > > >
> > > > /* perform load, it should succeed */
> > > >
> > > > Important is to use a different map-in-map from ringbuf_arr, so that
> > > > load fails, unless set_inner_map_fd() properly updates internals of a
> > > > map.
> > >
> > > Is that what you mean?
> >
> > yes
> >
> >
> > > https://github.com/ykaliuta/linux/commit/59fedd3b00678023d04b74bac61581a04896602e
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > index 159de99621c7..5794317d05dd 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > @@ -44,6 +44,7 @@ void test_ringbuf_multi(void)
> > >         struct ring_buffer *ringbuf = NULL;
> > >         int err;
> > >         int page_size = getpagesize();
> > > +       int proto_fd;
> > >
> > >         skel = test_ringbuf_multi__open();
> > >         if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> > > @@ -61,6 +62,16 @@ void test_ringbuf_multi(void)
> > >         if (CHECK(err != 0, "bpf_map__set_max_entries",
> > > "bpf_map__set_max_entries failed\n"))
> > >                 goto cleanup;
> > >
> > > +       proto_fd = bpf_create_map(BPF_MAP_TYPE_RINGBUF, 0, 0, page_size, 0);
> > > +       if (CHECK(proto_fd == -1, "bpf_create_map", "bpf_create_map failed\n"))
> > > +               goto cleanup;
> > > +
> > > +       err = bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> > > +       if (CHECK(err != 0, "bpf_map__set_inner_map_fd",
> > > "bpf_map__set_inner_map_fd failed\n"))
> > > +               goto cleanup;
> > > +
> > > +       close(proto_fd);
>
> I think the problem is this, you are deleting that inner map too soon,
> before the kernel can get it. Try moving close(proto_fd) after the
> load step.

Ah, of course, stupid me :) Thanks, it works!

>
> > > +
> > >         err = test_ringbuf_multi__load(skel);
> > >         if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> > >                 goto cleanup;
> > > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > index 055c10b2ff80..197b86546dca 100644
> > > --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > @@ -30,6 +30,17 @@ struct {
> > >         },
> > >  };
> > >
> > > +struct {
> > > +       __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
> > > +       __uint(max_entries, 1);
> > > +       __type(key, int);
> > > +       __array(values, struct ringbuf_map);
> > > +} ringbuf_hash SEC(".maps") = {
> > > +       .values = {
> > > +               [0] = &ringbuf1,
> > > +       },
> > > +};
> > > +
> > >
> > >
> > > I get with it:
> > >
> > > test_ringbuf_multi:PASS:bpf_map__set_inner_map_fd 0 nsec
> > > libbpf: Error in bpf_create_map_xattr(ringbuf_arr):Invalid
> > > argument(-22). Retrying without BTF.
> > > libbpf: Error in bpf_create_map_xattr(ringbuf_hash):Invalid
> > > argument(-22). Retrying without BTF.
> > > libbpf: map 'ringbuf_hash': failed to create: Invalid argument(-22)
> > > libbpf: failed to load object 'test_ringbuf_multi'
> > > libbpf: failed to load BPF skeleton 'test_ringbuf_multi': -22
> > > test_ringbuf_multi:FAIL:skel_load skeleton load failed
> > > #90 ringbuf_multi:FAIL
> >
> > Did you try to investigate why it doesn't work?
> >
> > >
> > >
> > > >
> > > > >         /* only trigger BPF program for current process */
> > > > >         skel->bss->pid = getpid();
> > > > >
> > > > > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > > > index edf3b6953533..055c10b2ff80 100644
> > > > > --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > > > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > > > @@ -15,7 +15,6 @@ struct sample {
> > > > >
> > > > >  struct ringbuf_map {
> > > > >         __uint(type, BPF_MAP_TYPE_RINGBUF);
> > > > > -       __uint(max_entries, 1 << 12);
> > > > >  } ringbuf1 SEC(".maps"),
> > > > >    ringbuf2 SEC(".maps");
> > > > >
> > > > > --
> > > > > 2.31.1
> > > > >
> > > >
> > >
> > >
> > > --
> > > WBR, Yauheni
> > >
>


-- 
WBR, Yauheni


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

end of thread, other threads:[~2021-04-08  5:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31 16:44 [PATCH bpf-next v3 0/8] bpf/selftests: page size fixes Yauheni Kaliuta
2021-03-31 16:44 ` [PATCH bpf-next v3 1/8] selftests/bpf: test_progs/sockopt_sk: remove version Yauheni Kaliuta
2021-03-31 16:44   ` [PATCH bpf-next v3 2/8] selftests/bpf: test_progs/sockopt_sk: Convert to use BPF skeleton Yauheni Kaliuta
2021-03-31 16:44   ` [PATCH bpf-next v3 3/8] selftests/bpf: pass page size from userspace in sockopt_sk Yauheni Kaliuta
2021-03-31 18:44     ` Andrii Nakryiko
2021-03-31 16:45   ` [PATCH bpf-next v3 4/8] selftests/bpf: pass page size from userspace in map_ptr Yauheni Kaliuta
2021-03-31 16:45   ` [PATCH bpf-next v3 5/8] selftests/bpf: mmap: use runtime page size Yauheni Kaliuta
2021-03-31 16:45   ` [PATCH bpf-next v3 6/8] selftests/bpf: ringbuf: " Yauheni Kaliuta
2021-03-31 16:45   ` [PATCH bpf-next v3 7/8] libbpf: add bpf_map__inner_map API Yauheni Kaliuta
2021-03-31 18:48     ` Andrii Nakryiko
2021-03-31 16:45   ` [PATCH bpf-next v3 8/8] selftests/bpf: ringbuf_multi: use runtime page size Yauheni Kaliuta
2021-03-31 18:52     ` Andrii Nakryiko
2021-04-07 16:36       ` Yauheni Kaliuta
2021-04-07 18:05         ` Andrii Nakryiko
2021-04-07 18:17           ` Andrii Nakryiko
2021-04-08  5:56             ` Yauheni Kaliuta

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