Skip to content

L
linux
Commit 2d39d7c5 authored by Andrii Nakryiko's avatar Andrii Nakryiko Committed by Alexei Starovoitov
Browse files
Options

libbpf: Refactor map creation logic and fix cleanup leak 

Factor out map creation and destruction logic to simplify code and especially
error handling. Also fix map FD leak in case of partially successful map
creation during bpf_object load operation.

Fixes: 57a00f41 ("libbpf: Add auto-pinning of maps when loading BPF objects")
Signed-off-by: default avatarAndrii Nakryiko <andriin@fb.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Acked-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/bpf/20200429002739.48006-3-andriin@fb.com
parent 41017e56
Hide whitespace changes
Inline Side-by-side
Showing with 121 additions and 105 deletions
tools/lib/bpf/libbpf.c
View file @ 2d39d7c5
...@@ -3493,107 +3493,111 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map) ...@@ -3493,107 +3493,111 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
return 0; return 0;
} }
static void bpf_map__destroy(struct bpf_map *map);
static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map)
{
struct bpf_create_map_attr create_attr;
struct bpf_map_def *def = &map->def;
memset(&create_attr, 0, sizeof(create_attr));
if (obj->caps.name)
create_attr.name = map->name;
create_attr.map_ifindex = map->map_ifindex;
create_attr.map_type = def->type;
create_attr.map_flags = def->map_flags;
create_attr.key_size = def->key_size;
create_attr.value_size = def->value_size;
if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY && !def->max_entries) {
int nr_cpus;
nr_cpus = libbpf_num_possible_cpus();
if (nr_cpus < 0) {
pr_warn("map '%s': failed to determine number of system CPUs: %d\n",
map->name, nr_cpus);
return nr_cpus;
}
pr_debug("map '%s': setting size to %d\n", map->name, nr_cpus);
create_attr.max_entries = nr_cpus;
} else {
create_attr.max_entries = def->max_entries;
}
if (bpf_map__is_struct_ops(map))
create_attr.btf_vmlinux_value_type_id =
map->btf_vmlinux_value_type_id;
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
if (obj->btf && !bpf_map_find_btf_info(obj, map)) {
create_attr.btf_fd = btf__fd(obj->btf);
create_attr.btf_key_type_id = map->btf_key_type_id;
create_attr.btf_value_type_id = map->btf_value_type_id;
}
map->fd = bpf_create_map_xattr(&create_attr);
if (map->fd < 0 && (create_attr.btf_key_type_id ||
create_attr.btf_value_type_id)) {
char *cp, errmsg[STRERR_BUFSIZE];
int err = -errno;
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
map->name, cp, err);
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
map->btf_key_type_id = 0;
map->btf_value_type_id = 0;
map->fd = bpf_create_map_xattr(&create_attr);
}
if (map->fd < 0)
return -errno;
return 0;
}
static int static int
bpf_object__create_maps(struct bpf_object *obj) bpf_object__create_maps(struct bpf_object *obj)
{ {
struct bpf_create_map_attr create_attr = {}; struct bpf_map *map;
int nr_cpus = 0; char *cp, errmsg[STRERR_BUFSIZE];
unsigned int i; unsigned int i, j;
int err; int err;
for (i = 0; i < obj->nr_maps; i++) { for (i = 0; i < obj->nr_maps; i++) {
struct bpf_map *map = &obj->maps[i]; map = &obj->maps[i];
struct bpf_map_def *def = &map->def;
char *cp, errmsg[STRERR_BUFSIZE];
int *pfd = &map->fd;
if (map->pin_path) { if (map->pin_path) {
err = bpf_object__reuse_map(map); err = bpf_object__reuse_map(map);
if (err) { if (err) {
pr_warn("error reusing pinned map %s\n", pr_warn("map '%s': error reusing pinned map\n",
map->name); map->name);
return err; goto err_out;
} }
} }
if (map->fd >= 0) { if (map->fd >= 0) {
pr_debug("skip map create (preset) %s: fd=%d\n", pr_debug("map '%s': skipping creation (preset fd=%d)\n",
map->name, map->fd); map->name, map->fd);
continue; continue;
} }
if (obj->caps.name) err = bpf_object__create_map(obj, map);
create_attr.name = map->name; if (err)
create_attr.map_ifindex = map->map_ifindex; goto err_out;
create_attr.map_type = def->type;
create_attr.map_flags = def->map_flags;
create_attr.key_size = def->key_size;
create_attr.value_size = def->value_size;
if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY &&
!def->max_entries) {
if (!nr_cpus)
nr_cpus = libbpf_num_possible_cpus();
if (nr_cpus < 0) {
pr_warn("failed to determine number of system CPUs: %d\n",
nr_cpus);
err = nr_cpus;
goto err_out;
}
pr_debug("map '%s': setting size to %d\n",
map->name, nr_cpus);
create_attr.max_entries = nr_cpus;
} else {
create_attr.max_entries = def->max_entries;
}
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
if (bpf_map_type__is_map_in_map(def->type) &&
map->inner_map_fd >= 0)
create_attr.inner_map_fd = map->inner_map_fd;
if (bpf_map__is_struct_ops(map))
create_attr.btf_vmlinux_value_type_id =
map->btf_vmlinux_value_type_id;
if (obj->btf && !bpf_map_find_btf_info(obj, map)) {
create_attr.btf_fd = btf__fd(obj->btf);
create_attr.btf_key_type_id = map->btf_key_type_id;
create_attr.btf_value_type_id = map->btf_value_type_id;
}
*pfd = bpf_create_map_xattr(&create_attr);
if (*pfd < 0 && (create_attr.btf_key_type_id ||
create_attr.btf_value_type_id)) {
err = -errno;
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
map->name, cp, err);
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
map->btf_key_type_id = 0;
map->btf_value_type_id = 0;
*pfd = bpf_create_map_xattr(&create_attr);
}
if (*pfd < 0) {
size_t j;
err = -errno; pr_debug("map '%s': created successfully, fd=%d\n", map->name,
err_out: map->fd);
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("failed to create map (name: '%s'): %s(%d)\n",
map->name, cp, err);
pr_perm_msg(err);
for (j = 0; j < i; j++)
zclose(obj->maps[j].fd);
return err;
}
if (bpf_map__is_internal(map)) { if (bpf_map__is_internal(map)) {
err = bpf_object__populate_internal_map(obj, map); err = bpf_object__populate_internal_map(obj, map);
if (err < 0) { if (err < 0) {
zclose(*pfd); zclose(map->fd);
goto err_out; goto err_out;
} }
} }
...@@ -3601,16 +3605,23 @@ bpf_object__create_maps(struct bpf_object *obj) ...@@ -3601,16 +3605,23 @@ bpf_object__create_maps(struct bpf_object *obj)
if (map->pin_path && !map->pinned) { if (map->pin_path && !map->pinned) {
err = bpf_map__pin(map, NULL); err = bpf_map__pin(map, NULL);
if (err) { if (err) {
pr_warn("failed to auto-pin map name '%s' at '%s'\n", pr_warn("map '%s': failed to auto-pin at '%s': %d\n",
map->name, map->pin_path); map->name, map->pin_path, err);
return err; zclose(map->fd);
goto err_out;
} }
} }
pr_debug("created map %s: fd=%d\n", map->name, *pfd);
} }
return 0; return 0;
err_out:
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("map '%s': failed to create: %s(%d)\n", map->name, cp, err);
pr_perm_msg(err);
for (j = 0; j < i; j++)
zclose(obj->maps[j].fd);
return err;
} }
static int static int
...@@ -5966,6 +5977,32 @@ int bpf_object__pin(struct bpf_object *obj, const char *path) ...@@ -5966,6 +5977,32 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
return 0; return 0;
} }
static void bpf_map__destroy(struct bpf_map *map)
{
if (map->clear_priv)
map->clear_priv(map, map->priv);
map->priv = NULL;
map->clear_priv = NULL;
if (map->mmaped) {
munmap(map->mmaped, bpf_map_mmap_sz(map));
map->mmaped = NULL;
}
if (map->st_ops) {
zfree(&map->st_ops->data);
zfree(&map->st_ops->progs);
zfree(&map->st_ops->kern_func_off);
zfree(&map->st_ops);
}
zfree(&map->name);
zfree(&map->pin_path);
if (map->fd >= 0)
zclose(map->fd);
}
void bpf_object__close(struct bpf_object *obj) void bpf_object__close(struct bpf_object *obj)
{ {
size_t i; size_t i;
...@@ -5981,29 +6018,8 @@ void bpf_object__close(struct bpf_object *obj) ...@@ -5981,29 +6018,8 @@ void bpf_object__close(struct bpf_object *obj)
btf__free(obj->btf); btf__free(obj->btf);
btf_ext__free(obj->btf_ext); btf_ext__free(obj->btf_ext);
for (i = 0; i < obj->nr_maps; i++) { for (i = 0; i < obj->nr_maps; i++)
struct bpf_map *map = &obj->maps[i]; bpf_map__destroy(&obj->maps[i]);
if (map->clear_priv)
map->clear_priv(map, map->priv);
map->priv = NULL;
map->clear_priv = NULL;
if (map->mmaped) {
munmap(map->mmaped, bpf_map_mmap_sz(map));
map->mmaped = NULL;
}
if (map->st_ops) {
zfree(&map->st_ops->data);
zfree(&map->st_ops->progs);
zfree(&map->st_ops->kern_func_off);
zfree(&map->st_ops);
}
zfree(&map->name);
zfree(&map->pin_path);
}
zfree(&obj->kconfig); zfree(&obj->kconfig);
zfree(&obj->externs); zfree(&obj->externs);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7