* [PATCH bpf-next v2 0/4] btf_dump fixes for s390
@ 2021-10-13 16:08 Ilya Leoshkevich
2021-10-13 16:08 ` [PATCH bpf-next v2 1/4] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2021-10-13 16:08 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich
v1: https://lore.kernel.org/bpf/20211012023218.399568-1-iii@linux.ibm.com/
v1 -> v2:
- Remove redundant local variables, use t->size directly instead.
- Add btf__align_of() patch.
Pending questions:
- Can things like defined(__i386__) break cross-compilation?
- Why exactly do we need both cpu_number and cpu_profile_flip? If we do,
is there a suitable replacement for cpu_number in common code?
---
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.
* Patch 4 improves overly pessimistic alignment handling.
[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 (4):
selftests/bpf: Use cpu_number only on arches that have it
libbpf: Fix dumping big-endian bitfields
libbpf: Fix dumping non-aligned __int128
libbpf: Fix ptr_is_aligned() usages
tools/lib/bpf/btf_dump.c | 34 +++++++++----------
.../selftests/bpf/prog_tests/btf_dump.c | 2 ++
2 files changed, 19 insertions(+), 17 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 1/4] selftests/bpf: Use cpu_number only on arches that have it
2021-10-13 16:08 [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Ilya Leoshkevich
@ 2021-10-13 16:08 ` Ilya Leoshkevich
2021-10-13 16:09 ` [PATCH bpf-next v2 2/4] libbpf: Fix dumping big-endian bitfields Ilya Leoshkevich
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2021-10-13 16:08 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] 10+ messages in thread
* [PATCH bpf-next v2 2/4] libbpf: Fix dumping big-endian bitfields
2021-10-13 16:08 [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Ilya Leoshkevich
2021-10-13 16:08 ` [PATCH bpf-next v2 1/4] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich
@ 2021-10-13 16:09 ` Ilya Leoshkevich
2021-10-13 16:09 ` [PATCH bpf-next v2 3/4] libbpf: Fix dumping non-aligned __int128 Ilya Leoshkevich
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2021-10-13 16:09 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 | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index ad6df97295ae..614719c73bd7 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1562,29 +1562,28 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
__u64 *value)
{
__u16 left_shift_bits, right_shift_bits;
- __u8 nr_copy_bits, nr_copy_bytes;
const __u8 *bytes = data;
- int sz = t->size;
+ __u8 nr_copy_bits;
__u64 num = 0;
int i;
/* Maximum supported bitfield size is 64 bits */
- if (sz > 8) {
- pr_warn("unexpected bitfield size %d\n", sz);
+ if (t->size > 8) {
+ pr_warn("unexpected bitfield size %d\n", t->size);
return -EINVAL;
}
/* 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--)
+ for (i = t->size - 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++)
+ for (i = 0; i < t->size; i++)
num = num * 256 + bytes[i];
+ nr_copy_bits = t->size * 8 - bits_offset;
#else
# error "Unrecognized __BYTE_ORDER__"
#endif
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 3/4] libbpf: Fix dumping non-aligned __int128
2021-10-13 16:08 [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Ilya Leoshkevich
2021-10-13 16:08 ` [PATCH bpf-next v2 1/4] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich
2021-10-13 16:09 ` [PATCH bpf-next v2 2/4] libbpf: Fix dumping big-endian bitfields Ilya Leoshkevich
@ 2021-10-13 16:09 ` Ilya Leoshkevich
2021-10-13 16:09 ` [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages Ilya Leoshkevich
2021-10-20 18:48 ` [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Andrii Nakryiko
4 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2021-10-13 16:09 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich
Non-aligned integers are dumped as bields, 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 614719c73bd7..25ce60828e8d 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1670,9 +1670,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;
}
@@ -1680,8 +1681,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] 10+ messages in thread
* [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages
2021-10-13 16:08 [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Ilya Leoshkevich
` (2 preceding siblings ...)
2021-10-13 16:09 ` [PATCH bpf-next v2 3/4] libbpf: Fix dumping non-aligned __int128 Ilya Leoshkevich
@ 2021-10-13 16:09 ` Ilya Leoshkevich
2021-10-20 18:44 ` Andrii Nakryiko
2021-10-20 18:48 ` [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Andrii Nakryiko
4 siblings, 1 reply; 10+ messages in thread
From: Ilya Leoshkevich @ 2021-10-13 16:09 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich
Currently ptr_is_aligned() takes size, and not alignment, as a
parameter, which may be overly pessimistic e.g. for __i128 on s390,
which must be only 8-byte aligned. Fix by using btf__align_of() where
possible - one notable exception is ptr_sz, for which there is no
corresponding type.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/btf_dump.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 25ce60828e8d..da345520892f 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1657,9 +1657,9 @@ static int btf_dump_base_type_check_zero(struct btf_dump *d,
return 0;
}
-static bool ptr_is_aligned(const void *data, int data_sz)
+static bool ptr_is_aligned(const void *data, int alignment)
{
- return ((uintptr_t)data) % data_sz == 0;
+ return ((uintptr_t)data) % alignment == 0;
}
static int btf_dump_int_data(struct btf_dump *d,
@@ -1681,7 +1681,7 @@ 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)) {
+ if (!ptr_is_aligned(data, btf__align_of(d->btf, type_id))) {
memcpy(buf, data, sz);
data = buf;
}
@@ -1770,7 +1770,7 @@ static int btf_dump_float_data(struct btf_dump *d,
int sz = t->size;
/* handle unaligned data; copy to local union */
- if (!ptr_is_aligned(data, sz)) {
+ if (!ptr_is_aligned(data, btf__align_of(d->btf, type_id))) {
memcpy(&fl, data, sz);
flp = &fl;
}
@@ -1953,10 +1953,8 @@ static int btf_dump_get_enum_value(struct btf_dump *d,
__u32 id,
__s64 *value)
{
- int sz = t->size;
-
/* handle unaligned enum value */
- if (!ptr_is_aligned(data, sz)) {
+ if (!ptr_is_aligned(data, btf__align_of(d->btf, id))) {
__u64 val;
int err;
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages
2021-10-13 16:09 ` [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages Ilya Leoshkevich
@ 2021-10-20 18:44 ` Andrii Nakryiko
2021-10-20 23:02 ` Ilya Leoshkevich
0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2021-10-20 18:44 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik
On Wed, Oct 13, 2021 at 9:09 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Currently ptr_is_aligned() takes size, and not alignment, as a
> parameter, which may be overly pessimistic e.g. for __i128 on s390,
> which must be only 8-byte aligned. Fix by using btf__align_of() where
> possible - one notable exception is ptr_sz, for which there is no
> corresponding type.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/lib/bpf/btf_dump.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index 25ce60828e8d..da345520892f 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c
> @@ -1657,9 +1657,9 @@ static int btf_dump_base_type_check_zero(struct btf_dump *d,
> return 0;
> }
>
> -static bool ptr_is_aligned(const void *data, int data_sz)
> +static bool ptr_is_aligned(const void *data, int alignment)
> {
> - return ((uintptr_t)data) % data_sz == 0;
> + return ((uintptr_t)data) % alignment == 0;
btf__align_of() can return 0 on error and this will be div by 0. I
think the better approach would be for ptr_is_aligned to accept struct
btf *btf and __u32 type_id, call btf__align_of() based on btf and type
id, handle 0 case pessimistically (assume not aligned).
> }
>
> static int btf_dump_int_data(struct btf_dump *d,
> @@ -1681,7 +1681,7 @@ 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)) {
> + if (!ptr_is_aligned(data, btf__align_of(d->btf, type_id))) {
> memcpy(buf, data, sz);
> data = buf;
> }
> @@ -1770,7 +1770,7 @@ static int btf_dump_float_data(struct btf_dump *d,
> int sz = t->size;
>
> /* handle unaligned data; copy to local union */
> - if (!ptr_is_aligned(data, sz)) {
> + if (!ptr_is_aligned(data, btf__align_of(d->btf, type_id))) {
> memcpy(&fl, data, sz);
> flp = &fl;
> }
> @@ -1953,10 +1953,8 @@ static int btf_dump_get_enum_value(struct btf_dump *d,
> __u32 id,
> __s64 *value)
> {
> - int sz = t->size;
> -
> /* handle unaligned enum value */
> - if (!ptr_is_aligned(data, sz)) {
> + if (!ptr_is_aligned(data, btf__align_of(d->btf, id))) {
> __u64 val;
> int err;
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 0/4] btf_dump fixes for s390
2021-10-13 16:08 [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Ilya Leoshkevich
` (3 preceding siblings ...)
2021-10-13 16:09 ` [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages Ilya Leoshkevich
@ 2021-10-20 18:48 ` Andrii Nakryiko
4 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2021-10-20 18:48 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik
On Wed, Oct 13, 2021 at 9:09 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> v1: https://lore.kernel.org/bpf/20211012023218.399568-1-iii@linux.ibm.com/
> v1 -> v2:
> - Remove redundant local variables, use t->size directly instead.
> - Add btf__align_of() patch.
> Pending questions:
> - Can things like defined(__i386__) break cross-compilation?
> - Why exactly do we need both cpu_number and cpu_profile_flip? If we do,
> is there a suitable replacement for cpu_number in common code?
>
> ---
>
> 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.
> * Patch 4 improves overly pessimistic alignment handling.
>
> [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 (4):
> selftests/bpf: Use cpu_number only on arches that have it
> libbpf: Fix dumping big-endian bitfields
> libbpf: Fix dumping non-aligned __int128
> libbpf: Fix ptr_is_aligned() usages
>
> tools/lib/bpf/btf_dump.c | 34 +++++++++----------
> .../selftests/bpf/prog_tests/btf_dump.c | 2 ++
> 2 files changed, 19 insertions(+), 17 deletions(-)
>
> --
> 2.31.1
>
I've dropped patch 4 for now, applied the first three to bpf-next, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages
2021-10-20 18:44 ` Andrii Nakryiko
@ 2021-10-20 23:02 ` Ilya Leoshkevich
2021-10-20 23:10 ` Andrii Nakryiko
0 siblings, 1 reply; 10+ messages in thread
From: Ilya Leoshkevich @ 2021-10-20 23:02 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik
On Wed, 2021-10-20 at 11:44 -0700, Andrii Nakryiko wrote:
> On Wed, Oct 13, 2021 at 9:09 AM Ilya Leoshkevich <iii@linux.ibm.com>
> wrote:
> >
> > Currently ptr_is_aligned() takes size, and not alignment, as a
> > parameter, which may be overly pessimistic e.g. for __i128 on s390,
> > which must be only 8-byte aligned. Fix by using btf__align_of()
> > where
> > possible - one notable exception is ptr_sz, for which there is no
> > corresponding type.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > tools/lib/bpf/btf_dump.c | 12 +++++-------
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> > index 25ce60828e8d..da345520892f 100644
> > --- a/tools/lib/bpf/btf_dump.c
> > +++ b/tools/lib/bpf/btf_dump.c
> > @@ -1657,9 +1657,9 @@ static int
> > btf_dump_base_type_check_zero(struct btf_dump *d,
> > return 0;
> > }
> >
> > -static bool ptr_is_aligned(const void *data, int data_sz)
> > +static bool ptr_is_aligned(const void *data, int alignment)
> > {
> > - return ((uintptr_t)data) % data_sz == 0;
> > + return ((uintptr_t)data) % alignment == 0;
>
> btf__align_of() can return 0 on error and this will be div by 0. I
> think the better approach would be for ptr_is_aligned to accept
> struct
> btf *btf and __u32 type_id, call btf__align_of() based on btf and
> type
> id, handle 0 case pessimistically (assume not aligned).
I thought about this, but it won't cover the ptr_sz case. Maybe we
just need two functions - I'll give it a try tomorrow.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages
2021-10-20 23:02 ` Ilya Leoshkevich
@ 2021-10-20 23:10 ` Andrii Nakryiko
2021-10-21 10:29 ` Ilya Leoshkevich
0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2021-10-20 23:10 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik
On Wed, Oct 20, 2021 at 4:05 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Wed, 2021-10-20 at 11:44 -0700, Andrii Nakryiko wrote:
> > On Wed, Oct 13, 2021 at 9:09 AM Ilya Leoshkevich <iii@linux.ibm.com>
> > wrote:
> > >
> > > Currently ptr_is_aligned() takes size, and not alignment, as a
> > > parameter, which may be overly pessimistic e.g. for __i128 on s390,
> > > which must be only 8-byte aligned. Fix by using btf__align_of()
> > > where
> > > possible - one notable exception is ptr_sz, for which there is no
> > > corresponding type.
> > >
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > > tools/lib/bpf/btf_dump.c | 12 +++++-------
> > > 1 file changed, 5 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> > > index 25ce60828e8d..da345520892f 100644
> > > --- a/tools/lib/bpf/btf_dump.c
> > > +++ b/tools/lib/bpf/btf_dump.c
> > > @@ -1657,9 +1657,9 @@ static int
> > > btf_dump_base_type_check_zero(struct btf_dump *d,
> > > return 0;
> > > }
> > >
> > > -static bool ptr_is_aligned(const void *data, int data_sz)
> > > +static bool ptr_is_aligned(const void *data, int alignment)
> > > {
> > > - return ((uintptr_t)data) % data_sz == 0;
> > > + return ((uintptr_t)data) % alignment == 0;
> >
> > btf__align_of() can return 0 on error and this will be div by 0. I
> > think the better approach would be for ptr_is_aligned to accept
> > struct
> > btf *btf and __u32 type_id, call btf__align_of() based on btf and
> > type
> > id, handle 0 case pessimistically (assume not aligned).
>
> I thought about this, but it won't cover the ptr_sz case. Maybe we
> just need two functions - I'll give it a try tomorrow.
>
Sorry, what's the ptr_sz case? Is this about btf_ptr_sz() helper somehow?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages
2021-10-20 23:10 ` Andrii Nakryiko
@ 2021-10-21 10:29 ` Ilya Leoshkevich
0 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2021-10-21 10:29 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik
On Wed, 2021-10-20 at 16:10 -0700, Andrii Nakryiko wrote:
> On Wed, Oct 20, 2021 at 4:05 PM Ilya Leoshkevich <iii@linux.ibm.com>
> wrote:
> >
> > On Wed, 2021-10-20 at 11:44 -0700, Andrii Nakryiko wrote:
> > > On Wed, Oct 13, 2021 at 9:09 AM Ilya Leoshkevich
> > > <iii@linux.ibm.com>
> > > wrote:
> > > >
> > > > Currently ptr_is_aligned() takes size, and not alignment, as a
> > > > parameter, which may be overly pessimistic e.g. for __i128 on
> > > > s390,
> > > > which must be only 8-byte aligned. Fix by using btf__align_of()
> > > > where
> > > > possible - one notable exception is ptr_sz, for which there is no
> > > > corresponding type.
> > > >
> > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > ---
> > > > tools/lib/bpf/btf_dump.c | 12 +++++-------
> > > > 1 file changed, 5 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> > > > index 25ce60828e8d..da345520892f 100644
> > > > --- a/tools/lib/bpf/btf_dump.c
> > > > +++ b/tools/lib/bpf/btf_dump.c
> > > > @@ -1657,9 +1657,9 @@ static int
> > > > btf_dump_base_type_check_zero(struct btf_dump *d,
> > > > return 0;
> > > > }
> > > >
> > > > -static bool ptr_is_aligned(const void *data, int data_sz)
> > > > +static bool ptr_is_aligned(const void *data, int alignment)
> > > > {
> > > > - return ((uintptr_t)data) % data_sz == 0;
> > > > + return ((uintptr_t)data) % alignment == 0;
> > >
> > > btf__align_of() can return 0 on error and this will be div by 0. I
> > > think the better approach would be for ptr_is_aligned to accept
> > > struct
> > > btf *btf and __u32 type_id, call btf__align_of() based on btf and
> > > type
> > > id, handle 0 case pessimistically (assume not aligned).
> >
> > I thought about this, but it won't cover the ptr_sz case. Maybe we
> > just need two functions - I'll give it a try tomorrow.
> >
>
> Sorry, what's the ptr_sz case? Is this about btf_ptr_sz() helper
> somehow?
Almost, I'm referring to this check:
static int btf_dump_ptr_data(struct btf_dump *d,
const struct btf_type *t,
__u32 id,
const void *data)
{
if (ptr_is_aligned(data, d->ptr_sz) && d->ptr_sz ==
sizeof(void *)) {
...
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-10-21 10:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 16:08 [PATCH bpf-next v2 0/4] btf_dump fixes for s390 Ilya Leoshkevich
2021-10-13 16:08 ` [PATCH bpf-next v2 1/4] selftests/bpf: Use cpu_number only on arches that have it Ilya Leoshkevich
2021-10-13 16:09 ` [PATCH bpf-next v2 2/4] libbpf: Fix dumping big-endian bitfields Ilya Leoshkevich
2021-10-13 16:09 ` [PATCH bpf-next v2 3/4] libbpf: Fix dumping non-aligned __int128 Ilya Leoshkevich
2021-10-13 16:09 ` [PATCH bpf-next v2 4/4] libbpf: Fix ptr_is_aligned() usages Ilya Leoshkevich
2021-10-20 18:44 ` Andrii Nakryiko
2021-10-20 23:02 ` Ilya Leoshkevich
2021-10-20 23:10 ` Andrii Nakryiko
2021-10-21 10:29 ` Ilya Leoshkevich
2021-10-20 18:48 ` [PATCH bpf-next v2 0/4] btf_dump fixes for s390 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).