* [PATCH bpf-next 1/5] bpf: Mutex protect used_maps array and count
2020-08-20 9:42 [PATCH bpf-next 0/5] Allow storage of flexible metadata information for eBPF programs YiFei Zhu
@ 2020-08-20 9:42 ` YiFei Zhu
2020-08-20 21:18 ` Yonghong Song
2020-08-20 9:42 ` [PATCH bpf-next 2/5] bpf: Add BPF_PROG_BIND_MAP syscall YiFei Zhu
` (3 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: YiFei Zhu @ 2020-08-20 9:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
From: YiFei Zhu <zhuyifei@google.com>
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 <zhuyifei@google.com>
---
.../net/ethernet/netronome/nfp/bpf/offload.c | 18 ++++++++++++------
include/linux/bpf.h | 1 +
kernel/bpf/core.c | 15 +++++++++++----
kernel/bpf/syscall.c | 16 ++++++++++++----
net/core/dev.c | 11 ++++++++---
5 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index ac02369174a9..53851853562c 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -111,7 +111,9 @@ static int
nfp_map_ptrs_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
struct bpf_prog *prog)
{
- int i, cnt, err;
+ int i, cnt, err = 0;
+
+ mutex_lock(&prog->aux->used_maps_mutex);
/* Quickly count the maps we will have to remember */
cnt = 0;
@@ -119,13 +121,15 @@ nfp_map_ptrs_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
if (bpf_map_offload_neutral(prog->aux->used_maps[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])) {
@@ -133,12 +137,14 @@ nfp_map_ptrs_record(struct nfp_app_bpf *bpf, struct nfp_prog *nfp_prog,
prog->aux->used_maps[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
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 55f694b63164..abbdde104cea 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -723,6 +723,7 @@ struct bpf_prog_aux {
struct bpf_ksym ksym;
const struct bpf_prog_ops *ops;
struct bpf_map **used_maps;
+ struct mutex used_maps_mutex; /* mutex for used_maps and used_map_cnt */
struct bpf_prog *prog;
struct user_struct *user;
u64 load_time; /* ns since boottime */
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ed0b3578867c..2a20c2833996 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -98,6 +98,7 @@ struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flag
fp->jit_requested = ebpf_jit_enabled();
INIT_LIST_HEAD_RCU(&fp->aux->ksym.lnode);
+ mutex_init(&fp->aux->used_maps_mutex);
return fp;
}
@@ -253,6 +254,7 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
void __bpf_prog_free(struct bpf_prog *fp)
{
if (fp->aux) {
+ mutex_destroy(&fp->aux->used_maps_mutex);
free_percpu(fp->aux->stats);
kfree(fp->aux->poke_tab);
kfree(fp->aux);
@@ -1747,8 +1749,9 @@ 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;
- int i;
+ int i, ret = 0;
+ mutex_lock(&aux->used_maps_mutex);
for (i = 0; i < aux->used_map_cnt; i++) {
struct bpf_map *map = aux->used_maps[i];
struct bpf_array *array;
@@ -1757,11 +1760,15 @@ static int bpf_check_tail_call(const struct bpf_prog *fp)
continue;
array = container_of(map, struct bpf_array, map);
- if (!bpf_prog_array_compatible(array, fp))
- return -EINVAL;
+ if (!bpf_prog_array_compatible(array, fp)) {
+ ret = -EINVAL;
+ goto out;
+ }
}
- return 0;
+out:
+ mutex_unlock(&aux->used_maps_mutex);
+ return ret;
}
static void bpf_prog_select_func(struct bpf_prog *fp)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 86299a292214..f49fd709ccd5 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3152,21 +3152,25 @@ static const struct bpf_map *bpf_map_from_imm(const struct bpf_prog *prog,
const struct bpf_map *map;
int i;
+ mutex_lock(&prog->aux->used_maps_mutex);
for (i = 0, *off = 0; i < prog->aux->used_map_cnt; i++) {
map = prog->aux->used_maps[i];
if (map == (void *)addr) {
*type = BPF_PSEUDO_MAP_FD;
- return map;
+ goto out;
}
if (!map->ops->map_direct_value_meta)
continue;
if (!map->ops->map_direct_value_meta(map, addr, off)) {
*type = BPF_PSEUDO_MAP_VALUE;
- return map;
+ goto out;
}
}
+ map = NULL;
- return NULL;
+out:
+ mutex_unlock(&prog->aux->used_maps_mutex);
+ return map;
}
static struct bpf_insn *bpf_insn_prepare_dump(const struct bpf_prog *prog,
@@ -3284,6 +3288,7 @@ 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));
+ mutex_lock(&prog->aux->used_maps_mutex);
ulen = info.nr_map_ids;
info.nr_map_ids = prog->aux->used_map_cnt;
ulen = min_t(u32, info.nr_map_ids, ulen);
@@ -3293,9 +3298,12 @@ static int bpf_prog_get_info_by_fd(struct file *file,
for (i = 0; i < ulen; i++)
if (put_user(prog->aux->used_maps[i]->id,
- &user_map_ids[i]))
+ &user_map_ids[i])) {
+ mutex_unlock(&prog->aux->used_maps_mutex);
return -EFAULT;
+ }
}
+ mutex_unlock(&prog->aux->used_maps_mutex);
err = set_info_rec_size(&info);
if (err)
diff --git a/net/core/dev.c b/net/core/dev.c
index b5d1129d8310..6957b31127d9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5441,15 +5441,20 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
if (new) {
u32 i;
+ mutex_lock(&new->aux->used_maps_mutex);
+
/* 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]))
- return -EINVAL;
- if (cpu_map_prog_allowed(new->aux->used_maps[i]))
+ if (dev_map_can_have_prog(new->aux->used_maps[i]) ||
+ cpu_map_prog_allowed(new->aux->used_maps[i])) {
+ mutex_unlock(&new->aux->used_maps_mutex);
return -EINVAL;
+ }
}
+
+ mutex_unlock(&new->aux->used_maps_mutex);
}
switch (xdp->command) {
--
2.28.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 1/5] bpf: Mutex protect used_maps array and count
2020-08-20 9:42 ` [PATCH bpf-next 1/5] bpf: Mutex protect used_maps array and count YiFei Zhu
@ 2020-08-20 21:18 ` Yonghong Song
0 siblings, 0 replies; 24+ messages in thread
From: Yonghong Song @ 2020-08-20 21:18 UTC (permalink / raw)
To: YiFei Zhu, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
On 8/20/20 2:42 AM, YiFei Zhu wrote:
> From: YiFei Zhu <zhuyifei@google.com>
>
> 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 <zhuyifei@google.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next 2/5] bpf: Add BPF_PROG_BIND_MAP syscall
2020-08-20 9:42 [PATCH bpf-next 0/5] Allow storage of flexible metadata information for eBPF programs YiFei Zhu
2020-08-20 9:42 ` [PATCH bpf-next 1/5] bpf: Mutex protect used_maps array and count YiFei Zhu
@ 2020-08-20 9:42 ` YiFei Zhu
2020-08-20 21:23 ` Yonghong Song
2020-08-20 9:42 ` [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section YiFei Zhu
` (2 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: YiFei Zhu @ 2020-08-20 9:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
From: YiFei Zhu <zhuyifei@google.com>
This syscall binds a map to a program. -EEXIST if the map is
already bound to the program.
Signed-off-by: YiFei Zhu <zhuyifei@google.com>
---
include/uapi/linux/bpf.h | 7 ++++
kernel/bpf/syscall.c | 65 ++++++++++++++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 7 ++++
3 files changed, 79 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0480f893facd..d11b2ee62148 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -124,6 +124,7 @@ enum bpf_cmd {
BPF_ENABLE_STATS,
BPF_ITER_CREATE,
BPF_LINK_DETACH,
+ BPF_PROG_BIND_MAP,
};
enum bpf_map_type {
@@ -649,6 +650,12 @@ union bpf_attr {
__u32 flags;
} iter_create;
+ struct { /* struct used by BPF_PROG_BIND_MAP command */
+ __u32 prog_fd;
+ __u32 map_fd;
+ __u32 flags; /* extra flags */
+ } prog_bind_map;
+
} __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 f49fd709ccd5..f3e0457819a0 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4141,6 +4141,68 @@ static int bpf_iter_create(union bpf_attr *attr)
return err;
}
+#define BPF_PROG_BIND_MAP_LAST_FIELD prog_bind_map.flags
+
+static int bpf_prog_bind_map(union bpf_attr *attr)
+{
+ struct bpf_prog *prog;
+ struct bpf_map *map;
+ struct bpf_map **used_maps_old, **used_maps_new;
+ int i, ret = 0;
+
+ if (CHECK_ATTR(BPF_PROG_BIND_MAP))
+ return -EINVAL;
+
+ if (attr->prog_bind_map.flags)
+ return -EINVAL;
+
+ prog = bpf_prog_get(attr->prog_bind_map.prog_fd);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ map = bpf_map_get(attr->prog_bind_map.map_fd);
+ if (IS_ERR(map)) {
+ ret = PTR_ERR(map);
+ goto out_prog_put;
+ }
+
+ mutex_lock(&prog->aux->used_maps_mutex);
+
+ used_maps_old = prog->aux->used_maps;
+
+ for (i = 0; i < prog->aux->used_map_cnt; i++)
+ if (used_maps_old[i] == map) {
+ ret = -EEXIST;
+ goto out_unlock;
+ }
+
+ used_maps_new = kmalloc_array(prog->aux->used_map_cnt + 1,
+ sizeof(used_maps_new[0]),
+ GFP_KERNEL);
+ if (!used_maps_new) {
+ ret = -ENOMEM;
+ goto out_unlock;
+ }
+
+ memcpy(used_maps_new, used_maps_old,
+ sizeof(used_maps_old[0]) * prog->aux->used_map_cnt);
+ used_maps_new[prog->aux->used_map_cnt] = map;
+
+ prog->aux->used_map_cnt++;
+ prog->aux->used_maps = used_maps_new;
+
+ kfree(used_maps_old);
+
+out_unlock:
+ mutex_unlock(&prog->aux->used_maps_mutex);
+
+ if (ret)
+ bpf_map_put(map);
+out_prog_put:
+ bpf_prog_put(prog);
+ return ret;
+}
+
SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
{
union bpf_attr attr;
@@ -4274,6 +4336,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
case BPF_LINK_DETACH:
err = link_detach(&attr);
break;
+ case BPF_PROG_BIND_MAP:
+ err = bpf_prog_bind_map(&attr);
+ break;
default:
err = -EINVAL;
break;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0480f893facd..d11b2ee62148 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -124,6 +124,7 @@ enum bpf_cmd {
BPF_ENABLE_STATS,
BPF_ITER_CREATE,
BPF_LINK_DETACH,
+ BPF_PROG_BIND_MAP,
};
enum bpf_map_type {
@@ -649,6 +650,12 @@ union bpf_attr {
__u32 flags;
} iter_create;
+ struct { /* struct used by BPF_PROG_BIND_MAP command */
+ __u32 prog_fd;
+ __u32 map_fd;
+ __u32 flags; /* extra flags */
+ } prog_bind_map;
+
} __attribute__((aligned(8)));
/* The description below is an attempt at providing documentation to eBPF
--
2.28.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section
2020-08-20 9:42 [PATCH bpf-next 0/5] Allow storage of flexible metadata information for eBPF programs YiFei Zhu
2020-08-20 9:42 ` [PATCH bpf-next 1/5] bpf: Mutex protect used_maps array and count YiFei Zhu
2020-08-20 9:42 ` [PATCH bpf-next 2/5] bpf: Add BPF_PROG_BIND_MAP syscall YiFei Zhu
@ 2020-08-20 9:42 ` YiFei Zhu
2020-08-20 20:38 ` Yonghong Song
` (2 more replies)
2020-08-20 9:42 ` [PATCH bpf-next 4/5] bpftool: support dumping metadata YiFei Zhu
2020-08-20 9:42 ` [PATCH bpf-next 5/5] selftests/bpf: Test bpftool loading and " YiFei Zhu
4 siblings, 3 replies; 24+ messages in thread
From: YiFei Zhu @ 2020-08-20 9:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
From: YiFei Zhu <zhuyifei@google.com>
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 <zhuyifei@google.com>
---
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(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 82b983ff6569..383b29ecb1fd 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -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));
+}
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 015d13f25fcc..32994a4e0bf6 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -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
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 77d420c02094..4725859099c5 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -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";
+
+ prog = bpf_load_program_xattr(&prog_attr, NULL, 0);
+ if (prog < 0) {
+ ret = -errno;
+ cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
+ pr_warn("Error in %s():%s(%d). Couldn't create simple program.\n",
+ __func__, cp, -ret);
+
+ 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,
@@ -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,
}
};
@@ -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)
pr_debug("verifier log:\n%s", log_buf);
+
+ if (prog->obj->metadata_map && kernel_supports(FEAT_PROG_BIND_MAP)) {
+ if (bpf_prog_bind_map(ret, bpf_map__fd(prog->obj->metadata_map), 0) &&
+ errno != EEXIST) {
+ int fd = ret;
+
+ ret = -errno;
+ 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;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index e35bd6cdbdbf..4baf18a6df69 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section
2020-08-20 9:42 ` [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section YiFei Zhu
@ 2020-08-20 20:38 ` Yonghong Song
2020-08-21 7:52 ` YiFei Zhu
2020-08-25 20:45 ` Andrey Ignatov
2020-08-26 4:02 ` Andrii Nakryiko
2 siblings, 1 reply; 24+ messages in thread
From: Yonghong Song @ 2020-08-20 20:38 UTC (permalink / raw)
To: YiFei Zhu, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
On 8/20/20 2:42 AM, YiFei Zhu wrote:
> From: YiFei Zhu <zhuyifei@google.com>
>
> 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 <zhuyifei@google.com>
> ---
> 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(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 82b983ff6569..383b29ecb1fd 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -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));
> +}
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 015d13f25fcc..32994a4e0bf6 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -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);
Maybe put "flags" as an optional parameter? Currently "flags" is not
used. Not sure how widely it may be used in the future. See other
syscall interface in the same file, e.g., bpf_link_create().
> #ifdef __cplusplus
> } /* extern "C" */
> #endif
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 77d420c02094..4725859099c5 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -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";
> +
> + prog = bpf_load_program_xattr(&prog_attr, NULL, 0);
> + if (prog < 0) {
> + ret = -errno;
> + cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
> + pr_warn("Error in %s():%s(%d). Couldn't create simple program.\n",
> + __func__, cp, -ret);
> +
> + close(map);
> + return ret;
> + }
A lot of duplicated codes here vs. probe_global_data.
Can we abstract common codes into separate routines?
> +
> + 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,
> }
> };
>
> @@ -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)
> pr_debug("verifier log:\n%s", log_buf);
> +
> + if (prog->obj->metadata_map && kernel_supports(FEAT_PROG_BIND_MAP)) {
> + if (bpf_prog_bind_map(ret, bpf_map__fd(prog->obj->metadata_map), 0) &&
> + errno != EEXIST) {
could you explain and possibly add comments in the code why EEXIST is
ignored in the failure case?
> + int fd = ret;
> +
> + ret = -errno;
libbpf_strerror_r understands positive and negative errno, so no need
"ret = -errno".
Question: should bpftool freeze the metadata map or not?
> + 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;
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index e35bd6cdbdbf..4baf18a6df69 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -288,6 +288,7 @@ LIBBPF_0.1.0 {
> bpf_map__set_value_size;
> bpf_map__type;
> bpf_map__value_size;
This needs to be in a new kernel release. For example
LIBBPF_0.1.1
> + bpf_prog_bind_map;
> bpf_program__attach_xdp;
> bpf_program__autoload;
> bpf_program__is_sk_lookup;
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section
2020-08-20 20:38 ` Yonghong Song
@ 2020-08-21 7:52 ` YiFei Zhu
2020-08-21 15:14 ` Yonghong Song
0 siblings, 1 reply; 24+ messages in thread
From: YiFei Zhu @ 2020-08-21 7:52 UTC (permalink / raw)
To: Yonghong Song
Cc: YiFei Zhu, bpf, Alexei Starovoitov, Daniel Borkmann,
Stanislav Fomichev, Mahesh Bandewar
On Thu, Aug 20, 2020 at 3:38 PM Yonghong Song <yhs@fb.com> wrote:
>
> > + int fd = ret;
> > +
> > + ret = -errno;
>
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section
2020-08-21 7:52 ` YiFei Zhu
@ 2020-08-21 15:14 ` Yonghong Song
0 siblings, 0 replies; 24+ messages in thread
From: Yonghong Song @ 2020-08-21 15:14 UTC (permalink / raw)
To: YiFei Zhu
Cc: YiFei Zhu, bpf, Alexei Starovoitov, Daniel Borkmann,
Stanislav Fomichev, Mahesh Bandewar
On 8/21/20 12:52 AM, YiFei Zhu wrote:
> On Thu, Aug 20, 2020 at 3:38 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>> + int fd = ret;
>>> +
>>> + ret = -errno;
>>
>> 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?
Aha, sorry. My bad. I missed that 'ret' is used later for the negative
code return value. Yes, your code looks fine.
>
> YiFei Zhu
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section
2020-08-20 9:42 ` [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section YiFei Zhu
2020-08-20 20:38 ` Yonghong Song
@ 2020-08-25 20:45 ` Andrey Ignatov
2020-08-26 4:02 ` Andrii Nakryiko
2 siblings, 0 replies; 24+ messages in thread
From: Andrey Ignatov @ 2020-08-25 20:45 UTC (permalink / raw)
To: YiFei Zhu
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
YiFei Zhu <zhuyifei1999@gmail.com> [Thu, 2020-08-20 02:43 -0700]:
> From: YiFei Zhu <zhuyifei@google.com>
>
> 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 <zhuyifei@google.com>
...
> @@ -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;
I wonder if the map should have BPF_F_RDONLY / BPF_F_RDONLY_PROG flags
set by libbpf?
At least in my use-case metadata should never change once the map is
created, neither from program nor from syscall side.
> +
> pr_debug("map %td is \"%s\"\n", map - obj->maps, map->name);
> return 0;
> }
--
Andrey Ignatov
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section
2020-08-20 9:42 ` [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section YiFei Zhu
2020-08-20 20:38 ` Yonghong Song
2020-08-25 20:45 ` Andrey Ignatov
@ 2020-08-26 4:02 ` Andrii Nakryiko
2 siblings, 0 replies; 24+ messages in thread
From: Andrii Nakryiko @ 2020-08-26 4:02 UTC (permalink / raw)
To: YiFei Zhu
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
On Thu, Aug 20, 2020 at 2:43 AM YiFei Zhu <zhuyifei1999@gmail.com> wrote:
>
> From: YiFei Zhu <zhuyifei@google.com>
>
> 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 <zhuyifei@google.com>
> ---
> 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(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 82b983ff6569..383b29ecb1fd 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -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 = {};
use explicit memset() to avoid potential issues with uninitialized paddings
> +
> + 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));
> +}
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 015d13f25fcc..32994a4e0bf6 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -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
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 77d420c02094..4725859099c5 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -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 */
not helper, see below
> + 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;
Let's keep the approach consistent with other special maps. See how
it's done for Kconfig with kconfig_map_idx.
> +
> 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);
don't log here, it's already logged by few more basic feature checks
> + 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";
> +
> + prog = bpf_load_program_xattr(&prog_attr, NULL, 0);
> + if (prog < 0) {
> + ret = -errno;
> + cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
> + pr_warn("Error in %s():%s(%d). Couldn't create simple program.\n",
> + __func__, cp, -ret);
> +
same, no need for logging
> + 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,
> @@ -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,
it's not a helper, it's bpf() syscall command (BPF_PROG_BIND_MAP,
right?), so "BPF_PROG_BIND_MAP support" would probably be an ok short
description.
> }
> };
>
> @@ -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)
> pr_debug("verifier log:\n%s", log_buf);
> +
> + if (prog->obj->metadata_map && kernel_supports(FEAT_PROG_BIND_MAP)) {
> + if (bpf_prog_bind_map(ret, bpf_map__fd(prog->obj->metadata_map), 0) &&
> + errno != EEXIST) {
> + int fd = ret;
> +
> + ret = -errno;
> + cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
> + pr_warn("add metadata map failed: %s\n", cp);
please use error message consistent with most libbpf code, something
like this should be enough:
pr_warn("prog '%s': failed to bind .metadata map: %d\n", prog->name, ret);
> + close(fd);
Do we really want to fail loading the program because we failed to
attach .metadata? Surely feature detection should ensure that this
doesn't fail, but stil...
> + goto out;
> + }
> + }
> +
> *pfd = ret;
> ret = 0;
> goto out;
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index e35bd6cdbdbf..4baf18a6df69 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -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
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-20 9:42 [PATCH bpf-next 0/5] Allow storage of flexible metadata information for eBPF programs YiFei Zhu
` (2 preceding siblings ...)
2020-08-20 9:42 ` [PATCH bpf-next 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section YiFei Zhu
@ 2020-08-20 9:42 ` YiFei Zhu
2020-08-20 21:11 ` Yonghong Song
2020-08-26 5:36 ` Andrii Nakryiko
2020-08-20 9:42 ` [PATCH bpf-next 5/5] selftests/bpf: Test bpftool loading and " YiFei Zhu
4 siblings, 2 replies; 24+ messages in thread
From: YiFei Zhu @ 2020-08-20 9:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
From: YiFei Zhu <zhuyifei@google.com>
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 <zhuyifei@google.com>
---
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(+)
diff --git a/tools/bpf/bpftool/json_writer.c b/tools/bpf/bpftool/json_writer.c
index 86501cd3c763..7fea83bedf48 100644
--- a/tools/bpf/bpftool/json_writer.c
+++ b/tools/bpf/bpftool/json_writer.c
@@ -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)
{
diff --git a/tools/bpf/bpftool/json_writer.h b/tools/bpf/bpftool/json_writer.h
index 35cf1f00f96c..8ace65cdb92f 100644
--- a/tools/bpf/bpftool/json_writer.h
+++ b/tools/bpf/bpftool/json_writer.h
@@ -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);
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 4a191fcbeb82..a681d568cfa7 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -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)
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index c46e52137b87..8750758e9150 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -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;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index d393eb8263a6..ee767b8d90fb 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -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);
+ __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;
+
+ 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;
+
+ 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)
+ goto out_close;
+
+ if (bpf_map_lookup_elem(map_fd, &key, value))
+ goto out_free;
+
+ 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,
+ };
+
+ jsonw_name(json_wtr, "metadata");
+
+ jsonw_start_object(json_wtr);
+ for (i = 0; i < vlen; i++) {
+ t_var = btf__type_by_id(btf, vsi[i].type);
+
+ if (BTF_INFO_KIND(t_var->info) != BTF_KIND_VAR)
+ continue;
+
+ 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,
+ };
+ if (!btf_wtr)
+ goto out_free;
+
+ printf("\tmetadata:");
+
+ for (i = 0; i < vlen; i++) {
+ t_var = btf__type_by_id(btf, vsi[i].type);
+
+ if (BTF_INFO_KIND(t_var->info) != BTF_KIND_VAR)
+ continue;
+
+ 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)
--
2.28.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-20 9:42 ` [PATCH bpf-next 4/5] bpftool: support dumping metadata YiFei Zhu
@ 2020-08-20 21:11 ` Yonghong Song
2020-08-21 8:58 ` Toke Høiland-Jørgensen
2020-08-26 5:36 ` Andrii Nakryiko
1 sibling, 1 reply; 24+ messages in thread
From: Yonghong Song @ 2020-08-20 21:11 UTC (permalink / raw)
To: YiFei Zhu, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
On 8/20/20 2:42 AM, YiFei Zhu wrote:
> From: YiFei Zhu <zhuyifei@google.com>
>
> 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 <zhuyifei@google.com>
> ---
> 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(+)
>
> diff --git a/tools/bpf/bpftool/json_writer.c b/tools/bpf/bpftool/json_writer.c
> index 86501cd3c763..7fea83bedf48 100644
> --- a/tools/bpf/bpftool/json_writer.c
> +++ b/tools/bpf/bpftool/json_writer.c
> @@ -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)
> {
> diff --git a/tools/bpf/bpftool/json_writer.h b/tools/bpf/bpftool/json_writer.h
> index 35cf1f00f96c..8ace65cdb92f 100644
> --- a/tools/bpf/bpftool/json_writer.h
> +++ b/tools/bpf/bpftool/json_writer.h
> @@ -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);
>
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 4a191fcbeb82..a681d568cfa7 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -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)
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index c46e52137b87..8750758e9150 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -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;
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index d393eb8263a6..ee767b8d90fb 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -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);
> + __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?
> +
> + 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.
> +
> + 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)
> + 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.
If we can put metadata in skeleton like
<metadata_type> *metadata;
and then it will be very easy for users to access it.
> +
> + 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,
> + };
> +
> + jsonw_name(json_wtr, "metadata");
> +
> + jsonw_start_object(json_wtr);
> + for (i = 0; i < vlen; i++) {
> + t_var = btf__type_by_id(btf, vsi[i].type);
> +
> + if (BTF_INFO_KIND(t_var->info) != BTF_KIND_VAR)
> + continue;
this should not happen.
> +
> + 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,
> + };
> + if (!btf_wtr)
> + goto out_free;
> +
> + printf("\tmetadata:");
> +
> + for (i = 0; i < vlen; i++) {
> + t_var = btf__type_by_id(btf, vsi[i].type);
> +
> + if (BTF_INFO_KIND(t_var->info) != BTF_KIND_VAR)
> + continue;
this should not happen.
> +
> + 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)
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-20 21:11 ` Yonghong Song
@ 2020-08-21 8:58 ` Toke Høiland-Jørgensen
2020-08-21 20:10 ` YiFei Zhu
0 siblings, 1 reply; 24+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-21 8:58 UTC (permalink / raw)
To: Yonghong Song, YiFei Zhu, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
Yonghong Song <yhs@fb.com> writes:
> On 8/20/20 2:42 AM, YiFei Zhu wrote:
>> From: YiFei Zhu <zhuyifei@google.com>
>>
>> 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 <zhuyifei@google.com>
>> ---
>> 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(+)
>>
>> diff --git a/tools/bpf/bpftool/json_writer.c b/tools/bpf/bpftool/json_writer.c
>> index 86501cd3c763..7fea83bedf48 100644
>> --- a/tools/bpf/bpftool/json_writer.c
>> +++ b/tools/bpf/bpftool/json_writer.c
>> @@ -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)
>> {
>> diff --git a/tools/bpf/bpftool/json_writer.h b/tools/bpf/bpftool/json_writer.h
>> index 35cf1f00f96c..8ace65cdb92f 100644
>> --- a/tools/bpf/bpftool/json_writer.h
>> +++ b/tools/bpf/bpftool/json_writer.h
>> @@ -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);
>>
>> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
>> index 4a191fcbeb82..a681d568cfa7 100644
>> --- a/tools/bpf/bpftool/main.c
>> +++ b/tools/bpf/bpftool/main.c
>> @@ -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)
>> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
>> index c46e52137b87..8750758e9150 100644
>> --- a/tools/bpf/bpftool/main.h
>> +++ b/tools/bpf/bpftool/main.h
>> @@ -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;
>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>> index d393eb8263a6..ee767b8d90fb 100644
>> --- a/tools/bpf/bpftool/prog.c
>> +++ b/tools/bpf/bpftool/prog.c
>> @@ -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);
>> + __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?
>
>> +
>> + 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.
>
>> +
>> + 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)
>> + 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.
Yeah, please put in a libbpf accessor as well; I would like to use this
from libxdp - without a skeleton :)
-Toke
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-21 8:58 ` Toke Høiland-Jørgensen
@ 2020-08-21 20:10 ` YiFei Zhu
2020-08-23 18:36 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 24+ messages in thread
From: YiFei Zhu @ 2020-08-21 20:10 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Yonghong Song, YiFei Zhu, bpf, Alexei Starovoitov,
Daniel Borkmann, Stanislav Fomichev, Mahesh Bandewar
On Fri, Aug 21, 2020 at 3:58 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> Yonghong Song <yhs@fb.com> writes:
> > 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
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?
YiFei Zhu
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-21 20:10 ` YiFei Zhu
@ 2020-08-23 18:36 ` Toke Høiland-Jørgensen
2020-08-28 17:00 ` sdf
0 siblings, 1 reply; 24+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-23 18:36 UTC (permalink / raw)
To: YiFei Zhu
Cc: Yonghong Song, YiFei Zhu, bpf, Alexei Starovoitov,
Daniel Borkmann, Stanislav Fomichev, Mahesh Bandewar
YiFei Zhu <zhuyifei@google.com> writes:
> On Fri, Aug 21, 2020 at 3:58 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> Yonghong Song <yhs@fb.com> writes:
>> > 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
>
> 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?
-Toke
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-23 18:36 ` Toke Høiland-Jørgensen
@ 2020-08-28 17:00 ` sdf
2020-08-28 20:55 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 24+ messages in thread
From: sdf @ 2020-08-28 17:00 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: YiFei Zhu, Yonghong Song, YiFei Zhu, bpf, Alexei Starovoitov,
Daniel Borkmann, Mahesh Bandewar
On 08/23, Toke H�iland-J�rgensen wrote:
> YiFei Zhu <zhuyifei@google.com> writes:
> > On Fri, Aug 21, 2020 at 3:58 AM Toke H�iland-J�rgensen
> <toke@redhat.com> wrote:
> >> Yonghong Song <yhs@fb.com> writes:
> >> > 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
> >
> > 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?
Sounds good, I'll be taking over this patch series as YiFei's internship
has ended. I'll try to address that.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-28 17:00 ` sdf
@ 2020-08-28 20:55 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 24+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-28 20:55 UTC (permalink / raw)
To: sdf
Cc: YiFei Zhu, Yonghong Song, YiFei Zhu, bpf, Alexei Starovoitov,
Daniel Borkmann, Mahesh Bandewar
sdf@google.com writes:
> On 08/23, Toke H�iland-J�rgensen wrote:
>> YiFei Zhu <zhuyifei@google.com> writes:
>
>> > On Fri, Aug 21, 2020 at 3:58 AM Toke H�iland-J�rgensen
>> <toke@redhat.com> wrote:
>> >> Yonghong Song <yhs@fb.com> writes:
>> >> > 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
>> >
>> > 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?
> Sounds good, I'll be taking over this patch series as YiFei's internship
> has ended. I'll try to address that.
Great, thanks! :)
-Toke
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-20 9:42 ` [PATCH bpf-next 4/5] bpftool: support dumping metadata YiFei Zhu
2020-08-20 21:11 ` Yonghong Song
@ 2020-08-26 5:36 ` Andrii Nakryiko
2020-08-28 16:59 ` sdf
1 sibling, 1 reply; 24+ messages in thread
From: Andrii Nakryiko @ 2020-08-26 5:36 UTC (permalink / raw)
To: YiFei Zhu
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
On Thu, Aug 20, 2020 at 2:44 AM YiFei Zhu <zhuyifei1999@gmail.com> wrote:
>
> From: YiFei Zhu <zhuyifei@google.com>
>
> 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 <zhuyifei@google.com>
> ---
> 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(+)
>
[...]
> + 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"))
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.
> + goto next_map;
> +
> + goto found;
> +
> +next_map:
> + close(map_fd);
> + }
> +
> + return;
> +
> +found:
[...]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-26 5:36 ` Andrii Nakryiko
@ 2020-08-28 16:59 ` sdf
2020-09-03 5:18 ` Andrii Nakryiko
0 siblings, 1 reply; 24+ messages in thread
From: sdf @ 2020-08-28 16:59 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: YiFei Zhu, bpf, Alexei Starovoitov, Daniel Borkmann,
Mahesh Bandewar, YiFei Zhu
On 08/25, Andrii Nakryiko wrote:
> On Thu, Aug 20, 2020 at 2:44 AM YiFei Zhu <zhuyifei1999@gmail.com> wrote:
> >
> > From: YiFei Zhu <zhuyifei@google.com>
> >
> > 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 <zhuyifei@google.com>
> > ---
> > 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(+)
> >
> [...]
> > + 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"))
> 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 :-/
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 4/5] bpftool: support dumping metadata
2020-08-28 16:59 ` sdf
@ 2020-09-03 5:18 ` Andrii Nakryiko
0 siblings, 0 replies; 24+ messages in thread
From: Andrii Nakryiko @ 2020-09-03 5:18 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: YiFei Zhu, bpf, Alexei Starovoitov, Daniel Borkmann,
Mahesh Bandewar, YiFei Zhu
On Fri, Aug 28, 2020 at 9:59 AM <sdf@google.com> wrote:
>
> On 08/25, Andrii Nakryiko wrote:
> > On Thu, Aug 20, 2020 at 2:44 AM YiFei Zhu <zhuyifei1999@gmail.com> wrote:
> > >
> > > From: YiFei Zhu <zhuyifei@google.com>
> > >
> > > 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 <zhuyifei@google.com>
> > > ---
> > > 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(+)
> > >
>
> > [...]
>
> > > + 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"))
>
> > 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 :-/
Just because we made a mistake once, doesn't mean we need to keep
making it. ".metadata" is 9 characters already, which leaves 6
characters for object name prefix, that's not a lot of useful
information anyway. As I said, we should probably fix it for other
global data maps as well, but we will have to do it gradually. For
.metadata we can do a nice and clean ".metadata" immediately, no need
to jump through migration and deprecation hoops.
Also, how is "test_vml.bss" consistent with "test_v.metadata"?
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next 5/5] selftests/bpf: Test bpftool loading and dumping metadata
2020-08-20 9:42 [PATCH bpf-next 0/5] Allow storage of flexible metadata information for eBPF programs YiFei Zhu
` (3 preceding siblings ...)
2020-08-20 9:42 ` [PATCH bpf-next 4/5] bpftool: support dumping metadata YiFei Zhu
@ 2020-08-20 9:42 ` YiFei Zhu
2020-08-20 21:15 ` Yonghong Song
4 siblings, 1 reply; 24+ messages in thread
From: YiFei Zhu @ 2020-08-20 9:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
From: YiFei Zhu <zhuyifei@google.com>
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 <zhuyifei@google.com>
---
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
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index a83b5827532f..04e56c6843c6 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -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 \
diff --git a/tools/testing/selftests/bpf/progs/metadata_unused.c b/tools/testing/selftests/bpf/progs/metadata_unused.c
new file mode 100644
index 000000000000..523b3c332426
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/metadata_unused.c
@@ -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";
diff --git a/tools/testing/selftests/bpf/progs/metadata_used.c b/tools/testing/selftests/bpf/progs/metadata_used.c
new file mode 100644
index 000000000000..59785404f7bb
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/metadata_used.c
@@ -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";
diff --git a/tools/testing/selftests/bpf/test_bpftool_metadata.sh b/tools/testing/selftests/bpf/test_bpftool_metadata.sh
new file mode 100755
index 000000000000..a7515c09dc2d
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_bpftool_metadata.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+TESTNAME=bpftool_metadata
+BPF_FS=$(awk '$3 == "bpf" {print $2; exit}' /proc/mounts)
+BPF_DIR=$BPF_FS/test_$TESTNAME
+
+_cleanup()
+{
+ set +e
+ rm -rf $BPF_DIR 2> /dev/null
+}
+
+cleanup_skip()
+{
+ echo "selftests: $TESTNAME [SKIP]"
+ _cleanup
+
+ exit $ksft_skip
+}
+
+cleanup()
+{
+ if [ "$?" = 0 ]; then
+ echo "selftests: $TESTNAME [PASS]"
+ else
+ echo "selftests: $TESTNAME [FAILED]"
+ fi
+ _cleanup
+}
+
+if [ $(id -u) -ne 0 ]; then
+ echo "selftests: $TESTNAME [SKIP] Need root privileges"
+ exit $ksft_skip
+fi
+
+if [ -z "$BPF_FS" ]; then
+ echo "selftests: $TESTNAME [SKIP] Could not run test without bpffs mounted"
+ exit $ksft_skip
+fi
+
+if ! bpftool version > /dev/null 2>&1; then
+ echo "selftests: $TESTNAME [SKIP] Could not run test without bpftool"
+ exit $ksft_skip
+fi
+
+set -e
+
+trap cleanup_skip EXIT
+
+mkdir $BPF_DIR
+
+trap cleanup EXIT
+
+bpftool prog load metadata_unused.o $BPF_DIR/unused
+
+METADATA_PLAIN="$(bpftool prog --metadata)"
+echo "$METADATA_PLAIN" | grep 'metadata_a = "foo"' > /dev/null
+echo "$METADATA_PLAIN" | grep 'metadata_b = 1' > /dev/null
+
+bpftool prog --metadata --json | grep '"metadata":{"metadata_a":"foo","metadata_b":1}' > /dev/null
+
+bpftool map | grep 'metada.metadata' > /dev/null
+
+rm $BPF_DIR/unused
+
+bpftool prog load metadata_used.o $BPF_DIR/used
+
+METADATA_PLAIN="$(bpftool prog --metadata)"
+echo "$METADATA_PLAIN" | grep 'metadata_a = "bar"' > /dev/null
+echo "$METADATA_PLAIN" | grep 'metadata_b = 2' > /dev/null
+
+bpftool prog --metadata --json | grep '"metadata":{"metadata_a":"bar","metadata_b":2}' > /dev/null
+
+bpftool map | grep 'metada.metadata' > /dev/null
+
+rm $BPF_DIR/used
+
+exit 0
--
2.28.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 5/5] selftests/bpf: Test bpftool loading and dumping metadata
2020-08-20 9:42 ` [PATCH bpf-next 5/5] selftests/bpf: Test bpftool loading and " YiFei Zhu
@ 2020-08-20 21:15 ` Yonghong Song
2020-08-26 4:00 ` Andrii Nakryiko
0 siblings, 1 reply; 24+ messages in thread
From: Yonghong Song @ 2020-08-20 21:15 UTC (permalink / raw)
To: YiFei Zhu, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Stanislav Fomichev,
Mahesh Bandewar, YiFei Zhu
On 8/20/20 2:42 AM, YiFei Zhu wrote:
> From: YiFei Zhu <zhuyifei@google.com>
>
> 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 <zhuyifei@google.com>
> ---
> 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
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index a83b5827532f..04e56c6843c6 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -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.
>
> TEST_PROGS_EXTENDED := with_addr.sh \
> with_tunnels.sh \
> diff --git a/tools/testing/selftests/bpf/progs/metadata_unused.c b/tools/testing/selftests/bpf/progs/metadata_unused.c
> new file mode 100644
> index 000000000000..523b3c332426
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/metadata_unused.c
> @@ -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";
[...]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next 5/5] selftests/bpf: Test bpftool loading and dumping metadata
2020-08-20 21:15 ` Yonghong Song
@ 2020-08-26 4:00 ` Andrii Nakryiko
0 siblings, 0 replies; 24+ messages in thread
From: Andrii Nakryiko @ 2020-08-26 4:00 UTC (permalink / raw)
To: Yonghong Song
Cc: YiFei Zhu, bpf, Alexei Starovoitov, Daniel Borkmann,
Stanislav Fomichev, Mahesh Bandewar, YiFei Zhu
On Thu, Aug 20, 2020 at 3:03 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 8/20/20 2:42 AM, YiFei Zhu wrote:
> > From: YiFei Zhu <zhuyifei@google.com>
> >
> > 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 <zhuyifei@google.com>
> > ---
> > 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
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index a83b5827532f..04e56c6843c6 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -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.
>
I concur. It also seems that program code can use metadata variables
just like .rodata variables (e.g., for debug logging, etc), so we need
to add tests exercising that ability as well.
> >
> > TEST_PROGS_EXTENDED := with_addr.sh \
> > with_tunnels.sh \
> > diff --git a/tools/testing/selftests/bpf/progs/metadata_unused.c b/tools/testing/selftests/bpf/progs/metadata_unused.c
> > new file mode 100644
> > index 000000000000..523b3c332426
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/metadata_unused.c
> > @@ -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";
> [...]
^ permalink raw reply [flat|nested] 24+ messages in thread