| Message ID | 20240503111836.25275-1-jose.marchesi@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [RFC,bpf-next] bpf: avoid clang-specific push/pop attribute pragmas in bpftool | expand |
On Fri, 2024-05-03 at 13:18 +0200, Jose E. Marchesi wrote: [...] > This patch modifies bpftool in order to, instead of using the pragmas, > define ATTR_PRESERVE_ACCESS_INDEX to conditionally expand to the CO-RE > attribute: > > #ifndef __VMLINUX_H__ > #define __VMLINUX_H__ > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > #define ATTR_PRESERVE_ACCESS_INDEX __attribute__((preserve_access_index)) > #else > #define ATTR_PRESERVE_ACCESS_INDEX > #endif Nit: maybe swap the branches to avoid double negation? > > [... type definitions generated from kernel BTF ... ] > > #undef ATTR_PRESERVE_ACCESS_INDEX > > and then the new btf_dump__dump_type_with_opts is used with options > specifying that we wish to have struct type attributes: > > DECLARE_LIBBPF_OPTS(btf_dump_type_opts, opts); > [...] > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; > [...] > err = btf_dump__dump_type_with_opts(d, root_type_ids[i], &opts); > > This is a RFC because introducing a new libbpf public function > btf_dump__dump_type_with_opts may not be desirable. > > An alternative could be to, instead of passing the record_attrs_str > option in a btf_dump_type_opts, pass it in the global dumper's option > btf_dump_opts: > > DECLARE_LIBBPF_OPTS(btf_dump_opts, opts); > [...] > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; > [...] > d = btf_dump__new(btf, btf_dump_printf, NULL, &opts); > [...] > err = btf_dump__dump_type(d, root_type_ids[i]); > > This would be less disruptive regarding library API, and an overall > simpler change. But it would prevent to use the same btf dumper to > dump types with and without attribute definitions. Not sure if that > matters much in practice. > > Thoughts? I think that generating attributes explicitly is fine. I also think that moving '.record_attrs_str' to 'btf_dump_opts' is preferable, in order to avoid adding new API functions. Could you please add a doc-string somewhere saying that ".record_attrs_str" applies to 'struct' and 'union'? Spent some time reading clang to verify that this is the case for 'applies_to=record' and it is. (build/tools/clang/include/clang/Sema/AttrParsedAttrImpl.inc, function checkAttributeMatchRuleAppliesTo(), case for attr::SubjectMatchRule_record). [...]
On Fri, 2024-05-03 at 13:36 -0700, Eduard Zingerman wrote: > On Fri, 2024-05-03 at 13:18 +0200, Jose E. Marchesi wrote: > [...] > > > This patch modifies bpftool in order to, instead of using the pragmas, > > define ATTR_PRESERVE_ACCESS_INDEX to conditionally expand to the CO-RE > > attribute: > > > > #ifndef __VMLINUX_H__ > > #define __VMLINUX_H__ > > > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > > #define ATTR_PRESERVE_ACCESS_INDEX __attribute__((preserve_access_index)) > > #else > > #define ATTR_PRESERVE_ACCESS_INDEX > > #endif > > Nit: maybe swap the branches to avoid double negation? > > > > > [... type definitions generated from kernel BTF ... ] > > > > #undef ATTR_PRESERVE_ACCESS_INDEX > > > > and then the new btf_dump__dump_type_with_opts is used with options > > specifying that we wish to have struct type attributes: > > > > DECLARE_LIBBPF_OPTS(btf_dump_type_opts, opts); > > [...] > > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; > > [...] > > err = btf_dump__dump_type_with_opts(d, root_type_ids[i], &opts); > > > > This is a RFC because introducing a new libbpf public function > > btf_dump__dump_type_with_opts may not be desirable. > > > > An alternative could be to, instead of passing the record_attrs_str > > option in a btf_dump_type_opts, pass it in the global dumper's option > > btf_dump_opts: > > > > DECLARE_LIBBPF_OPTS(btf_dump_opts, opts); > > [...] > > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; > > [...] > > d = btf_dump__new(btf, btf_dump_printf, NULL, &opts); > > [...] > > err = btf_dump__dump_type(d, root_type_ids[i]); > > > > This would be less disruptive regarding library API, and an overall > > simpler change. But it would prevent to use the same btf dumper to > > dump types with and without attribute definitions. Not sure if that > > matters much in practice. > > > > Thoughts? > > I think that generating attributes explicitly is fine. > > I also think that moving '.record_attrs_str' to 'btf_dump_opts' is preferable, > in order to avoid adding new API functions. On more argument for making it a part of btf_dump_opts is that btf_dump__dump_type() walks the chain of dependent types, so attribute placement control is not per-type anyways. I also remembered my stalled attempt to emit preserve_static_offset attribute for certain types [1] (need to finish with it). There I needed to attach attributes to a dozen specific types. [1] https://lore.kernel.org/bpf/20231220133411.22978-3-eddyz87@gmail.com/ So, I think that it would be better if '.record_attrs_str' would be a callback accepting the name of the type and it's kind. Wdyt? [...]
On Fri, May 3, 2024 at 2:18 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Fri, 2024-05-03 at 13:36 -0700, Eduard Zingerman wrote: > > On Fri, 2024-05-03 at 13:18 +0200, Jose E. Marchesi wrote: > > [...] > > > > > This patch modifies bpftool in order to, instead of using the pragmas, > > > define ATTR_PRESERVE_ACCESS_INDEX to conditionally expand to the CO-RE > > > attribute: > > > > > > #ifndef __VMLINUX_H__ > > > #define __VMLINUX_H__ > > > > > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > > > #define ATTR_PRESERVE_ACCESS_INDEX __attribute__((preserve_access_index)) > > > #else > > > #define ATTR_PRESERVE_ACCESS_INDEX > > > #endif > > > > Nit: maybe swap the branches to avoid double negation? > > > > > > > > [... type definitions generated from kernel BTF ... ] > > > > > > #undef ATTR_PRESERVE_ACCESS_INDEX > > > > > > and then the new btf_dump__dump_type_with_opts is used with options > > > specifying that we wish to have struct type attributes: > > > > > > DECLARE_LIBBPF_OPTS(btf_dump_type_opts, opts); > > > [...] > > > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; > > > [...] > > > err = btf_dump__dump_type_with_opts(d, root_type_ids[i], &opts); > > > > > > This is a RFC because introducing a new libbpf public function > > > btf_dump__dump_type_with_opts may not be desirable. > > > > > > An alternative could be to, instead of passing the record_attrs_str > > > option in a btf_dump_type_opts, pass it in the global dumper's option > > > btf_dump_opts: > > > > > > DECLARE_LIBBPF_OPTS(btf_dump_opts, opts); > > > [...] > > > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; > > > [...] > > > d = btf_dump__new(btf, btf_dump_printf, NULL, &opts); > > > [...] > > > err = btf_dump__dump_type(d, root_type_ids[i]); > > > > > > This would be less disruptive regarding library API, and an overall > > > simpler change. But it would prevent to use the same btf dumper to > > > dump types with and without attribute definitions. Not sure if that > > > matters much in practice. > > > > > > Thoughts? > > > > I think that generating attributes explicitly is fine. > > > > I also think that moving '.record_attrs_str' to 'btf_dump_opts' is preferable, > > in order to avoid adding new API functions. > > On more argument for making it a part of btf_dump_opts is that > btf_dump__dump_type() walks the chain of dependent types, > so attribute placement control is not per-type anyways. And that's very unfortunate, which makes this not a good option, IMO. > > I also remembered my stalled attempt to emit preserve_static_offset > attribute for certain types [1] (need to finish with it). > There I needed to attach attributes to a dozen specific types. > > [1] https://lore.kernel.org/bpf/20231220133411.22978-3-eddyz87@gmail.com/ > > So, I think that it would be better if '.record_attrs_str' would be a > callback accepting the name of the type and it's kind. Wdyt? I think if we are talking about the current API, then extending it with some pre/post type callback would solve this specific problem (but even then it feels dirty, because of "this callback is called after } but before ," sadness). I really dislike callbacks as part of public APIs like this. It feels like the user has to have control and the library should provide building blocks. So how about an alternative view on this problem. What if we add an API that will sort types in "C type system" order, i.e., it will return a sequence of BTF type ID + a flag whether it's a full BTF type definition or forward declaration only for that type. And then, separately, instead of btf_dump__dump_type() API that emits type *and* all its dependencies (unless they were already emitted, it's very stateful), we'll have an analogous API that will emit a full definition of one isolated btf_type (and no dependencies). The user will need to add semicolons after each type (and empty lines and stuff like that, probably), but they will also have control over appending/prepending any extra attributes and whatnot (or #ifdef guards). Also, when we have this API, we'll have all the necessary building blocks to finally be able to emit only types for module BTF without duplicated types from vmlinux.h (under assumption that vmlinux.h will be included before that). Libbpf will return fully sorted type order, including vmlinux BTF types, but bpftool (or whoever is using this API) will be smart in ignoring those types and/or emitting just forward declaration for them. With the decomposition into sort + emit string representation, it's now trivial to use in this flexible way. Thoughts? > > [...]
> On Fri, May 3, 2024 at 2:18 PM Eduard Zingerman <eddyz87@gmail.com> wrote: >> >> On Fri, 2024-05-03 at 13:36 -0700, Eduard Zingerman wrote: >> > On Fri, 2024-05-03 at 13:18 +0200, Jose E. Marchesi wrote: >> > [...] >> > >> > > This patch modifies bpftool in order to, instead of using the pragmas, >> > > define ATTR_PRESERVE_ACCESS_INDEX to conditionally expand to the CO-RE >> > > attribute: >> > > >> > > #ifndef __VMLINUX_H__ >> > > #define __VMLINUX_H__ >> > > >> > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX >> > > #define ATTR_PRESERVE_ACCESS_INDEX __attribute__((preserve_access_index)) >> > > #else >> > > #define ATTR_PRESERVE_ACCESS_INDEX >> > > #endif >> > >> > Nit: maybe swap the branches to avoid double negation? >> > >> > > >> > > [... type definitions generated from kernel BTF ... ] >> > > >> > > #undef ATTR_PRESERVE_ACCESS_INDEX >> > > >> > > and then the new btf_dump__dump_type_with_opts is used with options >> > > specifying that we wish to have struct type attributes: >> > > >> > > DECLARE_LIBBPF_OPTS(btf_dump_type_opts, opts); >> > > [...] >> > > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; >> > > [...] >> > > err = btf_dump__dump_type_with_opts(d, root_type_ids[i], &opts); >> > > >> > > This is a RFC because introducing a new libbpf public function >> > > btf_dump__dump_type_with_opts may not be desirable. >> > > >> > > An alternative could be to, instead of passing the record_attrs_str >> > > option in a btf_dump_type_opts, pass it in the global dumper's option >> > > btf_dump_opts: >> > > >> > > DECLARE_LIBBPF_OPTS(btf_dump_opts, opts); >> > > [...] >> > > opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; >> > > [...] >> > > d = btf_dump__new(btf, btf_dump_printf, NULL, &opts); >> > > [...] >> > > err = btf_dump__dump_type(d, root_type_ids[i]); >> > > >> > > This would be less disruptive regarding library API, and an overall >> > > simpler change. But it would prevent to use the same btf dumper to >> > > dump types with and without attribute definitions. Not sure if that >> > > matters much in practice. >> > > >> > > Thoughts? >> > >> > I think that generating attributes explicitly is fine. >> > >> > I also think that moving '.record_attrs_str' to 'btf_dump_opts' is preferable, >> > in order to avoid adding new API functions. >> >> On more argument for making it a part of btf_dump_opts is that >> btf_dump__dump_type() walks the chain of dependent types, >> so attribute placement control is not per-type anyways. > > And that's very unfortunate, which makes this not a good option, IMO. Indeed. But for the specific case case of preserve_access_info and vmlinux.h, having the attribute applied to all types (both directly referred and indirectly dependent) is actually what is required, isn't it? That is what the current implicity push-attribute clang pragma does. I have sent a tentative patch that adds the `record_attrs_str' configuration parameter to the btf_dump_opts, incorporating a few changes after Eduard's suggestions regarding avoiding double negations and docstrings. >> I also remembered my stalled attempt to emit preserve_static_offset >> attribute for certain types [1] (need to finish with it). >> There I needed to attach attributes to a dozen specific types. >> >> [1] https://lore.kernel.org/bpf/20231220133411.22978-3-eddyz87@gmail.com/ >> >> So, I think that it would be better if '.record_attrs_str' would be a >> callback accepting the name of the type and it's kind. Wdyt? > > I think if we are talking about the current API, then extending it > with some pre/post type callback would solve this specific problem > (but even then it feels dirty, because of "this callback is called > after } but before ," sadness). I really dislike callbacks as part of > public APIs like this. It feels like the user has to have control and > the library should provide building blocks. > > So how about an alternative view on this problem. What if we add an > API that will sort types in "C type system" order, i.e., it will > return a sequence of BTF type ID + a flag whether it's a full BTF type > definition or forward declaration only for that type. And then, > separately, instead of btf_dump__dump_type() API that emits type *and* > all its dependencies (unless they were already emitted, it's very > stateful), we'll have an analogous API that will emit a full > definition of one isolated btf_type (and no dependencies). > > The user will need to add semicolons after each type (and empty lines > and stuff like that, probably), but they will also have control over > appending/prepending any extra attributes and whatnot (or #ifdef > guards). > > Also, when we have this API, we'll have all the necessary building > blocks to finally be able to emit only types for module BTF without > duplicated types from vmlinux.h (under assumption that vmlinux.h will > be included before that). Libbpf will return fully sorted type order, > including vmlinux BTF types, but bpftool (or whoever is using this > API) will be smart in ignoring those types and/or emitting just > forward declaration for them. > > With the decomposition into sort + emit string representation, it's > now trivial to use in this flexible way. > > Thoughts? I am not familiar with the particular use cases, but generally speaking separating sorting and emission makes sense to me. I would also prefer that to iterators. > > >> >> [...]
On Fri, 2024-05-03 at 15:14 -0700, Andrii Nakryiko wrote: [...] > With the decomposition into sort + emit string representation, it's > now trivial to use in this flexible way. > > Thoughts? Compared to callbacks for attributes this adds the following: - ability to filter-out some types; - ability to add some pre-processor statements between specific types. Compared to callbacks for attributes this lacks the following: - ability to specify attributes for nested anonymous types (not important for preserve_access_index). As I ranted in the off-list discussion, full flexibility is achievable only with some kind of C AST: - an API to produce such an AST; - an API to modify AST where necessary; - an API to serialize the AST as C code. Adding such AST to libbpf is completely out of scope. So, what we are left with is a set of half-measures: 1. a fixed attribute string as in Jose's patch; 2. a callback before printing attributes as suggested by me; 3. two API functions to get a sorted list of types and to print a type as suggested by Andrii. And a set of use-cases: a. capability to add some attribute for all structs; b. capability to add some attribute for specific types; c. capability to filter printed types. (1) covers only (a); (2) covers (a,b); (3) covers (a,b,c). Still, (3) has limited flexibility and I do not exclude the necessity to add some sort of (2) in the future. On the other hand, necessity to modify dump output arises not often, so I think that (3) is preferable at the moment.
On Sat, 2024-05-04 at 23:09 +0200, Jose E. Marchesi wrote: [...] > I have sent a tentative patch that adds the `record_attrs_str' > configuration parameter to the btf_dump_opts, incorporating a few > changes after Eduard's suggestions regarding avoiding double negations > and docstrings. [...] > I am not familiar with the particular use cases, but generally speaking > separating sorting and emission makes sense to me. I would also prefer > that to iterators. Hi Jose, I've discussed this issue with Andrii today, and we decided that we want to proceed with API changes, that introduce two functions: one for sorting ids, one for printing single type. I can do this change on Tue or Wed, if that is ok with you. Thanks, Eduard
> On Sat, 2024-05-04 at 23:09 +0200, Jose E. Marchesi wrote: > > [...] > >> I have sent a tentative patch that adds the `record_attrs_str' >> configuration parameter to the btf_dump_opts, incorporating a few >> changes after Eduard's suggestions regarding avoiding double negations >> and docstrings. > > [...] > >> I am not familiar with the particular use cases, but generally speaking >> separating sorting and emission makes sense to me. I would also prefer >> that to iterators. > > Hi Jose, > > I've discussed this issue with Andrii today, > and we decided that we want to proceed with API changes, > that introduce two functions: one for sorting ids, > one for printing single type. > > I can do this change on Tue or Wed, if that is ok with you. Sure, thank you.
On Mon, May 6, 2024 at 12:10 PM Jose E. Marchesi <jose.marchesi@oracle.com> wrote: > > > > On Sat, 2024-05-04 at 23:09 +0200, Jose E. Marchesi wrote: > > > > [...] > > > >> I have sent a tentative patch that adds the `record_attrs_str' > >> configuration parameter to the btf_dump_opts, incorporating a few > >> changes after Eduard's suggestions regarding avoiding double negations > >> and docstrings. > > > > [...] > > > >> I am not familiar with the particular use cases, but generally speaking > >> separating sorting and emission makes sense to me. I would also prefer > >> that to iterators. > > > > Hi Jose, > > > > I've discussed this issue with Andrii today, > > and we decided that we want to proceed with API changes, > > that introduce two functions: one for sorting ids, > > one for printing single type. > > > > I can do this change on Tue or Wed, if that is ok with you. > > Sure, thank you. For now, for GCC-BPF-specific case we can add -DBPF_NO_PRESERVE_ACCESS_INDEX in Makefile, if that's blocking GCC-BPF effort.
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c index 91fcb75babe3..e563b60fedd0 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -463,6 +463,7 @@ static void __printf(2, 0) btf_dump_printf(void *ctx, static int dump_btf_c(const struct btf *btf, __u32 *root_type_ids, int root_type_cnt) { + DECLARE_LIBBPF_OPTS(btf_dump_type_opts, opts); struct btf_dump *d; int err = 0, i; @@ -474,12 +475,17 @@ static int dump_btf_c(const struct btf *btf, printf("#define __VMLINUX_H__\n"); printf("\n"); printf("#ifndef BPF_NO_PRESERVE_ACCESS_INDEX\n"); - printf("#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)\n"); + printf("#define ATTR_PRESERVE_ACCESS_INDEX __attribute__((preserve_access_index))\n"); + printf("#else\n"); + printf("#define ATTR_PRESERVE_ACCESS_INDEX\n"); printf("#endif\n\n"); + printf("\n"); + + opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; if (root_type_cnt) { for (i = 0; i < root_type_cnt; i++) { - err = btf_dump__dump_type(d, root_type_ids[i]); + err = btf_dump__dump_type_with_opts(d, root_type_ids[i], &opts); if (err) goto done; } @@ -487,15 +493,13 @@ static int dump_btf_c(const struct btf *btf, int cnt = btf__type_cnt(btf); for (i = 1; i < cnt; i++) { - err = btf_dump__dump_type(d, i); + err = btf_dump__dump_type_with_opts(d, i, &opts); if (err) goto done; } } - printf("#ifndef BPF_NO_PRESERVE_ACCESS_INDEX\n"); - printf("#pragma clang attribute pop\n"); - printf("#endif\n"); + printf("#undef ATTR_PRESERVE_ACCESS_INDEX\n"); printf("\n"); printf("#endif /* __VMLINUX_H__ */\n"); diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h index 8e6880d91c84..802ec856f824 100644 --- a/tools/lib/bpf/btf.h +++ b/tools/lib/bpf/btf.h @@ -249,6 +249,17 @@ LIBBPF_API void btf_dump__free(struct btf_dump *d); LIBBPF_API int btf_dump__dump_type(struct btf_dump *d, __u32 id); +struct btf_dump_type_opts { + /* size of this struct, for forward/backward compatibility */ + size_t sz; + const char *record_attrs_str; + size_t :0; +}; +#define btf_dump_type_opts__last_field record_attrs_str + +LIBBPF_API int btf_dump__dump_type_with_opts(struct btf_dump *d, __u32 id, + const struct btf_dump_type_opts *opts); + struct btf_dump_emit_type_decl_opts { /* size of this struct, for forward/backward compatiblity */ size_t sz; diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index 5dbca76b953f..f4ef42d0b392 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -116,6 +116,11 @@ struct btf_dump { * data for typed display; allocated if needed. */ struct btf_dump_data *typed_dump; + /* + * string with C attributes to be used in record + * types. + */ + const char *record_attrs_str; }; static size_t str_hash_fn(long key, void *ctx) @@ -296,6 +301,20 @@ int btf_dump__dump_type(struct btf_dump *d, __u32 id) return 0; } +/* This is like btf_dump__dump_type but it gets a set of options. */ +int btf_dump__dump_type_with_opts(struct btf_dump *d, __u32 id, + const struct btf_dump_type_opts *opts) +{ + int ret; + + if (!OPTS_VALID(opts, btf_dump_type_opts)) + return libbpf_err(-EINVAL); + d->record_attrs_str = OPTS_GET(opts, record_attrs_str, 0); + ret = btf_dump__dump_type(d, id); + d->record_attrs_str = NULL; + return ret; +} + /* * Mark all types that are referenced from any other type. This is used to * determine top-level anonymous enums that need to be emitted as an @@ -1024,6 +1043,8 @@ static void btf_dump_emit_struct_def(struct btf_dump *d, } if (packed) btf_dump_printf(d, " __attribute__((packed))"); + if (d->record_attrs_str) + btf_dump_printf(d, " %s", d->record_attrs_str); } static const char *missing_base_types[][2] = { diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index c1ce8aa3520b..9e56de31c5be 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -422,4 +422,5 @@ LIBBPF_1.5.0 { bpf_program__attach_sockmap; ring__consume_n; ring_buffer__consume_n; + btf_dump__dump_type_with_opts; } LIBBPF_1.4.0;
The vmlinux.h file generated by bpftool makes use of compiler pragmas in order to install the CO-RE preserve_access_index in all the struct types derived from the BTF info: #ifndef __VMLINUX_H__ #define __VMLINUX_H__ #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute push (__attribute__((preserve_access_index)), apply_t = record #endif [... type definitions generated from kernel BTF ... ] #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute pop #endif The `clang attribute push/pop' pragmas are specific to clang/llvm and are not supported by GCC. This patch modifies bpftool in order to, instead of using the pragmas, define ATTR_PRESERVE_ACCESS_INDEX to conditionally expand to the CO-RE attribute: #ifndef __VMLINUX_H__ #define __VMLINUX_H__ #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #define ATTR_PRESERVE_ACCESS_INDEX __attribute__((preserve_access_index)) #else #define ATTR_PRESERVE_ACCESS_INDEX #endif [... type definitions generated from kernel BTF ... ] #undef ATTR_PRESERVE_ACCESS_INDEX and then the new btf_dump__dump_type_with_opts is used with options specifying that we wish to have struct type attributes: DECLARE_LIBBPF_OPTS(btf_dump_type_opts, opts); [...] opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; [...] err = btf_dump__dump_type_with_opts(d, root_type_ids[i], &opts); This is a RFC because introducing a new libbpf public function btf_dump__dump_type_with_opts may not be desirable. An alternative could be to, instead of passing the record_attrs_str option in a btf_dump_type_opts, pass it in the global dumper's option btf_dump_opts: DECLARE_LIBBPF_OPTS(btf_dump_opts, opts); [...] opts.record_attrs_str = "ATTR_PRESERVE_ACCESS_INDEX"; [...] d = btf_dump__new(btf, btf_dump_printf, NULL, &opts); [...] err = btf_dump__dump_type(d, root_type_ids[i]); This would be less disruptive regarding library API, and an overall simpler change. But it would prevent to use the same btf dumper to dump types with and without attribute definitions. Not sure if that matters much in practice. Thoughts? Tested in bpf-next master. No regressions. Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com> Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com> Cc: Yonghong Song <yonghong.song@linux.dev> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com> Cc: david.faust@oracle.com Cc: cupertino.miranda@oracle.com --- tools/bpf/bpftool/btf.c | 16 ++++++++++------ tools/lib/bpf/btf.h | 11 +++++++++++ tools/lib/bpf/btf_dump.c | 21 +++++++++++++++++++++ tools/lib/bpf/libbpf.map | 1 + 4 files changed, 43 insertions(+), 6 deletions(-)