Re: [PATCH perf/core 1/6] tools lib bpf: Fix map offsets in relocation

[Wangnan (F)]

On 2017/1/19 7:57, Joe Stringer wrote:

Commit 4708bbda5cb2 ("tools lib bpf: Fix maps resolution") attempted to
fix map resolution by identifying the number of symbols that point to
maps, and using this number to resolve each of the maps.

However, during relocation the original definition of the map size was
still in use.

Oops... Didn't realize that.

  For up to two maps, the calculation was correct if there
was a small difference in size between the map definition in libbpf and
the one that the client library uses. However if the difference was
large, particularly if more than two maps were used in the BPF program,
the relocation would fail.

For example, when using a map definition with size 28, with three maps,
map relocation would count
     (sym_offset / sizeof(struct bpf_map_def) => map_idx)
     (0 / 16 => 0), ie map_idx = 0
     (28 / 16 => 1), ie map_idx = 1
     (56 / 16 => 3), ie map_idx = 3

So, libbpf reports:
     libbpf: bpf relocation: map_idx 3 large than 2

Understand.

Fix map relocation by tracking the size of the map definition during
map counting, then reuse this instead of the size of the libbpf map
structure.

Then we add a restriction that map size is fixed
in one object, but we don't have to. During relocating,
we can get exact map positioning information from
sym.st_value. I think finding a map with exact offset
from the map array would be better.

I'll write a patch to replace this one.

Thank you.

  With this patch applied, libbpf will accept such ELFs.

Fixes: 4708bbda5cb2 ("tools lib bpf: Fix maps resolution")
Signed-off-by: Joe Stringer <j...@ovn.org>
---
  tools/lib/bpf/libbpf.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 84e6b35da4bd..350ee4c59f85 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -201,6 +201,7 @@ struct bpf_object {
        size_t nr_programs;
        struct bpf_map *maps;
        size_t nr_maps;
+       size_t map_len;

bool loaded; @@ -379,6 +380,7 @@ static struct bpf_object *bpf_object__new(const char *path,

        obj->efile.maps_shndx = -1;

obj->loaded = false;

+       obj->map_len = sizeof(struct bpf_map_def);

INIT_LIST_HEAD(&obj->list);

        list_add(&obj->list, &bpf_objects_list);
@@ -602,6 +604,7 @@ bpf_object__init_maps(struct bpf_object *obj)
                return -ENOMEM;
        }
        obj->nr_maps = nr_maps;
+       obj->map_len = data->d_size / nr_maps;

/*

         * fill all fd with -1 so won't close incorrect
@@ -829,7 +832,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
                        return -LIBBPF_ERRNO__RELOC;
                }

- map_idx = sym.st_value / sizeof(struct bpf_map_def);

+               map_idx = sym.st_value / prog->obj->map_len;
                if (map_idx >= nr_maps) {
                        pr_warning("bpf relocation: map_idx %d large than %d\n",
                                   (int)map_idx, (int)nr_maps - 1);