[PATCH net-next v3 0/2] libbpf: support more map options

[PATCH net-next v3 0/2] libbpf: support more map options

[Craig Gallek]

From: Craig Gallek <kraig@google.com>

The functional change to this series is the ability to use flags when
creating maps from object files loaded by libbpf.  In order to do this,
the first patch updates the library to handle map definitions that
differ in size from libbpf's struct bpf_map_def.

For object files with a larger map definition, libbpf will continue to load
if the unknown fields are all zero, otherwise the map is rejected.  If the
map definition in the object file is smaller than expected, libbpf will use
zero as a default value in the missing fields.

Craig Gallek (2):
  libbpf: parse maps sections of varying size
  libbpf: use map_flags when creating maps

 tools/lib/bpf/libbpf.c | 72 +++++++++++++++++++++++++++++---------------------
 tools/lib/bpf/libbpf.h |  1 +
 2 files changed, 43 insertions(+), 30 deletions(-)

-- 
v3:
  - explicit memcpy instead of struct assignment.
  - remove unnecessary bpf_object__validate_maps function

v2
  - determine bpf_map_def structure size dynamically from object file

2.14.2.920.gcf0c67979c-goog

[PATCH net-next v3 1/2] libbpf: parse maps sections of varying size

[Craig Gallek]

From: Craig Gallek <kraig@google.com>

This library previously assumed a fixed-size map options structure.
Any new options were ignored.  In order to allow the options structure
to grow and to support parsing older programs, this patch updates
the maps section parsing to handle varying sizes.

Object files with maps sections smaller than expected will have the new
fields initialized to zero.  Object files which have larger than expected
maps sections will be rejected unless all of the unrecognized data is zero.

This change still assumes that each map definition in the maps section
is the same size.

Signed-off-by: Craig Gallek <kraig@google.com>
---
 tools/lib/bpf/libbpf.c | 70 +++++++++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 29 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 4f402dcdf372..23152890ec60 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -579,31 +579,6 @@ bpf_object__init_kversion(struct bpf_object *obj,
 	return 0;
 }
 
-static int
-bpf_object__validate_maps(struct bpf_object *obj)
-{
-	int i;
-
-	/*
-	 * If there's only 1 map, the only error case should have been
-	 * catched in bpf_object__init_maps().
-	 */
-	if (!obj->maps || !obj->nr_maps || (obj->nr_maps == 1))
-		return 0;
-
-	for (i = 1; i < obj->nr_maps; i++) {
-		const struct bpf_map *a = &obj->maps[i - 1];
-		const struct bpf_map *b = &obj->maps[i];
-
-		if (b->offset - a->offset < sizeof(struct bpf_map_def)) {
-			pr_warning("corrupted map section in %s: map \"%s\" too small\n",
-				   obj->path, a->name);
-			return -EINVAL;
-		}
-	}
-	return 0;
-}
-
 static int compare_bpf_map(const void *_a, const void *_b)
 {
 	const struct bpf_map *a = _a;
@@ -615,7 +590,7 @@ static int compare_bpf_map(const void *_a, const void *_b)
 static int
 bpf_object__init_maps(struct bpf_object *obj)
 {
-	int i, map_idx, nr_maps = 0;
+	int i, map_idx, map_def_sz, nr_maps = 0;
 	Elf_Scn *scn;
 	Elf_Data *data;
 	Elf_Data *symbols = obj->efile.symbols;
@@ -658,6 +633,15 @@ bpf_object__init_maps(struct bpf_object *obj)
 	if (!nr_maps)
 		return 0;
 
+	/* Assume equally sized map definitions */
+	map_def_sz = data->d_size / nr_maps;
+	if (!data->d_size || (data->d_size % nr_maps) != 0) {
+		pr_warning("unable to determine map definition size "
+			   "section %s, %d maps in %zd bytes\n",
+			   obj->path, nr_maps, data->d_size);
+		return -EINVAL;
+	}
+
 	obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
 	if (!obj->maps) {
 		pr_warning("alloc maps for object failed\n");
@@ -690,7 +674,7 @@ bpf_object__init_maps(struct bpf_object *obj)
 				      obj->efile.strtabidx,
 				      sym.st_name);
 		obj->maps[map_idx].offset = sym.st_value;
-		if (sym.st_value + sizeof(struct bpf_map_def) > data->d_size) {
+		if (sym.st_value + map_def_sz > data->d_size) {
 			pr_warning("corrupted maps section in %s: last map \"%s\" too small\n",
 				   obj->path, map_name);
 			return -EINVAL;
@@ -704,12 +688,40 @@ bpf_object__init_maps(struct bpf_object *obj)
 		pr_debug("map %d is \"%s\"\n", map_idx,
 			 obj->maps[map_idx].name);
 		def = (struct bpf_map_def *)(data->d_buf + sym.st_value);
-		obj->maps[map_idx].def = *def;
+		/*
+		 * If the definition of the map in the object file fits in
+		 * bpf_map_def, copy it.  Any extra fields in our version
+		 * of bpf_map_def will default to zero as a result of the
+		 * calloc above.
+		 */
+		if (map_def_sz <= sizeof(struct bpf_map_def)) {
+			memcpy(&obj->maps[map_idx].def, def, map_def_sz);
+		} else {
+			/*
+			 * Here the map structure being read is bigger than what
+			 * we expect, truncate if the excess bits are all zero.
+			 * If they are not zero, reject this map as
+			 * incompatible.
+			 */
+			char *b;
+			for (b = ((char *)def) + sizeof(struct bpf_map_def);
+			     b < ((char *)def) + map_def_sz; b++) {
+				if (*b != 0) {
+					pr_warning("maps section in %s: \"%s\" "
+						   "has unrecognized, non-zero "
+						   "options\n",
+						   obj->path, map_name);
+					return -EINVAL;
+				}
+			}
+			memcpy(&obj->maps[map_idx].def, def,
+			       sizeof(struct bpf_map_def));
+		}
 		map_idx++;
 	}
 
 	qsort(obj->maps, obj->nr_maps, sizeof(obj->maps[0]), compare_bpf_map);
-	return bpf_object__validate_maps(obj);
+	return 0;
 }
 
 static int bpf_object__elf_collect(struct bpf_object *obj)
-- 
2.14.2.920.gcf0c67979c-goog

Re: [PATCH net-next v3 1/2] libbpf: parse maps sections of varying size

[Jesper Dangaard Brouer]

On Thu,  5 Oct 2017 10:41:57 -0400 Craig Gallek <kraigatgoog@gmail.com> wrote:

> From: Craig Gallek <kraig@google.com>
> 
> This library previously assumed a fixed-size map options structure.
> Any new options were ignored.  In order to allow the options structure
> to grow and to support parsing older programs, this patch updates
> the maps section parsing to handle varying sizes.
> 
> Object files with maps sections smaller than expected will have the new
> fields initialized to zero.  Object files which have larger than expected
> maps sections will be rejected unless all of the unrecognized data is zero.
> 
> This change still assumes that each map definition in the maps section
> is the same size.
> 
> Signed-off-by: Craig Gallek <kraig@google.com>
> ---
>  tools/lib/bpf/libbpf.c | 70 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 41 insertions(+), 29 deletions(-)

Thank you for working on this! :-)

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next v3 1/2] libbpf: parse maps sections of varying size

[Daniel Borkmann]

On 10/05/2017 04:41 PM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> This library previously assumed a fixed-size map options structure.
> Any new options were ignored.  In order to allow the options structure
> to grow and to support parsing older programs, this patch updates
> the maps section parsing to handle varying sizes.
>
> Object files with maps sections smaller than expected will have the new
> fields initialized to zero.  Object files which have larger than expected
> maps sections will be rejected unless all of the unrecognized data is zero.
>
> This change still assumes that each map definition in the maps section
> is the same size.
>
> Signed-off-by: Craig Gallek <kraig@google.com>

Thanks,

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

[PATCH net-next v3 2/2] libbpf: use map_flags when creating maps

[Craig Gallek]

From: Craig Gallek <kraig@google.com>

This is required to use BPF_MAP_TYPE_LPM_TRIE or any other map type
which requires flags.

Signed-off-by: Craig Gallek <kraig@google.com>
---
 tools/lib/bpf/libbpf.c | 2 +-
 tools/lib/bpf/libbpf.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 23152890ec60..5aa45f89da93 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -942,7 +942,7 @@ bpf_object__create_maps(struct bpf_object *obj)
 					   def->key_size,
 					   def->value_size,
 					   def->max_entries,
-					   0);
+					   def->map_flags);
 		if (*pfd < 0) {
 			size_t j;
 			int err = *pfd;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 7959086eb9c9..6e20003109e0 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -207,6 +207,7 @@ struct bpf_map_def {
 	unsigned int key_size;
 	unsigned int value_size;
 	unsigned int max_entries;
+	unsigned int map_flags;
 };
 
 /*
-- 
2.14.2.920.gcf0c67979c-goog

Re: [PATCH net-next v3 2/2] libbpf: use map_flags when creating maps

[Daniel Borkmann]

On 10/05/2017 04:41 PM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> This is required to use BPF_MAP_TYPE_LPM_TRIE or any other map type
> which requires flags.
>
> Signed-off-by: Craig Gallek <kraig@google.com>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [PATCH net-next v3 0/2] libbpf: support more map options

[Alexei Starovoitov]

On 10/5/17 7:41 AM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> The functional change to this series is the ability to use flags when
> creating maps from object files loaded by libbpf.  In order to do this,
> the first patch updates the library to handle map definitions that
> differ in size from libbpf's struct bpf_map_def.
>
> For object files with a larger map definition, libbpf will continue to load
> if the unknown fields are all zero, otherwise the map is rejected.  If the
> map definition in the object file is smaller than expected, libbpf will use
> zero as a default value in the missing fields.
>
> Craig Gallek (2):
>   libbpf: parse maps sections of varying size
>   libbpf: use map_flags when creating maps
>
>  tools/lib/bpf/libbpf.c | 72 +++++++++++++++++++++++++++++---------------------
>  tools/lib/bpf/libbpf.h |  1 +
>  2 files changed, 43 insertions(+), 30 deletions(-)

lgtm
Acked-by: Alexei Starovoitov <ast@kernel.org>

Re: [PATCH net-next v3 0/2] libbpf: support more map options

[David Miller]

From: Craig Gallek <kraigatgoog@gmail.com>
Date: Thu,  5 Oct 2017 10:41:56 -0400

> The functional change to this series is the ability to use flags when
> creating maps from object files loaded by libbpf.  In order to do this,
> the first patch updates the library to handle map definitions that
> differ in size from libbpf's struct bpf_map_def.
> 
> For object files with a larger map definition, libbpf will continue to load
> if the unknown fields are all zero, otherwise the map is rejected.  If the
> map definition in the object file is smaller than expected, libbpf will use
> zero as a default value in the missing fields.

Series applied, thanks.