Re: [PATCH bpf-next 1/3] libbpf: avoid use of __int128 in typed dump display

From: Andrii Nakryiko
Date: Mon Jul 19 2021 - 20:30:47 EST


On Mon, Jul 19, 2021 at 2:41 PM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
>
> __int128 is not supported for some 32-bit platforms (arm and i386).
> __int128 was used in carrying out computations on bitfields which
> aid display, but the same calculations could be done with __u64
> with the small effect of not supporting 128-bit bitfields.
>
> With these changes, a big-endian issue with casting 128-bit integers
> to 64-bit for enum bitfields is solved also, as we now use 64-bit
> integers for bitfield calculations.
>
> Reported-by: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>
> Reported-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> ---

Changes look good to me, thanks. But they didn't appear in patchworks
yet so I can't easily test and apply them. It might be because of
patchworks delay or due to a very long CC list. Try trimming the cc
list down and re-submit?

Also, while I agree that supporting 128-bit bitfields isn't important,
I wonder if we should warn/error on that (instead of shifting by
negative amount and reporting some garbage value), what do you think?
Is there one place in the code where we can error out early if the
type actually has bitfield with > 64 bits? I'd prefer to keep
btf_dump_bitfield_get_data() itself non-failing though.


> tools/lib/bpf/btf_dump.c | 62 +++++++++++++++++++++++++++---------------------
> 1 file changed, 35 insertions(+), 27 deletions(-)
>

[...]