lkml.org 
[lkml]   [2015]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 04/16] bpf tools: Collect map definition in bpf_object


    On 2015/11/27 4:56, Arnaldo Carvalho de Melo wrote:
    > Em Tue, Nov 24, 2015 at 01:36:09PM +0000, Wang Nan escreveu:

    [SNIP]

    >> + }
    >> return 0;
    >> }
    >>
    >> @@ -688,37 +707,15 @@ static int
    >> bpf_object__create_maps(struct bpf_object *obj)
    >> {
    >> unsigned int i;
    >> - size_t nr_maps;
    >> - int *pfd;
    >> -
    >> - nr_maps = obj->maps_buf_sz / sizeof(struct bpf_map_def);
    >> - if (!obj->maps_buf || !nr_maps) {
    >> - pr_debug("don't need create maps for %s\n",
    >> - obj->path);
    >> - return 0;
    >> - }
    >> -
    >> - obj->map_fds = malloc(sizeof(int) * nr_maps);
    > perhaps calloc?

    This line is being removed, so you'll still see this in my next version...

    >
    >> - if (!obj->map_fds) {
    >> - pr_warning("realloc perf_bpf_map_fds failed\n");
    >> - return -ENOMEM;
    >> - }
    >> - obj->nr_map_fds = nr_maps;
    >>
    >> - /* fill all fd with -1 */
    >> - memset(obj->map_fds, -1, sizeof(int) * nr_maps);
    >> + for (i = 0; i < obj->nr_maps; i++) {
    >> + struct bpf_map_def *def = &obj->maps[i].def;
    >> + int *pfd = &obj->maps[i].fd;
    >>
    >> - pfd = obj->map_fds;
    >> - for (i = 0; i < nr_maps; i++) {
    >> - struct bpf_map_def def;
    >> -
    >> - def = *(struct bpf_map_def *)(obj->maps_buf +
    >> - i * sizeof(struct bpf_map_def));
    >> -
    >> - *pfd = bpf_create_map(def.type,
    >> - def.key_size,
    >> - def.value_size,
    >> - def.max_entries);
    >> + *pfd = bpf_create_map(def->type,
    >> + def->key_size,
    >> + def->value_size,
    >> + def->max_entries);
    >> if (*pfd < 0) {
    >> size_t j;
    >> int err = *pfd;
    >> @@ -726,22 +723,17 @@ bpf_object__create_maps(struct bpf_object *obj)
    >> pr_warning("failed to create map: %s\n",
    >> strerror(errno));
    >> for (j = 0; j < i; j++)
    >> - zclose(obj->map_fds[j]);
    >> - obj->nr_map_fds = 0;
    >> - zfree(&obj->map_fds);
    >> + zclose(obj->maps[j].fd);
    >> return err;
    >> }
    >> pr_debug("create map: fd=%d\n", *pfd);
    >> - pfd++;
    >> }
    >>
    >> - zfree(&obj->maps_buf);
    >> - obj->maps_buf_sz = 0;
    >> return 0;
    >> }
    >>
    >> static int
    >> -bpf_program__relocate(struct bpf_program *prog, int *map_fds)
    >> +bpf_program__relocate(struct bpf_program *prog, struct bpf_object *obj)
    >> {
    >> int i;
    >>
    >> @@ -761,7 +753,7 @@ bpf_program__relocate(struct bpf_program *prog, int *map_fds)
    >> return -LIBBPF_ERRNO__RELOC;
    >> }
    >> insns[insn_idx].src_reg = BPF_PSEUDO_MAP_FD;
    >> - insns[insn_idx].imm = map_fds[map_idx];
    >> + insns[insn_idx].imm = obj->maps[map_idx].fd;
    >> }
    >>
    >> zfree(&prog->reloc_desc);
    >> @@ -780,7 +772,7 @@ bpf_object__relocate(struct bpf_object *obj)
    >> for (i = 0; i < obj->nr_programs; i++) {
    >> prog = &obj->programs[i];
    >>
    >> - err = bpf_program__relocate(prog, obj->map_fds);
    >> + err = bpf_program__relocate(prog, obj);
    >> if (err) {
    >> pr_warning("failed to relocate '%s'\n",
    >> prog->section_name);
    >> @@ -804,8 +796,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
    >> Elf_Data *data = obj->efile.reloc[i].data;
    >> int idx = shdr->sh_info;
    >> struct bpf_program *prog;
    >> - size_t nr_maps = obj->maps_buf_sz /
    >> - sizeof(struct bpf_map_def);
    >> + size_t nr_maps = obj->nr_maps;
    >>
    >> if (shdr->sh_type != SHT_REL) {
    >> pr_warning("internal error at %d\n", __LINE__);
    >> @@ -1050,10 +1041,8 @@ int bpf_object__unload(struct bpf_object *obj)
    >> if (!obj)
    >> return -EINVAL;
    >>
    >> - for (i = 0; i < obj->nr_map_fds; i++)
    >> - zclose(obj->map_fds[i]);
    >> - zfree(&obj->map_fds);
    >> - obj->nr_map_fds = 0;
    >> + for (i = 0; i < obj->nr_maps; i++)
    >> + zclose(obj->maps[i].fd);
    >>
    >> for (i = 0; i < obj->nr_programs; i++)
    >> bpf_program__unload(&obj->programs[i]);
    >> @@ -1096,7 +1085,15 @@ void bpf_object__close(struct bpf_object *obj)
    >> bpf_object__elf_finish(obj);
    >> bpf_object__unload(obj);
    >>
    >> - zfree(&obj->maps_buf);
    >> + for (i = 0; i < obj->nr_maps; i++) {
    >> + if (obj->maps[i].clear_priv)
    >> + obj->maps[i].clear_priv(&obj->maps[i],
    >> + obj->maps[i].priv);
    >> + obj->maps[i].priv = NULL;
    >> + obj->maps[i].clear_priv = NULL;
    >> + }
    >> + zfree(&obj->maps);
    >> + obj->nr_maps = 0;
    >>
    >> if (obj->programs && obj->nr_programs) {
    >> for (i = 0; i < obj->nr_programs; i++)
    >> @@ -1251,3 +1248,72 @@ int bpf_program__nth_fd(struct bpf_program *prog, int n)
    >>
    >> return fd;
    >> }
    >> +
    >> +int bpf_map__get_fd(struct bpf_map *map)
    >> +{
    >> + if (!map)
    >> + return -EINVAL;
    >> +
    >> + return map->fd;
    >> +}
    >> +
    >> +int bpf_map__get_def(struct bpf_map *map, struct bpf_map_def *pdef)
    >> +{
    >> + if (!map || !pdef)
    >> + return -EINVAL;
    >> +
    >> + *pdef = map->def;
    >> + return 0;
    >> +}
    >> +
    >> +int bpf_map__set_private(struct bpf_map *map, void *priv,
    >> + bpf_map_clear_priv_t clear_priv)
    >> +{
    >> + if (!map)
    >> + return -EINVAL;
    >> +
    >> + if (map->priv) {
    >> + if (map->clear_priv)
    >> + map->clear_priv(map, map->priv);
    >> + }
    >> +
    >> + map->priv = priv;
    >> + map->clear_priv = clear_priv;
    >> + return 0;
    >> +}
    >> +
    >> +int bpf_map__get_private(struct bpf_map *map, void **ppriv)
    >> +{
    >> + if (!map)
    >> + return -EINVAL;
    >> +
    >> + if (ppriv)
    >> + *ppriv = map->priv;
    >> + return 0;
    >> +}
    >> +
    >> +struct bpf_map *
    >> +bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
    >> +{
    >> + size_t idx;
    >> + struct bpf_map *s, *e;
    >> +
    >> + if (!obj || !obj->maps)
    >> + return NULL;
    >> +
    >> + s = obj->maps;
    >> + e = obj->maps + obj->nr_maps;
    >> +
    >> + if (prev == NULL)
    >> + return s;
    >> +
    >> + if ((prev < s) || (prev >= e)) {
    >> + pr_warning("error: map handler doesn't belong to object\n");
    > I wonder if this shouldn't be made pr_debug, and as well have a function
    > prefix, otherwise we may think this is related to some other kind of
    > map, so I suggest:
    > pr_debug("%s: error: map handler doesn't belong to object\n", __func__);
    >
    > Or at least:
    >
    > pr_debug("BPF error: map handler doesn't belong to object\n");

    We always have a libbpf prefix before debug info from libbpf. Please see

    static __printf(1, 2) libbpf_print_fn_t __pr_warning = __base_pr;
    static __printf(1, 2) libbpf_print_fn_t __pr_info = __base_pr;
    static __printf(1, 2) libbpf_print_fn_t __pr_debug;
    #define __pr(func, fmt, ...) \
    do { \
    if ((func)) \
    (func)("libbpf: " fmt, ##__VA_ARGS__); \
    } while (0)

    #define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__)
    #define pr_info(fmt, ...) __pr(__pr_info, fmt, ##__VA_ARGS__)
    #define pr_debug(fmt, ...) __pr(__pr_debug, fmt, ##__VA_ARGS__)

    We allow __pr_{warning,info,debug} be overwritten but the 'libbpf:'
    prefix is
    always there.

    Also, don't forget in perf's context libbpf is muted.

    >> + return NULL;
    >> + }
    >> +
    >> + idx = (prev - obj->maps) + 1;
    >> + if (idx >= obj->nr_maps)
    >> + return NULL;
    >> + return &obj->maps[idx];
    >> +}
    >> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
    >> index 949df4b..709d2fa 100644
    >> --- a/tools/lib/bpf/libbpf.h
    >> +++ b/tools/lib/bpf/libbpf.h
    >> @@ -165,4 +165,25 @@ struct bpf_map_def {
    >> unsigned int max_entries;
    >> };
    >>
    >> +/*
    >> + * There is another 'struct bpf_map' in include/linux/map.h. However,
    >> + * it is not a uapi header so no need to consider name confliction.
    > s/confliction/conflict/g
    >
    > But I would use "name clash".

    Good word.

    Thank you.


    \
     
     \ /
      Last update: 2015-11-27 08:01    [W:4.232 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site