* [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 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).