Re: [PATCH bpf-next v4 2/5] libbpf: Store map pin path and status in struct bpf_map
From: Andrii Nakryiko <hidden>
Date: 2019-10-31 17:22:33
Also in:
bpf
On Tue, Oct 29, 2019 at 12:39 PM Toke Høiland-Jørgensen [off-list ref] wrote:
From: Toke Høiland-Jørgensen <redacted>
Support storing and setting a pin path in struct bpf_map, which can be used
for automatic pinning. Also store the pin status so we can avoid attempts
to re-pin a map that has already been pinned (or reused from a previous
pinning).
The behaviour of bpf_object__{un,}pin_maps() is changed so that if it is
called with a NULL path argument (which was previously illegal), it will
(un)pin only those maps that have a pin_path set.
Signed-off-by: Toke Høiland-Jørgensen <redacted>
---Looks good, thanks! Just some minor things to fix up below. Acked-by: Andrii Nakryiko <redacted>
quoted hunk ↗ jump to hunk
tools/lib/bpf/libbpf.c | 164 +++++++++++++++++++++++++++++++++++----------- tools/lib/bpf/libbpf.h | 8 ++ tools/lib/bpf/libbpf.map | 3 + 3 files changed, 134 insertions(+), 41 deletions(-)diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index ce5ef3ddd263..fd11f6aeb32c 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c@@ -226,6 +226,8 @@ struct bpf_map { void *priv; bpf_map_clear_priv_t clear_priv; enum libbpf_map_type libbpf_type; + char *pin_path; + bool pinned; }; struct bpf_secdata {@@ -4025,47 +4027,119 @@ int bpf_map__pin(struct bpf_map *map, const char *path) char *cp, errmsg[STRERR_BUFSIZE]; int err; - err = check_path(path); - if (err) - return err; - if (map == NULL) { pr_warn("invalid map pointer\n"); return -EINVAL; } - if (bpf_obj_pin(map->fd, path)) { - cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg)); - pr_warn("failed to pin map: %s\n", cp); - return -errno; + if (map->pin_path) { + if (path && strcmp(path, map->pin_path)) { + pr_warn("map '%s' already has pin path '%s' different from '%s'\n", + bpf_map__name(map), map->pin_path, path); + return -EINVAL; + } else if (map->pinned) { + pr_debug("map '%s' already pinned at '%s'; not re-pinning\n", + bpf_map__name(map), map->pin_path); + return 0; + }
`if (map->pinned)` check is the same in both branches, so I'd do it first, before this map->pin_path if/else.
quoted hunk ↗ jump to hunk
+ } else { + if (!path) { + pr_warn("missing a path to pin map '%s' at\n", + bpf_map__name(map)); + return -EINVAL; + } else if (map->pinned) { + pr_warn("map '%s' already pinned\n", bpf_map__name(map)); + return -EEXIST; + } + + map->pin_path = strdup(path); + if (!map->pin_path) { + err = -errno; + goto out_err; + } }
[...]
quoted hunk ↗ jump to hunk
+ + err = check_path(path); + if (err) + return err; + err = unlink(path); if (err != 0) return -errno; - pr_debug("unpinned map '%s'\n", path); + + map->pinned = false; + pr_debug("unpinned map from '%s' from '%s'\n", bpf_map__name(map), path);
typo: extra from before map name?
return 0;
}[...]
quoted hunk ↗ jump to hunk
return err;@@ -4131,17 +4205,24 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path) return -ENOENT; bpf_object__for_each_map(map, obj) { + char *pin_path = NULL; char buf[PATH_MAX];
you can call buf as pin_path and get rid of extra pointer?
quoted hunk ↗ jump to hunk
- int len; - len = snprintf(buf, PATH_MAX, "%s/%s", path, - bpf_map__name(map)); - if (len < 0) - return -EINVAL; - else if (len >= PATH_MAX) - return -ENAMETOOLONG; + if (path) { + int len; + + len = snprintf(buf, PATH_MAX, "%s/%s", path, + bpf_map__name(map)); + if (len < 0) + return -EINVAL; + else if (len >= PATH_MAX) + return -ENAMETOOLONG; + pin_path = buf; + } else if (!map->pin_path) { + continue; + } - err = bpf_map__unpin(map, buf); + err = bpf_map__unpin(map, pin_path); if (err) return err; }
[...]
quoted hunk ↗ jump to hunk
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index d1473ea4d7a5..c24d4c01591d 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map@@ -197,4 +197,7 @@ LIBBPF_0.0.6 { bpf_object__open_mem; bpf_program__get_expected_attach_type; bpf_program__get_type; + bpf_map__get_pin_path; + bpf_map__set_pin_path; + bpf_map__is_pinned;
we try to keep this list alphabetically sorted
} LIBBPF_0.0.5;