* [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
@ 2022-06-15 23:03 Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Yonghong Song @ 2022-06-15 23:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team
Add support for enum64. For 64-bit enumerator value,
previously, the value is truncated into 32bit, e.g.,
for the following enum in linux uapi bpf.h,
enum {
BPF_F_INDEX_MASK = 0xffffffffULL,
BPF_F_CURRENT_CPU = BPF_F_INDEX_MASK,
/* BPF_FUNC_perf_event_output for sk_buff input context. */
BPF_F_CTXLEN_MASK = (0xfffffULL << 32),
};
BPF_F_CTXLEN_MASK will be encoded with 0 with BTF_KIND_ENUM
after pahole dwarf-to-btf conversion.
With this patch, the BPF_F_CTXLEN_MASK will be encoded properly
with BTF_KIND_ENUM64.
This patch is on top of tmp.master since tmp.master has not
been sync'ed with master branch yet.
Changelogs:
v1 -> v2:
- Add flag --skip_encoding_btf_enum64 to disable newly-added functionality.
Yonghong Song (2):
libbpf: Sync with latest libbpf repo
btf: Support BTF_KIND_ENUM64
btf_encoder.c | 65 +++++++++++++++++++++++++++++++++++------------
btf_encoder.h | 2 +-
dwarf_loader.c | 12 +++++++++
dwarves.h | 4 ++-
dwarves_fprintf.c | 6 ++++-
lib/bpf | 2 +-
pahole.c | 10 +++++++-
7 files changed, 80 insertions(+), 21 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo
2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
@ 2022-06-15 23:03 ` Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
2022-06-28 9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
2 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2022-06-15 23:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team
Sync up to commit
645500dd7d2d ci: blacklist mptcp test on s390x
Signed-off-by: Yonghong Song <yhs@fb.com>
---
lib/bpf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bpf b/lib/bpf
index 87dff0a..645500d 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 87dff0a2c775c5943ca9233e69c81a25f2ed1a77
+Subproject commit 645500dd7d2d6b5bb76e4c0375d597d4f0c4814e
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64
2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
@ 2022-06-15 23:03 ` Yonghong Song
2022-06-27 22:30 ` Andrii Nakryiko
2022-06-28 9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
2 siblings, 1 reply; 10+ messages in thread
From: Yonghong Song @ 2022-06-15 23:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team
BTF_KIND_ENUM64 is supported with latest libbpf, which
supports 64-bit enum values. Latest libbpf also supports
signedness for enum values. Add enum64 support in
dwarf-to-btf conversion.
The following is an example of new encoding which covers
signed/unsigned enum64/enum variations.
$cat t.c
enum { /* signed, enum64 */
A = -1,
B = 0xffffffff,
} g1;
enum { /* unsigned, enum64 */
C = 1,
D = 0xfffffffff,
} g2;
enum { /* signed, enum */
E = -1,
F = 0xfffffff,
} g3;
enum { /* unsigned, enum */
G = 1,
H = 0xfffffff,
} g4;
$ clang -g -c t.c
$ pahole -JV t.o
btf_encoder__new: 't.o' doesn't have '.data..percpu' section
Found 0 per-CPU variables!
File t.o:
[1] ENUM64 (anon) size=8
A val=-1
B val=4294967295
[2] INT long size=8 nr_bits=64 encoding=SIGNED
[3] ENUM64 (anon) size=8
C val=1
D val=68719476735
[4] INT unsigned long size=8 nr_bits=64 encoding=(none)
[5] ENUM (anon) size=4
E val=-1
F val=268435455
[6] INT int size=4 nr_bits=32 encoding=SIGNED
[7] ENUM (anon) size=4
G val=1
H val=268435455
[8] INT unsigned int size=4 nr_bits=32 encoding=(none)
With the flag to skip enum64 encoding,
$ pahole -JV t.o --skip_encoding_btf_enum64
btf_encoder__new: 't.o' doesn't have '.data..percpu' section
Found 0 per-CPU variables!
File t.o:
[1] ENUM (anon) size=8
A val=4294967295
B val=4294967295
[2] INT long size=8 nr_bits=64 encoding=SIGNED
[3] ENUM (anon) size=8
C val=1
D val=4294967295
[4] INT unsigned long size=8 nr_bits=64 encoding=(none)
[5] ENUM (anon) size=4
E val=4294967295
F val=268435455
[6] INT int size=4 nr_bits=32 encoding=SIGNED
[7] ENUM (anon) size=4
G val=1
H val=268435455
[8] INT unsigned int size=4 nr_bits=32 encoding=(none)
In the above btf encoding without enum64, all enum types
with the same type size as the corresponding enum64. All these
enum types have unsigned type (kflag = 0) which is required
before kernel enum64 support.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
btf_encoder.c | 65 +++++++++++++++++++++++++++++++++++------------
btf_encoder.h | 2 +-
dwarf_loader.c | 12 +++++++++
dwarves.h | 4 ++-
dwarves_fprintf.c | 6 ++++-
pahole.c | 10 +++++++-
6 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index 9e708e4..96de54c 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -144,6 +144,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
[BTF_KIND_FLOAT] = "FLOAT",
[BTF_KIND_DECL_TAG] = "DECL_TAG",
[BTF_KIND_TYPE_TAG] = "TYPE_TAG",
+ [BTF_KIND_ENUM64] = "ENUM64",
};
static const char *btf__printable_name(const struct btf *btf, uint32_t offset)
@@ -490,34 +491,64 @@ static int32_t btf_encoder__add_struct(struct btf_encoder *encoder, uint8_t kind
return id;
}
-static int32_t btf_encoder__add_enum(struct btf_encoder *encoder, const char *name, uint32_t bit_size)
+static int32_t btf_encoder__add_enum(struct btf_encoder *encoder, const char *name, uint32_t bit_size,
+ bool is_signed, bool no_enum64)
{
struct btf *btf = encoder->btf;
const struct btf_type *t;
int32_t id, size;
+ bool is_enum32;
size = BITS_ROUNDUP_BYTES(bit_size);
- id = btf__add_enum(btf, name, size);
+ is_enum32 = size <= 4 || no_enum64;
+ if (is_enum32)
+ id = btf__add_enum(btf, name, size);
+ else
+ id = btf__add_enum64(btf, name, size, is_signed);
if (id > 0) {
t = btf__type_by_id(btf, id);
btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
} else {
- btf__log_err(btf, BTF_KIND_ENUM, name, true,
+ btf__log_err(btf, is_enum32 ? BTF_KIND_ENUM : BTF_KIND_ENUM64, name, true,
"size=%u Error emitting BTF type", size);
}
return id;
}
-static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int32_t value)
+static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int64_t value,
+ bool is_signed, bool is_enum64, bool no_enum64)
{
- int err = btf__add_enum_value(encoder->btf, name, value);
+ const char *fmt_str;
+ int err;
+
+ /* If enum64 is not allowed, generate enum32 with unsigned int value. In enum64-supported
+ * libbpf library, btf__add_enum_value() will set the kflag (sign bit) in common_type
+ * if the value is negative.
+ */
+ if (no_enum64)
+ err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
+ else if (is_enum64)
+ err = btf__add_enum64_value(encoder->btf, name, value);
+ else
+ err = btf__add_enum_value(encoder->btf, name, value);
if (!err) {
- if (encoder->verbose)
- printf("\t%s val=%d\n", name, value);
+ if (encoder->verbose) {
+ if (no_enum64) {
+ printf("\t%s val=%u\n", name, (uint32_t)value);
+ } else {
+ fmt_str = is_signed ? "\t%s val=%lld\n" : "\t%s val=%llu\n";
+ printf(fmt_str, name, (unsigned long long)value);
+ }
+ }
} else {
- fprintf(stderr, "\t%s val=%d Error emitting BTF enum value\n",
- name, value);
+ if (no_enum64) {
+ fprintf(stderr, "\t%s val=%u Error emitting BTF enum value\n", name, (uint32_t)value);
+ } else {
+ fmt_str = is_signed ? "\t%s val=%lld Error emitting BTF enum value\n"
+ : "\t%s val=%llu Error emitting BTF enum value\n";
+ fprintf(stderr, fmt_str, name, (unsigned long long)value);
+ }
}
return err;
}
@@ -844,27 +875,29 @@ static uint32_t array_type__nelems(struct tag *tag)
return nelem;
}
-static int32_t btf_encoder__add_enum_type(struct btf_encoder *encoder, struct tag *tag)
+static int32_t btf_encoder__add_enum_type(struct btf_encoder *encoder, struct tag *tag, bool no_enum64)
{
struct type *etype = tag__type(tag);
struct enumerator *pos;
const char *name = type__name(etype);
int32_t type_id;
- type_id = btf_encoder__add_enum(encoder, name, etype->size);
+ type_id = btf_encoder__add_enum(encoder, name, etype->size, etype->is_signed_enum, no_enum64);
if (type_id < 0)
return type_id;
type__for_each_enumerator(etype, pos) {
name = enumerator__name(pos);
- if (btf_encoder__add_enum_val(encoder, name, pos->value))
+ if (btf_encoder__add_enum_val(encoder, name, pos->value, etype->is_signed_enum,
+ etype->size > 32, no_enum64))
return -1;
}
return type_id;
}
-static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag, uint32_t type_id_off)
+static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag, uint32_t type_id_off,
+ struct conf_load *conf_load)
{
/* single out type 0 as it represents special type "void" */
uint32_t ref_type_id = tag->type == 0 ? 0 : type_id_off + tag->type;
@@ -903,7 +936,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
encoder->need_index_type = true;
return btf_encoder__add_array(encoder, ref_type_id, encoder->array_index_id, array_type__nelems(tag));
case DW_TAG_enumeration_type:
- return btf_encoder__add_enum_type(encoder, tag);
+ return btf_encoder__add_enum_type(encoder, tag, conf_load->skip_encoding_btf_enum64);
case DW_TAG_subroutine_type:
return btf_encoder__add_func_proto(encoder, tag__ftype(tag), type_id_off);
default:
@@ -1422,7 +1455,7 @@ void btf_encoder__delete(struct btf_encoder *encoder)
free(encoder);
}
-int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu)
+int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load)
{
uint32_t type_id_off = btf__type_cnt(encoder->btf) - 1;
struct llvm_annotation *annot;
@@ -1446,7 +1479,7 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu)
}
cu__for_each_type(cu, core_id, pos) {
- btf_type_id = btf_encoder__encode_tag(encoder, pos, type_id_off);
+ btf_type_id = btf_encoder__encode_tag(encoder, pos, type_id_off, conf_load);
if (btf_type_id < 0 ||
tag__check_id_drift(pos, core_id, btf_type_id, type_id_off)) {
diff --git a/btf_encoder.h b/btf_encoder.h
index 339fae2..a65120c 100644
--- a/btf_encoder.h
+++ b/btf_encoder.h
@@ -21,7 +21,7 @@ void btf_encoder__delete(struct btf_encoder *encoder);
int btf_encoder__encode(struct btf_encoder *encoder);
-int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu);
+int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load);
void btf_encoders__add(struct list_head *encoders, struct btf_encoder *encoder);
diff --git a/dwarf_loader.c b/dwarf_loader.c
index a0d964b..4767602 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -632,6 +632,18 @@ static void type__init(struct type *type, Dwarf_Die *die, struct cu *cu, struct
type->resized = 0;
type->nr_members = 0;
type->nr_static_members = 0;
+ type->is_signed_enum = 0;
+
+ Dwarf_Attribute attr;
+ if (dwarf_attr(die, DW_AT_type, &attr) != NULL) {
+ Dwarf_Die type_die;
+ if (dwarf_formref_die(&attr, &type_die) != NULL) {
+ uint64_t encoding = attr_numeric(&type_die, DW_AT_encoding);
+
+ if (encoding == DW_ATE_signed || encoding == DW_ATE_signed_char)
+ type->is_signed_enum = 1;
+ }
+ }
}
static struct type *type__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
diff --git a/dwarves.h b/dwarves.h
index 4d0e4b6..bec9f08 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -65,6 +65,7 @@ struct conf_load {
bool skip_encoding_btf_decl_tag;
bool skip_missing;
bool skip_encoding_btf_type_tag;
+ bool skip_encoding_btf_enum64;
uint8_t hashtable_bits;
uint8_t max_hashtable_bits;
uint16_t kabi_prefix_len;
@@ -1046,6 +1047,7 @@ struct type {
uint8_t definition_emitted:1;
uint8_t fwd_decl_emitted:1;
uint8_t resized:1;
+ uint8_t is_signed_enum:1;
};
void __type__init(struct type *type);
@@ -1365,7 +1367,7 @@ static inline struct string_type *tag__string_type(const struct tag *tag)
struct enumerator {
struct tag tag;
const char *name;
- uint32_t value;
+ uint64_t value;
struct tag_cu type_enum; // To cache the type_enum searches
};
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index 2cec584..ce64c79 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -437,7 +437,11 @@ size_t enumeration__fprintf(const struct tag *tag, const struct conf_fprintf *co
type__for_each_enumerator(type, pos) {
printed += fprintf(fp, "%.*s\t%-*s = ", indent, tabs,
max_entry_name_len, enumerator__name(pos));
- printed += fprintf(fp, conf->hex_fmt ? "%#x" : "%u", pos->value);
+ if (conf->hex_fmt)
+ printed += fprintf(fp, "%#llx", (unsigned long long)pos->value);
+ else
+ printed += fprintf(fp, type->is_signed_enum ? "%lld" : "%llu",
+ (unsigned long long)pos->value);
printed += fprintf(fp, ",\n");
}
diff --git a/pahole.c b/pahole.c
index 78caa08..e87d9a4 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1220,6 +1220,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
#define ARGP_compile 334
#define ARGP_languages 335
#define ARGP_languages_exclude 336
+#define ARGP_skip_encoding_btf_enum64 337
static const struct argp_option pahole__options[] = {
{
@@ -1622,6 +1623,11 @@ static const struct argp_option pahole__options[] = {
.arg = "LANGUAGES",
.doc = "Don't consider compilation units written in these languages"
},
+ {
+ .name = "skip_encoding_btf_enum64",
+ .key = ARGP_skip_encoding_btf_enum64,
+ .doc = "Do not encode ENUM64sin BTF."
+ },
{
.name = NULL,
}
@@ -1787,6 +1793,8 @@ static error_t pahole__options_parser(int key, char *arg,
/* fallthru */
case ARGP_languages:
languages.str = arg; break;
+ case ARGP_skip_encoding_btf_enum64:
+ conf_load.skip_encoding_btf_enum64 = true; break;
default:
return ARGP_ERR_UNKNOWN;
}
@@ -3067,7 +3075,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
encoder = btf_encoder;
}
- if (btf_encoder__encode_cu(encoder, cu)) {
+ if (btf_encoder__encode_cu(encoder, cu, conf_load)) {
fprintf(stderr, "Encountered error while encoding BTF.\n");
exit(1);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64
2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
@ 2022-06-27 22:30 ` Andrii Nakryiko
2022-06-28 6:19 ` Yonghong Song
0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-06-27 22:30 UTC (permalink / raw)
To: Yonghong Song
Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team
On Wed, Jun 15, 2022 at 4:03 PM Yonghong Song <yhs@fb.com> wrote:
>
> BTF_KIND_ENUM64 is supported with latest libbpf, which
> supports 64-bit enum values. Latest libbpf also supports
> signedness for enum values. Add enum64 support in
> dwarf-to-btf conversion.
>
> The following is an example of new encoding which covers
> signed/unsigned enum64/enum variations.
>
> $cat t.c
> enum { /* signed, enum64 */
> A = -1,
> B = 0xffffffff,
> } g1;
> enum { /* unsigned, enum64 */
> C = 1,
> D = 0xfffffffff,
> } g2;
> enum { /* signed, enum */
> E = -1,
> F = 0xfffffff,
> } g3;
> enum { /* unsigned, enum */
> G = 1,
> H = 0xfffffff,
> } g4;
> $ clang -g -c t.c
> $ pahole -JV t.o
> btf_encoder__new: 't.o' doesn't have '.data..percpu' section
> Found 0 per-CPU variables!
> File t.o:
> [1] ENUM64 (anon) size=8
> A val=-1
> B val=4294967295
> [2] INT long size=8 nr_bits=64 encoding=SIGNED
> [3] ENUM64 (anon) size=8
> C val=1
> D val=68719476735
> [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
> [5] ENUM (anon) size=4
> E val=-1
> F val=268435455
> [6] INT int size=4 nr_bits=32 encoding=SIGNED
> [7] ENUM (anon) size=4
> G val=1
> H val=268435455
> [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>
> With the flag to skip enum64 encoding,
>
> $ pahole -JV t.o --skip_encoding_btf_enum64
> btf_encoder__new: 't.o' doesn't have '.data..percpu' section
> Found 0 per-CPU variables!
> File t.o:
> [1] ENUM (anon) size=8
> A val=4294967295
> B val=4294967295
> [2] INT long size=8 nr_bits=64 encoding=SIGNED
> [3] ENUM (anon) size=8
> C val=1
> D val=4294967295
> [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
> [5] ENUM (anon) size=4
> E val=4294967295
> F val=268435455
> [6] INT int size=4 nr_bits=32 encoding=SIGNED
> [7] ENUM (anon) size=4
> G val=1
> H val=268435455
> [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>
> In the above btf encoding without enum64, all enum types
> with the same type size as the corresponding enum64. All these
> enum types have unsigned type (kflag = 0) which is required
> before kernel enum64 support.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> btf_encoder.c | 65 +++++++++++++++++++++++++++++++++++------------
> btf_encoder.h | 2 +-
> dwarf_loader.c | 12 +++++++++
> dwarves.h | 4 ++-
> dwarves_fprintf.c | 6 ++++-
> pahole.c | 10 +++++++-
> 6 files changed, 79 insertions(+), 20 deletions(-)
>
Sorry for late review, I don't always catch up on emails from older
emails first :(
[...]
> size = BITS_ROUNDUP_BYTES(bit_size);
> - id = btf__add_enum(btf, name, size);
> + is_enum32 = size <= 4 || no_enum64;
> + if (is_enum32)
> + id = btf__add_enum(btf, name, size);
> + else
> + id = btf__add_enum64(btf, name, size, is_signed);
> if (id > 0) {
> t = btf__type_by_id(btf, id);
> btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
> } else {
> - btf__log_err(btf, BTF_KIND_ENUM, name, true,
> + btf__log_err(btf, is_enum32 ? BTF_KIND_ENUM : BTF_KIND_ENUM64, name, true,
> "size=%u Error emitting BTF type", size);
> }
> return id;
> }
>
> -static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int32_t value)
> +static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int64_t value,
> + bool is_signed, bool is_enum64, bool no_enum64)
It was quite confusing to see "is_enum64" and "no_enum64" as arguments
to the same function :)
I'll let Arnaldo decide for himself, but I think it would be cleaner
to pass such configuration switches as fields in struct btf_encoder
itself and just check such flags from relevant btf_encoder__add_xxx()
functions. Such flags are global by nature, so it seems fitting.
But other than that looks good to me.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> {
> - int err = btf__add_enum_value(encoder->btf, name, value);
> + const char *fmt_str;
> + int err;
> +
> + /* If enum64 is not allowed, generate enum32 with unsigned int value. In enum64-supported
> + * libbpf library, btf__add_enum_value() will set the kflag (sign bit) in common_type
> + * if the value is negative.
> + */
> + if (no_enum64)
> + err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
> + else if (is_enum64)
> + err = btf__add_enum64_value(encoder->btf, name, value);
> + else
> + err = btf__add_enum_value(encoder->btf, name, value);
[...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64
2022-06-27 22:30 ` Andrii Nakryiko
@ 2022-06-28 6:19 ` Yonghong Song
0 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2022-06-28 6:19 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team
On 6/27/22 3:30 PM, Andrii Nakryiko wrote:
> On Wed, Jun 15, 2022 at 4:03 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> BTF_KIND_ENUM64 is supported with latest libbpf, which
>> supports 64-bit enum values. Latest libbpf also supports
>> signedness for enum values. Add enum64 support in
>> dwarf-to-btf conversion.
>>
>> The following is an example of new encoding which covers
>> signed/unsigned enum64/enum variations.
>>
>> $cat t.c
>> enum { /* signed, enum64 */
>> A = -1,
>> B = 0xffffffff,
>> } g1;
>> enum { /* unsigned, enum64 */
>> C = 1,
>> D = 0xfffffffff,
>> } g2;
>> enum { /* signed, enum */
>> E = -1,
>> F = 0xfffffff,
>> } g3;
>> enum { /* unsigned, enum */
>> G = 1,
>> H = 0xfffffff,
>> } g4;
>> $ clang -g -c t.c
>> $ pahole -JV t.o
>> btf_encoder__new: 't.o' doesn't have '.data..percpu' section
>> Found 0 per-CPU variables!
>> File t.o:
>> [1] ENUM64 (anon) size=8
>> A val=-1
>> B val=4294967295
>> [2] INT long size=8 nr_bits=64 encoding=SIGNED
>> [3] ENUM64 (anon) size=8
>> C val=1
>> D val=68719476735
>> [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
>> [5] ENUM (anon) size=4
>> E val=-1
>> F val=268435455
>> [6] INT int size=4 nr_bits=32 encoding=SIGNED
>> [7] ENUM (anon) size=4
>> G val=1
>> H val=268435455
>> [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>>
>> With the flag to skip enum64 encoding,
>>
>> $ pahole -JV t.o --skip_encoding_btf_enum64
>> btf_encoder__new: 't.o' doesn't have '.data..percpu' section
>> Found 0 per-CPU variables!
>> File t.o:
>> [1] ENUM (anon) size=8
>> A val=4294967295
>> B val=4294967295
>> [2] INT long size=8 nr_bits=64 encoding=SIGNED
>> [3] ENUM (anon) size=8
>> C val=1
>> D val=4294967295
>> [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
>> [5] ENUM (anon) size=4
>> E val=4294967295
>> F val=268435455
>> [6] INT int size=4 nr_bits=32 encoding=SIGNED
>> [7] ENUM (anon) size=4
>> G val=1
>> H val=268435455
>> [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>>
>> In the above btf encoding without enum64, all enum types
>> with the same type size as the corresponding enum64. All these
>> enum types have unsigned type (kflag = 0) which is required
>> before kernel enum64 support.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> btf_encoder.c | 65 +++++++++++++++++++++++++++++++++++------------
>> btf_encoder.h | 2 +-
>> dwarf_loader.c | 12 +++++++++
>> dwarves.h | 4 ++-
>> dwarves_fprintf.c | 6 ++++-
>> pahole.c | 10 +++++++-
>> 6 files changed, 79 insertions(+), 20 deletions(-)
>>
>
> Sorry for late review, I don't always catch up on emails from older
> emails first :(
>
> [...]
>
>> size = BITS_ROUNDUP_BYTES(bit_size);
>> - id = btf__add_enum(btf, name, size);
>> + is_enum32 = size <= 4 || no_enum64;
>> + if (is_enum32)
>> + id = btf__add_enum(btf, name, size);
>> + else
>> + id = btf__add_enum64(btf, name, size, is_signed);
>> if (id > 0) {
>> t = btf__type_by_id(btf, id);
>> btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
>> } else {
>> - btf__log_err(btf, BTF_KIND_ENUM, name, true,
>> + btf__log_err(btf, is_enum32 ? BTF_KIND_ENUM : BTF_KIND_ENUM64, name, true,
>> "size=%u Error emitting BTF type", size);
>> }
>> return id;
>> }
>>
>> -static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int32_t value)
>> +static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int64_t value,
>> + bool is_signed, bool is_enum64, bool no_enum64)
>
> It was quite confusing to see "is_enum64" and "no_enum64" as arguments
> to the same function :)
>
> I'll let Arnaldo decide for himself, but I think it would be cleaner
> to pass such configuration switches as fields in struct btf_encoder
> itself and just check such flags from relevant btf_encoder__add_xxx()
> functions. Such flags are global by nature, so it seems fitting.
This will require an additional struct definition. It should work but I
will wait for Arnaldo's comments as well.
>
> But other than that looks good to me.
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
>> {
>> - int err = btf__add_enum_value(encoder->btf, name, value);
>> + const char *fmt_str;
>> + int err;
>> +
>> + /* If enum64 is not allowed, generate enum32 with unsigned int value. In enum64-supported
>> + * libbpf library, btf__add_enum_value() will set the kflag (sign bit) in common_type
>> + * if the value is negative.
>> + */
>> + if (no_enum64)
>> + err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
>> + else if (is_enum64)
>> + err = btf__add_enum64_value(encoder->btf, name, value);
>> + else
>> + err = btf__add_enum_value(encoder->btf, name, value);
>
> [...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
@ 2022-06-28 9:51 ` Jiri Olsa
2022-06-29 16:30 ` Arnaldo Carvalho de Melo
2 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2022-06-28 9:51 UTC (permalink / raw)
To: Yonghong Song
Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team
On Wed, Jun 15, 2022 at 04:03:06PM -0700, Yonghong Song wrote:
> Add support for enum64. For 64-bit enumerator value,
> previously, the value is truncated into 32bit, e.g.,
> for the following enum in linux uapi bpf.h,
> enum {
> BPF_F_INDEX_MASK = 0xffffffffULL,
> BPF_F_CURRENT_CPU = BPF_F_INDEX_MASK,
> /* BPF_FUNC_perf_event_output for sk_buff input context. */
> BPF_F_CTXLEN_MASK = (0xfffffULL << 32),
> };
>
> BPF_F_CTXLEN_MASK will be encoded with 0 with BTF_KIND_ENUM
> after pahole dwarf-to-btf conversion.
> With this patch, the BPF_F_CTXLEN_MASK will be encoded properly
> with BTF_KIND_ENUM64.
yep, tried this on latest vmlinux and got:
[705813] ENUM64 (anon) size=8
BPF_F_INDEX_MASK val=4294967295
BPF_F_CURRENT_CPU val=4294967295
BPF_F_CTXLEN_MASK val=4503595332403200
which is correct
Tested-by: Jiri Olsa <jolsa@kernel.org>
jirka
>
> This patch is on top of tmp.master since tmp.master has not
> been sync'ed with master branch yet.
>
> Changelogs:
> v1 -> v2:
> - Add flag --skip_encoding_btf_enum64 to disable newly-added functionality.
>
> Yonghong Song (2):
> libbpf: Sync with latest libbpf repo
> btf: Support BTF_KIND_ENUM64
>
> btf_encoder.c | 65 +++++++++++++++++++++++++++++++++++------------
> btf_encoder.h | 2 +-
> dwarf_loader.c | 12 +++++++++
> dwarves.h | 4 ++-
> dwarves_fprintf.c | 6 ++++-
> lib/bpf | 2 +-
> pahole.c | 10 +++++++-
> 7 files changed, 80 insertions(+), 21 deletions(-)
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
2022-06-28 9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
@ 2022-06-29 16:30 ` Arnaldo Carvalho de Melo
2022-06-29 16:56 ` [PATCH] btf_loader: support BTF_KIND_ENUM64 was " Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-06-29 16:30 UTC (permalink / raw)
To: Jiri Olsa
Cc: Yonghong Song, Arnaldo Carvalho de Melo, dwarves,
Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
Em Tue, Jun 28, 2022 at 11:51:52AM +0200, Jiri Olsa escreveu:
> On Wed, Jun 15, 2022 at 04:03:06PM -0700, Yonghong Song wrote:
> > Add support for enum64. For 64-bit enumerator value,
> > previously, the value is truncated into 32bit, e.g.,
> > for the following enum in linux uapi bpf.h,
> > enum {
> > BPF_F_INDEX_MASK = 0xffffffffULL,
> > BPF_F_CURRENT_CPU = BPF_F_INDEX_MASK,
> > /* BPF_FUNC_perf_event_output for sk_buff input context. */
> > BPF_F_CTXLEN_MASK = (0xfffffULL << 32),
> > };
> >
> > BPF_F_CTXLEN_MASK will be encoded with 0 with BTF_KIND_ENUM
> > after pahole dwarf-to-btf conversion.
> > With this patch, the BPF_F_CTXLEN_MASK will be encoded properly
> > with BTF_KIND_ENUM64.
>
> yep, tried this on latest vmlinux and got:
>
> [705813] ENUM64 (anon) size=8
> BPF_F_INDEX_MASK val=4294967295
> BPF_F_CURRENT_CPU val=4294967295
> BPF_F_CTXLEN_MASK val=4503595332403200
>
> which is correct
>
> Tested-by: Jiri Olsa <jolsa@kernel.org>
I'm testing with v3:
⬢[acme@toolbox pahole]$ pahole -JV vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
BPF_FUNC_xdp_get_buff_len val=188
BPF_FUNC_xdp_load_bytes val=189
BPF_FUNC_xdp_store_bytes val=190
BPF_FUNC_copy_from_user_task val=191
BPF_FUNC_skb_set_tstamp val=192
BPF_FUNC_ima_file_hash val=193
__BPF_FUNC_MAX_ID val=194
[672880] ENUM64 (anon) size=8
BPF_F_INDEX_MASK val=4294967295
BPF_F_CURRENT_CPU val=4294967295
BPF_F_CTXLEN_MASK val=4503595332403200
[672881] ENUM (anon) size=4
BPF_F_GET_BRANCH_RECORDS_SIZE val=1
[672882] ARRAY (anon) type_id=672304 index_type_id=18 nr_elems=4
[672883] STRUCT (anon) size=16
tp_name type_id=672266 bits_offset=0
But:
⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
BTF: idx: 4173, Unknown kind 19
BTF: idx: 4975, Unknown kind 19
BTF: idx: 6673, Unknown kind 19
BTF: idx: 27413, Unknown kind 19
BTF: idx: 30626, Unknown kind 19
BTF: idx: 30829, Unknown kind 19
BTF: idx: 38040, Unknown kind 19
BTF: idx: 56969, Unknown kind 19
BTF: idx: 83004, Unknown kind 19
⬢[acme@toolbox pahole]$
Ok, I need to update pahole's BTF loader to support:
lib/bpf/src/btf.h:#define BTF_KIND_ENUM64 19 /* Enum for up-to 64bit values */
Working on it now.
Jiri, can I keep your Tested-by for v3?
- Arnaldo
> jirka
>
> >
> > This patch is on top of tmp.master since tmp.master has not
> > been sync'ed with master branch yet.
> >
> > Changelogs:
> > v1 -> v2:
> > - Add flag --skip_encoding_btf_enum64 to disable newly-added functionality.
> >
> > Yonghong Song (2):
> > libbpf: Sync with latest libbpf repo
> > btf: Support BTF_KIND_ENUM64
> >
> > btf_encoder.c | 65 +++++++++++++++++++++++++++++++++++------------
> > btf_encoder.h | 2 +-
> > dwarf_loader.c | 12 +++++++++
> > dwarves.h | 4 ++-
> > dwarves_fprintf.c | 6 ++++-
> > lib/bpf | 2 +-
> > pahole.c | 10 +++++++-
> > 7 files changed, 80 insertions(+), 21 deletions(-)
> >
> > --
> > 2.30.2
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] btf_loader: support BTF_KIND_ENUM64 was Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
2022-06-29 16:30 ` Arnaldo Carvalho de Melo
@ 2022-06-29 16:56 ` Arnaldo Carvalho de Melo
2022-07-06 4:28 ` Andrii Nakryiko
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-06-29 16:56 UTC (permalink / raw)
To: Yonghong Song, Jiri Olsa
Cc: dwarves, Alexei Starovoitov, Andrii Nakryiko, bpf,
Daniel Borkmann, kernel-team
Em Wed, Jun 29, 2022 at 01:30:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
> BTF: idx: 4173, Unknown kind 19
> BTF: idx: 4975, Unknown kind 19
> BTF: idx: 6673, Unknown kind 19
> BTF: idx: 27413, Unknown kind 19
> BTF: idx: 30626, Unknown kind 19
> BTF: idx: 30829, Unknown kind 19
> BTF: idx: 38040, Unknown kind 19
> BTF: idx: 56969, Unknown kind 19
> BTF: idx: 83004, Unknown kind 19
> ⬢[acme@toolbox pahole]$
>
> Ok, I need to update pahole's BTF loader to support:
>
> lib/bpf/src/btf.h:#define BTF_KIND_ENUM64 19 /* Enum for up-to 64bit values */
>
>
> Working on it now.
⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B5 -A5 BPF_F_CTXLEN_MASK
/* 27413 */
enum {
BPF_F_INDEX_MASK = 4294967295,
BPF_F_CURRENT_CPU = 4294967295,
BPF_F_CTXLEN_MASK = 4503595332403200,
} __attribute__((__packed__)); /* size: 8 */
/* 27414 */
enum {
BPF_F_GET_BRANCH_RECORDS_SIZE = 1,
⬢[acme@toolbox pahole]$
Quick patch here, please Ack, if possible:
diff --git a/btf_loader.c b/btf_loader.c
index b5d444643adf30b1..e57ecce2cde26e4e 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -312,6 +312,49 @@ out_free:
return -ENOMEM;
}
+static struct enumerator *enumerator__new64(const char *name, uint64_t value)
+{
+ struct enumerator *en = tag__alloc(sizeof(*en));
+
+ if (en != NULL) {
+ en->name = name;
+ en->value = value; // Value is already 64-bit, as this is used with DWARF as well
+ en->tag.tag = DW_TAG_enumerator;
+ }
+
+ return en;
+}
+
+static int create_new_enumeration64(struct cu *cu, const struct btf_type *tp, uint32_t id)
+{
+ struct btf_enum64 *ep = btf_enum64(tp);
+ uint16_t i, vlen = btf_vlen(tp);
+ struct type *enumeration = type__new(DW_TAG_enumeration_type,
+ cu__btf_str(cu, tp->name_off),
+ tp->size ? tp->size * 8 : (sizeof(int) * 8));
+
+ if (enumeration == NULL)
+ return -ENOMEM;
+
+ for (i = 0; i < vlen; i++) {
+ const char *name = cu__btf_str(cu, ep[i].name_off);
+ uint64_t value = ((uint64_t)ep[i].val_hi32) << 32 | ep[i].val_lo32;
+ struct enumerator *enumerator = enumerator__new64(name, value);
+
+ if (enumerator == NULL)
+ goto out_free;
+
+ enumeration__add(enumeration, enumerator);
+ }
+
+ cu__add_tag_with_id(cu, &enumeration->namespace.tag, id);
+
+ return 0;
+out_free:
+ enumeration__delete(enumeration);
+ return -ENOMEM;
+}
+
static int create_new_subroutine_type(struct cu *cu, const struct btf_type *tp, uint32_t id)
{
struct ftype *proto = tag__alloc(sizeof(*proto));
@@ -419,6 +462,9 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
case BTF_KIND_ENUM:
err = create_new_enumeration(cu, type_ptr, type_index);
break;
+ case BTF_KIND_ENUM64:
+ err = create_new_enumeration64(cu, type_ptr, type_index);
+ break;
case BTF_KIND_FWD:
err = create_new_forward_decl(cu, type_ptr, type_index);
break;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] btf_loader: support BTF_KIND_ENUM64 was Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
2022-06-29 16:56 ` [PATCH] btf_loader: support BTF_KIND_ENUM64 was " Arnaldo Carvalho de Melo
@ 2022-07-06 4:28 ` Andrii Nakryiko
2022-07-06 15:51 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-07-06 4:28 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Yonghong Song, Jiri Olsa, dwarves, Alexei Starovoitov,
Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team
On Wed, Jun 29, 2022 at 9:56 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Jun 29, 2022 at 01:30:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> > ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
> > BTF: idx: 4173, Unknown kind 19
> > BTF: idx: 4975, Unknown kind 19
> > BTF: idx: 6673, Unknown kind 19
> > BTF: idx: 27413, Unknown kind 19
> > BTF: idx: 30626, Unknown kind 19
> > BTF: idx: 30829, Unknown kind 19
> > BTF: idx: 38040, Unknown kind 19
> > BTF: idx: 56969, Unknown kind 19
> > BTF: idx: 83004, Unknown kind 19
> > ⬢[acme@toolbox pahole]$
> >
> > Ok, I need to update pahole's BTF loader to support:
> >
> > lib/bpf/src/btf.h:#define BTF_KIND_ENUM64 19 /* Enum for up-to 64bit values */
> >
> >
> > Working on it now.
>
> ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B5 -A5 BPF_F_CTXLEN_MASK
>
> /* 27413 */
> enum {
> BPF_F_INDEX_MASK = 4294967295,
> BPF_F_CURRENT_CPU = 4294967295,
> BPF_F_CTXLEN_MASK = 4503595332403200,
> } __attribute__((__packed__)); /* size: 8 */
>
> /* 27414 */
> enum {
> BPF_F_GET_BRANCH_RECORDS_SIZE = 1,
> ⬢[acme@toolbox pahole]$
>
> Quick patch here, please Ack, if possible:
>
two minor nits, but looks good overall:
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> diff --git a/btf_loader.c b/btf_loader.c
> index b5d444643adf30b1..e57ecce2cde26e4e 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -312,6 +312,49 @@ out_free:
> return -ENOMEM;
> }
>
> +static struct enumerator *enumerator__new64(const char *name, uint64_t value)
> +{
> + struct enumerator *en = tag__alloc(sizeof(*en));
> +
> + if (en != NULL) {
> + en->name = name;
> + en->value = value; // Value is already 64-bit, as this is used with DWARF as well
> + en->tag.tag = DW_TAG_enumerator;
> + }
> +
> + return en;
> +}
> +
> +static int create_new_enumeration64(struct cu *cu, const struct btf_type *tp, uint32_t id)
> +{
> + struct btf_enum64 *ep = btf_enum64(tp);
> + uint16_t i, vlen = btf_vlen(tp);
> + struct type *enumeration = type__new(DW_TAG_enumeration_type,
> + cu__btf_str(cu, tp->name_off),
> + tp->size ? tp->size * 8 : (sizeof(int) * 8));
tp->size should always be valid, so this fall back to sizeof(int)
isn't necessary
> +
> + if (enumeration == NULL)
> + return -ENOMEM;
> +
> + for (i = 0; i < vlen; i++) {
> + const char *name = cu__btf_str(cu, ep[i].name_off);
> + uint64_t value = ((uint64_t)ep[i].val_hi32) << 32 | ep[i].val_lo32;
use btf_enum64_value() defined in libbpf's btf.h header
> + struct enumerator *enumerator = enumerator__new64(name, value);
> +
> + if (enumerator == NULL)
> + goto out_free;
> +
> + enumeration__add(enumeration, enumerator);
> + }
> +
> + cu__add_tag_with_id(cu, &enumeration->namespace.tag, id);
> +
> + return 0;
> +out_free:
> + enumeration__delete(enumeration);
> + return -ENOMEM;
> +}
> +
> static int create_new_subroutine_type(struct cu *cu, const struct btf_type *tp, uint32_t id)
> {
> struct ftype *proto = tag__alloc(sizeof(*proto));
> @@ -419,6 +462,9 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
> case BTF_KIND_ENUM:
> err = create_new_enumeration(cu, type_ptr, type_index);
> break;
> + case BTF_KIND_ENUM64:
> + err = create_new_enumeration64(cu, type_ptr, type_index);
> + break;
> case BTF_KIND_FWD:
> err = create_new_forward_decl(cu, type_ptr, type_index);
> break;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btf_loader: support BTF_KIND_ENUM64 was Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
2022-07-06 4:28 ` Andrii Nakryiko
@ 2022-07-06 15:51 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-07-06 15:51 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Yonghong Song, Jiri Olsa, dwarves, Alexei Starovoitov,
Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team
Em Tue, Jul 05, 2022 at 09:28:05PM -0700, Andrii Nakryiko escreveu:
> On Wed, Jun 29, 2022 at 9:56 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, Jun 29, 2022 at 01:30:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
> > > BTF: idx: 4173, Unknown kind 19
> > > BTF: idx: 4975, Unknown kind 19
> > > BTF: idx: 6673, Unknown kind 19
> > > BTF: idx: 27413, Unknown kind 19
> > > BTF: idx: 30626, Unknown kind 19
> > > BTF: idx: 30829, Unknown kind 19
> > > BTF: idx: 38040, Unknown kind 19
> > > BTF: idx: 56969, Unknown kind 19
> > > BTF: idx: 83004, Unknown kind 19
> > > ⬢[acme@toolbox pahole]$
> > >
> > > Ok, I need to update pahole's BTF loader to support:
> > >
> > > lib/bpf/src/btf.h:#define BTF_KIND_ENUM64 19 /* Enum for up-to 64bit values */
> > >
> > >
> > > Working on it now.
> >
> > ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B5 -A5 BPF_F_CTXLEN_MASK
> >
> > /* 27413 */
> > enum {
> > BPF_F_INDEX_MASK = 4294967295,
> > BPF_F_CURRENT_CPU = 4294967295,
> > BPF_F_CTXLEN_MASK = 4503595332403200,
> > } __attribute__((__packed__)); /* size: 8 */
> >
> > /* 27414 */
> > enum {
> > BPF_F_GET_BRANCH_RECORDS_SIZE = 1,
> > ⬢[acme@toolbox pahole]$
> >
> > Quick patch here, please Ack, if possible:
> >
>
> two minor nits, but looks good overall:
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
Thanks for reviewing it!
> > diff --git a/btf_loader.c b/btf_loader.c
> > index b5d444643adf30b1..e57ecce2cde26e4e 100644
> > --- a/btf_loader.c
> > +++ b/btf_loader.c
> > @@ -312,6 +312,49 @@ out_free:
> > return -ENOMEM;
> > }
> >
> > +static struct enumerator *enumerator__new64(const char *name, uint64_t value)
> > +{
> > + struct enumerator *en = tag__alloc(sizeof(*en));
> > +
> > + if (en != NULL) {
> > + en->name = name;
> > + en->value = value; // Value is already 64-bit, as this is used with DWARF as well
> > + en->tag.tag = DW_TAG_enumerator;
> > + }
> > +
> > + return en;
> > +}
> > +
> > +static int create_new_enumeration64(struct cu *cu, const struct btf_type *tp, uint32_t id)
> > +{
> > + struct btf_enum64 *ep = btf_enum64(tp);
> > + uint16_t i, vlen = btf_vlen(tp);
> > + struct type *enumeration = type__new(DW_TAG_enumeration_type,
> > + cu__btf_str(cu, tp->name_off),
> > + tp->size ? tp->size * 8 : (sizeof(int) * 8));
>
> tp->size should always be valid, so this fall back to sizeof(int)
> isn't necessary
This was just a copy from the create_new_enumeration() one, will remove
it there as well as a prep patch, then not have it in the 64-bit
version.
> > +
> > + if (enumeration == NULL)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < vlen; i++) {
> > + const char *name = cu__btf_str(cu, ep[i].name_off);
> > + uint64_t value = ((uint64_t)ep[i].val_hi32) << 32 | ep[i].val_lo32;
>
> use btf_enum64_value() defined in libbpf's btf.h header
Great, will pick that
> > + struct enumerator *enumerator = enumerator__new64(name, value);
> > +
> > + if (enumerator == NULL)
> > + goto out_free;
> > +
> > + enumeration__add(enumeration, enumerator);
> > + }
> > +
> > + cu__add_tag_with_id(cu, &enumeration->namespace.tag, id);
> > +
> > + return 0;
> > +out_free:
> > + enumeration__delete(enumeration);
> > + return -ENOMEM;
> > +}
> > +
> > static int create_new_subroutine_type(struct cu *cu, const struct btf_type *tp, uint32_t id)
> > {
> > struct ftype *proto = tag__alloc(sizeof(*proto));
> > @@ -419,6 +462,9 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
> > case BTF_KIND_ENUM:
> > err = create_new_enumeration(cu, type_ptr, type_index);
> > break;
> > + case BTF_KIND_ENUM64:
> > + err = create_new_enumeration64(cu, type_ptr, type_index);
> > + break;
> > case BTF_KIND_FWD:
> > err = create_new_forward_decl(cu, type_ptr, type_index);
> > break;
--
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-07-06 15:53 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
2022-06-27 22:30 ` Andrii Nakryiko
2022-06-28 6:19 ` Yonghong Song
2022-06-28 9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
2022-06-29 16:30 ` Arnaldo Carvalho de Melo
2022-06-29 16:56 ` [PATCH] btf_loader: support BTF_KIND_ENUM64 was " Arnaldo Carvalho de Melo
2022-07-06 4:28 ` Andrii Nakryiko
2022-07-06 15:51 ` Arnaldo Carvalho de Melo
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.