Skip to content

Commit

Permalink
Merge branch 'bpf_metadata'
Browse files Browse the repository at this point in the history
Stanislav Fomichev says:

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

This patch set introduces a new syscall BPF_PROG_BIND_MAP to add a map
to a program's used_maps, even if the program instructions does not
reference the map.

libbpf is extended to always BPF_PROG_BIND_MAP .rodata section so the
metadata is kept in place.
bpftool is also extended to print metadata in the 'bpftool prog' list.

The variable is considered metadata if it starts with the
magic 'bpf_metadata_' prefix; everything after the prefix is the
metadata name.

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

  volatile const char bpf_metadata_commit_hash[] SEC(".rodata") = "abcdef123456";

and bpftool would emit:

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

v6 changes:
* libbpf: drop FEAT_GLOBAL_DATA from probe_prog_bind_map (Andrii Nakryiko)
* bpftool: combine find_metadata_map_id & find_metadata;
  drops extra bpf_map_get_fd_by_id and bpf_map_get_fd_by_id (Andrii Nakryiko)
* bpftool: use strncmp instead of strstr (Andrii Nakryiko)
* bpftool: memset(map_info) and extra empty line (Andrii Nakryiko)

v5 changes:
* selftest: verify that prog holds rodata (Andrii Nakryiko)
* selftest: use volatile for metadata (Andrii Nakryiko)
* bpftool: use sizeof in BPF_METADATA_PREFIX_LEN (Andrii Nakryiko)
* bpftool: new find_metadata that does map lookup (Andrii Nakryiko)
* libbpf: don't generalize probe_create_global_data (Andrii Nakryiko)
* libbpf: use OPTS_VALID in bpf_prog_bind_map (Andrii Nakryiko)
* libbpf: keep LIBBPF_0.2.0 sorted (Andrii Nakryiko)

v4 changes:
* Don't return EEXIST from syscall if already bound (Andrii Nakryiko)
* Removed --metadata argument (Andrii Nakryiko)
* Removed custom .metadata section (Alexei Starovoitov)
* Addressed Andrii's suggestions about btf helpers and vsi (Andrii Nakryiko)
* Moved bpf_prog_find_metadata into bpftool (Alexei Starovoitov)

v3 changes:
* API changes for bpf_prog_find_metadata (Toke Høiland-Jørgensen)

v2 changes:
* Made struct bpf_prog_bind_opts in libbpf so flags is optional.
* Deduped probe_kern_global_data and probe_prog_bind_map into a common
  helper.
* Added comment regarding why EEXIST is ignored in libbpf bind map.
* Froze all LIBBPF_MAP_METADATA internal maps.
* Moved bpf_prog_bind_map into new LIBBPF_0.1.1 in libbpf.map.
* Added p_err() calls on error cases in bpftool show_prog_metadata.
* Reverse christmas tree coding style in bpftool show_prog_metadata.
* Made bpftool gen skeleton recognize .metadata as an internal map and
  generate datasec definition in skeleton.
* Added C test using skeleton to see asset that the metadata is what we
  expect and rebinding causes EEXIST.

v1 changes:
* Fixed a few missing unlocks, and missing close while iterating map fds.
* Move mutex initialization to right after prog aux allocation, and mutex
  destroy to right after prog aux free.
* s/ADD_MAP/BIND_MAP/
* Use mutex only instead of RCU to protect the used_map array & count.

Cc: YiFei Zhu <zhuyifei1999@gmail.com>
====================

Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Alexei Starovoitov committed Sep 16, 2020
2 parents d317b0a + d42d1cc commit ffa915f
Show file tree
Hide file tree
Showing 19 changed files with 678 additions and 18 deletions.
18 changes: 12 additions & 6 deletions drivers/net/ethernet/netronome/nfp/bpf/offload.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,34 +111,40 @@ 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;
for (i = 0; i < prog->aux->used_map_cnt; i++)
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])) {
err = nfp_map_ptr_record(bpf, 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
Expand Down
1 change: 1 addition & 0 deletions include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,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 */
Expand Down
7 changes: 7 additions & 0 deletions include/uapi/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ enum bpf_cmd {
BPF_ENABLE_STATS,
BPF_ITER_CREATE,
BPF_LINK_DETACH,
BPF_PROG_BIND_MAP,
};

enum bpf_map_type {
Expand Down Expand Up @@ -658,6 +659,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
Expand Down
15 changes: 11 additions & 4 deletions kernel/bpf/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down
79 changes: 75 additions & 4 deletions kernel/bpf/syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -3162,21 +3162,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,
Expand Down Expand Up @@ -3294,6 +3298,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);
Expand All @@ -3303,9 +3308,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)
Expand Down Expand Up @@ -4153,6 +4161,66 @@ 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)
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;
Expand Down Expand Up @@ -4286,6 +4354,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;
Expand Down
11 changes: 8 additions & 3 deletions net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions tools/bpf/bpftool/json_writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
3 changes: 3 additions & 0 deletions tools/bpf/bpftool/json_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading

0 comments on commit ffa915f

Please sign in to comment.