[RFC bpf-next] bpf: avoid clang-specific push/pop attribute pragmas in bpftool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
Cc: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>
Cc: Yonghong Song <yonghong.song@xxxxxxxxx>
Cc: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>
Cc: david.faust@xxxxxxxxxx
Cc: cupertino.miranda@xxxxxxxxxx
---
 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





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux