Allow storage of flexible metadata information for eBPF programs

From: YiFei Zhu <[email protected]>

To support modifying the used_maps array, we use a mutex to protect the use of the counter and the array. The mutex is initialized right after the prog aux is allocated, and destroyed right before prog aux is freed. This way we guarantee it's initialized for both cBPF and eBPF.

Signed-off-by YiFei Zhu <[email protected]>

From: YiFei Zhu <[email protected]>

This syscall binds a map to a program. -EEXIST if the map is already bound to the program.

Signed-off-by YiFei Zhu <[email protected]>

From: YiFei Zhu <[email protected]>

The patch adds a simple wrapper bpf_prog_bind_map around the syscall. And when using libbpf to load a program, it will probe the kernel for the support of this syscall, and scan for the .metadata ELF section and load it as an internal map like a .data section.

In the case that kernel supports the BPF_PROG_BIND_MAP syscall and a .metadata section exists, the map will be explicitly bound to the program via the syscall immediately after program is loaded. -EEXIST is ignored for this syscall.

Signed-off-by YiFei Zhu <[email protected]>

---
  tools/lib/bpf/bpf.c      |  11 +++++
  tools/lib/bpf/bpf.h      |   1 +
  tools/lib/bpf/libbpf.c   | 100 ++++++++++++++++++++++++++++++++++++++-
  tools/lib/bpf/libbpf.map |   1 +
  4 files changed, 112 insertions(+), 1 deletion(-)

tools/lib/bpf/bpf.c +11 -0

index 82b983ff6569..383b29ecb1fd 100644
@@ -872,3 +872,14 @@ int bpf_enable_stats(enum bpf_stats_type type)
 
  	return sys_bpf(BPF_ENABLE_STATS, &attr, sizeof(attr));
  }
+
+int bpf_prog_bind_map(int prog_fd, int map_fd, int flags)
+{
+	union bpf_attr attr = {};
+
+	attr.prog_bind_map.prog_fd = prog_fd;
+	attr.prog_bind_map.map_fd = map_fd;
+	attr.prog_bind_map.flags = flags;
+
+	return sys_bpf(BPF_PROG_BIND_MAP, &attr, sizeof(attr));
+}

tools/lib/bpf/bpf.h +0 -0

index 015d13f25fcc..32994a4e0bf6 100644

tools/lib/bpf/bpf.h +0 -0

  #ifdef __cplusplus
  } /* extern "C" */
  #endif

tools/lib/bpf/libbpf.c +63 -1

index 77d420c02094..4725859099c5 100644
@@ -174,6 +174,8 @@ enum kern_feature_id {
  	FEAT_EXP_ATTACH_TYPE,
  	/* bpf_probe_read_{kernel,user}[_str] helpers */
  	FEAT_PROBE_READ_KERN,
+	/* bpf_prog_bind_map helper */
+	FEAT_PROG_BIND_MAP,
  	__FEAT_CNT,
  };
 

@@ -283,6 +285,7 @@ struct bpf_struct_ops {
  #define KCONFIG_SEC ".kconfig"
  #define KSYMS_SEC ".ksyms"
  #define STRUCT_OPS_SEC ".struct_ops"
+#define METADATA_SEC ".metadata"
 
  enum libbpf_map_type {
  	LIBBPF_MAP_UNSPEC,

@@ -290,6 +293,7 @@ enum libbpf_map_type {
  	LIBBPF_MAP_BSS,
  	LIBBPF_MAP_RODATA,
  	LIBBPF_MAP_KCONFIG,
+	LIBBPF_MAP_METADATA,
  };
 
  static const char * const libbpf_type_to_btf_name[] = {

@@ -297,6 +301,7 @@ static const char * const libbpf_type_to_btf_name[] = {
  	[LIBBPF_MAP_BSS]	= BSS_SEC,
  	[LIBBPF_MAP_RODATA]	= RODATA_SEC,
  	[LIBBPF_MAP_KCONFIG]	= KCONFIG_SEC,
+	[LIBBPF_MAP_METADATA]	= METADATA_SEC,
  };
 
  struct bpf_map {

@@ -375,6 +380,8 @@ struct bpf_object {
  	size_t nr_maps;
  	size_t maps_cap;
 
+	struct bpf_map *metadata_map;
+
  	char *kconfig;
  	struct extern_desc *externs;
  	int nr_extern;

@@ -398,6 +405,7 @@ struct bpf_object {
  		Elf_Data *rodata;
  		Elf_Data *bss;
  		Elf_Data *st_ops_data;
+		Elf_Data *metadata;
  		size_t strtabidx;
  		struct {
  			GElf_Shdr shdr;

@@ -413,6 +421,7 @@ struct bpf_object {
  		int rodata_shndx;
  		int bss_shndx;
  		int st_ops_shndx;
+		int metadata_shndx;
  	} efile;
  	/*
  	 * All loaded bpf_object is linked in a list, which is

@@ -1022,6 +1031,7 @@ static struct bpf_object *bpf_object__new(const char *path,
  	obj->efile.obj_buf_sz = obj_buf_sz;
  	obj->efile.maps_shndx = -1;
  	obj->efile.btf_maps_shndx = -1;
+	obj->efile.metadata_shndx = -1;
  	obj->efile.data_shndx = -1;
  	obj->efile.rodata_shndx = -1;
  	obj->efile.bss_shndx = -1;

@@ -1387,6 +1397,9 @@ bpf_object__init_internal_map(struct bpf_object *obj, enum libbpf_map_type type,
  	if (data)
  		memcpy(map->mmaped, data, data_sz);
 
+	if (type == LIBBPF_MAP_METADATA)
+		obj->metadata_map = map;
+
  	pr_debug("map %td is \"%s\"\n", map - obj->maps, map->name);
  	return 0;
  }

@@ -1422,6 +1435,14 @@ static int bpf_object__init_global_data_maps(struct bpf_object *obj)
  		if (err)
  			return err;
  	}
+	if (obj->efile.metadata_shndx >= 0) {
+		err = bpf_object__init_internal_map(obj, LIBBPF_MAP_METADATA,
+						    obj->efile.metadata_shndx,
+						    obj->efile.metadata->d_buf,
+						    obj->efile.metadata->d_size);
+		if (err)
+			return err;
+	}
  	return 0;
  }
 

@@ -2698,6 +2719,9 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
  			} else if (strcmp(name, STRUCT_OPS_SEC) == 0) {
  				obj->efile.st_ops_data = data;
  				obj->efile.st_ops_shndx = idx;
+			} else if (strcmp(name, METADATA_SEC) == 0) {
+				obj->efile.metadata = data;
+				obj->efile.metadata_shndx = idx;
  			} else {
  				pr_debug("skip section(%d) %s\n", idx, name);
  			}

@@ -3111,7 +3135,8 @@ static bool bpf_object__shndx_is_data(const struct bpf_object *obj,
  {
  	return shndx == obj->efile.data_shndx ||
  	       shndx == obj->efile.bss_shndx ||
-	       shndx == obj->efile.rodata_shndx;
+	       shndx == obj->efile.rodata_shndx ||
+	       shndx == obj->efile.metadata_shndx;
  }
 
  static bool bpf_object__shndx_is_maps(const struct bpf_object *obj,

@@ -3132,6 +3157,8 @@ bpf_object__section_to_libbpf_map_type(const struct bpf_object *obj, int shndx)
  		return LIBBPF_MAP_RODATA;
  	else if (shndx == obj->efile.symbols_shndx)
  		return LIBBPF_MAP_KCONFIG;
+	else if (shndx == obj->efile.metadata_shndx)
+		return LIBBPF_MAP_METADATA;
  	else
  		return LIBBPF_MAP_UNSPEC;
  }

@@ -3655,6 +3682,60 @@ static int probe_kern_probe_read_kernel(void)
  	return probe_fd(bpf_load_program_xattr(&attr, NULL, 0));
  }
 
+static int probe_prog_bind_map(void)
+{
+	struct bpf_load_program_attr prog_attr;
+	struct bpf_create_map_attr map_attr;
+	char *cp, errmsg[STRERR_BUFSIZE];
+	struct bpf_insn insns[] = {
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	int ret = 0, prog, map;
+
+	if (!kernel_supports(FEAT_GLOBAL_DATA))
+		return 0;
+
+	memset(&map_attr, 0, sizeof(map_attr));
+	map_attr.map_type = BPF_MAP_TYPE_ARRAY;
+	map_attr.key_size = sizeof(int);
+	map_attr.value_size = 32;
+	map_attr.max_entries = 1;
+
+	map = bpf_create_map_xattr(&map_attr);
+	if (map < 0) {
+		ret = -errno;
+		cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
+		pr_warn("Error in %s():%s(%d). Couldn't create simple array map.\n",
+			__func__, cp, -ret);
+		return ret;
+	}
+
+	memset(&prog_attr, 0, sizeof(prog_attr));
+	prog_attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
+	prog_attr.insns = insns;
+	prog_attr.insns_cnt = ARRAY_SIZE(insns);
+	prog_attr.license = "GPL";
+

tools/lib/bpf/libbpf.c +12 -0

+
+	if (!bpf_prog_bind_map(prog, map, 0))
+		ret = 1;
+
+	close(map);
+	close(prog);
+	return ret;
+}
+
  enum kern_feature_result {
  	FEAT_UNKNOWN = 0,
  	FEAT_SUPPORTED = 1,

@@ -3695,6 +3776,9 @@ static struct kern_feature_desc {
  	},
  	[FEAT_PROBE_READ_KERN] = {
  		"bpf_probe_read_kernel() helper", probe_kern_probe_read_kernel,
+	},
+	[FEAT_PROG_BIND_MAP] = {
+		"bpf_prog_bind_map() helper", probe_prog_bind_map,
  	}
  };
 

tools/lib/bpf/libbpf.c +7 -0

+				cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+				pr_warn("add metadata map failed: %s\n", cp);
+				close(fd);
+				goto out;
+			}
+		}
+
  		*pfd = ret;
  		ret = 0;
  		goto out;

tools/lib/bpf/libbpf.map +0 -0

index e35bd6cdbdbf..4baf18a6df69 100644

tools/lib/bpf/libbpf.map +1 -0

+		bpf_prog_bind_map;
  		bpf_program__attach_xdp;
  		bpf_program__autoload;
  		bpf_program__is_sk_lookup;

tools/lib/bpf/libbpf.c load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt, · line 6047

Quoted 4 lines

tools/lib/bpf/libbpf.c +3 -0

+                             int fd = ret;
+
+                             ret = -errno;

libbpf_strerror_r understands positive and negative errno, so no need "ret = -errno".

Quoted 5 lines · Yonghong Song

Quoted 4 lines

tools/lib/bpf/libbpf.c +3 -0

+                             int fd = ret;
+
+                             ret = -errno;

Quoted 2 lines

libbpf_strerror_r understands positive and negative errno, so no need "ret = -errno".

I don't understand this one. The use of ret = -errno here is that when we goto out later we return a -errno. If this line is removed then fd is returned after fd is closed, in the case of a bind map failure, without writing to *pfd. Am I misunderstanding something?

YiFei Zhu

From: YiFei Zhu <[email protected]>

The patch adds a simple wrapper bpf_prog_bind_map around the syscall. And when using libbpf to load a program, it will probe the kernel for the support of this syscall, and scan for the .metadata ELF section and load it as an internal map like a .data section.

In the case that kernel supports the BPF_PROG_BIND_MAP syscall and a .metadata section exists, the map will be explicitly bound to the program via the syscall immediately after program is loaded. -EEXIST is ignored for this syscall.

Signed-off-by YiFei Zhu <[email protected]>

tools/lib/bpf/libbpf.c +1 -0

+
 	pr_debug("map %td is \"%s\"\n", map - obj->maps, map->name);
 	return 0;
 }

From: YiFei Zhu <[email protected]>

The patch adds a simple wrapper bpf_prog_bind_map around the syscall. And when using libbpf to load a program, it will probe the kernel for the support of this syscall, and scan for the .metadata ELF section and load it as an internal map like a .data section.

In the case that kernel supports the BPF_PROG_BIND_MAP syscall and a .metadata section exists, the map will be explicitly bound to the program via the syscall immediately after program is loaded. -EEXIST is ignored for this syscall.

Signed-off-by YiFei Zhu <[email protected]>

---
 tools/lib/bpf/bpf.c      |  11 +++++
 tools/lib/bpf/bpf.h      |   1 +
 tools/lib/bpf/libbpf.c   | 100 ++++++++++++++++++++++++++++++++++++++-
 tools/lib/bpf/libbpf.map |   1 +
 4 files changed, 112 insertions(+), 1 deletion(-)

tools/lib/bpf/bpf.c +0 -0

index 82b983ff6569..383b29ecb1fd 100644

tools/lib/bpf/bpf.c +7 -0

+
+       attr.prog_bind_map.prog_fd = prog_fd;
+       attr.prog_bind_map.map_fd = map_fd;
+       attr.prog_bind_map.flags = flags;
+
+       return sys_bpf(BPF_PROG_BIND_MAP, &attr, sizeof(attr));
+}

tools/lib/bpf/bpf.h +1 -0

index 015d13f25fcc..32994a4e0bf6 100644
@@ -243,6 +243,7 @@ LIBBPF_API int bpf_task_fd_query(int pid, int fd, __u32 flags, char *buf,
 enum bpf_stats_type; /* defined in up-to-date linux/bpf.h */
 LIBBPF_API int bpf_enable_stats(enum bpf_stats_type type);
 
+LIBBPF_API int bpf_prog_bind_map(int prog_fd, int map_fd, int flags);
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

tools/lib/bpf/libbpf.c +0 -0

index 77d420c02094..4725859099c5 100644

tools/lib/bpf/libbpf.c +9 -0

+       FEAT_PROG_BIND_MAP,
        __FEAT_CNT,
 };
 

@@ -283,6 +285,7 @@ struct bpf_struct_ops {
 #define KCONFIG_SEC ".kconfig"
 #define KSYMS_SEC ".ksyms"
 #define STRUCT_OPS_SEC ".struct_ops"
+#define METADATA_SEC ".metadata"
 
 enum libbpf_map_type {
        LIBBPF_MAP_UNSPEC,

@@ -290,6 +293,7 @@ enum libbpf_map_type {
        LIBBPF_MAP_BSS,
        LIBBPF_MAP_RODATA,
        LIBBPF_MAP_KCONFIG,
+       LIBBPF_MAP_METADATA,
 };
 
 static const char * const libbpf_type_to_btf_name[] = {

@@ -297,6 +301,7 @@ static const char * const libbpf_type_to_btf_name[] = {
        [LIBBPF_MAP_BSS]        = BSS_SEC,
        [LIBBPF_MAP_RODATA]     = RODATA_SEC,
        [LIBBPF_MAP_KCONFIG]    = KCONFIG_SEC,
+       [LIBBPF_MAP_METADATA]   = METADATA_SEC,
 };
 
 struct bpf_map {

@@ -375,6 +380,8 @@ struct bpf_object {
        size_t nr_maps;
        size_t maps_cap;
 
+       struct bpf_map *metadata_map;
+
        char *kconfig;
        struct extern_desc *externs;
        int nr_extern;

@@ -398,6 +405,7 @@ struct bpf_object {
                Elf_Data *rodata;
                Elf_Data *bss;
                Elf_Data *st_ops_data;
+               Elf_Data *metadata;
                size_t strtabidx;
                struct {
                        GElf_Shdr shdr;

@@ -413,6 +421,7 @@ struct bpf_object {
                int rodata_shndx;
                int bss_shndx;
                int st_ops_shndx;
+               int metadata_shndx;
        } efile;
        /*
         * All loaded bpf_object is linked in a list, which is

@@ -1022,6 +1031,7 @@ static struct bpf_object *bpf_object__new(const char *path,
        obj->efile.obj_buf_sz = obj_buf_sz;
        obj->efile.maps_shndx = -1;
        obj->efile.btf_maps_shndx = -1;
+       obj->efile.metadata_shndx = -1;
        obj->efile.data_shndx = -1;
        obj->efile.rodata_shndx = -1;
        obj->efile.bss_shndx = -1;

tools/lib/bpf/libbpf.c +32 -1

+
        pr_debug("map %td is \"%s\"\n", map - obj->maps, map->name);
        return 0;
 }

@@ -1422,6 +1435,14 @@ static int bpf_object__init_global_data_maps(struct bpf_object *obj)
                if (err)
                        return err;
        }
+       if (obj->efile.metadata_shndx >= 0) {
+               err = bpf_object__init_internal_map(obj, LIBBPF_MAP_METADATA,
+                                                   obj->efile.metadata_shndx,
+                                                   obj->efile.metadata->d_buf,
+                                                   obj->efile.metadata->d_size);
+               if (err)
+                       return err;
+       }
        return 0;
 }
 

@@ -2698,6 +2719,9 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
                        } else if (strcmp(name, STRUCT_OPS_SEC) == 0) {
                                obj->efile.st_ops_data = data;
                                obj->efile.st_ops_shndx = idx;
+                       } else if (strcmp(name, METADATA_SEC) == 0) {
+                               obj->efile.metadata = data;
+                               obj->efile.metadata_shndx = idx;
                        } else {
                                pr_debug("skip section(%d) %s\n", idx, name);
                        }

@@ -3111,7 +3135,8 @@ static bool bpf_object__shndx_is_data(const struct bpf_object *obj,
 {
        return shndx == obj->efile.data_shndx ||
               shndx == obj->efile.bss_shndx ||
-              shndx == obj->efile.rodata_shndx;
+              shndx == obj->efile.rodata_shndx ||
+              shndx == obj->efile.metadata_shndx;
 }
 
 static bool bpf_object__shndx_is_maps(const struct bpf_object *obj,

@@ -3132,6 +3157,8 @@ bpf_object__section_to_libbpf_map_type(const struct bpf_object *obj, int shndx)
                return LIBBPF_MAP_RODATA;
        else if (shndx == obj->efile.symbols_shndx)
                return LIBBPF_MAP_KCONFIG;
+       else if (shndx == obj->efile.metadata_shndx)
+               return LIBBPF_MAP_METADATA;
        else
                return LIBBPF_MAP_UNSPEC;
 }

@@ -3655,6 +3682,60 @@ static int probe_kern_probe_read_kernel(void)
        return probe_fd(bpf_load_program_xattr(&attr, NULL, 0));
 }
 
+static int probe_prog_bind_map(void)
+{
+       struct bpf_load_program_attr prog_attr;
+       struct bpf_create_map_attr map_attr;
+       char *cp, errmsg[STRERR_BUFSIZE];
+       struct bpf_insn insns[] = {
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       };
+       int ret = 0, prog, map;
+
+       if (!kernel_supports(FEAT_GLOBAL_DATA))
+               return 0;
+
+       memset(&map_attr, 0, sizeof(map_attr));
+       map_attr.map_type = BPF_MAP_TYPE_ARRAY;

tools/lib/bpf/libbpf.c +6 -0

+               return ret;
+       }
+
+       memset(&prog_attr, 0, sizeof(prog_attr));
+       prog_attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
+       prog_attr.insns = insns;

tools/lib/bpf/libbpf.c +12 -0

+               close(map);
+               return ret;
+       }
+
+       if (!bpf_prog_bind_map(prog, map, 0))
+               ret = 1;
+
+       close(map);
+       close(prog);
+       return ret;
+}
+
 enum kern_feature_result {
        FEAT_UNKNOWN = 0,
        FEAT_SUPPORTED = 1,

tools/lib/bpf/libbpf.c +0 -0

        }
 };
 

@@ -5954,6 +6038,20 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
        if (ret >= 0) {
                if (log_buf && load_attr.log_level)

tools/lib/bpf/libbpf.c +4 -0

+                               goto out;
+                       }
+               }
+
                *pfd = ret;
                ret = 0;
                goto out;

tools/lib/bpf/libbpf.map +1 -1

index e35bd6cdbdbf..4baf18a6df69 100644
@@ -288,6 +288,7 @@ LIBBPF_0.1.0 {
                bpf_map__set_value_size;
                bpf_map__type;
                bpf_map__value_size;
+               bpf_prog_bind_map;
                bpf_program__attach_xdp;
                bpf_program__autoload;
                bpf_program__is_sk_lookup;
--

2.28.0

From: YiFei Zhu <[email protected]>

Added a flag "--metadata" to bpftool prog list to dump the metadata contents. For some formatting some BTF code is put directly in the metadata dumping. Sanity checks on the map and the kind of the btf_type to make sure we are actually dumping what we are expecting.

A helper jsonw_reset is added to json writer so we can reuse the same json writer without having extraneous commas.

Sample output:

   $ bpftool prog --metadata
   6: cgroup_skb  name prog  tag bcf7977d3b93787c  gpl
   [...]
   	btf_id 4
   	metadata:
   		metadata_a = "foo"
   		metadata_b = 1

   $ bpftool prog --metadata --json --pretty
   [{
           "id": 6,
   [...]
           "btf_id": 4,
           "metadata": {
               "metadata_a": "foo",
               "metadata_b": 1
           }
       }
   ]

Signed-off-by YiFei Zhu <[email protected]>

---
  tools/bpf/bpftool/json_writer.c |   6 ++
  tools/bpf/bpftool/json_writer.h |   3 +
  tools/bpf/bpftool/main.c        |  10 +++
  tools/bpf/bpftool/main.h        |   1 +
  tools/bpf/bpftool/prog.c        | 135 ++++++++++++++++++++++++++++++++
  5 files changed, 155 insertions(+)

tools/bpf/bpftool/json_writer.c +6 -0

index 86501cd3c763..7fea83bedf48 100644
@@ -119,6 +119,12 @@ void jsonw_pretty(json_writer_t *self, bool on)
  	self->pretty = on;
  }
 
+void jsonw_reset(json_writer_t *self)
+{
+	assert(self->depth == 0);
+	self->sep = '\0';
+}
+
  /* Basic blocks */
  static void jsonw_begin(json_writer_t *self, int c)
  {

tools/bpf/bpftool/json_writer.h +3 -0

index 35cf1f00f96c..8ace65cdb92f 100644
@@ -27,6 +27,9 @@ void jsonw_destroy(json_writer_t **self_p);
  /* Cause output to have pretty whitespace */
  void jsonw_pretty(json_writer_t *self, bool on);
 
+/* Reset separator to create new JSON */
+void jsonw_reset(json_writer_t *self);
+
  /* Add property name */
  void jsonw_name(json_writer_t *self, const char *name);
 

tools/bpf/bpftool/main.c +10 -0

index 4a191fcbeb82..a681d568cfa7 100644
@@ -28,6 +28,7 @@ bool show_pinned;
  bool block_mount;
  bool verifier_logs;
  bool relaxed_maps;
+bool dump_metadata;
  struct pinned_obj_table prog_table;
  struct pinned_obj_table map_table;
  struct pinned_obj_table link_table;

@@ -351,6 +352,10 @@ static int do_batch(int argc, char **argv)
  	return err;
  }
 
+enum bpftool_longonly_opts {
+	OPT_METADATA = 256,
+};
+
  int main(int argc, char **argv)
  {
  	static const struct option options[] = {

@@ -362,6 +367,7 @@ int main(int argc, char **argv)
  		{ "mapcompat",	no_argument,	NULL,	'm' },
  		{ "nomount",	no_argument,	NULL,	'n' },
  		{ "debug",	no_argument,	NULL,	'd' },
+		{ "metadata",	no_argument,	NULL,	OPT_METADATA },
  		{ 0 }
  	};
  	int opt, ret;

@@ -371,6 +377,7 @@ int main(int argc, char **argv)
  	json_output = false;
  	show_pinned = false;
  	block_mount = false;
+	dump_metadata = false;
  	bin_name = argv[0];
 
  	hash_init(prog_table.table);

@@ -412,6 +419,9 @@ int main(int argc, char **argv)
  			libbpf_set_print(print_all_levels);
  			verifier_logs = true;
  			break;
+		case OPT_METADATA:
+			dump_metadata = true;
+			break;
  		default:
  			p_err("unrecognized option '%s'", argv[optind - 1]);
  			if (json_output)

tools/bpf/bpftool/main.h +1 -0

index c46e52137b87..8750758e9150 100644
@@ -90,6 +90,7 @@ extern bool show_pids;
  extern bool block_mount;
  extern bool verifier_logs;
  extern bool relaxed_maps;
+extern bool dump_metadata;
  extern struct pinned_obj_table prog_table;
  extern struct pinned_obj_table map_table;
  extern struct pinned_obj_table link_table;

tools/bpf/bpftool/prog.c +5 -0

index d393eb8263a6..ee767b8d90fb 100644
@@ -151,6 +151,135 @@ static void show_prog_maps(int fd, __u32 num_maps)
  	}
  }
 
+static void show_prog_metadata(int fd, __u32 num_maps)
+{
+	struct bpf_prog_info prog_info = {};
+	struct bpf_map_info map_info = {};
+	__u32 prog_info_len = sizeof(prog_info);

tools/bpf/bpftool/prog.c +26 -0

+
+	for (i = 0; i < prog_info.nr_map_ids; i++) {
+		map_fd = bpf_map_get_fd_by_id(map_ids[i]);
+		if (map_fd < 0)
+			return;
+
+		err = bpf_obj_get_info_by_fd(map_fd, &map_info, &map_info_len);
+		if (err)
+			goto out_close;
+
+		if (map_info.type != BPF_MAP_TYPE_ARRAY)
+			goto next_map;
+		if (map_info.key_size != sizeof(int))
+			goto next_map;
+		if (map_info.max_entries != 1)
+			goto next_map;
+		if (!map_info.btf_value_type_id)
+			goto next_map;
+		if (!strstr(map_info.name, ".metadata"))
+			goto next_map;
+
+		goto found;
+
+next_map:
+		close(map_fd);
+	}

tools/bpf/bpftool/prog.c +17 -0

+
+	err = btf__get_from_id(map_info.btf_id, &btf);
+	if (err || !btf)
+		goto out_free;
+
+	t_datasec = btf__type_by_id(btf, map_info.btf_value_type_id);
+	if (BTF_INFO_KIND(t_datasec->info) != BTF_KIND_DATASEC)
+		goto out_free;
+
+	vlen = BTF_INFO_VLEN(t_datasec->info);
+	vsi = (struct btf_var_secinfo *)(t_datasec + 1);
+
+	if (json_output) {
+		struct btf_dumper d = {
+			.btf = btf,
+			.jw = json_wtr,
+			.is_plain_text = false,

tools/bpf/bpftool/prog.c +14 -0

+
+			jsonw_name(json_wtr, btf__name_by_offset(btf, t_var->name_off));
+			err = btf_dumper_type(&d, t_var->type, value + vsi[i].offset);
+			if (err)
+				break;
+		}
+		jsonw_end_object(json_wtr);
+	} else {
+		json_writer_t *btf_wtr = jsonw_new(stdout);
+		struct btf_dumper d = {
+			.btf = btf,
+			.jw = btf_wtr,
+			.is_plain_text = true,
+		};

tools/bpf/bpftool/prog.c +26 -0

+
+			printf("\n\t\t%s = ", btf__name_by_offset(btf, t_var->name_off));
+
+			jsonw_reset(btf_wtr);
+			err = btf_dumper_type(&d, t_var->type, value + vsi[i].offset);
+			if (err)
+				break;
+		}
+
+		jsonw_destroy(&btf_wtr);
+	}
+
+out_free:
+	btf__free(btf);
+	free(value);
+
+out_close:
+	close(map_fd);
+}
+
  static void print_prog_header_json(struct bpf_prog_info *info)
  {
  	jsonw_uint_field(json_wtr, "id", info->id);

@@ -228,6 +357,9 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
 
  	emit_obj_refs_json(&refs_table, info->id, json_wtr);
 
+	if (dump_metadata)
+		show_prog_metadata(fd, info->nr_map_ids);
+
  	jsonw_end_object(json_wtr);
  }
 

@@ -297,6 +429,9 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
  	emit_obj_refs_plain(&refs_table, info->id, "\n\tpids ");
 
  	printf("\n");
+
+	if (dump_metadata)
+		show_prog_metadata(fd, info->nr_map_ids);
  }
 
  static int show_prog(int fd)

Quoted 146 lines · YiFei Zhu

From: YiFei Zhu <[email protected]>

Added a flag "--metadata" to bpftool prog list to dump the metadata contents. For some formatting some BTF code is put directly in the metadata dumping. Sanity checks on the map and the kind of the btf_type to make sure we are actually dumping what we are expecting.

A helper jsonw_reset is added to json writer so we can reuse the same json writer without having extraneous commas.

Sample output:

   $ bpftool prog --metadata
   6: cgroup_skb  name prog  tag bcf7977d3b93787c  gpl
   [...]
   	btf_id 4
   	metadata:
   		metadata_a = "foo"
   		metadata_b = 1

   $ bpftool prog --metadata --json --pretty
   [{
           "id": 6,
   [...]
           "btf_id": 4,
           "metadata": {
               "metadata_a": "foo",
               "metadata_b": 1
           }
       }
   ]

Signed-off-by YiFei Zhu <[email protected]>

---
  tools/bpf/bpftool/json_writer.c |   6 ++
  tools/bpf/bpftool/json_writer.h |   3 +
  tools/bpf/bpftool/main.c        |  10 +++
  tools/bpf/bpftool/main.h        |   1 +
  tools/bpf/bpftool/prog.c        | 135 ++++++++++++++++++++++++++++++++
  5 files changed, 155 insertions(+)

tools/bpf/bpftool/json_writer.c +6 -0

index 86501cd3c763..7fea83bedf48 100644
@@ -119,6 +119,12 @@ void jsonw_pretty(json_writer_t *self, bool on)
  	self->pretty = on;
  }
 
+void jsonw_reset(json_writer_t *self)
+{
+	assert(self->depth == 0);
+	self->sep = '\0';
+}
+
  /* Basic blocks */
  static void jsonw_begin(json_writer_t *self, int c)
  {

tools/bpf/bpftool/json_writer.h +3 -0

index 35cf1f00f96c..8ace65cdb92f 100644
@@ -27,6 +27,9 @@ void jsonw_destroy(json_writer_t **self_p);
  /* Cause output to have pretty whitespace */
  void jsonw_pretty(json_writer_t *self, bool on);
 
+/* Reset separator to create new JSON */
+void jsonw_reset(json_writer_t *self);
+
  /* Add property name */
  void jsonw_name(json_writer_t *self, const char *name);
 

tools/bpf/bpftool/main.c +10 -0

index 4a191fcbeb82..a681d568cfa7 100644
@@ -28,6 +28,7 @@ bool show_pinned;
  bool block_mount;
  bool verifier_logs;
  bool relaxed_maps;
+bool dump_metadata;
  struct pinned_obj_table prog_table;
  struct pinned_obj_table map_table;
  struct pinned_obj_table link_table;

@@ -351,6 +352,10 @@ static int do_batch(int argc, char **argv)
  	return err;
  }
 
+enum bpftool_longonly_opts {
+	OPT_METADATA = 256,
+};
+
  int main(int argc, char **argv)
  {
  	static const struct option options[] = {

@@ -362,6 +367,7 @@ int main(int argc, char **argv)
  		{ "mapcompat",	no_argument,	NULL,	'm' },
  		{ "nomount",	no_argument,	NULL,	'n' },
  		{ "debug",	no_argument,	NULL,	'd' },
+		{ "metadata",	no_argument,	NULL,	OPT_METADATA },
  		{ 0 }
  	};
  	int opt, ret;

@@ -371,6 +377,7 @@ int main(int argc, char **argv)
  	json_output = false;
  	show_pinned = false;
  	block_mount = false;
+	dump_metadata = false;
  	bin_name = argv[0];
 
  	hash_init(prog_table.table);

@@ -412,6 +419,9 @@ int main(int argc, char **argv)
  			libbpf_set_print(print_all_levels);
  			verifier_logs = true;
  			break;
+		case OPT_METADATA:
+			dump_metadata = true;
+			break;
  		default:
  			p_err("unrecognized option '%s'", argv[optind - 1]);
  			if (json_output)

tools/bpf/bpftool/main.h +1 -0

index c46e52137b87..8750758e9150 100644
@@ -90,6 +90,7 @@ extern bool show_pids;
  extern bool block_mount;
  extern bool verifier_logs;
  extern bool relaxed_maps;
+extern bool dump_metadata;
  extern struct pinned_obj_table prog_table;
  extern struct pinned_obj_table map_table;
  extern struct pinned_obj_table link_table;

tools/bpf/bpftool/prog.c +5 -0

index d393eb8263a6..ee767b8d90fb 100644
@@ -151,6 +151,135 @@ static void show_prog_maps(int fd, __u32 num_maps)
  	}
  }
 
+static void show_prog_metadata(int fd, __u32 num_maps)
+{
+	struct bpf_prog_info prog_info = {};
+	struct bpf_map_info map_info = {};
+	__u32 prog_info_len = sizeof(prog_info);

tools/bpf/bpftool/prog.c static void show_prog_maps(int fd, __u32 num_maps) · line 168

Quoted 10 lines

tools/bpf/bpftool/prog.c +10 -0

+	__u32 map_info_len = sizeof(map_info);
+	__u32 map_ids[num_maps];
+	void *value = NULL;
+	struct btf *btf = NULL;
+	const struct btf_type *t_datasec, *t_var;
+	struct btf_var_secinfo *vsi;
+	int key = 0;
+	unsigned int i, vlen;
+	int map_fd;
+	int err;

try to follow reverse christmas tree coding styple?

tools/bpf/bpftool/prog.c static void show_prog_maps(int fd, __u32 num_maps) · line 175

Quoted 7 lines

tools/bpf/bpftool/prog.c +7 -0

+
+	prog_info.nr_map_ids = num_maps;
+	prog_info.map_ids = ptr_to_u64(map_ids);
+
+	err = bpf_obj_get_info_by_fd(fd, &prog_info, &prog_info_len);
+	if (err || !prog_info.nr_map_ids)
+		return;

print out something for "err" case and "!prog_info.nr_map_ids" case? The same for some other below returns.

Quoted 32 lines

tools/bpf/bpftool/prog.c +32 -0

+
+	for (i = 0; i < prog_info.nr_map_ids; i++) {
+		map_fd = bpf_map_get_fd_by_id(map_ids[i]);
+		if (map_fd < 0)
+			return;
+
+		err = bpf_obj_get_info_by_fd(map_fd, &map_info, &map_info_len);
+		if (err)
+			goto out_close;
+
+		if (map_info.type != BPF_MAP_TYPE_ARRAY)
+			goto next_map;
+		if (map_info.key_size != sizeof(int))
+			goto next_map;
+		if (map_info.max_entries != 1)
+			goto next_map;
+		if (!map_info.btf_value_type_id)
+			goto next_map;
+		if (!strstr(map_info.name, ".metadata"))
+			goto next_map;
+
+		goto found;
+
+next_map:
+		close(map_fd);
+	}
+
+	return;
+
+found:
+	value = malloc(map_info.value_size);
+	if (!value)

tools/bpf/bpftool/prog.c static void show_prog_maps(int fd, __u32 num_maps) · line 211

Quoted 4 lines

tools/bpf/bpftool/prog.c +4 -0

+		goto out_close;
+
+	if (bpf_map_lookup_elem(map_fd, &key, value))
+		goto out_free;

Not sure whether we need formal libbpf API to access metadata or not. This may help other applications too. But we can delay until it is necessary.

Quoted 3 lines · Yonghong Song

Not sure whether we need formal libbpf API to access metadata or not. This may help other applications too. But we can delay until it is necessary.

Yeah, please put in a libbpf accessor as well; I would like to use this from libxdp - without a skeleton :)

-Toke

Quoted 8 lines · Toke Høiland-Jørgensen

Quoted 3 lines · Yonghong Song

Not sure whether we need formal libbpf API to access metadata or not. This may help other applications too. But we can delay until it is necessary.

Yeah, please put in a libbpf accessor as well; I would like to use this from libxdp - without a skeleton :)

Quoted 1 line

-Toke

I don't think I have an idea on a good API in libbpf that could be used to get the metadata of an existing program in kernel, that could be reused by bpftool without duplicating all the code. Maybe we can discuss this in a follow up series?

Quoted 1 line · YiFei Zhu

On Fri, Aug 21, 2020 at 3:58 AM Toke H�iland-J�rgensen

Quoted 8 lines · <[email protected]> wrote:

Quoted 8 lines

Quoted 3 lines · Yonghong Song

Not sure whether we need formal libbpf API to access metadata or not. This may help other applications too. But we can delay until it is necessary.

Yeah, please put in a libbpf accessor as well; I would like to use this from libxdp - without a skeleton :)

Quoted 6 lines

Quoted 1 line

-Toke

I don't think I have an idea on a good API in libbpf that could be used to get the metadata of an existing program in kernel, that could be reused by bpftool without duplicating all the code. Maybe we can discuss this in a follow up series?

I think the most important part is getting a reference to the metadata map. So a function that basically does what the top half of what your show_prog_metadata() function does: given a prog fd, walk the map ids, check if any of them looks like a metadata map, and if so return the map fd.

Should be pretty straight-forward to reuse between bpftool/libbpf, no?

Quoted 24 lines · Toke H�iland-J�rgensen

Quoted 1 line · YiFei Zhu

On Fri, Aug 21, 2020 at 3:58 AM Toke H�iland-J�rgensen

Quoted 8 lines · <[email protected]> wrote:

Quoted 8 lines

Quoted 3 lines · Yonghong Song

Not sure whether we need formal libbpf API to access metadata or not. This may help other applications too. But we can delay until it is necessary.

Yeah, please put in a libbpf accessor as well; I would like to use this from libxdp - without a skeleton :)

Quoted 6 lines

Quoted 1 line

-Toke

I don't think I have an idea on a good API in libbpf that could be used to get the metadata of an existing program in kernel, that could be reused by bpftool without duplicating all the code. Maybe we can discuss this in a follow up series?

I think the most important part is getting a reference to the metadata map. So a function that basically does what the top half of what your show_prog_metadata() function does: given a prog fd, walk the map ids, check if any of them looks like a metadata map, and if so return the map fd.

Quoted 1 line

Should be pretty straight-forward to reuse between bpftool/libbpf, no?

Sounds good, I'll be taking over this patch series as YiFei's internship has ended. I'll try to address that.

From: YiFei Zhu <[email protected]>

Added a flag "--metadata" to bpftool prog list to dump the metadata contents. For some formatting some BTF code is put directly in the metadata dumping. Sanity checks on the map and the kind of the btf_type to make sure we are actually dumping what we are expecting.

A helper jsonw_reset is added to json writer so we can reuse the same json writer without having extraneous commas.

Sample output:

  $ bpftool prog --metadata
  6: cgroup_skb  name prog  tag bcf7977d3b93787c  gpl
  [...]
        btf_id 4
        metadata:
                metadata_a = "foo"
                metadata_b = 1

  $ bpftool prog --metadata --json --pretty
  [{
          "id": 6,
  [...]
          "btf_id": 4,
          "metadata": {
              "metadata_a": "foo",
              "metadata_b": 1
          }
      }
  ]

Signed-off-by YiFei Zhu <[email protected]>

---
 tools/bpf/bpftool/json_writer.c |   6 ++
 tools/bpf/bpftool/json_writer.h |   3 +
 tools/bpf/bpftool/main.c        |  10 +++
 tools/bpf/bpftool/main.h        |   1 +
 tools/bpf/bpftool/prog.c        | 135 ++++++++++++++++++++++++++++++++
 5 files changed, 155 insertions(+)

tools/bpf/bpftool/prog.c +8 -0

+       for (i = 0; i < prog_info.nr_map_ids; i++) {
+               map_fd = bpf_map_get_fd_by_id(map_ids[i]);
+               if (map_fd < 0)
+                       return;
+
+               err = bpf_obj_get_info_by_fd(map_fd, &map_info, &map_info_len);
+               if (err)
+                       goto out_close;

tools/bpf/bpftool/prog.c +11 -0

+                       goto next_map;
+
+               goto found;
+
+next_map:
+               close(map_fd);
+       }
+
+       return;
+
+found:

Quoted 42 lines · YiFei Zhu

From: YiFei Zhu <[email protected]>

Added a flag "--metadata" to bpftool prog list to dump the metadata contents. For some formatting some BTF code is put directly in the metadata dumping. Sanity checks on the map and the kind of the btf_type to make sure we are actually dumping what we are expecting.

A helper jsonw_reset is added to json writer so we can reuse the same json writer without having extraneous commas.

Sample output:

  $ bpftool prog --metadata
  6: cgroup_skb  name prog  tag bcf7977d3b93787c  gpl
  [...]
        btf_id 4
        metadata:
                metadata_a = "foo"
                metadata_b = 1

  $ bpftool prog --metadata --json --pretty
  [{
          "id": 6,
  [...]
          "btf_id": 4,
          "metadata": {
              "metadata_a": "foo",
              "metadata_b": 1
          }
      }
  ]

Signed-off-by YiFei Zhu <[email protected]>

---
 tools/bpf/bpftool/json_writer.c |   6 ++
 tools/bpf/bpftool/json_writer.h |   3 +
 tools/bpf/bpftool/main.c        |  10 +++
 tools/bpf/bpftool/main.h        |   1 +
 tools/bpf/bpftool/prog.c        | 135 ++++++++++++++++++++++++++++++++
 5 files changed, 155 insertions(+)

[...]

tools/bpf/bpftool/prog.c static void show_prog_maps(int fd, __u32 num_maps)

Quoted 6 lines

tools/bpf/bpftool/prog.c +6 -0

+       for (i = 0; i < prog_info.nr_map_ids; i++) {
+               map_fd = bpf_map_get_fd_by_id(map_ids[i]);
+               if (map_fd < 0)
+                       return;
+
+               err = bpf_obj_get_info_by_fd(map_fd, &map_info,  

&map_info_len);

tools/bpf/bpftool/prog.c static void show_prog_maps(int fd, __u32 num_maps) · line 194

Quoted 12 lines

tools/bpf/bpftool/prog.c +12 -0

+               if (err)
+                       goto out_close;
+
+               if (map_info.type != BPF_MAP_TYPE_ARRAY)
+                       goto next_map;
+               if (map_info.key_size != sizeof(int))
+                       goto next_map;
+               if (map_info.max_entries != 1)
+                       goto next_map;
+               if (!map_info.btf_value_type_id)
+                       goto next_map;
+               if (!strstr(map_info.name, ".metadata"))

This substring check sucks. Let's make libbpf call this map strictly ".metadata". Current convention of "some part of object name" + "." + {rodata,data,bss} is extremely confusing. In practice it's something incomprehensible and "unguessable" like "test_pr.rodata". I think it makes sense to call them just ".data", ".rodata", ".bss", and ".metadata". But that might break existing apps that do lookups based on map name (and might break skeleton as it is today, not sure). But let's at least start with ".metadata", as it's a new map and we can get it right from the start.

Quoted 66 lines · Andrii Nakryiko

Quoted 42 lines · YiFei Zhu

From: YiFei Zhu <[email protected]>

Added a flag "--metadata" to bpftool prog list to dump the metadata contents. For some formatting some BTF code is put directly in the metadata dumping. Sanity checks on the map and the kind of the btf_type to make sure we are actually dumping what we are expecting.

A helper jsonw_reset is added to json writer so we can reuse the same json writer without having extraneous commas.

Sample output:

  $ bpftool prog --metadata
  6: cgroup_skb  name prog  tag bcf7977d3b93787c  gpl
  [...]
        btf_id 4
        metadata:
                metadata_a = "foo"
                metadata_b = 1

  $ bpftool prog --metadata --json --pretty
  [{
          "id": 6,
  [...]
          "btf_id": 4,
          "metadata": {
              "metadata_a": "foo",
              "metadata_b": 1
          }
      }
  ]

Signed-off-by YiFei Zhu <[email protected]>

---
 tools/bpf/bpftool/json_writer.c |   6 ++
 tools/bpf/bpftool/json_writer.h |   3 +
 tools/bpf/bpftool/main.c        |  10 +++
 tools/bpf/bpftool/main.h        |   1 +
 tools/bpf/bpftool/prog.c        | 135 ++++++++++++++++++++++++++++++++
 5 files changed, 155 insertions(+)

[...]

tools/bpf/bpftool/prog.c static void show_prog_maps(int fd, __u32 num_maps)

Quoted 6 lines

tools/bpf/bpftool/prog.c +6 -0

+       for (i = 0; i < prog_info.nr_map_ids; i++) {
+               map_fd = bpf_map_get_fd_by_id(map_ids[i]);
+               if (map_fd < 0)
+                       return;
+
+               err = bpf_obj_get_info_by_fd(map_fd, &map_info,

&map_info_len);

tools/bpf/bpftool/prog.c static void show_prog_maps(int fd, __u32 num_maps) · line 194

Quoted 12 lines

tools/bpf/bpftool/prog.c +12 -0

+               if (err)
+                       goto out_close;
+
+               if (map_info.type != BPF_MAP_TYPE_ARRAY)
+                       goto next_map;
+               if (map_info.key_size != sizeof(int))
+                       goto next_map;
+               if (map_info.max_entries != 1)
+                       goto next_map;
+               if (!map_info.btf_value_type_id)
+                       goto next_map;
+               if (!strstr(map_info.name, ".metadata"))

Quoted 9 lines

This substring check sucks. Let's make libbpf call this map strictly ".metadata". Current convention of "some part of object name" + "." + {rodata,data,bss} is extremely confusing. In practice it's something incomprehensible and "unguessable" like "test_pr.rodata". I think it makes sense to call them just ".data", ".rodata", ".bss", and ".metadata". But that might break existing apps that do lookups based on map name (and might break skeleton as it is today, not sure). But let's at least start with ".metadata", as it's a new map and we can get it right from the start.

Isn't it bad from the consistency point of view? Even if it's bad, at least it's consistent :-/

From: YiFei Zhu <[email protected]>

This is a simple test to check that loading and dumping metadata works, whether or not metadata contents are used by the program.

Signed-off-by YiFei Zhu <[email protected]>

---
  tools/testing/selftests/bpf/Makefile          |  3 +-
  .../selftests/bpf/progs/metadata_unused.c     | 15 ++++
  .../selftests/bpf/progs/metadata_used.c       | 15 ++++
  .../selftests/bpf/test_bpftool_metadata.sh    | 82 +++++++++++++++++++
  4 files changed, 114 insertions(+), 1 deletion(-)

create mode 100644 tools/testing/selftests/bpf/progs/metadata_unused.c create mode 100644 tools/testing/selftests/bpf/progs/metadata_used.c create mode 100755 tools/testing/selftests/bpf/test_bpftool_metadata.sh

tools/testing/selftests/bpf/Makefile +0 -0

index a83b5827532f..04e56c6843c6 100644

tools/testing/selftests/bpf/Makefile +0 -0

  TEST_PROGS_EXTENDED := with_addr.sh \
  	with_tunnels.sh \

tools/testing/selftests/bpf/progs/metadata_unused.c new file +15 -0

new file mode 100644
index 000000000000..523b3c332426
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+char metadata_a[] SEC(".metadata") = "foo";
+int metadata_b SEC(".metadata") = 1;
+
+SEC("cgroup_skb/egress")
+int prog(struct xdp_md *ctx)
+{
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";

Quoted 16 lines · YiFei Zhu

From: YiFei Zhu <[email protected]>

This is a simple test to check that loading and dumping metadata works, whether or not metadata contents are used by the program.

Signed-off-by YiFei Zhu <[email protected]>

---
  tools/testing/selftests/bpf/Makefile          |  3 +-
  .../selftests/bpf/progs/metadata_unused.c     | 15 ++++
  .../selftests/bpf/progs/metadata_used.c       | 15 ++++
  .../selftests/bpf/test_bpftool_metadata.sh    | 82 +++++++++++++++++++
  4 files changed, 114 insertions(+), 1 deletion(-)

create mode 100644 tools/testing/selftests/bpf/progs/metadata_unused.c create mode 100644 tools/testing/selftests/bpf/progs/metadata_used.c create mode 100755 tools/testing/selftests/bpf/test_bpftool_metadata.sh

tools/testing/selftests/bpf/Makefile TEST_PROGS := test_kmod.sh \ · line 72

Quoted 11 lines

tools/testing/selftests/bpf/Makefile +2 -1

index a83b5827532f..04e56c6843c6 100644
@@ -68,7 +68,8 @@ TEST_PROGS := test_kmod.sh \
      test_tc_edt.sh \
      test_xdping.sh \
      test_bpftool_build.sh \
-     test_bpftool.sh
+     test_bpftool.sh \
+     test_bpftool_metadata.sh \

This is mostly testing bpftool side.
We should add testing to test_progs too as it is what most developer runs. If you add skeleton support for metadata, similar to bss, it will both make user interface easy and make testing easy.

Quoted 24 lines

tools/testing/selftests/bpf/Makefile +0 -0

  TEST_PROGS_EXTENDED := with_addr.sh \
      with_tunnels.sh \

tools/testing/selftests/bpf/progs/metadata_unused.c new file +15 -0

new file mode 100644
index 000000000000..523b3c332426
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+char metadata_a[] SEC(".metadata") = "foo";
+int metadata_b SEC(".metadata") = 1;
+
+SEC("cgroup_skb/egress")
+int prog(struct xdp_md *ctx)
+{
+     return 0;
+}
+
+char _license[] SEC("license") = "GPL";

[...]