lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 27 Nov 2015 14:16:36 +0800 From: "Wangnan (F)" <wangnan0@...wei.com> To: Arnaldo Carvalho de Melo <acme@...nel.org> CC: <masami.hiramatsu.pt@...achi.com>, <ast@...nel.org>, <lizefan@...wei.com>, <pi3orama@....com>, <linux-kernel@...r.kernel.org>, Namhyung Kim <namhyung@...nel.org> Subject: Re: [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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists