Re: [PATCH bpf-next 1/3] libbpf: Store map pin path in struct bpf_map
From: Andrii Nakryiko <hidden>
Date: 2019-10-22 17:37:30
Also in:
bpf
On Tue, Oct 22, 2019 at 9:08 AM Toke Høiland-Jørgensen [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Toke Høiland-Jørgensen <redacted> When pinning a map, store the pin path in struct bpf_map so it can be re-used later for un-pinning. This simplifies the later addition of per-map pin paths. Signed-off-by: Toke Høiland-Jørgensen <redacted> --- tools/lib/bpf/libbpf.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index cccfd9355134..b4fdd8ee3bbd 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c@@ -226,6 +226,7 @@ struct bpf_map { void *priv; bpf_map_clear_priv_t clear_priv; enum libbpf_map_type libbpf_type; + char *pin_path; }; struct bpf_secdata {@@ -1929,6 +1930,7 @@ int bpf_map__reuse_fd(struct bpf_map *map, int fd) if (err) goto err_close_new_fd; free(map->name); + zfree(&map->pin_path);
While you are touching this function, can you please also fix error handling in it? We should store -errno locally on error, before we call close() which might change errno.
quoted hunk ↗ jump to hunk
map->fd = new_fd; map->name = new_name;@@ -4022,6 +4024,7 @@ int bpf_map__pin(struct bpf_map *map, const char *path) return -errno; } + map->pin_path = strdup(path);
if (!map->pin_path) {
err = -errno;
goto err_close_new_fd;
}
quoted hunk ↗ jump to hunk
pr_debug("pinned map '%s'\n", path); return 0;@@ -4031,6 +4034,9 @@ int bpf_map__unpin(struct bpf_map *map, const char *path) { int err; + if (!path) + path = map->pin_path;
This semantics is kind of weird. Given we now remember pin_path, should we instead check that user-provided path is actually correct and matches what we stored? Alternatively, bpf_map__unpin() w/o path argument looks like a cleaner API.
quoted hunk ↗ jump to hunk
+ err = check_path(path); if (err) return err;@@ -4044,6 +4050,7 @@ int bpf_map__unpin(struct bpf_map *map, const char *path) if (err != 0) return -errno; pr_debug("unpinned map '%s'\n", path); + zfree(&map->pin_path); return 0; }@@ -4088,17 +4095,10 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path) err_unpin_maps: while ((map = bpf_map__prev(map, obj))) { - char buf[PATH_MAX]; - int len; - - len = snprintf(buf, PATH_MAX, "%s/%s", path, - bpf_map__name(map)); - if (len < 0) - continue; - else if (len >= PATH_MAX) + if (!map->pin_path) continue; - bpf_map__unpin(map, buf); + bpf_map__unpin(map, NULL); } return err;@@ -4248,6 +4248,7 @@ void bpf_object__close(struct bpf_object *obj) for (i = 0; i < obj->nr_maps; i++) { zfree(&obj->maps[i].name); + zfree(&obj->maps[i].pin_path); if (obj->maps[i].clear_priv) obj->maps[i].clear_priv(&obj->maps[i], obj->maps[i].priv);