[PATCH RFC bpf-next 0/3] Revamp bpf_attr and make it easier to evolve

[PATCH RFC bpf-next 0/3] Revamp bpf_attr and make it easier to evolve

[Andrii Nakryiko]

RFC patch set revamping anonymous substructs of union bpf_attr, which would
allow nicer and more coherent evolution of bpf() syscall arguments, especially
for commands like BPF_MAP_CREATE and BPF_PROG_LOAD. See patch #1 for
justification and more details. Patch #2 demonstrates how straightforward it
is to switch to new-style substricts in kernel code (and keep in mind that
this is optional until we need some new field for a given command, so we can
do it completely asynchronously from landing bpf_attr changes themselves).
Patch #3 shows also similar libbpf changes, except for libbpf single patches
switches over entire libbpf code base to new-style substructs (except
skel_internal.h, due to concerns that users might be reliant on outdated
system-wide linux/bpf.h UAPI header).

Andrii Nakryiko (3):
  bpf: revamp bpf_attr and name each command's field and substruct
  bpf: use new named bpf_attr substructs for few commands
  libbpf: use new bpf_xxx_attr structs for bpf() commands

 include/uapi/linux/bpf.h        | 235 +++++++++++++++++----
 kernel/bpf/syscall.c            |  77 ++++---
 tools/include/uapi/linux/bpf.h  | 235 +++++++++++++++++----
 tools/lib/bpf/bpf.c             | 355 ++++++++++++++++----------------
 tools/lib/bpf/gen_loader.c      |  78 +++----
 tools/lib/bpf/libbpf.c          |   4 +-
 tools/lib/bpf/libbpf_internal.h |   2 +-
 7 files changed, 641 insertions(+), 345 deletions(-)

-- 
2.34.1

[PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct

[Andrii Nakryiko]

union bpf_attr, used to provide input argument to bpf() syscall, has
per-command (or sometimes a per-group of commands) substruct section
with relevant arguments.

Unfortunately, as it evolved, we ended up with a situation where some
commands use anonymous struct field and some other named fields. Member
of anonymous field structs are basically in a "global namespace" and
would potentially conflict with fields in other such anonymous field
substruct. This means that commands like BPF_PROG_LOAD, BPF_BTF_LOAD,
BPF_MAP_CREATE, etc have to avoid naming fields the same, even if it
makes total sense. E.g., just because of this, BPF_BTF_LOAD couldn't
have uniformly and logically named `log_level`, `log_size`, and
`log_buf` fields, and we had to "namespace" them as `btf_log_level`,
`btf_log_size`, and `btf_log_buf`. If some other command relying on
anonymous field struct would like to add logging support, we'd need to
use yet another naming prefix to avoid conflicts with BPF_PROG_LOAD.

This is quite suboptimal to say the least, espcially when needing to add
some conceptually the same field across multiple commands. One such
example might be "token_fd" that would represent a BPF token object
(patches not posted yet, just an example and motivating case for these
changes). It would be great to just add `__u32 token_fd;` across
multiple commands with uniform name, but with current state of `union
bpf_attr` this is impossible, and we'll need to do `prog_load_token_fd`
vs `map_create_token_fd` vs `btf_load_token_fd`, etc.

This patch is attempting to rectify this situation and set bpf_attr and
bpf() syscall for cleaner evolution story. To that end, each command or
group of similar commands always use named fields. E.g., BPF_PROG_CREATE
will have `prog_create` field with a struct that contains all relevant
attributes. Of course, we can't break backwards compatibility, so to
make all the old code both compile with no changes and keep working with
no changes, we keep all the currently anonymous field structs, just move
them to the end into "LEGACY" section of bpf_attr. In their stead,
though, we add named field with exactly the same memory layout and
(mostly) the same field names. This makes sure that layout is exactly
the same, but gives each command its own naming namespace that will
allow to make naming decisions independent from any other command.

Another aspect is that these per-command structs are not anonymous
anymore, and they are actually named now. This significantly minimizes
amount of user-space code changes for applications and libraries that
would like to use new named field substructs (e.g., like libbpf in the
next patch). But this is not even the main benefit. Having each
substruct named allows to reduce accidental use of wrong fields that
don't belong to a given command. Follow up kernel-side patch
demonstrates how we can modify kernel code to ensure that we only work
with per-command part of bpf_attr, instead of hoping to never mix up
field names.

Given we keep source code and ABI backwards compat, we can do gradual
migration of both kernel and user-space code to new named substructs,
where necessary and/or benefitial.

While at it, I've used an opportunity to split some commands that just
happened to be "colocated" in terms of which substruct was used, but
that are conceptually completely separate. E.g., BPF_MAP_FREEZE gets its
own tiny struct. GET_FD_BY_ID group is split from OBJ_GET_NEXT_ID
command. I've also split PROG_ATTACH from PROG_DETACH, OBJ_PIN from
OBJ_GET, etc.

Also some fields were renamed to make their use clearer (e.g., for
PROG_DETACH `attach_bpf_fd` made no sense). For GET_FD_BY_ID union of
"link_id", "prog_id", "map_id", and "btf_id" were unified into "obj_id",
which matches struct's `bpf_obj_fd_by_id_attr` name nicely.

For a few commands we had a mix of "flags", "file_flags", and
"open_flags". What's worse, "file_flags" for OBJ_PIN/OBJ_GET is already
a misnomer as it has more than just file permissions flags. So for few
of those commands where there was single flags field, I unified the
naming to be just "flags".

For OBJ_GET command "bpf_fd" was meaningless, so I renamed it to
"__reserved" and we could reuse it later, if necessary. For
MAP_{LOOKUP,DELETE,etc}_ELEM, `next_key` field was meaningless and
dropped. It stayed in split out bpf_map_next_key_attr struct, which had
`value` dropped.

And there were a bunch of other similar changes. Please take a thorough
look and suggest more changes or which changes to drop. I'm not married
to any of them, it just felt like a good improvement.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 include/uapi/linux/bpf.h       | 235 +++++++++++++++++++++++++++------
 kernel/bpf/syscall.c           |  40 +++---
 tools/include/uapi/linux/bpf.h | 235 +++++++++++++++++++++++++++------
 3 files changed, 405 insertions(+), 105 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9273c654743c..83066cc0f24b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1320,7 +1320,8 @@ struct bpf_stack_build_id {
 #define BPF_OBJ_NAME_LEN 16U
 
 union bpf_attr {
-	struct { /* anonymous struct used by BPF_MAP_CREATE command */
+	/* BPF_MAP_CREATE command */
+	struct bpf_map_create_attr {
 		__u32	map_type;	/* one of enum bpf_map_type */
 		__u32	key_size;	/* size of key in bytes */
 		__u32	value_size;	/* size of value in bytes */
@@ -1348,19 +1349,30 @@ union bpf_attr {
 		 * to using 5 hash functions).
 		 */
 		__u64	map_extra;
-	};
+	} map_create;
 
-	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
+	/* BPF_MAP_{LOOKUP,UPDATE,DELETE,LOOKUP_AND_DELETE}_ELEM commands */
+	struct bpf_map_elem_attr {
 		__u32		map_fd;
 		__aligned_u64	key;
-		union {
-			__aligned_u64 value;
-			__aligned_u64 next_key;
-		};
+		__aligned_u64	value;
 		__u64		flags;
-	};
+	} map_elem;
 
-	struct { /* struct used by BPF_MAP_*_BATCH commands */
+	/* BPF_MAP_GET_NEXT_KEY command */
+	struct bpf_map_next_key_attr {
+		__u32		map_fd;
+		__aligned_u64	key;
+		__aligned_u64	next_key;
+	} map_next_key;
+
+	/* BPF_MAP_FREEZE command */
+	struct bpf_map_freeze_attr {
+		__u32		map_fd;
+	} map_freeze;
+
+	/* BPF_MAP_{LOOKUP,UPDATE,DELETE,LOOKUP_AND_DELETE}_BATCH commands */
+	struct bpf_map_batch_attr {
 		__aligned_u64	in_batch;	/* start batch,
 						 * NULL to start from beginning
 						 */
@@ -1377,7 +1389,8 @@ union bpf_attr {
 		__u64		flags;
 	} batch;
 
-	struct { /* anonymous struct used by BPF_PROG_LOAD command */
+	/* BPF_PROG_LOAD command */
+	struct bpf_prog_load_attr {
 		__u32		prog_type;	/* one of enum bpf_prog_type */
 		__u32		insn_cnt;
 		__aligned_u64	insns;
@@ -1417,12 +1430,13 @@ union bpf_attr {
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
 		__u32		log_true_size;
-	};
+	} prog_load;
 
-	struct { /* anonymous struct used by BPF_OBJ_* commands */
+	/* BPF_OBJ_PIN command */
+	struct bpf_obj_pin_attr {
 		__aligned_u64	pathname;
 		__u32		bpf_fd;
-		__u32		file_flags;
+		__u32		flags;
 		/* Same as dirfd in openat() syscall; see openat(2)
 		 * manpage for details of path FD and pathname semantics;
 		 * path_fd should accompanied by BPF_F_PATH_FD flag set in
@@ -1430,9 +1444,24 @@ union bpf_attr {
 		 * if BPF_F_PATH_FD flag is not set, AT_FDCWD is assumed.
 		 */
 		__s32		path_fd;
-	};
+	} obj_pin;
 
-	struct { /* anonymous struct used by BPF_PROG_ATTACH/DETACH commands */
+	/* BPF_OBJ_GET command */
+	struct bpf_obj_get_attr {
+		__aligned_u64	pathname;
+		__u32		__reserved;
+		__u32		flags;
+		/* Same as dirfd in openat() syscall; see openat(2)
+		 * manpage for details of path FD and pathname semantics;
+		 * path_fd should accompanied by BPF_F_PATH_FD flag set in
+		 * file_flags field, otherwise it should be set to zero;
+		 * if BPF_F_PATH_FD flag is not set, AT_FDCWD is assumed.
+		 */
+		__s32		path_fd;
+	} obj_get;
+
+	/* BPF_PROG_ATTACH command */
+	struct bpf_prog_attach_attr {
 		__u32		target_fd;	/* container object to attach to */
 		__u32		attach_bpf_fd;	/* eBPF program to attach */
 		__u32		attach_type;
@@ -1441,9 +1470,16 @@ union bpf_attr {
 						 * program to replace if
 						 * BPF_F_REPLACE is used
 						 */
-	};
+	} prog_attach;
 
-	struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
+	/* BPF_PROG_DETACH command */
+	struct bpf_prog_detach_attr {
+		__u32		target_fd;	/* container object to attach to */
+		__u32		prog_fd;	/* eBPF program to detach */
+		__u32		attach_type;
+	} prog_detach;
+
+	struct bpf_prog_run_attr { /* anonymous struct used by BPF_PROG_TEST_RUN command */
 		__u32		prog_fd;
 		__u32		retval;
 		__u32		data_size_in;	/* input: len of data_in */
@@ -1467,25 +1503,26 @@ union bpf_attr {
 		__u32		batch_size;
 	} test;
 
-	struct { /* anonymous struct used by BPF_*_GET_*_ID */
-		union {
-			__u32		start_id;
-			__u32		prog_id;
-			__u32		map_id;
-			__u32		btf_id;
-			__u32		link_id;
-		};
+	/* BPF_{MAP,PROG,BTF,LINK}_GET_FD_BY_ID commands */
+	struct bpf_obj_fd_by_id_attr {
+		__u32		obj_id;
+		__u32		__reserved;
+		__u32		flags;
+	} obj_fd_by_id;
+
+	/* BPF_OBJ_GET_NEXT_ID command */
+	struct bpf_obj_next_id_attr {
+		__u32		start_id;
 		__u32		next_id;
-		__u32		open_flags;
-	};
+	} obj_next_id;
 
-	struct { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
+	struct bpf_obj_info_by_fd_attr { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
 		__u32		bpf_fd;
 		__u32		info_len;
 		__aligned_u64	info;
 	} info;
 
-	struct { /* anonymous struct used by BPF_PROG_QUERY command */
+	struct bpf_prog_query_attr { /* anonymous struct used by BPF_PROG_QUERY command */
 		__u32		target_fd;	/* container object to query */
 		__u32		attach_type;
 		__u32		query_flags;
@@ -1498,25 +1535,26 @@ union bpf_attr {
 		__aligned_u64	prog_attach_flags;
 	} query;
 
-	struct { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
+	struct bpf_raw_tp_open_attr { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
 		__u64 name;
 		__u32 prog_fd;
 	} raw_tracepoint;
 
-	struct { /* anonymous struct for BPF_BTF_LOAD */
+	/* BPF_BTF_LOAD command */
+	struct bpf_btf_load_attr {
 		__aligned_u64	btf;
-		__aligned_u64	btf_log_buf;
+		__aligned_u64	log_buf;
 		__u32		btf_size;
-		__u32		btf_log_size;
-		__u32		btf_log_level;
+		__u32		log_size;
+		__u32		log_level;
 		/* output: actual total log contents size (including termintaing zero).
 		 * It could be both larger than original log_size (if log was
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
-		__u32		btf_log_true_size;
-	};
+		__u32		log_true_size;
+	} btf_load;
 
-	struct {
+	struct bpf_task_fd_query_attr {
 		__u32		pid;		/* input: pid */
 		__u32		fd;		/* input: fd */
 		__u32		flags;		/* input: flags */
@@ -1532,7 +1570,7 @@ union bpf_attr {
 		__u64		probe_addr;	/* output: probe_addr */
 	} task_fd_query;
 
-	struct { /* struct used by BPF_LINK_CREATE command */
+	struct bpf_link_create_attr { /* struct used by BPF_LINK_CREATE command */
 		union {
 			__u32		prog_fd;	/* eBPF program to attach */
 			__u32		map_fd;		/* struct_ops to attach */
@@ -1581,7 +1619,7 @@ union bpf_attr {
 		};
 	} link_create;
 
-	struct { /* struct used by BPF_LINK_UPDATE command */
+	struct bpf_link_update_attr { /* struct used by BPF_LINK_UPDATE command */
 		__u32		link_fd;	/* link fd */
 		union {
 			/* new program fd to update link with */
@@ -1602,25 +1640,136 @@ union bpf_attr {
 		};
 	} link_update;
 
-	struct {
+	struct bpf_link_detach_attr {
 		__u32		link_fd;
 	} link_detach;
 
-	struct { /* struct used by BPF_ENABLE_STATS command */
+	struct bpf_enable_stats_attr { /* struct used by BPF_ENABLE_STATS command */
 		__u32		type;
 	} enable_stats;
 
-	struct { /* struct used by BPF_ITER_CREATE command */
+	struct bpf_iter_create_attr { /* struct used by BPF_ITER_CREATE command */
 		__u32		link_fd;
 		__u32		flags;
 	} iter_create;
 
-	struct { /* struct used by BPF_PROG_BIND_MAP command */
+	struct bpf_prog_bind_map_attr { /* struct used by BPF_PROG_BIND_MAP command */
 		__u32		prog_fd;
 		__u32		map_fd;
 		__u32		flags;		/* extra flags */
 	} prog_bind_map;
 
+	/*
+	 * LEGACY anonymous substructs, for backwards compatibility.
+	 * Each of the below anonymous substructs are ABI compatible with one
+	 * of the above named substructs. Please use named substructs.
+	 */
+
+	struct { /* legacy BPF_MAP_CREATE attrs, use .map_create instead */
+		__u32	map_type;
+		__u32	key_size;
+		__u32	value_size;
+		__u32	max_entries;
+		__u32	map_flags;
+		__u32	inner_map_fd;
+		__u32	numa_node;
+		char	map_name[BPF_OBJ_NAME_LEN];
+		__u32	map_ifindex;
+		__u32	btf_fd;
+		__u32	btf_key_type_id;
+		__u32	btf_value_type_id;
+		__u32	btf_vmlinux_value_type_id;
+		__u64	map_extra;
+	};
+	/*
+	 * legacy BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY attrs,
+	 * use .map_elem or .get_next_key, respectively, instead
+	 */
+	struct {
+		__u32		map_fd;
+		__aligned_u64	key;
+		union {
+			__aligned_u64 value;
+			__aligned_u64 next_key;
+		};
+		__u64		flags;
+	};
+	struct { /* legacy BPF_PROG_LOAD attrs, use .prog_load instead */
+		__u32		prog_type;
+		__u32		insn_cnt;
+		__aligned_u64	insns;
+		__aligned_u64	license;
+		__u32		log_level;
+		__u32		log_size;
+		__aligned_u64	log_buf;
+		__u32		kern_version;
+		__u32		prog_flags;
+		char		prog_name[BPF_OBJ_NAME_LEN];
+		__u32		prog_ifindex;
+		__u32		expected_attach_type;
+		__u32		prog_btf_fd;
+		__u32		func_info_rec_size;
+		__aligned_u64	func_info;
+		__u32		func_info_cnt;
+		__u32		line_info_rec_size;
+		__aligned_u64	line_info;
+		__u32		line_info_cnt;
+		__u32		attach_btf_id;
+		union {
+			__u32		attach_prog_fd;
+			__u32		attach_btf_obj_fd;
+		};
+		__u32		core_relo_cnt;
+		__aligned_u64	fd_array;
+		__aligned_u64	core_relos;
+		__u32		core_relo_rec_size;
+		__u32		log_true_size;
+	};
+	/* legacy BPF_OBJ_{PIN, GET} attrs, use .obj_pin or .obj_get instead */
+	struct {
+		__aligned_u64	pathname;
+		__u32		bpf_fd;
+		__u32		file_flags;
+		__s32		path_fd;
+	};
+	/*
+	 * legacy BPF_PROG_{ATTACH,DETACH} attrs,
+	 * use .prog_attach or .prog_detach instead
+	 */
+	struct {
+		__u32		target_fd;	/* container object to attach to */
+		__u32		attach_bpf_fd;	/* eBPF program to attach */
+		__u32		attach_type;
+		__u32		attach_flags;
+		__u32		replace_bpf_fd;	/* previously attached eBPF
+						 * program to replace if
+						 * BPF_F_REPLACE is used
+						 */
+	};
+	/*
+	 * legacy BPF_*_GET_FD_BY_ID and BPF_OBJ_GET_NEXT_ID attrs,
+	 * use .obj_fd_by_id or .obj_next_id, respectively, instead
+	 */
+	struct {
+		union {
+			__u32		start_id;
+			__u32		prog_id;
+			__u32		map_id;
+			__u32		btf_id;
+			__u32		link_id;
+		};
+		__u32		next_id;
+		__u32		open_flags;
+	};
+	/* legacy BPF_BTF_LOAD attrs, use .btf_load instead */
+	struct {
+		__aligned_u64	btf;
+		__aligned_u64	btf_log_buf;
+		__u32		btf_size;
+		__u32		btf_log_size;
+		__u32		btf_log_level;
+		__u32		btf_log_true_size;
+	};
 } __attribute__((aligned(8)));
 
 /* The description below is an attempt at providing documentation to eBPF
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index c7f6807215e6..babf1d56c2d9 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1123,7 +1123,7 @@ static int map_check_btf(struct bpf_map *map, const struct btf *btf,
 	return ret;
 }
 
-#define BPF_MAP_CREATE_LAST_FIELD map_extra
+#define BPF_MAP_CREATE_LAST_FIELD map_create.map_extra
 /* called via syscall */
 static int map_create(union bpf_attr *attr)
 {
@@ -1352,7 +1352,7 @@ static void *___bpf_copy_key(bpfptr_t ukey, u64 key_size)
 }
 
 /* last field in 'union bpf_attr' used by this command */
-#define BPF_MAP_LOOKUP_ELEM_LAST_FIELD flags
+#define BPF_MAP_LOOKUP_ELEM_LAST_FIELD map_elem.flags
 
 static int map_lookup_elem(union bpf_attr *attr)
 {
@@ -1427,7 +1427,7 @@ static int map_lookup_elem(union bpf_attr *attr)
 }
 
 
-#define BPF_MAP_UPDATE_ELEM_LAST_FIELD flags
+#define BPF_MAP_UPDATE_ELEM_LAST_FIELD map_elem.flags
 
 static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
 {
@@ -1483,7 +1483,7 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
 	return err;
 }
 
-#define BPF_MAP_DELETE_ELEM_LAST_FIELD key
+#define BPF_MAP_DELETE_ELEM_LAST_FIELD map_elem.key
 
 static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
 {
@@ -1538,7 +1538,7 @@ static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
 }
 
 /* last field in 'union bpf_attr' used by this command */
-#define BPF_MAP_GET_NEXT_KEY_LAST_FIELD next_key
+#define BPF_MAP_GET_NEXT_KEY_LAST_FIELD map_next_key.next_key
 
 static int map_get_next_key(union bpf_attr *attr)
 {
@@ -1817,7 +1817,7 @@ int generic_map_lookup_batch(struct bpf_map *map,
 	return err;
 }
 
-#define BPF_MAP_LOOKUP_AND_DELETE_ELEM_LAST_FIELD flags
+#define BPF_MAP_LOOKUP_AND_DELETE_ELEM_LAST_FIELD map_elem.flags
 
 static int map_lookup_and_delete_elem(union bpf_attr *attr)
 {
@@ -1910,7 +1910,7 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
 	return err;
 }
 
-#define BPF_MAP_FREEZE_LAST_FIELD map_fd
+#define BPF_MAP_FREEZE_LAST_FIELD map_freeze.map_fd
 
 static int map_freeze(const union bpf_attr *attr)
 {
@@ -2493,7 +2493,7 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type)
 }
 
 /* last field in 'union bpf_attr' used by this command */
-#define	BPF_PROG_LOAD_LAST_FIELD log_true_size
+#define BPF_PROG_LOAD_LAST_FIELD prog_load.log_true_size
 
 static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 {
@@ -2697,13 +2697,13 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 	return err;
 }
 
-#define BPF_OBJ_LAST_FIELD path_fd
+#define BPF_OBJ_PIN_LAST_FIELD obj_pin.path_fd
 
 static int bpf_obj_pin(const union bpf_attr *attr)
 {
 	int path_fd;
 
-	if (CHECK_ATTR(BPF_OBJ) || attr->file_flags & ~BPF_F_PATH_FD)
+	if (CHECK_ATTR(BPF_OBJ_PIN) || attr->file_flags & ~BPF_F_PATH_FD)
 		return -EINVAL;
 
 	/* path_fd has to be accompanied by BPF_F_PATH_FD flag */
@@ -2715,11 +2715,13 @@ static int bpf_obj_pin(const union bpf_attr *attr)
 				u64_to_user_ptr(attr->pathname));
 }
 
+#define BPF_OBJ_GET_LAST_FIELD obj_get.path_fd
+
 static int bpf_obj_get(const union bpf_attr *attr)
 {
 	int path_fd;
 
-	if (CHECK_ATTR(BPF_OBJ) || attr->bpf_fd != 0 ||
+	if (CHECK_ATTR(BPF_OBJ_GET) || attr->bpf_fd != 0 ||
 	    attr->file_flags & ~(BPF_OBJ_FLAG_MASK | BPF_F_PATH_FD))
 		return -EINVAL;
 
@@ -3526,7 +3528,7 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
 	}
 }
 
-#define BPF_PROG_ATTACH_LAST_FIELD replace_bpf_fd
+#define BPF_PROG_ATTACH_LAST_FIELD prog_attach.replace_bpf_fd
 
 #define BPF_F_ATTACH_MASK \
 	(BPF_F_ALLOW_OVERRIDE | BPF_F_ALLOW_MULTI | BPF_F_REPLACE)
@@ -3590,7 +3592,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
 	return ret;
 }
 
-#define BPF_PROG_DETACH_LAST_FIELD attach_type
+#define BPF_PROG_DETACH_LAST_FIELD prog_detach.attach_type
 
 static int bpf_prog_detach(const union bpf_attr *attr)
 {
@@ -3706,7 +3708,7 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
 	return ret;
 }
 
-#define BPF_OBJ_GET_NEXT_ID_LAST_FIELD next_id
+#define BPF_OBJ_GET_NEXT_ID_LAST_FIELD obj_next_id.next_id
 
 static int bpf_obj_get_next_id(const union bpf_attr *attr,
 			       union bpf_attr __user *uattr,
@@ -3772,7 +3774,7 @@ struct bpf_prog *bpf_prog_get_curr_or_next(u32 *id)
 	return prog;
 }
 
-#define BPF_PROG_GET_FD_BY_ID_LAST_FIELD prog_id
+#define BPF_PROG_GET_FD_BY_ID_LAST_FIELD obj_fd_by_id.obj_id
 
 struct bpf_prog *bpf_prog_by_id(u32 id)
 {
@@ -3814,7 +3816,7 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
 	return fd;
 }
 
-#define BPF_MAP_GET_FD_BY_ID_LAST_FIELD open_flags
+#define BPF_MAP_GET_FD_BY_ID_LAST_FIELD obj_fd_by_id.flags
 
 static int bpf_map_get_fd_by_id(const union bpf_attr *attr)
 {
@@ -4385,7 +4387,7 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
 	return err;
 }
 
-#define BPF_BTF_LOAD_LAST_FIELD btf_log_true_size
+#define BPF_BTF_LOAD_LAST_FIELD btf_load.log_true_size
 
 static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size)
 {
@@ -4398,7 +4400,7 @@ static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_
 	return btf_new_fd(attr, uattr, uattr_size);
 }
 
-#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD btf_id
+#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD obj_fd_by_id.obj_id
 
 static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
 {
@@ -4859,7 +4861,7 @@ struct bpf_link *bpf_link_get_curr_or_next(u32 *id)
 	return link;
 }
 
-#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id
+#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD obj_fd_by_id.obj_id
 
 static int bpf_link_get_fd_by_id(const union bpf_attr *attr)
 {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9273c654743c..83066cc0f24b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1320,7 +1320,8 @@ struct bpf_stack_build_id {
 #define BPF_OBJ_NAME_LEN 16U
 
 union bpf_attr {
-	struct { /* anonymous struct used by BPF_MAP_CREATE command */
+	/* BPF_MAP_CREATE command */
+	struct bpf_map_create_attr {
 		__u32	map_type;	/* one of enum bpf_map_type */
 		__u32	key_size;	/* size of key in bytes */
 		__u32	value_size;	/* size of value in bytes */
@@ -1348,19 +1349,30 @@ union bpf_attr {
 		 * to using 5 hash functions).
 		 */
 		__u64	map_extra;
-	};
+	} map_create;
 
-	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
+	/* BPF_MAP_{LOOKUP,UPDATE,DELETE,LOOKUP_AND_DELETE}_ELEM commands */
+	struct bpf_map_elem_attr {
 		__u32		map_fd;
 		__aligned_u64	key;
-		union {
-			__aligned_u64 value;
-			__aligned_u64 next_key;
-		};
+		__aligned_u64	value;
 		__u64		flags;
-	};
+	} map_elem;
 
-	struct { /* struct used by BPF_MAP_*_BATCH commands */
+	/* BPF_MAP_GET_NEXT_KEY command */
+	struct bpf_map_next_key_attr {
+		__u32		map_fd;
+		__aligned_u64	key;
+		__aligned_u64	next_key;
+	} map_next_key;
+
+	/* BPF_MAP_FREEZE command */
+	struct bpf_map_freeze_attr {
+		__u32		map_fd;
+	} map_freeze;
+
+	/* BPF_MAP_{LOOKUP,UPDATE,DELETE,LOOKUP_AND_DELETE}_BATCH commands */
+	struct bpf_map_batch_attr {
 		__aligned_u64	in_batch;	/* start batch,
 						 * NULL to start from beginning
 						 */
@@ -1377,7 +1389,8 @@ union bpf_attr {
 		__u64		flags;
 	} batch;
 
-	struct { /* anonymous struct used by BPF_PROG_LOAD command */
+	/* BPF_PROG_LOAD command */
+	struct bpf_prog_load_attr {
 		__u32		prog_type;	/* one of enum bpf_prog_type */
 		__u32		insn_cnt;
 		__aligned_u64	insns;
@@ -1417,12 +1430,13 @@ union bpf_attr {
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
 		__u32		log_true_size;
-	};
+	} prog_load;
 
-	struct { /* anonymous struct used by BPF_OBJ_* commands */
+	/* BPF_OBJ_PIN command */
+	struct bpf_obj_pin_attr {
 		__aligned_u64	pathname;
 		__u32		bpf_fd;
-		__u32		file_flags;
+		__u32		flags;
 		/* Same as dirfd in openat() syscall; see openat(2)
 		 * manpage for details of path FD and pathname semantics;
 		 * path_fd should accompanied by BPF_F_PATH_FD flag set in
@@ -1430,9 +1444,24 @@ union bpf_attr {
 		 * if BPF_F_PATH_FD flag is not set, AT_FDCWD is assumed.
 		 */
 		__s32		path_fd;
-	};
+	} obj_pin;
 
-	struct { /* anonymous struct used by BPF_PROG_ATTACH/DETACH commands */
+	/* BPF_OBJ_GET command */
+	struct bpf_obj_get_attr {
+		__aligned_u64	pathname;
+		__u32		__reserved;
+		__u32		flags;
+		/* Same as dirfd in openat() syscall; see openat(2)
+		 * manpage for details of path FD and pathname semantics;
+		 * path_fd should accompanied by BPF_F_PATH_FD flag set in
+		 * file_flags field, otherwise it should be set to zero;
+		 * if BPF_F_PATH_FD flag is not set, AT_FDCWD is assumed.
+		 */
+		__s32		path_fd;
+	} obj_get;
+
+	/* BPF_PROG_ATTACH command */
+	struct bpf_prog_attach_attr {
 		__u32		target_fd;	/* container object to attach to */
 		__u32		attach_bpf_fd;	/* eBPF program to attach */
 		__u32		attach_type;
@@ -1441,9 +1470,16 @@ union bpf_attr {
 						 * program to replace if
 						 * BPF_F_REPLACE is used
 						 */
-	};
+	} prog_attach;
 
-	struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
+	/* BPF_PROG_DETACH command */
+	struct bpf_prog_detach_attr {
+		__u32		target_fd;	/* container object to attach to */
+		__u32		prog_fd;	/* eBPF program to detach */
+		__u32		attach_type;
+	} prog_detach;
+
+	struct bpf_prog_run_attr { /* anonymous struct used by BPF_PROG_TEST_RUN command */
 		__u32		prog_fd;
 		__u32		retval;
 		__u32		data_size_in;	/* input: len of data_in */
@@ -1467,25 +1503,26 @@ union bpf_attr {
 		__u32		batch_size;
 	} test;
 
-	struct { /* anonymous struct used by BPF_*_GET_*_ID */
-		union {
-			__u32		start_id;
-			__u32		prog_id;
-			__u32		map_id;
-			__u32		btf_id;
-			__u32		link_id;
-		};
+	/* BPF_{MAP,PROG,BTF,LINK}_GET_FD_BY_ID commands */
+	struct bpf_obj_fd_by_id_attr {
+		__u32		obj_id;
+		__u32		__reserved;
+		__u32		flags;
+	} obj_fd_by_id;
+
+	/* BPF_OBJ_GET_NEXT_ID command */
+	struct bpf_obj_next_id_attr {
+		__u32		start_id;
 		__u32		next_id;
-		__u32		open_flags;
-	};
+	} obj_next_id;
 
-	struct { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
+	struct bpf_obj_info_by_fd_attr { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
 		__u32		bpf_fd;
 		__u32		info_len;
 		__aligned_u64	info;
 	} info;
 
-	struct { /* anonymous struct used by BPF_PROG_QUERY command */
+	struct bpf_prog_query_attr { /* anonymous struct used by BPF_PROG_QUERY command */
 		__u32		target_fd;	/* container object to query */
 		__u32		attach_type;
 		__u32		query_flags;
@@ -1498,25 +1535,26 @@ union bpf_attr {
 		__aligned_u64	prog_attach_flags;
 	} query;
 
-	struct { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
+	struct bpf_raw_tp_open_attr { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
 		__u64 name;
 		__u32 prog_fd;
 	} raw_tracepoint;
 
-	struct { /* anonymous struct for BPF_BTF_LOAD */
+	/* BPF_BTF_LOAD command */
+	struct bpf_btf_load_attr {
 		__aligned_u64	btf;
-		__aligned_u64	btf_log_buf;
+		__aligned_u64	log_buf;
 		__u32		btf_size;
-		__u32		btf_log_size;
-		__u32		btf_log_level;
+		__u32		log_size;
+		__u32		log_level;
 		/* output: actual total log contents size (including termintaing zero).
 		 * It could be both larger than original log_size (if log was
 		 * truncated), or smaller (if log buffer wasn't filled completely).
 		 */
-		__u32		btf_log_true_size;
-	};
+		__u32		log_true_size;
+	} btf_load;
 
-	struct {
+	struct bpf_task_fd_query_attr {
 		__u32		pid;		/* input: pid */
 		__u32		fd;		/* input: fd */
 		__u32		flags;		/* input: flags */
@@ -1532,7 +1570,7 @@ union bpf_attr {
 		__u64		probe_addr;	/* output: probe_addr */
 	} task_fd_query;
 
-	struct { /* struct used by BPF_LINK_CREATE command */
+	struct bpf_link_create_attr { /* struct used by BPF_LINK_CREATE command */
 		union {
 			__u32		prog_fd;	/* eBPF program to attach */
 			__u32		map_fd;		/* struct_ops to attach */
@@ -1581,7 +1619,7 @@ union bpf_attr {
 		};
 	} link_create;
 
-	struct { /* struct used by BPF_LINK_UPDATE command */
+	struct bpf_link_update_attr { /* struct used by BPF_LINK_UPDATE command */
 		__u32		link_fd;	/* link fd */
 		union {
 			/* new program fd to update link with */
@@ -1602,25 +1640,136 @@ union bpf_attr {
 		};
 	} link_update;
 
-	struct {
+	struct bpf_link_detach_attr {
 		__u32		link_fd;
 	} link_detach;
 
-	struct { /* struct used by BPF_ENABLE_STATS command */
+	struct bpf_enable_stats_attr { /* struct used by BPF_ENABLE_STATS command */
 		__u32		type;
 	} enable_stats;
 
-	struct { /* struct used by BPF_ITER_CREATE command */
+	struct bpf_iter_create_attr { /* struct used by BPF_ITER_CREATE command */
 		__u32		link_fd;
 		__u32		flags;
 	} iter_create;
 
-	struct { /* struct used by BPF_PROG_BIND_MAP command */
+	struct bpf_prog_bind_map_attr { /* struct used by BPF_PROG_BIND_MAP command */
 		__u32		prog_fd;
 		__u32		map_fd;
 		__u32		flags;		/* extra flags */
 	} prog_bind_map;
 
+	/*
+	 * LEGACY anonymous substructs, for backwards compatibility.
+	 * Each of the below anonymous substructs are ABI compatible with one
+	 * of the above named substructs. Please use named substructs.
+	 */
+
+	struct { /* legacy BPF_MAP_CREATE attrs, use .map_create instead */
+		__u32	map_type;
+		__u32	key_size;
+		__u32	value_size;
+		__u32	max_entries;
+		__u32	map_flags;
+		__u32	inner_map_fd;
+		__u32	numa_node;
+		char	map_name[BPF_OBJ_NAME_LEN];
+		__u32	map_ifindex;
+		__u32	btf_fd;
+		__u32	btf_key_type_id;
+		__u32	btf_value_type_id;
+		__u32	btf_vmlinux_value_type_id;
+		__u64	map_extra;
+	};
+	/*
+	 * legacy BPF_MAP_*_ELEM and BPF_MAP_GET_NEXT_KEY attrs,
+	 * use .map_elem or .get_next_key, respectively, instead
+	 */
+	struct {
+		__u32		map_fd;
+		__aligned_u64	key;
+		union {
+			__aligned_u64 value;
+			__aligned_u64 next_key;
+		};
+		__u64		flags;
+	};
+	struct { /* legacy BPF_PROG_LOAD attrs, use .prog_load instead */
+		__u32		prog_type;
+		__u32		insn_cnt;
+		__aligned_u64	insns;
+		__aligned_u64	license;
+		__u32		log_level;
+		__u32		log_size;
+		__aligned_u64	log_buf;
+		__u32		kern_version;
+		__u32		prog_flags;
+		char		prog_name[BPF_OBJ_NAME_LEN];
+		__u32		prog_ifindex;
+		__u32		expected_attach_type;
+		__u32		prog_btf_fd;
+		__u32		func_info_rec_size;
+		__aligned_u64	func_info;
+		__u32		func_info_cnt;
+		__u32		line_info_rec_size;
+		__aligned_u64	line_info;
+		__u32		line_info_cnt;
+		__u32		attach_btf_id;
+		union {
+			__u32		attach_prog_fd;
+			__u32		attach_btf_obj_fd;
+		};
+		__u32		core_relo_cnt;
+		__aligned_u64	fd_array;
+		__aligned_u64	core_relos;
+		__u32		core_relo_rec_size;
+		__u32		log_true_size;
+	};
+	/* legacy BPF_OBJ_{PIN, GET} attrs, use .obj_pin or .obj_get instead */
+	struct {
+		__aligned_u64	pathname;
+		__u32		bpf_fd;
+		__u32		file_flags;
+		__s32		path_fd;
+	};
+	/*
+	 * legacy BPF_PROG_{ATTACH,DETACH} attrs,
+	 * use .prog_attach or .prog_detach instead
+	 */
+	struct {
+		__u32		target_fd;	/* container object to attach to */
+		__u32		attach_bpf_fd;	/* eBPF program to attach */
+		__u32		attach_type;
+		__u32		attach_flags;
+		__u32		replace_bpf_fd;	/* previously attached eBPF
+						 * program to replace if
+						 * BPF_F_REPLACE is used
+						 */
+	};
+	/*
+	 * legacy BPF_*_GET_FD_BY_ID and BPF_OBJ_GET_NEXT_ID attrs,
+	 * use .obj_fd_by_id or .obj_next_id, respectively, instead
+	 */
+	struct {
+		union {
+			__u32		start_id;
+			__u32		prog_id;
+			__u32		map_id;
+			__u32		btf_id;
+			__u32		link_id;
+		};
+		__u32		next_id;
+		__u32		open_flags;
+	};
+	/* legacy BPF_BTF_LOAD attrs, use .btf_load instead */
+	struct {
+		__aligned_u64	btf;
+		__aligned_u64	btf_log_buf;
+		__u32		btf_size;
+		__u32		btf_log_size;
+		__u32		btf_log_level;
+		__u32		btf_log_true_size;
+	};
 } __attribute__((aligned(8)));
 
 /* The description below is an attempt at providing documentation to eBPF
-- 
2.34.1

Re: [PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct

[Alexei Starovoitov]

On Wed, May 24, 2023 at 02:02:41PM -0700, Andrii Nakryiko wrote:
> 
> And there were a bunch of other similar changes. Please take a thorough
> look and suggest more changes or which changes to drop. I'm not married
> to any of them, it just felt like a good improvement.

Agree that current layout sucks, but ...

>  include/uapi/linux/bpf.h       | 235 +++++++++++++++++++++++++++------
>  kernel/bpf/syscall.c           |  40 +++---
>  tools/include/uapi/linux/bpf.h | 235 +++++++++++++++++++++++++++------
>  3 files changed, 405 insertions(+), 105 deletions(-)

... the diff makes it worse. The diffstat for "nop" change is a red flag.

> +	/*
> +	 * LEGACY anonymous substructs, for backwards compatibility.
> +	 * Each of the below anonymous substructs are ABI compatible with one
> +	 * of the above named substructs. Please use named substructs.
> +	 */
> +

All of them cannot be removed. This bagage will be a forever eyesore.
Currently it's not pretty. The diffs make uapi file just ugly.
Especially considering how 'named' and 'legacy' will start diverging.
New commands are thankfully named. We've learned the lesson,
but prior mistake is unfixable. We have to live with it.

Re: [PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct

[Andrii Nakryiko]

On Wed, May 24, 2023 at 8:18 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, May 24, 2023 at 02:02:41PM -0700, Andrii Nakryiko wrote:
> >
> > And there were a bunch of other similar changes. Please take a thorough
> > look and suggest more changes or which changes to drop. I'm not married
> > to any of them, it just felt like a good improvement.
>
> Agree that current layout sucks, but ...
>
> >  include/uapi/linux/bpf.h       | 235 +++++++++++++++++++++++++++------
> >  kernel/bpf/syscall.c           |  40 +++---
> >  tools/include/uapi/linux/bpf.h | 235 +++++++++++++++++++++++++++------
> >  3 files changed, 405 insertions(+), 105 deletions(-)
>
> ... the diff makes it worse. The diffstat for "nop" change is a red flag.

Only 100 lines are a real "nop" change to copy/paste existing fields
that are in unnamed fields. The rest is a value add.

I don't think the deal is in stats, though, right?

>
> > +     /*
> > +      * LEGACY anonymous substructs, for backwards compatibility.
> > +      * Each of the below anonymous substructs are ABI compatible with one
> > +      * of the above named substructs. Please use named substructs.
> > +      */
> > +
>
> All of them cannot be removed. This bagage will be a forever eyesore.
> Currently it's not pretty. The diffs make uapi file just ugly.
> Especially considering how 'named' and 'legacy' will start diverging.

We have to allow "divergence" (only in the sense that new fields only
go into named substructs, but the existing fields stay fixed, of
course), to avoid more naming conflicts. If that wasn't the case,
using struct_group() macro could have been used to avoid a copy/paste
of those anonymous field/struct copies.

So I'm not happy about those 100 lines copy paste of fixed fields
either, but at least that would get us out of the current global
naming namespace for PROG_LOAD, MAP_CREATE, BTF_LOAD, etc.

> New commands are thankfully named. We've learned the lesson,

Unfortunately, the problem is that unnamed commands are the ones that
are most likely to keep evolving.

> but prior mistake is unfixable. We have to live with it.

Ok, too bad, but it's fine. It was worth a try.

I tried to come up with something like struct_group() approach to
minimize code changes in UAPI header, but we have a more complicated
situation where part of struct has to be both anonymous and named,
while another part (newly added fields) should go only to named parts.
And that doesn't seem to be possible to support with a macro,
unfortunately.

Re: [PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct

[Daniel Borkmann]

On 5/25/23 7:19 PM, Andrii Nakryiko wrote:
> On Wed, May 24, 2023 at 8:18 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Wed, May 24, 2023 at 02:02:41PM -0700, Andrii Nakryiko wrote:
>>>
>>> And there were a bunch of other similar changes. Please take a thorough
>>> look and suggest more changes or which changes to drop. I'm not married
>>> to any of them, it just felt like a good improvement.
>>
>> Agree that current layout sucks, but ...
>>
>>>   include/uapi/linux/bpf.h       | 235 +++++++++++++++++++++++++++------
>>>   kernel/bpf/syscall.c           |  40 +++---
>>>   tools/include/uapi/linux/bpf.h | 235 +++++++++++++++++++++++++++------
>>>   3 files changed, 405 insertions(+), 105 deletions(-)
>>
>> ... the diff makes it worse. The diffstat for "nop" change is a red flag.
> 
> Only 100 lines are a real "nop" change to copy/paste existing fields
> that are in unnamed fields. The rest is a value add.
> 
> I don't think the deal is in stats, though, right?
> 
>>> +     /*
>>> +      * LEGACY anonymous substructs, for backwards compatibility.
>>> +      * Each of the below anonymous substructs are ABI compatible with one
>>> +      * of the above named substructs. Please use named substructs.
>>> +      */
>>
>> All of them cannot be removed. This bagage will be a forever eyesore.
>> Currently it's not pretty. The diffs make uapi file just ugly.
>> Especially considering how 'named' and 'legacy' will start diverging.
> 
> We have to allow "divergence" (only in the sense that new fields only
> go into named substructs, but the existing fields stay fixed, of
> course), to avoid more naming conflicts. If that wasn't the case,
> using struct_group() macro could have been used to avoid a copy/paste
> of those anonymous field/struct copies.
> 
> So I'm not happy about those 100 lines copy paste of fixed fields
> either, but at least that would get us out of the current global
> naming namespace for PROG_LOAD, MAP_CREATE, BTF_LOAD, etc.
> 
>> New commands are thankfully named. We've learned the lesson,
> 
> Unfortunately, the problem is that unnamed commands are the ones that
> are most likely to keep evolving.
> 
>> but prior mistake is unfixable. We have to live with it.
> 
> Ok, too bad, but it's fine. It was worth a try.
> 
> I tried to come up with something like struct_group() approach to
> minimize code changes in UAPI header, but we have a more complicated
> situation where part of struct has to be both anonymous and named,
> while another part (newly added fields) should go only to named parts.
> And that doesn't seem to be possible to support with a macro,
> unfortunately.

Nice idea on the struct_group()-like approach, but agree that this is
going to be tough given we need to divert anonymous and named parts as
you mention. One other wild thought ... we remove the bpf_attr entirely
from the uapi header, and have a kernel/bpf/bpf.cmd description and
generate the bpf_attr into a uapi header via script which the main header
can include. Kind of similar to the suggestion, but more flexible than
macro magic. We also have things like syscall table header generated via
script.. so it wouldn't be first. Still doesn't remove the eyesore, just
packages it differently. ;/

Thanks,
Daniel

Re: [PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct

[Andrii Nakryiko]

On Thu, May 25, 2023 at 2:51 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 5/25/23 7:19 PM, Andrii Nakryiko wrote:
> > On Wed, May 24, 2023 at 8:18 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> >> On Wed, May 24, 2023 at 02:02:41PM -0700, Andrii Nakryiko wrote:
> >>>
> >>> And there were a bunch of other similar changes. Please take a thorough
> >>> look and suggest more changes or which changes to drop. I'm not married
> >>> to any of them, it just felt like a good improvement.
> >>
> >> Agree that current layout sucks, but ...
> >>
> >>>   include/uapi/linux/bpf.h       | 235 +++++++++++++++++++++++++++------
> >>>   kernel/bpf/syscall.c           |  40 +++---
> >>>   tools/include/uapi/linux/bpf.h | 235 +++++++++++++++++++++++++++------
> >>>   3 files changed, 405 insertions(+), 105 deletions(-)
> >>
> >> ... the diff makes it worse. The diffstat for "nop" change is a red flag.
> >
> > Only 100 lines are a real "nop" change to copy/paste existing fields
> > that are in unnamed fields. The rest is a value add.
> >
> > I don't think the deal is in stats, though, right?
> >
> >>> +     /*
> >>> +      * LEGACY anonymous substructs, for backwards compatibility.
> >>> +      * Each of the below anonymous substructs are ABI compatible with one
> >>> +      * of the above named substructs. Please use named substructs.
> >>> +      */
> >>
> >> All of them cannot be removed. This bagage will be a forever eyesore.
> >> Currently it's not pretty. The diffs make uapi file just ugly.
> >> Especially considering how 'named' and 'legacy' will start diverging.
> >
> > We have to allow "divergence" (only in the sense that new fields only
> > go into named substructs, but the existing fields stay fixed, of
> > course), to avoid more naming conflicts. If that wasn't the case,
> > using struct_group() macro could have been used to avoid a copy/paste
> > of those anonymous field/struct copies.
> >
> > So I'm not happy about those 100 lines copy paste of fixed fields
> > either, but at least that would get us out of the current global
> > naming namespace for PROG_LOAD, MAP_CREATE, BTF_LOAD, etc.
> >
> >> New commands are thankfully named. We've learned the lesson,
> >
> > Unfortunately, the problem is that unnamed commands are the ones that
> > are most likely to keep evolving.
> >
> >> but prior mistake is unfixable. We have to live with it.
> >
> > Ok, too bad, but it's fine. It was worth a try.
> >
> > I tried to come up with something like struct_group() approach to
> > minimize code changes in UAPI header, but we have a more complicated
> > situation where part of struct has to be both anonymous and named,
> > while another part (newly added fields) should go only to named parts.
> > And that doesn't seem to be possible to support with a macro,
> > unfortunately.
>
> Nice idea on the struct_group()-like approach, but agree that this is
> going to be tough given we need to divert anonymous and named parts as
> you mention. One other wild thought ... we remove the bpf_attr entirely
> from the uapi header, and have a kernel/bpf/bpf.cmd description and
> generate the bpf_attr into a uapi header via script which the main header
> can include. Kind of similar to the suggestion, but more flexible than
> macro magic. We also have things like syscall table header generated via
> script.. so it wouldn't be first. Still doesn't remove the eyesore, just
> packages it differently. ;/

There are two more ways, neither is that pretty. But I'll just outline
them here for completeness.

First, we can define about 6 variants (one for each command with anon
field) of macro with different numbers of arguments, one for each
existing field. Replace all semicolons with commas and do something
like this (we can prettify the below some more, I didn't want to waste
too much time on this demo):

#define __bpf_cmd4(type, f1, f2, f3, f4, new_fields...)        \
       struct {                                                \
               f1; f2; f3; f4;                                 \
       };                                                      \
       struct type {                                           \
               f1; f2; f3; f4;                                 \
               new_fields                                      \
       }

       /* BPF_OBJ_PIN command */
       __bpf_cmd4(bpf_obj_pin_attr,
               __aligned_u64   pathname,
               __u32           bpf_fd,
               __u32           file_flags,
               /* Same as dirfd in openat() syscall; see openat(2)
                * manpage for details of path FD and pathname semantics;
                * path_fd should accompanied by BPF_F_PATH_FD flag set in
                * file_flags field, otherwise it should be set to zero;
                * if BPF_F_PATH_FD flag is not set, AT_FDCWD is assumed.
                */
               __s32           path_fd,
               __u32           token_fd;
       ) obj_pin;

Note that I also added `__u32 token_fd;` as a demonstration how we can
new fields, and that new fields will have proper semicolons at the
end. The largest command (BPF_PROG_LOAD) will need 28 arg variant, but
that can be fit in few lines pretty cleanly, if the overall approach
would be deemed acceptable.

This approach also has a slight downside that we can rename fields
(e.g. for BPF_BTF_LOAD command). We still can split out dedicated new
named structs. So too big of a deal.


Second approach. If it's mostly about aesthetics, then we can add
include/uapi/linux/bpf_legacy.h, where we put all these unnamed fields
and structs in one stashed away place, and then in original
include/uapi/linux/bpf.h header we just

union bpf_attr {
   ... named structs and fields go here ...

/* include backwards compat legacy anon fields/structs */
#include "bpf_legacy.h"
};

This way this eyesore will be somewhat hidden away (but still lookup-able).


Curious if any of the above is more palatable?


>
> Thanks,
> Daniel

Re: [PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct

[Alexei Starovoitov]

On Thu, May 25, 2023 at 4:40 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 25, 2023 at 2:51 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 5/25/23 7:19 PM, Andrii Nakryiko wrote:
> > > On Wed, May 24, 2023 at 8:18 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > >> On Wed, May 24, 2023 at 02:02:41PM -0700, Andrii Nakryiko wrote:
> > >>>
> > >>> And there were a bunch of other similar changes. Please take a thorough
> > >>> look and suggest more changes or which changes to drop. I'm not married
> > >>> to any of them, it just felt like a good improvement.
> > >>
> > >> Agree that current layout sucks, but ...
> > >>
> > >>>   include/uapi/linux/bpf.h       | 235 +++++++++++++++++++++++++++------
> > >>>   kernel/bpf/syscall.c           |  40 +++---
> > >>>   tools/include/uapi/linux/bpf.h | 235 +++++++++++++++++++++++++++------
> > >>>   3 files changed, 405 insertions(+), 105 deletions(-)
> > >>
> > >> ... the diff makes it worse. The diffstat for "nop" change is a red flag.
> > >
> > > Only 100 lines are a real "nop" change to copy/paste existing fields
> > > that are in unnamed fields. The rest is a value add.
> > >
> > > I don't think the deal is in stats, though, right?
> > >
> > >>> +     /*
> > >>> +      * LEGACY anonymous substructs, for backwards compatibility.
> > >>> +      * Each of the below anonymous substructs are ABI compatible with one
> > >>> +      * of the above named substructs. Please use named substructs.
> > >>> +      */
> > >>
> > >> All of them cannot be removed. This bagage will be a forever eyesore.
> > >> Currently it's not pretty. The diffs make uapi file just ugly.
> > >> Especially considering how 'named' and 'legacy' will start diverging.
> > >
> > > We have to allow "divergence" (only in the sense that new fields only
> > > go into named substructs, but the existing fields stay fixed, of
> > > course), to avoid more naming conflicts. If that wasn't the case,
> > > using struct_group() macro could have been used to avoid a copy/paste
> > > of those anonymous field/struct copies.
> > >
> > > So I'm not happy about those 100 lines copy paste of fixed fields
> > > either, but at least that would get us out of the current global
> > > naming namespace for PROG_LOAD, MAP_CREATE, BTF_LOAD, etc.
> > >
> > >> New commands are thankfully named. We've learned the lesson,
> > >
> > > Unfortunately, the problem is that unnamed commands are the ones that
> > > are most likely to keep evolving.
> > >
> > >> but prior mistake is unfixable. We have to live with it.
> > >
> > > Ok, too bad, but it's fine. It was worth a try.
> > >
> > > I tried to come up with something like struct_group() approach to
> > > minimize code changes in UAPI header, but we have a more complicated
> > > situation where part of struct has to be both anonymous and named,
> > > while another part (newly added fields) should go only to named parts.
> > > And that doesn't seem to be possible to support with a macro,
> > > unfortunately.
> >
> > Nice idea on the struct_group()-like approach, but agree that this is
> > going to be tough given we need to divert anonymous and named parts as
> > you mention. One other wild thought ... we remove the bpf_attr entirely
> > from the uapi header, and have a kernel/bpf/bpf.cmd description and
> > generate the bpf_attr into a uapi header via script which the main header
> > can include. Kind of similar to the suggestion, but more flexible than
> > macro magic. We also have things like syscall table header generated via
> > script.. so it wouldn't be first. Still doesn't remove the eyesore, just
> > packages it differently. ;/
>
> There are two more ways, neither is that pretty. But I'll just outline
> them here for completeness.
>
> First, we can define about 6 variants (one for each command with anon
> field) of macro with different numbers of arguments, one for each
> existing field. Replace all semicolons with commas and do something
> like this (we can prettify the below some more, I didn't want to waste
> too much time on this demo):
>
> #define __bpf_cmd4(type, f1, f2, f3, f4, new_fields...)        \
>        struct {                                                \
>                f1; f2; f3; f4;                                 \
>        };                                                      \
>        struct type {                                           \
>                f1; f2; f3; f4;                                 \
>                new_fields                                      \
>        }
>
>        /* BPF_OBJ_PIN command */
>        __bpf_cmd4(bpf_obj_pin_attr,
>                __aligned_u64   pathname,
>                __u32           bpf_fd,
>                __u32           file_flags,
>                /* Same as dirfd in openat() syscall; see openat(2)
>                 * manpage for details of path FD and pathname semantics;
>                 * path_fd should accompanied by BPF_F_PATH_FD flag set in
>                 * file_flags field, otherwise it should be set to zero;
>                 * if BPF_F_PATH_FD flag is not set, AT_FDCWD is assumed.
>                 */
>                __s32           path_fd,
>                __u32           token_fd;
>        ) obj_pin;
>
> Note that I also added `__u32 token_fd;` as a demonstration how we can
> new fields, and that new fields will have proper semicolons at the
> end. The largest command (BPF_PROG_LOAD) will need 28 arg variant, but
> that can be fit in few lines pretty cleanly, if the overall approach
> would be deemed acceptable.
>
> This approach also has a slight downside that we can rename fields
> (e.g. for BPF_BTF_LOAD command). We still can split out dedicated new
> named structs. So too big of a deal.
>
>
> Second approach. If it's mostly about aesthetics, then we can add
> include/uapi/linux/bpf_legacy.h, where we put all these unnamed fields
> and structs in one stashed away place, and then in original
> include/uapi/linux/bpf.h header we just
>
> union bpf_attr {
>    ... named structs and fields go here ...
>
> /* include backwards compat legacy anon fields/structs */
> #include "bpf_legacy.h"
> };
>
> This way this eyesore will be somewhat hidden away (but still lookup-able).
>
>
> Curious if any of the above is more palatable?

Frankly I don't like either Daniel's .cmd idea or these two "aesthetics".
We just need new *_token_fd fields in several structures.
imo adding several such fields with different prefixes are cleaner
than revamping the whole thing.

Re: [PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct

[Andrii Nakryiko]

On Tue, May 30, 2023 at 10:41 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, May 25, 2023 at 4:40 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, May 25, 2023 at 2:51 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> > >
> > > On 5/25/23 7:19 PM, Andrii Nakryiko wrote:
> > > > On Wed, May 24, 2023 at 8:18 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > >> On Wed, May 24, 2023 at 02:02:41PM -0700, Andrii Nakryiko wrote:
> > > >>>
> > > >>> And there were a bunch of other similar changes. Please take a thorough
> > > >>> look and suggest more changes or which changes to drop. I'm not married
> > > >>> to any of them, it just felt like a good improvement.
> > > >>
> > > >> Agree that current layout sucks, but ...
> > > >>
> > > >>>   include/uapi/linux/bpf.h       | 235 +++++++++++++++++++++++++++------
> > > >>>   kernel/bpf/syscall.c           |  40 +++---
> > > >>>   tools/include/uapi/linux/bpf.h | 235 +++++++++++++++++++++++++++------
> > > >>>   3 files changed, 405 insertions(+), 105 deletions(-)
> > > >>
> > > >> ... the diff makes it worse. The diffstat for "nop" change is a red flag.
> > > >
> > > > Only 100 lines are a real "nop" change to copy/paste existing fields
> > > > that are in unnamed fields. The rest is a value add.
> > > >
> > > > I don't think the deal is in stats, though, right?
> > > >
> > > >>> +     /*
> > > >>> +      * LEGACY anonymous substructs, for backwards compatibility.
> > > >>> +      * Each of the below anonymous substructs are ABI compatible with one
> > > >>> +      * of the above named substructs. Please use named substructs.
> > > >>> +      */
> > > >>
> > > >> All of them cannot be removed. This bagage will be a forever eyesore.
> > > >> Currently it's not pretty. The diffs make uapi file just ugly.
> > > >> Especially considering how 'named' and 'legacy' will start diverging.
> > > >
> > > > We have to allow "divergence" (only in the sense that new fields only
> > > > go into named substructs, but the existing fields stay fixed, of
> > > > course), to avoid more naming conflicts. If that wasn't the case,
> > > > using struct_group() macro could have been used to avoid a copy/paste
> > > > of those anonymous field/struct copies.
> > > >
> > > > So I'm not happy about those 100 lines copy paste of fixed fields
> > > > either, but at least that would get us out of the current global
> > > > naming namespace for PROG_LOAD, MAP_CREATE, BTF_LOAD, etc.
> > > >
> > > >> New commands are thankfully named. We've learned the lesson,
> > > >
> > > > Unfortunately, the problem is that unnamed commands are the ones that
> > > > are most likely to keep evolving.
> > > >
> > > >> but prior mistake is unfixable. We have to live with it.
> > > >
> > > > Ok, too bad, but it's fine. It was worth a try.
> > > >
> > > > I tried to come up with something like struct_group() approach to
> > > > minimize code changes in UAPI header, but we have a more complicated
> > > > situation where part of struct has to be both anonymous and named,
> > > > while another part (newly added fields) should go only to named parts.
> > > > And that doesn't seem to be possible to support with a macro,
> > > > unfortunately.
> > >
> > > Nice idea on the struct_group()-like approach, but agree that this is
> > > going to be tough given we need to divert anonymous and named parts as
> > > you mention. One other wild thought ... we remove the bpf_attr entirely
> > > from the uapi header, and have a kernel/bpf/bpf.cmd description and
> > > generate the bpf_attr into a uapi header via script which the main header
> > > can include. Kind of similar to the suggestion, but more flexible than
> > > macro magic. We also have things like syscall table header generated via
> > > script.. so it wouldn't be first. Still doesn't remove the eyesore, just
> > > packages it differently. ;/
> >
> > There are two more ways, neither is that pretty. But I'll just outline
> > them here for completeness.
> >
> > First, we can define about 6 variants (one for each command with anon
> > field) of macro with different numbers of arguments, one for each
> > existing field. Replace all semicolons with commas and do something
> > like this (we can prettify the below some more, I didn't want to waste
> > too much time on this demo):
> >
> > #define __bpf_cmd4(type, f1, f2, f3, f4, new_fields...)        \
> >        struct {                                                \
> >                f1; f2; f3; f4;                                 \
> >        };                                                      \
> >        struct type {                                           \
> >                f1; f2; f3; f4;                                 \
> >                new_fields                                      \
> >        }
> >
> >        /* BPF_OBJ_PIN command */
> >        __bpf_cmd4(bpf_obj_pin_attr,
> >                __aligned_u64   pathname,
> >                __u32           bpf_fd,
> >                __u32           file_flags,
> >                /* Same as dirfd in openat() syscall; see openat(2)
> >                 * manpage for details of path FD and pathname semantics;
> >                 * path_fd should accompanied by BPF_F_PATH_FD flag set in
> >                 * file_flags field, otherwise it should be set to zero;
> >                 * if BPF_F_PATH_FD flag is not set, AT_FDCWD is assumed.
> >                 */
> >                __s32           path_fd,
> >                __u32           token_fd;
> >        ) obj_pin;
> >
> > Note that I also added `__u32 token_fd;` as a demonstration how we can
> > new fields, and that new fields will have proper semicolons at the
> > end. The largest command (BPF_PROG_LOAD) will need 28 arg variant, but
> > that can be fit in few lines pretty cleanly, if the overall approach
> > would be deemed acceptable.
> >
> > This approach also has a slight downside that we can rename fields
> > (e.g. for BPF_BTF_LOAD command). We still can split out dedicated new
> > named structs. So too big of a deal.
> >
> >
> > Second approach. If it's mostly about aesthetics, then we can add
> > include/uapi/linux/bpf_legacy.h, where we put all these unnamed fields
> > and structs in one stashed away place, and then in original
> > include/uapi/linux/bpf.h header we just
> >
> > union bpf_attr {
> >    ... named structs and fields go here ...
> >
> > /* include backwards compat legacy anon fields/structs */
> > #include "bpf_legacy.h"
> > };
> >
> > This way this eyesore will be somewhat hidden away (but still lookup-able).
> >
> >
> > Curious if any of the above is more palatable?
>
> Frankly I don't like either Daniel's .cmd idea or these two "aesthetics".
> We just need new *_token_fd fields in several structures.
> imo adding several such fields with different prefixes are cleaner
> than revamping the whole thing.

Ok, sgtm.

[PATCH RFC bpf-next 2/3] bpf: use new named bpf_attr substructs for few commands

[Andrii Nakryiko]

Switch to new named bpf_xxx_attr substructs for a few commands to
demonstrate how minimal are the changes, especially for simple commands.

For more "sprawling" commands like BPF_MAP_CREATE and BPF_PROG_LOAD
changes will be of a similar nature (replacing union bpf_attr with
command-specific struct bpf_xxx_attr), but will touch more of helper
functions. Given the RFC status I wanted to avoid unnecessary mundane
work before getting a green light for the entire approach.

Note the changes to CHECK_ATTR() implementation. It's equivalent in
functionality, but, IMO, more straightforward and actually support both
old-style `union bpf_attr` and new-style `struct bpf_xxx_attr` usage
transparently.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/syscall.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index babf1d56c2d9..fbc5c86c7bca 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -974,11 +974,8 @@ int bpf_get_file_flag(int flags)
 
 /* helper macro to check that unused fields 'union bpf_attr' are zero */
 #define CHECK_ATTR(CMD) \
-	memchr_inv((void *) &attr->CMD##_LAST_FIELD + \
-		   sizeof(attr->CMD##_LAST_FIELD), 0, \
-		   sizeof(*attr) - \
-		   offsetof(union bpf_attr, CMD##_LAST_FIELD) - \
-		   sizeof(attr->CMD##_LAST_FIELD)) != NULL
+	memchr_inv((void *)attr + offsetofend(union bpf_attr, CMD##_LAST_FIELD), 0, \
+		   sizeof(union bpf_attr) - offsetofend(union bpf_attr, CMD##_LAST_FIELD)) != NULL
 
 /* dst and src must have at least "size" number of bytes.
  * Return strlen on success and < 0 on error.
@@ -1354,7 +1351,7 @@ static void *___bpf_copy_key(bpfptr_t ukey, u64 key_size)
 /* last field in 'union bpf_attr' used by this command */
 #define BPF_MAP_LOOKUP_ELEM_LAST_FIELD map_elem.flags
 
-static int map_lookup_elem(union bpf_attr *attr)
+static int map_lookup_elem(struct bpf_map_elem_attr *attr)
 {
 	void __user *ukey = u64_to_user_ptr(attr->key);
 	void __user *uvalue = u64_to_user_ptr(attr->value);
@@ -1429,7 +1426,7 @@ static int map_lookup_elem(union bpf_attr *attr)
 
 #define BPF_MAP_UPDATE_ELEM_LAST_FIELD map_elem.flags
 
-static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
+static int map_update_elem(struct bpf_map_elem_attr *attr, bpfptr_t uattr)
 {
 	bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
 	bpfptr_t uvalue = make_bpfptr(attr->value, uattr.is_kernel);
@@ -1485,7 +1482,7 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
 
 #define BPF_MAP_DELETE_ELEM_LAST_FIELD map_elem.key
 
-static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
+static int map_delete_elem(struct bpf_map_elem_attr *attr, bpfptr_t uattr)
 {
 	bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
 	int ufd = attr->map_fd;
@@ -1540,7 +1537,7 @@ static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
 /* last field in 'union bpf_attr' used by this command */
 #define BPF_MAP_GET_NEXT_KEY_LAST_FIELD map_next_key.next_key
 
-static int map_get_next_key(union bpf_attr *attr)
+static int map_get_next_key(struct bpf_map_next_key_attr *attr)
 {
 	void __user *ukey = u64_to_user_ptr(attr->key);
 	void __user *unext_key = u64_to_user_ptr(attr->next_key);
@@ -1912,7 +1909,7 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
 
 #define BPF_MAP_FREEZE_LAST_FIELD map_freeze.map_fd
 
-static int map_freeze(const union bpf_attr *attr)
+static int map_freeze(const struct bpf_map_freeze_attr *attr)
 {
 	int err = 0, ufd = attr->map_fd;
 	struct bpf_map *map;
@@ -3710,7 +3707,7 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
 
 #define BPF_OBJ_GET_NEXT_ID_LAST_FIELD obj_next_id.next_id
 
-static int bpf_obj_get_next_id(const union bpf_attr *attr,
+static int bpf_obj_get_next_id(const struct bpf_obj_next_id_attr *attr,
 			       union bpf_attr __user *uattr,
 			       struct idr *idr,
 			       spinlock_t *lock)
@@ -5064,19 +5061,19 @@ static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
 		err = map_create(&attr);
 		break;
 	case BPF_MAP_LOOKUP_ELEM:
-		err = map_lookup_elem(&attr);
+		err = map_lookup_elem(&attr.map_elem);
 		break;
 	case BPF_MAP_UPDATE_ELEM:
-		err = map_update_elem(&attr, uattr);
+		err = map_update_elem(&attr.map_elem, uattr);
 		break;
 	case BPF_MAP_DELETE_ELEM:
-		err = map_delete_elem(&attr, uattr);
+		err = map_delete_elem(&attr.map_elem, uattr);
 		break;
 	case BPF_MAP_GET_NEXT_KEY:
-		err = map_get_next_key(&attr);
+		err = map_get_next_key(&attr.map_next_key);
 		break;
 	case BPF_MAP_FREEZE:
-		err = map_freeze(&attr);
+		err = map_freeze(&attr.map_freeze);
 		break;
 	case BPF_PROG_LOAD:
 		err = bpf_prog_load(&attr, uattr, size);
@@ -5100,15 +5097,15 @@ static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
 		err = bpf_prog_test_run(&attr, uattr.user);
 		break;
 	case BPF_PROG_GET_NEXT_ID:
-		err = bpf_obj_get_next_id(&attr, uattr.user,
+		err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
 					  &prog_idr, &prog_idr_lock);
 		break;
 	case BPF_MAP_GET_NEXT_ID:
-		err = bpf_obj_get_next_id(&attr, uattr.user,
+		err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
 					  &map_idr, &map_idr_lock);
 		break;
 	case BPF_BTF_GET_NEXT_ID:
-		err = bpf_obj_get_next_id(&attr, uattr.user,
+		err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
 					  &btf_idr, &btf_idr_lock);
 		break;
 	case BPF_PROG_GET_FD_BY_ID:
@@ -5158,7 +5155,7 @@ static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
 		err = bpf_link_get_fd_by_id(&attr);
 		break;
 	case BPF_LINK_GET_NEXT_ID:
-		err = bpf_obj_get_next_id(&attr, uattr.user,
+		err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
 					  &link_idr, &link_idr_lock);
 		break;
 	case BPF_ENABLE_STATS:
-- 
2.34.1

[PATCH RFC bpf-next 3/3] libbpf: use new bpf_xxx_attr structs for bpf() commands

[Andrii Nakryiko]

Adapt libbpf code to new named bpf_attr substructs. The only exception
is skel_internal.h which is left as is to not harm users that might have
old system-wide bpf.h UAPI headers installed.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/bpf.c             | 355 ++++++++++++++++----------------
 tools/lib/bpf/gen_loader.c      |  78 +++----
 tools/lib/bpf/libbpf.c          |   4 +-
 tools/lib/bpf/libbpf_internal.h |   2 +-
 4 files changed, 219 insertions(+), 220 deletions(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index ed86b37d8024..ab69fb2a4b24 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -69,13 +69,13 @@ static inline __u64 ptr_to_u64(const void *ptr)
 	return (__u64) (unsigned long) ptr;
 }
 
-static inline int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr,
+static inline int sys_bpf(enum bpf_cmd cmd, void *attr,
 			  unsigned int size)
 {
 	return syscall(__NR_bpf, cmd, attr, size);
 }
 
-static inline int sys_bpf_fd(enum bpf_cmd cmd, union bpf_attr *attr,
+static inline int sys_bpf_fd(enum bpf_cmd cmd, void *attr,
 			     unsigned int size)
 {
 	int fd;
@@ -84,7 +84,7 @@ static inline int sys_bpf_fd(enum bpf_cmd cmd, union bpf_attr *attr,
 	return ensure_good_fd(fd);
 }
 
-int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
+int sys_bpf_prog_load(struct bpf_prog_load_attr *attr, unsigned int size, int attempts)
 {
 	int fd;
 
@@ -105,13 +105,13 @@ int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
  */
 int probe_memcg_account(void)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, attach_btf_obj_fd);
+	const size_t attr_sz = offsetofend(struct bpf_prog_load_attr, attach_btf_obj_fd);
 	struct bpf_insn insns[] = {
 		BPF_EMIT_CALL(BPF_FUNC_ktime_get_coarse_ns),
 		BPF_EXIT_INSN(),
 	};
 	size_t insn_cnt = ARRAY_SIZE(insns);
-	union bpf_attr attr;
+	struct bpf_prog_load_attr attr;
 	int prog_fd;
 
 	/* attempt loading freplace trying to use custom BTF */
@@ -169,17 +169,16 @@ int bpf_map_create(enum bpf_map_type map_type,
 		   __u32 max_entries,
 		   const struct bpf_map_create_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, map_extra);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_create_attr, map_extra);
+	struct bpf_map_create_attr attr;
 	int fd;
 
-	bump_rlimit_memlock();
-
-	memset(&attr, 0, attr_sz);
-
 	if (!OPTS_VALID(opts, bpf_map_create_opts))
 		return libbpf_err(-EINVAL);
 
+	bump_rlimit_memlock();
+
+	memset(&attr, 0, attr_sz);
 	attr.map_type = map_type;
 	if (map_name && kernel_supports(NULL, FEAT_PROG_NAME))
 		libbpf_strlcpy(attr.map_name, map_name, sizeof(attr.map_name));
@@ -232,20 +231,20 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
 		  const struct bpf_insn *insns, size_t insn_cnt,
 		  struct bpf_prog_load_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, log_true_size);
+	const size_t attr_sz = offsetofend(struct bpf_prog_load_attr, log_true_size);
 	void *finfo = NULL, *linfo = NULL;
 	const char *func_info, *line_info;
 	__u32 log_size, log_level, attach_prog_fd, attach_btf_obj_fd;
 	__u32 func_info_rec_size, line_info_rec_size;
 	int fd, attempts;
-	union bpf_attr attr;
+	struct bpf_prog_load_attr attr;
 	char *log_buf;
 
-	bump_rlimit_memlock();
-
 	if (!OPTS_VALID(opts, bpf_prog_load_opts))
 		return libbpf_err(-EINVAL);
 
+	bump_rlimit_memlock();
+
 	attempts = OPTS_GET(opts, attempts, 0);
 	if (attempts < 0)
 		return libbpf_err(-EINVAL);
@@ -380,8 +379,8 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
 int bpf_map_update_elem(int fd, const void *key, const void *value,
 			__u64 flags)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_elem_attr, flags);
+	struct bpf_map_elem_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -396,8 +395,8 @@ int bpf_map_update_elem(int fd, const void *key, const void *value,
 
 int bpf_map_lookup_elem(int fd, const void *key, void *value)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_elem_attr, flags);
+	struct bpf_map_elem_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -411,8 +410,8 @@ int bpf_map_lookup_elem(int fd, const void *key, void *value)
 
 int bpf_map_lookup_elem_flags(int fd, const void *key, void *value, __u64 flags)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_elem_attr, flags);
+	struct bpf_map_elem_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -427,8 +426,8 @@ int bpf_map_lookup_elem_flags(int fd, const void *key, void *value, __u64 flags)
 
 int bpf_map_lookup_and_delete_elem(int fd, const void *key, void *value)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_elem_attr, flags);
+	struct bpf_map_elem_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -442,8 +441,8 @@ int bpf_map_lookup_and_delete_elem(int fd, const void *key, void *value)
 
 int bpf_map_lookup_and_delete_elem_flags(int fd, const void *key, void *value, __u64 flags)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_elem_attr, flags);
+	struct bpf_map_elem_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -458,8 +457,8 @@ int bpf_map_lookup_and_delete_elem_flags(int fd, const void *key, void *value, _
 
 int bpf_map_delete_elem(int fd, const void *key)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_elem_attr, flags);
+	struct bpf_map_elem_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -472,8 +471,8 @@ int bpf_map_delete_elem(int fd, const void *key)
 
 int bpf_map_delete_elem_flags(int fd, const void *key, __u64 flags)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_elem_attr, flags);
+	struct bpf_map_elem_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -487,8 +486,8 @@ int bpf_map_delete_elem_flags(int fd, const void *key, __u64 flags)
 
 int bpf_map_get_next_key(int fd, const void *key, void *next_key)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, next_key);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_next_key_attr, next_key);
+	struct bpf_map_next_key_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -502,8 +501,8 @@ int bpf_map_get_next_key(int fd, const void *key, void *next_key)
 
 int bpf_map_freeze(int fd)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, map_fd);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_freeze_attr, map_fd);
+	struct bpf_map_freeze_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -518,25 +517,25 @@ static int bpf_map_batch_common(int cmd, int fd, void  *in_batch,
 				__u32 *count,
 				const struct bpf_map_batch_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, batch);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_map_batch_attr, flags);
+	struct bpf_map_batch_attr attr;
 	int ret;
 
 	if (!OPTS_VALID(opts, bpf_map_batch_opts))
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.batch.map_fd = fd;
-	attr.batch.in_batch = ptr_to_u64(in_batch);
-	attr.batch.out_batch = ptr_to_u64(out_batch);
-	attr.batch.keys = ptr_to_u64(keys);
-	attr.batch.values = ptr_to_u64(values);
-	attr.batch.count = *count;
-	attr.batch.elem_flags  = OPTS_GET(opts, elem_flags, 0);
-	attr.batch.flags = OPTS_GET(opts, flags, 0);
+	attr.map_fd = fd;
+	attr.in_batch = ptr_to_u64(in_batch);
+	attr.out_batch = ptr_to_u64(out_batch);
+	attr.keys = ptr_to_u64(keys);
+	attr.values = ptr_to_u64(values);
+	attr.count = *count;
+	attr.elem_flags  = OPTS_GET(opts, elem_flags, 0);
+	attr.flags = OPTS_GET(opts, flags, 0);
 
 	ret = sys_bpf(cmd, &attr, attr_sz);
-	*count = attr.batch.count;
+	*count = attr.count;
 
 	return libbpf_err_errno(ret);
 }
@@ -574,8 +573,8 @@ int bpf_map_update_batch(int fd, const void *keys, const void *values, __u32 *co
 
 int bpf_obj_pin_opts(int fd, const char *pathname, const struct bpf_obj_pin_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, path_fd);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_pin_attr, path_fd);
+	struct bpf_obj_pin_attr attr;
 	int ret;
 
 	if (!OPTS_VALID(opts, bpf_obj_pin_opts))
@@ -584,7 +583,7 @@ int bpf_obj_pin_opts(int fd, const char *pathname, const struct bpf_obj_pin_opts
 	memset(&attr, 0, attr_sz);
 	attr.path_fd = OPTS_GET(opts, path_fd, 0);
 	attr.pathname = ptr_to_u64((void *)pathname);
-	attr.file_flags = OPTS_GET(opts, file_flags, 0);
+	attr.flags = OPTS_GET(opts, file_flags, 0);
 	attr.bpf_fd = fd;
 
 	ret = sys_bpf(BPF_OBJ_PIN, &attr, attr_sz);
@@ -603,8 +602,8 @@ int bpf_obj_get(const char *pathname)
 
 int bpf_obj_get_opts(const char *pathname, const struct bpf_obj_get_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, path_fd);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_get_attr, path_fd);
+	struct bpf_obj_get_attr attr;
 	int fd;
 
 	if (!OPTS_VALID(opts, bpf_obj_get_opts))
@@ -613,7 +612,7 @@ int bpf_obj_get_opts(const char *pathname, const struct bpf_obj_get_opts *opts)
 	memset(&attr, 0, attr_sz);
 	attr.path_fd = OPTS_GET(opts, path_fd, 0);
 	attr.pathname = ptr_to_u64((void *)pathname);
-	attr.file_flags = OPTS_GET(opts, file_flags, 0);
+	attr.flags = OPTS_GET(opts, file_flags, 0);
 
 	fd = sys_bpf_fd(BPF_OBJ_GET, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -633,8 +632,8 @@ int bpf_prog_attach_opts(int prog_fd, int target_fd,
 			  enum bpf_attach_type type,
 			  const struct bpf_prog_attach_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, replace_bpf_fd);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_prog_attach_attr, replace_bpf_fd);
+	struct bpf_prog_attach_attr attr;
 	int ret;
 
 	if (!OPTS_VALID(opts, bpf_prog_attach_opts))
@@ -653,8 +652,8 @@ int bpf_prog_attach_opts(int prog_fd, int target_fd,
 
 int bpf_prog_detach(int target_fd, enum bpf_attach_type type)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, replace_bpf_fd);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_prog_detach_attr, attach_type);
+	struct bpf_prog_detach_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
@@ -667,13 +666,13 @@ int bpf_prog_detach(int target_fd, enum bpf_attach_type type)
 
 int bpf_prog_detach2(int prog_fd, int target_fd, enum bpf_attach_type type)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, replace_bpf_fd);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_prog_detach_attr, attach_type);
+	struct bpf_prog_detach_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
-	attr.target_fd	 = target_fd;
-	attr.attach_bpf_fd = prog_fd;
+	attr.target_fd  = target_fd;
+	attr.prog_fd = prog_fd;
 	attr.attach_type = type;
 
 	ret = sys_bpf(BPF_PROG_DETACH, &attr, attr_sz);
@@ -684,9 +683,9 @@ int bpf_link_create(int prog_fd, int target_fd,
 		    enum bpf_attach_type attach_type,
 		    const struct bpf_link_create_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, link_create);
+	const size_t attr_sz = sizeof(struct bpf_link_create_attr);
 	__u32 target_btf_id, iter_info_len;
-	union bpf_attr attr;
+	struct bpf_link_create_attr attr;
 	int fd, err;
 
 	if (!OPTS_VALID(opts, bpf_link_create_opts))
@@ -704,32 +703,32 @@ int bpf_link_create(int prog_fd, int target_fd,
 	}
 
 	memset(&attr, 0, attr_sz);
-	attr.link_create.prog_fd = prog_fd;
-	attr.link_create.target_fd = target_fd;
-	attr.link_create.attach_type = attach_type;
-	attr.link_create.flags = OPTS_GET(opts, flags, 0);
+	attr.prog_fd = prog_fd;
+	attr.target_fd = target_fd;
+	attr.attach_type = attach_type;
+	attr.flags = OPTS_GET(opts, flags, 0);
 
 	if (target_btf_id) {
-		attr.link_create.target_btf_id = target_btf_id;
+		attr.target_btf_id = target_btf_id;
 		goto proceed;
 	}
 
 	switch (attach_type) {
 	case BPF_TRACE_ITER:
-		attr.link_create.iter_info = ptr_to_u64(OPTS_GET(opts, iter_info, (void *)0));
-		attr.link_create.iter_info_len = iter_info_len;
+		attr.iter_info = ptr_to_u64(OPTS_GET(opts, iter_info, (void *)0));
+		attr.iter_info_len = iter_info_len;
 		break;
 	case BPF_PERF_EVENT:
-		attr.link_create.perf_event.bpf_cookie = OPTS_GET(opts, perf_event.bpf_cookie, 0);
+		attr.perf_event.bpf_cookie = OPTS_GET(opts, perf_event.bpf_cookie, 0);
 		if (!OPTS_ZEROED(opts, perf_event))
 			return libbpf_err(-EINVAL);
 		break;
 	case BPF_TRACE_KPROBE_MULTI:
-		attr.link_create.kprobe_multi.flags = OPTS_GET(opts, kprobe_multi.flags, 0);
-		attr.link_create.kprobe_multi.cnt = OPTS_GET(opts, kprobe_multi.cnt, 0);
-		attr.link_create.kprobe_multi.syms = ptr_to_u64(OPTS_GET(opts, kprobe_multi.syms, 0));
-		attr.link_create.kprobe_multi.addrs = ptr_to_u64(OPTS_GET(opts, kprobe_multi.addrs, 0));
-		attr.link_create.kprobe_multi.cookies = ptr_to_u64(OPTS_GET(opts, kprobe_multi.cookies, 0));
+		attr.kprobe_multi.flags = OPTS_GET(opts, kprobe_multi.flags, 0);
+		attr.kprobe_multi.cnt = OPTS_GET(opts, kprobe_multi.cnt, 0);
+		attr.kprobe_multi.syms = ptr_to_u64(OPTS_GET(opts, kprobe_multi.syms, 0));
+		attr.kprobe_multi.addrs = ptr_to_u64(OPTS_GET(opts, kprobe_multi.addrs, 0));
+		attr.kprobe_multi.cookies = ptr_to_u64(OPTS_GET(opts, kprobe_multi.cookies, 0));
 		if (!OPTS_ZEROED(opts, kprobe_multi))
 			return libbpf_err(-EINVAL);
 		break;
@@ -737,7 +736,7 @@ int bpf_link_create(int prog_fd, int target_fd,
 	case BPF_TRACE_FEXIT:
 	case BPF_MODIFY_RETURN:
 	case BPF_LSM_MAC:
-		attr.link_create.tracing.cookie = OPTS_GET(opts, tracing.cookie, 0);
+		attr.tracing.cookie = OPTS_GET(opts, tracing.cookie, 0);
 		if (!OPTS_ZEROED(opts, tracing))
 			return libbpf_err(-EINVAL);
 		break;
@@ -760,7 +759,7 @@ int bpf_link_create(int prog_fd, int target_fd,
 	/* if user used features not supported by
 	 * BPF_RAW_TRACEPOINT_OPEN command, then just give up immediately
 	 */
-	if (attr.link_create.target_fd || attr.link_create.target_btf_id)
+	if (attr.target_fd || attr.target_btf_id)
 		return libbpf_err(err);
 	if (!OPTS_ZEROED(opts, sz))
 		return libbpf_err(err);
@@ -783,12 +782,12 @@ int bpf_link_create(int prog_fd, int target_fd,
 
 int bpf_link_detach(int link_fd)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, link_detach);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_link_detach_attr, link_fd);
+	struct bpf_link_detach_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
-	attr.link_detach.link_fd = link_fd;
+	attr.link_fd = link_fd;
 
 	ret = sys_bpf(BPF_LINK_DETACH, &attr, attr_sz);
 	return libbpf_err_errno(ret);
@@ -797,8 +796,8 @@ int bpf_link_detach(int link_fd)
 int bpf_link_update(int link_fd, int new_prog_fd,
 		    const struct bpf_link_update_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, link_update);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_link_update_attr, old_prog_fd);
+	struct bpf_link_update_attr attr;
 	int ret;
 
 	if (!OPTS_VALID(opts, bpf_link_update_opts))
@@ -808,13 +807,13 @@ int bpf_link_update(int link_fd, int new_prog_fd,
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.link_update.link_fd = link_fd;
-	attr.link_update.new_prog_fd = new_prog_fd;
-	attr.link_update.flags = OPTS_GET(opts, flags, 0);
+	attr.link_fd = link_fd;
+	attr.new_prog_fd = new_prog_fd;
+	attr.flags = OPTS_GET(opts, flags, 0);
 	if (OPTS_GET(opts, old_prog_fd, 0))
-		attr.link_update.old_prog_fd = OPTS_GET(opts, old_prog_fd, 0);
+		attr.old_prog_fd = OPTS_GET(opts, old_prog_fd, 0);
 	else if (OPTS_GET(opts, old_map_fd, 0))
-		attr.link_update.old_map_fd = OPTS_GET(opts, old_map_fd, 0);
+		attr.old_map_fd = OPTS_GET(opts, old_map_fd, 0);
 
 	ret = sys_bpf(BPF_LINK_UPDATE, &attr, attr_sz);
 	return libbpf_err_errno(ret);
@@ -822,12 +821,12 @@ int bpf_link_update(int link_fd, int new_prog_fd,
 
 int bpf_iter_create(int link_fd)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, iter_create);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_iter_create_attr, flags);
+	struct bpf_iter_create_attr attr;
 	int fd;
 
 	memset(&attr, 0, attr_sz);
-	attr.iter_create.link_fd = link_fd;
+	attr.link_fd = link_fd;
 
 	fd = sys_bpf_fd(BPF_ITER_CREATE, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -837,8 +836,8 @@ int bpf_prog_query_opts(int target_fd,
 			enum bpf_attach_type type,
 			struct bpf_prog_query_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, query);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_prog_query_attr, prog_attach_flags);
+	struct bpf_prog_query_attr attr;
 	int ret;
 
 	if (!OPTS_VALID(opts, bpf_prog_query_opts))
@@ -846,17 +845,17 @@ int bpf_prog_query_opts(int target_fd,
 
 	memset(&attr, 0, attr_sz);
 
-	attr.query.target_fd	= target_fd;
-	attr.query.attach_type	= type;
-	attr.query.query_flags	= OPTS_GET(opts, query_flags, 0);
-	attr.query.prog_cnt	= OPTS_GET(opts, prog_cnt, 0);
-	attr.query.prog_ids	= ptr_to_u64(OPTS_GET(opts, prog_ids, NULL));
-	attr.query.prog_attach_flags = ptr_to_u64(OPTS_GET(opts, prog_attach_flags, NULL));
+	attr.target_fd	= target_fd;
+	attr.attach_type	= type;
+	attr.query_flags	= OPTS_GET(opts, query_flags, 0);
+	attr.prog_cnt	= OPTS_GET(opts, prog_cnt, 0);
+	attr.prog_ids	= ptr_to_u64(OPTS_GET(opts, prog_ids, NULL));
+	attr.prog_attach_flags = ptr_to_u64(OPTS_GET(opts, prog_attach_flags, NULL));
 
 	ret = sys_bpf(BPF_PROG_QUERY, &attr, attr_sz);
 
-	OPTS_SET(opts, attach_flags, attr.query.attach_flags);
-	OPTS_SET(opts, prog_cnt, attr.query.prog_cnt);
+	OPTS_SET(opts, attach_flags, attr.attach_flags);
+	OPTS_SET(opts, prog_cnt, attr.prog_cnt);
 
 	return libbpf_err_errno(ret);
 }
@@ -882,43 +881,43 @@ int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
 
 int bpf_prog_test_run_opts(int prog_fd, struct bpf_test_run_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, test);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_prog_run_attr, batch_size);
+	struct bpf_prog_run_attr attr;
 	int ret;
 
 	if (!OPTS_VALID(opts, bpf_test_run_opts))
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.test.prog_fd = prog_fd;
-	attr.test.batch_size = OPTS_GET(opts, batch_size, 0);
-	attr.test.cpu = OPTS_GET(opts, cpu, 0);
-	attr.test.flags = OPTS_GET(opts, flags, 0);
-	attr.test.repeat = OPTS_GET(opts, repeat, 0);
-	attr.test.duration = OPTS_GET(opts, duration, 0);
-	attr.test.ctx_size_in = OPTS_GET(opts, ctx_size_in, 0);
-	attr.test.ctx_size_out = OPTS_GET(opts, ctx_size_out, 0);
-	attr.test.data_size_in = OPTS_GET(opts, data_size_in, 0);
-	attr.test.data_size_out = OPTS_GET(opts, data_size_out, 0);
-	attr.test.ctx_in = ptr_to_u64(OPTS_GET(opts, ctx_in, NULL));
-	attr.test.ctx_out = ptr_to_u64(OPTS_GET(opts, ctx_out, NULL));
-	attr.test.data_in = ptr_to_u64(OPTS_GET(opts, data_in, NULL));
-	attr.test.data_out = ptr_to_u64(OPTS_GET(opts, data_out, NULL));
+	attr.prog_fd = prog_fd;
+	attr.batch_size = OPTS_GET(opts, batch_size, 0);
+	attr.cpu = OPTS_GET(opts, cpu, 0);
+	attr.flags = OPTS_GET(opts, flags, 0);
+	attr.repeat = OPTS_GET(opts, repeat, 0);
+	attr.duration = OPTS_GET(opts, duration, 0);
+	attr.ctx_size_in = OPTS_GET(opts, ctx_size_in, 0);
+	attr.ctx_size_out = OPTS_GET(opts, ctx_size_out, 0);
+	attr.data_size_in = OPTS_GET(opts, data_size_in, 0);
+	attr.data_size_out = OPTS_GET(opts, data_size_out, 0);
+	attr.ctx_in = ptr_to_u64(OPTS_GET(opts, ctx_in, NULL));
+	attr.ctx_out = ptr_to_u64(OPTS_GET(opts, ctx_out, NULL));
+	attr.data_in = ptr_to_u64(OPTS_GET(opts, data_in, NULL));
+	attr.data_out = ptr_to_u64(OPTS_GET(opts, data_out, NULL));
 
 	ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, attr_sz);
 
-	OPTS_SET(opts, data_size_out, attr.test.data_size_out);
-	OPTS_SET(opts, ctx_size_out, attr.test.ctx_size_out);
-	OPTS_SET(opts, duration, attr.test.duration);
-	OPTS_SET(opts, retval, attr.test.retval);
+	OPTS_SET(opts, data_size_out, attr.data_size_out);
+	OPTS_SET(opts, ctx_size_out, attr.ctx_size_out);
+	OPTS_SET(opts, duration, attr.duration);
+	OPTS_SET(opts, retval, attr.retval);
 
 	return libbpf_err_errno(ret);
 }
 
 static int bpf_obj_get_next_id(__u32 start_id, __u32 *next_id, int cmd)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_next_id_attr, next_id);
+	struct bpf_obj_next_id_attr attr;
 	int err;
 
 	memset(&attr, 0, attr_sz);
@@ -954,16 +953,16 @@ int bpf_link_get_next_id(__u32 start_id, __u32 *next_id)
 int bpf_prog_get_fd_by_id_opts(__u32 id,
 			       const struct bpf_get_fd_by_id_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_fd_by_id_attr, flags);
+	struct bpf_obj_fd_by_id_attr attr;
 	int fd;
 
 	if (!OPTS_VALID(opts, bpf_get_fd_by_id_opts))
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.prog_id = id;
-	attr.open_flags = OPTS_GET(opts, open_flags, 0);
+	attr.obj_id = id;
+	attr.flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_PROG_GET_FD_BY_ID, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -977,16 +976,16 @@ int bpf_prog_get_fd_by_id(__u32 id)
 int bpf_map_get_fd_by_id_opts(__u32 id,
 			      const struct bpf_get_fd_by_id_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_fd_by_id_attr, flags);
+	struct bpf_obj_fd_by_id_attr attr;
 	int fd;
 
 	if (!OPTS_VALID(opts, bpf_get_fd_by_id_opts))
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.map_id = id;
-	attr.open_flags = OPTS_GET(opts, open_flags, 0);
+	attr.obj_id = id;
+	attr.flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_MAP_GET_FD_BY_ID, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -1000,16 +999,16 @@ int bpf_map_get_fd_by_id(__u32 id)
 int bpf_btf_get_fd_by_id_opts(__u32 id,
 			      const struct bpf_get_fd_by_id_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_fd_by_id_attr, flags);
+	struct bpf_obj_fd_by_id_attr attr;
 	int fd;
 
 	if (!OPTS_VALID(opts, bpf_get_fd_by_id_opts))
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.btf_id = id;
-	attr.open_flags = OPTS_GET(opts, open_flags, 0);
+	attr.obj_id = id;
+	attr.flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_BTF_GET_FD_BY_ID, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -1023,16 +1022,16 @@ int bpf_btf_get_fd_by_id(__u32 id)
 int bpf_link_get_fd_by_id_opts(__u32 id,
 			       const struct bpf_get_fd_by_id_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_fd_by_id_attr, flags);
+	struct bpf_obj_fd_by_id_attr attr;
 	int fd;
 
 	if (!OPTS_VALID(opts, bpf_get_fd_by_id_opts))
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.link_id = id;
-	attr.open_flags = OPTS_GET(opts, open_flags, 0);
+	attr.obj_id = id;
+	attr.flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_LINK_GET_FD_BY_ID, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -1045,18 +1044,18 @@ int bpf_link_get_fd_by_id(__u32 id)
 
 int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, info);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_obj_info_by_fd_attr, info);
+	struct bpf_obj_info_by_fd_attr attr;
 	int err;
 
 	memset(&attr, 0, attr_sz);
-	attr.info.bpf_fd = bpf_fd;
-	attr.info.info_len = *info_len;
-	attr.info.info = ptr_to_u64(info);
+	attr.bpf_fd = bpf_fd;
+	attr.info_len = *info_len;
+	attr.info = ptr_to_u64(info);
 
 	err = sys_bpf(BPF_OBJ_GET_INFO_BY_FD, &attr, attr_sz);
 	if (!err)
-		*info_len = attr.info.info_len;
+		*info_len = attr.info_len;
 	return libbpf_err_errno(err);
 }
 
@@ -1082,13 +1081,13 @@ int bpf_link_get_info_by_fd(int link_fd, struct bpf_link_info *info, __u32 *info
 
 int bpf_raw_tracepoint_open(const char *name, int prog_fd)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, raw_tracepoint);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_raw_tp_open_attr, prog_fd);
+	struct bpf_raw_tp_open_attr attr;
 	int fd;
 
 	memset(&attr, 0, attr_sz);
-	attr.raw_tracepoint.name = ptr_to_u64(name);
-	attr.raw_tracepoint.prog_fd = prog_fd;
+	attr.name = ptr_to_u64(name);
+	attr.prog_fd = prog_fd;
 
 	fd = sys_bpf_fd(BPF_RAW_TRACEPOINT_OPEN, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -1096,20 +1095,20 @@ int bpf_raw_tracepoint_open(const char *name, int prog_fd)
 
 int bpf_btf_load(const void *btf_data, size_t btf_size, struct bpf_btf_load_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, btf_log_true_size);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_btf_load_attr, log_true_size);
+	struct bpf_btf_load_attr attr;
 	char *log_buf;
 	size_t log_size;
 	__u32 log_level;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_btf_load_opts))
+		return libbpf_err(-EINVAL);
+
 	bump_rlimit_memlock();
 
 	memset(&attr, 0, attr_sz);
 
-	if (!OPTS_VALID(opts, bpf_btf_load_opts))
-		return libbpf_err(-EINVAL);
-
 	log_buf = OPTS_GET(opts, log_buf, NULL);
 	log_size = OPTS_GET(opts, log_size, 0);
 	log_level = OPTS_GET(opts, log_level, 0);
@@ -1127,20 +1126,20 @@ int bpf_btf_load(const void *btf_data, size_t btf_size, struct bpf_btf_load_opts
 	 * APIs within libbpf and provides a sensible behavior in practice
 	 */
 	if (log_level) {
-		attr.btf_log_buf = ptr_to_u64(log_buf);
-		attr.btf_log_size = (__u32)log_size;
-		attr.btf_log_level = log_level;
+		attr.log_buf = ptr_to_u64(log_buf);
+		attr.log_size = (__u32)log_size;
+		attr.log_level = log_level;
 	}
 
 	fd = sys_bpf_fd(BPF_BTF_LOAD, &attr, attr_sz);
 	if (fd < 0 && log_buf && log_level == 0) {
-		attr.btf_log_buf = ptr_to_u64(log_buf);
-		attr.btf_log_size = (__u32)log_size;
-		attr.btf_log_level = 1;
+		attr.log_buf = ptr_to_u64(log_buf);
+		attr.log_size = (__u32)log_size;
+		attr.log_level = 1;
 		fd = sys_bpf_fd(BPF_BTF_LOAD, &attr, attr_sz);
 	}
 
-	OPTS_SET(opts, log_true_size, attr.btf_log_true_size);
+	OPTS_SET(opts, log_true_size, attr.log_true_size);
 	return libbpf_err_errno(fd);
 }
 
@@ -1148,36 +1147,36 @@ int bpf_task_fd_query(int pid, int fd, __u32 flags, char *buf, __u32 *buf_len,
 		      __u32 *prog_id, __u32 *fd_type, __u64 *probe_offset,
 		      __u64 *probe_addr)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, task_fd_query);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_task_fd_query_attr, probe_addr);
+	struct bpf_task_fd_query_attr attr;
 	int err;
 
 	memset(&attr, 0, attr_sz);
-	attr.task_fd_query.pid = pid;
-	attr.task_fd_query.fd = fd;
-	attr.task_fd_query.flags = flags;
-	attr.task_fd_query.buf = ptr_to_u64(buf);
-	attr.task_fd_query.buf_len = *buf_len;
+	attr.pid = pid;
+	attr.fd = fd;
+	attr.flags = flags;
+	attr.buf = ptr_to_u64(buf);
+	attr.buf_len = *buf_len;
 
 	err = sys_bpf(BPF_TASK_FD_QUERY, &attr, attr_sz);
 
-	*buf_len = attr.task_fd_query.buf_len;
-	*prog_id = attr.task_fd_query.prog_id;
-	*fd_type = attr.task_fd_query.fd_type;
-	*probe_offset = attr.task_fd_query.probe_offset;
-	*probe_addr = attr.task_fd_query.probe_addr;
+	*buf_len = attr.buf_len;
+	*prog_id = attr.prog_id;
+	*fd_type = attr.fd_type;
+	*probe_offset = attr.probe_offset;
+	*probe_addr = attr.probe_addr;
 
 	return libbpf_err_errno(err);
 }
 
 int bpf_enable_stats(enum bpf_stats_type type)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, enable_stats);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_enable_stats_attr, type);
+	struct bpf_enable_stats_attr attr;
 	int fd;
 
 	memset(&attr, 0, attr_sz);
-	attr.enable_stats.type = type;
+	attr.type = type;
 
 	fd = sys_bpf_fd(BPF_ENABLE_STATS, &attr, attr_sz);
 	return libbpf_err_errno(fd);
@@ -1186,17 +1185,17 @@ int bpf_enable_stats(enum bpf_stats_type type)
 int bpf_prog_bind_map(int prog_fd, int map_fd,
 		      const struct bpf_prog_bind_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, prog_bind_map);
-	union bpf_attr attr;
+	const size_t attr_sz = offsetofend(struct bpf_prog_bind_map_attr, flags);
+	struct bpf_prog_bind_map_attr attr;
 	int ret;
 
 	if (!OPTS_VALID(opts, bpf_prog_bind_opts))
 		return libbpf_err(-EINVAL);
 
 	memset(&attr, 0, attr_sz);
-	attr.prog_bind_map.prog_fd = prog_fd;
-	attr.prog_bind_map.map_fd = map_fd;
-	attr.prog_bind_map.flags = OPTS_GET(opts, flags, 0);
+	attr.prog_fd = prog_fd;
+	attr.map_fd = map_fd;
+	attr.flags = OPTS_GET(opts, flags, 0);
 
 	ret = sys_bpf(BPF_PROG_BIND_MAP, &attr, attr_sz);
 	return libbpf_err_errno(ret);
diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index cf3323fd47b8..f6c540fb6250 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -417,9 +417,9 @@ void bpf_gen__free(struct bpf_gen *gen)
 void bpf_gen__load_btf(struct bpf_gen *gen, const void *btf_raw_data,
 		       __u32 btf_raw_size)
 {
-	int attr_size = offsetofend(union bpf_attr, btf_log_level);
+	int attr_size = offsetofend(struct bpf_btf_load_attr, log_level);
 	int btf_data, btf_load_attr;
-	union bpf_attr attr;
+	struct bpf_btf_load_attr attr;
 
 	memset(&attr, 0, attr_size);
 	pr_debug("gen: load_btf: size %d\n", btf_raw_size);
@@ -429,14 +429,14 @@ void bpf_gen__load_btf(struct bpf_gen *gen, const void *btf_raw_data,
 	btf_load_attr = add_data(gen, &attr, attr_size);
 
 	/* populate union bpf_attr with user provided log details */
-	move_ctx2blob(gen, attr_field(btf_load_attr, btf_log_level), 4,
+	move_ctx2blob(gen, attr_field(btf_load_attr, btf_load.log_level), 4,
 		      offsetof(struct bpf_loader_ctx, log_level), false);
-	move_ctx2blob(gen, attr_field(btf_load_attr, btf_log_size), 4,
+	move_ctx2blob(gen, attr_field(btf_load_attr, btf_load.log_size), 4,
 		      offsetof(struct bpf_loader_ctx, log_size), false);
-	move_ctx2blob(gen, attr_field(btf_load_attr, btf_log_buf), 8,
+	move_ctx2blob(gen, attr_field(btf_load_attr, btf_load.log_buf), 8,
 		      offsetof(struct bpf_loader_ctx, log_buf), false);
 	/* populate union bpf_attr with a pointer to the BTF data */
-	emit_rel_store(gen, attr_field(btf_load_attr, btf), btf_data);
+	emit_rel_store(gen, attr_field(btf_load_attr, btf_load.btf), btf_data);
 	/* emit BTF_LOAD command */
 	emit_sys_bpf(gen, BPF_BTF_LOAD, btf_load_attr, attr_size);
 	debug_ret(gen, "btf_load size %d", btf_raw_size);
@@ -451,10 +451,10 @@ void bpf_gen__map_create(struct bpf_gen *gen,
 			 __u32 key_size, __u32 value_size, __u32 max_entries,
 			 struct bpf_map_create_opts *map_attr, int map_idx)
 {
-	int attr_size = offsetofend(union bpf_attr, map_extra);
+	int attr_size = offsetofend(struct bpf_map_create_attr, map_extra);
 	bool close_inner_map_fd = false;
 	int map_create_attr, idx;
-	union bpf_attr attr;
+	struct bpf_map_create_attr attr;
 
 	memset(&attr, 0, attr_size);
 	attr.map_type = map_type;
@@ -476,12 +476,12 @@ void bpf_gen__map_create(struct bpf_gen *gen,
 	map_create_attr = add_data(gen, &attr, attr_size);
 	if (attr.btf_value_type_id)
 		/* populate union bpf_attr with btf_fd saved in the stack earlier */
-		move_stack2blob(gen, attr_field(map_create_attr, btf_fd), 4,
+		move_stack2blob(gen, attr_field(map_create_attr, map_create.btf_fd), 4,
 				stack_off(btf_fd));
 	switch (attr.map_type) {
 	case BPF_MAP_TYPE_ARRAY_OF_MAPS:
 	case BPF_MAP_TYPE_HASH_OF_MAPS:
-		move_stack2blob(gen, attr_field(map_create_attr, inner_map_fd), 4,
+		move_stack2blob(gen, attr_field(map_create_attr, map_create.inner_map_fd), 4,
 				stack_off(inner_map_fd));
 		close_inner_map_fd = true;
 		break;
@@ -490,7 +490,7 @@ void bpf_gen__map_create(struct bpf_gen *gen,
 	}
 	/* conditionally update max_entries */
 	if (map_idx >= 0)
-		move_ctx2blob(gen, attr_field(map_create_attr, max_entries), 4,
+		move_ctx2blob(gen, attr_field(map_create_attr, map_create.max_entries), 4,
 			      sizeof(struct bpf_loader_ctx) +
 			      sizeof(struct bpf_map_desc) * map_idx +
 			      offsetof(struct bpf_map_desc, max_entries),
@@ -937,8 +937,8 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
 			struct bpf_prog_load_opts *load_attr, int prog_idx)
 {
 	int prog_load_attr, license_off, insns_off, func_info, line_info, core_relos;
-	int attr_size = offsetofend(union bpf_attr, core_relo_rec_size);
-	union bpf_attr attr;
+	int attr_size = offsetofend(struct bpf_prog_load_attr, core_relo_rec_size);
+	struct bpf_prog_load_attr attr;
 
 	memset(&attr, 0, attr_size);
 	pr_debug("gen: prog_load: type %d insns_cnt %zd progi_idx %d\n",
@@ -975,32 +975,32 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
 	prog_load_attr = add_data(gen, &attr, attr_size);
 
 	/* populate union bpf_attr with a pointer to license */
-	emit_rel_store(gen, attr_field(prog_load_attr, license), license_off);
+	emit_rel_store(gen, attr_field(prog_load_attr, prog_load.license), license_off);
 
 	/* populate union bpf_attr with a pointer to instructions */
-	emit_rel_store(gen, attr_field(prog_load_attr, insns), insns_off);
+	emit_rel_store(gen, attr_field(prog_load_attr, prog_load.insns), insns_off);
 
 	/* populate union bpf_attr with a pointer to func_info */
-	emit_rel_store(gen, attr_field(prog_load_attr, func_info), func_info);
+	emit_rel_store(gen, attr_field(prog_load_attr, prog_load.func_info), func_info);
 
 	/* populate union bpf_attr with a pointer to line_info */
-	emit_rel_store(gen, attr_field(prog_load_attr, line_info), line_info);
+	emit_rel_store(gen, attr_field(prog_load_attr, prog_load.line_info), line_info);
 
 	/* populate union bpf_attr with a pointer to core_relos */
-	emit_rel_store(gen, attr_field(prog_load_attr, core_relos), core_relos);
+	emit_rel_store(gen, attr_field(prog_load_attr, prog_load.core_relos), core_relos);
 
 	/* populate union bpf_attr fd_array with a pointer to data where map_fds are saved */
-	emit_rel_store(gen, attr_field(prog_load_attr, fd_array), gen->fd_array);
+	emit_rel_store(gen, attr_field(prog_load_attr, prog_load.fd_array), gen->fd_array);
 
 	/* populate union bpf_attr with user provided log details */
-	move_ctx2blob(gen, attr_field(prog_load_attr, log_level), 4,
+	move_ctx2blob(gen, attr_field(prog_load_attr, prog_load.log_level), 4,
 		      offsetof(struct bpf_loader_ctx, log_level), false);
-	move_ctx2blob(gen, attr_field(prog_load_attr, log_size), 4,
+	move_ctx2blob(gen, attr_field(prog_load_attr, prog_load.log_size), 4,
 		      offsetof(struct bpf_loader_ctx, log_size), false);
-	move_ctx2blob(gen, attr_field(prog_load_attr, log_buf), 8,
+	move_ctx2blob(gen, attr_field(prog_load_attr, prog_load.log_buf), 8,
 		      offsetof(struct bpf_loader_ctx, log_buf), false);
 	/* populate union bpf_attr with btf_fd saved in the stack earlier */
-	move_stack2blob(gen, attr_field(prog_load_attr, prog_btf_fd), 4,
+	move_stack2blob(gen, attr_field(prog_load_attr, prog_load.prog_btf_fd), 4,
 			stack_off(btf_fd));
 	if (gen->attach_kind) {
 		emit_find_attach_target(gen);
@@ -1008,10 +1008,10 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
 		emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_0, BPF_PSEUDO_MAP_IDX_VALUE,
 						 0, 0, 0, prog_load_attr));
 		emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_7,
-				      offsetof(union bpf_attr, attach_btf_id)));
+				      offsetof(union bpf_attr, prog_load.attach_btf_id)));
 		emit(gen, BPF_ALU64_IMM(BPF_RSH, BPF_REG_7, 32));
 		emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_7,
-				      offsetof(union bpf_attr, attach_btf_obj_fd)));
+				      offsetof(union bpf_attr, prog_load.attach_btf_obj_fd)));
 	}
 	emit_relos(gen, insns_off);
 	/* emit PROG_LOAD command */
@@ -1021,7 +1021,7 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
 	cleanup_relos(gen, insns_off);
 	if (gen->attach_kind) {
 		emit_sys_close_blob(gen,
-				    attr_field(prog_load_attr, attach_btf_obj_fd));
+				    attr_field(prog_load_attr, prog_load.attach_btf_obj_fd));
 		gen->attach_kind = 0;
 	}
 	emit_check_err(gen);
@@ -1034,9 +1034,9 @@ void bpf_gen__prog_load(struct bpf_gen *gen,
 void bpf_gen__map_update_elem(struct bpf_gen *gen, int map_idx, void *pvalue,
 			      __u32 value_size)
 {
-	int attr_size = offsetofend(union bpf_attr, flags);
+	int attr_size = offsetofend(struct bpf_map_elem_attr, flags);
 	int map_update_attr, value, key;
-	union bpf_attr attr;
+	struct bpf_map_elem_attr attr;
 	int zero = 0;
 
 	memset(&attr, 0, attr_size);
@@ -1068,10 +1068,10 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, int map_idx, void *pvalue,
 	emit(gen, BPF_EMIT_CALL(BPF_FUNC_probe_read_kernel));
 
 	map_update_attr = add_data(gen, &attr, attr_size);
-	move_blob2blob(gen, attr_field(map_update_attr, map_fd), 4,
+	move_blob2blob(gen, attr_field(map_update_attr, map_elem.map_fd), 4,
 		       blob_fd_array_off(gen, map_idx));
-	emit_rel_store(gen, attr_field(map_update_attr, key), key);
-	emit_rel_store(gen, attr_field(map_update_attr, value), value);
+	emit_rel_store(gen, attr_field(map_update_attr, map_elem.key), key);
+	emit_rel_store(gen, attr_field(map_update_attr, map_elem.value), value);
 	/* emit MAP_UPDATE_ELEM command */
 	emit_sys_bpf(gen, BPF_MAP_UPDATE_ELEM, map_update_attr, attr_size);
 	debug_ret(gen, "update_elem idx %d value_size %d", map_idx, value_size);
@@ -1081,9 +1081,9 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, int map_idx, void *pvalue,
 void bpf_gen__populate_outer_map(struct bpf_gen *gen, int outer_map_idx, int slot,
 				 int inner_map_idx)
 {
-	int attr_size = offsetofend(union bpf_attr, flags);
+	int attr_size = offsetofend(struct bpf_map_elem_attr, flags);
 	int map_update_attr, key;
-	union bpf_attr attr;
+	struct bpf_map_elem_attr attr;
 
 	memset(&attr, 0, attr_size);
 	pr_debug("gen: populate_outer_map: outer %d key %d inner %d\n",
@@ -1092,10 +1092,10 @@ void bpf_gen__populate_outer_map(struct bpf_gen *gen, int outer_map_idx, int slo
 	key = add_data(gen, &slot, sizeof(slot));
 
 	map_update_attr = add_data(gen, &attr, attr_size);
-	move_blob2blob(gen, attr_field(map_update_attr, map_fd), 4,
+	move_blob2blob(gen, attr_field(map_update_attr, map_elem.map_fd), 4,
 		       blob_fd_array_off(gen, outer_map_idx));
-	emit_rel_store(gen, attr_field(map_update_attr, key), key);
-	emit_rel_store(gen, attr_field(map_update_attr, value),
+	emit_rel_store(gen, attr_field(map_update_attr, map_elem.key), key);
+	emit_rel_store(gen, attr_field(map_update_attr, map_elem.value),
 		       blob_fd_array_off(gen, inner_map_idx));
 
 	/* emit MAP_UPDATE_ELEM command */
@@ -1107,14 +1107,14 @@ void bpf_gen__populate_outer_map(struct bpf_gen *gen, int outer_map_idx, int slo
 
 void bpf_gen__map_freeze(struct bpf_gen *gen, int map_idx)
 {
-	int attr_size = offsetofend(union bpf_attr, map_fd);
+	int attr_size = offsetofend(struct bpf_map_freeze_attr, map_fd);
 	int map_freeze_attr;
-	union bpf_attr attr;
+	struct bpf_map_freeze_attr attr;
 
 	memset(&attr, 0, attr_size);
 	pr_debug("gen: map_freeze: idx %d\n", map_idx);
 	map_freeze_attr = add_data(gen, &attr, attr_size);
-	move_blob2blob(gen, attr_field(map_freeze_attr, map_fd), 4,
+	move_blob2blob(gen, attr_field(map_freeze_attr, map_freeze.map_fd), 4,
 		       blob_fd_array_off(gen, map_idx));
 	/* emit MAP_FREEZE command */
 	emit_sys_bpf(gen, BPF_MAP_FREEZE, map_freeze_attr, attr_size);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5cca00979aae..113dbe9cb240 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4525,12 +4525,12 @@ static int probe_fd(int fd)
 
 static int probe_kern_prog_name(void)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, prog_name);
+	const size_t attr_sz = offsetofend(struct bpf_prog_load_attr, prog_name);
 	struct bpf_insn insns[] = {
 		BPF_MOV64_IMM(BPF_REG_0, 0),
 		BPF_EXIT_INSN(),
 	};
-	union bpf_attr attr;
+	struct bpf_prog_load_attr attr;
 	int ret;
 
 	memset(&attr, 0, attr_sz);
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index e4d05662a96c..9a809c610f36 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -575,6 +575,6 @@ static inline bool is_pow_of_2(size_t x)
 }
 
 #define PROG_LOAD_ATTEMPTS 5
-int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts);
+int sys_bpf_prog_load(struct bpf_prog_load_attr *attr, unsigned int size, int attempts);
 
 #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
-- 
2.34.1