Re: [PATCH 06/54] perf tools: Add API to config maps in bpf object

[Arnaldo Carvalho de Melo]

Sorry for the top post, but the message below didn't made it thru due to
local problems as I recently switched notebooks, my postfix setup barfed
this one :-\

This is what I have in my tmp.perf/bpf_map:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/bpf_map&id=5c78fe3c5a944ba7f9a85f59548295211f3d252c

Please take a look and see if you're ok with it,

I'm going thru the other patches in this week's patchkit, thanks
for keeping up with this work,

Regards,

- Arnaldo

Em Fri, Feb 05, 2016 at 02:01:31PM +0000, Wang Nan escreveu:
> bpf__config_obj() is introduced as a core API to config BPF object
> after loading. One configuration option of maps is introduced. After
> this patch BPF object can accept configuration like:
> 
>  maps:my_map.value=1234

Ok, tested this, works great, trace_printk gets what I pass via the
event definition, etc.

One suggestion, tho. I think "maps", plural, is strange, we're referring
to one map, not multiple ones when we write: "map:my_map.value=1234", so
I have this on a separate branch, tmp.perf/bpf_map, do you agree?
Shorter, one character less to type :-)

I also changed some error reports and the commit log, hope that improved
it, please also take a look.

I'm now working on the patch right after this, all will be on the
tmp.perf/bpf_map shortly.

- Arnaldo

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index e24d5b7a9fec..fdb23c785ced 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -921,8 +921,8 @@ bpf__obj_config_map(struct bpf_object *obj,
 		    struct perf_evlist *evlist,
 		    int *key_scan_pos)
 {
-	/* key is "maps:<mapname>.<config opt>" */
-	char *map_name = strdup(term->config + sizeof("maps:") - 1);
+	/* key is "map:<mapname>.<config opt>" */
+	char *map_name = strdup(term->config + sizeof("map:") - 1);
 	struct bpf_map *map;
 	int err = -BPF_LOADER_ERRNO__OBJCONF_OPT;
 	char *map_opt;
@@ -961,8 +961,7 @@ bpf__obj_config_map(struct bpf_object *obj,
 		}
 	}
 
-	pr_debug("ERROR: Invalid config option '%s' for maps\n",
-		 map_opt);
+	pr_debug("ERROR: Invalid map config option '%s'\n", map_opt);
 	err = -BPF_LOADER_ERRNO__OBJCONF_MAP_OPT;
 out:
 	free(map_name);
@@ -982,8 +981,8 @@ int bpf__config_obj(struct bpf_object *obj,
 	if (!obj || !term || !term->config)
 		return -EINVAL;
 
-	if (!prefixcmp(term->config, "maps:")) {
-		key_scan_pos = sizeof("maps:") - 1;
+	if (!prefixcmp(term->config, "map:")) {
+		key_scan_pos = sizeof("map:") - 1;
 		err = bpf__obj_config_map(obj, term, evlist, &key_scan_pos);
 		goto out;
 	}
@@ -1011,7 +1010,7 @@ static const char *bpf_loader_strerror_table[NR_ERRNO] = {
 	[ERRCODE_OFFSET(PROLOGUEOOB)]	= "Offset out of bound for prologue",
 	[ERRCODE_OFFSET(OBJCONF_OPT)]	= "Invalid object config option",
 	[ERRCODE_OFFSET(OBJCONF_CONF)]	= "Config value not set (missing '=')",
-	[ERRCODE_OFFSET(OBJCONF_MAP_OPT)]	= "Invalid object maps config option",
+	[ERRCODE_OFFSET(OBJCONF_MAP_OPT)]	= "Invalid object map config option",
 	[ERRCODE_OFFSET(OBJCONF_MAP_NOTEXIST)]	= "Target map doesn't exist",
 	[ERRCODE_OFFSET(OBJCONF_MAP_VALUE)]	= "Incorrect value type for map",
 	[ERRCODE_OFFSET(OBJCONF_MAP_TYPE)]	= "Incorrect map type",

----- End forwarded message -----