* [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps
@ 2021-09-30 16:14 Hengqi Chen
2021-09-30 16:14 ` [PATCH bpf-next 1/2] " Hengqi Chen
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Hengqi Chen @ 2021-09-30 16:14 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, yhs, john.fastabend, hengqi.chen
Currently a bunch of (usually pretty specialized) BPF maps do not support
specifying BTF types for they key and value. For such maps, specifying
their definition like this:
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
__type(key, int);
__type(value, int);
} my_perf_buf SEC(".maps");
Would actually produce warnings about retrying BPF map creation without BTF.
Users are forced to know such nuances and use __uint(key_size, 4) instead.
This is non-uniform, annoying, and inconvenient.
This patch set teaches libbpf to recognize those specialized maps and removes
BTF type IDs when creating BPF map. Also, update existing BPF selftests to
exericse this change.
Hengqi Chen (2):
libbpf: Support uniform BTF-defined key/value specification across all
BPF maps
selftests/bpf: Use BTF-defined key/value for map definitions
tools/lib/bpf/libbpf.c | 24 +++++++++++++++++++
tools/testing/selftests/bpf/progs/kfree_skb.c | 4 ++--
.../selftests/bpf/progs/perf_event_stackmap.c | 4 ++--
.../bpf/progs/sockmap_verdict_prog.c | 12 +++++-----
.../selftests/bpf/progs/test_btf_map_in_map.c | 14 +++++------
.../selftests/bpf/progs/test_map_in_map.c | 10 ++++----
.../bpf/progs/test_map_in_map_invalid.c | 2 +-
.../bpf/progs/test_pe_preserve_elems.c | 8 +++----
.../selftests/bpf/progs/test_perf_buffer.c | 4 ++--
.../bpf/progs/test_select_reuseport_kern.c | 4 ++--
.../bpf/progs/test_stacktrace_build_id.c | 4 ++--
.../selftests/bpf/progs/test_stacktrace_map.c | 4 ++--
.../selftests/bpf/progs/test_tcpnotify_kern.c | 4 ++--
.../selftests/bpf/progs/test_xdp_bpf2bpf.c | 4 ++--
14 files changed, 62 insertions(+), 40 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next 1/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps
2021-09-30 16:14 [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps Hengqi Chen
@ 2021-09-30 16:14 ` Hengqi Chen
2021-09-30 16:14 ` [PATCH bpf-next 2/2] selftests/bpf: Use BTF-defined key/value for map definitions Hengqi Chen
2021-10-01 22:35 ` [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps Andrii Nakryiko
2 siblings, 0 replies; 7+ messages in thread
From: Hengqi Chen @ 2021-09-30 16:14 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, yhs, john.fastabend, hengqi.chen
A bunch of BPF maps do not support specifying BTF types for key and value.
This is non-uniform and inconvenient[0]. Currently, libbpf uses a retry
logic which removes BTF type IDs when BPF map creation failed. Instead
of retrying, this commit recognizes those specialized maps and removes
BTF type IDs when creating BPF map.
[0] Closes: https://github.com/libbpf/libbpf/issues/355
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
tools/lib/bpf/libbpf.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ef5db34bf913..b96eb3a64cca 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4643,6 +4643,30 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
create_attr.inner_map_fd = map->inner_map_fd;
}
+ switch (def->type) {
+ case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
+ case BPF_MAP_TYPE_CGROUP_ARRAY:
+ case BPF_MAP_TYPE_STACK_TRACE:
+ case BPF_MAP_TYPE_ARRAY_OF_MAPS:
+ case BPF_MAP_TYPE_HASH_OF_MAPS:
+ case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_HASH:
+ case BPF_MAP_TYPE_CPUMAP:
+ case BPF_MAP_TYPE_XSKMAP:
+ case BPF_MAP_TYPE_SOCKMAP:
+ case BPF_MAP_TYPE_SOCKHASH:
+ case BPF_MAP_TYPE_QUEUE:
+ case BPF_MAP_TYPE_STACK:
+ case BPF_MAP_TYPE_RINGBUF:
+ create_attr.btf_fd = 0;
+ create_attr.btf_key_type_id = 0;
+ create_attr.btf_value_type_id = 0;
+ map->btf_key_type_id = 0;
+ map->btf_value_type_id = 0;
+ default:
+ break;
+ }
+
if (obj->gen_loader) {
bpf_gen__map_create(obj->gen_loader, &create_attr, is_inner ? -1 : map - obj->maps);
/* Pretend to have valid FD to pass various fd >= 0 checks.
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Use BTF-defined key/value for map definitions
2021-09-30 16:14 [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps Hengqi Chen
2021-09-30 16:14 ` [PATCH bpf-next 1/2] " Hengqi Chen
@ 2021-09-30 16:14 ` Hengqi Chen
2021-10-01 22:35 ` [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps Andrii Nakryiko
2 siblings, 0 replies; 7+ messages in thread
From: Hengqi Chen @ 2021-09-30 16:14 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, yhs, john.fastabend, hengqi.chen
Change map definitions in BPF selftests to use BTF-defined
key/value types. This unifies the map definitions and ensures
libbpf won't emit warning about retrying map creation.
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
tools/testing/selftests/bpf/progs/kfree_skb.c | 4 ++--
.../selftests/bpf/progs/perf_event_stackmap.c | 4 ++--
.../selftests/bpf/progs/sockmap_verdict_prog.c | 12 ++++++------
.../selftests/bpf/progs/test_btf_map_in_map.c | 14 +++++++-------
.../testing/selftests/bpf/progs/test_map_in_map.c | 10 ++++------
.../selftests/bpf/progs/test_map_in_map_invalid.c | 2 +-
.../selftests/bpf/progs/test_pe_preserve_elems.c | 8 ++++----
.../testing/selftests/bpf/progs/test_perf_buffer.c | 4 ++--
.../bpf/progs/test_select_reuseport_kern.c | 4 ++--
.../selftests/bpf/progs/test_stacktrace_build_id.c | 4 ++--
.../selftests/bpf/progs/test_stacktrace_map.c | 4 ++--
.../selftests/bpf/progs/test_tcpnotify_kern.c | 4 ++--
.../testing/selftests/bpf/progs/test_xdp_bpf2bpf.c | 4 ++--
13 files changed, 38 insertions(+), 40 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/kfree_skb.c b/tools/testing/selftests/bpf/progs/kfree_skb.c
index 55e283050cab..7236da72ce80 100644
--- a/tools/testing/selftests/bpf/progs/kfree_skb.c
+++ b/tools/testing/selftests/bpf/progs/kfree_skb.c
@@ -9,8 +9,8 @@
char _license[] SEC("license") = "GPL";
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
} perf_buf_map SEC(".maps");
#define _(P) (__builtin_preserve_access_index(P))
diff --git a/tools/testing/selftests/bpf/progs/perf_event_stackmap.c b/tools/testing/selftests/bpf/progs/perf_event_stackmap.c
index 25467d13c356..b3fcb5274ee0 100644
--- a/tools/testing/selftests/bpf/progs/perf_event_stackmap.c
+++ b/tools/testing/selftests/bpf/progs/perf_event_stackmap.c
@@ -11,8 +11,8 @@ typedef __u64 stack_trace_t[PERF_MAX_STACK_DEPTH];
struct {
__uint(type, BPF_MAP_TYPE_STACK_TRACE);
__uint(max_entries, 16384);
- __uint(key_size, sizeof(__u32));
- __uint(value_size, sizeof(stack_trace_t));
+ __type(key, __u32);
+ __type(value, stack_trace_t);
} stackmap SEC(".maps");
struct {
diff --git a/tools/testing/selftests/bpf/progs/sockmap_verdict_prog.c b/tools/testing/selftests/bpf/progs/sockmap_verdict_prog.c
index 4797dc985064..73872c535cbb 100644
--- a/tools/testing/selftests/bpf/progs/sockmap_verdict_prog.c
+++ b/tools/testing/selftests/bpf/progs/sockmap_verdict_prog.c
@@ -7,22 +7,22 @@ int _version SEC("version") = 1;
struct {
__uint(type, BPF_MAP_TYPE_SOCKMAP);
__uint(max_entries, 20);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
} sock_map_rx SEC(".maps");
struct {
__uint(type, BPF_MAP_TYPE_SOCKMAP);
__uint(max_entries, 20);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
} sock_map_tx SEC(".maps");
struct {
__uint(type, BPF_MAP_TYPE_SOCKMAP);
__uint(max_entries, 20);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
} sock_map_msg SEC(".maps");
struct {
diff --git a/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c b/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c
index c1e0c8c7c55f..c218cf8989a9 100644
--- a/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c
+++ b/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c
@@ -21,8 +21,8 @@ struct inner_map_sz2 {
struct outer_arr {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(max_entries, 3);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
/* it's possible to use anonymous struct as inner map definition here */
__array(values, struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
@@ -61,8 +61,8 @@ struct inner_map_sz4 {
struct outer_arr_dyn {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(max_entries, 3);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
__array(values, struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(map_flags, BPF_F_INNER_MAP);
@@ -81,7 +81,7 @@ struct outer_arr_dyn {
struct outer_hash {
__uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
__uint(max_entries, 5);
- __uint(key_size, sizeof(int));
+ __type(key, int);
/* Here everything works flawlessly due to reuse of struct inner_map
* and compiler will complain at the attempt to use non-inner_map
* references below. This is great experience.
@@ -111,8 +111,8 @@ struct sockarr_sz2 {
struct outer_sockarr_sz1 {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(max_entries, 1);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
__array(values, struct sockarr_sz1);
} outer_sockarr SEC(".maps") = {
.values = { (void *)&sockarr_sz1 },
diff --git a/tools/testing/selftests/bpf/progs/test_map_in_map.c b/tools/testing/selftests/bpf/progs/test_map_in_map.c
index 1cfeb940cf9f..68a984695184 100644
--- a/tools/testing/selftests/bpf/progs/test_map_in_map.c
+++ b/tools/testing/selftests/bpf/progs/test_map_in_map.c
@@ -9,18 +9,16 @@ struct {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(max_entries, 1);
__uint(map_flags, 0);
- __uint(key_size, sizeof(__u32));
- /* must be sizeof(__u32) for map in map */
- __uint(value_size, sizeof(__u32));
+ __type(key, __u32);
+ __type(value, __u32);
} mim_array SEC(".maps");
struct {
__uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
__uint(max_entries, 1);
__uint(map_flags, 0);
- __uint(key_size, sizeof(int));
- /* must be sizeof(__u32) for map in map */
- __uint(value_size, sizeof(__u32));
+ __type(key, int);
+ __type(value, __u32);
} mim_hash SEC(".maps");
SEC("xdp_mimtest")
diff --git a/tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c b/tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c
index 703c08e06442..9c7d75cf0bd6 100644
--- a/tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c
+++ b/tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c
@@ -13,7 +13,7 @@ struct inner {
struct {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(max_entries, 0); /* This will make map creation to fail */
- __uint(key_size, sizeof(__u32));
+ __type(key, __u32);
__array(values, struct inner);
} mim SEC(".maps");
diff --git a/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c b/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c
index fb22de7c365d..1249a945699f 100644
--- a/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c
+++ b/tools/testing/selftests/bpf/progs/test_pe_preserve_elems.c
@@ -7,15 +7,15 @@
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
__uint(max_entries, 1);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
} array_1 SEC(".maps");
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
__uint(max_entries, 1);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
__uint(map_flags, BPF_F_PRESERVE_ELEMS);
} array_2 SEC(".maps");
diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
index 8207a2dc2f9d..d37ce29fd393 100644
--- a/tools/testing/selftests/bpf/progs/test_perf_buffer.c
+++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
@@ -8,8 +8,8 @@
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
} perf_buf_map SEC(".maps");
SEC("tp/raw_syscalls/sys_enter")
diff --git a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c
index 26e77dcc7e91..0f9bc258225e 100644
--- a/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_select_reuseport_kern.c
@@ -24,8 +24,8 @@ int _version SEC("version") = 1;
struct {
__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
__uint(max_entries, 1);
- __uint(key_size, sizeof(__u32));
- __uint(value_size, sizeof(__u32));
+ __type(key, __u32);
+ __type(value, __u32);
} outer_map SEC(".maps");
struct {
diff --git a/tools/testing/selftests/bpf/progs/test_stacktrace_build_id.c b/tools/testing/selftests/bpf/progs/test_stacktrace_build_id.c
index 0cf0134631b4..7449fdb1763b 100644
--- a/tools/testing/selftests/bpf/progs/test_stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/progs/test_stacktrace_build_id.c
@@ -28,8 +28,8 @@ struct {
__uint(type, BPF_MAP_TYPE_STACK_TRACE);
__uint(max_entries, 128);
__uint(map_flags, BPF_F_STACK_BUILD_ID);
- __uint(key_size, sizeof(__u32));
- __uint(value_size, sizeof(stack_trace_t));
+ __type(key, __u32);
+ __type(value, stack_trace_t);
} stackmap SEC(".maps");
struct {
diff --git a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
index 00ed48672620..a8233e7f173b 100644
--- a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
+++ b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
@@ -27,8 +27,8 @@ typedef __u64 stack_trace_t[PERF_MAX_STACK_DEPTH];
struct {
__uint(type, BPF_MAP_TYPE_STACK_TRACE);
__uint(max_entries, 16384);
- __uint(key_size, sizeof(__u32));
- __uint(value_size, sizeof(stack_trace_t));
+ __type(key, __u32);
+ __type(value, stack_trace_t);
} stackmap SEC(".maps");
struct {
diff --git a/tools/testing/selftests/bpf/progs/test_tcpnotify_kern.c b/tools/testing/selftests/bpf/progs/test_tcpnotify_kern.c
index ac63410bb541..24e9344994ef 100644
--- a/tools/testing/selftests/bpf/progs/test_tcpnotify_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tcpnotify_kern.c
@@ -24,8 +24,8 @@ struct {
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
__uint(max_entries, 2);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(__u32));
+ __type(key, int);
+ __type(value, __u32);
} perf_event_map SEC(".maps");
int _version SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
index a038e827f850..58cf4345f5cc 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
@@ -36,8 +36,8 @@ struct meta {
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
- __uint(key_size, sizeof(int));
- __uint(value_size, sizeof(int));
+ __type(key, int);
+ __type(value, int);
} perf_buf_map SEC(".maps");
__u64 test_result_fentry = 0;
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps
2021-09-30 16:14 [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps Hengqi Chen
2021-09-30 16:14 ` [PATCH bpf-next 1/2] " Hengqi Chen
2021-09-30 16:14 ` [PATCH bpf-next 2/2] selftests/bpf: Use BTF-defined key/value for map definitions Hengqi Chen
@ 2021-10-01 22:35 ` Andrii Nakryiko
2 siblings, 0 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2021-10-01 22:35 UTC (permalink / raw)
To: Hengqi Chen
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Yonghong Song, john fastabend
On Thu, Sep 30, 2021 at 9:15 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Currently a bunch of (usually pretty specialized) BPF maps do not support
> specifying BTF types for they key and value. For such maps, specifying
> their definition like this:
>
> struct {
> __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> __type(key, int);
> __type(value, int);
> } my_perf_buf SEC(".maps");
>
> Would actually produce warnings about retrying BPF map creation without BTF.
> Users are forced to know such nuances and use __uint(key_size, 4) instead.
> This is non-uniform, annoying, and inconvenient.
>
> This patch set teaches libbpf to recognize those specialized maps and removes
> BTF type IDs when creating BPF map. Also, update existing BPF selftests to
> exericse this change.
>
> Hengqi Chen (2):
> libbpf: Support uniform BTF-defined key/value specification across all
> BPF maps
> selftests/bpf: Use BTF-defined key/value for map definitions
>
> tools/lib/bpf/libbpf.c | 24 +++++++++++++++++++
> tools/testing/selftests/bpf/progs/kfree_skb.c | 4 ++--
> .../selftests/bpf/progs/perf_event_stackmap.c | 4 ++--
> .../bpf/progs/sockmap_verdict_prog.c | 12 +++++-----
> .../selftests/bpf/progs/test_btf_map_in_map.c | 14 +++++------
> .../selftests/bpf/progs/test_map_in_map.c | 10 ++++----
> .../bpf/progs/test_map_in_map_invalid.c | 2 +-
> .../bpf/progs/test_pe_preserve_elems.c | 8 +++----
> .../selftests/bpf/progs/test_perf_buffer.c | 4 ++--
> .../bpf/progs/test_select_reuseport_kern.c | 4 ++--
> .../bpf/progs/test_stacktrace_build_id.c | 4 ++--
> .../selftests/bpf/progs/test_stacktrace_map.c | 4 ++--
> .../selftests/bpf/progs/test_tcpnotify_kern.c | 4 ++--
> .../selftests/bpf/progs/test_xdp_bpf2bpf.c | 4 ++--
> 14 files changed, 62 insertions(+), 40 deletions(-)
>
> --
> 2.30.2
Looks good. Applied to bpf-next, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps
2021-09-09 4:26 ` Andrii Nakryiko
@ 2021-09-30 15:58 ` Hengqi Chen
0 siblings, 0 replies; 7+ messages in thread
From: Hengqi Chen @ 2021-09-30 15:58 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Yonghong Song, john fastabend
On 9/9/21 12:26 PM, Andrii Nakryiko wrote:
> On Sun, Sep 5, 2021 at 3:09 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>>
>> A bunch of BPF maps do not support specifying types for key and value.
>
> s/types/BTF types/, it's a bit confusing otherwise
>
>> This is non-uniform and inconvenient[0]. Currently, libbpf uses a retry
>> logic which removes BTF type IDs when BPF map creation failed. Instead
>> of retrying, this commit recognizes those specialized map and removes
>
> s/map/maps/
>
>> BTF type IDs when creating BPF map.
>>
>> [0] Closes: https://github.com/libbpf/libbpf/issues/355
>>
>> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
>> ---
>
> For patch sets consisting of two or more patches, we ask for a cover
> letter, so for the next revision please provide a cover letter with an
> overall description of what the series is about.
>
Hello, Andrii
Sorry for the long delay. Will send a v2 for review.
>> tools/lib/bpf/libbpf.c | 35 ++++++++++++++++++++---------------
>> 1 file changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 88d8825fc6f6..7068c4d07337 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -4613,6 +4613,26 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
>> create_attr.inner_map_fd = map->inner_map_fd;
>> }
>>
>> + if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY ||
>> + def->type == BPF_MAP_TYPE_STACK_TRACE ||
>> + def->type == BPF_MAP_TYPE_CGROUP_ARRAY ||
>> + def->type == BPF_MAP_TYPE_ARRAY_OF_MAPS ||
>> + def->type == BPF_MAP_TYPE_HASH_OF_MAPS ||
>> + def->type == BPF_MAP_TYPE_DEVMAP ||
>> + def->type == BPF_MAP_TYPE_SOCKMAP ||
>> + def->type == BPF_MAP_TYPE_CPUMAP ||
>> + def->type == BPF_MAP_TYPE_XSKMAP ||
>> + def->type == BPF_MAP_TYPE_SOCKHASH ||
>> + def->type == BPF_MAP_TYPE_QUEUE ||
>> + def->type == BPF_MAP_TYPE_STACK ||
>> + def->type == BPF_MAP_TYPE_DEVMAP_HASH) {
>> + create_attr.btf_fd = 0;
>> + create_attr.btf_key_type_id = 0;
>> + create_attr.btf_value_type_id = 0;
>> + map->btf_key_type_id = 0;
>> + map->btf_value_type_id = 0;
>> + }
>
> Let's do this as a more succinct switch statement. Consider also
> slightly rearranging entries to keep "related" map types together:
> - SOCKMAP + SOCKHASH
> - DEVMAP + DEVMAP_HASH + CPUMAP + XSKMAP
>
> Thanks!
>>
>> +
>> if (obj->gen_loader) {
>> bpf_gen__map_create(obj->gen_loader, &create_attr, is_inner ? -1 : map - obj->maps);
>> /* Pretend to have valid FD to pass various fd >= 0 checks.
>> @@ -4622,21 +4642,6 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
>> } else {
>> map->fd = bpf_create_map_xattr(&create_attr);
>> }
>> - if (map->fd < 0 && (create_attr.btf_key_type_id ||
>> - create_attr.btf_value_type_id)) {
>> - char *cp, errmsg[STRERR_BUFSIZE];
>> -
>> - err = -errno;
>> - cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
>> - pr_warn("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
>> - map->name, cp, err);
>> - create_attr.btf_fd = 0;
>> - create_attr.btf_key_type_id = 0;
>> - create_attr.btf_value_type_id = 0;
>> - map->btf_key_type_id = 0;
>> - map->btf_value_type_id = 0;
>> - map->fd = bpf_create_map_xattr(&create_attr);
>> - }
>>
>
> Please don't remove this fallback logic. There are multiple situations
> where libbpf might need to retry map creation without BTF.
>
>> err = map->fd < 0 ? -errno : 0;
>>
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps
2021-09-05 10:09 [PATCH bpf-next 1/2] " Hengqi Chen
@ 2021-09-09 4:26 ` Andrii Nakryiko
2021-09-30 15:58 ` Hengqi Chen
0 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2021-09-09 4:26 UTC (permalink / raw)
To: Hengqi Chen
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Yonghong Song, john fastabend
On Sun, Sep 5, 2021 at 3:09 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> A bunch of BPF maps do not support specifying types for key and value.
s/types/BTF types/, it's a bit confusing otherwise
> This is non-uniform and inconvenient[0]. Currently, libbpf uses a retry
> logic which removes BTF type IDs when BPF map creation failed. Instead
> of retrying, this commit recognizes those specialized map and removes
s/map/maps/
> BTF type IDs when creating BPF map.
>
> [0] Closes: https://github.com/libbpf/libbpf/issues/355
>
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
For patch sets consisting of two or more patches, we ask for a cover
letter, so for the next revision please provide a cover letter with an
overall description of what the series is about.
> tools/lib/bpf/libbpf.c | 35 ++++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 88d8825fc6f6..7068c4d07337 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4613,6 +4613,26 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
> create_attr.inner_map_fd = map->inner_map_fd;
> }
>
> + if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY ||
> + def->type == BPF_MAP_TYPE_STACK_TRACE ||
> + def->type == BPF_MAP_TYPE_CGROUP_ARRAY ||
> + def->type == BPF_MAP_TYPE_ARRAY_OF_MAPS ||
> + def->type == BPF_MAP_TYPE_HASH_OF_MAPS ||
> + def->type == BPF_MAP_TYPE_DEVMAP ||
> + def->type == BPF_MAP_TYPE_SOCKMAP ||
> + def->type == BPF_MAP_TYPE_CPUMAP ||
> + def->type == BPF_MAP_TYPE_XSKMAP ||
> + def->type == BPF_MAP_TYPE_SOCKHASH ||
> + def->type == BPF_MAP_TYPE_QUEUE ||
> + def->type == BPF_MAP_TYPE_STACK ||
> + def->type == BPF_MAP_TYPE_DEVMAP_HASH) {
> + create_attr.btf_fd = 0;
> + create_attr.btf_key_type_id = 0;
> + create_attr.btf_value_type_id = 0;
> + map->btf_key_type_id = 0;
> + map->btf_value_type_id = 0;
> + }
Let's do this as a more succinct switch statement. Consider also
slightly rearranging entries to keep "related" map types together:
- SOCKMAP + SOCKHASH
- DEVMAP + DEVMAP_HASH + CPUMAP + XSKMAP
Thanks!
> +
> if (obj->gen_loader) {
> bpf_gen__map_create(obj->gen_loader, &create_attr, is_inner ? -1 : map - obj->maps);
> /* Pretend to have valid FD to pass various fd >= 0 checks.
> @@ -4622,21 +4642,6 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
> } else {
> map->fd = bpf_create_map_xattr(&create_attr);
> }
> - if (map->fd < 0 && (create_attr.btf_key_type_id ||
> - create_attr.btf_value_type_id)) {
> - char *cp, errmsg[STRERR_BUFSIZE];
> -
> - err = -errno;
> - cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
> - pr_warn("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> - map->name, cp, err);
> - create_attr.btf_fd = 0;
> - create_attr.btf_key_type_id = 0;
> - create_attr.btf_value_type_id = 0;
> - map->btf_key_type_id = 0;
> - map->btf_value_type_id = 0;
> - map->fd = bpf_create_map_xattr(&create_attr);
> - }
>
Please don't remove this fallback logic. There are multiple situations
where libbpf might need to retry map creation without BTF.
> err = map->fd < 0 ? -errno : 0;
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next 1/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps
@ 2021-09-05 10:09 Hengqi Chen
2021-09-09 4:26 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Hengqi Chen @ 2021-09-05 10:09 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, yhs, john.fastabend, hengqi.chen
A bunch of BPF maps do not support specifying types for key and value.
This is non-uniform and inconvenient[0]. Currently, libbpf uses a retry
logic which removes BTF type IDs when BPF map creation failed. Instead
of retrying, this commit recognizes those specialized map and removes
BTF type IDs when creating BPF map.
[0] Closes: https://github.com/libbpf/libbpf/issues/355
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
tools/lib/bpf/libbpf.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 88d8825fc6f6..7068c4d07337 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4613,6 +4613,26 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
create_attr.inner_map_fd = map->inner_map_fd;
}
+ if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY ||
+ def->type == BPF_MAP_TYPE_STACK_TRACE ||
+ def->type == BPF_MAP_TYPE_CGROUP_ARRAY ||
+ def->type == BPF_MAP_TYPE_ARRAY_OF_MAPS ||
+ def->type == BPF_MAP_TYPE_HASH_OF_MAPS ||
+ def->type == BPF_MAP_TYPE_DEVMAP ||
+ def->type == BPF_MAP_TYPE_SOCKMAP ||
+ def->type == BPF_MAP_TYPE_CPUMAP ||
+ def->type == BPF_MAP_TYPE_XSKMAP ||
+ def->type == BPF_MAP_TYPE_SOCKHASH ||
+ def->type == BPF_MAP_TYPE_QUEUE ||
+ def->type == BPF_MAP_TYPE_STACK ||
+ def->type == BPF_MAP_TYPE_DEVMAP_HASH) {
+ create_attr.btf_fd = 0;
+ create_attr.btf_key_type_id = 0;
+ create_attr.btf_value_type_id = 0;
+ map->btf_key_type_id = 0;
+ map->btf_value_type_id = 0;
+ }
+
if (obj->gen_loader) {
bpf_gen__map_create(obj->gen_loader, &create_attr, is_inner ? -1 : map - obj->maps);
/* Pretend to have valid FD to pass various fd >= 0 checks.
@@ -4622,21 +4642,6 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
} else {
map->fd = bpf_create_map_xattr(&create_attr);
}
- if (map->fd < 0 && (create_attr.btf_key_type_id ||
- create_attr.btf_value_type_id)) {
- char *cp, errmsg[STRERR_BUFSIZE];
-
- err = -errno;
- cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
- pr_warn("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
- map->name, cp, err);
- create_attr.btf_fd = 0;
- create_attr.btf_key_type_id = 0;
- create_attr.btf_value_type_id = 0;
- map->btf_key_type_id = 0;
- map->btf_value_type_id = 0;
- map->fd = bpf_create_map_xattr(&create_attr);
- }
err = map->fd < 0 ? -errno : 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-01 22:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-30 16:14 [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps Hengqi Chen
2021-09-30 16:14 ` [PATCH bpf-next 1/2] " Hengqi Chen
2021-09-30 16:14 ` [PATCH bpf-next 2/2] selftests/bpf: Use BTF-defined key/value for map definitions Hengqi Chen
2021-10-01 22:35 ` [PATCH bpf-next 0/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps Andrii Nakryiko
-- strict thread matches above, loose matches on Subject: below --
2021-09-05 10:09 [PATCH bpf-next 1/2] " Hengqi Chen
2021-09-09 4:26 ` Andrii Nakryiko
2021-09-30 15:58 ` Hengqi Chen
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).