* [PATCH bpf-next] libbpf: Fix strict mode calculation
@ 2022-02-04 22:04 Mauricio Vásquez
2022-02-04 22:24 ` Mauricio Vásquez Bernal
2022-02-04 22:55 ` Andrii Nakryiko
0 siblings, 2 replies; 4+ messages in thread
From: Mauricio Vásquez @ 2022-02-04 22:04 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet
The correct formula to get all possible values is
((__LIBBPF_STRICT_LAST - 1) * 2 - 1) as stated in
libbpf_set_strict_mode().
Fixes: 93b8952d223a ("libbpf: deprecate legacy BPF map definitions")
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
---
tools/bpf/bpftool/main.c | 6 +++++-
tools/testing/selftests/bpf/prog_tests/btf.c | 10 ++++++++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 9d01fa9de033..c5b27e41d1e9 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -483,8 +483,12 @@ int main(int argc, char **argv)
/* Allow legacy map definitions for skeleton generation.
* It will still be rejected if users use LIBBPF_STRICT_ALL
* mode for loading generated skeleton.
+ *
+ * __LIBBPF_STRICT_LAST is the last power-of-2 value used + 1, so to
+ * get all possible values we compensate last +1, and then (2*x - 1)
+ * to get the bit mask
*/
- mode = (__LIBBPF_STRICT_LAST - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS;
+ mode = ((__LIBBPF_STRICT_LAST - 1) * 2 - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS;
ret = libbpf_set_strict_mode(mode);
if (ret)
p_err("failed to enable libbpf strict mode: %d", ret);
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 14f9b6136794..90d5cd4f504c 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -4533,6 +4533,7 @@ static void do_test_file(unsigned int test_num)
struct btf_ext *btf_ext = NULL;
struct bpf_prog_info info = {};
struct bpf_object *obj = NULL;
+ enum libbpf_strict_mode mode;
struct bpf_func_info *finfo;
struct bpf_program *prog;
__u32 info_len, rec_size;
@@ -4560,8 +4561,13 @@ static void do_test_file(unsigned int test_num)
has_btf_ext = btf_ext != NULL;
btf_ext__free(btf_ext);
- /* temporary disable LIBBPF_STRICT_MAP_DEFINITIONS to test legacy maps */
- libbpf_set_strict_mode((__LIBBPF_STRICT_LAST - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS);
+ /* temporary disable LIBBPF_STRICT_MAP_DEFINITIONS to test legacy maps
+ * __LIBBPF_STRICT_LAST is the last power-of-2 value used + 1, so to
+ * get all possible values we compensate last +1, and then (2*x - 1)
+ * to get the bit mask
+ */
+ mode = ((__LIBBPF_STRICT_LAST - 1) * 2 - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS
+ libbpf_set_strict_mode(mode);
obj = bpf_object__open(test->file);
err = libbpf_get_error(obj);
if (CHECK(err, "obj: %d", err))
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: Fix strict mode calculation
2022-02-04 22:04 [PATCH bpf-next] libbpf: Fix strict mode calculation Mauricio Vásquez
@ 2022-02-04 22:24 ` Mauricio Vásquez Bernal
2022-02-04 22:54 ` Andrii Nakryiko
2022-02-04 22:55 ` Andrii Nakryiko
1 sibling, 1 reply; 4+ messages in thread
From: Mauricio Vásquez Bernal @ 2022-02-04 22:24 UTC (permalink / raw)
To: Networking, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet
On Fri, Feb 4, 2022 at 5:05 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> The correct formula to get all possible values is
> ((__LIBBPF_STRICT_LAST - 1) * 2 - 1) as stated in
> libbpf_set_strict_mode().
>
> Fixes: 93b8952d223a ("libbpf: deprecate legacy BPF map definitions")
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
This patch fixes the problem but I'm not totally convinced it's the
right approach. As a user I'd expected that `LIBBPF_STRICT_ALL &
~LIBBPF_STRICT_MAP_DEFINITIONS` disables
`LIBBPF_STRICT_MAP_DEFINITIONS`, but it doesn't work because the test
at libbpf_set_strict_mode() returns -EINVAL.
What about using one of the following ideas instead?
1. Remove the check from libbpf_set_strict_mode().
2. Define `LIBBPF_STRICT_ALL` containing only the bits set of the
existing options. `LIBBPF_STRICT_ALL = ((__LIBBPF_STRICT_LAST - 1) *
2)- 1`.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: Fix strict mode calculation
2022-02-04 22:24 ` Mauricio Vásquez Bernal
@ 2022-02-04 22:54 ` Andrii Nakryiko
0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 22:54 UTC (permalink / raw)
To: Mauricio Vásquez Bernal
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet
On Fri, Feb 4, 2022 at 2:24 PM Mauricio Vásquez Bernal
<mauricio@kinvolk.io> wrote:
>
> On Fri, Feb 4, 2022 at 5:05 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > The correct formula to get all possible values is
> > ((__LIBBPF_STRICT_LAST - 1) * 2 - 1) as stated in
> > libbpf_set_strict_mode().
> >
> > Fixes: 93b8952d223a ("libbpf: deprecate legacy BPF map definitions")
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
>
> This patch fixes the problem but I'm not totally convinced it's the
> right approach. As a user I'd expected that `LIBBPF_STRICT_ALL &
> ~LIBBPF_STRICT_MAP_DEFINITIONS` disables
> `LIBBPF_STRICT_MAP_DEFINITIONS`, but it doesn't work because the test
> at libbpf_set_strict_mode() returns -EINVAL.
>
> What about using one of the following ideas instead?
> 1. Remove the check from libbpf_set_strict_mode().
> 2. Define `LIBBPF_STRICT_ALL` containing only the bits set of the
> existing options. `LIBBPF_STRICT_ALL = ((__LIBBPF_STRICT_LAST - 1) *
> 2)- 1`.
can't do the 2) because the point was that applications that compiled
against older libbpf_legacy.h would still be opting into latest
LIBBPF_STRICT_ALL features. I think removing entire check in
libbpf_set_strict_mode() is ok. Let's do that and simplify selftests
and bpftool by straightforward turning off of the bit with
LIBBPF_STRICT_ALL & ~LIBBPF_STRICT_MAP_DEFINITIONS.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: Fix strict mode calculation
2022-02-04 22:04 [PATCH bpf-next] libbpf: Fix strict mode calculation Mauricio Vásquez
2022-02-04 22:24 ` Mauricio Vásquez Bernal
@ 2022-02-04 22:55 ` Andrii Nakryiko
1 sibling, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 22:55 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet
On Fri, Feb 4, 2022 at 2:05 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> The correct formula to get all possible values is
> ((__LIBBPF_STRICT_LAST - 1) * 2 - 1) as stated in
> libbpf_set_strict_mode().
>
> Fixes: 93b8952d223a ("libbpf: deprecate legacy BPF map definitions")
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> ---
> tools/bpf/bpftool/main.c | 6 +++++-
> tools/testing/selftests/bpf/prog_tests/btf.c | 10 ++++++++--
please split changes to bpftool and separately selftests/bpf (and in
v2 you'll have a separate libbpf patch as well).
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 9d01fa9de033..c5b27e41d1e9 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -483,8 +483,12 @@ int main(int argc, char **argv)
> /* Allow legacy map definitions for skeleton generation.
> * It will still be rejected if users use LIBBPF_STRICT_ALL
> * mode for loading generated skeleton.
> + *
> + * __LIBBPF_STRICT_LAST is the last power-of-2 value used + 1, so to
> + * get all possible values we compensate last +1, and then (2*x - 1)
> + * to get the bit mask
> */
> - mode = (__LIBBPF_STRICT_LAST - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS;
> + mode = ((__LIBBPF_STRICT_LAST - 1) * 2 - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS;
> ret = libbpf_set_strict_mode(mode);
> if (ret)
> p_err("failed to enable libbpf strict mode: %d", ret);
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 14f9b6136794..90d5cd4f504c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -4533,6 +4533,7 @@ static void do_test_file(unsigned int test_num)
> struct btf_ext *btf_ext = NULL;
> struct bpf_prog_info info = {};
> struct bpf_object *obj = NULL;
> + enum libbpf_strict_mode mode;
> struct bpf_func_info *finfo;
> struct bpf_program *prog;
> __u32 info_len, rec_size;
> @@ -4560,8 +4561,13 @@ static void do_test_file(unsigned int test_num)
> has_btf_ext = btf_ext != NULL;
> btf_ext__free(btf_ext);
>
> - /* temporary disable LIBBPF_STRICT_MAP_DEFINITIONS to test legacy maps */
> - libbpf_set_strict_mode((__LIBBPF_STRICT_LAST - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS);
> + /* temporary disable LIBBPF_STRICT_MAP_DEFINITIONS to test legacy maps
> + * __LIBBPF_STRICT_LAST is the last power-of-2 value used + 1, so to
> + * get all possible values we compensate last +1, and then (2*x - 1)
> + * to get the bit mask
> + */
> + mode = ((__LIBBPF_STRICT_LAST - 1) * 2 - 1) & ~LIBBPF_STRICT_MAP_DEFINITIONS
> + libbpf_set_strict_mode(mode);
> obj = bpf_object__open(test->file);
> err = libbpf_get_error(obj);
> if (CHECK(err, "obj: %d", err))
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-04 22:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-04 22:04 [PATCH bpf-next] libbpf: Fix strict mode calculation Mauricio Vásquez
2022-02-04 22:24 ` Mauricio Vásquez Bernal
2022-02-04 22:54 ` Andrii Nakryiko
2022-02-04 22:55 ` 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).