From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: bpf@vger.kernel.org
Cc: "Jose E . Marchesi" <jose.marchesi@oracle.com>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Yonghong Song <yonghong.song@linux.dev>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>,
david.faust@oracle.com, cupertino.miranda@oracle.com
Subject: [RFC bpf-next] bpf: avoid clang-specific push/pop attribute pragmas in bpftool
Date: Fri, 3 May 2024 13:18:36 +0200 [thread overview]
Message-ID: <20240503111836.25275-1-jose.marchesi@oracle.com> (raw)
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(-)
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;
--
2.30.2
next reply other threads:[~2024-05-03 11:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 11:18 Jose E. Marchesi [this message]
2024-05-03 20:36 ` [RFC bpf-next] bpf: avoid clang-specific push/pop attribute pragmas in bpftool Eduard Zingerman
2024-05-03 21:18 ` Eduard Zingerman
2024-05-03 22:14 ` Andrii Nakryiko
2024-05-04 21:09 ` Jose E. Marchesi
2024-05-06 18:55 ` Eduard Zingerman
2024-05-06 19:10 ` Jose E. Marchesi
2024-05-06 21:35 ` Andrii Nakryiko
2024-05-06 6:26 ` Eduard Zingerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240503111836.25275-1-jose.marchesi@oracle.com \
--to=jose.marchesi@oracle.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=cupertino.miranda@oracle.com \
--cc=david.faust@oracle.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).