Messages in this thread Patch in this message | | | Subject | Re: [PATCH] btf: support ints larger than 128 bits | From | Yonghong Song <> | Date | Thu, 17 Dec 2020 18:12:11 -0800 |
| |
On 12/17/20 7:01 AM, Sean Young wrote: > clang supports arbitrary length ints using the _ExtInt extension. This > can be useful to hold very large values, e.g. 256 bit or 512 bit types. > > Larger types (e.g. 1024 bits) are possible but I am unaware of a use > case for these. > > This requires the _ExtInt extension to enabled for BPF in clang, which > is under review. > > Link: https://clang.llvm.org/docs/LanguageExtensions.html#extended-integer-types > Link: https://reviews.llvm.org/D93103 > > Signed-off-by: Sean Young <sean@mess.org> > --- > Documentation/bpf/btf.rst | 4 ++-- > include/uapi/linux/btf.h | 2 +- > tools/bpf/bpftool/btf_dumper.c | 39 ++++++++++++++++++++++++++++++++++ > tools/include/uapi/linux/btf.h | 2 +- > 4 files changed, 43 insertions(+), 4 deletions(-)
Thanks for the patch. But the change is not enough and no tests in the patch set.
For example, in kernel/bpf/btf.c, we BITS_PER_U128 to guard in various places where the number of integer bits must be <= 128 bits which is what we supported now. In function btf_type_int_is_regular(), # of int bits larger than 128 considered false. The extint like 256/512bits should be also regular int.
extint permits non-power-of-2 bits (e.g., 192bits), to support them may not be necessary and this is not your use case. what do you think?
lib/bpf/btf.c btf__and_int() function also has the following check,
/* byte_sz must be power of 2 */ if (!byte_sz || (byte_sz & (byte_sz - 1)) || byte_sz > 16) return -EINVAL;
So Extint 256 bits will fail here.
Please do add some selftests tools/testing/selftests/bpf directories: - to ensure btf with newly supported int types loaded successfully in kernel - to ensure bpftool map [pretty] print working fine with new types - to ensure kernel map pretty print works fine (tests at tools/testing/selftests/bpf/prog_tests/btf.c) - to ensure btf manipulation APIs works with new types.
> > diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst > index 44dc789de2b4..784f1743dbc7 100644 > --- a/Documentation/bpf/btf.rst > +++ b/Documentation/bpf/btf.rst > @@ -132,7 +132,7 @@ The following sections detail encoding of each kind. > > #define BTF_INT_ENCODING(VAL) (((VAL) & 0x0f000000) >> 24) > #define BTF_INT_OFFSET(VAL) (((VAL) & 0x00ff0000) >> 16) > - #define BTF_INT_BITS(VAL) ((VAL) & 0x000000ff) > + #define BTF_INT_BITS(VAL) ((VAL) & 0x000003ff) > > The ``BTF_INT_ENCODING`` has the following attributes:: > > @@ -147,7 +147,7 @@ pretty print. At most one encoding can be specified for the int type. > The ``BTF_INT_BITS()`` specifies the number of actual bits held by this int > type. For example, a 4-bit bitfield encodes ``BTF_INT_BITS()`` equals to 4. > The ``btf_type.size * 8`` must be equal to or greater than ``BTF_INT_BITS()`` > -for the type. The maximum value of ``BTF_INT_BITS()`` is 128. > +for the type. The maximum value of ``BTF_INT_BITS()`` is 512. > > The ``BTF_INT_OFFSET()`` specifies the starting bit offset to calculate values > for this int. For example, a bitfield struct member has: > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h > index 5a667107ad2c..1696fd02b302 100644 > --- a/include/uapi/linux/btf.h > +++ b/include/uapi/linux/btf.h > @@ -84,7 +84,7 @@ struct btf_type { > */ > #define BTF_INT_ENCODING(VAL) (((VAL) & 0x0f000000) >> 24) > #define BTF_INT_OFFSET(VAL) (((VAL) & 0x00ff0000) >> 16) > -#define BTF_INT_BITS(VAL) ((VAL) & 0x000000ff) > +#define BTF_INT_BITS(VAL) ((VAL) & 0x000003ff) > > /* Attributes stored in the BTF_INT_ENCODING */ > #define BTF_INT_SIGNED (1 << 0) > diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c > index 0e9310727281..45ed45ea9962 100644 > --- a/tools/bpf/bpftool/btf_dumper.c > +++ b/tools/bpf/bpftool/btf_dumper.c > @@ -271,6 +271,40 @@ static void btf_int128_print(json_writer_t *jw, const void *data, > } > } > > +static void btf_bigint_print(json_writer_t *jw, const void *data, int nr_bits, > + bool is_plain_text) > +{ > + char buf[nr_bits / 4 + 1]; > + bool first = true; > + int i; > + > +#ifdef __BIG_ENDIAN_BITFIELD > + for (i = 0; i < nr_bits / 64; i++) { > +#else > + for (i = nr_bits / 64 - 1; i >= 0; i++) { > +#endif > + __u64 v = ((__u64 *)data)[i]; > + > + if (first) { > + if (!v) > + continue; > + > + snprintf(buf, sizeof(buf), "%llx", v); > + > + first = false; > + } else { > + size_t off = strlen(buf); > + > + snprintf(buf + off, sizeof(buf) - off, "%016llx", v); > + } > + } > + > + if (is_plain_text) > + jsonw_printf(jw, "0x%s", buf); > + else > + jsonw_printf(jw, "\"0x%s\"", buf); > +} > + > static void btf_int128_shift(__u64 *print_num, __u16 left_shift_bits, > __u16 right_shift_bits) > { > @@ -373,6 +407,11 @@ static int btf_dumper_int(const struct btf_type *t, __u8 bit_offset, > return 0; > } > > + if (nr_bits > 128) { > + btf_bigint_print(jw, data, nr_bits, is_plain_text); > + return 0; > + } > + > if (nr_bits == 128) { > btf_int128_print(jw, data, is_plain_text); > return 0; [...]
| |