All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements
@ 2022-05-09  0:41 Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 1/9] selftests/bpf: prevent skeleton generation race Andrii Nakryiko
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

This patch set is a mix of mostly mutually unrelated libbpf and selftests
fixes and improvements. Individual patches provide details on each one.

Andrii Nakryiko (9):
  selftests/bpf: prevent skeleton generation race
  libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag()
    attr
  libbpf: improve usability of field-based CO-RE helpers
  selftests/bpf: use both syntaxes for field-based CO-RE helpers
  libbpf: complete field-based CO-RE helpers with field offset helper
  selftests/bpf: add bpf_core_field_offset() tests
  libbpf: provide barrier() and barrier_var() in bpf_helpers.h
  libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary
  selftests/bpf: test libbpf's ringbuf size fix up logic

 tools/lib/bpf/bpf_core_read.h                 | 37 ++++++++++++++--
 tools/lib/bpf/bpf_helpers.h                   | 29 ++++++++++---
 tools/lib/bpf/libbpf.c                        | 42 ++++++++++++++++++-
 tools/testing/selftests/bpf/Makefile          | 10 ++---
 .../selftests/bpf/prog_tests/core_reloc.c     | 13 +++++-
 .../selftests/bpf/prog_tests/ringbuf_multi.c  | 12 ------
 .../progs/btf__core_reloc_size___diff_offs.c  |  3 ++
 .../selftests/bpf/progs/core_reloc_types.h    | 18 ++++++++
 .../selftests/bpf/progs/exhandler_kern.c      |  2 -
 tools/testing/selftests/bpf/progs/loop5.c     |  1 -
 tools/testing/selftests/bpf/progs/profiler1.c |  1 -
 tools/testing/selftests/bpf/progs/pyperf.h    |  2 -
 .../bpf/progs/test_core_reloc_existence.c     | 11 +++--
 .../bpf/progs/test_core_reloc_size.c          | 31 ++++++++++++--
 .../selftests/bpf/progs/test_pkt_access.c     |  2 -
 .../selftests/bpf/progs/test_ringbuf_multi.c  |  2 +
 16 files changed, 169 insertions(+), 47 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_size___diff_offs.c

-- 
2.30.2


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

* [PATCH bpf-next 1/9] selftests/bpf: prevent skeleton generation race
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag() attr Andrii Nakryiko
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Prevent "classic" and light skeleton generation rules from stomping on
each other's toes due to the use of the same <obj>.linked{1,2,3}.o
naming pattern. There is no coordination and synchronizataion between
.skel.h and .lskel.h rules, so they can easily overwrite each other's
intermediate object files, leading to errors like:

  /bin/sh: line 1: 170928 Bus error               (core dumped)
  /data/users/andriin/linux/tools/testing/selftests/bpf/tools/sbin/bpftool gen skeleton
  /data/users/andriin/linux/tools/testing/selftests/bpf/test_ksyms_weak.linked3.o
  name test_ksyms_weak
  > /data/users/andriin/linux/tools/testing/selftests/bpf/test_ksyms_weak.skel.h
  make: *** [Makefile:507: /data/users/andriin/linux/tools/testing/selftests/bpf/test_ksyms_weak.skel.h] Error 135
  make: *** Deleting file '/data/users/andriin/linux/tools/testing/selftests/bpf/test_ksyms_weak.skel.h'

Fix by using different suffix for light skeleton rule.

Fixes: c48e51c8b07a ("bpf: selftests: Add selftests for module kfunc support")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index bafdc5373a13..6bbc03161544 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -423,11 +423,11 @@ $(TRUNNER_BPF_SKELS): %.skel.h: %.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
 
 $(TRUNNER_BPF_LSKELS): %.lskel.h: %.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
 	$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
-	$(Q)$$(BPFTOOL) gen object $$(<:.o=.linked1.o) $$<
-	$(Q)$$(BPFTOOL) gen object $$(<:.o=.linked2.o) $$(<:.o=.linked1.o)
-	$(Q)$$(BPFTOOL) gen object $$(<:.o=.linked3.o) $$(<:.o=.linked2.o)
-	$(Q)diff $$(<:.o=.linked2.o) $$(<:.o=.linked3.o)
-	$(Q)$$(BPFTOOL) gen skeleton -L $$(<:.o=.linked3.o) name $$(notdir $$(<:.o=_lskel)) > $$@
+	$(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked1.o) $$<
+	$(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked2.o) $$(<:.o=.llinked1.o)
+	$(Q)$$(BPFTOOL) gen object $$(<:.o=.llinked3.o) $$(<:.o=.llinked2.o)
+	$(Q)diff $$(<:.o=.llinked2.o) $$(<:.o=.llinked3.o)
+	$(Q)$$(BPFTOOL) gen skeleton -L $$(<:.o=.llinked3.o) name $$(notdir $$(<:.o=_lskel)) > $$@
 
 $(TRUNNER_BPF_SKELS_LINKED): $(TRUNNER_BPF_OBJS) $(BPFTOOL) | $(TRUNNER_OUTPUT)
 	$$(call msg,LINK-BPF,$(TRUNNER_BINARY),$$(@:.skel.h=.o))
-- 
2.30.2


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

* [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag() attr
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 1/9] selftests/bpf: prevent skeleton generation race Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-12-26 11:34   ` [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref Rong Tao
  2022-05-09  0:41 ` [PATCH bpf-next 3/9] libbpf: improve usability of field-based CO-RE helpers Andrii Nakryiko
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

It will be annoying and surprising for users of __kptr and __kptr_ref if
libbpf silently ignores them just because Clang used for compilation
didn't support btf_type_tag(). It's much better to get clear compiler
error than debug BPF verifier failures later on.

Fixes: ef89654f2bc7 ("libbpf: Add kptr type tag macros to bpf_helpers.h")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/bpf_helpers.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 5de3eb267125..bbae9a057bc8 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -149,13 +149,8 @@ enum libbpf_tristate {
 
 #define __kconfig __attribute__((section(".kconfig")))
 #define __ksym __attribute__((section(".ksyms")))
-#if __has_attribute(btf_type_tag)
 #define __kptr __attribute__((btf_type_tag("kptr")))
 #define __kptr_ref __attribute__((btf_type_tag("kptr_ref")))
-#else
-#define __kptr
-#define __kptr_ref
-#endif
 
 #ifndef ___bpf_concat
 #define ___bpf_concat(a, b) a ## b
-- 
2.30.2


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

* [PATCH bpf-next 3/9] libbpf: improve usability of field-based CO-RE helpers
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 1/9] selftests/bpf: prevent skeleton generation race Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag() attr Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 4/9] selftests/bpf: use both syntaxes for " Andrii Nakryiko
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Allow to specify field reference in two ways:
  - if user has variable of necessary type, they can use variable-based
    reference (my_var.my_field or my_var_ptr->my_field). This was the
    only supported syntax up till now.
  - now, bpf_core_field_exists() and bpf_core_field_size() support also
    specifying field in a fashion similar to offsetof() macro, by
    specifying type of the containing struct/union separately and field
    name separately: bpf_core_field_exists(struct my_type, my_field).
    This forms is quite often more convenient in practice and it matches
    type-based CO-RE helpers that support specifying type by its name
    without requiring any variables.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/bpf_core_read.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index e4aa9996a550..5ad415f9051f 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -110,21 +110,38 @@ enum bpf_enum_value_kind {
 	val;								      \
 })
 
+#define ___bpf_field_ref1(field)	(field)
+#define ___bpf_field_ref2(type, field)	(((typeof(type) *)0)->field)
+#define ___bpf_field_ref(args...)					    \
+	___bpf_apply(___bpf_field_ref, ___bpf_narg(args))(args)
+
 /*
  * Convenience macro to check that field actually exists in target kernel's.
  * Returns:
  *    1, if matching field is present in target kernel;
  *    0, if no matching field found.
+ *
+ * Supports two forms:
+ *   - field reference through variable access:
+ *     bpf_core_field_exists(p->my_field);
+ *   - field reference through type and field names:
+ *     bpf_core_field_exists(struct my_type, my_field).
  */
-#define bpf_core_field_exists(field)					    \
-	__builtin_preserve_field_info(field, BPF_FIELD_EXISTS)
+#define bpf_core_field_exists(field...)					    \
+	__builtin_preserve_field_info(___bpf_field_ref(field), BPF_FIELD_EXISTS)
 
 /*
  * Convenience macro to get the byte size of a field. Works for integers,
  * struct/unions, pointers, arrays, and enums.
+ *
+ * Supports two forms:
+ *   - field reference through variable access:
+ *     bpf_core_field_size(p->my_field);
+ *   - field reference through type and field names:
+ *     bpf_core_field_size(struct my_type, my_field).
  */
-#define bpf_core_field_size(field)					    \
-	__builtin_preserve_field_info(field, BPF_FIELD_BYTE_SIZE)
+#define bpf_core_field_size(field...)					    \
+	__builtin_preserve_field_info(___bpf_field_ref(field), BPF_FIELD_BYTE_SIZE)
 
 /*
  * Convenience macro to get BTF type ID of a specified type, using a local BTF
-- 
2.30.2


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

* [PATCH bpf-next 4/9] selftests/bpf: use both syntaxes for field-based CO-RE helpers
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2022-05-09  0:41 ` [PATCH bpf-next 3/9] libbpf: improve usability of field-based CO-RE helpers Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 5/9] libbpf: complete field-based CO-RE helpers with field offset helper Andrii Nakryiko
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Excercise both supported forms of bpf_core_field_exists() and
bpf_core_field_size() helpers: variable-based field reference and
type/field name-based one.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/progs/test_core_reloc_existence.c   | 11 +++++------
 .../selftests/bpf/progs/test_core_reloc_size.c        |  8 ++++----
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_existence.c b/tools/testing/selftests/bpf/progs/test_core_reloc_existence.c
index 7e45e2bdf6cd..5b8a75097ea3 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_existence.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_existence.c
@@ -45,35 +45,34 @@ int test_core_existence(void *ctx)
 	struct core_reloc_existence_output *out = (void *)&data.out;
 
 	out->a_exists = bpf_core_field_exists(in->a);
-	if (bpf_core_field_exists(in->a))
+	if (bpf_core_field_exists(struct core_reloc_existence, a))
 		out->a_value = BPF_CORE_READ(in, a);
 	else
 		out->a_value = 0xff000001u;
 
 	out->b_exists = bpf_core_field_exists(in->b);
-	if (bpf_core_field_exists(in->b))
+	if (bpf_core_field_exists(struct core_reloc_existence, b))
 		out->b_value = BPF_CORE_READ(in, b);
 	else
 		out->b_value = 0xff000002u;
 
 	out->c_exists = bpf_core_field_exists(in->c);
-	if (bpf_core_field_exists(in->c))
+	if (bpf_core_field_exists(struct core_reloc_existence, c))
 		out->c_value = BPF_CORE_READ(in, c);
 	else
 		out->c_value = 0xff000003u;
 
 	out->arr_exists = bpf_core_field_exists(in->arr);
-	if (bpf_core_field_exists(in->arr))
+	if (bpf_core_field_exists(struct core_reloc_existence, arr))
 		out->arr_value = BPF_CORE_READ(in, arr[0]);
 	else
 		out->arr_value = 0xff000004u;
 
 	out->s_exists = bpf_core_field_exists(in->s);
-	if (bpf_core_field_exists(in->s))
+	if (bpf_core_field_exists(struct core_reloc_existence, s))
 		out->s_value = BPF_CORE_READ(in, s.x);
 	else
 		out->s_value = 0xff000005u;
 
 	return 0;
 }
-
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_size.c b/tools/testing/selftests/bpf/progs/test_core_reloc_size.c
index 7b2d576aeea1..6766addd2583 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_size.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_size.c
@@ -44,10 +44,10 @@ int test_core_size(void *ctx)
 	out->struct_sz = bpf_core_field_size(in->struct_field);
 	out->union_sz = bpf_core_field_size(in->union_field);
 	out->arr_sz = bpf_core_field_size(in->arr_field);
-	out->arr_elem_sz = bpf_core_field_size(in->arr_field[0]);
-	out->ptr_sz = bpf_core_field_size(in->ptr_field);
-	out->enum_sz = bpf_core_field_size(in->enum_field);
-	out->float_sz = bpf_core_field_size(in->float_field);
+	out->arr_elem_sz = bpf_core_field_size(struct core_reloc_size, arr_field[0]);
+	out->ptr_sz = bpf_core_field_size(struct core_reloc_size, ptr_field);
+	out->enum_sz = bpf_core_field_size(struct core_reloc_size, enum_field);
+	out->float_sz = bpf_core_field_size(struct core_reloc_size, float_field);
 
 	return 0;
 }
-- 
2.30.2


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

* [PATCH bpf-next 5/9] libbpf: complete field-based CO-RE helpers with field offset helper
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2022-05-09  0:41 ` [PATCH bpf-next 4/9] selftests/bpf: use both syntaxes for " Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 6/9] selftests/bpf: add bpf_core_field_offset() tests Andrii Nakryiko
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Add bpf_core_field_offset() helper to complete field-based CO-RE
helpers. This helper can be useful for feature-detection and for some
more advanced cases of field reading (e.g., reading flexible array members).

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/bpf_core_read.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index 5ad415f9051f..fd48b1ff59ca 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -143,6 +143,18 @@ enum bpf_enum_value_kind {
 #define bpf_core_field_size(field...)					    \
 	__builtin_preserve_field_info(___bpf_field_ref(field), BPF_FIELD_BYTE_SIZE)
 
+/*
+ * Convenience macro to get field's byte offset.
+ *
+ * Supports two forms:
+ *   - field reference through variable access:
+ *     bpf_core_field_offset(p->my_field);
+ *   - field reference through type and field names:
+ *     bpf_core_field_offset(struct my_type, my_field).
+ */
+#define bpf_core_field_offset(field...)					    \
+	__builtin_preserve_field_info(___bpf_field_ref(field), BPF_FIELD_BYTE_OFFSET)
+
 /*
  * Convenience macro to get BTF type ID of a specified type, using a local BTF
  * information. Return 32-bit unsigned integer with type ID from program's own
-- 
2.30.2


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

* [PATCH bpf-next 6/9] selftests/bpf: add bpf_core_field_offset() tests
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2022-05-09  0:41 ` [PATCH bpf-next 5/9] libbpf: complete field-based CO-RE helpers with field offset helper Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 7/9] libbpf: provide barrier() and barrier_var() in bpf_helpers.h Andrii Nakryiko
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Add test cases for bpf_core_field_offset() helper.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/prog_tests/core_reloc.c     | 13 ++++++++--
 .../progs/btf__core_reloc_size___diff_offs.c  |  3 +++
 .../selftests/bpf/progs/core_reloc_types.h    | 18 +++++++++++++
 .../bpf/progs/test_core_reloc_size.c          | 25 ++++++++++++++++++-
 4 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_size___diff_offs.c

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index f28f75aa9154..3712dfe1be59 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -277,13 +277,21 @@ static int duration = 0;
 #define SIZE_OUTPUT_DATA(type)						\
 	STRUCT_TO_CHAR_PTR(core_reloc_size_output) {			\
 		.int_sz = sizeof(((type *)0)->int_field),		\
+		.int_off = offsetof(type, int_field),			\
 		.struct_sz = sizeof(((type *)0)->struct_field),		\
+		.struct_off = offsetof(type, struct_field),		\
 		.union_sz = sizeof(((type *)0)->union_field),		\
+		.union_off = offsetof(type, union_field),		\
 		.arr_sz = sizeof(((type *)0)->arr_field),		\
-		.arr_elem_sz = sizeof(((type *)0)->arr_field[0]),	\
+		.arr_off = offsetof(type, arr_field),			\
+		.arr_elem_sz = sizeof(((type *)0)->arr_field[1]),	\
+		.arr_elem_off = offsetof(type, arr_field[1]),		\
 		.ptr_sz = 8, /* always 8-byte pointer for BPF */	\
+		.ptr_off = offsetof(type, ptr_field),			\
 		.enum_sz = sizeof(((type *)0)->enum_field),		\
+		.enum_off = offsetof(type, enum_field),			\
 		.float_sz = sizeof(((type *)0)->float_field),		\
+		.float_off = offsetof(type, float_field),		\
 	}
 
 #define SIZE_CASE(name) {						\
@@ -714,9 +722,10 @@ static const struct core_reloc_test_case test_cases[] = {
 	}),
 	BITFIELDS_ERR_CASE(bitfields___err_too_big_bitfield),
 
-	/* size relocation checks */
+	/* field size and offset relocation checks */
 	SIZE_CASE(size),
 	SIZE_CASE(size___diff_sz),
+	SIZE_CASE(size___diff_offs),
 	SIZE_ERR_CASE(size___err_ambiguous),
 
 	/* validate type existence and size relocations */
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_size___diff_offs.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_size___diff_offs.c
new file mode 100644
index 000000000000..3824345d82ab
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_size___diff_offs.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_size___diff_offs x) {}
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index c95c0cabe951..f9dc9766546e 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -785,13 +785,21 @@ struct core_reloc_bitfields___err_too_big_bitfield {
  */
 struct core_reloc_size_output {
 	int int_sz;
+	int int_off;
 	int struct_sz;
+	int struct_off;
 	int union_sz;
+	int union_off;
 	int arr_sz;
+	int arr_off;
 	int arr_elem_sz;
+	int arr_elem_off;
 	int ptr_sz;
+	int ptr_off;
 	int enum_sz;
+	int enum_off;
 	int float_sz;
+	int float_off;
 };
 
 struct core_reloc_size {
@@ -814,6 +822,16 @@ struct core_reloc_size___diff_sz {
 	double float_field;
 };
 
+struct core_reloc_size___diff_offs {
+	float float_field;
+	enum { YET_OTHER_VALUE = 123 } enum_field;
+	void *ptr_field;
+	int arr_field[4];
+	union { int x; } union_field;
+	struct { int x; } struct_field;
+	int int_field;
+};
+
 /* Error case of two candidates with the fields (int_field) at the same
  * offset, but with differing final relocation values: size 4 vs size 1
  */
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_size.c b/tools/testing/selftests/bpf/progs/test_core_reloc_size.c
index 6766addd2583..5b686053ce42 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_size.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_size.c
@@ -15,13 +15,21 @@ struct {
 
 struct core_reloc_size_output {
 	int int_sz;
+	int int_off;
 	int struct_sz;
+	int struct_off;
 	int union_sz;
+	int union_off;
 	int arr_sz;
+	int arr_off;
 	int arr_elem_sz;
+	int arr_elem_off;
 	int ptr_sz;
+	int ptr_off;
 	int enum_sz;
+	int enum_off;
 	int float_sz;
+	int float_off;
 };
 
 struct core_reloc_size {
@@ -41,13 +49,28 @@ int test_core_size(void *ctx)
 	struct core_reloc_size_output *out = (void *)&data.out;
 
 	out->int_sz = bpf_core_field_size(in->int_field);
+	out->int_off = bpf_core_field_offset(in->int_field);
+
 	out->struct_sz = bpf_core_field_size(in->struct_field);
+	out->struct_off = bpf_core_field_offset(in->struct_field);
+
 	out->union_sz = bpf_core_field_size(in->union_field);
+	out->union_off = bpf_core_field_offset(in->union_field);
+
 	out->arr_sz = bpf_core_field_size(in->arr_field);
-	out->arr_elem_sz = bpf_core_field_size(struct core_reloc_size, arr_field[0]);
+	out->arr_off = bpf_core_field_offset(in->arr_field);
+
+	out->arr_elem_sz = bpf_core_field_size(struct core_reloc_size, arr_field[1]);
+	out->arr_elem_off = bpf_core_field_offset(struct core_reloc_size, arr_field[1]);
+
 	out->ptr_sz = bpf_core_field_size(struct core_reloc_size, ptr_field);
+	out->ptr_off = bpf_core_field_offset(struct core_reloc_size, ptr_field);
+
 	out->enum_sz = bpf_core_field_size(struct core_reloc_size, enum_field);
+	out->enum_off = bpf_core_field_offset(struct core_reloc_size, enum_field);
+
 	out->float_sz = bpf_core_field_size(struct core_reloc_size, float_field);
+	out->float_off = bpf_core_field_offset(struct core_reloc_size, float_field);
 
 	return 0;
 }
-- 
2.30.2


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

* [PATCH bpf-next 7/9] libbpf: provide barrier() and barrier_var() in bpf_helpers.h
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
                   ` (5 preceding siblings ...)
  2022-05-09  0:41 ` [PATCH bpf-next 6/9] selftests/bpf: add bpf_core_field_offset() tests Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-09  0:41 ` [PATCH bpf-next 8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary Andrii Nakryiko
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Add barrier() and barrier_var() macros into bpf_helpers.h to be used by
end users. While a bit advanced and specialized instruments, they are
sometimes indispensable. Instead of requiring each user to figure out
exact asm volatile incantations for themselves, provide them from
bpf_helpers.h.

Also remove conflicting definitions from selftests. Some tests rely on
barrier_var() definition being nothing, those will still work as libbpf
does the #ifndef/#endif guarding for barrier() and barrier_var(),
allowing users to redefine them, if necessary.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/bpf_helpers.h                   | 24 +++++++++++++++++++
 .../selftests/bpf/progs/exhandler_kern.c      |  2 --
 tools/testing/selftests/bpf/progs/loop5.c     |  1 -
 tools/testing/selftests/bpf/progs/profiler1.c |  1 -
 tools/testing/selftests/bpf/progs/pyperf.h    |  2 --
 .../selftests/bpf/progs/test_pkt_access.c     |  2 --
 6 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index bbae9a057bc8..fb04eaf367f1 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -75,6 +75,30 @@
 	})
 #endif
 
+/*
+ * Compiler (optimization) barrier.
+ */
+#ifndef barrier
+#define barrier() asm volatile("" ::: "memory")
+#endif
+
+/* Variable-specific compiler (optimization) barrier. It's a no-op which makes
+ * compiler believe that there is some black box modification of a given
+ * variable and thus prevents compiler from making extra assumption about its
+ * value and potential simplifications and optimizations on this variable.
+ *
+ * E.g., compiler might often delay or even omit 32-bit to 64-bit casting of
+ * a variable, making some code patterns unverifiable. Putting barrier_var()
+ * in place will ensure that cast is performed before the barrier_var()
+ * invocation, because compiler has to pessimistically assume that embedded
+ * asm section might perform some extra operations on that variable.
+ *
+ * This is a variable-specific variant of more global barrier().
+ */
+#ifndef barrier_var
+#define barrier_var(var) asm volatile("" : "=r"(var) : "0"(var))
+#endif
+
 /*
  * Helper macro to throw a compilation error if __bpf_unreachable() gets
  * built into the resulting code. This works given BPF back end does not
diff --git a/tools/testing/selftests/bpf/progs/exhandler_kern.c b/tools/testing/selftests/bpf/progs/exhandler_kern.c
index dd9b30a0f0fc..20d009e2d266 100644
--- a/tools/testing/selftests/bpf/progs/exhandler_kern.c
+++ b/tools/testing/selftests/bpf/progs/exhandler_kern.c
@@ -7,8 +7,6 @@
 #include <bpf/bpf_tracing.h>
 #include <bpf/bpf_core_read.h>
 
-#define barrier_var(var) asm volatile("" : "=r"(var) : "0"(var))
-
 char _license[] SEC("license") = "GPL";
 
 unsigned int exception_triggered;
diff --git a/tools/testing/selftests/bpf/progs/loop5.c b/tools/testing/selftests/bpf/progs/loop5.c
index 913791923fa3..1b13f37f85ec 100644
--- a/tools/testing/selftests/bpf/progs/loop5.c
+++ b/tools/testing/selftests/bpf/progs/loop5.c
@@ -2,7 +2,6 @@
 // Copyright (c) 2019 Facebook
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
-#define barrier() __asm__ __volatile__("": : :"memory")
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/profiler1.c b/tools/testing/selftests/bpf/progs/profiler1.c
index 4df9088bfc00..fb6b13522949 100644
--- a/tools/testing/selftests/bpf/progs/profiler1.c
+++ b/tools/testing/selftests/bpf/progs/profiler1.c
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2020 Facebook */
-#define barrier_var(var) asm volatile("" : "=r"(var) : "0"(var))
 #define UNROLL
 #define INLINE __always_inline
 #include "profiler.inc.h"
diff --git a/tools/testing/selftests/bpf/progs/pyperf.h b/tools/testing/selftests/bpf/progs/pyperf.h
index 5d3dc4d66d47..6c7b1fb268d6 100644
--- a/tools/testing/selftests/bpf/progs/pyperf.h
+++ b/tools/testing/selftests/bpf/progs/pyperf.h
@@ -171,8 +171,6 @@ struct process_frame_ctx {
 	bool done;
 };
 
-#define barrier_var(var) asm volatile("" : "=r"(var) : "0"(var))
-
 static int process_frame_callback(__u32 i, struct process_frame_ctx *ctx)
 {
 	int zero = 0;
diff --git a/tools/testing/selftests/bpf/progs/test_pkt_access.c b/tools/testing/selftests/bpf/progs/test_pkt_access.c
index 0558544e1ff0..5cd7c096f62d 100644
--- a/tools/testing/selftests/bpf/progs/test_pkt_access.c
+++ b/tools/testing/selftests/bpf/progs/test_pkt_access.c
@@ -14,8 +14,6 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_endian.h>
 
-#define barrier() __asm__ __volatile__("": : :"memory")
-
 /* llvm will optimize both subprograms into exactly the same BPF assembly
  *
  * Disassembly of section .text:
-- 
2.30.2


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

* [PATCH bpf-next 8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
                   ` (6 preceding siblings ...)
  2022-05-09  0:41 ` [PATCH bpf-next 7/9] libbpf: provide barrier() and barrier_var() in bpf_helpers.h Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-10 15:34   ` Nathan Chancellor
  2022-05-09  0:41 ` [PATCH bpf-next 9/9] selftests/bpf: test libbpf's ringbuf size fix up logic Andrii Nakryiko
  2022-05-09 15:20 ` [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements patchwork-bot+netdevbpf
  9 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Kernel imposes a pretty particular restriction on ringbuf map size. It
has to be a power-of-2 multiple of page size. While generally this isn't
hard for user to satisfy, sometimes it's impossible to do this
declaratively in BPF source code or just plain inconvenient to do at
runtime.

One such example might be BPF libraries that are supposed to work on
different architectures, which might not agree on what the common page
size is.

Let libbpf find the right size for user instead, if it turns out to not
satisfy kernel requirements. If user didn't set size at all, that's most
probably a mistake so don't upsize such zero size to one full page,
though. Also we need to be careful about not overflowing __u32
max_entries.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 63c0f412266c..15117b9a4d1e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4943,6 +4943,44 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
 
 static void bpf_map__destroy(struct bpf_map *map);
 
+static bool is_pow_of_2(size_t x)
+{
+	return x && (x & (x - 1));
+}
+
+static size_t adjust_ringbuf_sz(size_t sz)
+{
+	__u32 page_sz = sysconf(_SC_PAGE_SIZE);
+	__u32 i, mul;
+
+	/* if user forgot to set any size, make sure they see error */
+	if (sz == 0)
+		return 0;
+	/* Kernel expects BPF_MAP_TYPE_RINGBUF's max_entries to be
+	 * a power-of-2 multiple of kernel's page size. If user diligently
+	 * satisified these conditions, pass the size through.
+	 */
+	if ((sz % page_sz) == 0 && is_pow_of_2(sz / page_sz))
+		return sz;
+
+	/* Otherwise find closest (page_sz * power_of_2) product bigger than
+	 * user-set size to satisfy both user size request and kernel
+	 * requirements and substitute correct max_entries for map creation.
+	 */
+	for (i = 0, mul = 1; ; i++, mul <<= 1) {
+		if (mul > UINT_MAX / page_sz) /* prevent __u32 overflow */
+			break;
+		if (mul * page_sz > sz)
+			return mul * page_sz;
+	}
+
+	/* if it's impossible to satisfy the conditions (i.e., user size is
+	 * very close to UINT_MAX but is not a power-of-2 multiple of
+	 * page_size) then just return original size and let kernel reject it
+	 */
+	return sz;
+}
+
 static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, bool is_inner)
 {
 	LIBBPF_OPTS(bpf_map_create_opts, create_attr);
@@ -4981,6 +5019,9 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
 	}
 
 	switch (def->type) {
+	case BPF_MAP_TYPE_RINGBUF:
+		map->def.max_entries = adjust_ringbuf_sz(map->def.max_entries);
+		/* fallthrough */
 	case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
 	case BPF_MAP_TYPE_CGROUP_ARRAY:
 	case BPF_MAP_TYPE_STACK_TRACE:
@@ -4994,7 +5035,6 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
 	case BPF_MAP_TYPE_SOCKHASH:
 	case BPF_MAP_TYPE_QUEUE:
 	case BPF_MAP_TYPE_STACK:
-	case BPF_MAP_TYPE_RINGBUF:
 		create_attr.btf_fd = 0;
 		create_attr.btf_key_type_id = 0;
 		create_attr.btf_value_type_id = 0;
-- 
2.30.2


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

* [PATCH bpf-next 9/9] selftests/bpf: test libbpf's ringbuf size fix up logic
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
                   ` (7 preceding siblings ...)
  2022-05-09  0:41 ` [PATCH bpf-next 8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary Andrii Nakryiko
@ 2022-05-09  0:41 ` Andrii Nakryiko
  2022-05-09 15:20 ` [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements patchwork-bot+netdevbpf
  9 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-09  0:41 UTC (permalink / raw)
  To: bpf, ast, daniel; +Cc: andrii, kernel-team

Make sure we always excercise libbpf's ringbuf map size adjustment logic
by specifying non-zero size that's definitely not a page size multiple.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../testing/selftests/bpf/prog_tests/ringbuf_multi.c | 12 ------------
 .../testing/selftests/bpf/progs/test_ringbuf_multi.c |  2 ++
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
index e945195b24c9..eb5f7f5aa81a 100644
--- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
@@ -50,18 +50,6 @@ void test_ringbuf_multi(void)
 	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;
-
 	proto_fd = bpf_map_create(BPF_MAP_TYPE_RINGBUF, NULL, 0, 0, page_size, NULL);
 	if (CHECK(proto_fd < 0, "bpf_map_create", "bpf_map_create 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 197b86546dca..e416e0ce12b7 100644
--- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
@@ -15,6 +15,8 @@ struct sample {
 
 struct ringbuf_map {
 	__uint(type, BPF_MAP_TYPE_RINGBUF);
+	/* libbpf will adjust to valid page size */
+	__uint(max_entries, 1000);
 } ringbuf1 SEC(".maps"),
   ringbuf2 SEC(".maps");
 
-- 
2.30.2


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

* Re: [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements
  2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
                   ` (8 preceding siblings ...)
  2022-05-09  0:41 ` [PATCH bpf-next 9/9] selftests/bpf: test libbpf's ringbuf size fix up logic Andrii Nakryiko
@ 2022-05-09 15:20 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 15+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-05-09 15:20 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Sun, 8 May 2022 17:41:39 -0700 you wrote:
> This patch set is a mix of mostly mutually unrelated libbpf and selftests
> fixes and improvements. Individual patches provide details on each one.
> 
> Andrii Nakryiko (9):
>   selftests/bpf: prevent skeleton generation race
>   libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag()
>     attr
>   libbpf: improve usability of field-based CO-RE helpers
>   selftests/bpf: use both syntaxes for field-based CO-RE helpers
>   libbpf: complete field-based CO-RE helpers with field offset helper
>   selftests/bpf: add bpf_core_field_offset() tests
>   libbpf: provide barrier() and barrier_var() in bpf_helpers.h
>   libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary
>   selftests/bpf: test libbpf's ringbuf size fix up logic
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/9] selftests/bpf: prevent skeleton generation race
    https://git.kernel.org/bpf/bpf-next/c/1e2666e029e5
  - [bpf-next,2/9] libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag() attr
    https://git.kernel.org/bpf/bpf-next/c/8e2f618e8be6
  - [bpf-next,3/9] libbpf: improve usability of field-based CO-RE helpers
    https://git.kernel.org/bpf/bpf-next/c/73d0280f6b79
  - [bpf-next,4/9] selftests/bpf: use both syntaxes for field-based CO-RE helpers
    https://git.kernel.org/bpf/bpf-next/c/2a4ca46b7d2a
  - [bpf-next,5/9] libbpf: complete field-based CO-RE helpers with field offset helper
    https://git.kernel.org/bpf/bpf-next/c/7715f549a9d8
  - [bpf-next,6/9] selftests/bpf: add bpf_core_field_offset() tests
    https://git.kernel.org/bpf/bpf-next/c/785c3342cf6c
  - [bpf-next,7/9] libbpf: provide barrier() and barrier_var() in bpf_helpers.h
    https://git.kernel.org/bpf/bpf-next/c/f760d0537925
  - [bpf-next,8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary
    https://git.kernel.org/bpf/bpf-next/c/0087a681fa8c
  - [bpf-next,9/9] selftests/bpf: test libbpf's ringbuf size fix up logic
    https://git.kernel.org/bpf/bpf-next/c/7b3a06382442

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH bpf-next 8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary
  2022-05-09  0:41 ` [PATCH bpf-next 8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary Andrii Nakryiko
@ 2022-05-10 15:34   ` Nathan Chancellor
  2022-05-10 18:47     ` Andrii Nakryiko
  0 siblings, 1 reply; 15+ messages in thread
From: Nathan Chancellor @ 2022-05-10 15:34 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team, llvm

Hi Andrii,

On Sun, May 08, 2022 at 05:41:47PM -0700, Andrii Nakryiko wrote:
> Kernel imposes a pretty particular restriction on ringbuf map size. It
> has to be a power-of-2 multiple of page size. While generally this isn't
> hard for user to satisfy, sometimes it's impossible to do this
> declaratively in BPF source code or just plain inconvenient to do at
> runtime.
> 
> One such example might be BPF libraries that are supposed to work on
> different architectures, which might not agree on what the common page
> size is.
> 
> Let libbpf find the right size for user instead, if it turns out to not
> satisfy kernel requirements. If user didn't set size at all, that's most
> probably a mistake so don't upsize such zero size to one full page,
> though. Also we need to be careful about not overflowing __u32
> max_entries.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

<snip>

> +static size_t adjust_ringbuf_sz(size_t sz)
> +{
> +	__u32 page_sz = sysconf(_SC_PAGE_SIZE);
> +	__u32 i, mul;
> +
> +	/* if user forgot to set any size, make sure they see error */
> +	if (sz == 0)
> +		return 0;
> +	/* Kernel expects BPF_MAP_TYPE_RINGBUF's max_entries to be
> +	 * a power-of-2 multiple of kernel's page size. If user diligently
> +	 * satisified these conditions, pass the size through.
> +	 */
> +	if ((sz % page_sz) == 0 && is_pow_of_2(sz / page_sz))
> +		return sz;
> +
> +	/* Otherwise find closest (page_sz * power_of_2) product bigger than
> +	 * user-set size to satisfy both user size request and kernel
> +	 * requirements and substitute correct max_entries for map creation.
> +	 */
> +	for (i = 0, mul = 1; ; i++, mul <<= 1) {
> +		if (mul > UINT_MAX / page_sz) /* prevent __u32 overflow */
> +			break;
> +		if (mul * page_sz > sz)
> +			return mul * page_sz;
> +	}
> +
> +	/* if it's impossible to satisfy the conditions (i.e., user size is
> +	 * very close to UINT_MAX but is not a power-of-2 multiple of
> +	 * page_size) then just return original size and let kernel reject it
> +	 */
> +	return sz;
> +}

This patch in -next as commit 0087a681fa8c ("libbpf: Automatically fix
up BPF_MAP_TYPE_RINGBUF size, if necessary") breaks the build with tip
of tree LLVM due to [1] strengthening -Wunused-but-set-variable:

libbpf.c:4954:8: error: variable 'i' set but not used [-Werror,-Wunused-but-set-variable]
        __u32 i, mul;
              ^
1 error generated.

Should i be removed or was it intended to be used somewhere that it is
not?

[1]: https://github.com/llvm/llvm-project/commit/2af845a6519c9cde5c8f58db5554f8b1084ce1ed

Cheers,
Nathan

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

* Re: [PATCH bpf-next 8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary
  2022-05-10 15:34   ` Nathan Chancellor
@ 2022-05-10 18:47     ` Andrii Nakryiko
  0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2022-05-10 18:47 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team, llvm

On Tue, May 10, 2022 at 8:34 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Hi Andrii,
>
> On Sun, May 08, 2022 at 05:41:47PM -0700, Andrii Nakryiko wrote:
> > Kernel imposes a pretty particular restriction on ringbuf map size. It
> > has to be a power-of-2 multiple of page size. While generally this isn't
> > hard for user to satisfy, sometimes it's impossible to do this
> > declaratively in BPF source code or just plain inconvenient to do at
> > runtime.
> >
> > One such example might be BPF libraries that are supposed to work on
> > different architectures, which might not agree on what the common page
> > size is.
> >
> > Let libbpf find the right size for user instead, if it turns out to not
> > satisfy kernel requirements. If user didn't set size at all, that's most
> > probably a mistake so don't upsize such zero size to one full page,
> > though. Also we need to be careful about not overflowing __u32
> > max_entries.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> <snip>
>
> > +static size_t adjust_ringbuf_sz(size_t sz)
> > +{
> > +     __u32 page_sz = sysconf(_SC_PAGE_SIZE);
> > +     __u32 i, mul;
> > +
> > +     /* if user forgot to set any size, make sure they see error */
> > +     if (sz == 0)
> > +             return 0;
> > +     /* Kernel expects BPF_MAP_TYPE_RINGBUF's max_entries to be
> > +      * a power-of-2 multiple of kernel's page size. If user diligently
> > +      * satisified these conditions, pass the size through.
> > +      */
> > +     if ((sz % page_sz) == 0 && is_pow_of_2(sz / page_sz))
> > +             return sz;
> > +
> > +     /* Otherwise find closest (page_sz * power_of_2) product bigger than
> > +      * user-set size to satisfy both user size request and kernel
> > +      * requirements and substitute correct max_entries for map creation.
> > +      */
> > +     for (i = 0, mul = 1; ; i++, mul <<= 1) {
> > +             if (mul > UINT_MAX / page_sz) /* prevent __u32 overflow */
> > +                     break;
> > +             if (mul * page_sz > sz)
> > +                     return mul * page_sz;
> > +     }
> > +
> > +     /* if it's impossible to satisfy the conditions (i.e., user size is
> > +      * very close to UINT_MAX but is not a power-of-2 multiple of
> > +      * page_size) then just return original size and let kernel reject it
> > +      */
> > +     return sz;
> > +}
>
> This patch in -next as commit 0087a681fa8c ("libbpf: Automatically fix
> up BPF_MAP_TYPE_RINGBUF size, if necessary") breaks the build with tip
> of tree LLVM due to [1] strengthening -Wunused-but-set-variable:
>
> libbpf.c:4954:8: error: variable 'i' set but not used [-Werror,-Wunused-but-set-variable]
>         __u32 i, mul;
>               ^
> 1 error generated.
>
> Should i be removed or was it intended to be used somewhere that it is
> not?

my initial implementation had some hard limit on number of iterations
which I later removed. I'll drop the i completely, thanks for pointing
out.

>
> [1]: https://github.com/llvm/llvm-project/commit/2af845a6519c9cde5c8f58db5554f8b1084ce1ed
>
> Cheers,
> Nathan

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

* Re: [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref
  2022-05-09  0:41 ` [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag() attr Andrii Nakryiko
@ 2022-12-26 11:34   ` Rong Tao
  2022-12-28 19:03     ` Yonghong Song
  0 siblings, 1 reply; 15+ messages in thread
From: Rong Tao @ 2022-12-26 11:34 UTC (permalink / raw)
  To: andrii; +Cc: ast, bpf, daniel, kernel-team

Hi, Andrii. It is much better to get an explicit compiler error than
to debug the BPF Verifier failure later. But should we let the other
selftests continue to compile?

I get the following compilation error, and the compilation is aborted:


$ make -C tools/testing/selftests/bpf/
  CLNG-BPF [test_maps] cgrp_ls_attach_cgroup.bpf.o
progs/cb_refs.c:7:29: error: unknown attribute 'btf_type_tag' ignored [-Werror,-Wunknown-attributes]
        struct prog_test_ref_kfunc __kptr_ref *ptr;
                                   ^~~~~~~~~~
/home/rongtao/Git/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:178:35: note: expanded from macro '__kptr_ref'
#define __kptr_ref __attribute__((btf_type_tag("kptr_ref")))
                                  ^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [Makefile:541: /home/rongtao/Git/linux/tools/testing/selftests/bpf/cb_refs.bpf.o] Error 1
make: *** Waiting for unfinished jobs....
In file included from progs/cgrp_kfunc_failure.c:8:
progs/cgrp_kfunc_common.h:13:16: error: unknown attribute 'btf_type_tag' ignored [-Werror,-Wunknown-attributes]
        struct cgroup __kptr_ref * cgrp;
                      ^~~~~~~~~~
/home/rongtao/Git/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:178:35: note: expanded from macro '__kptr_ref'
#define __kptr_ref __attribute__((btf_type_tag("kptr_ref")))
                                  ^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [Makefile:541: /home/rongtao/Git/linux/tools/testing/selftests/bpf/cgrp_kfunc_failure.bpf.o] Error 1
In file included from progs/cgrp_kfunc_success.c:8:
progs/cgrp_kfunc_common.h:13:16: error: unknown attribute 'btf_type_tag' ignored [-Werror,-Wunknown-attributes]
        struct cgroup __kptr_ref * cgrp;
                      ^~~~~~~~~~
/home/rongtao/Git/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:178:35: note: expanded from macro '__kptr_ref'
#define __kptr_ref __attribute__((btf_type_tag("kptr_ref")))
                                  ^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [Makefile:541: /home/rongtao/Git/linux/tools/testing/selftests/bpf/cgrp_kfunc_success.bpf.o] Error 1
make: Leaving directory '/home/rongtao/Git/linux/tools/testing/selftests/bpf'

Best wishes.
Rong Tao

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

* Re: [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref
  2022-12-26 11:34   ` [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref Rong Tao
@ 2022-12-28 19:03     ` Yonghong Song
  0 siblings, 0 replies; 15+ messages in thread
From: Yonghong Song @ 2022-12-28 19:03 UTC (permalink / raw)
  To: Rong Tao, andrii; +Cc: ast, bpf, daniel, kernel-team



On 12/26/22 3:34 AM, Rong Tao wrote:
> Hi, Andrii. It is much better to get an explicit compiler error than
> to debug the BPF Verifier failure later. But should we let the other
> selftests continue to compile?
> 
> I get the following compilation error, and the compilation is aborted:


btf_type_tag and btf_decl_tag are supported since llvm14, could you 
upgrade your compiler? If you want to truely test selftests, you should
try to use recent compilers, otherwise, some selftests may fail and
some others may be skipped.


> 
> 
> $ make -C tools/testing/selftests/bpf/
>    CLNG-BPF [test_maps] cgrp_ls_attach_cgroup.bpf.o
> progs/cb_refs.c:7:29: error: unknown attribute 'btf_type_tag' ignored [-Werror,-Wunknown-attributes]
>          struct prog_test_ref_kfunc __kptr_ref *ptr;
>                                     ^~~~~~~~~~
> /home/rongtao/Git/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:178:35: note: expanded from macro '__kptr_ref'
> #define __kptr_ref __attribute__((btf_type_tag("kptr_ref")))
>                                    ^~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> make: *** [Makefile:541: /home/rongtao/Git/linux/tools/testing/selftests/bpf/cb_refs.bpf.o] Error 1
> make: *** Waiting for unfinished jobs....
> In file included from progs/cgrp_kfunc_failure.c:8:
> progs/cgrp_kfunc_common.h:13:16: error: unknown attribute 'btf_type_tag' ignored [-Werror,-Wunknown-attributes]
>          struct cgroup __kptr_ref * cgrp;
>                        ^~~~~~~~~~
> /home/rongtao/Git/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:178:35: note: expanded from macro '__kptr_ref'
> #define __kptr_ref __attribute__((btf_type_tag("kptr_ref")))
>                                    ^~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> make: *** [Makefile:541: /home/rongtao/Git/linux/tools/testing/selftests/bpf/cgrp_kfunc_failure.bpf.o] Error 1
> In file included from progs/cgrp_kfunc_success.c:8:
> progs/cgrp_kfunc_common.h:13:16: error: unknown attribute 'btf_type_tag' ignored [-Werror,-Wunknown-attributes]
>          struct cgroup __kptr_ref * cgrp;
>                        ^~~~~~~~~~
> /home/rongtao/Git/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:178:35: note: expanded from macro '__kptr_ref'
> #define __kptr_ref __attribute__((btf_type_tag("kptr_ref")))
>                                    ^~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> make: *** [Makefile:541: /home/rongtao/Git/linux/tools/testing/selftests/bpf/cgrp_kfunc_success.bpf.o] Error 1
> make: Leaving directory '/home/rongtao/Git/linux/tools/testing/selftests/bpf'
> 
> Best wishes.
> Rong Tao

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

end of thread, other threads:[~2022-12-28 19:03 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09  0:41 [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 1/9] selftests/bpf: prevent skeleton generation race Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref unconditionally use btf_type_tag() attr Andrii Nakryiko
2022-12-26 11:34   ` [PATCH bpf-next 2/9] libbpf: make __kptr and __kptr_ref Rong Tao
2022-12-28 19:03     ` Yonghong Song
2022-05-09  0:41 ` [PATCH bpf-next 3/9] libbpf: improve usability of field-based CO-RE helpers Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 4/9] selftests/bpf: use both syntaxes for " Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 5/9] libbpf: complete field-based CO-RE helpers with field offset helper Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 6/9] selftests/bpf: add bpf_core_field_offset() tests Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 7/9] libbpf: provide barrier() and barrier_var() in bpf_helpers.h Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 8/9] libbpf: automatically fix up BPF_MAP_TYPE_RINGBUF size, if necessary Andrii Nakryiko
2022-05-10 15:34   ` Nathan Chancellor
2022-05-10 18:47     ` Andrii Nakryiko
2022-05-09  0:41 ` [PATCH bpf-next 9/9] selftests/bpf: test libbpf's ringbuf size fix up logic Andrii Nakryiko
2022-05-09 15:20 ` [PATCH bpf-next 0/9] Misc libbpf fixes and small improvements patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.