From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751041AbdASGIw (ORCPT ); Thu, 19 Jan 2017 01:08:52 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:32285 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750765AbdASGIu (ORCPT ); Thu, 19 Jan 2017 01:08:50 -0500 Subject: Re: [PATCH perf/core 1/6] tools lib bpf: Fix map offsets in relocation To: Joe Stringer , References: <20170118235724.26103-1-joe@ovn.org> <20170118235724.26103-2-joe@ovn.org> CC: , , From: "Wangnan (F)" Message-ID: Date: Thu, 19 Jan 2017 14:07:40 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170118235724.26103-2-joe@ovn.org> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.194.139] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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);