Re: [bpf-next PATCH v2 2/2] bpf: bpftool, add flag to allow non-compat map definitions

2018-10-15 Thread Jakub Kicinski
On Mon, 15 Oct 2018 08:17:53 -0700, John Fastabend wrote:
> Multiple map definition structures exist and user may have non-zero
> fields in their definition that are not recognized by bpftool and
> libbpf. The normal behavior is to then fail loading the map. Although
> this is a good default behavior users may still want to load the map
> for debugging or other reasons. This patch adds a --mapcompat flag
> that can be used to override the default behavior and allow loading
> the map even when it has additional non-zero fields.
> 
> For now the only user is 'bpftool prog' we can switch over other
> subcommands as needed. The library exposes an API that consumes
> a flags field now but I kept the original API around also in case
> users of the API don't want to expose this. The flags field is an
> int in case we need more control over how the API call handles
> errors/features/etc in the future.
> 
> Signed-off-by: John Fastabend 

No strong opinion on the functionality, but may I be a grump and again
request adding the new option to completions and the man page? :)


[bpf-next PATCH v2 2/2] bpf: bpftool, add flag to allow non-compat map definitions

2018-10-15 Thread John Fastabend
Multiple map definition structures exist and user may have non-zero
fields in their definition that are not recognized by bpftool and
libbpf. The normal behavior is to then fail loading the map. Although
this is a good default behavior users may still want to load the map
for debugging or other reasons. This patch adds a --mapcompat flag
that can be used to override the default behavior and allow loading
the map even when it has additional non-zero fields.

For now the only user is 'bpftool prog' we can switch over other
subcommands as needed. The library exposes an API that consumes
a flags field now but I kept the original API around also in case
users of the API don't want to expose this. The flags field is an
int in case we need more control over how the API call handles
errors/features/etc in the future.

Signed-off-by: John Fastabend 
---
 tools/bpf/bpftool/main.c |7 ++-
 tools/bpf/bpftool/main.h |3 ++-
 tools/bpf/bpftool/prog.c |2 +-
 tools/lib/bpf/bpf.h  |3 +++
 tools/lib/bpf/libbpf.c   |   27 ++-
 tools/lib/bpf/libbpf.h   |2 ++
 6 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 79dc3f1..828dde3 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -55,6 +55,7 @@
 bool pretty_output;
 bool json_output;
 bool show_pinned;
+int bpf_flags;
 struct pinned_obj_table prog_table;
 struct pinned_obj_table map_table;
 
@@ -341,6 +342,7 @@ int main(int argc, char **argv)
{ "pretty", no_argument,NULL,   'p' },
{ "version",no_argument,NULL,   'V' },
{ "bpffs",  no_argument,NULL,   'f' },
+   { "mapcompat",  no_argument,NULL,   'm' },
{ 0 }
};
int opt, ret;
@@ -355,7 +357,7 @@ int main(int argc, char **argv)
hash_init(map_table.table);
 
opterr = 0;
-   while ((opt = getopt_long(argc, argv, "Vhpjf",
+   while ((opt = getopt_long(argc, argv, "Vhpjfm",
  options, NULL)) >= 0) {
switch (opt) {
case 'V':
@@ -379,6 +381,9 @@ int main(int argc, char **argv)
case 'f':
show_pinned = true;
break;
+   case 'm':
+   bpf_flags = MAPS_RELAX_COMPAT;
+   break;
default:
p_err("unrecognized option '%s'", argv[optind - 1]);
if (json_output)
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 40492cd..91fd697 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -74,7 +74,7 @@
 #define HELP_SPEC_PROGRAM  \
"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }"
 #define HELP_SPEC_OPTIONS  \
-   "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} }"
+   "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} | 
{-m|--mapcompat}"
 #define HELP_SPEC_MAP  \
"MAP := { id MAP_ID | pinned FILE }"
 
@@ -89,6 +89,7 @@ enum bpf_obj_type {
 extern json_writer_t *json_wtr;
 extern bool json_output;
 extern bool show_pinned;
+extern int bpf_flags;
 extern struct pinned_obj_table prog_table;
 extern struct pinned_obj_table map_table;
 
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 99ab42c..3350289 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -908,7 +908,7 @@ static int do_load(int argc, char **argv)
}
}
 
-   obj = bpf_object__open_xattr();
+   obj = __bpf_object__open_xattr(, bpf_flags);
if (IS_ERR_OR_NULL(obj)) {
p_err("failed to open object file");
goto err_free_reuse_maps;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 87520a8..69a4d40 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -69,6 +69,9 @@ struct bpf_load_program_attr {
__u32 prog_ifindex;
 };
 
+/* Flags to direct loading requirements */
+#define MAPS_RELAX_COMPAT  0x01
+
 /* Recommend log buffer size */
 #define BPF_LOG_BUF_SIZE (256 * 1024)
 int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 176cf55..bd71efc 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -562,8 +562,9 @@ static int compare_bpf_map(const void *_a, const void *_b)
 }
 
 static int
-bpf_object__init_maps(struct bpf_object *obj)
+bpf_object__init_maps(struct bpf_object *obj, int flags)
 {
+   bool strict = !(flags & MAPS_RELAX_COMPAT);
int i, map_idx, map_def_sz, nr_maps = 0;
Elf_Scn *scn;
Elf_Data *data;
@@ -685,7 +686,8 @@ static int compare_bpf_map(const void *_a, const void *_b)