[bpf-next,v6,3/5] bpftool: generated shadow variables for struct_ops maps.

Commit Message

Kui-Feng Lee Feb. 29, 2024, 6:45 a.m. UTC

Declares and defines a pointer of the shadow type for each struct_ops map.

The code generator will create an anonymous struct type as the shadow type
for each struct_ops map. The shadow type is translated from the original
struct type of the map. The user of the skeleton use pointers of them to
access the values of struct_ops maps.

However, shadow types only supports certain types of fields, including
scalar types and function pointers. Any fields of unsupported types are
translated into an array of characters to occupy the space of the original
field. Function pointers are translated into pointers of the struct
bpf_program. Additionally, padding fields are generated to occupy the space
between two consecutive fields.

The pointers of shadow types of struct_osp maps are initialized when
*__open_opts() in skeletons are called. For a map called FOO, the user can
access it through the pointer at skel->struct_ops.FOO.

Reviewed-by: Quentin Monnet <[e-mail]>
Signed-off-by: Kui-Feng Lee <[e-mail]>
---
 tools/bpf/bpftool/gen.c | 237 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 236 insertions(+), 1 deletion(-)

Comments

Andrii Nakryiko Feb. 29, 2024, 10:25 p.m. UTC | #1

On Wed, Feb 28, 2024 at 10:45 PM Kui-Feng Lee <[e-mail]> wrote:
>
> Declares and defines a pointer of the shadow type for each struct_ops map.
>
> The code generator will create an anonymous struct type as the shadow type
> for each struct_ops map. The shadow type is translated from the original
> struct type of the map. The user of the skeleton use pointers of them to
> access the values of struct_ops maps.
>
> However, shadow types only supports certain types of fields, including
> scalar types and function pointers. Any fields of unsupported types are
> translated into an array of characters to occupy the space of the original
> field. Function pointers are translated into pointers of the struct
> bpf_program. Additionally, padding fields are generated to occupy the space
> between two consecutive fields.
>
> The pointers of shadow types of struct_osp maps are initialized when
> *__open_opts() in skeletons are called. For a map called FOO, the user can
> access it through the pointer at skel->struct_ops.FOO.
>
> Reviewed-by: Quentin Monnet <[e-mail]>
> Signed-off-by: Kui-Feng Lee <[e-mail]>
> ---
>  tools/bpf/bpftool/gen.c | 237 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 236 insertions(+), 1 deletion(-)
>

[...]

> +/* Generate the pointer of the shadow type for a struct_ops map.
> + *
> + * This function adds a pointer of the shadow type for a struct_ops map.
> + * The members of a struct_ops map can be exported through a pointer to a
> + * shadow type. The user can access these members through the pointer.
> + *
> + * A shadow type includes not all members, only members of some types.
> + * They are scalar types and function pointers. The function pointers are
> + * translated to the pointer of the struct bpf_program. The scalar types
> + * are translated to the original type without any modifiers.
> + *
> + * Unsupported types will be translated to a char array to occupy the same
> + * space as the original field, being renamed as __unsupported_*.  The user
> + * should treat these fields as opaque data.
> + */
> +static int gen_st_ops_shadow_type(const char *obj_name, struct btf *btf, const char *ident,
> +                                 const struct bpf_map *map)
> +{
> +       const struct btf_type *map_type;
> +       const char *type_name;
> +       __u32 map_type_id;
> +       int err;
> +
> +       map_type_id = bpf_map__btf_value_type_id(map);
> +       if (map_type_id == 0)
> +               return -EINVAL;
> +       map_type = btf__type_by_id(btf, map_type_id);
> +       if (!map_type)
> +               return -EINVAL;
> +
> +       type_name = btf__name_by_offset(btf, map_type->name_off);
> +
> +       printf("\t\tstruct %s_%s_%s {\n", obj_name, ident, type_name);

it should be %s__%s__%s, I fixed it up

> +
> +       err = walk_st_ops_shadow_vars(btf, ident, map_type, map_type_id);
> +       if (err)
> +               return err;
> +
> +       printf("\t\t} *%s;\n", ident);
> +
> +       return 0;
> +}
> +
> +static int gen_st_ops_shadow(const char *obj_name, struct btf *btf, struct bpf_object *obj)
> +{
> +       int err, st_ops_cnt = 0;
> +       struct bpf_map *map;
> +       char ident[256];
> +
> +       if (!btf)
> +               return 0;
> +
> +       /* Generate the pointers to shadow types of
> +        * struct_ops maps.
> +        */
> +       bpf_object__for_each_map(map, obj) {
> +               if (bpf_map__type(map) != BPF_MAP_TYPE_STRUCT_OPS)
> +                       continue;
> +               if (!get_map_ident(map, ident, sizeof(ident)))
> +                       continue;
> +
> +               if (!st_ops_cnt++)

goodness, too much operator precedence knowledge assumed, I simplified
this to what I can follow myself:

if (st_ops_cnt == 0)
    printf(...);
st_ops_cnt++;

I don't think saving one line of code is worth it.

> +                       printf("\tstruct {\n");
> +
> +               err = gen_st_ops_shadow_type(obj_name, btf, ident, map);
> +               if (err)
> +                       return err;
> +       }
> +
> +       if (st_ops_cnt)
> +               printf("\t} struct_ops;\n");
> +
> +       return 0;
> +}
> +

[...]

Kui-Feng Lee March 1, 2024, 6:04 p.m. UTC | #2

On 2/29/24 14:25, Andrii Nakryiko wrote:
> On Wed, Feb 28, 2024 at 10:45 PM Kui-Feng Lee <[e-mail]> wrote:
>>
... skipped ...
>> +static int gen_st_ops_shadow(const char *obj_name, struct btf *btf, struct bpf_object *obj)
>> +{
>> +       int err, st_ops_cnt = 0;
>> +       struct bpf_map *map;
>> +       char ident[256];
>> +
>> +       if (!btf)
>> +               return 0;
>> +
>> +       /* Generate the pointers to shadow types of
>> +        * struct_ops maps.
>> +        */
>> +       bpf_object__for_each_map(map, obj) {
>> +               if (bpf_map__type(map) != BPF_MAP_TYPE_STRUCT_OPS)
>> +                       continue;
>> +               if (!get_map_ident(map, ident, sizeof(ident)))
>> +                       continue;
>> +
>> +               if (!st_ops_cnt++)
> 
> goodness, too much operator precedence knowledge assumed, I simplified
> this to what I can follow myself:
> 
> if (st_ops_cnt == 0)
>      printf(...);
> st_ops_cnt++;
> 
> I don't think saving one line of code is worth it.

It is totally reasonable.

Thanks for review!

Patch

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index a9334c57e859..da8ae75eb6cb 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -55,6 +55,20 @@  static bool str_has_suffix(const char *str, const char *suffix)
 	return true;
 }
 
+static const struct btf_type *
+resolve_func_ptr(const struct btf *btf, __u32 id, __u32 *res_id)
+{
+	const struct btf_type *t;
+
+	t = skip_mods_and_typedefs(btf, id, NULL);
+	if (!btf_is_ptr(t))
+		return NULL;
+
+	t = skip_mods_and_typedefs(btf, t->type, res_id);
+
+	return btf_is_func_proto(t) ? t : NULL;
+}
+
 static void get_obj_name(char *name, const char *file)
 {
 	char file_copy[PATH_MAX];
@@ -909,6 +923,206 @@  codegen_progs_skeleton(struct bpf_object *obj, size_t prog_cnt, bool populate_li
 	}
 }
 
+static int walk_st_ops_shadow_vars(struct btf *btf, const char *ident,
+				   const struct btf_type *map_type, __u32 map_type_id)
+{
+	LIBBPF_OPTS(btf_dump_emit_type_decl_opts, opts, .indent_level = 3);
+	const struct btf_type *member_type;
+	__u32 offset, next_offset = 0;
+	const struct btf_member *m;
+	struct btf_dump *d = NULL;
+	const char *member_name;
+	__u32 member_type_id;
+	int i, err = 0, n;
+	int size;
+
+	d = btf_dump__new(btf, codegen_btf_dump_printf, NULL, NULL);
+	if (!d)
+		return -errno;
+
+	n = btf_vlen(map_type);
+	for (i = 0, m = btf_members(map_type); i < n; i++, m++) {
+		member_type = skip_mods_and_typedefs(btf, m->type, &member_type_id);
+		member_name = btf__name_by_offset(btf, m->name_off);
+
+		offset = m->offset / 8;
+		if (next_offset < offset)
+			printf("\t\t\tchar __padding_%d[%d];\n", i, offset - next_offset);
+
+		switch (btf_kind(member_type)) {
+		case BTF_KIND_INT:
+		case BTF_KIND_FLOAT:
+		case BTF_KIND_ENUM:
+		case BTF_KIND_ENUM64:
+			/* scalar type */
+			printf("\t\t\t");
+			opts.field_name = member_name;
+			err = btf_dump__emit_type_decl(d, member_type_id, &opts);
+			if (err) {
+				p_err("Failed to emit type declaration for %s: %d", member_name, err);
+				goto out;
+			}
+			printf(";\n");
+
+			size = btf__resolve_size(btf, member_type_id);
+			if (size < 0) {
+				p_err("Failed to resolve size of %s: %d\n", member_name, size);
+				err = size;
+				goto out;
+			}
+
+			next_offset = offset + size;
+			break;
+
+		case BTF_KIND_PTR:
+			if (resolve_func_ptr(btf, m->type, NULL)) {
+				/* Function pointer */
+				printf("\t\t\tstruct bpf_program *%s;\n", member_name);
+
+				next_offset = offset + sizeof(void *);
+				break;
+			}
+			/* All pointer types are unsupported except for
+			 * function pointers.
+			 */
+			fallthrough;
+
+		default:
+			/* Unsupported types
+			 *
+			 * Types other than scalar types and function
+			 * pointers are currently not supported in order to
+			 * prevent conflicts in the generated code caused
+			 * by multiple definitions. For instance, if the
+			 * struct type FOO is used in a struct_ops map,
+			 * bpftool has to generate definitions for FOO,
+			 * which may result in conflicts if FOO is defined
+			 * in different skeleton files.
+			 */
+			size = btf__resolve_size(btf, member_type_id);
+			if (size < 0) {
+				p_err("Failed to resolve size of %s: %d\n", member_name, size);
+				err = size;
+				goto out;
+			}
+			printf("\t\t\tchar __unsupported_%d[%d];\n", i, size);
+
+			next_offset = offset + size;
+			break;
+		}
+	}
+
+	/* Can not fail since it must be a struct type */
+	size = btf__resolve_size(btf, map_type_id);
+	if (next_offset < (__u32)size)
+		printf("\t\t\tchar __padding_end[%d];\n", size - next_offset);
+
+out:
+	btf_dump__free(d);
+
+	return err;
+}
+
+/* Generate the pointer of the shadow type for a struct_ops map.
+ *
+ * This function adds a pointer of the shadow type for a struct_ops map.
+ * The members of a struct_ops map can be exported through a pointer to a
+ * shadow type. The user can access these members through the pointer.
+ *
+ * A shadow type includes not all members, only members of some types.
+ * They are scalar types and function pointers. The function pointers are
+ * translated to the pointer of the struct bpf_program. The scalar types
+ * are translated to the original type without any modifiers.
+ *
+ * Unsupported types will be translated to a char array to occupy the same
+ * space as the original field, being renamed as __unsupported_*.  The user
+ * should treat these fields as opaque data.
+ */
+static int gen_st_ops_shadow_type(const char *obj_name, struct btf *btf, const char *ident,
+				  const struct bpf_map *map)
+{
+	const struct btf_type *map_type;
+	const char *type_name;
+	__u32 map_type_id;
+	int err;
+
+	map_type_id = bpf_map__btf_value_type_id(map);
+	if (map_type_id == 0)
+		return -EINVAL;
+	map_type = btf__type_by_id(btf, map_type_id);
+	if (!map_type)
+		return -EINVAL;
+
+	type_name = btf__name_by_offset(btf, map_type->name_off);
+
+	printf("\t\tstruct %s_%s_%s {\n", obj_name, ident, type_name);
+
+	err = walk_st_ops_shadow_vars(btf, ident, map_type, map_type_id);
+	if (err)
+		return err;
+
+	printf("\t\t} *%s;\n", ident);
+
+	return 0;
+}
+
+static int gen_st_ops_shadow(const char *obj_name, struct btf *btf, struct bpf_object *obj)
+{
+	int err, st_ops_cnt = 0;
+	struct bpf_map *map;
+	char ident[256];
+
+	if (!btf)
+		return 0;
+
+	/* Generate the pointers to shadow types of
+	 * struct_ops maps.
+	 */
+	bpf_object__for_each_map(map, obj) {
+		if (bpf_map__type(map) != BPF_MAP_TYPE_STRUCT_OPS)
+			continue;
+		if (!get_map_ident(map, ident, sizeof(ident)))
+			continue;
+
+		if (!st_ops_cnt++)
+			printf("\tstruct {\n");
+
+		err = gen_st_ops_shadow_type(obj_name, btf, ident, map);
+		if (err)
+			return err;
+	}
+
+	if (st_ops_cnt)
+		printf("\t} struct_ops;\n");
+
+	return 0;
+}
+
+/* Generate the code to initialize the pointers of shadow types. */
+static void gen_st_ops_shadow_init(struct btf *btf, struct bpf_object *obj)
+{
+	struct bpf_map *map;
+	char ident[256];
+
+	if (!btf)
+		return;
+
+	/* Initialize the pointers to_ops shadow types of
+	 * struct_ops maps.
+	 */
+	bpf_object__for_each_map(map, obj) {
+		if (bpf_map__type(map) != BPF_MAP_TYPE_STRUCT_OPS)
+			continue;
+		if (!get_map_ident(map, ident, sizeof(ident)))
+			continue;
+		codegen("\
+			\n\
+				obj->struct_ops.%1$s = bpf_map__initial_value(obj->maps.%1$s, NULL);\n\
+			\n\
+			", ident);
+	}
+}
+
 static int do_skeleton(int argc, char **argv)
 {
 	char header_guard[MAX_OBJ_NAME_LEN + sizeof("__SKEL_H__")];
@@ -1039,6 +1253,8 @@  static int do_skeleton(int argc, char **argv)
 		);
 	}
 
+	btf = bpf_object__btf(obj);
+
 	if (map_cnt) {
 		printf("\tstruct {\n");
 		bpf_object__for_each_map(map, obj) {
@@ -1052,6 +1268,10 @@  static int do_skeleton(int argc, char **argv)
 		printf("\t} maps;\n");
 	}
 
+	err = gen_st_ops_shadow(obj_name, btf, obj);
+	if (err)
+		goto out;
+
 	if (prog_cnt) {
 		printf("\tstruct {\n");
 		bpf_object__for_each_program(prog, obj) {
@@ -1075,7 +1295,6 @@  static int do_skeleton(int argc, char **argv)
 		printf("\t} links;\n");
 	}
 
-	btf = bpf_object__btf(obj);
 	if (btf) {
 		err = codegen_datasecs(obj, obj_name);
 		if (err)
@@ -1133,6 +1352,12 @@  static int do_skeleton(int argc, char **argv)
 			if (err)					    \n\
 				goto err_out;				    \n\
 									    \n\
+		", obj_name);
+
+	gen_st_ops_shadow_init(btf, obj);
+
+	codegen("\
+		\n\
 			return obj;					    \n\
 		err_out:						    \n\
 			%1$s__destroy(obj);				    \n\
@@ -1442,6 +1667,10 @@  static int do_subskeleton(int argc, char **argv)
 		printf("\t} maps;\n");
 	}
 
+	err = gen_st_ops_shadow(obj_name, btf, obj);
+	if (err)
+		goto out;
+
 	if (prog_cnt) {
 		printf("\tstruct {\n");
 		bpf_object__for_each_program(prog, obj) {
@@ -1553,6 +1782,12 @@  static int do_subskeleton(int argc, char **argv)
 			if (err)					    \n\
 				goto err;				    \n\
 									    \n\
+		");
+
+	gen_st_ops_shadow_init(btf, obj);
+
+	codegen("\
+		\n\
 			return obj;					    \n\
 		err:							    \n\
 			%1$s__destroy(obj);				    \n\