* [PATCH bpf-next 0/3] btf_dump fixes for s390 @ 2021-10-12 2:32 Ilya Leoshkevich 2021-10-12 2:32 ` [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Ilya Leoshkevich @ 2021-10-12 2:32 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich Hi, This series along with [1] and [2] fixes all the failures in the btf_dump testsuite currently present on s390, in particular: * [1] fixes intermittent build bug causing "failed to encode tag ..." * error messages. * [2] fixes missing VAR entries on s390. * Patch 1 disables Intel-specific code in a testcase. * Patch 2 fixes an endianness-related bug. * Patch 3 fixes an alignment-related bug. [1] https://lore.kernel.org/bpf/20211012022521.399302-1-iii@linux.ibm.com/ [2] https://lore.kernel.org/bpf/20211012022637.399365-1-iii@linux.ibm.com/ Best regards, Ilya Ilya Leoshkevich (3): selftests/bpf: Use cpu_number only on arches that have it libbpf: Fix dumping big-endian bitfields libbpf: Fix dumping __int128 tools/lib/bpf/btf_dump.c | 12 ++++++++---- tools/testing/selftests/bpf/prog_tests/btf_dump.c | 2 ++ 2 files changed, 10 insertions(+), 4 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it 2021-10-12 2:32 [PATCH bpf-next 0/3] btf_dump fixes for s390 Ilya Leoshkevich @ 2021-10-12 2:32 ` Ilya Leoshkevich 2021-10-12 3:56 ` Andrii Nakryiko 2021-10-12 2:32 ` [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields Ilya Leoshkevich 2021-10-12 2:32 ` [PATCH bpf-next 3/3] libbpf: Fix dumping __int128 Ilya Leoshkevich 2 siblings, 1 reply; 11+ messages in thread From: Ilya Leoshkevich @ 2021-10-12 2:32 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich cpu_number exists only on Intel and aarch64, so skip the test involing it on other arches. An alternative would be to replace it with an exported non-ifdefed primitive-typed percpu variable from the common code, but there appears to be none. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/prog_tests/btf_dump.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c index 87f9df653e4e..12f457b6786d 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c @@ -778,8 +778,10 @@ static void test_btf_dump_struct_data(struct btf *btf, struct btf_dump *d, static void test_btf_dump_var_data(struct btf *btf, struct btf_dump *d, char *str) { +#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__) TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_number", int, BTF_F_COMPACT, "int cpu_number = (int)100", 100); +#endif TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_profile_flip", int, BTF_F_COMPACT, "static int cpu_profile_flip = (int)2", 2); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it 2021-10-12 2:32 ` [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich @ 2021-10-12 3:56 ` Andrii Nakryiko 2021-10-12 11:02 ` Ilya Leoshkevich 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-12 3:56 UTC (permalink / raw) To: Ilya Leoshkevich, Alan Maguire Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Tue, Oct 12, 2021 at 4:51 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > cpu_number exists only on Intel and aarch64, so skip the test involing > it on other arches. An alternative would be to replace it with an > exported non-ifdefed primitive-typed percpu variable from the common > code, but there appears to be none. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/testing/selftests/bpf/prog_tests/btf_dump.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c > index 87f9df653e4e..12f457b6786d 100644 > --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c > +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c > @@ -778,8 +778,10 @@ static void test_btf_dump_struct_data(struct btf *btf, struct btf_dump *d, > static void test_btf_dump_var_data(struct btf *btf, struct btf_dump *d, > char *str) > { > +#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__) > TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_number", int, BTF_F_COMPACT, > "int cpu_number = (int)100", 100); > +#endif We are in the talks about supporting cross-compilation of selftests, and this will be just another breakage that we'll have to undo. Can we find some other variable that will be available on all architectures? Maybe "runqueues"? > TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_profile_flip", int, BTF_F_COMPACT, > "static int cpu_profile_flip = (int)2", 2); > } > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it 2021-10-12 3:56 ` Andrii Nakryiko @ 2021-10-12 11:02 ` Ilya Leoshkevich 2021-10-20 18:32 ` Andrii Nakryiko 0 siblings, 1 reply; 11+ messages in thread From: Ilya Leoshkevich @ 2021-10-12 11:02 UTC (permalink / raw) To: Andrii Nakryiko, Alan Maguire Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Tue, 2021-10-12 at 05:56 +0200, Andrii Nakryiko wrote: > On Tue, Oct 12, 2021 at 4:51 AM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > > > cpu_number exists only on Intel and aarch64, so skip the test > > involing > > it on other arches. An alternative would be to replace it with an > > exported non-ifdefed primitive-typed percpu variable from the > > common > > code, but there appears to be none. > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > --- > > tools/testing/selftests/bpf/prog_tests/btf_dump.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > b/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > index 87f9df653e4e..12f457b6786d 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > @@ -778,8 +778,10 @@ static void test_btf_dump_struct_data(struct > > btf *btf, struct btf_dump *d, > > static void test_btf_dump_var_data(struct btf *btf, struct > > btf_dump *d, > > char *str) > > { > > +#if defined(__i386__) || defined(__x86_64__) || > > defined(__aarch64__) > > TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_number", int, > > BTF_F_COMPACT, > > "int cpu_number = (int)100", 100); > > +#endif > > We are in the talks about supporting cross-compilation of selftests, > and this will be just another breakage that we'll have to undo. Why would this break? Cross-compilation should define these macros based on target, not build system. > Can we find some other variable that will be available on all > architectures? Maybe "runqueues"? Wouldn't runqueues be pointless? We already have cpu_profile_flip. I thought the idea here was to have something marked with EXPORT_PER_CPU_SYMBOL. > > TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_profile_flip", > > int, BTF_F_COMPACT, > > "static int cpu_profile_flip = (int)2", > > 2); > > } > > -- > > 2.31.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it 2021-10-12 11:02 ` Ilya Leoshkevich @ 2021-10-20 18:32 ` Andrii Nakryiko 0 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-20 18:32 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Alan Maguire, Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Tue, Oct 12, 2021 at 4:03 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > On Tue, 2021-10-12 at 05:56 +0200, Andrii Nakryiko wrote: > > On Tue, Oct 12, 2021 at 4:51 AM Ilya Leoshkevich <iii@linux.ibm.com> > > wrote: > > > > > > cpu_number exists only on Intel and aarch64, so skip the test > > > involing > > > it on other arches. An alternative would be to replace it with an > > > exported non-ifdefed primitive-typed percpu variable from the > > > common > > > code, but there appears to be none. > > > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > > --- > > > tools/testing/selftests/bpf/prog_tests/btf_dump.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > > b/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > > index 87f9df653e4e..12f457b6786d 100644 > > > --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > > +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c > > > @@ -778,8 +778,10 @@ static void test_btf_dump_struct_data(struct > > > btf *btf, struct btf_dump *d, > > > static void test_btf_dump_var_data(struct btf *btf, struct > > > btf_dump *d, > > > char *str) > > > { > > > +#if defined(__i386__) || defined(__x86_64__) || > > > defined(__aarch64__) > > > TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_number", int, > > > BTF_F_COMPACT, > > > "int cpu_number = (int)100", 100); > > > +#endif > > > > We are in the talks about supporting cross-compilation of selftests, > > and this will be just another breakage that we'll have to undo. > > Why would this break? Cross-compilation should define these macros > based on target, not build system. Ok, then it should be good. > > > Can we find some other variable that will be available on all > > architectures? Maybe "runqueues"? > > Wouldn't runqueues be pointless? We already have cpu_profile_flip. I > thought the idea here was to have something marked with > EXPORT_PER_CPU_SYMBOL. No idea what the idea was, tbh. > > > > TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_profile_flip", > > > int, BTF_F_COMPACT, > > > "static int cpu_profile_flip = (int)2", > > > 2); > > > } > > > -- > > > 2.31.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields 2021-10-12 2:32 [PATCH bpf-next 0/3] btf_dump fixes for s390 Ilya Leoshkevich 2021-10-12 2:32 ` [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich @ 2021-10-12 2:32 ` Ilya Leoshkevich 2021-10-12 4:03 ` Andrii Nakryiko 2021-10-12 2:32 ` [PATCH bpf-next 3/3] libbpf: Fix dumping __int128 Ilya Leoshkevich 2 siblings, 1 reply; 11+ messages in thread From: Ilya Leoshkevich @ 2021-10-12 2:32 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich On big-endian arches not only bytes, but also bits are numbered in reverse order (see e.g. S/390 ELF ABI Supplement, but this is also true for other big-endian arches as well). Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/lib/bpf/btf_dump.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index ad6df97295ae..ab45771d0cb4 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -1577,14 +1577,15 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d, /* Bitfield value retrieval is done in two steps; first relevant bytes are * stored in num, then we left/right shift num to eliminate irrelevant bits. */ - nr_copy_bits = bit_sz + bits_offset; nr_copy_bytes = t->size; #if __BYTE_ORDER == __LITTLE_ENDIAN for (i = nr_copy_bytes - 1; i >= 0; i--) num = num * 256 + bytes[i]; + nr_copy_bits = bit_sz + bits_offset; #elif __BYTE_ORDER == __BIG_ENDIAN for (i = 0; i < nr_copy_bytes; i++) num = num * 256 + bytes[i]; + nr_copy_bits = nr_copy_bytes * 8 - bits_offset; #else # error "Unrecognized __BYTE_ORDER__" #endif -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields 2021-10-12 2:32 ` [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields Ilya Leoshkevich @ 2021-10-12 4:03 ` Andrii Nakryiko 2021-10-12 11:43 ` Ilya Leoshkevich 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-12 4:03 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Tue, Oct 12, 2021 at 4:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > On big-endian arches not only bytes, but also bits are numbered in > reverse order (see e.g. S/390 ELF ABI Supplement, but this is also true > for other big-endian arches as well). > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/lib/bpf/btf_dump.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c > index ad6df97295ae..ab45771d0cb4 100644 > --- a/tools/lib/bpf/btf_dump.c > +++ b/tools/lib/bpf/btf_dump.c > @@ -1577,14 +1577,15 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d, > /* Bitfield value retrieval is done in two steps; first relevant bytes are > * stored in num, then we left/right shift num to eliminate irrelevant bits. > */ > - nr_copy_bits = bit_sz + bits_offset; > nr_copy_bytes = t->size; > #if __BYTE_ORDER == __LITTLE_ENDIAN > for (i = nr_copy_bytes - 1; i >= 0; i--) > num = num * 256 + bytes[i]; > + nr_copy_bits = bit_sz + bits_offset; > #elif __BYTE_ORDER == __BIG_ENDIAN > for (i = 0; i < nr_copy_bytes; i++) > num = num * 256 + bytes[i]; > + nr_copy_bits = nr_copy_bytes * 8 - bits_offset; oh, I remember dealing with this in the context of pahole. Just one nit, please use t->size instead of nr_copy_bytes, I think it will make it a bit more explicit (nr_copy_bytes is logically mutable, though only in little-endian case, but still). > #else > # error "Unrecognized __BYTE_ORDER__" > #endif > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields 2021-10-12 4:03 ` Andrii Nakryiko @ 2021-10-12 11:43 ` Ilya Leoshkevich 0 siblings, 0 replies; 11+ messages in thread From: Ilya Leoshkevich @ 2021-10-12 11:43 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Tue, 2021-10-12 at 06:03 +0200, Andrii Nakryiko wrote: > On Tue, Oct 12, 2021 at 4:32 AM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > > > On big-endian arches not only bytes, but also bits are numbered in > > reverse order (see e.g. S/390 ELF ABI Supplement, but this is also > > true > > for other big-endian arches as well). > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > --- > > tools/lib/bpf/btf_dump.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c > > index ad6df97295ae..ab45771d0cb4 100644 > > --- a/tools/lib/bpf/btf_dump.c > > +++ b/tools/lib/bpf/btf_dump.c > > @@ -1577,14 +1577,15 @@ static int > > btf_dump_get_bitfield_value(struct btf_dump *d, > > /* Bitfield value retrieval is done in two steps; first > > relevant bytes are > > * stored in num, then we left/right shift num to eliminate > > irrelevant bits. > > */ > > - nr_copy_bits = bit_sz + bits_offset; > > nr_copy_bytes = t->size; > > #if __BYTE_ORDER == __LITTLE_ENDIAN > > for (i = nr_copy_bytes - 1; i >= 0; i--) > > num = num * 256 + bytes[i]; > > + nr_copy_bits = bit_sz + bits_offset; > > #elif __BYTE_ORDER == __BIG_ENDIAN > > for (i = 0; i < nr_copy_bytes; i++) > > num = num * 256 + bytes[i]; > > + nr_copy_bits = nr_copy_bytes * 8 - bits_offset; > > oh, I remember dealing with this in the context of pahole. Just one > nit, please use t->size instead of nr_copy_bytes, I think it will > make > it a bit more explicit (nr_copy_bytes is logically mutable, though > only in little-endian case, but still). Both sz and num_copy_bytes look redundant to be honest. What do you think about dropping both completely and just using t->size everywhere instead? > > > #else > > # error "Unrecognized __BYTE_ORDER__" > > #endif > > -- > > 2.31.1 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next 3/3] libbpf: Fix dumping __int128 2021-10-12 2:32 [PATCH bpf-next 0/3] btf_dump fixes for s390 Ilya Leoshkevich 2021-10-12 2:32 ` [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich 2021-10-12 2:32 ` [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields Ilya Leoshkevich @ 2021-10-12 2:32 ` Ilya Leoshkevich 2021-10-12 3:52 ` Andrii Nakryiko 2 siblings, 1 reply; 11+ messages in thread From: Ilya Leoshkevich @ 2021-10-12 2:32 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich On s390 __int128 can be 8-byte aligned, therefore in libbpf will occasionally consider variables of this type non-aligned and try to dump them as a bitfield, which is supported for at most 64-bit integers. Fix by using the same trick as btf_dump_float_data(): copy non-aligned values to the local buffer. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/lib/bpf/btf_dump.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index ab45771d0cb4..d8264c1762e8 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -1672,9 +1672,10 @@ static int btf_dump_int_data(struct btf_dump *d, { __u8 encoding = btf_int_encoding(t); bool sign = encoding & BTF_INT_SIGNED; + char buf[16] __aligned(16); int sz = t->size; - if (sz == 0) { + if (sz == 0 || sz > sizeof(buf)) { pr_warn("unexpected size %d for id [%u]\n", sz, type_id); return -EINVAL; } @@ -1682,8 +1683,10 @@ static int btf_dump_int_data(struct btf_dump *d, /* handle packed int data - accesses of integers not aligned on * int boundaries can cause problems on some platforms. */ - if (!ptr_is_aligned(data, sz)) - return btf_dump_bitfield_data(d, t, data, 0, 0); + if (!ptr_is_aligned(data, sz)) { + memcpy(buf, data, sz); + data = buf; + } switch (sz) { case 16: { -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 3/3] libbpf: Fix dumping __int128 2021-10-12 2:32 ` [PATCH bpf-next 3/3] libbpf: Fix dumping __int128 Ilya Leoshkevich @ 2021-10-12 3:52 ` Andrii Nakryiko 2021-10-12 11:44 ` Ilya Leoshkevich 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2021-10-12 3:52 UTC (permalink / raw) To: Ilya Leoshkevich, Alan Maguire Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Tue, Oct 12, 2021 at 4:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > On s390 __int128 can be 8-byte aligned, therefore in libbpf will > occasionally consider variables of this type non-aligned and try to > dump them as a bitfield, which is supported for at most 64-bit > integers. > > Fix by using the same trick as btf_dump_float_data(): copy non-aligned > values to the local buffer. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/lib/bpf/btf_dump.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c > index ab45771d0cb4..d8264c1762e8 100644 > --- a/tools/lib/bpf/btf_dump.c > +++ b/tools/lib/bpf/btf_dump.c > @@ -1672,9 +1672,10 @@ static int btf_dump_int_data(struct btf_dump *d, > { > __u8 encoding = btf_int_encoding(t); > bool sign = encoding & BTF_INT_SIGNED; > + char buf[16] __aligned(16); > int sz = t->size; > > - if (sz == 0) { > + if (sz == 0 || sz > sizeof(buf)) { > pr_warn("unexpected size %d for id [%u]\n", sz, type_id); > return -EINVAL; > } > @@ -1682,8 +1683,10 @@ static int btf_dump_int_data(struct btf_dump *d, > /* handle packed int data - accesses of integers not aligned on > * int boundaries can cause problems on some platforms. > */ > - if (!ptr_is_aligned(data, sz)) > - return btf_dump_bitfield_data(d, t, data, 0, 0); > + if (!ptr_is_aligned(data, sz)) { I think ptr_is_aligned() logic is wrong. We should probably not assume that __int128 has 16-byte alignment. Can you try fixing it by using btf__align_of() API to get the natural alignment? > + memcpy(buf, data, sz); > + data = buf; > + } > > switch (sz) { > case 16: { > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 3/3] libbpf: Fix dumping __int128 2021-10-12 3:52 ` Andrii Nakryiko @ 2021-10-12 11:44 ` Ilya Leoshkevich 0 siblings, 0 replies; 11+ messages in thread From: Ilya Leoshkevich @ 2021-10-12 11:44 UTC (permalink / raw) To: Andrii Nakryiko, Alan Maguire Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik On Tue, 2021-10-12 at 05:52 +0200, Andrii Nakryiko wrote: > On Tue, Oct 12, 2021 at 4:32 AM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > > > On s390 __int128 can be 8-byte aligned, therefore in libbpf will > > occasionally consider variables of this type non-aligned and try to > > dump them as a bitfield, which is supported for at most 64-bit > > integers. > > > > Fix by using the same trick as btf_dump_float_data(): copy non- > > aligned > > values to the local buffer. > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > --- > > tools/lib/bpf/btf_dump.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c > > index ab45771d0cb4..d8264c1762e8 100644 > > --- a/tools/lib/bpf/btf_dump.c > > +++ b/tools/lib/bpf/btf_dump.c > > @@ -1672,9 +1672,10 @@ static int btf_dump_int_data(struct btf_dump > > *d, > > { > > __u8 encoding = btf_int_encoding(t); > > bool sign = encoding & BTF_INT_SIGNED; > > + char buf[16] __aligned(16); > > int sz = t->size; > > > > - if (sz == 0) { > > + if (sz == 0 || sz > sizeof(buf)) { > > pr_warn("unexpected size %d for id [%u]\n", sz, > > type_id); > > return -EINVAL; > > } > > @@ -1682,8 +1683,10 @@ static int btf_dump_int_data(struct btf_dump > > *d, > > /* handle packed int data - accesses of integers not > > aligned on > > * int boundaries can cause problems on some platforms. > > */ > > - if (!ptr_is_aligned(data, sz)) > > - return btf_dump_bitfield_data(d, t, data, 0, 0); > > + if (!ptr_is_aligned(data, sz)) { > > I think ptr_is_aligned() logic is wrong. We should probably not > assume > that __int128 has 16-byte alignment. Can you try fixing it by using > btf__align_of() API to get the natural alignment? Ok, but this patch makes sense anyway, doesn't it? I can fix ptr_is_aligned() separately. > > > + memcpy(buf, data, sz); > > + data = buf; > > + } > > > > switch (sz) { > > case 16: { > > -- > > 2.31.1 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-20 18:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-12 2:32 [PATCH bpf-next 0/3] btf_dump fixes for s390 Ilya Leoshkevich 2021-10-12 2:32 ` [PATCH bpf-next 1/3] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich 2021-10-12 3:56 ` Andrii Nakryiko 2021-10-12 11:02 ` Ilya Leoshkevich 2021-10-20 18:32 ` Andrii Nakryiko 2021-10-12 2:32 ` [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields Ilya Leoshkevich 2021-10-12 4:03 ` Andrii Nakryiko 2021-10-12 11:43 ` Ilya Leoshkevich 2021-10-12 2:32 ` [PATCH bpf-next 3/3] libbpf: Fix dumping __int128 Ilya Leoshkevich 2021-10-12 3:52 ` Andrii Nakryiko 2021-10-12 11:44 ` Ilya Leoshkevich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.