| Message ID | cover.1609855479.git.sean@mess.org |
|---|---|
| Headers | show |
| Series | btf: support ints larger than 128 bits | expand |
On Tue, Jan 05, 2021 at 02:45:30PM +0000, 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 enabled in clang, which is under > review. 1. Please explain the use case. 2. All patches have the same commit message which is not useful. Please spend some time in the commit message to explain what each individual patch does. 3. The test_extint.py is mostly a copy-and-paste from the existing test_offload.py? Does it need most of the test_offload.py to test the BTF 256/512 bit int? Please create a minimal test and use the test_progs.c infra-structure.
On Tue, Jan 5, 2021 at 6:45 AM Sean Young <sean@mess.org> 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 enabled 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 +- > kernel/bpf/btf.c | 54 ++++++++++++++++++++++++++++------ > tools/include/uapi/linux/btf.h | 2 +- > 4 files changed, 49 insertions(+), 13 deletions(-) > > 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/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 8d6bdb4f4d61..44bc17207e9b 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -166,7 +166,8 @@ > * > */ > > -#define BITS_PER_U128 (sizeof(u64) * BITS_PER_BYTE * 2) > +#define BITS_PER_U128 128 > +#define BITS_PER_U512 512 > #define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1) > #define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK) > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > @@ -1907,9 +1908,9 @@ static int btf_int_check_member(struct btf_verifier_env *env, > nr_copy_bits = BTF_INT_BITS(int_data) + > BITS_PER_BYTE_MASKED(struct_bits_off); > > - if (nr_copy_bits > BITS_PER_U128) { > + if (nr_copy_bits > BITS_PER_U512) { > btf_verifier_log_member(env, struct_type, member, > - "nr_copy_bits exceeds 128"); > + "nr_copy_bits exceeds 512"); > return -EINVAL; > } > > @@ -1963,9 +1964,9 @@ static int btf_int_check_kflag_member(struct btf_verifier_env *env, > > bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off); > nr_copy_bits = nr_bits + BITS_PER_BYTE_MASKED(struct_bits_off); > - if (nr_copy_bits > BITS_PER_U128) { > + if (nr_copy_bits > BITS_PER_U512) { > btf_verifier_log_member(env, struct_type, member, > - "nr_copy_bits exceeds 128"); > + "nr_copy_bits exceeds 512"); > return -EINVAL; > } > > @@ -2012,9 +2013,9 @@ static s32 btf_int_check_meta(struct btf_verifier_env *env, > > nr_bits = BTF_INT_BITS(int_data) + BTF_INT_OFFSET(int_data); > > - if (nr_bits > BITS_PER_U128) { > - btf_verifier_log_type(env, t, "nr_bits exceeds %zu", > - BITS_PER_U128); > + if (nr_bits > BITS_PER_U512) { > + btf_verifier_log_type(env, t, "nr_bits exceeds %u", > + BITS_PER_U512); > return -EINVAL; > } > > @@ -2080,6 +2081,37 @@ static void btf_int128_print(struct btf_show *show, void *data) > lower_num); > } > > +static void btf_bigint_print(struct btf_show *show, void *data, u16 nr_bits) > +{ > + /* data points to 256 or 512 bit int type */ > + char buf[129]; > + int last_u64 = nr_bits / 64 - 1; > + bool seen_nonzero = false; > + int i; > + > + for (i = 0; i <= last_u64; i++) { > +#ifdef __BIG_ENDIAN_BITFIELD > + u64 v = ((u64 *)data)[i]; > +#else > + u64 v = ((u64 *)data)[last_u64 - i]; > +#endif > + if (!seen_nonzero) { > + if (!v && i != last_u64) > + continue; > + > + snprintf(buf, sizeof(buf), "%llx", v); > + > + seen_nonzero = true; > + } else { > + size_t off = strlen(buf); this is wasteful, snprintf() returns number of characters printed, so you can maintain offset properly > + > + snprintf(buf + off, sizeof(buf) - off, "%016llx", v); > + } > + } > + > + btf_show_type_value(show, "0x%s", buf); > +} seen_nonzero is a bit convoluted, two simple loops might be more straightforward: u64 v; int off; /* find first non-zero u64 (or stop on the last one regardless) */ for (i = 0; i < last_u64; i++) { v = ...; if (!v) continue; } /* print non-zero or zero, but last u64 */ off = snprintf(buf, sizeof(buf), "%llx", v); /* print the rest with zero padding */ for (i++; i <= last_u64; i++) { v = ...; off += snprintf(buf + off, sizeof(buf) - off, "%016llx", v); } > + > static void btf_int128_shift(u64 *print_num, u16 left_shift_bits, > u16 right_shift_bits) > { > @@ -2172,7 +2204,7 @@ static void btf_int_show(const struct btf *btf, const struct btf_type *t, > u32 int_data = btf_type_int(t); > u8 encoding = BTF_INT_ENCODING(int_data); > bool sign = encoding & BTF_INT_SIGNED; > - u8 nr_bits = BTF_INT_BITS(int_data); > + u16 nr_bits = BTF_INT_BITS(int_data); > void *safe_data; > > safe_data = btf_show_start_type(show, t, type_id, data); > @@ -2186,6 +2218,10 @@ static void btf_int_show(const struct btf *btf, const struct btf_type *t, > } > > switch (nr_bits) { > + case 512: > + case 256: > + btf_bigint_print(show, safe_data, nr_bits); > + break; > case 128: > btf_int128_print(show, safe_data); btf_bigint_print() supersedes btf_int128_print(), why maintain both? > break; > diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h > index 5a667107ad2c..1696fd02b302 100644 > --- a/tools/include/uapi/linux/btf.h > +++ b/tools/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) > -- > 2.29.2 >
On Tue, Jan 5, 2021 at 6:45 AM Sean Young <sean@mess.org> 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. > > This requires the _ExtInt extension enabled 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> > --- all the same comments as in patch #1 > tools/bpf/bpftool/btf_dumper.c | 40 ++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c > index 0e9310727281..8b5318ec5c26 100644 > --- a/tools/bpf/bpftool/btf_dumper.c > +++ b/tools/bpf/bpftool/btf_dumper.c > @@ -271,6 +271,41 @@ static void btf_int128_print(json_writer_t *jw, const void *data, > } > } [...]
On Tue, Jan 5, 2021 at 9:10 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Jan 5, 2021 at 6:45 AM Sean Young <sean@mess.org> 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 enabled 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 +- > > kernel/bpf/btf.c | 54 ++++++++++++++++++++++++++++------ > > tools/include/uapi/linux/btf.h | 2 +- > > 4 files changed, 49 insertions(+), 13 deletions(-) > > > > 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/kernel/bpf/btf.c b/kernel/bpf/btf.c > > index 8d6bdb4f4d61..44bc17207e9b 100644 > > --- a/kernel/bpf/btf.c > > +++ b/kernel/bpf/btf.c > > @@ -166,7 +166,8 @@ > > * > > */ > > > > -#define BITS_PER_U128 (sizeof(u64) * BITS_PER_BYTE * 2) > > +#define BITS_PER_U128 128 > > +#define BITS_PER_U512 512 > > #define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1) > > #define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK) > > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > > @@ -1907,9 +1908,9 @@ static int btf_int_check_member(struct btf_verifier_env *env, > > nr_copy_bits = BTF_INT_BITS(int_data) + > > BITS_PER_BYTE_MASKED(struct_bits_off); > > > > - if (nr_copy_bits > BITS_PER_U128) { > > + if (nr_copy_bits > BITS_PER_U512) { > > btf_verifier_log_member(env, struct_type, member, > > - "nr_copy_bits exceeds 128"); > > + "nr_copy_bits exceeds 512"); > > return -EINVAL; > > } > > > > @@ -1963,9 +1964,9 @@ static int btf_int_check_kflag_member(struct btf_verifier_env *env, > > > > bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off); > > nr_copy_bits = nr_bits + BITS_PER_BYTE_MASKED(struct_bits_off); > > - if (nr_copy_bits > BITS_PER_U128) { > > + if (nr_copy_bits > BITS_PER_U512) { > > btf_verifier_log_member(env, struct_type, member, > > - "nr_copy_bits exceeds 128"); > > + "nr_copy_bits exceeds 512"); > > return -EINVAL; > > } > > > > @@ -2012,9 +2013,9 @@ static s32 btf_int_check_meta(struct btf_verifier_env *env, > > > > nr_bits = BTF_INT_BITS(int_data) + BTF_INT_OFFSET(int_data); > > > > - if (nr_bits > BITS_PER_U128) { > > - btf_verifier_log_type(env, t, "nr_bits exceeds %zu", > > - BITS_PER_U128); > > + if (nr_bits > BITS_PER_U512) { > > + btf_verifier_log_type(env, t, "nr_bits exceeds %u", > > + BITS_PER_U512); > > return -EINVAL; > > } > > > > @@ -2080,6 +2081,37 @@ static void btf_int128_print(struct btf_show *show, void *data) > > lower_num); > > } > > > > +static void btf_bigint_print(struct btf_show *show, void *data, u16 nr_bits) > > +{ > > + /* data points to 256 or 512 bit int type */ > > + char buf[129]; > > + int last_u64 = nr_bits / 64 - 1; > > + bool seen_nonzero = false; > > + int i; > > + > > + for (i = 0; i <= last_u64; i++) { > > +#ifdef __BIG_ENDIAN_BITFIELD > > + u64 v = ((u64 *)data)[i]; > > +#else > > + u64 v = ((u64 *)data)[last_u64 - i]; > > +#endif to avoid duplicating this #ifdef with my suggestion, you can do something like #ifdef __BIG_ENDIAN_BITFIELD u64 *v = (u64 *)data; int step = 1; #else u64 *v = (u64 *)data + last_u64; int step = -1; #endif and then just `v += step;` everywhere > > + if (!seen_nonzero) { > > + if (!v && i != last_u64) > > + continue; > > + > > + snprintf(buf, sizeof(buf), "%llx", v); > > + > > + seen_nonzero = true; > > + } else { > > + size_t off = strlen(buf); > > this is wasteful, snprintf() returns number of characters printed, so > you can maintain offset properly > > > + > > + snprintf(buf + off, sizeof(buf) - off, "%016llx", v); > > + } > > + } > > + > > + btf_show_type_value(show, "0x%s", buf); > > +} > > seen_nonzero is a bit convoluted, two simple loops might be more > straightforward: > > u64 v; > int off; > > /* find first non-zero u64 (or stop on the last one regardless) */ > for (i = 0; i < last_u64; i++) { > v = ...; > if (!v) > continue; > } > /* print non-zero or zero, but last u64 */ > off = snprintf(buf, sizeof(buf), "%llx", v); > /* print the rest with zero padding */ > for (i++; i <= last_u64; i++) { > v = ...; > off += snprintf(buf + off, sizeof(buf) - off, "%016llx", v); > } > > > + > > static void btf_int128_shift(u64 *print_num, u16 left_shift_bits, > > u16 right_shift_bits) > > { > > @@ -2172,7 +2204,7 @@ static void btf_int_show(const struct btf *btf, const struct btf_type *t, > > u32 int_data = btf_type_int(t); > > u8 encoding = BTF_INT_ENCODING(int_data); > > bool sign = encoding & BTF_INT_SIGNED; > > - u8 nr_bits = BTF_INT_BITS(int_data); > > + u16 nr_bits = BTF_INT_BITS(int_data); > > void *safe_data; > > > > safe_data = btf_show_start_type(show, t, type_id, data); > > @@ -2186,6 +2218,10 @@ static void btf_int_show(const struct btf *btf, const struct btf_type *t, > > } > > > > switch (nr_bits) { > > + case 512: > > + case 256: > > + btf_bigint_print(show, safe_data, nr_bits); > > + break; > > case 128: > > btf_int128_print(show, safe_data); > > btf_bigint_print() supersedes btf_int128_print(), why maintain both? > > > break; > > diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h > > index 5a667107ad2c..1696fd02b302 100644 > > --- a/tools/include/uapi/linux/btf.h > > +++ b/tools/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) > > -- > > 2.29.2 > >
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 enabled 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> changes since v2: - split patches into 4 distinct patches changes since v1: - added tests as suggested by Yonghong Song - added kernel pretty-printer Sean Young (4): btf: add support for ints larger than 128 bits libbpf: add support for ints larger than 128 bits bpftool: add support for ints larger than 128 bits bpf: add tests for ints larger than 128 bits Documentation/bpf/btf.rst | 4 +- include/uapi/linux/btf.h | 2 +- kernel/bpf/btf.c | 54 +- tools/bpf/bpftool/btf_dumper.c | 40 ++ tools/include/uapi/linux/btf.h | 2 +- tools/lib/bpf/btf.c | 2 +- tools/testing/selftests/bpf/Makefile | 3 +- tools/testing/selftests/bpf/prog_tests/btf.c | 3 +- .../selftests/bpf/progs/test_btf_extint.c | 50 ++ tools/testing/selftests/bpf/test_extint.py | 535 ++++++++++++++++++ 10 files changed, 679 insertions(+), 16 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/test_btf_extint.c create mode 100755 tools/testing/selftests/bpf/test_extint.py