Skip to contentSkip to conversation

Allow storage of flexible metadata information for eBPF programs

RFC PATCH In review YiFei Zhu 2 participants 7 messages Aug 14, 2020 – Aug 19, 2020

Subsystems (10)
lore
Share & tools
lore

YiFei Zhu

author
#0 lore raw

cover Allow storage of flexible metadata information for eBPF programs

From: YiFei Zhu <[email protected]>

Currently, if a user wants to store arbitrary metadata for an eBPF program, for example, the program build commit hash or version, they could store it in a map, and conveniently libbpf uses .data section to populate an internal map. However, if the program does not actually reference the map, then the map would be de-refcounted and freed.

This patch set introduces a new syscall BPF_PROG_ADD_MAP to attach a map to a program's used_maps, even if the program instructions does not reference the map. libbpf is extended to recognize the .metadata section and load it as an internal map, and use the new syscall to ensure the map is attached. bpftool is also extended to have a new flag to prog subcommand, "--metadata" to dump the contents of the metadata section without a separate map dump call.

An example use of this would be BPF C file declaring:

char commit_hash[] SEC(".metadata") = "abcdef123456";

and bpftool would emit:

  $ bpftool prog --metadata
  [...]
        metadata:
                commit_hash = "abcdef123456"

Patch 1 moves the used_maps array and count into an RCU-protected struct.

Patch 2 implements the new syscall.

Patch 3 extends libbpf to have a wrapper around the syscall, probe the kernel for support of this new syscall, and use it on .metadata section if supported and the section exists.

Patch 4 extends bpftool so that it is able to dump metadata from prog show.

Patch 5 adds a test to check the metadata loading and dumping.

YiFei Zhu (5):
bpf: RCU protect used_maps array and count
bpf: Add BPF_PROG_ADD_MAP syscall
libbpf: Add BPF_PROG_ADD_MAP syscall and use it on .metadata section
bpftool: support dumping metadata
selftests/bpf: Test bpftool loading and dumping metadata

 .../net/ethernet/netronome/nfp/bpf/offload.c  |  33 +++--
 include/linux/bpf.h                           |  11 +-
 include/uapi/linux/bpf.h                      |   7 +
 kernel/bpf/core.c                             |  25 +++-
 kernel/bpf/syscall.c                          | 102 +++++++++++++-
 kernel/bpf/verifier.c                         |  37 +++--
 net/core/dev.c                                |  12 +-
 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                      | 132 ++++++++++++++++++
 tools/include/uapi/linux/bpf.h                |   7 +
 tools/lib/bpf/bpf.c                           |  11 ++
 tools/lib/bpf/bpf.h                           |   1 +
 tools/lib/bpf/libbpf.c                        |  97 ++++++++++++-
 tools/lib/bpf/libbpf.map                      |   1 +
 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 +++++++++++
 21 files changed, 572 insertions(+), 39 deletions(-)
 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
Signature · 1 line
2.28.0
author
#1 lore raw

RFC PATCH 1/5 bpf: RCU protect used_maps array and count

From: YiFei Zhu <[email protected]>

To support modifying the used_maps array, we use RCU to protect the use of the counter and the array. A mutex is used as the write-side lock, and it is initialized in the verifier where the rcu struct is allocated.

Most uses are non-sleeping and very straight forward, just holding rcu_read_lock. bpf_check_tail_call can be called for a cBPF map (by bpf_prog_select_runtime, bpf_migrate_filter) so an extra check is added for the case when the program did not pass through the eBPF verifier and the used_maps pointer is NULL.

The Netronome driver does allocate memory while reading the used_maps array so for simplicity it is made to hold the write-side lock.

---
 .../net/ethernet/netronome/nfp/bpf/offload.c  | 33 +++++++++++------
 include/linux/bpf.h                           | 11 +++++-
 kernel/bpf/core.c                             | 25 ++++++++++---
 kernel/bpf/syscall.c                          | 19 ++++++++--
 kernel/bpf/verifier.c                         | 37 +++++++++++++------
 net/core/dev.c                                | 12 ++++--
 6 files changed, 100 insertions(+), 37 deletions(-)
drivers/net/ethernet/netronome/nfp/bpf/offload.c +22 -11
index ac02369174a9..74ed42b678e2 100644
@@ -111,34 +111,45 @@ static int
 nfp_map_ptrs_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
 		    struct bpf_prog *prog)
 {
-	int i, cnt, err;
+	struct bpf_used_maps *used_maps;
+	int i, cnt, err = 0;
+
+	/* We are calling sleepable functions while reading used_maps array */
+	mutex_lock(&prog->aux->used_maps_mutex);
+
+	used_maps = rcu_dereference_protected(prog->aux->used_maps,
+			lockdep_is_held(&prog->aux->used_maps_mutex));
 
 	/* Quickly count the maps we will have to remember */
 	cnt = 0;
-	for (i = 0; i < prog->aux->used_map_cnt; i++)
-		if (bpf_map_offload_neutral(prog->aux->used_maps[i]))
+	for (i = 0; i < used_maps->cnt; i++)
+		if (bpf_map_offload_neutral(used_maps->arr[i]))
 			cnt++;
 	if (!cnt)
-		return 0;
+		goto out;
 
 	nfp_prog->map_records = kmalloc_array(cnt,
 					      sizeof(nfp_prog->map_records[0]),
 					      GFP_KERNEL);
-	if (!nfp_prog->map_records)
-		return -ENOMEM;
+	if (!nfp_prog->map_records) {
+		err = -ENOMEM;
+		goto out;
+	}
 
-	for (i = 0; i < prog->aux->used_map_cnt; i++)
-		if (bpf_map_offload_neutral(prog->aux->used_maps[i])) {
+	for (i = 0; i < used_maps->cnt; i++)
+		if (bpf_map_offload_neutral(used_maps->arr[i])) {
 			err = nfp_map_ptr_record(bpf, nfp_prog,
-						 prog->aux->used_maps[i]);
+						 used_maps->arr[i]);
 			if (err) {
 				nfp_map_ptrs_forget(bpf, nfp_prog);
-				return err;
+				goto out;
 			}
 		}
 	WARN_ON(cnt != nfp_prog->map_records_cnt);
 
-	return 0;
+out:
+	mutex_unlock(&prog->aux->used_maps_mutex);
+	return err;
 }
 
 static int
include/linux/bpf.h +9 -2
index cef4ef0d2b4e..417189b4061d 100644
@@ -689,9 +689,15 @@ struct bpf_ctx_arg_aux {
 	u32 btf_id;
 };
 
+/* rcu struct for prog used_maps */
+struct bpf_used_maps {
+	u32 cnt;
+	struct bpf_map **arr;
+	struct rcu_head rcu;
+};
+
 struct bpf_prog_aux {
 	atomic64_t refcnt;
-	u32 used_map_cnt;
 	u32 max_ctx_offset;
 	u32 max_pkt_offset;
 	u32 max_tp_access;
@@ -722,7 +728,8 @@ struct bpf_prog_aux {
 	u32 size_poke_tab;
 	struct bpf_ksym ksym;
 	const struct bpf_prog_ops *ops;
-	struct bpf_map **used_maps;
+	struct mutex used_maps_mutex; /* write-side mutex for used_maps */
+	struct bpf_used_maps __rcu *used_maps;
 	struct bpf_prog *prog;
 	struct user_struct *user;
 	u64 load_time; /* ns since boottime */
kernel/bpf/core.c +20 -5
index bde93344164d..9766aa0337d9 100644
@@ -1746,11 +1746,16 @@ bool bpf_prog_array_compatible(struct bpf_array *array,
 
 static int bpf_check_tail_call(const struct bpf_prog *fp)
 {
-	struct bpf_prog_aux *aux = fp->aux;
+	const struct bpf_used_maps *used_maps;
 	int i;
 
-	for (i = 0; i < aux->used_map_cnt; i++) {
-		struct bpf_map *map = aux->used_maps[i];
+	rcu_read_lock();
+	used_maps = rcu_dereference(fp->aux->used_maps);
+	if (!used_maps)
+		goto out;
+
+	for (i = 0; i < used_maps->cnt; i++) {
+		struct bpf_map *map = used_maps->arr[i];
 		struct bpf_array *array;
 
 		if (map->map_type != BPF_MAP_TYPE_PROG_ARRAY)
@@ -1761,6 +1766,8 @@ static int bpf_check_tail_call(const struct bpf_prog *fp)
 			return -EINVAL;
 	}
 
+out:
+	rcu_read_unlock();
 	return 0;
 }
 
@@ -2113,8 +2120,16 @@ void __bpf_free_used_maps(struct bpf_prog_aux *aux,
 
 static void bpf_free_used_maps(struct bpf_prog_aux *aux)
 {
-	__bpf_free_used_maps(aux, aux->used_maps, aux->used_map_cnt);
-	kfree(aux->used_maps);
+	struct bpf_used_maps *used_maps = aux->used_maps;
+
+	if (!used_maps)
+		return;
+
+	__bpf_free_used_maps(aux, used_maps->arr, used_maps->cnt);
+	kfree(used_maps->arr);
+	kfree(used_maps);
+
+	mutex_destroy(&aux->used_maps_mutex);
 }
 
 static void bpf_prog_free_deferred(struct work_struct *work)
kernel/bpf/syscall.c +15 -4
index 2f343ce15747..3fde9dc4b595 100644
@@ -3149,11 +3149,15 @@ static const struct bpf_map *bpf_map_from_imm(const struct bpf_prog *prog,
 					      unsigned long addr, u32 *off,
 					      u32 *type)
 {
+	const struct bpf_used_maps *used_maps;
 	const struct bpf_map *map;
 	int i;
 
-	for (i = 0, *off = 0; i < prog->aux->used_map_cnt; i++) {
-		map = prog->aux->used_maps[i];
+	rcu_read_lock();
+	used_maps = rcu_dereference(prog->aux->used_maps);
+
+	for (i = 0, *off = 0; i < used_maps->cnt; i++) {
+		map = used_maps->arr[i];
 		if (map == (void *)addr) {
 			*type = BPF_PSEUDO_MAP_FD;
 			return map;
@@ -3166,6 +3170,7 @@ static const struct bpf_map *bpf_map_from_imm(const struct bpf_prog *prog,
 		}
 	}
 
+	rcu_read_unlock();
 	return NULL;
 }
 
@@ -3263,6 +3268,7 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	struct bpf_prog_stats stats;
 	char __user *uinsns;
 	u32 ulen;
+	const struct bpf_used_maps *used_maps;
 	int err;
 
 	err = bpf_check_uarg_tail_zero(uinfo, sizeof(info), info_len);
@@ -3284,19 +3290,24 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	memcpy(info.tag, prog->tag, sizeof(prog->tag));
 	memcpy(info.name, prog->aux->name, sizeof(prog->aux->name));
 
+	rcu_read_lock();
+	used_maps = rcu_dereference(prog->aux->used_maps);
+
 	ulen = info.nr_map_ids;
-	info.nr_map_ids = prog->aux->used_map_cnt;
+	info.nr_map_ids = used_maps->cnt;
 	ulen = min_t(u32, info.nr_map_ids, ulen);
 	if (ulen) {
 		u32 __user *user_map_ids = u64_to_user_ptr(info.map_ids);
 		u32 i;
 
 		for (i = 0; i < ulen; i++)
-			if (put_user(prog->aux->used_maps[i]->id,
+			if (put_user(used_maps->arr[i]->id,
 				     &user_map_ids[i]))
 				return -EFAULT;
 	}
 
+	rcu_read_unlock();
+
 	err = set_info_rec_size(&info);
 	if (err)
 		return err;
kernel/bpf/verifier.c +25 -12
index b6ccfce3bf4c..9a6ca16667c7 100644
@@ -11232,25 +11232,38 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 		goto err_release_maps;
 	}
 
-	if (ret == 0 && env->used_map_cnt) {
+	if (ret == 0) {
 		/* if program passed verifier, update used_maps in bpf_prog_info */
-		env->prog->aux->used_maps = kmalloc_array(env->used_map_cnt,
-							  sizeof(env->used_maps[0]),
+		struct bpf_used_maps *used_maps = kzalloc(sizeof(*used_maps),
 							  GFP_KERNEL);
-
-		if (!env->prog->aux->used_maps) {
+		if (!used_maps) {
 			ret = -ENOMEM;
 			goto err_release_maps;
 		}
 
-		memcpy(env->prog->aux->used_maps, env->used_maps,
-		       sizeof(env->used_maps[0]) * env->used_map_cnt);
-		env->prog->aux->used_map_cnt = env->used_map_cnt;
+		if (env->used_map_cnt) {
+			used_maps->arr = kmalloc_array(env->used_map_cnt,
+						       sizeof(env->used_maps[0]),
+						       GFP_KERNEL);
 
-		/* program is valid. Convert pseudo bpf_ld_imm64 into generic
-		 * bpf_ld_imm64 instructions
-		 */
-		convert_pseudo_ld_imm64(env);
+			if (!used_maps->arr) {
+				kfree(used_maps);
+				ret = -ENOMEM;
+				goto err_release_maps;
+			}
+
+			memcpy(used_maps->arr, env->used_maps,
+			       sizeof(env->used_maps[0]) * env->used_map_cnt);
+			used_maps->cnt = env->used_map_cnt;
+
+			/* program is valid. Convert pseudo bpf_ld_imm64 into generic
+			 * bpf_ld_imm64 instructions
+			 */
+			convert_pseudo_ld_imm64(env);
+		}
+
+		env->prog->aux->used_maps = used_maps;
+		mutex_init(&env->prog->aux->used_maps_mutex);
 	}
 
 	if (ret == 0)
net/core/dev.c +9 -3
index 7df6c9617321..bebbf8abd9a7 100644
@@ -5439,17 +5439,23 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 	int ret = 0;
 
 	if (new) {
+		const struct bpf_used_maps *used_maps;
 		u32 i;
 
+		rcu_read_lock();
+		used_maps = rcu_dereference(new->aux->used_maps);
+
 		/* generic XDP does not work with DEVMAPs that can
 		 * have a bpf_prog installed on an entry
 		 */
-		for (i = 0; i < new->aux->used_map_cnt; i++) {
-			if (dev_map_can_have_prog(new->aux->used_maps[i]))
+		for (i = 0; i < used_maps->cnt; i++) {
+			if (dev_map_can_have_prog(used_maps->arr[i]))
 				return -EINVAL;
-			if (cpu_map_prog_allowed(new->aux->used_maps[i]))
+			if (cpu_map_prog_allowed(used_maps->arr[i]))
 				return -EINVAL;
 		}
+
+		rcu_read_unlock();
 	}
 
 	switch (xdp->command) {
Signature · 1 line
2.28.0
#2 lore raw

Re: [RFC PATCH bpf-next 1/5] bpf: RCU protect used_maps array and count

Quoted 16 lines · YiFei Zhu
kernel/bpf/syscall.c +4 -0
@@ -3263,6 +3268,7 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	struct bpf_prog_stats stats;
 	char __user *uinsns;
 	u32 ulen;
+	const struct bpf_used_maps *used_maps;
 	int err;
 
 	err = bpf_check_uarg_tail_zero(uinfo, sizeof(info), info_len);
@@ -3284,19 +3290,24 @@ static int bpf_prog_get_info_by_fd(struct file *file,
 	memcpy(info.tag, prog->tag, sizeof(prog->tag));
 	memcpy(info.name, prog->aux->name, sizeof(prog->aux->name));
 
+	rcu_read_lock();
+	used_maps = rcu_dereference(prog->aux->used_maps);
+
 	ulen = info.nr_map_ids;

kernel/bpf/syscall.c static int bpf_prog_get_info_by_fd(struct file *file, · line 3304

Quoted 10 lines
kernel/bpf/syscall.c +2 -2
-	info.nr_map_ids = prog->aux->used_map_cnt;
+	info.nr_map_ids = used_maps->cnt;
 	ulen = min_t(u32, info.nr_map_ids, ulen);
 	if (ulen) {
 		u32 __user *user_map_ids = u64_to_user_ptr(info.map_ids);
 		u32 i;
 
 		for (i = 0; i < ulen; i++)
-			if (put_user(prog->aux->used_maps[i]->id,
+			if (put_user(used_maps->arr[i]->id,

put_user() under rcu_read_lock() is not allowed.
You should see a splat like this:

[    2.297943] BUG: sleeping function called from invalid context at kernel/bpf/syscall.c:3305
[    2.305554] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 81, name: test_progs
[    2.312802] 1 lock held by test_progs/81:
[    2.316545]  #0: ffffffff82265760 (rcu_read_lock){....}-{1:2}, at: bpf_prog_get_info_by_fd.isra.31+0xb9/0xdf0
[    2.325446] Preemption disabled at:
[    2.325454] [<ffffffff812aec64>] __fd_install+0x24/0x2b0
[    2.333571] CPU: 1 PID: 81 Comm: test_progs Not tainted 5.8.0-ga96d3fdf9 #1
[    2.339818] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
[    2.347795] Call Trace:
[    2.350082]  dump_stack+0x78/0xab
[    2.353109]  ___might_sleep+0x17d/0x260
[    2.356554]  __might_fault+0x34/0x90
[    2.359873]  bpf_prog_get_info_by_fd.isra.31+0x21b/0xdf0
[    2.364674]  ? __lock_acquire+0x2e4/0x1df0
[    2.368377]  ? bpf_obj_get_info_by_fd+0x20f/0x3e0
[    2.372621]  bpf_obj_get_info_by_fd+0x20f/0x3e0

Please test your patches with kernel debug flags
like CONFIG_DEBUG_ATOMIC_SLEEP=y at a minimum.
kasan and lockdep are good to have as well.

In general I think rcu here is overkill.
Why not to use mutex everywhere? It's not like it's in critical path.

Also I think better name is needed. ADD_MAP is too specific. When we get around to implement Daniel's suggestion of replacing one map with another ADD_MAP as command won't fit.
Single purpose commands are ok, but this one feels too narrow. May be BPF_PROG_BIND_MAP ?
Then later adding target_map_fd field to prog_bind_map struct would do it. I thought about BPF_PROG_ATTACH_MAP name, but we use the word "attach" in too many places and it could be confusing.

author
#3 lore raw

RFC PATCH 2/5 bpf: Add BPF_PROG_ADD_MAP syscall

In reply to YiFei Zhu

From: YiFei Zhu <[email protected]>

This syscall attaches a map to a program. -EEXIST if the map is already attached to the program. call_rcu is used to free the old used_maps struct after an RCU grace period.

---
 include/uapi/linux/bpf.h       |  7 +++
 kernel/bpf/syscall.c           | 83 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  7 +++
 3 files changed, 97 insertions(+)
include/uapi/linux/bpf.h +7 -0
index b134e679e9db..01b32036a0f5 100644
@@ -118,6 +118,7 @@ enum bpf_cmd {
 	BPF_ENABLE_STATS,
 	BPF_ITER_CREATE,
 	BPF_LINK_DETACH,
+	BPF_PROG_ADD_MAP,
 };
 
 enum bpf_map_type {
@@ -648,6 +649,12 @@ union bpf_attr {
 		__u32		flags;
 	} iter_create;
 
+	struct { /* struct used by BPF_PROG_ADD_MAP command */
+		__u32		prog_fd;
+		__u32		map_fd;
+		__u32		flags;		/* extra flags */
+	} prog_add_map;
+
 } __attribute__((aligned(8)));
 
 /* The description below is an attempt at providing documentation to eBPF
kernel/bpf/syscall.c +83 -0

95 lines not on this page · raw message

tools/include/uapi/linux/bpf.h +7 -0
index b134e679e9db..01b32036a0f5 100644
@@ -118,6 +118,7 @@ enum bpf_cmd {
 	BPF_ENABLE_STATS,
 	BPF_ITER_CREATE,
 	BPF_LINK_DETACH,
+	BPF_PROG_ADD_MAP,
 };
 
 enum bpf_map_type {
@@ -648,6 +649,12 @@ union bpf_attr {
 		__u32		flags;
 	} iter_create;
 
+	struct { /* struct used by BPF_PROG_ADD_MAP command */
+		__u32		prog_fd;
+		__u32		map_fd;
+		__u32		flags;		/* extra flags */
+	} prog_add_map;
+
 } __attribute__((aligned(8)));
 
 /* The description below is an attempt at providing documentation to eBPF
Signature · 1 line
2.28.0
author
#4 lore raw

RFC PATCH 3/5 libbpf: Add BPF_PROG_ADD_MAP syscall and use it on .metadata section

In reply to YiFei Zhu

From: YiFei Zhu <[email protected]>

The patch adds a simple wrapper bpf_prog_add_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_ADD_MAP syscall and a .metadata section exists, the map will be explicitly attached to the program via the syscall immediately after program is loaded. -EEXIST is ignored for this syscall.

---
 tools/lib/bpf/bpf.c      | 11 +++++
 tools/lib/bpf/bpf.h      |  1 +
 tools/lib/bpf/libbpf.c   | 97 +++++++++++++++++++++++++++++++++++++++-
 tools/lib/bpf/libbpf.map |  1 +
 4 files changed, 109 insertions(+), 1 deletion(-)
tools/lib/bpf/bpf.c +11 -0
index eab14c97c15d..9b2173d4f92c 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_add_map(int prog_fd, int map_fd, int flags)
+{
+	union bpf_attr attr = {};
+
+	attr.prog_add_map.prog_fd = prog_fd;
+	attr.prog_add_map.map_fd = map_fd;
+	attr.prog_add_map.flags = flags;
+
+	return sys_bpf(BPF_PROG_ADD_MAP, &attr, sizeof(attr));
+}
tools/lib/bpf/bpf.h +1 -0
index 28855fd5b5f4..d76fee8f84e0 100644
@@ -240,6 +240,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_add_map(int prog_fd, int map_fd, int flags);
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
tools/lib/bpf/libbpf.c +96 -1

193 lines not on this page · raw message

tools/lib/bpf/libbpf.map +1 -0
index 0c4722bfdd0a..86494a464c34 100644
@@ -288,6 +288,7 @@ LIBBPF_0.1.0 {
 		bpf_map__set_value_size;
 		bpf_map__type;
 		bpf_map__value_size;
+		bpf_prog_add_map;
 		bpf_program__attach_xdp;
 		bpf_program__autoload;
 		bpf_program__is_sk_lookup;
Signature · 1 line
2.28.0
author
#5 lore raw

RFC PATCH 4/5 bpftool: support dumping metadata

In reply to 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
          }
      }
  ]
---
 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        | 132 ++++++++++++++++++++++++++++++++
 5 files changed, 152 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 e3a79b5a9960..54adfda5a9c9 100644
@@ -82,6 +82,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 +132 -0

150 lines not on this page · raw message

Signature · 1 line
2.28.0
author
#6 lore raw

RFC PATCH 5/5 selftests/bpf: Test bpftool loading and dumping metadata

In reply to 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.

---
 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 +2 -1
index e7a8cf83ba48..5f559c79a977 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 \
 
 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";
tools/testing/selftests/bpf/progs/metadata_used.c new file +15 -0
new file mode 100644
index 000000000000..59785404f7bb
@@ -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") = "bar";
+int metadata_b SEC(".metadata") = 2;
+
+SEC("cgroup_skb/egress")
+int prog(struct xdp_md *ctx)
+{
+	return metadata_b ? 1 : 0;
+}
+
+char _license[] SEC("license") = "GPL";
tools/testing/selftests/bpf/test_bpftool_metadata.sh new file +82 -0

82 lines not on this page · raw message

Signature · 1 line
2.28.0