bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros
@ 2019-10-03 21:28 Andrii Nakryiko
  2019-10-03 21:28 ` [PATCH v3 bpf-next 1/7] selftests/bpf: undo GCC-specific bpf_helpers.h changes Andrii Nakryiko
                   ` (6 more replies)
  0 siblings, 7 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

This patch set makes bpf_helpers.h and bpf_endian.h a part of libbpf itself
for consumption by user BPF programs, not just selftests. It also splits off
tracing helpers into bpf_tracing.h, which also becomes part of libbpf. Some of
the legacy stuff (BPF_ANNOTATE_KV_PAIR, load_{byte,half,word}, bpf_map_def
with unsupported fields, etc, is extracted into selftests-only bpf_legacy.h.
All the selftests and samples are switched to use libbpf's headers and
selftests' ones are removed.

As part of this patch set we also add BPF_CORE_READ variadic macros, that are
simplifying BPF CO-RE reads, especially the ones that have to follow few
pointers. E.g., what in non-BPF world (and when using BCC) would be:

int x = s->a->b.c->d; /* s, a, and b.c are pointers */

today would have to be written using explicit bpf_probe_read() calls as:

  void *t;
  int x;
  bpf_probe_read(&t, sizeof(t), s->a);
  bpf_probe_read(&t, sizeof(t), ((struct b *)t)->b.c);
  bpf_probe_read(&x, sizeof(x), ((struct c *)t)->d);

This is super inconvenient and distracts from program logic a lot. Now, with
added BPF_CORE_READ() macros, you can write the above as:

  int x = BPF_CORE_READ(s, a, b.c, d);

Up to 9 levels of pointer chasing are supported, which should be enough for
any practical purpose, hopefully, without adding too much boilerplate macro
definitions (though there is admittedly some, given how variadic and recursive
C macro have to be implemented).

There is also BPF_CORE_READ_INTO() variant, which relies on caller to allocate
space for result:

  int x;
  BPF_CORE_READ_INTO(&x, s, a, b.c, d);

Result of last bpf_probe_read() call in the chain of calls is the result of
BPF_CORE_READ_INTO(). If any intermediate bpf_probe_read() aall fails, then
all the subsequent ones will fail too, so this is sufficient to know whether
overall "operation" succeeded or not. No short-circuiting of bpf_probe_read()s
is done, though.

BPF_CORE_READ_STR_INTO() is added as well, which differs from
BPF_CORE_READ_INTO() only in that last bpf_probe_read() call (to read final
field after chasing pointers) is replaced with bpf_probe_read_str(). Result of
bpf_probe_read_str() is returned as a result of BPF_CORE_READ_STR_INTO() macro
itself, so that applications can track return code and/or length of read
string.

Patch set outline:
- patch #1 undoes previously added GCC-specific bpf-helpers.h include;
- patch #2 splits off legacy stuff we don't want to carry over;
- patch #3 adjusts CO-RE reloc tests to avoid subsequent naming conflict with
  BPF_CORE_READ;
- patch #4 splits off bpf_tracing.h;
- patch #5 moves bpf_{helpers,endian,tracing}.h into libbpf and adjusts
  Makefiles to include libbpf for header search;
- patch #6 adds variadic BPF_CORE_READ() macro family, as described above;
- patch #7 adds tests to verify all possible levels of pointer nestedness for
  BPF_CORE_READ(), as well as correctness test for BPF_CORE_READ_STR_INTO().

v2->v3:
- small formatting fixes and macro () fixes (Song);

v1->v2:
- fix CO-RE reloc tests before bpf_helpers.h move (Song);
- split off legacy stuff we don't want to carry over (Daniel, Toke);
- split off bpf_tracing.h (Daniel);
- fix samples/bpf build (assuming other fixes are applied);
- switch remaining maps either to bpf_map_def_legacy or BTF-defined maps;

Andrii Nakryiko (7):
  selftests/bpf: undo GCC-specific bpf_helpers.h changes
  selftests/bpf: samples/bpf: split off legacy stuff from bpf_helpers.h
  selftests/bpf: adjust CO-RE reloc tests for new bpf_core_read() macro
  selftests/bpf: split off tracing-only helpers into bpf_tracing.h
  libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  libbpf: add BPF_CORE_READ/BPF_CORE_READ_INTO helpers
  selftests/bpf: add BPF_CORE_READ and BPF_CORE_READ_STR_INTO macro
    tests

 samples/bpf/Makefile                          |   2 +-
 samples/bpf/hbm_kern.h                        |  27 +-
 samples/bpf/map_perf_test_kern.c              |  24 +-
 samples/bpf/offwaketime_kern.c                |   1 +
 samples/bpf/parse_ldabs.c                     |   1 +
 samples/bpf/sampleip_kern.c                   |   1 +
 samples/bpf/sockex1_kern.c                    |   1 +
 samples/bpf/sockex2_kern.c                    |   1 +
 samples/bpf/sockex3_kern.c                    |   1 +
 samples/bpf/spintest_kern.c                   |   1 +
 samples/bpf/tcbpf1_kern.c                     |   1 +
 samples/bpf/test_map_in_map_kern.c            |  16 +-
 samples/bpf/test_overhead_kprobe_kern.c       |   1 +
 samples/bpf/test_probe_write_user_kern.c      |   1 +
 samples/bpf/trace_event_kern.c                |   1 +
 samples/bpf/tracex1_kern.c                    |   1 +
 samples/bpf/tracex2_kern.c                    |   1 +
 samples/bpf/tracex3_kern.c                    |   1 +
 samples/bpf/tracex4_kern.c                    |   1 +
 samples/bpf/tracex5_kern.c                    |   1 +
 tools/lib/bpf/Makefile                        |   5 +-
 .../selftests => lib}/bpf/bpf_endian.h        |   0
 .../selftests => lib}/bpf/bpf_helpers.h       | 399 +++++++-----------
 tools/lib/bpf/bpf_tracing.h                   | 195 +++++++++
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_legacy.h      |  39 ++
 .../selftests/bpf/prog_tests/core_reloc.c     |   8 +-
 .../selftests/bpf/progs/core_reloc_types.h    |   9 +
 tools/testing/selftests/bpf/progs/loop1.c     |   1 +
 tools/testing/selftests/bpf/progs/loop2.c     |   1 +
 tools/testing/selftests/bpf/progs/loop3.c     |   1 +
 .../testing/selftests/bpf/progs/sockopt_sk.c  |  13 +-
 tools/testing/selftests/bpf/progs/tcp_rtt.c   |  13 +-
 .../selftests/bpf/progs/test_btf_haskv.c      |   1 +
 .../selftests/bpf/progs/test_btf_newkv.c      |   1 +
 .../bpf/progs/test_core_reloc_arrays.c        |  10 +-
 .../bpf/progs/test_core_reloc_flavors.c       |   8 +-
 .../bpf/progs/test_core_reloc_ints.c          |  18 +-
 .../bpf/progs/test_core_reloc_kernel.c        |  60 ++-
 .../bpf/progs/test_core_reloc_misc.c          |   8 +-
 .../bpf/progs/test_core_reloc_mods.c          |  18 +-
 .../bpf/progs/test_core_reloc_nesting.c       |   6 +-
 .../bpf/progs/test_core_reloc_primitives.c    |  12 +-
 .../bpf/progs/test_core_reloc_ptr_as_arr.c    |   4 +-
 44 files changed, 592 insertions(+), 325 deletions(-)
 rename tools/{testing/selftests => lib}/bpf/bpf_endian.h (100%)
 rename tools/{testing/selftests => lib}/bpf/bpf_helpers.h (65%)
 create mode 100644 tools/lib/bpf/bpf_tracing.h
 create mode 100644 tools/testing/selftests/bpf/bpf_legacy.h

-- 
2.17.1


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

* [PATCH v3 bpf-next 1/7] selftests/bpf: undo GCC-specific bpf_helpers.h changes
  2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
@ 2019-10-03 21:28 ` Andrii Nakryiko
  2019-10-04  7:00   ` Toke Høiland-Jørgensen
  2019-10-03 21:28 ` [PATCH v3 bpf-next 2/7] selftests/bpf: samples/bpf: split off legacy stuff from bpf_helpers.h Andrii Nakryiko
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Having GCC provide its own bpf-helper.h is not the right approach and is
going to be changed. Undo bpf_helpers.h change before moving
bpf_helpers.h into libbpf.

Acked-by: Song Liu <songliubraving@fb.com>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/bpf_helpers.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 54a50699bbfd..a1d9b97b8e15 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -13,8 +13,6 @@
 			 ##__VA_ARGS__);		\
 })
 
-#ifdef __clang__
-
 /* helper macro to place programs, maps, license in
  * different sections in elf_bpf file. Section names
  * are interpreted by elf_bpf loader
@@ -258,12 +256,6 @@ struct bpf_map_def {
 	unsigned int numa_node;
 };
 
-#else
-
-#include <bpf-helpers.h>
-
-#endif
-
 #define BPF_ANNOTATE_KV_PAIR(name, type_key, type_val)		\
 	struct ____btf_map_##name {				\
 		type_key key;					\
-- 
2.17.1


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

* [PATCH v3 bpf-next 2/7] selftests/bpf: samples/bpf: split off legacy stuff from bpf_helpers.h
  2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
  2019-10-03 21:28 ` [PATCH v3 bpf-next 1/7] selftests/bpf: undo GCC-specific bpf_helpers.h changes Andrii Nakryiko
@ 2019-10-03 21:28 ` Andrii Nakryiko
  2019-10-04  7:00   ` Toke Høiland-Jørgensen
  2019-10-03 21:28 ` [PATCH v3 bpf-next 3/7] selftests/bpf: adjust CO-RE reloc tests for new bpf_core_read() macro Andrii Nakryiko
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Split off few legacy things from bpf_helpers.h into separate
bpf_legacy.h file:
- load_{byte|half|word};
- remove extra inner_idx and numa_node fields from bpf_map_def and
  introduce bpf_map_def_legacy for use in samples;
- move BPF_ANNOTATE_KV_PAIR into bpf_legacy.h.

Adjust samples and selftests accordingly by either including
bpf_legacy.h and using bpf_map_def_legacy, or switching to BTF-defined
maps altogether.

Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 samples/bpf/hbm_kern.h                        | 27 ++++++-------
 samples/bpf/map_perf_test_kern.c              | 23 +++++------
 samples/bpf/parse_ldabs.c                     |  1 +
 samples/bpf/sockex1_kern.c                    |  1 +
 samples/bpf/sockex2_kern.c                    |  1 +
 samples/bpf/sockex3_kern.c                    |  1 +
 samples/bpf/tcbpf1_kern.c                     |  1 +
 samples/bpf/test_map_in_map_kern.c            | 15 +++----
 tools/testing/selftests/bpf/bpf_helpers.h     | 24 +-----------
 tools/testing/selftests/bpf/bpf_legacy.h      | 39 +++++++++++++++++++
 .../testing/selftests/bpf/progs/sockopt_sk.c  | 13 +++----
 tools/testing/selftests/bpf/progs/tcp_rtt.c   | 13 +++----
 .../selftests/bpf/progs/test_btf_haskv.c      |  1 +
 .../selftests/bpf/progs/test_btf_newkv.c      |  1 +
 14 files changed, 91 insertions(+), 70 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/bpf_legacy.h

diff --git a/samples/bpf/hbm_kern.h b/samples/bpf/hbm_kern.h
index aa207a2eebbd..4edaf47876ca 100644
--- a/samples/bpf/hbm_kern.h
+++ b/samples/bpf/hbm_kern.h
@@ -59,21 +59,18 @@
 #define BYTES_PER_NS(delta, rate) ((((u64)(delta)) * (rate)) >> 20)
 #define BYTES_TO_NS(bytes, rate) div64_u64(((u64)(bytes)) << 20, (u64)(rate))
 
-struct bpf_map_def SEC("maps") queue_state = {
-	.type = BPF_MAP_TYPE_CGROUP_STORAGE,
-	.key_size = sizeof(struct bpf_cgroup_storage_key),
-	.value_size = sizeof(struct hbm_vqueue),
-};
-BPF_ANNOTATE_KV_PAIR(queue_state, struct bpf_cgroup_storage_key,
-		     struct hbm_vqueue);
-
-struct bpf_map_def SEC("maps") queue_stats = {
-	.type = BPF_MAP_TYPE_ARRAY,
-	.key_size = sizeof(u32),
-	.value_size = sizeof(struct hbm_queue_stats),
-	.max_entries = 1,
-};
-BPF_ANNOTATE_KV_PAIR(queue_stats, int, struct hbm_queue_stats);
+struct {
+	__uint(type, BPF_MAP_TYPE_CGROUP_STORAGE);
+	__type(key, struct bpf_cgroup_storage_key);
+	__type(value, struct hbm_vqueue);
+} queue_state SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, u32);
+	__type(value, struct hvm_queue_stats);
+} queue_stats SEC(".maps");
 
 struct hbm_pkt_info {
 	int	cwnd;
diff --git a/samples/bpf/map_perf_test_kern.c b/samples/bpf/map_perf_test_kern.c
index 2b2ffb97018b..f47ee513cb7c 100644
--- a/samples/bpf/map_perf_test_kern.c
+++ b/samples/bpf/map_perf_test_kern.c
@@ -9,25 +9,26 @@
 #include <linux/version.h>
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 
 #define MAX_ENTRIES 1000
 #define MAX_NR_CPUS 1024
 
-struct bpf_map_def SEC("maps") hash_map = {
+struct bpf_map_def_legacy SEC("maps") hash_map = {
 	.type = BPF_MAP_TYPE_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
 	.max_entries = MAX_ENTRIES,
 };
 
-struct bpf_map_def SEC("maps") lru_hash_map = {
+struct bpf_map_def_legacy SEC("maps") lru_hash_map = {
 	.type = BPF_MAP_TYPE_LRU_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
 	.max_entries = 10000,
 };
 
-struct bpf_map_def SEC("maps") nocommon_lru_hash_map = {
+struct bpf_map_def_legacy SEC("maps") nocommon_lru_hash_map = {
 	.type = BPF_MAP_TYPE_LRU_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
@@ -35,7 +36,7 @@ struct bpf_map_def SEC("maps") nocommon_lru_hash_map = {
 	.map_flags = BPF_F_NO_COMMON_LRU,
 };
 
-struct bpf_map_def SEC("maps") inner_lru_hash_map = {
+struct bpf_map_def_legacy SEC("maps") inner_lru_hash_map = {
 	.type = BPF_MAP_TYPE_LRU_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
@@ -44,20 +45,20 @@ struct bpf_map_def SEC("maps") inner_lru_hash_map = {
 	.numa_node = 0,
 };
 
-struct bpf_map_def SEC("maps") array_of_lru_hashs = {
+struct bpf_map_def_legacy SEC("maps") array_of_lru_hashs = {
 	.type = BPF_MAP_TYPE_ARRAY_OF_MAPS,
 	.key_size = sizeof(u32),
 	.max_entries = MAX_NR_CPUS,
 };
 
-struct bpf_map_def SEC("maps") percpu_hash_map = {
+struct bpf_map_def_legacy SEC("maps") percpu_hash_map = {
 	.type = BPF_MAP_TYPE_PERCPU_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
 	.max_entries = MAX_ENTRIES,
 };
 
-struct bpf_map_def SEC("maps") hash_map_alloc = {
+struct bpf_map_def_legacy SEC("maps") hash_map_alloc = {
 	.type = BPF_MAP_TYPE_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
@@ -65,7 +66,7 @@ struct bpf_map_def SEC("maps") hash_map_alloc = {
 	.map_flags = BPF_F_NO_PREALLOC,
 };
 
-struct bpf_map_def SEC("maps") percpu_hash_map_alloc = {
+struct bpf_map_def_legacy SEC("maps") percpu_hash_map_alloc = {
 	.type = BPF_MAP_TYPE_PERCPU_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
@@ -73,7 +74,7 @@ struct bpf_map_def SEC("maps") percpu_hash_map_alloc = {
 	.map_flags = BPF_F_NO_PREALLOC,
 };
 
-struct bpf_map_def SEC("maps") lpm_trie_map_alloc = {
+struct bpf_map_def_legacy SEC("maps") lpm_trie_map_alloc = {
 	.type = BPF_MAP_TYPE_LPM_TRIE,
 	.key_size = 8,
 	.value_size = sizeof(long),
@@ -81,14 +82,14 @@ struct bpf_map_def SEC("maps") lpm_trie_map_alloc = {
 	.map_flags = BPF_F_NO_PREALLOC,
 };
 
-struct bpf_map_def SEC("maps") array_map = {
+struct bpf_map_def_legacy SEC("maps") array_map = {
 	.type = BPF_MAP_TYPE_ARRAY,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
 	.max_entries = MAX_ENTRIES,
 };
 
-struct bpf_map_def SEC("maps") lru_hash_lookup_map = {
+struct bpf_map_def_legacy SEC("maps") lru_hash_lookup_map = {
 	.type = BPF_MAP_TYPE_LRU_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(long),
diff --git a/samples/bpf/parse_ldabs.c b/samples/bpf/parse_ldabs.c
index 6db6b21fdc6d..ef5892377beb 100644
--- a/samples/bpf/parse_ldabs.c
+++ b/samples/bpf/parse_ldabs.c
@@ -12,6 +12,7 @@
 #include <linux/udp.h>
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 
 #define DEFAULT_PKTGEN_UDP_PORT	9
 #define IP_MF			0x2000
diff --git a/samples/bpf/sockex1_kern.c b/samples/bpf/sockex1_kern.c
index ed18e9a4909c..f96943f443ab 100644
--- a/samples/bpf/sockex1_kern.c
+++ b/samples/bpf/sockex1_kern.c
@@ -3,6 +3,7 @@
 #include <uapi/linux/if_packet.h>
 #include <uapi/linux/ip.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 
 struct bpf_map_def SEC("maps") my_map = {
 	.type = BPF_MAP_TYPE_ARRAY,
diff --git a/samples/bpf/sockex2_kern.c b/samples/bpf/sockex2_kern.c
index f2f9dbc021b0..5566fa7d92fa 100644
--- a/samples/bpf/sockex2_kern.c
+++ b/samples/bpf/sockex2_kern.c
@@ -1,5 +1,6 @@
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 #include <uapi/linux/in.h>
 #include <uapi/linux/if.h>
 #include <uapi/linux/if_ether.h>
diff --git a/samples/bpf/sockex3_kern.c b/samples/bpf/sockex3_kern.c
index c527b57d3ec8..151dd842ecc0 100644
--- a/samples/bpf/sockex3_kern.c
+++ b/samples/bpf/sockex3_kern.c
@@ -6,6 +6,7 @@
  */
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 #include <uapi/linux/in.h>
 #include <uapi/linux/if.h>
 #include <uapi/linux/if_ether.h>
diff --git a/samples/bpf/tcbpf1_kern.c b/samples/bpf/tcbpf1_kern.c
index 274c884c87fe..ff43341bdfce 100644
--- a/samples/bpf/tcbpf1_kern.c
+++ b/samples/bpf/tcbpf1_kern.c
@@ -8,6 +8,7 @@
 #include <uapi/linux/filter.h>
 #include <uapi/linux/pkt_cls.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 
 /* compiler workaround */
 #define _htonl __builtin_bswap32
diff --git a/samples/bpf/test_map_in_map_kern.c b/samples/bpf/test_map_in_map_kern.c
index 42c44d091dd1..8101bf3dc7f7 100644
--- a/samples/bpf/test_map_in_map_kern.c
+++ b/samples/bpf/test_map_in_map_kern.c
@@ -11,11 +11,12 @@
 #include <uapi/linux/bpf.h>
 #include <uapi/linux/in6.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 
 #define MAX_NR_PORTS 65536
 
 /* map #0 */
-struct bpf_map_def SEC("maps") port_a = {
+struct bpf_map_def_legacy SEC("maps") port_a = {
 	.type = BPF_MAP_TYPE_ARRAY,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(int),
@@ -23,7 +24,7 @@ struct bpf_map_def SEC("maps") port_a = {
 };
 
 /* map #1 */
-struct bpf_map_def SEC("maps") port_h = {
+struct bpf_map_def_legacy SEC("maps") port_h = {
 	.type = BPF_MAP_TYPE_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(int),
@@ -31,7 +32,7 @@ struct bpf_map_def SEC("maps") port_h = {
 };
 
 /* map #2 */
-struct bpf_map_def SEC("maps") reg_result_h = {
+struct bpf_map_def_legacy SEC("maps") reg_result_h = {
 	.type = BPF_MAP_TYPE_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(int),
@@ -39,7 +40,7 @@ struct bpf_map_def SEC("maps") reg_result_h = {
 };
 
 /* map #3 */
-struct bpf_map_def SEC("maps") inline_result_h = {
+struct bpf_map_def_legacy SEC("maps") inline_result_h = {
 	.type = BPF_MAP_TYPE_HASH,
 	.key_size = sizeof(u32),
 	.value_size = sizeof(int),
@@ -47,7 +48,7 @@ struct bpf_map_def SEC("maps") inline_result_h = {
 };
 
 /* map #4 */ /* Test case #0 */
-struct bpf_map_def SEC("maps") a_of_port_a = {
+struct bpf_map_def_legacy SEC("maps") a_of_port_a = {
 	.type = BPF_MAP_TYPE_ARRAY_OF_MAPS,
 	.key_size = sizeof(u32),
 	.inner_map_idx = 0, /* map_fd[0] is port_a */
@@ -55,7 +56,7 @@ struct bpf_map_def SEC("maps") a_of_port_a = {
 };
 
 /* map #5 */ /* Test case #1 */
-struct bpf_map_def SEC("maps") h_of_port_a = {
+struct bpf_map_def_legacy SEC("maps") h_of_port_a = {
 	.type = BPF_MAP_TYPE_HASH_OF_MAPS,
 	.key_size = sizeof(u32),
 	.inner_map_idx = 0, /* map_fd[0] is port_a */
@@ -63,7 +64,7 @@ struct bpf_map_def SEC("maps") h_of_port_a = {
 };
 
 /* map #6 */ /* Test case #2 */
-struct bpf_map_def SEC("maps") h_of_port_h = {
+struct bpf_map_def_legacy SEC("maps") h_of_port_h = {
 	.type = BPF_MAP_TYPE_HASH_OF_MAPS,
 	.key_size = sizeof(u32),
 	.inner_map_idx = 1, /* map_fd[1] is port_h */
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index a1d9b97b8e15..7b75c38238e4 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -232,19 +232,8 @@ static long long (*bpf_tcp_gen_syncookie)(struct bpf_sock *sk, void *ip,
 					  int ip_len, void *tcp, int tcp_len) =
 	(void *) BPF_FUNC_tcp_gen_syncookie;
 
-/* llvm builtin functions that eBPF C program may use to
- * emit BPF_LD_ABS and BPF_LD_IND instructions
- */
-struct sk_buff;
-unsigned long long load_byte(void *skb,
-			     unsigned long long off) asm("llvm.bpf.load.byte");
-unsigned long long load_half(void *skb,
-			     unsigned long long off) asm("llvm.bpf.load.half");
-unsigned long long load_word(void *skb,
-			     unsigned long long off) asm("llvm.bpf.load.word");
-
 /* a helper structure used by eBPF C program
- * to describe map attributes to elf_bpf loader
+ * to describe BPF map attributes to libbpf loader
  */
 struct bpf_map_def {
 	unsigned int type;
@@ -252,19 +241,8 @@ struct bpf_map_def {
 	unsigned int value_size;
 	unsigned int max_entries;
 	unsigned int map_flags;
-	unsigned int inner_map_idx;
-	unsigned int numa_node;
 };
 
-#define BPF_ANNOTATE_KV_PAIR(name, type_key, type_val)		\
-	struct ____btf_map_##name {				\
-		type_key key;					\
-		type_val value;					\
-	};							\
-	struct ____btf_map_##name				\
-	__attribute__ ((section(".maps." #name), used))		\
-		____btf_map_##name = { }
-
 static int (*bpf_skb_load_bytes)(void *ctx, int off, void *to, int len) =
 	(void *) BPF_FUNC_skb_load_bytes;
 static int (*bpf_skb_load_bytes_relative)(void *ctx, int off, void *to, int len, __u32 start_header) =
diff --git a/tools/testing/selftests/bpf/bpf_legacy.h b/tools/testing/selftests/bpf/bpf_legacy.h
new file mode 100644
index 000000000000..6f8988738bc1
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_legacy.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+#ifndef __BPF_LEGACY__
+#define __BPF_LEGACY__
+
+/*
+ * legacy bpf_map_def with extra fields supported only by bpf_load(), do not
+ * use outside of samples/bpf
+ */
+struct bpf_map_def_legacy {
+	unsigned int type;
+	unsigned int key_size;
+	unsigned int value_size;
+	unsigned int max_entries;
+	unsigned int map_flags;
+	unsigned int inner_map_idx;
+	unsigned int numa_node;
+};
+
+#define BPF_ANNOTATE_KV_PAIR(name, type_key, type_val)		\
+	struct ____btf_map_##name {				\
+		type_key key;					\
+		type_val value;					\
+	};							\
+	struct ____btf_map_##name				\
+	__attribute__ ((section(".maps." #name), used))		\
+		____btf_map_##name = { }
+
+/* llvm builtin functions that eBPF C program may use to
+ * emit BPF_LD_ABS and BPF_LD_IND instructions
+ */
+unsigned long long load_byte(void *skb,
+			     unsigned long long off) asm("llvm.bpf.load.byte");
+unsigned long long load_half(void *skb,
+			     unsigned long long off) asm("llvm.bpf.load.half");
+unsigned long long load_word(void *skb,
+			     unsigned long long off) asm("llvm.bpf.load.word");
+
+#endif
+
diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c b/tools/testing/selftests/bpf/progs/sockopt_sk.c
index 9a3d1c79e6fe..1bafbb944e37 100644
--- a/tools/testing/selftests/bpf/progs/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c
@@ -14,13 +14,12 @@ struct sockopt_sk {
 	__u8 val;
 };
 
-struct bpf_map_def SEC("maps") socket_storage_map = {
-	.type = BPF_MAP_TYPE_SK_STORAGE,
-	.key_size = sizeof(int),
-	.value_size = sizeof(struct sockopt_sk),
-	.map_flags = BPF_F_NO_PREALLOC,
-};
-BPF_ANNOTATE_KV_PAIR(socket_storage_map, int, struct sockopt_sk);
+struct {
+	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct sockopt_sk);
+} socket_storage_map SEC(".maps");
 
 SEC("cgroup/getsockopt")
 int _getsockopt(struct bpf_sockopt *ctx)
diff --git a/tools/testing/selftests/bpf/progs/tcp_rtt.c b/tools/testing/selftests/bpf/progs/tcp_rtt.c
index 233bdcb1659e..2cf813a06b83 100644
--- a/tools/testing/selftests/bpf/progs/tcp_rtt.c
+++ b/tools/testing/selftests/bpf/progs/tcp_rtt.c
@@ -13,13 +13,12 @@ struct tcp_rtt_storage {
 	__u32 icsk_retransmits;
 };
 
-struct bpf_map_def SEC("maps") socket_storage_map = {
-	.type = BPF_MAP_TYPE_SK_STORAGE,
-	.key_size = sizeof(int),
-	.value_size = sizeof(struct tcp_rtt_storage),
-	.map_flags = BPF_F_NO_PREALLOC,
-};
-BPF_ANNOTATE_KV_PAIR(socket_storage_map, int, struct tcp_rtt_storage);
+struct {
+	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct tcp_rtt_storage);
+} socket_storage_map SEC(".maps");
 
 SEC("sockops")
 int _sockops(struct bpf_sock_ops *ctx)
diff --git a/tools/testing/selftests/bpf/progs/test_btf_haskv.c b/tools/testing/selftests/bpf/progs/test_btf_haskv.c
index e5c79fe0ffdb..763c51447c19 100644
--- a/tools/testing/selftests/bpf/progs/test_btf_haskv.c
+++ b/tools/testing/selftests/bpf/progs/test_btf_haskv.c
@@ -2,6 +2,7 @@
 /* Copyright (c) 2018 Facebook */
 #include <linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 
 int _version SEC("version") = 1;
 
diff --git a/tools/testing/selftests/bpf/progs/test_btf_newkv.c b/tools/testing/selftests/bpf/progs/test_btf_newkv.c
index 5ee3622ddebb..96f9e8451029 100644
--- a/tools/testing/selftests/bpf/progs/test_btf_newkv.c
+++ b/tools/testing/selftests/bpf/progs/test_btf_newkv.c
@@ -2,6 +2,7 @@
 /* Copyright (c) 2018 Facebook */
 #include <linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_legacy.h"
 
 int _version SEC("version") = 1;
 
-- 
2.17.1


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

* [PATCH v3 bpf-next 3/7] selftests/bpf: adjust CO-RE reloc tests for new bpf_core_read() macro
  2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
  2019-10-03 21:28 ` [PATCH v3 bpf-next 1/7] selftests/bpf: undo GCC-specific bpf_helpers.h changes Andrii Nakryiko
  2019-10-03 21:28 ` [PATCH v3 bpf-next 2/7] selftests/bpf: samples/bpf: split off legacy stuff from bpf_helpers.h Andrii Nakryiko
@ 2019-10-03 21:28 ` Andrii Nakryiko
  2019-10-03 21:28 ` [PATCH v3 bpf-next 4/7] selftests/bpf: split off tracing-only helpers into bpf_tracing.h Andrii Nakryiko
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

To allow adding a variadic BPF_CORE_READ macro with slightly different
syntax and semantics, define CORE_READ in CO-RE reloc tests, which is
a thin wrapper around low-level bpf_core_read() macro, which in turn is
just a wrapper around bpf_probe_read().

Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/bpf_helpers.h      |  8 ++++----
 .../bpf/progs/test_core_reloc_arrays.c         | 10 ++++++----
 .../bpf/progs/test_core_reloc_flavors.c        |  8 +++++---
 .../selftests/bpf/progs/test_core_reloc_ints.c | 18 ++++++++++--------
 .../bpf/progs/test_core_reloc_kernel.c         |  6 ++++--
 .../selftests/bpf/progs/test_core_reloc_misc.c |  8 +++++---
 .../selftests/bpf/progs/test_core_reloc_mods.c | 18 ++++++++++--------
 .../bpf/progs/test_core_reloc_nesting.c        |  6 ++++--
 .../bpf/progs/test_core_reloc_primitives.c     | 12 +++++++-----
 .../bpf/progs/test_core_reloc_ptr_as_arr.c     |  4 +++-
 10 files changed, 58 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 7b75c38238e4..5210cc7d7c5c 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -483,7 +483,7 @@ struct pt_regs;
 #endif
 
 /*
- * BPF_CORE_READ abstracts away bpf_probe_read() call and captures offset
+ * bpf_core_read() abstracts away bpf_probe_read() call and captures offset
  * relocation for source address using __builtin_preserve_access_index()
  * built-in, provided by Clang.
  *
@@ -498,8 +498,8 @@ struct pt_regs;
  * actual field offset, based on target kernel BTF type that matches original
  * (local) BTF, used to record relocation.
  */
-#define BPF_CORE_READ(dst, src)						\
-	bpf_probe_read((dst), sizeof(*(src)),				\
-		       __builtin_preserve_access_index(src))
+#define bpf_core_read(dst, sz, src)					    \
+	bpf_probe_read(dst, sz,						    \
+		       (const void *)__builtin_preserve_access_index(src))
 
 #endif
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_arrays.c b/tools/testing/selftests/bpf/progs/test_core_reloc_arrays.c
index bf67f0fdf743..8e3f6e6a90e7 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_arrays.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_arrays.c
@@ -31,6 +31,8 @@ struct core_reloc_arrays {
 	struct core_reloc_arrays_substruct d[1][2];
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_arrays(void *ctx)
 {
@@ -38,16 +40,16 @@ int test_core_arrays(void *ctx)
 	struct core_reloc_arrays_output *out = (void *)&data.out;
 
 	/* in->a[2] */
-	if (BPF_CORE_READ(&out->a2, &in->a[2]))
+	if (CORE_READ(&out->a2, &in->a[2]))
 		return 1;
 	/* in->b[1][2][3] */
-	if (BPF_CORE_READ(&out->b123, &in->b[1][2][3]))
+	if (CORE_READ(&out->b123, &in->b[1][2][3]))
 		return 1;
 	/* in->c[1].c */
-	if (BPF_CORE_READ(&out->c1c, &in->c[1].c))
+	if (CORE_READ(&out->c1c, &in->c[1].c))
 		return 1;
 	/* in->d[0][0].d */
-	if (BPF_CORE_READ(&out->d00d, &in->d[0][0].d))
+	if (CORE_READ(&out->d00d, &in->d[0][0].d))
 		return 1;
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_flavors.c b/tools/testing/selftests/bpf/progs/test_core_reloc_flavors.c
index 9fda73e87972..613474a18b45 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_flavors.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_flavors.c
@@ -39,6 +39,8 @@ struct core_reloc_flavors___weird {
 	};
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_flavors(void *ctx)
 {
@@ -48,13 +50,13 @@ int test_core_flavors(void *ctx)
 	struct core_reloc_flavors *out = (void *)&data.out;
 
 	/* read a using weird layout */
-	if (BPF_CORE_READ(&out->a, &in_weird->a))
+	if (CORE_READ(&out->a, &in_weird->a))
 		return 1;
 	/* read b using reversed layout */
-	if (BPF_CORE_READ(&out->b, &in_rev->b))
+	if (CORE_READ(&out->b, &in_rev->b))
 		return 1;
 	/* read c using original layout */
-	if (BPF_CORE_READ(&out->c, &in_orig->c))
+	if (CORE_READ(&out->c, &in_orig->c))
 		return 1;
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_ints.c b/tools/testing/selftests/bpf/progs/test_core_reloc_ints.c
index d99233c8008a..7a88a3975455 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_ints.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_ints.c
@@ -23,20 +23,22 @@ struct core_reloc_ints {
 	int64_t		s64_field;
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_ints(void *ctx)
 {
 	struct core_reloc_ints *in = (void *)&data.in;
 	struct core_reloc_ints *out = (void *)&data.out;
 
-	if (BPF_CORE_READ(&out->u8_field, &in->u8_field) ||
-	    BPF_CORE_READ(&out->s8_field, &in->s8_field) ||
-	    BPF_CORE_READ(&out->u16_field, &in->u16_field) ||
-	    BPF_CORE_READ(&out->s16_field, &in->s16_field) ||
-	    BPF_CORE_READ(&out->u32_field, &in->u32_field) ||
-	    BPF_CORE_READ(&out->s32_field, &in->s32_field) ||
-	    BPF_CORE_READ(&out->u64_field, &in->u64_field) ||
-	    BPF_CORE_READ(&out->s64_field, &in->s64_field))
+	if (CORE_READ(&out->u8_field, &in->u8_field) ||
+	    CORE_READ(&out->s8_field, &in->s8_field) ||
+	    CORE_READ(&out->u16_field, &in->u16_field) ||
+	    CORE_READ(&out->s16_field, &in->s16_field) ||
+	    CORE_READ(&out->u32_field, &in->u32_field) ||
+	    CORE_READ(&out->s32_field, &in->s32_field) ||
+	    CORE_READ(&out->u64_field, &in->u64_field) ||
+	    CORE_READ(&out->s64_field, &in->s64_field))
 		return 1;
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
index 37e02aa3f0c8..684a06cf41ea 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
@@ -17,6 +17,8 @@ struct task_struct {
 	int tgid;
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_kernel(void *ctx)
 {
@@ -24,8 +26,8 @@ int test_core_kernel(void *ctx)
 	uint64_t pid_tgid = bpf_get_current_pid_tgid();
 	int pid, tgid;
 
-	if (BPF_CORE_READ(&pid, &task->pid) ||
-	    BPF_CORE_READ(&tgid, &task->tgid))
+	if (CORE_READ(&pid, &task->pid) ||
+	    CORE_READ(&tgid, &task->tgid))
 		return 1;
 
 	/* validate pid + tgid matches */
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_misc.c b/tools/testing/selftests/bpf/progs/test_core_reloc_misc.c
index c59984bd3e23..10bdb2050552 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_misc.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_misc.c
@@ -32,6 +32,8 @@ struct core_reloc_misc_extensible {
 	int b;
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_misc(void *ctx)
 {
@@ -41,15 +43,15 @@ int test_core_misc(void *ctx)
 	struct core_reloc_misc_output *out = (void *)&data.out;
 
 	/* record two different relocations with the same accessor string */
-	if (BPF_CORE_READ(&out->a, &in_a->a1) ||	/* accessor: 0:0 */
-	    BPF_CORE_READ(&out->b, &in_b->b1))		/* accessor: 0:0 */
+	if (CORE_READ(&out->a, &in_a->a1) ||		/* accessor: 0:0 */
+	    CORE_READ(&out->b, &in_b->b1))		/* accessor: 0:0 */
 		return 1;
 
 	/* Validate relocations capture array-only accesses for structs with
 	 * fixed header, but with potentially extendable tail. This will read
 	 * first 4 bytes of 2nd element of in_ext array of potentially
 	 * variably sized struct core_reloc_misc_extensible. */ 
-	if (BPF_CORE_READ(&out->c, &in_ext[2]))		/* accessor: 2 */
+	if (CORE_READ(&out->c, &in_ext[2]))		/* accessor: 2 */
 		return 1;
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_mods.c b/tools/testing/selftests/bpf/progs/test_core_reloc_mods.c
index f98b942c062b..e930e7e88c5c 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_mods.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_mods.c
@@ -41,20 +41,22 @@ struct core_reloc_mods {
 	core_reloc_mods_substruct_t h;
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_mods(void *ctx)
 {
 	struct core_reloc_mods *in = (void *)&data.in;
 	struct core_reloc_mods_output *out = (void *)&data.out;
 
-	if (BPF_CORE_READ(&out->a, &in->a) ||
-	    BPF_CORE_READ(&out->b, &in->b) ||
-	    BPF_CORE_READ(&out->c, &in->c) ||
-	    BPF_CORE_READ(&out->d, &in->d) ||
-	    BPF_CORE_READ(&out->e, &in->e[2]) ||
-	    BPF_CORE_READ(&out->f, &in->f[1]) ||
-	    BPF_CORE_READ(&out->g, &in->g.x) ||
-	    BPF_CORE_READ(&out->h, &in->h.y))
+	if (CORE_READ(&out->a, &in->a) ||
+	    CORE_READ(&out->b, &in->b) ||
+	    CORE_READ(&out->c, &in->c) ||
+	    CORE_READ(&out->d, &in->d) ||
+	    CORE_READ(&out->e, &in->e[2]) ||
+	    CORE_READ(&out->f, &in->f[1]) ||
+	    CORE_READ(&out->g, &in->g.x) ||
+	    CORE_READ(&out->h, &in->h.y))
 		return 1;
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_nesting.c b/tools/testing/selftests/bpf/progs/test_core_reloc_nesting.c
index 3ca30cec2b39..b63007958290 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_nesting.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_nesting.c
@@ -30,15 +30,17 @@ struct core_reloc_nesting {
 	} b;
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_nesting(void *ctx)
 {
 	struct core_reloc_nesting *in = (void *)&data.in;
 	struct core_reloc_nesting *out = (void *)&data.out;
 
-	if (BPF_CORE_READ(&out->a.a.a, &in->a.a.a))
+	if (CORE_READ(&out->a.a.a, &in->a.a.a))
 		return 1;
-	if (BPF_CORE_READ(&out->b.b.b, &in->b.b.b))
+	if (CORE_READ(&out->b.b.b, &in->b.b.b))
 		return 1;
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_primitives.c b/tools/testing/selftests/bpf/progs/test_core_reloc_primitives.c
index add52f23ab35..7654f59914bc 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_primitives.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_primitives.c
@@ -25,17 +25,19 @@ struct core_reloc_primitives {
 	int (*f)(const char *);
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_primitives(void *ctx)
 {
 	struct core_reloc_primitives *in = (void *)&data.in;
 	struct core_reloc_primitives *out = (void *)&data.out;
 
-	if (BPF_CORE_READ(&out->a, &in->a) ||
-	    BPF_CORE_READ(&out->b, &in->b) ||
-	    BPF_CORE_READ(&out->c, &in->c) ||
-	    BPF_CORE_READ(&out->d, &in->d) ||
-	    BPF_CORE_READ(&out->f, &in->f))
+	if (CORE_READ(&out->a, &in->a) ||
+	    CORE_READ(&out->b, &in->b) ||
+	    CORE_READ(&out->c, &in->c) ||
+	    CORE_READ(&out->d, &in->d) ||
+	    CORE_READ(&out->f, &in->f))
 		return 1;
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_ptr_as_arr.c b/tools/testing/selftests/bpf/progs/test_core_reloc_ptr_as_arr.c
index 526b7ddc7ea1..709f7cba453f 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_ptr_as_arr.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_ptr_as_arr.c
@@ -16,13 +16,15 @@ struct core_reloc_ptr_as_arr {
 	int a;
 };
 
+#define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
+
 SEC("raw_tracepoint/sys_enter")
 int test_core_ptr_as_arr(void *ctx)
 {
 	struct core_reloc_ptr_as_arr *in = (void *)&data.in;
 	struct core_reloc_ptr_as_arr *out = (void *)&data.out;
 
-	if (BPF_CORE_READ(&out->a, &in[2].a))
+	if (CORE_READ(&out->a, &in[2].a))
 		return 1;
 
 	return 0;
-- 
2.17.1


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

* [PATCH v3 bpf-next 4/7] selftests/bpf: split off tracing-only helpers into bpf_tracing.h
  2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2019-10-03 21:28 ` [PATCH v3 bpf-next 3/7] selftests/bpf: adjust CO-RE reloc tests for new bpf_core_read() macro Andrii Nakryiko
@ 2019-10-03 21:28 ` Andrii Nakryiko
  2019-10-04  7:01   ` Toke Høiland-Jørgensen
  2019-10-03 21:28 ` [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf Andrii Nakryiko
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Split-off PT_REGS-related helpers into bpf_tracing.h header. Adjust
selftests and samples to include it where necessary.

Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 samples/bpf/map_perf_test_kern.c          |   1 +
 samples/bpf/offwaketime_kern.c            |   1 +
 samples/bpf/sampleip_kern.c               |   1 +
 samples/bpf/spintest_kern.c               |   1 +
 samples/bpf/test_map_in_map_kern.c        |   1 +
 samples/bpf/test_overhead_kprobe_kern.c   |   1 +
 samples/bpf/test_probe_write_user_kern.c  |   1 +
 samples/bpf/trace_event_kern.c            |   1 +
 samples/bpf/tracex1_kern.c                |   1 +
 samples/bpf/tracex2_kern.c                |   1 +
 samples/bpf/tracex3_kern.c                |   1 +
 samples/bpf/tracex4_kern.c                |   1 +
 samples/bpf/tracex5_kern.c                |   1 +
 tools/testing/selftests/bpf/bpf_helpers.h | 210 ++--------------------
 tools/testing/selftests/bpf/bpf_tracing.h | 195 ++++++++++++++++++++
 tools/testing/selftests/bpf/progs/loop1.c |   1 +
 tools/testing/selftests/bpf/progs/loop2.c |   1 +
 tools/testing/selftests/bpf/progs/loop3.c |   1 +
 18 files changed, 221 insertions(+), 200 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/bpf_tracing.h

diff --git a/samples/bpf/map_perf_test_kern.c b/samples/bpf/map_perf_test_kern.c
index f47ee513cb7c..5c11aefbc489 100644
--- a/samples/bpf/map_perf_test_kern.c
+++ b/samples/bpf/map_perf_test_kern.c
@@ -10,6 +10,7 @@
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
 #include "bpf_legacy.h"
+#include "bpf_tracing.h"
 
 #define MAX_ENTRIES 1000
 #define MAX_NR_CPUS 1024
diff --git a/samples/bpf/offwaketime_kern.c b/samples/bpf/offwaketime_kern.c
index e7d9a0a3d45b..9cb5207a692f 100644
--- a/samples/bpf/offwaketime_kern.c
+++ b/samples/bpf/offwaketime_kern.c
@@ -6,6 +6,7 @@
  */
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 #include <uapi/linux/ptrace.h>
 #include <uapi/linux/perf_event.h>
 #include <linux/version.h>
diff --git a/samples/bpf/sampleip_kern.c b/samples/bpf/sampleip_kern.c
index ceabf31079cf..4a190893894f 100644
--- a/samples/bpf/sampleip_kern.c
+++ b/samples/bpf/sampleip_kern.c
@@ -9,6 +9,7 @@
 #include <uapi/linux/bpf.h>
 #include <uapi/linux/bpf_perf_event.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 #define MAX_IPS		8192
 
diff --git a/samples/bpf/spintest_kern.c b/samples/bpf/spintest_kern.c
index ce0167d09cdc..6e9478aa2938 100644
--- a/samples/bpf/spintest_kern.c
+++ b/samples/bpf/spintest_kern.c
@@ -10,6 +10,7 @@
 #include <uapi/linux/bpf.h>
 #include <uapi/linux/perf_event.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 struct bpf_map_def SEC("maps") my_map = {
 	.type = BPF_MAP_TYPE_HASH,
diff --git a/samples/bpf/test_map_in_map_kern.c b/samples/bpf/test_map_in_map_kern.c
index 8101bf3dc7f7..4f80cbe74c72 100644
--- a/samples/bpf/test_map_in_map_kern.c
+++ b/samples/bpf/test_map_in_map_kern.c
@@ -12,6 +12,7 @@
 #include <uapi/linux/in6.h>
 #include "bpf_helpers.h"
 #include "bpf_legacy.h"
+#include "bpf_tracing.h"
 
 #define MAX_NR_PORTS 65536
 
diff --git a/samples/bpf/test_overhead_kprobe_kern.c b/samples/bpf/test_overhead_kprobe_kern.c
index 468a66a92ef9..8d2518e68db9 100644
--- a/samples/bpf/test_overhead_kprobe_kern.c
+++ b/samples/bpf/test_overhead_kprobe_kern.c
@@ -8,6 +8,7 @@
 #include <linux/ptrace.h>
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 #define _(P) ({typeof(P) val = 0; bpf_probe_read(&val, sizeof(val), &P); val;})
 
diff --git a/samples/bpf/test_probe_write_user_kern.c b/samples/bpf/test_probe_write_user_kern.c
index 3a677c807044..a543358218e6 100644
--- a/samples/bpf/test_probe_write_user_kern.c
+++ b/samples/bpf/test_probe_write_user_kern.c
@@ -9,6 +9,7 @@
 #include <uapi/linux/bpf.h>
 #include <linux/version.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 struct bpf_map_def SEC("maps") dnat_map = {
 	.type = BPF_MAP_TYPE_HASH,
diff --git a/samples/bpf/trace_event_kern.c b/samples/bpf/trace_event_kern.c
index 7068fbdde951..8dc18d233a27 100644
--- a/samples/bpf/trace_event_kern.c
+++ b/samples/bpf/trace_event_kern.c
@@ -10,6 +10,7 @@
 #include <uapi/linux/bpf_perf_event.h>
 #include <uapi/linux/perf_event.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 struct key_t {
 	char comm[TASK_COMM_LEN];
diff --git a/samples/bpf/tracex1_kern.c b/samples/bpf/tracex1_kern.c
index 107da148820f..1a15f6605129 100644
--- a/samples/bpf/tracex1_kern.c
+++ b/samples/bpf/tracex1_kern.c
@@ -9,6 +9,7 @@
 #include <uapi/linux/bpf.h>
 #include <linux/version.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 #define _(P) ({typeof(P) val = 0; bpf_probe_read(&val, sizeof(val), &P); val;})
 
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 5e11c20ce5ec..d70b3ea79ea7 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -9,6 +9,7 @@
 #include <linux/version.h>
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 struct bpf_map_def SEC("maps") my_map = {
 	.type = BPF_MAP_TYPE_HASH,
diff --git a/samples/bpf/tracex3_kern.c b/samples/bpf/tracex3_kern.c
index ea1d4c19c132..9af546bebfa9 100644
--- a/samples/bpf/tracex3_kern.c
+++ b/samples/bpf/tracex3_kern.c
@@ -9,6 +9,7 @@
 #include <linux/version.h>
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 struct bpf_map_def SEC("maps") my_map = {
 	.type = BPF_MAP_TYPE_HASH,
diff --git a/samples/bpf/tracex4_kern.c b/samples/bpf/tracex4_kern.c
index 6dd8e384de96..2a02cbe9d9a1 100644
--- a/samples/bpf/tracex4_kern.c
+++ b/samples/bpf/tracex4_kern.c
@@ -8,6 +8,7 @@
 #include <linux/version.h>
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 struct pair {
 	u64 val;
diff --git a/samples/bpf/tracex5_kern.c b/samples/bpf/tracex5_kern.c
index 35cb0eed3be5..b3557b21a8fe 100644
--- a/samples/bpf/tracex5_kern.c
+++ b/samples/bpf/tracex5_kern.c
@@ -11,6 +11,7 @@
 #include <uapi/linux/unistd.h>
 #include "syscall_nrs.h"
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 #define PROG(F) SEC("kprobe/"__stringify(F)) int bpf_func_##F
 
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 5210cc7d7c5c..cb9d4d2224af 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -232,17 +232,6 @@ static long long (*bpf_tcp_gen_syncookie)(struct bpf_sock *sk, void *ip,
 					  int ip_len, void *tcp, int tcp_len) =
 	(void *) BPF_FUNC_tcp_gen_syncookie;
 
-/* a helper structure used by eBPF C program
- * to describe BPF map attributes to libbpf loader
- */
-struct bpf_map_def {
-	unsigned int type;
-	unsigned int key_size;
-	unsigned int value_size;
-	unsigned int max_entries;
-	unsigned int map_flags;
-};
-
 static int (*bpf_skb_load_bytes)(void *ctx, int off, void *to, int len) =
 	(void *) BPF_FUNC_skb_load_bytes;
 static int (*bpf_skb_load_bytes_relative)(void *ctx, int off, void *to, int len, __u32 start_header) =
@@ -292,195 +281,16 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
 				  unsigned long long flags) =
 	(void *) BPF_FUNC_skb_adjust_room;
 
-/* Scan the ARCH passed in from ARCH env variable (see Makefile) */
-#if defined(__TARGET_ARCH_x86)
-	#define bpf_target_x86
-	#define bpf_target_defined
-#elif defined(__TARGET_ARCH_s390)
-	#define bpf_target_s390
-	#define bpf_target_defined
-#elif defined(__TARGET_ARCH_arm)
-	#define bpf_target_arm
-	#define bpf_target_defined
-#elif defined(__TARGET_ARCH_arm64)
-	#define bpf_target_arm64
-	#define bpf_target_defined
-#elif defined(__TARGET_ARCH_mips)
-	#define bpf_target_mips
-	#define bpf_target_defined
-#elif defined(__TARGET_ARCH_powerpc)
-	#define bpf_target_powerpc
-	#define bpf_target_defined
-#elif defined(__TARGET_ARCH_sparc)
-	#define bpf_target_sparc
-	#define bpf_target_defined
-#else
-	#undef bpf_target_defined
-#endif
-
-/* Fall back to what the compiler says */
-#ifndef bpf_target_defined
-#if defined(__x86_64__)
-	#define bpf_target_x86
-#elif defined(__s390__)
-	#define bpf_target_s390
-#elif defined(__arm__)
-	#define bpf_target_arm
-#elif defined(__aarch64__)
-	#define bpf_target_arm64
-#elif defined(__mips__)
-	#define bpf_target_mips
-#elif defined(__powerpc__)
-	#define bpf_target_powerpc
-#elif defined(__sparc__)
-	#define bpf_target_sparc
-#endif
-#endif
-
-#if defined(bpf_target_x86)
-
-#ifdef __KERNEL__
-#define PT_REGS_PARM1(x) ((x)->di)
-#define PT_REGS_PARM2(x) ((x)->si)
-#define PT_REGS_PARM3(x) ((x)->dx)
-#define PT_REGS_PARM4(x) ((x)->cx)
-#define PT_REGS_PARM5(x) ((x)->r8)
-#define PT_REGS_RET(x) ((x)->sp)
-#define PT_REGS_FP(x) ((x)->bp)
-#define PT_REGS_RC(x) ((x)->ax)
-#define PT_REGS_SP(x) ((x)->sp)
-#define PT_REGS_IP(x) ((x)->ip)
-#else
-#ifdef __i386__
-/* i386 kernel is built with -mregparm=3 */
-#define PT_REGS_PARM1(x) ((x)->eax)
-#define PT_REGS_PARM2(x) ((x)->edx)
-#define PT_REGS_PARM3(x) ((x)->ecx)
-#define PT_REGS_PARM4(x) 0
-#define PT_REGS_PARM5(x) 0
-#define PT_REGS_RET(x) ((x)->esp)
-#define PT_REGS_FP(x) ((x)->ebp)
-#define PT_REGS_RC(x) ((x)->eax)
-#define PT_REGS_SP(x) ((x)->esp)
-#define PT_REGS_IP(x) ((x)->eip)
-#else
-#define PT_REGS_PARM1(x) ((x)->rdi)
-#define PT_REGS_PARM2(x) ((x)->rsi)
-#define PT_REGS_PARM3(x) ((x)->rdx)
-#define PT_REGS_PARM4(x) ((x)->rcx)
-#define PT_REGS_PARM5(x) ((x)->r8)
-#define PT_REGS_RET(x) ((x)->rsp)
-#define PT_REGS_FP(x) ((x)->rbp)
-#define PT_REGS_RC(x) ((x)->rax)
-#define PT_REGS_SP(x) ((x)->rsp)
-#define PT_REGS_IP(x) ((x)->rip)
-#endif
-#endif
-
-#elif defined(bpf_target_s390)
-
-/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
-struct pt_regs;
-#define PT_REGS_S390 const volatile user_pt_regs
-#define PT_REGS_PARM1(x) (((PT_REGS_S390 *)(x))->gprs[2])
-#define PT_REGS_PARM2(x) (((PT_REGS_S390 *)(x))->gprs[3])
-#define PT_REGS_PARM3(x) (((PT_REGS_S390 *)(x))->gprs[4])
-#define PT_REGS_PARM4(x) (((PT_REGS_S390 *)(x))->gprs[5])
-#define PT_REGS_PARM5(x) (((PT_REGS_S390 *)(x))->gprs[6])
-#define PT_REGS_RET(x) (((PT_REGS_S390 *)(x))->gprs[14])
-/* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_FP(x) (((PT_REGS_S390 *)(x))->gprs[11])
-#define PT_REGS_RC(x) (((PT_REGS_S390 *)(x))->gprs[2])
-#define PT_REGS_SP(x) (((PT_REGS_S390 *)(x))->gprs[15])
-#define PT_REGS_IP(x) (((PT_REGS_S390 *)(x))->psw.addr)
-
-#elif defined(bpf_target_arm)
-
-#define PT_REGS_PARM1(x) ((x)->uregs[0])
-#define PT_REGS_PARM2(x) ((x)->uregs[1])
-#define PT_REGS_PARM3(x) ((x)->uregs[2])
-#define PT_REGS_PARM4(x) ((x)->uregs[3])
-#define PT_REGS_PARM5(x) ((x)->uregs[4])
-#define PT_REGS_RET(x) ((x)->uregs[14])
-#define PT_REGS_FP(x) ((x)->uregs[11]) /* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_RC(x) ((x)->uregs[0])
-#define PT_REGS_SP(x) ((x)->uregs[13])
-#define PT_REGS_IP(x) ((x)->uregs[12])
-
-#elif defined(bpf_target_arm64)
-
-/* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
-struct pt_regs;
-#define PT_REGS_ARM64 const volatile struct user_pt_regs
-#define PT_REGS_PARM1(x) (((PT_REGS_ARM64 *)(x))->regs[0])
-#define PT_REGS_PARM2(x) (((PT_REGS_ARM64 *)(x))->regs[1])
-#define PT_REGS_PARM3(x) (((PT_REGS_ARM64 *)(x))->regs[2])
-#define PT_REGS_PARM4(x) (((PT_REGS_ARM64 *)(x))->regs[3])
-#define PT_REGS_PARM5(x) (((PT_REGS_ARM64 *)(x))->regs[4])
-#define PT_REGS_RET(x) (((PT_REGS_ARM64 *)(x))->regs[30])
-/* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_FP(x) (((PT_REGS_ARM64 *)(x))->regs[29])
-#define PT_REGS_RC(x) (((PT_REGS_ARM64 *)(x))->regs[0])
-#define PT_REGS_SP(x) (((PT_REGS_ARM64 *)(x))->sp)
-#define PT_REGS_IP(x) (((PT_REGS_ARM64 *)(x))->pc)
-
-#elif defined(bpf_target_mips)
-
-#define PT_REGS_PARM1(x) ((x)->regs[4])
-#define PT_REGS_PARM2(x) ((x)->regs[5])
-#define PT_REGS_PARM3(x) ((x)->regs[6])
-#define PT_REGS_PARM4(x) ((x)->regs[7])
-#define PT_REGS_PARM5(x) ((x)->regs[8])
-#define PT_REGS_RET(x) ((x)->regs[31])
-#define PT_REGS_FP(x) ((x)->regs[30]) /* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_RC(x) ((x)->regs[1])
-#define PT_REGS_SP(x) ((x)->regs[29])
-#define PT_REGS_IP(x) ((x)->cp0_epc)
-
-#elif defined(bpf_target_powerpc)
-
-#define PT_REGS_PARM1(x) ((x)->gpr[3])
-#define PT_REGS_PARM2(x) ((x)->gpr[4])
-#define PT_REGS_PARM3(x) ((x)->gpr[5])
-#define PT_REGS_PARM4(x) ((x)->gpr[6])
-#define PT_REGS_PARM5(x) ((x)->gpr[7])
-#define PT_REGS_RC(x) ((x)->gpr[3])
-#define PT_REGS_SP(x) ((x)->sp)
-#define PT_REGS_IP(x) ((x)->nip)
-
-#elif defined(bpf_target_sparc)
-
-#define PT_REGS_PARM1(x) ((x)->u_regs[UREG_I0])
-#define PT_REGS_PARM2(x) ((x)->u_regs[UREG_I1])
-#define PT_REGS_PARM3(x) ((x)->u_regs[UREG_I2])
-#define PT_REGS_PARM4(x) ((x)->u_regs[UREG_I3])
-#define PT_REGS_PARM5(x) ((x)->u_regs[UREG_I4])
-#define PT_REGS_RET(x) ((x)->u_regs[UREG_I7])
-#define PT_REGS_RC(x) ((x)->u_regs[UREG_I0])
-#define PT_REGS_SP(x) ((x)->u_regs[UREG_FP])
-
-/* Should this also be a bpf_target check for the sparc case? */
-#if defined(__arch64__)
-#define PT_REGS_IP(x) ((x)->tpc)
-#else
-#define PT_REGS_IP(x) ((x)->pc)
-#endif
-
-#endif
-
-#if defined(bpf_target_powerpc)
-#define BPF_KPROBE_READ_RET_IP(ip, ctx)		({ (ip) = (ctx)->link; })
-#define BPF_KRETPROBE_READ_RET_IP		BPF_KPROBE_READ_RET_IP
-#elif defined(bpf_target_sparc)
-#define BPF_KPROBE_READ_RET_IP(ip, ctx)		({ (ip) = PT_REGS_RET(ctx); })
-#define BPF_KRETPROBE_READ_RET_IP		BPF_KPROBE_READ_RET_IP
-#else
-#define BPF_KPROBE_READ_RET_IP(ip, ctx)		({				\
-		bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx)); })
-#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)	({				\
-		bpf_probe_read(&(ip), sizeof(ip),				\
-				(void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
-#endif
+/* a helper structure used by eBPF C program
+ * to describe BPF map attributes to libbpf loader
+ */
+struct bpf_map_def {
+	unsigned int type;
+	unsigned int key_size;
+	unsigned int value_size;
+	unsigned int max_entries;
+	unsigned int map_flags;
+};
 
 /*
  * bpf_core_read() abstracts away bpf_probe_read() call and captures offset
diff --git a/tools/testing/selftests/bpf/bpf_tracing.h b/tools/testing/selftests/bpf/bpf_tracing.h
new file mode 100644
index 000000000000..b0dafe8b4ebc
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_tracing.h
@@ -0,0 +1,195 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+#ifndef __BPF_TRACING_H__
+#define __BPF_TRACING_H__
+
+/* Scan the ARCH passed in from ARCH env variable (see Makefile) */
+#if defined(__TARGET_ARCH_x86)
+	#define bpf_target_x86
+	#define bpf_target_defined
+#elif defined(__TARGET_ARCH_s390)
+	#define bpf_target_s390
+	#define bpf_target_defined
+#elif defined(__TARGET_ARCH_arm)
+	#define bpf_target_arm
+	#define bpf_target_defined
+#elif defined(__TARGET_ARCH_arm64)
+	#define bpf_target_arm64
+	#define bpf_target_defined
+#elif defined(__TARGET_ARCH_mips)
+	#define bpf_target_mips
+	#define bpf_target_defined
+#elif defined(__TARGET_ARCH_powerpc)
+	#define bpf_target_powerpc
+	#define bpf_target_defined
+#elif defined(__TARGET_ARCH_sparc)
+	#define bpf_target_sparc
+	#define bpf_target_defined
+#else
+	#undef bpf_target_defined
+#endif
+
+/* Fall back to what the compiler says */
+#ifndef bpf_target_defined
+#if defined(__x86_64__)
+	#define bpf_target_x86
+#elif defined(__s390__)
+	#define bpf_target_s390
+#elif defined(__arm__)
+	#define bpf_target_arm
+#elif defined(__aarch64__)
+	#define bpf_target_arm64
+#elif defined(__mips__)
+	#define bpf_target_mips
+#elif defined(__powerpc__)
+	#define bpf_target_powerpc
+#elif defined(__sparc__)
+	#define bpf_target_sparc
+#endif
+#endif
+
+#if defined(bpf_target_x86)
+
+#ifdef __KERNEL__
+#define PT_REGS_PARM1(x) ((x)->di)
+#define PT_REGS_PARM2(x) ((x)->si)
+#define PT_REGS_PARM3(x) ((x)->dx)
+#define PT_REGS_PARM4(x) ((x)->cx)
+#define PT_REGS_PARM5(x) ((x)->r8)
+#define PT_REGS_RET(x) ((x)->sp)
+#define PT_REGS_FP(x) ((x)->bp)
+#define PT_REGS_RC(x) ((x)->ax)
+#define PT_REGS_SP(x) ((x)->sp)
+#define PT_REGS_IP(x) ((x)->ip)
+#else
+#ifdef __i386__
+/* i386 kernel is built with -mregparm=3 */
+#define PT_REGS_PARM1(x) ((x)->eax)
+#define PT_REGS_PARM2(x) ((x)->edx)
+#define PT_REGS_PARM3(x) ((x)->ecx)
+#define PT_REGS_PARM4(x) 0
+#define PT_REGS_PARM5(x) 0
+#define PT_REGS_RET(x) ((x)->esp)
+#define PT_REGS_FP(x) ((x)->ebp)
+#define PT_REGS_RC(x) ((x)->eax)
+#define PT_REGS_SP(x) ((x)->esp)
+#define PT_REGS_IP(x) ((x)->eip)
+#else
+#define PT_REGS_PARM1(x) ((x)->rdi)
+#define PT_REGS_PARM2(x) ((x)->rsi)
+#define PT_REGS_PARM3(x) ((x)->rdx)
+#define PT_REGS_PARM4(x) ((x)->rcx)
+#define PT_REGS_PARM5(x) ((x)->r8)
+#define PT_REGS_RET(x) ((x)->rsp)
+#define PT_REGS_FP(x) ((x)->rbp)
+#define PT_REGS_RC(x) ((x)->rax)
+#define PT_REGS_SP(x) ((x)->rsp)
+#define PT_REGS_IP(x) ((x)->rip)
+#endif
+#endif
+
+#elif defined(bpf_target_s390)
+
+/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
+struct pt_regs;
+#define PT_REGS_S390 const volatile user_pt_regs
+#define PT_REGS_PARM1(x) (((PT_REGS_S390 *)(x))->gprs[2])
+#define PT_REGS_PARM2(x) (((PT_REGS_S390 *)(x))->gprs[3])
+#define PT_REGS_PARM3(x) (((PT_REGS_S390 *)(x))->gprs[4])
+#define PT_REGS_PARM4(x) (((PT_REGS_S390 *)(x))->gprs[5])
+#define PT_REGS_PARM5(x) (((PT_REGS_S390 *)(x))->gprs[6])
+#define PT_REGS_RET(x) (((PT_REGS_S390 *)(x))->gprs[14])
+/* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_FP(x) (((PT_REGS_S390 *)(x))->gprs[11])
+#define PT_REGS_RC(x) (((PT_REGS_S390 *)(x))->gprs[2])
+#define PT_REGS_SP(x) (((PT_REGS_S390 *)(x))->gprs[15])
+#define PT_REGS_IP(x) (((PT_REGS_S390 *)(x))->psw.addr)
+
+#elif defined(bpf_target_arm)
+
+#define PT_REGS_PARM1(x) ((x)->uregs[0])
+#define PT_REGS_PARM2(x) ((x)->uregs[1])
+#define PT_REGS_PARM3(x) ((x)->uregs[2])
+#define PT_REGS_PARM4(x) ((x)->uregs[3])
+#define PT_REGS_PARM5(x) ((x)->uregs[4])
+#define PT_REGS_RET(x) ((x)->uregs[14])
+#define PT_REGS_FP(x) ((x)->uregs[11]) /* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_RC(x) ((x)->uregs[0])
+#define PT_REGS_SP(x) ((x)->uregs[13])
+#define PT_REGS_IP(x) ((x)->uregs[12])
+
+#elif defined(bpf_target_arm64)
+
+/* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
+struct pt_regs;
+#define PT_REGS_ARM64 const volatile struct user_pt_regs
+#define PT_REGS_PARM1(x) (((PT_REGS_ARM64 *)(x))->regs[0])
+#define PT_REGS_PARM2(x) (((PT_REGS_ARM64 *)(x))->regs[1])
+#define PT_REGS_PARM3(x) (((PT_REGS_ARM64 *)(x))->regs[2])
+#define PT_REGS_PARM4(x) (((PT_REGS_ARM64 *)(x))->regs[3])
+#define PT_REGS_PARM5(x) (((PT_REGS_ARM64 *)(x))->regs[4])
+#define PT_REGS_RET(x) (((PT_REGS_ARM64 *)(x))->regs[30])
+/* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_FP(x) (((PT_REGS_ARM64 *)(x))->regs[29])
+#define PT_REGS_RC(x) (((PT_REGS_ARM64 *)(x))->regs[0])
+#define PT_REGS_SP(x) (((PT_REGS_ARM64 *)(x))->sp)
+#define PT_REGS_IP(x) (((PT_REGS_ARM64 *)(x))->pc)
+
+#elif defined(bpf_target_mips)
+
+#define PT_REGS_PARM1(x) ((x)->regs[4])
+#define PT_REGS_PARM2(x) ((x)->regs[5])
+#define PT_REGS_PARM3(x) ((x)->regs[6])
+#define PT_REGS_PARM4(x) ((x)->regs[7])
+#define PT_REGS_PARM5(x) ((x)->regs[8])
+#define PT_REGS_RET(x) ((x)->regs[31])
+#define PT_REGS_FP(x) ((x)->regs[30]) /* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_RC(x) ((x)->regs[1])
+#define PT_REGS_SP(x) ((x)->regs[29])
+#define PT_REGS_IP(x) ((x)->cp0_epc)
+
+#elif defined(bpf_target_powerpc)
+
+#define PT_REGS_PARM1(x) ((x)->gpr[3])
+#define PT_REGS_PARM2(x) ((x)->gpr[4])
+#define PT_REGS_PARM3(x) ((x)->gpr[5])
+#define PT_REGS_PARM4(x) ((x)->gpr[6])
+#define PT_REGS_PARM5(x) ((x)->gpr[7])
+#define PT_REGS_RC(x) ((x)->gpr[3])
+#define PT_REGS_SP(x) ((x)->sp)
+#define PT_REGS_IP(x) ((x)->nip)
+
+#elif defined(bpf_target_sparc)
+
+#define PT_REGS_PARM1(x) ((x)->u_regs[UREG_I0])
+#define PT_REGS_PARM2(x) ((x)->u_regs[UREG_I1])
+#define PT_REGS_PARM3(x) ((x)->u_regs[UREG_I2])
+#define PT_REGS_PARM4(x) ((x)->u_regs[UREG_I3])
+#define PT_REGS_PARM5(x) ((x)->u_regs[UREG_I4])
+#define PT_REGS_RET(x) ((x)->u_regs[UREG_I7])
+#define PT_REGS_RC(x) ((x)->u_regs[UREG_I0])
+#define PT_REGS_SP(x) ((x)->u_regs[UREG_FP])
+
+/* Should this also be a bpf_target check for the sparc case? */
+#if defined(__arch64__)
+#define PT_REGS_IP(x) ((x)->tpc)
+#else
+#define PT_REGS_IP(x) ((x)->pc)
+#endif
+
+#endif
+
+#if defined(bpf_target_powerpc)
+#define BPF_KPROBE_READ_RET_IP(ip, ctx)		({ (ip) = (ctx)->link; })
+#define BPF_KRETPROBE_READ_RET_IP		BPF_KPROBE_READ_RET_IP
+#elif defined(bpf_target_sparc)
+#define BPF_KPROBE_READ_RET_IP(ip, ctx)		({ (ip) = PT_REGS_RET(ctx); })
+#define BPF_KRETPROBE_READ_RET_IP		BPF_KPROBE_READ_RET_IP
+#else
+#define BPF_KPROBE_READ_RET_IP(ip, ctx)					    \
+	({ bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx)); })
+#define BPF_KRETPROBE_READ_RET_IP(ip, ctx)				    \
+	({ bpf_probe_read(&(ip), sizeof(ip),				    \
+			  (void *)(PT_REGS_FP(ctx) + sizeof(ip))); })
+#endif
+
+#endif
diff --git a/tools/testing/selftests/bpf/progs/loop1.c b/tools/testing/selftests/bpf/progs/loop1.c
index 7cdb7f878310..40ac722a9da5 100644
--- a/tools/testing/selftests/bpf/progs/loop1.c
+++ b/tools/testing/selftests/bpf/progs/loop1.c
@@ -7,6 +7,7 @@
 #include <stdbool.h>
 #include <linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/loop2.c b/tools/testing/selftests/bpf/progs/loop2.c
index 9b2f808a2863..bb80f29aa7f7 100644
--- a/tools/testing/selftests/bpf/progs/loop2.c
+++ b/tools/testing/selftests/bpf/progs/loop2.c
@@ -7,6 +7,7 @@
 #include <stdbool.h>
 #include <linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/loop3.c b/tools/testing/selftests/bpf/progs/loop3.c
index d727657d51e2..2b9165a7afe1 100644
--- a/tools/testing/selftests/bpf/progs/loop3.c
+++ b/tools/testing/selftests/bpf/progs/loop3.c
@@ -7,6 +7,7 @@
 #include <stdbool.h>
 #include <linux/bpf.h>
 #include "bpf_helpers.h"
+#include "bpf_tracing.h"
 
 char _license[] SEC("license") = "GPL";
 
-- 
2.17.1


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

* [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2019-10-03 21:28 ` [PATCH v3 bpf-next 4/7] selftests/bpf: split off tracing-only helpers into bpf_tracing.h Andrii Nakryiko
@ 2019-10-03 21:28 ` Andrii Nakryiko
  2019-10-04  7:01   ` Toke Høiland-Jørgensen
  2019-10-04 14:47   ` David Ahern
  2019-10-03 21:28 ` [PATCH v3 bpf-next 6/7] libbpf: add BPF_CORE_READ/BPF_CORE_READ_INTO helpers Andrii Nakryiko
  2019-10-03 21:28 ` [PATCH v3 bpf-next 7/7] selftests/bpf: add BPF_CORE_READ and BPF_CORE_READ_STR_INTO macro tests Andrii Nakryiko
  6 siblings, 2 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Move bpf_helpers.h, bpf_tracing.h, and bpf_endian.h into libbpf. Ensure
they are installed along the other libbpf headers. Also, adjust
selftests and samples include path to include libbpf now.

Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 samples/bpf/Makefile                               | 2 +-
 tools/lib/bpf/Makefile                             | 5 ++++-
 tools/{testing/selftests => lib}/bpf/bpf_endian.h  | 0
 tools/{testing/selftests => lib}/bpf/bpf_helpers.h | 0
 tools/{testing/selftests => lib}/bpf/bpf_tracing.h | 0
 tools/testing/selftests/bpf/Makefile               | 2 +-
 6 files changed, 6 insertions(+), 3 deletions(-)
 rename tools/{testing/selftests => lib}/bpf/bpf_endian.h (100%)
 rename tools/{testing/selftests => lib}/bpf/bpf_helpers.h (100%)
 rename tools/{testing/selftests => lib}/bpf/bpf_tracing.h (100%)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 42b571cde177..ecb3535d91e3 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -283,7 +283,7 @@ $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
 $(obj)/%.o: $(src)/%.c
 	@echo "  CLANG-bpf " $@
 	$(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
-		-I$(srctree)/tools/testing/selftests/bpf/ \
+		-I$(srctree)/tools/testing/selftests/bpf/ -I$(srctree)/tools/lib/bpf/ \
 		-D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
 		-D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \
 		-Wno-gnu-variable-sized-type-not-at-end \
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index c6f94cffe06e..20b5b0ff5c73 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -240,7 +240,10 @@ install_headers:
 		$(call do_install,libbpf.h,$(prefix)/include/bpf,644); \
 		$(call do_install,btf.h,$(prefix)/include/bpf,644); \
 		$(call do_install,libbpf_util.h,$(prefix)/include/bpf,644); \
-		$(call do_install,xsk.h,$(prefix)/include/bpf,644);
+		$(call do_install,xsk.h,$(prefix)/include/bpf,644); \
+		$(call do_install,bpf_helpers.h,$(prefix)/include/bpf,644); \
+		$(call do_install,bpf_tracing.h,$(prefix)/include/bpf,644); \
+		$(call do_install,bpf_endian.h,$(prefix)/include/bpf,644);
 
 install_pkgconfig: $(PC_FILE)
 	$(call QUIET_INSTALL, $(PC_FILE)) \
diff --git a/tools/testing/selftests/bpf/bpf_endian.h b/tools/lib/bpf/bpf_endian.h
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_endian.h
rename to tools/lib/bpf/bpf_endian.h
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_helpers.h
rename to tools/lib/bpf/bpf_helpers.h
diff --git a/tools/testing/selftests/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
similarity index 100%
rename from tools/testing/selftests/bpf/bpf_tracing.h
rename to tools/lib/bpf/bpf_tracing.h
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 6889c19a628c..b00a5d8046c7 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -148,7 +148,7 @@ $(shell $(1) -v -E - </dev/null 2>&1 \
 endef
 CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
 BPF_CFLAGS = -I. -I./include/uapi -I../../../include/uapi \
-	     -I$(OUTPUT)/../usr/include -D__TARGET_ARCH_$(SRCARCH)
+	     -I$(BPFDIR) -I$(OUTPUT)/../usr/include -D__TARGET_ARCH_$(SRCARCH)
 
 CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
 	       -Wno-compare-distinct-pointer-types
-- 
2.17.1


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

* [PATCH v3 bpf-next 6/7] libbpf: add BPF_CORE_READ/BPF_CORE_READ_INTO helpers
  2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2019-10-03 21:28 ` [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf Andrii Nakryiko
@ 2019-10-03 21:28 ` Andrii Nakryiko
  2019-10-03 21:28 ` [PATCH v3 bpf-next 7/7] selftests/bpf: add BPF_CORE_READ and BPF_CORE_READ_STR_INTO macro tests Andrii Nakryiko
  6 siblings, 0 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Add few macros simplifying BCC-like multi-level probe reads, while also
emitting CO-RE relocations for each read.

Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/bpf_helpers.h | 153 +++++++++++++++++++++++++++++++++++-
 1 file changed, 150 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index cb9d4d2224af..2d3d1f51cdd0 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -5,7 +5,7 @@
 #define __uint(name, val) int (*name)[val]
 #define __type(name, val) val *name
 
-/* helper macro to print out debug messages */
+/* Helper macro to print out debug messages */
 #define bpf_printk(fmt, ...)				\
 ({							\
 	char ____fmt[] = fmt;				\
@@ -13,12 +13,17 @@
 			 ##__VA_ARGS__);		\
 })
 
-/* helper macro to place programs, maps, license in
+/*
+ * Helper macro to place programs, maps, license in
  * different sections in elf_bpf file. Section names
  * are interpreted by elf_bpf loader
  */
 #define SEC(NAME) __attribute__((section(NAME), used))
 
+#ifndef __always_inline
+#define __always_inline __attribute__((always_inline))
+#endif
+
 /* helper functions called from eBPF programs written in C */
 static void *(*bpf_map_lookup_elem)(void *map, const void *key) =
 	(void *) BPF_FUNC_map_lookup_elem;
@@ -281,7 +286,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
 				  unsigned long long flags) =
 	(void *) BPF_FUNC_skb_adjust_room;
 
-/* a helper structure used by eBPF C program
+/*
+ * Helper structure used by eBPF C program
  * to describe BPF map attributes to libbpf loader
  */
 struct bpf_map_def {
@@ -312,4 +318,145 @@ struct bpf_map_def {
 	bpf_probe_read(dst, sz,						    \
 		       (const void *)__builtin_preserve_access_index(src))
 
+/*
+ * bpf_core_read_str() is a thin wrapper around bpf_probe_read_str()
+ * additionally emitting BPF CO-RE field relocation for specified source
+ * argument.
+ */
+#define bpf_core_read_str(dst, sz, src)					    \
+	bpf_probe_read_str(dst, sz,					    \
+			   (const void *)__builtin_preserve_access_index(src))
+
+#define ___concat(a, b) a ## b
+#define ___apply(fn, n) ___concat(fn, n)
+#define ___nth(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, __11, N, ...) N
+
+/*
+ * return number of provided arguments; used for switch-based variadic macro
+ * definitions (see ___last, ___arrow, etc below)
+ */
+#define ___narg(...) ___nth(_, ##__VA_ARGS__, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+/*
+ * return 0 if no arguments are passed, N - otherwise; used for
+ * recursively-defined macros to specify termination (0) case, and generic
+ * (N) case (e.g., ___read_ptrs, ___core_read)
+ */
+#define ___empty(...) ___nth(_, ##__VA_ARGS__, N, N, N, N, N, N, N, N, N, N, 0)
+
+#define ___last1(x) x
+#define ___last2(a, x) x
+#define ___last3(a, b, x) x
+#define ___last4(a, b, c, x) x
+#define ___last5(a, b, c, d, x) x
+#define ___last6(a, b, c, d, e, x) x
+#define ___last7(a, b, c, d, e, f, x) x
+#define ___last8(a, b, c, d, e, f, g, x) x
+#define ___last9(a, b, c, d, e, f, g, h, x) x
+#define ___last10(a, b, c, d, e, f, g, h, i, x) x
+#define ___last(...) ___apply(___last, ___narg(__VA_ARGS__))(__VA_ARGS__)
+
+#define ___nolast2(a, _) a
+#define ___nolast3(a, b, _) a, b
+#define ___nolast4(a, b, c, _) a, b, c
+#define ___nolast5(a, b, c, d, _) a, b, c, d
+#define ___nolast6(a, b, c, d, e, _) a, b, c, d, e
+#define ___nolast7(a, b, c, d, e, f, _) a, b, c, d, e, f
+#define ___nolast8(a, b, c, d, e, f, g, _) a, b, c, d, e, f, g
+#define ___nolast9(a, b, c, d, e, f, g, h, _) a, b, c, d, e, f, g, h
+#define ___nolast10(a, b, c, d, e, f, g, h, i, _) a, b, c, d, e, f, g, h, i
+#define ___nolast(...) ___apply(___nolast, ___narg(__VA_ARGS__))(__VA_ARGS__)
+
+#define ___arrow1(a) a
+#define ___arrow2(a, b) a->b
+#define ___arrow3(a, b, c) a->b->c
+#define ___arrow4(a, b, c, d) a->b->c->d
+#define ___arrow5(a, b, c, d, e) a->b->c->d->e
+#define ___arrow6(a, b, c, d, e, f) a->b->c->d->e->f
+#define ___arrow7(a, b, c, d, e, f, g) a->b->c->d->e->f->g
+#define ___arrow8(a, b, c, d, e, f, g, h) a->b->c->d->e->f->g->h
+#define ___arrow9(a, b, c, d, e, f, g, h, i) a->b->c->d->e->f->g->h->i
+#define ___arrow10(a, b, c, d, e, f, g, h, i, j) a->b->c->d->e->f->g->h->i->j
+#define ___arrow(...) ___apply(___arrow, ___narg(__VA_ARGS__))(__VA_ARGS__)
+
+#define ___type(...) typeof(___arrow(__VA_ARGS__))
+
+#define ___read(read_fn, dst, src_type, src, accessor)			    \
+	read_fn((void *)(dst), sizeof(*(dst)), &((src_type)(src))->accessor)
+
+/* "recursively" read a sequence of inner pointers using local __t var */
+#define ___rd_last(...)							    \
+	___read(bpf_core_read, &__t,					    \
+		___type(___nolast(__VA_ARGS__)), __t, ___last(__VA_ARGS__));
+#define ___rd_p0(src) const void *__t = src;
+#define ___rd_p1(...) ___rd_p0(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p2(...) ___rd_p1(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p3(...) ___rd_p2(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p4(...) ___rd_p3(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p5(...) ___rd_p4(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p6(...) ___rd_p5(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p7(...) ___rd_p6(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p8(...) ___rd_p7(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___rd_p9(...) ___rd_p8(___nolast(__VA_ARGS__)) ___rd_last(__VA_ARGS__)
+#define ___read_ptrs(src, ...)						    \
+	___apply(___rd_p, ___narg(__VA_ARGS__))(src, __VA_ARGS__)
+
+#define ___core_read0(fn, dst, src, a)					    \
+	___read(fn, dst, ___type(src), src, a);
+#define ___core_readN(fn, dst, src, ...)				    \
+	___read_ptrs(src, ___nolast(__VA_ARGS__))			    \
+	___read(fn, dst, ___type(src, ___nolast(__VA_ARGS__)), __t,	    \
+		___last(__VA_ARGS__));
+#define ___core_read(fn, dst, src, a, ...)				    \
+	___apply(___core_read, ___empty(__VA_ARGS__))(fn, dst,		    \
+						      src, a, ##__VA_ARGS__)
+
+/*
+ * BPF_CORE_READ_INTO() is a more performance-conscious variant of
+ * BPF_CORE_READ(), in which final field is read into user-provided storage.
+ * See BPF_CORE_READ() below for more details on general usage.
+ */
+#define BPF_CORE_READ_INTO(dst, src, a, ...)				    \
+	({								    \
+		___core_read(bpf_core_read, dst, src, a, ##__VA_ARGS__)	    \
+	})
+
+/*
+ * BPF_CORE_READ_STR_INTO() does same "pointer chasing" as
+ * BPF_CORE_READ() for intermediate pointers, but then executes (and returns
+ * corresponding error code) bpf_core_read_str() for final string read.
+ */
+#define BPF_CORE_READ_STR_INTO(dst, src, a, ...)			    \
+	({								    \
+		___core_read(bpf_core_read_str, dst, src, a, ##__VA_ARGS__) \
+	})
+
+/*
+ * BPF_CORE_READ() is used to simplify BPF CO-RE relocatable read, especially
+ * when there are few pointer chasing steps.
+ * E.g., what in non-BPF world (or in BPF w/ BCC) would be something like:
+ *	int x = s->a.b.c->d.e->f->g;
+ * can be succinctly achieved using BPF_CORE_READ as:
+ *	int x = BPF_CORE_READ(s, a.b.c, d.e, f, g);
+ *
+ * BPF_CORE_READ will decompose above statement into 4 bpf_core_read (BPF
+ * CO-RE relocatable bpf_probe_read() wrapper) calls, logically equivalent to:
+ * 1. const void *__t = s->a.b.c;
+ * 2. __t = __t->d.e;
+ * 3. __t = __t->f;
+ * 4. return __t->g;
+ *
+ * Equivalence is logical, because there is a heavy type casting/preservation
+ * involved, as well as all the reads are happening through bpf_probe_read()
+ * calls using __builtin_preserve_access_index() to emit CO-RE relocations.
+ *
+ * N.B. Only up to 9 "field accessors" are supported, which should be more
+ * than enough for any practical purpose.
+ */
+#define BPF_CORE_READ(src, a, ...)					    \
+	({								    \
+		___type(src, a, ##__VA_ARGS__) __r;			    \
+		BPF_CORE_READ_INTO(&__r, src, a, ##__VA_ARGS__);	    \
+		__r;							    \
+	})
+
 #endif
-- 
2.17.1


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

* [PATCH v3 bpf-next 7/7] selftests/bpf: add BPF_CORE_READ and BPF_CORE_READ_STR_INTO macro tests
  2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
                   ` (5 preceding siblings ...)
  2019-10-03 21:28 ` [PATCH v3 bpf-next 6/7] libbpf: add BPF_CORE_READ/BPF_CORE_READ_INTO helpers Andrii Nakryiko
@ 2019-10-03 21:28 ` Andrii Nakryiko
  6 siblings, 0 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-03 21:28 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Validate BPF_CORE_READ correctness and handling of up to 9 levels of
nestedness using cyclic task->(group_leader->)*->tgid chains.

Also add a test of maximum-dpeth BPF_CORE_READ_STR_INTO() macro.

Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/core_reloc.c     |  8 ++-
 .../selftests/bpf/progs/core_reloc_types.h    |  9 ++++
 .../bpf/progs/test_core_reloc_kernel.c        | 54 ++++++++++++++++++-
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index f3863f976a48..21a0dff66241 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -193,8 +193,12 @@ static struct core_reloc_test_case test_cases[] = {
 		.btf_src_file = NULL, /* load from /lib/modules/$(uname -r) */
 		.input = "",
 		.input_len = 0,
-		.output = "\1", /* true */
-		.output_len = 1,
+		.output = STRUCT_TO_CHAR_PTR(core_reloc_kernel_output) {
+			.valid = { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, },
+			.comm = "test_progs\0\0\0\0\0",
+			.comm_len = 11,
+		},
+		.output_len = sizeof(struct core_reloc_kernel_output),
 	},
 
 	/* validate BPF program can use multiple flavors to match against
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index f686a8138d90..9a6bdeb4894c 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -1,5 +1,14 @@
 #include <stdint.h>
 #include <stdbool.h>
+/*
+ * KERNEL
+ */
+
+struct core_reloc_kernel_output {
+	int valid[10];
+	char comm[16];
+	int comm_len;
+};
 
 /*
  * FLAVORS
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
index 684a06cf41ea..5603a6384283 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
@@ -12,9 +12,17 @@ static volatile struct data {
 	char out[256];
 } data;
 
+struct core_reloc_kernel_output {
+	int valid[10];
+	char comm[16];
+	int comm_len;
+};
+
 struct task_struct {
 	int pid;
 	int tgid;
+	char comm[16];
+	struct task_struct *group_leader;
 };
 
 #define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
@@ -23,7 +31,9 @@ SEC("raw_tracepoint/sys_enter")
 int test_core_kernel(void *ctx)
 {
 	struct task_struct *task = (void *)bpf_get_current_task();
+	struct core_reloc_kernel_output *out = (void *)&data.out;
 	uint64_t pid_tgid = bpf_get_current_pid_tgid();
+	uint32_t real_tgid = (uint32_t)pid_tgid;
 	int pid, tgid;
 
 	if (CORE_READ(&pid, &task->pid) ||
@@ -31,7 +41,49 @@ int test_core_kernel(void *ctx)
 		return 1;
 
 	/* validate pid + tgid matches */
-	data.out[0] = (((uint64_t)pid << 32) | tgid) == pid_tgid;
+	out->valid[0] = (((uint64_t)pid << 32) | tgid) == pid_tgid;
+
+	/* test variadic BPF_CORE_READ macros */
+	out->valid[1] = BPF_CORE_READ(task,
+				      tgid) == real_tgid;
+	out->valid[2] = BPF_CORE_READ(task,
+				      group_leader,
+				      tgid) == real_tgid;
+	out->valid[3] = BPF_CORE_READ(task,
+				      group_leader, group_leader,
+				      tgid) == real_tgid;
+	out->valid[4] = BPF_CORE_READ(task,
+				      group_leader, group_leader, group_leader,
+				      tgid) == real_tgid;
+	out->valid[5] = BPF_CORE_READ(task,
+				      group_leader, group_leader, group_leader,
+				      group_leader,
+				      tgid) == real_tgid;
+	out->valid[6] = BPF_CORE_READ(task,
+				      group_leader, group_leader, group_leader,
+				      group_leader, group_leader,
+				      tgid) == real_tgid;
+	out->valid[7] = BPF_CORE_READ(task,
+				      group_leader, group_leader, group_leader,
+				      group_leader, group_leader, group_leader,
+				      tgid) == real_tgid;
+	out->valid[8] = BPF_CORE_READ(task,
+				      group_leader, group_leader, group_leader,
+				      group_leader, group_leader, group_leader,
+				      group_leader,
+				      tgid) == real_tgid;
+	out->valid[9] = BPF_CORE_READ(task,
+				      group_leader, group_leader, group_leader,
+				      group_leader, group_leader, group_leader,
+				      group_leader, group_leader,
+				      tgid) == real_tgid;
+
+	/* test BPF_CORE_READ_STR_INTO() returns correct code and contents */
+	out->comm_len = BPF_CORE_READ_STR_INTO(
+		&out->comm, task,
+		group_leader, group_leader, group_leader, group_leader,
+		group_leader, group_leader, group_leader, group_leader,
+		comm);
 
 	return 0;
 }
-- 
2.17.1


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

* Re: [PATCH v3 bpf-next 1/7] selftests/bpf: undo GCC-specific bpf_helpers.h changes
  2019-10-03 21:28 ` [PATCH v3 bpf-next 1/7] selftests/bpf: undo GCC-specific bpf_helpers.h changes Andrii Nakryiko
@ 2019-10-04  7:00   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 28+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-04  7:00 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Andrii Nakryiko <andriin@fb.com> writes:

> Having GCC provide its own bpf-helper.h is not the right approach and is
> going to be changed. Undo bpf_helpers.h change before moving
> bpf_helpers.h into libbpf.
>
> Acked-by: Song Liu <songliubraving@fb.com>
> Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


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

* Re: [PATCH v3 bpf-next 2/7] selftests/bpf: samples/bpf: split off legacy stuff from bpf_helpers.h
  2019-10-03 21:28 ` [PATCH v3 bpf-next 2/7] selftests/bpf: samples/bpf: split off legacy stuff from bpf_helpers.h Andrii Nakryiko
@ 2019-10-04  7:00   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 28+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-04  7:00 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Andrii Nakryiko <andriin@fb.com> writes:

> Split off few legacy things from bpf_helpers.h into separate
> bpf_legacy.h file:
> - load_{byte|half|word};
> - remove extra inner_idx and numa_node fields from bpf_map_def and
>   introduce bpf_map_def_legacy for use in samples;
> - move BPF_ANNOTATE_KV_PAIR into bpf_legacy.h.
>
> Adjust samples and selftests accordingly by either including
> bpf_legacy.h and using bpf_map_def_legacy, or switching to BTF-defined
> maps altogether.
>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


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

* Re: [PATCH v3 bpf-next 4/7] selftests/bpf: split off tracing-only helpers into bpf_tracing.h
  2019-10-03 21:28 ` [PATCH v3 bpf-next 4/7] selftests/bpf: split off tracing-only helpers into bpf_tracing.h Andrii Nakryiko
@ 2019-10-04  7:01   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 28+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-04  7:01 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Andrii Nakryiko <andriin@fb.com> writes:

> Split-off PT_REGS-related helpers into bpf_tracing.h header. Adjust
> selftests and samples to include it where necessary.
>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-03 21:28 ` [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf Andrii Nakryiko
@ 2019-10-04  7:01   ` Toke Høiland-Jørgensen
  2019-10-04 14:47   ` David Ahern
  1 sibling, 0 replies; 28+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-10-04  7:01 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Andrii Nakryiko <andriin@fb.com> writes:

> Move bpf_helpers.h, bpf_tracing.h, and bpf_endian.h into libbpf. Ensure
> they are installed along the other libbpf headers. Also, adjust
> selftests and samples include path to include libbpf now.
>
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-03 21:28 ` [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf Andrii Nakryiko
  2019-10-04  7:01   ` Toke Høiland-Jørgensen
@ 2019-10-04 14:47   ` David Ahern
  2019-10-04 15:27     ` Andrii Nakryiko
  1 sibling, 1 reply; 28+ messages in thread
From: David Ahern @ 2019-10-04 14:47 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team

On 10/3/19 3:28 PM, Andrii Nakryiko wrote:
> Move bpf_helpers.h, bpf_tracing.h, and bpf_endian.h into libbpf. Ensure
> they are installed along the other libbpf headers. Also, adjust
> selftests and samples include path to include libbpf now.

There are side effects to bringing bpf_helpers.h into libbpf if this
gets propagated to the github sync.

bpf_helpers.h references BPF_FUNC_* which are defined in the
uapi/linux/bpf.h header. That is a kernel version dependent api file
which means attempts to use newer libbpf with older kernel headers is
going to throw errors when compiling bpf programs -- bpf_helpers.h will
contain undefined BPF_FUNC references.

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 14:47   ` David Ahern
@ 2019-10-04 15:27     ` Andrii Nakryiko
  2019-10-04 15:44       ` David Ahern
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-04 15:27 UTC (permalink / raw)
  To: David Ahern
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Fri, Oct 4, 2019 at 7:47 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 10/3/19 3:28 PM, Andrii Nakryiko wrote:
> > Move bpf_helpers.h, bpf_tracing.h, and bpf_endian.h into libbpf. Ensure
> > they are installed along the other libbpf headers. Also, adjust
> > selftests and samples include path to include libbpf now.
>
> There are side effects to bringing bpf_helpers.h into libbpf if this
> gets propagated to the github sync.
>
> bpf_helpers.h references BPF_FUNC_* which are defined in the
> uapi/linux/bpf.h header. That is a kernel version dependent api file
> which means attempts to use newer libbpf with older kernel headers is
> going to throw errors when compiling bpf programs -- bpf_helpers.h will
> contain undefined BPF_FUNC references.

That's true, but I'm wondering if maintaining a copy of that enum in
bpf_helpers.h itself is a good answer here?

bpf_helpers.h will be most probably used with BPF CO-RE and
auto-generated vmlinux.h with all the enums and types. In that case,
you'll probably want to use vmlinux.h for one of the latest kernels
anyways.

Nevertheless, it is a problem and thanks for bringing it up! I'd say
for now we should still go ahead with this move and try to solve with
issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
for someone, it's no worse than it is today when users don't have
bpf_helpers.h at all.

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 15:27     ` Andrii Nakryiko
@ 2019-10-04 15:44       ` David Ahern
  2019-10-04 16:00         ` Andrii Nakryiko
  0 siblings, 1 reply; 28+ messages in thread
From: David Ahern @ 2019-10-04 15:44 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On 10/4/19 9:27 AM, Andrii Nakryiko wrote:
> On Fri, Oct 4, 2019 at 7:47 AM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 10/3/19 3:28 PM, Andrii Nakryiko wrote:
>>> Move bpf_helpers.h, bpf_tracing.h, and bpf_endian.h into libbpf. Ensure
>>> they are installed along the other libbpf headers. Also, adjust
>>> selftests and samples include path to include libbpf now.
>>
>> There are side effects to bringing bpf_helpers.h into libbpf if this
>> gets propagated to the github sync.
>>
>> bpf_helpers.h references BPF_FUNC_* which are defined in the
>> uapi/linux/bpf.h header. That is a kernel version dependent api file
>> which means attempts to use newer libbpf with older kernel headers is
>> going to throw errors when compiling bpf programs -- bpf_helpers.h will
>> contain undefined BPF_FUNC references.
> 
> That's true, but I'm wondering if maintaining a copy of that enum in
> bpf_helpers.h itself is a good answer here?
> 
> bpf_helpers.h will be most probably used with BPF CO-RE and
> auto-generated vmlinux.h with all the enums and types. In that case,
> you'll probably want to use vmlinux.h for one of the latest kernels
> anyways.

I'm not following you; my interpretation of your comment seems like you
are making huge assumptions.

I build bpf programs for specific kernel versions using the devel
packages for the specific kernel of interest.

> 
> Nevertheless, it is a problem and thanks for bringing it up! I'd say
> for now we should still go ahead with this move and try to solve with
> issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> for someone, it's no worse than it is today when users don't have
> bpf_helpers.h at all.
> 

If this syncs to the github libbpf, it will be worse than today in the
sense of compile failures if someone's header file ordering picks
libbpf's bpf_helpers.h over whatever they are using today.

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 15:44       ` David Ahern
@ 2019-10-04 16:00         ` Andrii Nakryiko
  2019-10-04 18:30           ` Jakub Kicinski
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-04 16:00 UTC (permalink / raw)
  To: David Ahern
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 10/4/19 9:27 AM, Andrii Nakryiko wrote:
> > On Fri, Oct 4, 2019 at 7:47 AM David Ahern <dsahern@gmail.com> wrote:
> >>
> >> On 10/3/19 3:28 PM, Andrii Nakryiko wrote:
> >>> Move bpf_helpers.h, bpf_tracing.h, and bpf_endian.h into libbpf. Ensure
> >>> they are installed along the other libbpf headers. Also, adjust
> >>> selftests and samples include path to include libbpf now.
> >>
> >> There are side effects to bringing bpf_helpers.h into libbpf if this
> >> gets propagated to the github sync.
> >>
> >> bpf_helpers.h references BPF_FUNC_* which are defined in the
> >> uapi/linux/bpf.h header. That is a kernel version dependent api file
> >> which means attempts to use newer libbpf with older kernel headers is
> >> going to throw errors when compiling bpf programs -- bpf_helpers.h will
> >> contain undefined BPF_FUNC references.
> >
> > That's true, but I'm wondering if maintaining a copy of that enum in
> > bpf_helpers.h itself is a good answer here?
> >
> > bpf_helpers.h will be most probably used with BPF CO-RE and
> > auto-generated vmlinux.h with all the enums and types. In that case,
> > you'll probably want to use vmlinux.h for one of the latest kernels
> > anyways.
>
> I'm not following you; my interpretation of your comment seems like you
> are making huge assumptions.
>
> I build bpf programs for specific kernel versions using the devel
> packages for the specific kernel of interest.

Sure, and you can keep doing that, just don't include bpf_helpers.h?

What I was saying, though, especially having in mind tracing BPF
programs that need to inspect kernel structures, is that it's quite
impractical to have to build many different versions of BPF programs
for each supported kernel version and distribute them in binary form.
So people usually use BCC and do compilation on-the-fly using BCC's
embedded Clang.

BPF CO-RE is providing an alternative, which will allow to pre-compile
your program once for many different kernels you might be running your
program on. There is tooling that eliminates the need for system
headers. Instead we pre-generate a single vmlinux.h header with all
the types/enums/etc, that are then used w/ BPF CO-RE to build portable
BPF programs capable of working on multiple kernel versions.

So what I was pointing out there was that this vmlinux.h would be
ideally generated from latest kernel and not having latest
BPF_FUNC_xxx shouldn't be a problem. But see below about situation
being worse.

>
> >
> > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > for now we should still go ahead with this move and try to solve with
> > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > for someone, it's no worse than it is today when users don't have
> > bpf_helpers.h at all.
> >
>
> If this syncs to the github libbpf, it will be worse than today in the
> sense of compile failures if someone's header file ordering picks
> libbpf's bpf_helpers.h over whatever they are using today.

Today bpf_helpers.h don't exist for users or am I missing something?
bpf_helpers.h right now are purely for selftests. But they are really
useful outside that context, so I'm making it available for everyone
by distributing with libbpf sources. If bpf_helpers.h doesn't work for
some specific use case, just don't use it (yet?).

I'm still failing to see how it's worse than situation today.

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 16:00         ` Andrii Nakryiko
@ 2019-10-04 18:30           ` Jakub Kicinski
  2019-10-04 18:37             ` Yonghong Song
                               ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Jakub Kicinski @ 2019-10-04 18:30 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Daniel Borkmann, Kernel Team, Jiri Benc

On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
> On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
>> > I'm not following you; my interpretation of your comment seems like you
> > are making huge assumptions.
> >
> > I build bpf programs for specific kernel versions using the devel
> > packages for the specific kernel of interest.  
> 
> Sure, and you can keep doing that, just don't include bpf_helpers.h?
> 
> What I was saying, though, especially having in mind tracing BPF
> programs that need to inspect kernel structures, is that it's quite
> impractical to have to build many different versions of BPF programs
> for each supported kernel version and distribute them in binary form.
> So people usually use BCC and do compilation on-the-fly using BCC's
> embedded Clang.
> 
> BPF CO-RE is providing an alternative, which will allow to pre-compile
> your program once for many different kernels you might be running your
> program on. There is tooling that eliminates the need for system
> headers. Instead we pre-generate a single vmlinux.h header with all
> the types/enums/etc, that are then used w/ BPF CO-RE to build portable
> BPF programs capable of working on multiple kernel versions.
> 
> So what I was pointing out there was that this vmlinux.h would be
> ideally generated from latest kernel and not having latest
> BPF_FUNC_xxx shouldn't be a problem. But see below about situation
> being worse.

Surely for distroes tho - they would have kernel headers matching the
kernel release they ship. If parts of libbpf from GH only work with
the latest kernel, distroes should ship libbpf from the kernel source,
rather than GH.

> > > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > > for now we should still go ahead with this move and try to solve with
> > > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > > for someone, it's no worse than it is today when users don't have
> > > bpf_helpers.h at all.
> > >  
> >
> > If this syncs to the github libbpf, it will be worse than today in the
> > sense of compile failures if someone's header file ordering picks
> > libbpf's bpf_helpers.h over whatever they are using today.  
> 
> Today bpf_helpers.h don't exist for users or am I missing something?
> bpf_helpers.h right now are purely for selftests. But they are really
> useful outside that context, so I'm making it available for everyone
> by distributing with libbpf sources. If bpf_helpers.h doesn't work for
> some specific use case, just don't use it (yet?).
> 
> I'm still failing to see how it's worse than situation today.

Having a header which works today, but may not work tomorrow is going
to be pretty bad user experience :( No matter how many warnings you put
in the source people will get caught off guard by this :(

If you define the current state as "users can use all features of
libbpf and nothing should break on libbpf update" (which is in my
understanding a goal of the project, we bent over backwards trying 
to not break things) then adding this header will in fact make things
worse. The statement in quotes would no longer be true, no?

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 18:30           ` Jakub Kicinski
@ 2019-10-04 18:37             ` Yonghong Song
  2019-10-04 21:04               ` Jakub Kicinski
  2019-10-08 15:37               ` Jiri Benc
  2019-10-04 20:21             ` Andrii Nakryiko
  2019-10-08 15:29             ` Jiri Benc
  2 siblings, 2 replies; 28+ messages in thread
From: Yonghong Song @ 2019-10-04 18:37 UTC (permalink / raw)
  To: Jakub Kicinski, Andrii Nakryiko
  Cc: David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Daniel Borkmann, Kernel Team, Jiri Benc



On 10/4/19 11:30 AM, Jakub Kicinski wrote:
> On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
>> On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
>>>> I'm not following you; my interpretation of your comment seems like you
>>> are making huge assumptions.
>>>
>>> I build bpf programs for specific kernel versions using the devel
>>> packages for the specific kernel of interest.
>>
>> Sure, and you can keep doing that, just don't include bpf_helpers.h?
>>
>> What I was saying, though, especially having in mind tracing BPF
>> programs that need to inspect kernel structures, is that it's quite
>> impractical to have to build many different versions of BPF programs
>> for each supported kernel version and distribute them in binary form.
>> So people usually use BCC and do compilation on-the-fly using BCC's
>> embedded Clang.
>>
>> BPF CO-RE is providing an alternative, which will allow to pre-compile
>> your program once for many different kernels you might be running your
>> program on. There is tooling that eliminates the need for system
>> headers. Instead we pre-generate a single vmlinux.h header with all
>> the types/enums/etc, that are then used w/ BPF CO-RE to build portable
>> BPF programs capable of working on multiple kernel versions.
>>
>> So what I was pointing out there was that this vmlinux.h would be
>> ideally generated from latest kernel and not having latest
>> BPF_FUNC_xxx shouldn't be a problem. But see below about situation
>> being worse.
> 
> Surely for distroes tho - they would have kernel headers matching the
> kernel release they ship. If parts of libbpf from GH only work with
> the latest kernel, distroes should ship libbpf from the kernel source,
> rather than GH.
> 
>>>> Nevertheless, it is a problem and thanks for bringing it up! I'd say
>>>> for now we should still go ahead with this move and try to solve with
>>>> issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
>>>> for someone, it's no worse than it is today when users don't have
>>>> bpf_helpers.h at all.
>>>>   
>>>
>>> If this syncs to the github libbpf, it will be worse than today in the
>>> sense of compile failures if someone's header file ordering picks
>>> libbpf's bpf_helpers.h over whatever they are using today.
>>
>> Today bpf_helpers.h don't exist for users or am I missing something?
>> bpf_helpers.h right now are purely for selftests. But they are really
>> useful outside that context, so I'm making it available for everyone
>> by distributing with libbpf sources. If bpf_helpers.h doesn't work for
>> some specific use case, just don't use it (yet?).
>>
>> I'm still failing to see how it's worse than situation today.
> 
> Having a header which works today, but may not work tomorrow is going
> to be pretty bad user experience :( No matter how many warnings you put
> in the source people will get caught off guard by this :(
> 
> If you define the current state as "users can use all features of
> libbpf and nothing should break on libbpf update" (which is in my
> understanding a goal of the project, we bent over backwards trying
> to not break things) then adding this header will in fact make things
> worse. The statement in quotes would no longer be true, no?

distro can package bpf/btf uapi headers into libbpf package.
Users linking with libbpf.a/libbpf.so can use bpf/btf.h with include
path pointing to libbpf dev package include directory.
Could this work?

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 18:30           ` Jakub Kicinski
  2019-10-04 18:37             ` Yonghong Song
@ 2019-10-04 20:21             ` Andrii Nakryiko
  2019-10-04 21:06               ` Daniel Borkmann
  2019-10-08 15:29             ` Jiri Benc
  2 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-04 20:21 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Daniel Borkmann, Kernel Team, Jiri Benc

On Fri, Oct 4, 2019 at 11:30 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
> > On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
> >> > I'm not following you; my interpretation of your comment seems like you
> > > are making huge assumptions.
> > >
> > > I build bpf programs for specific kernel versions using the devel
> > > packages for the specific kernel of interest.
> >
> > Sure, and you can keep doing that, just don't include bpf_helpers.h?
> >
> > What I was saying, though, especially having in mind tracing BPF
> > programs that need to inspect kernel structures, is that it's quite
> > impractical to have to build many different versions of BPF programs
> > for each supported kernel version and distribute them in binary form.
> > So people usually use BCC and do compilation on-the-fly using BCC's
> > embedded Clang.
> >
> > BPF CO-RE is providing an alternative, which will allow to pre-compile
> > your program once for many different kernels you might be running your
> > program on. There is tooling that eliminates the need for system
> > headers. Instead we pre-generate a single vmlinux.h header with all
> > the types/enums/etc, that are then used w/ BPF CO-RE to build portable
> > BPF programs capable of working on multiple kernel versions.
> >
> > So what I was pointing out there was that this vmlinux.h would be
> > ideally generated from latest kernel and not having latest
> > BPF_FUNC_xxx shouldn't be a problem. But see below about situation
> > being worse.
>
> Surely for distroes tho - they would have kernel headers matching the
> kernel release they ship. If parts of libbpf from GH only work with
> the latest kernel, distroes should ship libbpf from the kernel source,
> rather than GH.
>
> > > > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > > > for now we should still go ahead with this move and try to solve with
> > > > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > > > for someone, it's no worse than it is today when users don't have
> > > > bpf_helpers.h at all.
> > > >
> > >
> > > If this syncs to the github libbpf, it will be worse than today in the
> > > sense of compile failures if someone's header file ordering picks
> > > libbpf's bpf_helpers.h over whatever they are using today.
> >
> > Today bpf_helpers.h don't exist for users or am I missing something?
> > bpf_helpers.h right now are purely for selftests. But they are really
> > useful outside that context, so I'm making it available for everyone
> > by distributing with libbpf sources. If bpf_helpers.h doesn't work for
> > some specific use case, just don't use it (yet?).
> >
> > I'm still failing to see how it's worse than situation today.
>
> Having a header which works today, but may not work tomorrow is going
> to be pretty bad user experience :( No matter how many warnings you put
> in the source people will get caught off guard by this :(
>
> If you define the current state as "users can use all features of
> libbpf and nothing should break on libbpf update" (which is in my
> understanding a goal of the project, we bent over backwards trying
> to not break things) then adding this header will in fact make things
> worse. The statement in quotes would no longer be true, no?

So there are few things here.

1. About "adding bpf_helpers.h will make things worse". I
categorically disagree, bpf_helpers.h doesn't exist in user land at
all and it's sorely missing. So adding it is strictly better
experience already. Right now people have to re-declare those helper
signatures and do all kinds of unnecessary hackery just to be able to
use BPF stuff, and they still can run into the same problem with
having too old kernel headers.

Also, I think we should have informal notion of "experimental"
features and APIs which we add to get real-world experience of using
it, before we crystalize it to something that we have to maintain
backwards compatibility and never-breaking user experience for. Its
sometimes impossible to finesse all the tiny issues before we get
prolonged enough experience w/ real applications in a real set up. If
we are going to wait to resolve all the tiny possible problems, we'll
cripple outselves very significantly. I think bpf_helpers.h move walls
into this "experimental" thing, which we obviously will try to
"stabilize" ASAP, but we need it to be part of libbpf first to start
using it in production. So in that sense it's a huge improvement
already and I think we should land it as is and keep improving it, not
stall progress right now.

2. As to the problem of running bleeding-edge libbpf against older
kernel. There are few possible solutions:

a. we hard-code all those BPF_FUNC_ constants. Super painful and not
nice, but will work.

b. copy/paste enum bpf_func_id definition into bpf_helpers.h itself
and try to keep it in sync with UAPI. Apart from obvious redundancy
that involves, we also will need to make sure this doesn't conflict
with vmlinux.h, so enum name should be different and each value should
be different (which means some wort of additional prefix or suffix).

c. BPF UAPI header has __BPF_FUNC_MAPPER macro "iterating" over all
defined values for a particular kernel version. We can use that and
additional macro trickery to conditionally define helpers. Again, we
need to keep in mind that w/ vmlinux.h there is no such macro, so this
should work as well.

I'm happy to hear opinions about these choices (maybe there are some
other I missed), but in any case I'd like to do it in a follow up
patch and land this one as is. It has already quite a lot packed in
it. I personally lean towards c) as it will have a benefit of not
declaring helpers that are not supported by kernel we are compiling
against, even though it requires additional macro trickery.

Opinions?

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 18:37             ` Yonghong Song
@ 2019-10-04 21:04               ` Jakub Kicinski
  2019-10-08 15:37               ` Jiri Benc
  1 sibling, 0 replies; 28+ messages in thread
From: Jakub Kicinski @ 2019-10-04 21:04 UTC (permalink / raw)
  To: Yonghong Song, Andrii Nakryiko
  Cc: David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Daniel Borkmann, Kernel Team, Jiri Benc

On Fri, 4 Oct 2019 18:37:44 +0000, Yonghong Song wrote:
> > Having a header which works today, but may not work tomorrow is going
> > to be pretty bad user experience :( No matter how many warnings you put
> > in the source people will get caught off guard by this :(
> > 
> > If you define the current state as "users can use all features of
> > libbpf and nothing should break on libbpf update" (which is in my
> > understanding a goal of the project, we bent over backwards trying
> > to not break things) then adding this header will in fact make things
> > worse. The statement in quotes would no longer be true, no?  
> 
> distro can package bpf/btf uapi headers into libbpf package.
> Users linking with libbpf.a/libbpf.so can use bpf/btf.h with include
> path pointing to libbpf dev package include directory.
> Could this work?

IMHO that'd be the first thing to try.

Andrii, your option (c) also seems to me like a pretty natural fit,
although it'd be a little strange to have code depending on the kernel
version in tree :S

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 20:21             ` Andrii Nakryiko
@ 2019-10-04 21:06               ` Daniel Borkmann
  2019-10-04 21:58                 ` Daniel Borkmann
  0 siblings, 1 reply; 28+ messages in thread
From: Daniel Borkmann @ 2019-10-04 21:06 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jakub Kicinski, David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team, Jiri Benc

On Fri, Oct 04, 2019 at 01:21:55PM -0700, Andrii Nakryiko wrote:
> On Fri, Oct 4, 2019 at 11:30 AM Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
> > On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
> > > On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
> > >> > I'm not following you; my interpretation of your comment seems like you
> > > > are making huge assumptions.
> > > >
> > > > I build bpf programs for specific kernel versions using the devel
> > > > packages for the specific kernel of interest.
> > >
> > > Sure, and you can keep doing that, just don't include bpf_helpers.h?
> > >
> > > What I was saying, though, especially having in mind tracing BPF
> > > programs that need to inspect kernel structures, is that it's quite
> > > impractical to have to build many different versions of BPF programs
> > > for each supported kernel version and distribute them in binary form.
> > > So people usually use BCC and do compilation on-the-fly using BCC's
> > > embedded Clang.
> > >
> > > BPF CO-RE is providing an alternative, which will allow to pre-compile
> > > your program once for many different kernels you might be running your
> > > program on. There is tooling that eliminates the need for system
> > > headers. Instead we pre-generate a single vmlinux.h header with all
> > > the types/enums/etc, that are then used w/ BPF CO-RE to build portable
> > > BPF programs capable of working on multiple kernel versions.
> > >
> > > So what I was pointing out there was that this vmlinux.h would be
> > > ideally generated from latest kernel and not having latest
> > > BPF_FUNC_xxx shouldn't be a problem. But see below about situation
> > > being worse.
> >
> > Surely for distroes tho - they would have kernel headers matching the
> > kernel release they ship. If parts of libbpf from GH only work with
> > the latest kernel, distroes should ship libbpf from the kernel source,
> > rather than GH.
> >
> > > > > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > > > > for now we should still go ahead with this move and try to solve with
> > > > > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > > > > for someone, it's no worse than it is today when users don't have
> > > > > bpf_helpers.h at all.
> > > > >
> > > >
> > > > If this syncs to the github libbpf, it will be worse than today in the
> > > > sense of compile failures if someone's header file ordering picks
> > > > libbpf's bpf_helpers.h over whatever they are using today.
> > >
> > > Today bpf_helpers.h don't exist for users or am I missing something?
> > > bpf_helpers.h right now are purely for selftests. But they are really
> > > useful outside that context, so I'm making it available for everyone
> > > by distributing with libbpf sources. If bpf_helpers.h doesn't work for
> > > some specific use case, just don't use it (yet?).
> > >
> > > I'm still failing to see how it's worse than situation today.
> >
> > Having a header which works today, but may not work tomorrow is going
> > to be pretty bad user experience :( No matter how many warnings you put
> > in the source people will get caught off guard by this :(
> >
> > If you define the current state as "users can use all features of
> > libbpf and nothing should break on libbpf update" (which is in my
> > understanding a goal of the project, we bent over backwards trying
> > to not break things) then adding this header will in fact make things
> > worse. The statement in quotes would no longer be true, no?
> 
> So there are few things here.
> 
> 1. About "adding bpf_helpers.h will make things worse". I
> categorically disagree, bpf_helpers.h doesn't exist in user land at
> all and it's sorely missing. So adding it is strictly better
> experience already. Right now people have to re-declare those helper
> signatures and do all kinds of unnecessary hackery just to be able to
> use BPF stuff, and they still can run into the same problem with
> having too old kernel headers.

Right, so apps tend to ship their own uapi bpf.h header and helper
signatures to avoid these issues. But question becomes once they
start using soley bpf_helper.h (also in non-tracing context which
is very reasonable to assume), then things might break with the patch
as-is once they have a newer libbpf with more signatures than their
linux/bpf.h defines (and yes, pulling from GH will have this problem),
so we'd need to have an answer to that in order to avoid breaking
compilation.

[...]
> 2. As to the problem of running bleeding-edge libbpf against older
> kernel. There are few possible solutions:
> 
> a. we hard-code all those BPF_FUNC_ constants. Super painful and not
> nice, but will work.
> 
> b. copy/paste enum bpf_func_id definition into bpf_helpers.h itself
> and try to keep it in sync with UAPI. Apart from obvious redundancy
> that involves, we also will need to make sure this doesn't conflict
> with vmlinux.h, so enum name should be different and each value should
> be different (which means some wort of additional prefix or suffix).
> 
> c. BPF UAPI header has __BPF_FUNC_MAPPER macro "iterating" over all
> defined values for a particular kernel version. We can use that and
> additional macro trickery to conditionally define helpers. Again, we
> need to keep in mind that w/ vmlinux.h there is no such macro, so this
> should work as well.
> 
> I'm happy to hear opinions about these choices (maybe there are some
> other I missed), but in any case I'd like to do it in a follow up
> patch and land this one as is. It has already quite a lot packed in
> it. I personally lean towards c) as it will have a benefit of not
> declaring helpers that are not supported by kernel we are compiling
> against, even though it requires additional macro trickery.
> 
> Opinions?

Was thinking about something like c) as well. So I tried to do a quick
hack. Here is how it could work, but it needs a small change in the
__BPF_FUNC_MAPPER(), at least I didn't find an immediate way around it:

static void (*__unspec)(void);
static void *(*__map_lookup_elem)(void *map, const void *key);
static int (*__map_update_elem)(void *map, const void *key, const void *value, unsigned long long flags);
static int (*__map_delete_elem)(void *map, const void *key);
static int (*__bpf_probe_read)(void *dst, int size, const void *unsafe_ptr);

#define __BPF_FUNC_MAPPER(FN, DELIM)    \
        FN(unspec) DELIM                \
        FN(map_lookup_elem) DELIM       \
        FN(map_update_elem) DELIM       \
        FN(map_delete_elem) DELIM

#define __BPF_ENUM_FN(x) BPF_FUNC_ ## x
enum bpf_func_id {
#define COM ,
        __BPF_FUNC_MAPPER(__BPF_ENUM_FN, COM)
        __BPF_FUNC_MAX_ID,
};
#undef __BPF_ENUM_FN

#define __BPF_ASSIGN_FN(x) typeof(__ ## x) x = (void *)BPF_FUNC_ ## x
#define SEM ;
__BPF_FUNC_MAPPER(__BPF_ASSIGN_FN, SEM)
#undef __BPF_ASSIGN_FN

int main(void)
{
        return(unsigned long)map_lookup_elem(0, 0);
}

It does seem to eat it:

$ gcc foo.c 
$

In short, libbpf's bpf_helper.h would define signatures (or better: types) as
above prefixed with __. The __BPF_FUNC_MAPPER() and the enum bpf_func_id {}
is the one in the uapi header, so out of our control from libbpf pov. But
aside from the signatures, the bpf_helper.h would have __BPF_ASSIGN_FN() and
this would avoid the breakage issue. The only thing libbpf needs to have in
the bpf_helper.h are the /latest/ signatures with /matching/ names.

Right now in the __BPF_FUNC_MAPPER() there is this comma, we probably need
to make this a parameter, at least from a quick glance I didn't see a way
to work around it.

Cheers,
Daniel

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 21:06               ` Daniel Borkmann
@ 2019-10-04 21:58                 ` Daniel Borkmann
  2019-10-04 22:47                   ` Andrii Nakryiko
  0 siblings, 1 reply; 28+ messages in thread
From: Daniel Borkmann @ 2019-10-04 21:58 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jakub Kicinski, David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team, Jiri Benc

On Fri, Oct 04, 2019 at 11:06:13PM +0200, Daniel Borkmann wrote:
> On Fri, Oct 04, 2019 at 01:21:55PM -0700, Andrii Nakryiko wrote:
> > On Fri, Oct 4, 2019 at 11:30 AM Jakub Kicinski
> > <jakub.kicinski@netronome.com> wrote:
> > > On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
> > > > On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
> > > >> > I'm not following you; my interpretation of your comment seems like you
> > > > > are making huge assumptions.
> > > > >
> > > > > I build bpf programs for specific kernel versions using the devel
> > > > > packages for the specific kernel of interest.
> > > >
> > > > Sure, and you can keep doing that, just don't include bpf_helpers.h?
> > > >
> > > > What I was saying, though, especially having in mind tracing BPF
> > > > programs that need to inspect kernel structures, is that it's quite
> > > > impractical to have to build many different versions of BPF programs
> > > > for each supported kernel version and distribute them in binary form.
> > > > So people usually use BCC and do compilation on-the-fly using BCC's
> > > > embedded Clang.
> > > >
> > > > BPF CO-RE is providing an alternative, which will allow to pre-compile
> > > > your program once for many different kernels you might be running your
> > > > program on. There is tooling that eliminates the need for system
> > > > headers. Instead we pre-generate a single vmlinux.h header with all
> > > > the types/enums/etc, that are then used w/ BPF CO-RE to build portable
> > > > BPF programs capable of working on multiple kernel versions.
> > > >
> > > > So what I was pointing out there was that this vmlinux.h would be
> > > > ideally generated from latest kernel and not having latest
> > > > BPF_FUNC_xxx shouldn't be a problem. But see below about situation
> > > > being worse.
> > >
> > > Surely for distroes tho - they would have kernel headers matching the
> > > kernel release they ship. If parts of libbpf from GH only work with
> > > the latest kernel, distroes should ship libbpf from the kernel source,
> > > rather than GH.
> > >
> > > > > > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > > > > > for now we should still go ahead with this move and try to solve with
> > > > > > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > > > > > for someone, it's no worse than it is today when users don't have
> > > > > > bpf_helpers.h at all.
> > > > > >
> > > > >
> > > > > If this syncs to the github libbpf, it will be worse than today in the
> > > > > sense of compile failures if someone's header file ordering picks
> > > > > libbpf's bpf_helpers.h over whatever they are using today.
> > > >
> > > > Today bpf_helpers.h don't exist for users or am I missing something?
> > > > bpf_helpers.h right now are purely for selftests. But they are really
> > > > useful outside that context, so I'm making it available for everyone
> > > > by distributing with libbpf sources. If bpf_helpers.h doesn't work for
> > > > some specific use case, just don't use it (yet?).
> > > >
> > > > I'm still failing to see how it's worse than situation today.
> > >
> > > Having a header which works today, but may not work tomorrow is going
> > > to be pretty bad user experience :( No matter how many warnings you put
> > > in the source people will get caught off guard by this :(
> > >
> > > If you define the current state as "users can use all features of
> > > libbpf and nothing should break on libbpf update" (which is in my
> > > understanding a goal of the project, we bent over backwards trying
> > > to not break things) then adding this header will in fact make things
> > > worse. The statement in quotes would no longer be true, no?
> > 
> > So there are few things here.
> > 
> > 1. About "adding bpf_helpers.h will make things worse". I
> > categorically disagree, bpf_helpers.h doesn't exist in user land at
> > all and it's sorely missing. So adding it is strictly better
> > experience already. Right now people have to re-declare those helper
> > signatures and do all kinds of unnecessary hackery just to be able to
> > use BPF stuff, and they still can run into the same problem with
> > having too old kernel headers.
> 
> Right, so apps tend to ship their own uapi bpf.h header and helper
> signatures to avoid these issues. But question becomes once they
> start using soley bpf_helper.h (also in non-tracing context which
> is very reasonable to assume), then things might break with the patch
> as-is once they have a newer libbpf with more signatures than their
> linux/bpf.h defines (and yes, pulling from GH will have this problem),
> so we'd need to have an answer to that in order to avoid breaking
> compilation.
> 
> [...]
> > 2. As to the problem of running bleeding-edge libbpf against older
> > kernel. There are few possible solutions:
> > 
> > a. we hard-code all those BPF_FUNC_ constants. Super painful and not
> > nice, but will work.
> > 
> > b. copy/paste enum bpf_func_id definition into bpf_helpers.h itself
> > and try to keep it in sync with UAPI. Apart from obvious redundancy
> > that involves, we also will need to make sure this doesn't conflict
> > with vmlinux.h, so enum name should be different and each value should
> > be different (which means some wort of additional prefix or suffix).
> > 
> > c. BPF UAPI header has __BPF_FUNC_MAPPER macro "iterating" over all
> > defined values for a particular kernel version. We can use that and
> > additional macro trickery to conditionally define helpers. Again, we
> > need to keep in mind that w/ vmlinux.h there is no such macro, so this
> > should work as well.
> > 
> > I'm happy to hear opinions about these choices (maybe there are some
> > other I missed), but in any case I'd like to do it in a follow up
> > patch and land this one as is. It has already quite a lot packed in
> > it. I personally lean towards c) as it will have a benefit of not
> > declaring helpers that are not supported by kernel we are compiling
> > against, even though it requires additional macro trickery.
> > 
> > Opinions?
> 
> Was thinking about something like c) as well. So I tried to do a quick
> hack. Here is how it could work, but it needs a small change in the
> __BPF_FUNC_MAPPER(), at least I didn't find an immediate way around it:
> 
> static void (*__unspec)(void);
> static void *(*__map_lookup_elem)(void *map, const void *key);
> static int (*__map_update_elem)(void *map, const void *key, const void *value, unsigned long long flags);
> static int (*__map_delete_elem)(void *map, const void *key);
> static int (*__bpf_probe_read)(void *dst, int size, const void *unsafe_ptr);
> 
> #define __BPF_FUNC_MAPPER(FN, DELIM)    \
>         FN(unspec) DELIM                \
>         FN(map_lookup_elem) DELIM       \
>         FN(map_update_elem) DELIM       \
>         FN(map_delete_elem) DELIM
> 
> #define __BPF_ENUM_FN(x) BPF_FUNC_ ## x
> enum bpf_func_id {
> #define COM ,
>         __BPF_FUNC_MAPPER(__BPF_ENUM_FN, COM)
>         __BPF_FUNC_MAX_ID,
> };
> #undef __BPF_ENUM_FN
> 
> #define __BPF_ASSIGN_FN(x) typeof(__ ## x) x = (void *)BPF_FUNC_ ## x
> #define SEM ;
> __BPF_FUNC_MAPPER(__BPF_ASSIGN_FN, SEM)
> #undef __BPF_ASSIGN_FN
> 
> int main(void)
> {
>         return(unsigned long)map_lookup_elem(0, 0);
> }
> 
> It does seem to eat it:
> 
> $ gcc foo.c 
> $
> 
> In short, libbpf's bpf_helper.h would define signatures (or better: types) as
> above prefixed with __. The __BPF_FUNC_MAPPER() and the enum bpf_func_id {}
> is the one in the uapi header, so out of our control from libbpf pov. But
> aside from the signatures, the bpf_helper.h would have __BPF_ASSIGN_FN() and
> this would avoid the breakage issue. The only thing libbpf needs to have in
> the bpf_helper.h are the /latest/ signatures with /matching/ names.
> 
> Right now in the __BPF_FUNC_MAPPER() there is this comma, we probably need
> to make this a parameter, at least from a quick glance I didn't see a way
> to work around it.

Small kernel side change would look like below, seems useful regardless, imho.
Perhaps that approach where you'd only need signatures like above would also
resolve the 'concern' from gcc folks where they wanted a special helper attribute
thingy to achieve the same.

From e89cd0d873ae07e82526ab573746212a5e1b69cb Mon Sep 17 00:00:00 2001
Message-Id: <e89cd0d873ae07e82526ab573746212a5e1b69cb.1570225802.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 4 Oct 2019 23:46:25 +0200
Subject: [PATCH] bpf: Allow __BPF_FUNC_MAPPER to be more customizable

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/uapi/linux/bpf.h | 228 ++++++++++++++++++++-------------------
 1 file changed, 116 insertions(+), 112 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 77c6be96d676..0f7e733d4d54 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2751,118 +2751,122 @@ union bpf_attr {
  *
  *		**-EPROTONOSUPPORT** IP packet version is not 4 or 6
  */
-#define __BPF_FUNC_MAPPER(FN)		\
-	FN(unspec),			\
-	FN(map_lookup_elem),		\
-	FN(map_update_elem),		\
-	FN(map_delete_elem),		\
-	FN(probe_read),			\
-	FN(ktime_get_ns),		\
-	FN(trace_printk),		\
-	FN(get_prandom_u32),		\
-	FN(get_smp_processor_id),	\
-	FN(skb_store_bytes),		\
-	FN(l3_csum_replace),		\
-	FN(l4_csum_replace),		\
-	FN(tail_call),			\
-	FN(clone_redirect),		\
-	FN(get_current_pid_tgid),	\
-	FN(get_current_uid_gid),	\
-	FN(get_current_comm),		\
-	FN(get_cgroup_classid),		\
-	FN(skb_vlan_push),		\
-	FN(skb_vlan_pop),		\
-	FN(skb_get_tunnel_key),		\
-	FN(skb_set_tunnel_key),		\
-	FN(perf_event_read),		\
-	FN(redirect),			\
-	FN(get_route_realm),		\
-	FN(perf_event_output),		\
-	FN(skb_load_bytes),		\
-	FN(get_stackid),		\
-	FN(csum_diff),			\
-	FN(skb_get_tunnel_opt),		\
-	FN(skb_set_tunnel_opt),		\
-	FN(skb_change_proto),		\
-	FN(skb_change_type),		\
-	FN(skb_under_cgroup),		\
-	FN(get_hash_recalc),		\
-	FN(get_current_task),		\
-	FN(probe_write_user),		\
-	FN(current_task_under_cgroup),	\
-	FN(skb_change_tail),		\
-	FN(skb_pull_data),		\
-	FN(csum_update),		\
-	FN(set_hash_invalid),		\
-	FN(get_numa_node_id),		\
-	FN(skb_change_head),		\
-	FN(xdp_adjust_head),		\
-	FN(probe_read_str),		\
-	FN(get_socket_cookie),		\
-	FN(get_socket_uid),		\
-	FN(set_hash),			\
-	FN(setsockopt),			\
-	FN(skb_adjust_room),		\
-	FN(redirect_map),		\
-	FN(sk_redirect_map),		\
-	FN(sock_map_update),		\
-	FN(xdp_adjust_meta),		\
-	FN(perf_event_read_value),	\
-	FN(perf_prog_read_value),	\
-	FN(getsockopt),			\
-	FN(override_return),		\
-	FN(sock_ops_cb_flags_set),	\
-	FN(msg_redirect_map),		\
-	FN(msg_apply_bytes),		\
-	FN(msg_cork_bytes),		\
-	FN(msg_pull_data),		\
-	FN(bind),			\
-	FN(xdp_adjust_tail),		\
-	FN(skb_get_xfrm_state),		\
-	FN(get_stack),			\
-	FN(skb_load_bytes_relative),	\
-	FN(fib_lookup),			\
-	FN(sock_hash_update),		\
-	FN(msg_redirect_hash),		\
-	FN(sk_redirect_hash),		\
-	FN(lwt_push_encap),		\
-	FN(lwt_seg6_store_bytes),	\
-	FN(lwt_seg6_adjust_srh),	\
-	FN(lwt_seg6_action),		\
-	FN(rc_repeat),			\
-	FN(rc_keydown),			\
-	FN(skb_cgroup_id),		\
-	FN(get_current_cgroup_id),	\
-	FN(get_local_storage),		\
-	FN(sk_select_reuseport),	\
-	FN(skb_ancestor_cgroup_id),	\
-	FN(sk_lookup_tcp),		\
-	FN(sk_lookup_udp),		\
-	FN(sk_release),			\
-	FN(map_push_elem),		\
-	FN(map_pop_elem),		\
-	FN(map_peek_elem),		\
-	FN(msg_push_data),		\
-	FN(msg_pop_data),		\
-	FN(rc_pointer_rel),		\
-	FN(spin_lock),			\
-	FN(spin_unlock),		\
-	FN(sk_fullsock),		\
-	FN(tcp_sock),			\
-	FN(skb_ecn_set_ce),		\
-	FN(get_listener_sock),		\
-	FN(skc_lookup_tcp),		\
-	FN(tcp_check_syncookie),	\
-	FN(sysctl_get_name),		\
-	FN(sysctl_get_current_value),	\
-	FN(sysctl_get_new_value),	\
-	FN(sysctl_set_new_value),	\
-	FN(strtol),			\
-	FN(strtoul),			\
-	FN(sk_storage_get),		\
-	FN(sk_storage_delete),		\
-	FN(send_signal),		\
-	FN(tcp_gen_syncookie),
+#define ____BPF_FUNC_MAPPER(FN, DL)	\
+	FN(unspec) DL			\
+	FN(map_lookup_elem) DL		\
+	FN(map_update_elem) DL		\
+	FN(map_delete_elem) DL		\
+	FN(probe_read) DL		\
+	FN(ktime_get_ns) DL		\
+	FN(trace_printk) DL		\
+	FN(get_prandom_u32) DL		\
+	FN(get_smp_processor_id) DL	\
+	FN(skb_store_bytes) DL		\
+	FN(l3_csum_replace) DL		\
+	FN(l4_csum_replace) DL		\
+	FN(tail_call) DL		\
+	FN(clone_redirect) DL		\
+	FN(get_current_pid_tgid) DL	\
+	FN(get_current_uid_gid) DL	\
+	FN(get_current_comm) DL		\
+	FN(get_cgroup_classid) DL	\
+	FN(skb_vlan_push) DL		\
+	FN(skb_vlan_pop) DL		\
+	FN(skb_get_tunnel_key) DL	\
+	FN(skb_set_tunnel_key) DL	\
+	FN(perf_event_read) DL		\
+	FN(redirect) DL			\
+	FN(get_route_realm) DL		\
+	FN(perf_event_output) DL	\
+	FN(skb_load_bytes) DL		\
+	FN(get_stackid) DL		\
+	FN(csum_diff) DL		\
+	FN(skb_get_tunnel_opt) DL	\
+	FN(skb_set_tunnel_opt) DL	\
+	FN(skb_change_proto) DL		\
+	FN(skb_change_type) DL		\
+	FN(skb_under_cgroup) DL		\
+	FN(get_hash_recalc) DL		\
+	FN(get_current_task) DL		\
+	FN(probe_write_user) DL		\
+	FN(current_task_under_cgroup) DL\
+	FN(skb_change_tail) DL		\
+	FN(skb_pull_data) DL		\
+	FN(csum_update) DL		\
+	FN(set_hash_invalid) DL		\
+	FN(get_numa_node_id) DL		\
+	FN(skb_change_head) DL		\
+	FN(xdp_adjust_head) DL		\
+	FN(probe_read_str) DL		\
+	FN(get_socket_cookie) DL	\
+	FN(get_socket_uid) DL		\
+	FN(set_hash) DL			\
+	FN(setsockopt) DL		\
+	FN(skb_adjust_room) DL		\
+	FN(redirect_map) DL		\
+	FN(sk_redirect_map) DL		\
+	FN(sock_map_update) DL		\
+	FN(xdp_adjust_meta) DL		\
+	FN(perf_event_read_value) DL	\
+	FN(perf_prog_read_value) DL	\
+	FN(getsockopt) DL		\
+	FN(override_return) DL		\
+	FN(sock_ops_cb_flags_set) DL	\
+	FN(msg_redirect_map) DL		\
+	FN(msg_apply_bytes) DL		\
+	FN(msg_cork_bytes) DL		\
+	FN(msg_pull_data) DL		\
+	FN(bind) DL			\
+	FN(xdp_adjust_tail) DL		\
+	FN(skb_get_xfrm_state) DL	\
+	FN(get_stack) DL		\
+	FN(skb_load_bytes_relative) DL	\
+	FN(fib_lookup) DL		\
+	FN(sock_hash_update) DL		\
+	FN(msg_redirect_hash) DL	\
+	FN(sk_redirect_hash) DL		\
+	FN(lwt_push_encap) DL		\
+	FN(lwt_seg6_store_bytes) DL	\
+	FN(lwt_seg6_adjust_srh) DL	\
+	FN(lwt_seg6_action) DL		\
+	FN(rc_repeat) DL		\
+	FN(rc_keydown) DL		\
+	FN(skb_cgroup_id) DL		\
+	FN(get_current_cgroup_id) DL	\
+	FN(get_local_storage) DL	\
+	FN(sk_select_reuseport) DL	\
+	FN(skb_ancestor_cgroup_id) DL	\
+	FN(sk_lookup_tcp) DL		\
+	FN(sk_lookup_udp) DL		\
+	FN(sk_release) DL		\
+	FN(map_push_elem) DL		\
+	FN(map_pop_elem) DL		\
+	FN(map_peek_elem) DL		\
+	FN(msg_push_data) DL		\
+	FN(msg_pop_data) DL		\
+	FN(rc_pointer_rel) DL		\
+	FN(spin_lock) DL		\
+	FN(spin_unlock) DL		\
+	FN(sk_fullsock) DL		\
+	FN(tcp_sock) DL			\
+	FN(skb_ecn_set_ce) DL		\
+	FN(get_listener_sock) DL	\
+	FN(skc_lookup_tcp) DL		\
+	FN(tcp_check_syncookie) DL	\
+	FN(sysctl_get_name) DL		\
+	FN(sysctl_get_current_value) DL	\
+	FN(sysctl_get_new_value) DL	\
+	FN(sysctl_set_new_value) DL	\
+	FN(strtol) DL			\
+	FN(strtoul) DL			\
+	FN(sk_storage_get) DL		\
+	FN(sk_storage_delete) DL	\
+	FN(send_signal) DL		\
+	FN(tcp_gen_syncookie) DL
+
+#define ____BPF_DELIM_COMMA ,
+#define __BPF_FUNC_MAPPER(FN) \
+	____BPF_FUNC_MAPPER(FN, ____BPF_DELIM_COMMA)
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.21.0

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 21:58                 ` Daniel Borkmann
@ 2019-10-04 22:47                   ` Andrii Nakryiko
  2019-10-04 22:51                     ` Andrii Nakryiko
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-04 22:47 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Jakub Kicinski, David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team, Jiri Benc

On Fri, Oct 4, 2019 at 2:58 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On Fri, Oct 04, 2019 at 11:06:13PM +0200, Daniel Borkmann wrote:
> > On Fri, Oct 04, 2019 at 01:21:55PM -0700, Andrii Nakryiko wrote:
> > > On Fri, Oct 4, 2019 at 11:30 AM Jakub Kicinski
> > > <jakub.kicinski@netronome.com> wrote:
> > > > On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
> > > > > On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
> > > > >> > I'm not following you; my interpretation of your comment seems like you
> > > > > > are making huge assumptions.
> > > > > >
> > > > > > I build bpf programs for specific kernel versions using the devel
> > > > > > packages for the specific kernel of interest.
> > > > >
> > > > > Sure, and you can keep doing that, just don't include bpf_helpers.h?
> > > > >
> > > > > What I was saying, though, especially having in mind tracing BPF
> > > > > programs that need to inspect kernel structures, is that it's quite
> > > > > impractical to have to build many different versions of BPF programs
> > > > > for each supported kernel version and distribute them in binary form.
> > > > > So people usually use BCC and do compilation on-the-fly using BCC's
> > > > > embedded Clang.
> > > > >
> > > > > BPF CO-RE is providing an alternative, which will allow to pre-compile
> > > > > your program once for many different kernels you might be running your
> > > > > program on. There is tooling that eliminates the need for system
> > > > > headers. Instead we pre-generate a single vmlinux.h header with all
> > > > > the types/enums/etc, that are then used w/ BPF CO-RE to build portable
> > > > > BPF programs capable of working on multiple kernel versions.
> > > > >
> > > > > So what I was pointing out there was that this vmlinux.h would be
> > > > > ideally generated from latest kernel and not having latest
> > > > > BPF_FUNC_xxx shouldn't be a problem. But see below about situation
> > > > > being worse.
> > > >
> > > > Surely for distroes tho - they would have kernel headers matching the
> > > > kernel release they ship. If parts of libbpf from GH only work with
> > > > the latest kernel, distroes should ship libbpf from the kernel source,
> > > > rather than GH.
> > > >
> > > > > > > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > > > > > > for now we should still go ahead with this move and try to solve with
> > > > > > > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > > > > > > for someone, it's no worse than it is today when users don't have
> > > > > > > bpf_helpers.h at all.
> > > > > > >
> > > > > >
> > > > > > If this syncs to the github libbpf, it will be worse than today in the
> > > > > > sense of compile failures if someone's header file ordering picks
> > > > > > libbpf's bpf_helpers.h over whatever they are using today.
> > > > >
> > > > > Today bpf_helpers.h don't exist for users or am I missing something?
> > > > > bpf_helpers.h right now are purely for selftests. But they are really
> > > > > useful outside that context, so I'm making it available for everyone
> > > > > by distributing with libbpf sources. If bpf_helpers.h doesn't work for
> > > > > some specific use case, just don't use it (yet?).
> > > > >
> > > > > I'm still failing to see how it's worse than situation today.
> > > >
> > > > Having a header which works today, but may not work tomorrow is going
> > > > to be pretty bad user experience :( No matter how many warnings you put
> > > > in the source people will get caught off guard by this :(
> > > >
> > > > If you define the current state as "users can use all features of
> > > > libbpf and nothing should break on libbpf update" (which is in my
> > > > understanding a goal of the project, we bent over backwards trying
> > > > to not break things) then adding this header will in fact make things
> > > > worse. The statement in quotes would no longer be true, no?
> > >
> > > So there are few things here.
> > >
> > > 1. About "adding bpf_helpers.h will make things worse". I
> > > categorically disagree, bpf_helpers.h doesn't exist in user land at
> > > all and it's sorely missing. So adding it is strictly better
> > > experience already. Right now people have to re-declare those helper
> > > signatures and do all kinds of unnecessary hackery just to be able to
> > > use BPF stuff, and they still can run into the same problem with
> > > having too old kernel headers.
> >
> > Right, so apps tend to ship their own uapi bpf.h header and helper
> > signatures to avoid these issues. But question becomes once they
> > start using soley bpf_helper.h (also in non-tracing context which
> > is very reasonable to assume), then things might break with the patch
> > as-is once they have a newer libbpf with more signatures than their
> > linux/bpf.h defines (and yes, pulling from GH will have this problem),
> > so we'd need to have an answer to that in order to avoid breaking
> > compilation.
> >
> > [...]
> > > 2. As to the problem of running bleeding-edge libbpf against older
> > > kernel. There are few possible solutions:
> > >
> > > a. we hard-code all those BPF_FUNC_ constants. Super painful and not
> > > nice, but will work.
> > >
> > > b. copy/paste enum bpf_func_id definition into bpf_helpers.h itself
> > > and try to keep it in sync with UAPI. Apart from obvious redundancy
> > > that involves, we also will need to make sure this doesn't conflict
> > > with vmlinux.h, so enum name should be different and each value should
> > > be different (which means some wort of additional prefix or suffix).
> > >
> > > c. BPF UAPI header has __BPF_FUNC_MAPPER macro "iterating" over all
> > > defined values for a particular kernel version. We can use that and
> > > additional macro trickery to conditionally define helpers. Again, we
> > > need to keep in mind that w/ vmlinux.h there is no such macro, so this
> > > should work as well.
> > >
> > > I'm happy to hear opinions about these choices (maybe there are some
> > > other I missed), but in any case I'd like to do it in a follow up
> > > patch and land this one as is. It has already quite a lot packed in
> > > it. I personally lean towards c) as it will have a benefit of not
> > > declaring helpers that are not supported by kernel we are compiling
> > > against, even though it requires additional macro trickery.
> > >
> > > Opinions?
> >
> > Was thinking about something like c) as well. So I tried to do a quick
> > hack. Here is how it could work, but it needs a small change in the
> > __BPF_FUNC_MAPPER(), at least I didn't find an immediate way around it:

Well, we are stuck with this comma, so rather than have to support two
bpf.h headers, I'd solve the problem for existing one. It's annoying,
but you can do it with having "/* <your macro> /*" in each FN macro
and then before you apply everything you add /* and after all the
applications you add */.

I'm going to prototype something like what you have below, but will
see we I can minimize amount of extra declarations we need. Do you
think this needs to be done as part of this patch set, or I can defer
that to a follow up patch?

> >
> > static void (*__unspec)(void);
> > static void *(*__map_lookup_elem)(void *map, const void *key);
> > static int (*__map_update_elem)(void *map, const void *key, const void *value, unsigned long long flags);
> > static int (*__map_delete_elem)(void *map, const void *key);
> > static int (*__bpf_probe_read)(void *dst, int size, const void *unsafe_ptr);
> >

[...]

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 22:47                   ` Andrii Nakryiko
@ 2019-10-04 22:51                     ` Andrii Nakryiko
  2019-10-04 23:25                       ` Andrii Nakryiko
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-04 22:51 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Jakub Kicinski, David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team, Jiri Benc

On Fri, Oct 4, 2019 at 3:47 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Oct 4, 2019 at 2:58 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On Fri, Oct 04, 2019 at 11:06:13PM +0200, Daniel Borkmann wrote:
> > > On Fri, Oct 04, 2019 at 01:21:55PM -0700, Andrii Nakryiko wrote:
> > > > On Fri, Oct 4, 2019 at 11:30 AM Jakub Kicinski
> > > > <jakub.kicinski@netronome.com> wrote:
> > > > > On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
> > > > > > On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
> > > > > >> > I'm not following you; my interpretation of your comment seems like you
> > > > > > > are making huge assumptions.
> > > > > > >
> > > > > > > I build bpf programs for specific kernel versions using the devel
> > > > > > > packages for the specific kernel of interest.
> > > > > >
> > > > > > Sure, and you can keep doing that, just don't include bpf_helpers.h?
> > > > > >
> > > > > > What I was saying, though, especially having in mind tracing BPF
> > > > > > programs that need to inspect kernel structures, is that it's quite
> > > > > > impractical to have to build many different versions of BPF programs
> > > > > > for each supported kernel version and distribute them in binary form.
> > > > > > So people usually use BCC and do compilation on-the-fly using BCC's
> > > > > > embedded Clang.
> > > > > >
> > > > > > BPF CO-RE is providing an alternative, which will allow to pre-compile
> > > > > > your program once for many different kernels you might be running your
> > > > > > program on. There is tooling that eliminates the need for system
> > > > > > headers. Instead we pre-generate a single vmlinux.h header with all
> > > > > > the types/enums/etc, that are then used w/ BPF CO-RE to build portable
> > > > > > BPF programs capable of working on multiple kernel versions.
> > > > > >
> > > > > > So what I was pointing out there was that this vmlinux.h would be
> > > > > > ideally generated from latest kernel and not having latest
> > > > > > BPF_FUNC_xxx shouldn't be a problem. But see below about situation
> > > > > > being worse.
> > > > >
> > > > > Surely for distroes tho - they would have kernel headers matching the
> > > > > kernel release they ship. If parts of libbpf from GH only work with
> > > > > the latest kernel, distroes should ship libbpf from the kernel source,
> > > > > rather than GH.
> > > > >
> > > > > > > > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > > > > > > > for now we should still go ahead with this move and try to solve with
> > > > > > > > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > > > > > > > for someone, it's no worse than it is today when users don't have
> > > > > > > > bpf_helpers.h at all.
> > > > > > > >
> > > > > > >
> > > > > > > If this syncs to the github libbpf, it will be worse than today in the
> > > > > > > sense of compile failures if someone's header file ordering picks
> > > > > > > libbpf's bpf_helpers.h over whatever they are using today.
> > > > > >
> > > > > > Today bpf_helpers.h don't exist for users or am I missing something?
> > > > > > bpf_helpers.h right now are purely for selftests. But they are really
> > > > > > useful outside that context, so I'm making it available for everyone
> > > > > > by distributing with libbpf sources. If bpf_helpers.h doesn't work for
> > > > > > some specific use case, just don't use it (yet?).
> > > > > >
> > > > > > I'm still failing to see how it's worse than situation today.
> > > > >
> > > > > Having a header which works today, but may not work tomorrow is going
> > > > > to be pretty bad user experience :( No matter how many warnings you put
> > > > > in the source people will get caught off guard by this :(
> > > > >
> > > > > If you define the current state as "users can use all features of
> > > > > libbpf and nothing should break on libbpf update" (which is in my
> > > > > understanding a goal of the project, we bent over backwards trying
> > > > > to not break things) then adding this header will in fact make things
> > > > > worse. The statement in quotes would no longer be true, no?
> > > >
> > > > So there are few things here.
> > > >
> > > > 1. About "adding bpf_helpers.h will make things worse". I
> > > > categorically disagree, bpf_helpers.h doesn't exist in user land at
> > > > all and it's sorely missing. So adding it is strictly better
> > > > experience already. Right now people have to re-declare those helper
> > > > signatures and do all kinds of unnecessary hackery just to be able to
> > > > use BPF stuff, and they still can run into the same problem with
> > > > having too old kernel headers.
> > >
> > > Right, so apps tend to ship their own uapi bpf.h header and helper
> > > signatures to avoid these issues. But question becomes once they
> > > start using soley bpf_helper.h (also in non-tracing context which
> > > is very reasonable to assume), then things might break with the patch
> > > as-is once they have a newer libbpf with more signatures than their
> > > linux/bpf.h defines (and yes, pulling from GH will have this problem),
> > > so we'd need to have an answer to that in order to avoid breaking
> > > compilation.
> > >
> > > [...]
> > > > 2. As to the problem of running bleeding-edge libbpf against older
> > > > kernel. There are few possible solutions:
> > > >
> > > > a. we hard-code all those BPF_FUNC_ constants. Super painful and not
> > > > nice, but will work.
> > > >
> > > > b. copy/paste enum bpf_func_id definition into bpf_helpers.h itself
> > > > and try to keep it in sync with UAPI. Apart from obvious redundancy
> > > > that involves, we also will need to make sure this doesn't conflict
> > > > with vmlinux.h, so enum name should be different and each value should
> > > > be different (which means some wort of additional prefix or suffix).
> > > >
> > > > c. BPF UAPI header has __BPF_FUNC_MAPPER macro "iterating" over all
> > > > defined values for a particular kernel version. We can use that and
> > > > additional macro trickery to conditionally define helpers. Again, we
> > > > need to keep in mind that w/ vmlinux.h there is no such macro, so this
> > > > should work as well.
> > > >
> > > > I'm happy to hear opinions about these choices (maybe there are some
> > > > other I missed), but in any case I'd like to do it in a follow up
> > > > patch and land this one as is. It has already quite a lot packed in
> > > > it. I personally lean towards c) as it will have a benefit of not
> > > > declaring helpers that are not supported by kernel we are compiling
> > > > against, even though it requires additional macro trickery.
> > > >
> > > > Opinions?
> > >
> > > Was thinking about something like c) as well. So I tried to do a quick
> > > hack. Here is how it could work, but it needs a small change in the
> > > __BPF_FUNC_MAPPER(), at least I didn't find an immediate way around it:
>
> Well, we are stuck with this comma, so rather than have to support two
> bpf.h headers, I'd solve the problem for existing one. It's annoying,
> but you can do it with having "/* <your macro> /*" in each FN macro

err, was supposed to be "*/ (close previous comment) <your macro> /*
(open next comment".


> and then before you apply everything you add /* and after all the
> applications you add */.
>
> I'm going to prototype something like what you have below, but will
> see we I can minimize amount of extra declarations we need. Do you
> think this needs to be done as part of this patch set, or I can defer
> that to a follow up patch?
>
> > >
> > > static void (*__unspec)(void);
> > > static void *(*__map_lookup_elem)(void *map, const void *key);
> > > static int (*__map_update_elem)(void *map, const void *key, const void *value, unsigned long long flags);
> > > static int (*__map_delete_elem)(void *map, const void *key);
> > > static int (*__bpf_probe_read)(void *dst, int size, const void *unsafe_ptr);
> > >
>
> [...]

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 22:51                     ` Andrii Nakryiko
@ 2019-10-04 23:25                       ` Andrii Nakryiko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-04 23:25 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Jakub Kicinski, David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team, Jiri Benc

On Fri, Oct 4, 2019 at 3:51 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Oct 4, 2019 at 3:47 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Oct 4, 2019 at 2:58 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> > >
> > > On Fri, Oct 04, 2019 at 11:06:13PM +0200, Daniel Borkmann wrote:
> > > > On Fri, Oct 04, 2019 at 01:21:55PM -0700, Andrii Nakryiko wrote:
> > > > > On Fri, Oct 4, 2019 at 11:30 AM Jakub Kicinski
> > > > > <jakub.kicinski@netronome.com> wrote:
> > > > > > On Fri, 4 Oct 2019 09:00:42 -0700, Andrii Nakryiko wrote:
> > > > > > > On Fri, Oct 4, 2019 at 8:44 AM David Ahern <dsahern@gmail.com> wrote:
> > > > > > >> > I'm not following you; my interpretation of your comment seems like you
> > > > > > > > are making huge assumptions.
> > > > > > > >
> > > > > > > > I build bpf programs for specific kernel versions using the devel
> > > > > > > > packages for the specific kernel of interest.
> > > > > > >
> > > > > > > Sure, and you can keep doing that, just don't include bpf_helpers.h?
> > > > > > >
> > > > > > > What I was saying, though, especially having in mind tracing BPF
> > > > > > > programs that need to inspect kernel structures, is that it's quite
> > > > > > > impractical to have to build many different versions of BPF programs
> > > > > > > for each supported kernel version and distribute them in binary form.
> > > > > > > So people usually use BCC and do compilation on-the-fly using BCC's
> > > > > > > embedded Clang.
> > > > > > >
> > > > > > > BPF CO-RE is providing an alternative, which will allow to pre-compile
> > > > > > > your program once for many different kernels you might be running your
> > > > > > > program on. There is tooling that eliminates the need for system
> > > > > > > headers. Instead we pre-generate a single vmlinux.h header with all
> > > > > > > the types/enums/etc, that are then used w/ BPF CO-RE to build portable
> > > > > > > BPF programs capable of working on multiple kernel versions.
> > > > > > >
> > > > > > > So what I was pointing out there was that this vmlinux.h would be
> > > > > > > ideally generated from latest kernel and not having latest
> > > > > > > BPF_FUNC_xxx shouldn't be a problem. But see below about situation
> > > > > > > being worse.
> > > > > >
> > > > > > Surely for distroes tho - they would have kernel headers matching the
> > > > > > kernel release they ship. If parts of libbpf from GH only work with
> > > > > > the latest kernel, distroes should ship libbpf from the kernel source,
> > > > > > rather than GH.
> > > > > >
> > > > > > > > > Nevertheless, it is a problem and thanks for bringing it up! I'd say
> > > > > > > > > for now we should still go ahead with this move and try to solve with
> > > > > > > > > issue once bpf_helpers.h is in libbpf. If bpf_helpers.h doesn't work
> > > > > > > > > for someone, it's no worse than it is today when users don't have
> > > > > > > > > bpf_helpers.h at all.
> > > > > > > > >
> > > > > > > >
> > > > > > > > If this syncs to the github libbpf, it will be worse than today in the
> > > > > > > > sense of compile failures if someone's header file ordering picks
> > > > > > > > libbpf's bpf_helpers.h over whatever they are using today.
> > > > > > >
> > > > > > > Today bpf_helpers.h don't exist for users or am I missing something?
> > > > > > > bpf_helpers.h right now are purely for selftests. But they are really
> > > > > > > useful outside that context, so I'm making it available for everyone
> > > > > > > by distributing with libbpf sources. If bpf_helpers.h doesn't work for
> > > > > > > some specific use case, just don't use it (yet?).
> > > > > > >
> > > > > > > I'm still failing to see how it's worse than situation today.
> > > > > >
> > > > > > Having a header which works today, but may not work tomorrow is going
> > > > > > to be pretty bad user experience :( No matter how many warnings you put
> > > > > > in the source people will get caught off guard by this :(
> > > > > >
> > > > > > If you define the current state as "users can use all features of
> > > > > > libbpf and nothing should break on libbpf update" (which is in my
> > > > > > understanding a goal of the project, we bent over backwards trying
> > > > > > to not break things) then adding this header will in fact make things
> > > > > > worse. The statement in quotes would no longer be true, no?
> > > > >
> > > > > So there are few things here.
> > > > >
> > > > > 1. About "adding bpf_helpers.h will make things worse". I
> > > > > categorically disagree, bpf_helpers.h doesn't exist in user land at
> > > > > all and it's sorely missing. So adding it is strictly better
> > > > > experience already. Right now people have to re-declare those helper
> > > > > signatures and do all kinds of unnecessary hackery just to be able to
> > > > > use BPF stuff, and they still can run into the same problem with
> > > > > having too old kernel headers.
> > > >
> > > > Right, so apps tend to ship their own uapi bpf.h header and helper
> > > > signatures to avoid these issues. But question becomes once they
> > > > start using soley bpf_helper.h (also in non-tracing context which
> > > > is very reasonable to assume), then things might break with the patch
> > > > as-is once they have a newer libbpf with more signatures than their
> > > > linux/bpf.h defines (and yes, pulling from GH will have this problem),
> > > > so we'd need to have an answer to that in order to avoid breaking
> > > > compilation.
> > > >
> > > > [...]
> > > > > 2. As to the problem of running bleeding-edge libbpf against older
> > > > > kernel. There are few possible solutions:
> > > > >
> > > > > a. we hard-code all those BPF_FUNC_ constants. Super painful and not
> > > > > nice, but will work.
> > > > >
> > > > > b. copy/paste enum bpf_func_id definition into bpf_helpers.h itself
> > > > > and try to keep it in sync with UAPI. Apart from obvious redundancy
> > > > > that involves, we also will need to make sure this doesn't conflict
> > > > > with vmlinux.h, so enum name should be different and each value should
> > > > > be different (which means some wort of additional prefix or suffix).
> > > > >
> > > > > c. BPF UAPI header has __BPF_FUNC_MAPPER macro "iterating" over all
> > > > > defined values for a particular kernel version. We can use that and
> > > > > additional macro trickery to conditionally define helpers. Again, we
> > > > > need to keep in mind that w/ vmlinux.h there is no such macro, so this
> > > > > should work as well.
> > > > >
> > > > > I'm happy to hear opinions about these choices (maybe there are some
> > > > > other I missed), but in any case I'd like to do it in a follow up
> > > > > patch and land this one as is. It has already quite a lot packed in
> > > > > it. I personally lean towards c) as it will have a benefit of not
> > > > > declaring helpers that are not supported by kernel we are compiling
> > > > > against, even though it requires additional macro trickery.
> > > > >
> > > > > Opinions?
> > > >
> > > > Was thinking about something like c) as well. So I tried to do a quick
> > > > hack. Here is how it could work, but it needs a small change in the
> > > > __BPF_FUNC_MAPPER(), at least I didn't find an immediate way around it:
> >
> > Well, we are stuck with this comma, so rather than have to support two
> > bpf.h headers, I'd solve the problem for existing one. It's annoying,
> > but you can do it with having "/* <your macro> /*" in each FN macro
>
> err, was supposed to be "*/ (close previous comment) <your macro> /*
> (open next comment".

That won't work because macro can't emit comments... There must be
some other creative way to "screen" that comma out, but I'm too tired
to figure this out at the moment.

But more importantly, if we go this way, we'll have to work with bpf.h
headers from existing kernels, without Daniel's patch, which makes
Daniel's patch useless for this.

Another alternative that came up in offline discussions was to just
auto-generate these helper definitions from bpf.h itself as part of
libbpf build. We can also preserve documentation comments along the
way. I want to try that as well and bypass this whole macro ugliness.
If we don't manage to do this in time for next release, we can always
temporarily hard-code all the constants and not depend on bpf.h. How's
that for a plan?

>
>
> > and then before you apply everything you add /* and after all the
> > applications you add */.
> >
> > I'm going to prototype something like what you have below, but will
> > see we I can minimize amount of extra declarations we need. Do you
> > think this needs to be done as part of this patch set, or I can defer
> > that to a follow up patch?
> >
> > > >
> > > > static void (*__unspec)(void);
> > > > static void *(*__map_lookup_elem)(void *map, const void *key);
> > > > static int (*__map_update_elem)(void *map, const void *key, const void *value, unsigned long long flags);
> > > > static int (*__map_delete_elem)(void *map, const void *key);
> > > > static int (*__bpf_probe_read)(void *dst, int size, const void *unsafe_ptr);
> > > >
> >
> > [...]

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 18:30           ` Jakub Kicinski
  2019-10-04 18:37             ` Yonghong Song
  2019-10-04 20:21             ` Andrii Nakryiko
@ 2019-10-08 15:29             ` Jiri Benc
  2 siblings, 0 replies; 28+ messages in thread
From: Jiri Benc @ 2019-10-08 15:29 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrii Nakryiko, David Ahern, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Daniel Borkmann, Kernel Team

On Fri, 4 Oct 2019 11:30:26 -0700, Jakub Kicinski wrote:
> Surely for distroes tho - they would have kernel headers matching the
> kernel release they ship. If parts of libbpf from GH only work with
> the latest kernel, distroes should ship libbpf from the kernel source,
> rather than GH.

I don't see a problem here for distros. Distros control both the kernel
and libbpf, there's no problem in keeping those in sync.

Packaging libbpf from the kernel source would not help anything -
updating libbpf would still require a new kernel. There's no difference
between updating libbpf from a github repo or from the kernel source,
as far as dependencies are concerned.

 Jiri

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-04 18:37             ` Yonghong Song
  2019-10-04 21:04               ` Jakub Kicinski
@ 2019-10-08 15:37               ` Jiri Benc
  2019-10-08 18:02                 ` Andrii Nakryiko
  1 sibling, 1 reply; 28+ messages in thread
From: Jiri Benc @ 2019-10-08 15:37 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Jakub Kicinski, Andrii Nakryiko, David Ahern, Andrii Nakryiko,
	bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team

On Fri, 4 Oct 2019 18:37:44 +0000, Yonghong Song wrote:
> distro can package bpf/btf uapi headers into libbpf package.
> Users linking with libbpf.a/libbpf.so can use bpf/btf.h with include
> path pointing to libbpf dev package include directory.
> Could this work?

I don't think it would. Distros have often a policy against bundling
files that are available from one package (in this case, kernel-headers
or similar) in a different package (libbpf).

The correct way is making the libbpf package depend on a particular
version of kernel-headers (or newer). As I said, I don't see a problem
here. It's not a special situation, it's just usual dependencies.

 Jiri

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

* Re: [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf
  2019-10-08 15:37               ` Jiri Benc
@ 2019-10-08 18:02                 ` Andrii Nakryiko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2019-10-08 18:02 UTC (permalink / raw)
  To: Jiri Benc
  Cc: Yonghong Song, Jakub Kicinski, David Ahern, Andrii Nakryiko, bpf,
	Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team

On Tue, Oct 8, 2019 at 8:37 AM Jiri Benc <jbenc@redhat.com> wrote:
>
> On Fri, 4 Oct 2019 18:37:44 +0000, Yonghong Song wrote:
> > distro can package bpf/btf uapi headers into libbpf package.
> > Users linking with libbpf.a/libbpf.so can use bpf/btf.h with include
> > path pointing to libbpf dev package include directory.
> > Could this work?
>
> I don't think it would. Distros have often a policy against bundling
> files that are available from one package (in this case, kernel-headers
> or similar) in a different package (libbpf).
>
> The correct way is making the libbpf package depend on a particular
> version of kernel-headers (or newer). As I said, I don't see a problem
> here. It's not a special situation, it's just usual dependencies.
>

We ended up switching to auto-generating BPF helpers from UAPI headers
w/ hardcoding BPF_FUNC_xxx values in bpf_helpers.h. So there is now no
dependency on any specific kernel version there.

>  Jiri

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

end of thread, other threads:[~2019-10-08 18:02 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 21:28 [PATCH v3 bpf-next 0/7] Move bpf_helpers and add BPF_CORE_READ macros Andrii Nakryiko
2019-10-03 21:28 ` [PATCH v3 bpf-next 1/7] selftests/bpf: undo GCC-specific bpf_helpers.h changes Andrii Nakryiko
2019-10-04  7:00   ` Toke Høiland-Jørgensen
2019-10-03 21:28 ` [PATCH v3 bpf-next 2/7] selftests/bpf: samples/bpf: split off legacy stuff from bpf_helpers.h Andrii Nakryiko
2019-10-04  7:00   ` Toke Høiland-Jørgensen
2019-10-03 21:28 ` [PATCH v3 bpf-next 3/7] selftests/bpf: adjust CO-RE reloc tests for new bpf_core_read() macro Andrii Nakryiko
2019-10-03 21:28 ` [PATCH v3 bpf-next 4/7] selftests/bpf: split off tracing-only helpers into bpf_tracing.h Andrii Nakryiko
2019-10-04  7:01   ` Toke Høiland-Jørgensen
2019-10-03 21:28 ` [PATCH v3 bpf-next 5/7] libbpf: move bpf_{helpers,endian,tracing}.h into libbpf Andrii Nakryiko
2019-10-04  7:01   ` Toke Høiland-Jørgensen
2019-10-04 14:47   ` David Ahern
2019-10-04 15:27     ` Andrii Nakryiko
2019-10-04 15:44       ` David Ahern
2019-10-04 16:00         ` Andrii Nakryiko
2019-10-04 18:30           ` Jakub Kicinski
2019-10-04 18:37             ` Yonghong Song
2019-10-04 21:04               ` Jakub Kicinski
2019-10-08 15:37               ` Jiri Benc
2019-10-08 18:02                 ` Andrii Nakryiko
2019-10-04 20:21             ` Andrii Nakryiko
2019-10-04 21:06               ` Daniel Borkmann
2019-10-04 21:58                 ` Daniel Borkmann
2019-10-04 22:47                   ` Andrii Nakryiko
2019-10-04 22:51                     ` Andrii Nakryiko
2019-10-04 23:25                       ` Andrii Nakryiko
2019-10-08 15:29             ` Jiri Benc
2019-10-03 21:28 ` [PATCH v3 bpf-next 6/7] libbpf: add BPF_CORE_READ/BPF_CORE_READ_INTO helpers Andrii Nakryiko
2019-10-03 21:28 ` [PATCH v3 bpf-next 7/7] selftests/bpf: add BPF_CORE_READ and BPF_CORE_READ_STR_INTO macro tests Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).