diff mbox series

[bpf,3/3] bpf: Introduce BPF_ANNOTATE_KV_PAIR

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

Commit Message

Martin KaFai Lau July 21, 2018, 1:39 a.m. UTC
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(-)

Comments

Martin KaFai Lau July 21, 2018, 5:06 p.m. UTC | #1
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 mbox series

Patch

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;