From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1427304AbcBSNxc (ORCPT ); Fri, 19 Feb 2016 08:53:32 -0500 Received: from mail.kernel.org ([198.145.29.136]:52105 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1424447AbcBSNxb (ORCPT ); Fri, 19 Feb 2016 08:53:31 -0500 Date: Fri, 19 Feb 2016 10:53:26 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: Alexei Starovoitov , Brendan Gregg , Adrian Hunter , Cody P Schafer , "David S. Miller" , He Kuang , =?iso-8859-1?Q?J=E9r=E9mie?= Galarneau , Jiri Olsa , Kirill Smelkov , Li Zefan , Masami Hiramatsu , Namhyung Kim , Peter Zijlstra , pi3orama@163.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 06/54] perf tools: Add API to config maps in bpf object Message-ID: <20160215200211.GB17690@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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:." */ - char *map_name = strdup(term->config + sizeof("maps:") - 1); + /* key is "map:." */ + 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 -----