[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzZR+7R9TjXSMHiYzbHCvuwL+Z_LBD=Pk+rRW+X+pgdg4w@mail.gmail.com>
Date: Wed, 27 Feb 2019 15:47:56 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
bpf@...r.kernel.org, oss-drivers@...ronome.com
Subject: Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define
for map iteration
On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
<jakub.kicinski@...ronome.com> wrote:
>
> For historical reasons the helper to loop over maps in an object
> is called bpf_map__for_each while it really should be called
> bpf_object__for_each_map. Rename and add a correctly named
> define for backward compatibility.
Seems like there are at least 3 more functions that are not named correctly:
- __bpf_map__iter (__bpf_object__iter_map?)
- bpf_map__next (=> bpf_object__next_map?)
- bpf_map__prev (=> bpf_object__prev_map?)
Let's rename them as well?
>
> Switch all in-tree users to the correct name (Quentin).
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@...ronome.com>
> ---
> tools/bpf/bpftool/prog.c | 4 ++--
> tools/lib/bpf/libbpf.c | 8 ++++----
> tools/lib/bpf/libbpf.h | 3 ++-
> tools/perf/util/bpf-loader.c | 4 ++--
> tools/testing/selftests/bpf/test_libbpf_open.c | 2 +-
> 5 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 0c35dd543d49..8ef80d65a474 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1053,7 +1053,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> j = 0;
> while (j < old_map_fds && map_replace[j].name) {
> i = 0;
> - bpf_map__for_each(map, obj) {
> + bpf_object__for_each_map(map, obj) {
> if (!strcmp(bpf_map__name(map), map_replace[j].name)) {
> map_replace[j].idx = i;
> break;
> @@ -1074,7 +1074,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> /* Set ifindex and name reuse */
> j = 0;
> idx = 0;
> - bpf_map__for_each(map, obj) {
> + bpf_object__for_each_map(map, obj) {
> if (!bpf_map__is_offload_neutral(map))
> bpf_map__set_ifindex(map, ifindex);
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b38dcbe7460a..f5eb60379c8d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2100,7 +2100,7 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
> if (err)
> return err;
>
> - bpf_map__for_each(map, obj) {
> + bpf_object__for_each_map(map, obj) {
> char buf[PATH_MAX];
> int len;
>
> @@ -2147,7 +2147,7 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
> if (!obj)
> return -ENOENT;
>
> - bpf_map__for_each(map, obj) {
> + bpf_object__for_each_map(map, obj) {
> char buf[PATH_MAX];
> int len;
>
> @@ -2835,7 +2835,7 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
> {
> struct bpf_map *pos;
>
> - bpf_map__for_each(pos, obj) {
> + bpf_object__for_each_map(pos, obj) {
> if (pos->name && !strcmp(pos->name, name))
> return pos;
> }
> @@ -2928,7 +2928,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> first_prog = prog;
> }
>
> - bpf_map__for_each(map, obj) {
> + bpf_object__for_each_map(map, obj) {
> if (!bpf_map__is_offload_neutral(map))
> map->map_ifindex = attr->ifindex;
> }
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 6c0168f8bba5..b4652aa1a58a 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
>
> LIBBPF_API struct bpf_map *
> bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
> -#define bpf_map__for_each(pos, obj) \
> +#define bpf_object__for_each_map(pos, obj) \
> for ((pos) = bpf_map__next(NULL, (obj)); \
> (pos) != NULL; \
> (pos) = bpf_map__next((pos), (obj)))
> +#define bpf_map__for_each bpf_object__for_each_map
Should we get rid of this as well, instead of accumulating cruft?
>
> LIBBPF_API struct bpf_map *
> bpf_map__prev(struct bpf_map *map, struct bpf_object *obj);
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 037d8ff6a634..31b7e5a1453b 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -1489,7 +1489,7 @@ apply_obj_config_object(struct bpf_object *obj)
> struct bpf_map *map;
> int err;
>
> - bpf_map__for_each(map, obj) {
> + bpf_object__for_each_map(map, obj) {
> err = apply_obj_config_map(map);
> if (err)
> return err;
> @@ -1513,7 +1513,7 @@ int bpf__apply_obj_config(void)
>
> #define bpf__for_each_map(pos, obj, objtmp) \
> bpf_object__for_each_safe(obj, objtmp) \
> - bpf_map__for_each(pos, obj)
> + bpf_object__for_each_map(pos, obj)
>
> #define bpf__for_each_map_named(pos, obj, objtmp, name) \
> bpf__for_each_map(pos, obj, objtmp) \
> diff --git a/tools/testing/selftests/bpf/test_libbpf_open.c b/tools/testing/selftests/bpf/test_libbpf_open.c
> index 1909ecf4d999..65cbd30704b5 100644
> --- a/tools/testing/selftests/bpf/test_libbpf_open.c
> +++ b/tools/testing/selftests/bpf/test_libbpf_open.c
> @@ -67,7 +67,7 @@ int test_walk_maps(struct bpf_object *obj, bool verbose)
> struct bpf_map *map;
> int cnt = 0;
>
> - bpf_map__for_each(map, obj) {
> + bpf_object__for_each_map(map, obj) {
> cnt++;
> if (verbose)
> printf("Map (count:%d) name: %s\n", cnt,
> --
> 2.19.2
>
Powered by blists - more mailing lists