Message ID | 20180721013933.2975241-4-kafai@fb.com |
---|---|
State | Changes Requested, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | Introduce BPF_ANNOTATE_KV_PAIR | expand |
On Fri, Jul 20, 2018 at 06:39:33PM -0700, Martin KaFai Lau wrote: > This patch introduces BPF_ANNOTATE_KV_PAIR to signal the > bpf loader about the btf key_type and value_type of a bpf map. > Please refer to the changes in test_btf_haskv.c for its usage. > Both iproute2 and libbpf loader will then have the same > convention to find out the map's btf_key_type_id and > btf_value_type_id from a map's name. > > Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf") > Suggested-by: Daniel Borkmann <daniel@iogearbox.net> > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > --- > tools/lib/bpf/btf.c | 8 +++ > tools/lib/bpf/btf.h | 1 + > tools/lib/bpf/libbpf.c | 71 +++++++++++--------- > tools/testing/selftests/bpf/bpf_helpers.h | 9 +++ > tools/testing/selftests/bpf/test_btf_haskv.c | 7 +- > 5 files changed, 59 insertions(+), 37 deletions(-) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index ce77b5b57912..748f0b11361d 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -288,6 +288,14 @@ __s32 btf__find_by_name(const struct btf *btf, const char *type_name) > return -ENOENT; > } > > +const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 id) > +{ > + if (!id || id > btf->nr_types) > + return ERR_PTR(-EINVAL); > + > + return btf->types[id]; > +} > + > void btf__free(struct btf *btf) > { > if (!btf) > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > index ed3a84370ccc..38ebf66613e4 100644 > --- a/tools/lib/bpf/btf.h > +++ b/tools/lib/bpf/btf.h > @@ -16,6 +16,7 @@ typedef int (*btf_print_fn_t)(const char *, ...) > void btf__free(struct btf *btf); > struct btf *btf__new(__u8 *data, __u32 size, btf_print_fn_t err_log); > __s32 btf__find_by_name(const struct btf *btf, const char *type_name); > +const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 id); > __s64 btf__resolve_size(const struct btf *btf, __u32 type_id); > int btf__fd(const struct btf *btf); > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 6deb4fe4fffe..5ff7755efa6b 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -36,6 +36,7 @@ > #include <linux/err.h> > #include <linux/kernel.h> > #include <linux/bpf.h> > +#include <linux/btf.h> > #include <linux/list.h> > #include <linux/limits.h> > #include <sys/stat.h> > @@ -1014,63 +1015,69 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr, > > static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) > { > + const struct btf_type *container_type; > + const struct btf_member *key, *value; > + __s32 key_id, value_id, container_id; > struct bpf_map_def *def = &map->def; > const size_t max_name = 256; > + char container_name[max_name]; > __s64 key_size, value_size; > - __s32 key_id, value_id; > - char name[max_name]; > > - /* Find key type by name from BTF */ > - if (snprintf(name, max_name, "%s_key", map->name) == max_name) { > - pr_warning("map:%s length of BTF key_type:%s_key is too long\n", > + if (snprintf(container_name, max_name, "____btf_map_%s", map->name) == > + max_name) { > + pr_warning("map:%s length of '____btf_map_%s' is too long\n", > map->name, map->name); > return -EINVAL; > } > > - key_id = btf__find_by_name(btf, name); > - if (key_id < 0) { > - pr_debug("map:%s key_type:%s cannot be found in BTF\n", > - map->name, name); > - return key_id; > + container_id = btf__find_by_name(btf, container_name); > + if (container_id < 0) { > + pr_debug("map:%s container_name:%s cannot be found in BTF. Missing BPF_ANNOTATE_KV_PAIR?\n", > + map->name, container_name); > + return container_id; > } > > - key_size = btf__resolve_size(btf, key_id); > - if (key_size < 0) { > - pr_warning("map:%s key_type:%s cannot get the BTF type_size\n", > - map->name, name); > - return key_size; > + container_type = btf__type_by_id(btf, container_id); > + if (IS_ERR(container_type)) { > + pr_warning("map:%s cannot find BTF type for container_id:%u\n", > + map->name, container_id); > + return PTR_ERR(container_type); > } > > - if (def->key_size != key_size) { > - pr_warning("map:%s key_type:%s has BTF type_size:%u != key_size:%u\n", > - map->name, name, (unsigned int)key_size, def->key_size); > + if (BTF_INFO_KIND(container_type->info) != BTF_KIND_STRUCT && > + BTF_INFO_VLEN(container_type->info) < 2) { There is a mistake in this check. I will spin v2. > + pr_warning("map:%s container_name:%s is an invalid container struct\n", > + map->name, container_name); > return -EINVAL; > } > > - /* Find value type from BTF */ > - if (snprintf(name, max_name, "%s_value", map->name) == max_name) { > - pr_warning("map:%s length of BTF value_type:%s_value is too long\n", > - map->name, map->name); > - return -EINVAL; > + key = (struct btf_member *)(container_type + 1); > + value = key + 1; > + key_id = key->type; > + value_id = value->type; > + > + key_size = btf__resolve_size(btf, key_id); > + if (key_size < 0) { > + pr_warning("map:%s invalid BTF key_type_size\n", > + map->name); > + return key_size; > } > > - value_id = btf__find_by_name(btf, name); > - if (value_id < 0) { > - pr_debug("map:%s value_type:%s cannot be found in BTF\n", > - map->name, name); > - return value_id; > + if (def->key_size != key_size) { > + pr_warning("map:%s btf_key_type_size:%u != map_def_key_size:%u\n", > + map->name, (__u32)key_size, def->key_size); > + return -EINVAL; > } > > value_size = btf__resolve_size(btf, value_id); > if (value_size < 0) { > - pr_warning("map:%s value_type:%s cannot get the BTF type_size\n", > - map->name, name); > + pr_warning("map:%s invalid BTF value_type_size\n", map->name); > return value_size; > } > > if (def->value_size != value_size) { > - pr_warning("map:%s value_type:%s has BTF type_size:%u != value_size:%u\n", > - map->name, name, (unsigned int)value_size, def->value_size); > + pr_warning("map:%s btf_value_type_size:%u != map_def_value_size:%u\n", > + map->name, (__u32)value_size, def->value_size); > return -EINVAL; > } > > diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h > index f2f28b6c8915..0ce47b46df70 100644 > --- a/tools/testing/selftests/bpf/bpf_helpers.h > +++ b/tools/testing/selftests/bpf/bpf_helpers.h > @@ -158,6 +158,15 @@ struct bpf_map_def { > unsigned int numa_node; > }; > > +#define BPF_ANNOTATE_KV_PAIR(name, type_key, type_val) \ > + struct ____btf_map_##name { \ > + type_key key; \ > + type_val value; \ > + }; \ > + struct ____btf_map_##name \ > + __attribute__ ((section(".maps." #name), used)) \ > + ____btf_map_##name = { } > + > static int (*bpf_skb_load_bytes)(void *ctx, int off, void *to, int len) = > (void *) BPF_FUNC_skb_load_bytes; > static int (*bpf_skb_store_bytes)(void *ctx, int off, void *from, int len, int flags) = > diff --git a/tools/testing/selftests/bpf/test_btf_haskv.c b/tools/testing/selftests/bpf/test_btf_haskv.c > index 8c7ca096ecf2..b21b876f475d 100644 > --- a/tools/testing/selftests/bpf/test_btf_haskv.c > +++ b/tools/testing/selftests/bpf/test_btf_haskv.c > @@ -10,11 +10,6 @@ struct ipv_counts { > unsigned int v6; > }; > > -typedef int btf_map_key; > -typedef struct ipv_counts btf_map_value; > -btf_map_key dumm_key; > -btf_map_value dummy_value; > - > struct bpf_map_def SEC("maps") btf_map = { > .type = BPF_MAP_TYPE_ARRAY, > .key_size = sizeof(int), > @@ -22,6 +17,8 @@ struct bpf_map_def SEC("maps") btf_map = { > .max_entries = 4, > }; > > +BPF_ANNOTATE_KV_PAIR(btf_map, int, struct ipv_counts); > + > struct dummy_tracepoint_args { > unsigned long long pad; > struct sock *sock; > -- > 2.17.1 >
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c index ce77b5b57912..748f0b11361d 100644 --- a/tools/lib/bpf/btf.c +++ b/tools/lib/bpf/btf.c @@ -288,6 +288,14 @@ __s32 btf__find_by_name(const struct btf *btf, const char *type_name) return -ENOENT; } +const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 id) +{ + if (!id || id > btf->nr_types) + return ERR_PTR(-EINVAL); + + return btf->types[id]; +} + void btf__free(struct btf *btf) { if (!btf) diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h index ed3a84370ccc..38ebf66613e4 100644 --- a/tools/lib/bpf/btf.h +++ b/tools/lib/bpf/btf.h @@ -16,6 +16,7 @@ typedef int (*btf_print_fn_t)(const char *, ...) void btf__free(struct btf *btf); struct btf *btf__new(__u8 *data, __u32 size, btf_print_fn_t err_log); __s32 btf__find_by_name(const struct btf *btf, const char *type_name); +const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 id); __s64 btf__resolve_size(const struct btf *btf, __u32 type_id); int btf__fd(const struct btf *btf); diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 6deb4fe4fffe..5ff7755efa6b 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -36,6 +36,7 @@ #include <linux/err.h> #include <linux/kernel.h> #include <linux/bpf.h> +#include <linux/btf.h> #include <linux/list.h> #include <linux/limits.h> #include <sys/stat.h> @@ -1014,63 +1015,69 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr, static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) { + const struct btf_type *container_type; + const struct btf_member *key, *value; + __s32 key_id, value_id, container_id; struct bpf_map_def *def = &map->def; const size_t max_name = 256; + char container_name[max_name]; __s64 key_size, value_size; - __s32 key_id, value_id; - char name[max_name]; - /* Find key type by name from BTF */ - if (snprintf(name, max_name, "%s_key", map->name) == max_name) { - pr_warning("map:%s length of BTF key_type:%s_key is too long\n", + if (snprintf(container_name, max_name, "____btf_map_%s", map->name) == + max_name) { + pr_warning("map:%s length of '____btf_map_%s' is too long\n", map->name, map->name); return -EINVAL; } - key_id = btf__find_by_name(btf, name); - if (key_id < 0) { - pr_debug("map:%s key_type:%s cannot be found in BTF\n", - map->name, name); - return key_id; + container_id = btf__find_by_name(btf, container_name); + if (container_id < 0) { + pr_debug("map:%s container_name:%s cannot be found in BTF. Missing BPF_ANNOTATE_KV_PAIR?\n", + map->name, container_name); + return container_id; } - key_size = btf__resolve_size(btf, key_id); - if (key_size < 0) { - pr_warning("map:%s key_type:%s cannot get the BTF type_size\n", - map->name, name); - return key_size; + container_type = btf__type_by_id(btf, container_id); + if (IS_ERR(container_type)) { + pr_warning("map:%s cannot find BTF type for container_id:%u\n", + map->name, container_id); + return PTR_ERR(container_type); } - if (def->key_size != key_size) { - pr_warning("map:%s key_type:%s has BTF type_size:%u != key_size:%u\n", - map->name, name, (unsigned int)key_size, def->key_size); + if (BTF_INFO_KIND(container_type->info) != BTF_KIND_STRUCT && + BTF_INFO_VLEN(container_type->info) < 2) { + pr_warning("map:%s container_name:%s is an invalid container struct\n", + map->name, container_name); return -EINVAL; } - /* Find value type from BTF */ - if (snprintf(name, max_name, "%s_value", map->name) == max_name) { - pr_warning("map:%s length of BTF value_type:%s_value is too long\n", - map->name, map->name); - return -EINVAL; + key = (struct btf_member *)(container_type + 1); + value = key + 1; + key_id = key->type; + value_id = value->type; + + key_size = btf__resolve_size(btf, key_id); + if (key_size < 0) { + pr_warning("map:%s invalid BTF key_type_size\n", + map->name); + return key_size; } - value_id = btf__find_by_name(btf, name); - if (value_id < 0) { - pr_debug("map:%s value_type:%s cannot be found in BTF\n", - map->name, name); - return value_id; + if (def->key_size != key_size) { + pr_warning("map:%s btf_key_type_size:%u != map_def_key_size:%u\n", + map->name, (__u32)key_size, def->key_size); + return -EINVAL; } value_size = btf__resolve_size(btf, value_id); if (value_size < 0) { - pr_warning("map:%s value_type:%s cannot get the BTF type_size\n", - map->name, name); + pr_warning("map:%s invalid BTF value_type_size\n", map->name); return value_size; } if (def->value_size != value_size) { - pr_warning("map:%s value_type:%s has BTF type_size:%u != value_size:%u\n", - map->name, name, (unsigned int)value_size, def->value_size); + pr_warning("map:%s btf_value_type_size:%u != map_def_value_size:%u\n", + map->name, (__u32)value_size, def->value_size); return -EINVAL; } diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h index f2f28b6c8915..0ce47b46df70 100644 --- a/tools/testing/selftests/bpf/bpf_helpers.h +++ b/tools/testing/selftests/bpf/bpf_helpers.h @@ -158,6 +158,15 @@ struct bpf_map_def { unsigned int numa_node; }; +#define BPF_ANNOTATE_KV_PAIR(name, type_key, type_val) \ + struct ____btf_map_##name { \ + type_key key; \ + type_val value; \ + }; \ + struct ____btf_map_##name \ + __attribute__ ((section(".maps." #name), used)) \ + ____btf_map_##name = { } + static int (*bpf_skb_load_bytes)(void *ctx, int off, void *to, int len) = (void *) BPF_FUNC_skb_load_bytes; static int (*bpf_skb_store_bytes)(void *ctx, int off, void *from, int len, int flags) = diff --git a/tools/testing/selftests/bpf/test_btf_haskv.c b/tools/testing/selftests/bpf/test_btf_haskv.c index 8c7ca096ecf2..b21b876f475d 100644 --- a/tools/testing/selftests/bpf/test_btf_haskv.c +++ b/tools/testing/selftests/bpf/test_btf_haskv.c @@ -10,11 +10,6 @@ struct ipv_counts { unsigned int v6; }; -typedef int btf_map_key; -typedef struct ipv_counts btf_map_value; -btf_map_key dumm_key; -btf_map_value dummy_value; - struct bpf_map_def SEC("maps") btf_map = { .type = BPF_MAP_TYPE_ARRAY, .key_size = sizeof(int), @@ -22,6 +17,8 @@ struct bpf_map_def SEC("maps") btf_map = { .max_entries = 4, }; +BPF_ANNOTATE_KV_PAIR(btf_map, int, struct ipv_counts); + struct dummy_tracepoint_args { unsigned long long pad; struct sock *sock;
This patch introduces BPF_ANNOTATE_KV_PAIR to signal the bpf loader about the btf key_type and value_type of a bpf map. Please refer to the changes in test_btf_haskv.c for its usage. Both iproute2 and libbpf loader will then have the same convention to find out the map's btf_key_type_id and btf_value_type_id from a map's name. Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf") Suggested-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- tools/lib/bpf/btf.c | 8 +++ tools/lib/bpf/btf.h | 1 + tools/lib/bpf/libbpf.c | 71 +++++++++++--------- tools/testing/selftests/bpf/bpf_helpers.h | 9 +++ tools/testing/selftests/bpf/test_btf_haskv.c | 7 +- 5 files changed, 59 insertions(+), 37 deletions(-)