libbpf: use map_flags when creating maps
Subsystems (4)
PATCH 1/1 libbpf: use map_flags when creating maps
From: Craig Gallek <[email protected]>
This extends struct bpf_map_def to include a flags field. Note that this has the potential to break the validation logic in bpf_object__validate_maps and bpf_object__init_maps as they use sizeof(struct bpf_map_def) as a minimal allowable size of a map section. Any bpf program compiled with a smaller struct bpf_map_def will fail this check.
I don't believe this will be an issue in practice as both compile-time definitions of struct bpf_map_def (in samples/bpf/bpf_load.h and tools/testing/selftests/bpf/bpf_helpers.h) have always been larger than this newly updated version in libbpf.h.
--- tools/lib/bpf/libbpf.c | 2 +- tools/lib/bpf/libbpf.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
tools/lib/bpf/libbpf.c +1 -1
@@ -874,7 +874,7 @@ bpf_object__create_maps(struct bpf_object *obj) def->key_size, def->value_size, def->max_entries, - 0); + def->map_flags);if (*pfd <0 ) {size_t j;int err = *pfd;
tools/lib/bpf/libbpf.h +1 -0
@@ -207,6 +207,7 @@ struct bpf_map_def {unsigned int key_size;unsigned int value_size;unsigned int max_entries; +unsigned int map_flags; };/*
Signature · 1 line
2.14.2.822.g60be5d43e6-goog
Quoted 38 lines · Craig Gallek
From: Craig Gallek <[email protected]>
This extends struct
bpf_map_defto include a flags field. Note that this has the potential to break the validation logic inbpf_object__validate_mapsandbpf_object__init_mapsas they usesizeof(struct bpf_map_def)as a minimal allowable size of a map section. Any bpf program compiled with a smaller structbpf_map_defwill fail this check.I don't believe this will be an issue in practice as both compile-time definitions of struct
bpf_map_def(insamples/bpf/bpf_load.handtools/testing/selftests/bpf/bpf_helpers.h) have always been larger than this newly updated version inlibbpf.h.Signed-off-by Craig Gallek <[email protected]>--- tools/lib/bpf/libbpf.c | 2 +- tools/lib/bpf/libbpf.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)tools/lib/bpf/libbpf.c +1
-1@@ -874,7 +874,7 @@ bpf_object__create_maps(struct bpf_object *obj) def->key_size, def->value_size, def->max_entries, - 0); + def->map_flags);if (*pfd <0 ) {size_t j;int err = *pfd;tools/lib/bpf/libbpf.h +0
-0
tools/lib/bpf/libbpf.h struct bpf_map_def { · line 211
Quoted 5 lines
tools/lib/bpf/libbpf.h +1
-0@@ -207,6 +207,7 @@ struct bpf_map_def {unsigned int key_size;unsigned int value_size;unsigned int max_entries; +unsigned int map_flags; };
yes it will break loading of pre-compiled .o
Instead of breaking, let's fix the loader to do it the waysamples/bpf/bpf_load.c does.
See commit 156450d9d964 ("samples/bpf: make bpf_load.c code compatible with ELF maps section changes")
Daniel Borkmann
maintainerQuoted 41 lines · Alexei Starovoitov
Quoted 40 lines · Craig Gallek
From: Craig Gallek <[email protected]>
This extends struct
bpf_map_defto include a flags field. Note that this has the potential to break the validation logic inbpf_object__validate_mapsandbpf_object__init_mapsas they usesizeof(struct bpf_map_def)as a minimal allowable size of a map section. Any bpf program compiled with a smaller structbpf_map_defwill fail this check.I don't believe this will be an issue in practice as both compile-time definitions of struct
bpf_map_def(insamples/bpf/bpf_load.handtools/testing/selftests/bpf/bpf_helpers.h) have always been larger than this newly updated version inlibbpf.h.Signed-off-by Craig Gallek <[email protected]>--- tools/lib/bpf/libbpf.c | 2 +- tools/lib/bpf/libbpf.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)tools/lib/bpf/libbpf.c +1
-1@@ -874,7 +874,7 @@ bpf_object__create_maps(struct bpf_object *obj) def->key_size, def->value_size, def->max_entries, - 0); + def->map_flags);if (*pfd <0 ) {size_t j;int err = *pfd;tools/lib/bpf/libbpf.h +0
-0@@ -207,6 +207,7 @@ struct bpf_map_def {unsigned int key_size;unsigned int value_size;
Quoted 8 lines
tools/lib/bpf/libbpf.h struct bpf_map_def { · line 211
Quoted 3 lines
tools/lib/bpf/libbpf.h +1
-0unsigned int max_entries; +unsigned int map_flags; };yes it will break loading of pre-compiled .o
Instead of breaking, let's fix the loader to do it the waysamples/bpf/bpf_load.cdoes.
See commit156450d9d964("samples/bpf: makebpf_load.ccode compatible with ELF maps section changes")
+1, iproute2 loader also does map spec fixup
For libbpf it would be good also such that it reduces the diff further between the libbpf and bpf_load so that it allows move to libbpf for samples in future.
Quoted 56 lines · Daniel Borkmann
Quoted 45 lines · Alexei Starovoitov
Quoted 43 lines · Craig Gallek
From: Craig Gallek <[email protected]>
This extends struct
bpf_map_defto include a flags field. Note that this has the potential to break the validation logic inbpf_object__validate_mapsandbpf_object__init_mapsas they usesizeof(struct bpf_map_def)as a minimal allowable size of a map section. Any bpf program compiled with a smaller structbpf_map_defwill fail this check.I don't believe this will be an issue in practice as both compile-time definitions of struct
bpf_map_def(insamples/bpf/bpf_load.handtools/testing/selftests/bpf/bpf_helpers.h) have always been larger than this newly updated version inlibbpf.h.Signed-off-by Craig Gallek <[email protected]>--- tools/lib/bpf/libbpf.c | 2 +- tools/lib/bpf/libbpf.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)tools/lib/bpf/libbpf.c +1
-1@@ -874,7 +874,7 @@ bpf_object__create_maps(struct bpf_object *obj) def->key_size, def->value_size, def->max_entries, - 0); + def->map_flags);if (*pfd <0 ) {size_t j;int err = *pfd;tools/lib/bpf/libbpf.h +1
-0@@ -207,6 +207,7 @@ struct bpf_map_def {unsigned int key_size;unsigned int value_size;unsigned int max_entries; +unsigned int map_flags;Quoted 8 lines
tools/lib/bpf/libbpf.h struct bpf_map_def { · line 211
Quoted 1 line
tools/lib/bpf/libbpf.h +0
-0};yes it will break loading of pre-compiled .o
Instead of breaking, let's fix the loader to do it the waysamples/bpf/bpf_load.cdoes.
See commit156450d9d964("samples/bpf: makebpf_load.ccode compatible with ELF maps section changes")
Quoted 5 lines
+1, iproute2 loader also does map spec fixup
For libbpf it would be good also such that it reduces the diff further between the libbpf and
bpf_loadso that it allows move to libbpf for samples in future.
Fair enough, I'll try to get this to work more dynamically. I did noticed that the fields of struct bpf_map_def in selftests/.../bpf_helpers.h and iproute2's struct bpf_elf_map have diverged. The flags field is the only thing missing from libbpf right now (and they are at the same offset for both), so it won't be an issue for this change, but it is going to make unifying all of these things under libbpf not trivial at some point...
Daniel Borkmann
maintainerQuoted 4 lines · Craig Gallek
Quoted 3 lines · Daniel Borkmann
Quoted 2 lines · Alexei Starovoitov
Quoted 1 line · Craig Gallek
From: Craig Gallek <[email protected]>
[...]
Quoted 13 lines
Quoted 7 lines
Quoted 6 lines
yes it will break loading of pre-compiled .o
Instead of breaking, let's fix the loader to do it the waysamples/bpf/bpf_load.cdoes.
See commit156450d9d964("samples/bpf: makebpf_load.ccode compatible with ELF maps section changes")Quoted 5 lines
+1, iproute2 loader also does map spec fixup
For libbpf it would be good also such that it reduces the diff further between the libbpf and
bpf_loadso that it allows move to libbpf for samples in future.
Quoted 7 lines
Fair enough, I'll try to get this to work more dynamically. I did noticed that the fields of struct
bpf_map_definselftests/.../bpf_helpers.hand iproute2's structbpf_elf_maphave diverged. The flags field is the only thing missing from libbpf right now (and they are at the same offset for both), so it won't be an issue for this change, but it is going to make unifying all of these things under libbpf not trivial at some point...
Yes, iproute2 uses its own loader with own specifics related to iproute2. With the above I rather meant that we can reduce the gap between libbpf and bpf_load from the BPF samples, so that it would allow us to migrate samples over entirely, there was unfortunately never a follow-up on 156450d9d964 though, but given there is need to extend libbpf (I guess mostly due to lpm map handling ;)), we need to do a similar thing there as well to not break existing obj files.