Re: [PATCH 04/16] bpf tools: Collect map definition in bpf_object
From: Wangnan (F)
Date: Fri Nov 27 2015 - 01:17:12 EST
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/