Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1278645 > unrolled thread

[PATCH v2 04/13] perf tools: Add API to config maps in bpf object

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 04/13] perf tools: Add API to config maps in bpf object Wang Nan <wangnan0@huawei.com> - 2015-11-27 10:00 +0100
    RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in bpf  object Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-28 02:20 +0100
      Re: RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in  bpf object "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-28 02:30 +0100
        Re: RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in  bpf object "Wangnan (F)" <wangnan0@huawei.com> - 2015-11-28 02:30 +0100
      Re: RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in  bpf object Namhyung Kim <namhyung@kernel.org> - 2015-11-29 17:30 +0100

#1278645 — [PATCH v2 04/13] perf tools: Add API to config maps in bpf object

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

(maps.my_map.value looks pretty. However, there's a small but hard
to fixed problem related to flex's greedy matching. Please see [1].
Choose ':' to avoid it in a simpler way.)

This patch is more complex than the work it really does because the
consideration of extension. In designing of BPF map configuration,
following things should be considered:

 1. Array indics selection: perf should allow user setting different
    value to different slots in an array, with syntax like:
    maps:my_map:value[0,3...6]=1234;

 2. A map can be config by different config terms, each for a part
    of it. For example, set each slot to pid of a thread;

 3. Type of value: integer is not the only valid value type. Perf
    event can also be put into a map after commit 35578d7984003097af2b1e3
    (bpf: Implement function bpf_perf_event_read() that get the selected
    hardware PMU conuter);

 4. For hash table, it is possible to use string or other as key;

 5. It is possible that map configuration is unable to be setup
    during parsing. Perf event is an example.

Therefore, this patch does following:

 1. Instead of updating map element during parsing, this patch stores
    map config options in 'struct bpf_map_priv'. Following patches
    would apply those configs at proper time;

 2. Link map operations to a list so a map can have multiple config
    terms attached, so different parts can be configured separately;

 3. Make 'struct bpf_map_priv' extensible so following patches can
    add new types of keys and operations;

 4. Use bpf_config_map_funcs array to support more maps config options.

Since the patch changing event parser to parse BPF object config is
relative large, I put in another commit. Code in this patch
could be tested after applying next patch.

[1] http://lkml.kernel.org/g/564ED621.4050500@huawei.com

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/util/bpf-loader.c | 266 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/bpf-loader.h |  38 +++++++
 2 files changed, 304 insertions(+)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 540a7ef..989c80f 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -739,6 +739,251 @@ int bpf__foreach_tev(struct bpf_object *obj,
 	return 0;
 }
 
+enum bpf_map_op_type {
+	BPF_MAP_OP_SET_VALUE,
+};
+
+enum bpf_map_key_type {
+	BPF_MAP_KEY_ALL,
+};
+
+struct bpf_map_op {
+	struct list_head list;
+	enum bpf_map_op_type op_type;
+	enum bpf_map_key_type key_type;
+	union {
+		u64 value;
+	} v;
+};
+
+struct bpf_map_priv {
+	struct list_head ops_list;
+};
+
+static void
+bpf_map_op__free(struct bpf_map_op *op)
+{
+	struct list_head *list = &op->list;
+	/*
+	 * bpf_map_op__free() needs to consider following cases:
+	 *   1. When the op is created but not linked to any list:
+	 *      impossible. This only happen in bpf_map_op__alloc()
+	 *      and it would be freed directly;
+	 *   2. Normal case, when the op is linked to a list;
+	 *   3. After the op has already be removed.
+	 * Thanks to list.h, if it has removed by list_del() then
+	 * list->{next,prev} should have been set to LIST_POISON{1,2}.
+	 */
+	if ((list->next != LIST_POISON1) && (list->prev != LIST_POISON2))
+		list_del(list);
+	free(op);
+}
+
+static void
+bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
+		    void *_priv)
+{
+	struct bpf_map_priv *priv = _priv;
+	struct bpf_map_op *pos, *n;
+
+	list_for_each_entry_safe(pos, n, &priv->ops_list, list)
+		bpf_map_op__free(pos);
+	free(priv);
+}
+
+static struct bpf_map_op *
+bpf_map_op__alloc(struct bpf_map *map)
+{
+	struct bpf_map_op *op;
+	struct bpf_map_priv *priv;
+	const char *map_name;
+	int err;
+
+	map_name = bpf_map__get_name(map);
+	err = bpf_map__get_private(map, (void **)&priv);
+	if (err) {
+		pr_debug("Failed to get private from map %s\n", map_name);
+		return ERR_PTR(err);
+	}
+
+	if (!priv) {
+		priv = zalloc(sizeof(*priv));
+		if (!priv) {
+			pr_debug("No enough memory to alloc map private\n");
+			return ERR_PTR(-ENOMEM);
+		}
+		INIT_LIST_HEAD(&priv->ops_list);
+
+		if (bpf_map__set_private(map, priv, bpf_map_priv__clear)) {
+			free(priv);
+			return ERR_PTR(-BPF_LOADER_ERRNO__INTERNAL);
+		}
+	}
+
+	op = zalloc(sizeof(*op));
+	if (!op) {
+		pr_debug("Failed to alloc bpf_map_op\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	op->key_type = BPF_MAP_KEY_ALL;
+	list_add_tail(&op->list, &priv->ops_list);
+	return op;
+}
+
+static int
+bpf__obj_config_map_array_value(struct bpf_map *map,
+				struct parse_events_term *term)
+{
+	struct bpf_map_def def;
+	struct bpf_map_op *op;
+	const char *map_name;
+	int err;
+
+	map_name = bpf_map__get_name(map);
+
+	err = bpf_map__get_def(map, &def);
+	if (err) {
+		pr_debug("Unable to get map definition from '%s'\n",
+			 map_name);
+		return -BPF_LOADER_ERRNO__INTERNAL;
+	}
+
+	if (def.type != BPF_MAP_TYPE_ARRAY) {
+		pr_debug("Map %s type is not BPF_MAP_TYPE_ARRAY\n",
+			 map_name);
+		return -BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE;
+	}
+	if (def.key_size < sizeof(unsigned int)) {
+		pr_debug("Map %s has incorrect key size\n", map_name);
+		return -BPF_LOADER_ERRNO__OBJCONF_MAP_KEYSIZE;
+	}
+	switch (def.value_size) {
+	case 1:
+	case 2:
+	case 4:
+	case 8:
+		break;
+	default:
+		pr_debug("Map %s has incorrect value size\n", map_name);
+		return -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUESIZE;
+	}
+
+	op = bpf_map_op__alloc(map);
+	if (IS_ERR(op))
+		return PTR_ERR(op);
+	op->op_type = BPF_MAP_OP_SET_VALUE;
+	op->v.value = term->val.num;
+	return 0;
+}
+
+static int
+bpf__obj_config_map_value(struct bpf_map *map,
+			  struct parse_events_term *term,
+			  struct perf_evlist *evlist __maybe_unused)
+{
+	if (!term->err_val) {
+		pr_debug("Config value not set\n");
+		return -BPF_LOADER_ERRNO__OBJCONF_CONF;
+	}
+
+	if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+		return bpf__obj_config_map_array_value(map, term);
+
+	pr_debug("ERROR: wrong value type\n");
+	return -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE;
+}
+
+struct bpf_obj_config_map_func {
+	const char *config_opt;
+	int (*config_func)(struct bpf_map *, struct parse_events_term *,
+			   struct perf_evlist *);
+};
+
+struct bpf_obj_config_map_func bpf_obj_config_map_funcs[] = {
+	{"value", bpf__obj_config_map_value},
+};
+
+static int
+bpf__obj_config_map(struct bpf_object *obj,
+		    struct parse_events_term *term,
+		    struct perf_evlist *evlist,
+		    int *key_scan_pos)
+{
+	/* key is "maps:<mapname>:<config opt>" */
+	char *map_name = strdup(term->config + sizeof("maps:") - 1);
+	struct bpf_map *map;
+	int err = -BPF_LOADER_ERRNO__OBJCONF_OPT;
+	char *map_opt;
+	size_t i;
+
+	if (!map_name)
+		return -ENOMEM;
+
+	map_opt = strchr(map_name, ':');
+	if (!map_opt) {
+		pr_debug("ERROR: Invalid map config: %s\n", map_name);
+		goto out;
+	}
+
+	*map_opt++ = '\0';
+	if (*map_opt == '\0') {
+		pr_debug("ERROR: Invalid map option: %s\n", term->config);
+		goto out;
+	}
+
+	map = bpf_object__get_map_by_name(obj, map_name);
+	if (!map) {
+		pr_debug("ERROR: Map %s is not exist\n", map_name);
+		err = -BPF_LOADER_ERRNO__OBJCONF_MAP_NOTEXIST;
+		goto out;
+	}
+
+	*key_scan_pos += map_opt - map_name;
+	for (i = 0; i < ARRAY_SIZE(bpf_obj_config_map_funcs); i++) {
+		struct bpf_obj_config_map_func *func =
+				&bpf_obj_config_map_funcs[i];
+
+		if (strcmp(map_opt, func->config_opt) == 0) {
+			err = func->config_func(map, term, evlist);
+			goto out;
+		}
+	}
+
+	pr_debug("ERROR: invalid config option '%s' for maps\n",
+		 map_opt);
+	err = -BPF_LOADER_ERRNO__OBJCONF_MAP_OPT;
+out:
+	free(map_name);
+	if (!err)
+		key_scan_pos += strlen(map_opt);
+	return err;
+}
+
+int bpf__config_obj(struct bpf_object *obj,
+		    struct parse_events_term *term,
+		    struct perf_evlist *evlist,
+		    int *error_pos)
+{
+	int key_scan_pos = 0;
+	int err;
+
+	if (!obj || !term || !term->config)
+		return -EINVAL;
+
+	if (!prefixcmp(term->config, "maps:")) {
+		key_scan_pos = sizeof("maps:") - 1;
+		err = bpf__obj_config_map(obj, term, evlist, &key_scan_pos);
+		goto out;
+	}
+	err = -BPF_LOADER_ERRNO__OBJCONF_OPT;
+out:
+	if (error_pos)
+		*error_pos = key_scan_pos;
+	return err;
+
+}
+
 #define ERRNO_OFFSET(e)		((e) - __BPF_LOADER_ERRNO__START)
 #define ERRCODE_OFFSET(c)	ERRNO_OFFSET(BPF_LOADER_ERRNO__##c)
 #define NR_ERRNO	(__BPF_LOADER_ERRNO__END - __BPF_LOADER_ERRNO__START)
@@ -753,6 +998,14 @@ static const char *bpf_loader_strerror_table[NR_ERRNO] = {
 	[ERRCODE_OFFSET(PROLOGUE)]	= "Failed to generate prologue",
 	[ERRCODE_OFFSET(PROLOGUE2BIG)]	= "Prologue too big for program",
 	[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 (lost '=')",
+	[ERRCODE_OFFSET(OBJCONF_MAP_OPT)]	= "Invalid object maps config option",
+	[ERRCODE_OFFSET(OBJCONF_MAP_NOTEXIST)]	= "Target map not exist",
+	[ERRCODE_OFFSET(OBJCONF_MAP_VALUE)]	= "Incorrect value type for map",
+	[ERRCODE_OFFSET(OBJCONF_MAP_TYPE)]	= "Incorrect map type",
+	[ERRCODE_OFFSET(OBJCONF_MAP_KEYSIZE)]	= "Incorrect map key size",
+	[ERRCODE_OFFSET(OBJCONF_MAP_VALUESIZE)]	= "Incorrect map value size",
 };
 
 static int
@@ -872,3 +1125,16 @@ int bpf__strerror_load(struct bpf_object *obj,
 	bpf__strerror_end(buf, size);
 	return 0;
 }
+
+int bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
+			     struct parse_events_term *term __maybe_unused,
+			     struct perf_evlist *evlist __maybe_unused,
+			     int *error_pos __maybe_unused, int err,
+			     char *buf, size_t size)
+{
+	bpf__strerror_head(err, buf, size);
+	bpf__strerror_entry(BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE,
+			    "Can't use this config term to this type of map");
+	bpf__strerror_end(buf, size);
+	return 0;
+}
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index 6fdc045..2464db9 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -10,6 +10,7 @@
 #include <string.h>
 #include <bpf/libbpf.h>
 #include "probe-event.h"
+#include "evlist.h"
 #include "debug.h"
 
 enum bpf_loader_errno {
@@ -24,10 +25,19 @@ enum bpf_loader_errno {
 	BPF_LOADER_ERRNO__PROLOGUE,	/* Failed to generate prologue */
 	BPF_LOADER_ERRNO__PROLOGUE2BIG,	/* Prologue too big for program */
 	BPF_LOADER_ERRNO__PROLOGUEOOB,	/* Offset out of bound for prologue */
+	BPF_LOADER_ERRNO__OBJCONF_OPT,	/* Invalid object config option */
+	BPF_LOADER_ERRNO__OBJCONF_CONF,	/* Config value not set (lost '=')) */
+	BPF_LOADER_ERRNO__OBJCONF_MAP_OPT,	/* Invalid object maps config option */
+	BPF_LOADER_ERRNO__OBJCONF_MAP_NOTEXIST,	/* Target map not exist */
+	BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE,	/* Incorrect value type for map */
+	BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE,	/* Incorrect map type */
+	BPF_LOADER_ERRNO__OBJCONF_MAP_KEYSIZE,	/* Incorrect map key size */
+	BPF_LOADER_ERRNO__OBJCONF_MAP_VALUESIZE,/* Incorrect map value size */
 	__BPF_LOADER_ERRNO__END,
 };
 
 struct bpf_object;
+struct parse_events_term;
 #define PERF_BPF_PROBE_GROUP "perf_bpf_probe"
 
 typedef int (*bpf_prog_iter_callback_t)(struct probe_trace_event *tev,
@@ -53,6 +63,14 @@ int bpf__strerror_load(struct bpf_object *obj, int err,
 		       char *buf, size_t size);
 int bpf__foreach_tev(struct bpf_object *obj,
 		     bpf_prog_iter_callback_t func, void *arg);
+
+int bpf__config_obj(struct bpf_object *obj, struct parse_events_term *term,
+		    struct perf_evlist *evlist, int *error_pos);
+int bpf__strerror_config_obj(struct bpf_object *obj,
+			     struct parse_events_term *term,
+			     struct perf_evlist *evlist,
+			     int *error_pos, int err, char *buf,
+			     size_t size);
 #else
 static inline struct bpf_object *
 bpf__prepare_load(const char *filename __maybe_unused,
@@ -84,6 +102,15 @@ bpf__foreach_tev(struct bpf_object *obj __maybe_unused,
 }
 
 static inline int
+bpf__config_obj(struct bpf_object *obj __maybe_unused,
+		struct parse_events_term *term __maybe_unused,
+		struct perf_evlist *evlist __maybe_unused,
+		int *error_pos __maybe_unused)
+{
+	return 0;
+}
+
+static inline int
 __bpf_strerror(char *buf, size_t size)
 {
 	if (!size)
@@ -118,5 +145,16 @@ static inline int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
 {
 	return __bpf_strerror(buf, size);
 }
+
+static inline int
+bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
+			 struct parse_events_term *term __maybe_unused,
+			 struct perf_evlist *evlist __maybe_unused,
+			 int *error_pos __maybe_unused,
+			 int err __maybe_unused,
+			 char *buf, size_t size)
+{
+	return __bpf_strerror(buf, size);
+}
 #endif
 #endif
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1279051 — RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in bpf object

Em Fri, Nov 27, 2015 at 08:47:38AM +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
> 
> (maps.my_map.value looks pretty. However, there's a small but hard
> to fixed problem related to flex's greedy matching. Please see [1].
> Choose ':' to avoid it in a simpler way.)

Understood the issues, but I would like to hear from Ingo, Jiri,
Namhyung, Brian and others here, since we're setting up syntax, and yes,
using:
  
  maps.my_map.value[0,3...6]=1234;

or even:

  maps->my_map->value[0,3...6]=1234;

Looks more natural than:

  maps:my_map:value[0,3...6]=1234;

I'll think harder about this, maybe we can find a way to use the more
familiar dot notation somehow.

And I have to anyway test this more extensively, so I'll push the
patches I have and try this as soon as I can.

- Arnaldo
> 
> This patch is more complex than the work it really does because the
> consideration of extension. In designing of BPF map configuration,
> following things should be considered:
> 
>  1. Array indics selection: perf should allow user setting different

indices :-)

>     value to different slots in an array, with syntax like:
>     maps:my_map:value[0,3...6]=1234;
> 
>  2. A map can be config by different config terms, each for a part
>     of it. For example, set each slot to pid of a thread;
> 
>  3. Type of value: integer is not the only valid value type. Perf
>     event can also be put into a map after commit 35578d7984003097af2b1e3
>     (bpf: Implement function bpf_perf_event_read() that get the selected
>     hardware PMU conuter);
> 
>  4. For hash table, it is possible to use string or other as key;
> 
>  5. It is possible that map configuration is unable to be setup
>     during parsing. Perf event is an example.
> 
> Therefore, this patch does following:
> 
>  1. Instead of updating map element during parsing, this patch stores
>     map config options in 'struct bpf_map_priv'. Following patches
>     would apply those configs at proper time;
> 
>  2. Link map operations to a list so a map can have multiple config
>     terms attached, so different parts can be configured separately;
> 
>  3. Make 'struct bpf_map_priv' extensible so following patches can
>     add new types of keys and operations;
> 
>  4. Use bpf_config_map_funcs array to support more maps config options.
> 
> Since the patch changing event parser to parse BPF object config is
> relative large, I put in another commit. Code in this patch
> could be tested after applying next patch.
> 
> [1] http://lkml.kernel.org/g/564ED621.4050500@huawei.com
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> ---
>  tools/perf/util/bpf-loader.c | 266 +++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/bpf-loader.h |  38 +++++++
>  2 files changed, 304 insertions(+)
> 
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 540a7ef..989c80f 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -739,6 +739,251 @@ int bpf__foreach_tev(struct bpf_object *obj,
>  	return 0;
>  }
>  
> +enum bpf_map_op_type {
> +	BPF_MAP_OP_SET_VALUE,
> +};
> +
> +enum bpf_map_key_type {
> +	BPF_MAP_KEY_ALL,
> +};
> +
> +struct bpf_map_op {
> +	struct list_head list;
> +	enum bpf_map_op_type op_type;
> +	enum bpf_map_key_type key_type;
> +	union {
> +		u64 value;
> +	} v;
> +};
> +
> +struct bpf_map_priv {
> +	struct list_head ops_list;
> +};
> +
> +static void
> +bpf_map_op__free(struct bpf_map_op *op)
> +{
> +	struct list_head *list = &op->list;
> +	/*
> +	 * bpf_map_op__free() needs to consider following cases:
> +	 *   1. When the op is created but not linked to any list:
> +	 *      impossible. This only happen in bpf_map_op__alloc()
> +	 *      and it would be freed directly;
> +	 *   2. Normal case, when the op is linked to a list;
> +	 *   3. After the op has already be removed.
> +	 * Thanks to list.h, if it has removed by list_del() then
> +	 * list->{next,prev} should have been set to LIST_POISON{1,2}.
> +	 */
> +	if ((list->next != LIST_POISON1) && (list->prev != LIST_POISON2))
> +		list_del(list);
> +	free(op);
> +}
> +
> +static void
> +bpf_map_priv__clear(struct bpf_map *map __maybe_unused,
> +		    void *_priv)
> +{
> +	struct bpf_map_priv *priv = _priv;
> +	struct bpf_map_op *pos, *n;
> +
> +	list_for_each_entry_safe(pos, n, &priv->ops_list, list)
> +		bpf_map_op__free(pos);
> +	free(priv);
> +}
> +
> +static struct bpf_map_op *
> +bpf_map_op__alloc(struct bpf_map *map)
> +{
> +	struct bpf_map_op *op;
> +	struct bpf_map_priv *priv;
> +	const char *map_name;
> +	int err;
> +
> +	map_name = bpf_map__get_name(map);
> +	err = bpf_map__get_private(map, (void **)&priv);
> +	if (err) {
> +		pr_debug("Failed to get private from map %s\n", map_name);
> +		return ERR_PTR(err);
> +	}
> +
> +	if (!priv) {
> +		priv = zalloc(sizeof(*priv));
> +		if (!priv) {
> +			pr_debug("No enough memory to alloc map private\n");
> +			return ERR_PTR(-ENOMEM);
> +		}
> +		INIT_LIST_HEAD(&priv->ops_list);
> +
> +		if (bpf_map__set_private(map, priv, bpf_map_priv__clear)) {
> +			free(priv);
> +			return ERR_PTR(-BPF_LOADER_ERRNO__INTERNAL);
> +		}
> +	}
> +
> +	op = zalloc(sizeof(*op));
> +	if (!op) {
> +		pr_debug("Failed to alloc bpf_map_op\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	op->key_type = BPF_MAP_KEY_ALL;
> +	list_add_tail(&op->list, &priv->ops_list);
> +	return op;
> +}
> +
> +static int
> +bpf__obj_config_map_array_value(struct bpf_map *map,
> +				struct parse_events_term *term)
> +{
> +	struct bpf_map_def def;
> +	struct bpf_map_op *op;
> +	const char *map_name;
> +	int err;
> +
> +	map_name = bpf_map__get_name(map);
> +
> +	err = bpf_map__get_def(map, &def);
> +	if (err) {
> +		pr_debug("Unable to get map definition from '%s'\n",
> +			 map_name);
> +		return -BPF_LOADER_ERRNO__INTERNAL;
> +	}
> +
> +	if (def.type != BPF_MAP_TYPE_ARRAY) {
> +		pr_debug("Map %s type is not BPF_MAP_TYPE_ARRAY\n",
> +			 map_name);
> +		return -BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE;
> +	}
> +	if (def.key_size < sizeof(unsigned int)) {
> +		pr_debug("Map %s has incorrect key size\n", map_name);
> +		return -BPF_LOADER_ERRNO__OBJCONF_MAP_KEYSIZE;
> +	}
> +	switch (def.value_size) {
> +	case 1:
> +	case 2:
> +	case 4:
> +	case 8:
> +		break;
> +	default:
> +		pr_debug("Map %s has incorrect value size\n", map_name);
> +		return -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUESIZE;
> +	}
> +
> +	op = bpf_map_op__alloc(map);
> +	if (IS_ERR(op))
> +		return PTR_ERR(op);
> +	op->op_type = BPF_MAP_OP_SET_VALUE;
> +	op->v.value = term->val.num;
> +	return 0;
> +}
> +
> +static int
> +bpf__obj_config_map_value(struct bpf_map *map,
> +			  struct parse_events_term *term,
> +			  struct perf_evlist *evlist __maybe_unused)
> +{
> +	if (!term->err_val) {
> +		pr_debug("Config value not set\n");
> +		return -BPF_LOADER_ERRNO__OBJCONF_CONF;
> +	}
> +
> +	if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
> +		return bpf__obj_config_map_array_value(map, term);
> +
> +	pr_debug("ERROR: wrong value type\n");
> +	return -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE;
> +}
> +
> +struct bpf_obj_config_map_func {
> +	const char *config_opt;
> +	int (*config_func)(struct bpf_map *, struct parse_events_term *,
> +			   struct perf_evlist *);
> +};
> +
> +struct bpf_obj_config_map_func bpf_obj_config_map_funcs[] = {
> +	{"value", bpf__obj_config_map_value},
> +};
> +
> +static int
> +bpf__obj_config_map(struct bpf_object *obj,
> +		    struct parse_events_term *term,
> +		    struct perf_evlist *evlist,
> +		    int *key_scan_pos)
> +{
> +	/* key is "maps:<mapname>:<config opt>" */
> +	char *map_name = strdup(term->config + sizeof("maps:") - 1);
> +	struct bpf_map *map;
> +	int err = -BPF_LOADER_ERRNO__OBJCONF_OPT;
> +	char *map_opt;
> +	size_t i;
> +
> +	if (!map_name)
> +		return -ENOMEM;
> +
> +	map_opt = strchr(map_name, ':');
> +	if (!map_opt) {
> +		pr_debug("ERROR: Invalid map config: %s\n", map_name);
> +		goto out;
> +	}
> +
> +	*map_opt++ = '\0';
> +	if (*map_opt == '\0') {
> +		pr_debug("ERROR: Invalid map option: %s\n", term->config);
> +		goto out;
> +	}
> +
> +	map = bpf_object__get_map_by_name(obj, map_name);
> +	if (!map) {
> +		pr_debug("ERROR: Map %s is not exist\n", map_name);
> +		err = -BPF_LOADER_ERRNO__OBJCONF_MAP_NOTEXIST;
> +		goto out;
> +	}
> +
> +	*key_scan_pos += map_opt - map_name;
> +	for (i = 0; i < ARRAY_SIZE(bpf_obj_config_map_funcs); i++) {
> +		struct bpf_obj_config_map_func *func =
> +				&bpf_obj_config_map_funcs[i];
> +
> +		if (strcmp(map_opt, func->config_opt) == 0) {
> +			err = func->config_func(map, term, evlist);
> +			goto out;
> +		}
> +	}
> +
> +	pr_debug("ERROR: invalid config option '%s' for maps\n",
> +		 map_opt);
> +	err = -BPF_LOADER_ERRNO__OBJCONF_MAP_OPT;
> +out:
> +	free(map_name);
> +	if (!err)
> +		key_scan_pos += strlen(map_opt);
> +	return err;
> +}
> +
> +int bpf__config_obj(struct bpf_object *obj,
> +		    struct parse_events_term *term,
> +		    struct perf_evlist *evlist,
> +		    int *error_pos)
> +{
> +	int key_scan_pos = 0;
> +	int err;
> +
> +	if (!obj || !term || !term->config)
> +		return -EINVAL;
> +
> +	if (!prefixcmp(term->config, "maps:")) {
> +		key_scan_pos = sizeof("maps:") - 1;
> +		err = bpf__obj_config_map(obj, term, evlist, &key_scan_pos);
> +		goto out;
> +	}
> +	err = -BPF_LOADER_ERRNO__OBJCONF_OPT;
> +out:
> +	if (error_pos)
> +		*error_pos = key_scan_pos;
> +	return err;
> +
> +}
> +
>  #define ERRNO_OFFSET(e)		((e) - __BPF_LOADER_ERRNO__START)
>  #define ERRCODE_OFFSET(c)	ERRNO_OFFSET(BPF_LOADER_ERRNO__##c)
>  #define NR_ERRNO	(__BPF_LOADER_ERRNO__END - __BPF_LOADER_ERRNO__START)
> @@ -753,6 +998,14 @@ static const char *bpf_loader_strerror_table[NR_ERRNO] = {
>  	[ERRCODE_OFFSET(PROLOGUE)]	= "Failed to generate prologue",
>  	[ERRCODE_OFFSET(PROLOGUE2BIG)]	= "Prologue too big for program",
>  	[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 (lost '=')",
> +	[ERRCODE_OFFSET(OBJCONF_MAP_OPT)]	= "Invalid object maps config option",
> +	[ERRCODE_OFFSET(OBJCONF_MAP_NOTEXIST)]	= "Target map not exist",
> +	[ERRCODE_OFFSET(OBJCONF_MAP_VALUE)]	= "Incorrect value type for map",
> +	[ERRCODE_OFFSET(OBJCONF_MAP_TYPE)]	= "Incorrect map type",
> +	[ERRCODE_OFFSET(OBJCONF_MAP_KEYSIZE)]	= "Incorrect map key size",
> +	[ERRCODE_OFFSET(OBJCONF_MAP_VALUESIZE)]	= "Incorrect map value size",
>  };
>  
>  static int
> @@ -872,3 +1125,16 @@ int bpf__strerror_load(struct bpf_object *obj,
>  	bpf__strerror_end(buf, size);
>  	return 0;
>  }
> +
> +int bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
> +			     struct parse_events_term *term __maybe_unused,
> +			     struct perf_evlist *evlist __maybe_unused,
> +			     int *error_pos __maybe_unused, int err,
> +			     char *buf, size_t size)
> +{
> +	bpf__strerror_head(err, buf, size);
> +	bpf__strerror_entry(BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE,
> +			    "Can't use this config term to this type of map");
> +	bpf__strerror_end(buf, size);
> +	return 0;
> +}
> diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
> index 6fdc045..2464db9 100644
> --- a/tools/perf/util/bpf-loader.h
> +++ b/tools/perf/util/bpf-loader.h
> @@ -10,6 +10,7 @@
>  #include <string.h>
>  #include <bpf/libbpf.h>
>  #include "probe-event.h"
> +#include "evlist.h"
>  #include "debug.h"
>  
>  enum bpf_loader_errno {
> @@ -24,10 +25,19 @@ enum bpf_loader_errno {
>  	BPF_LOADER_ERRNO__PROLOGUE,	/* Failed to generate prologue */
>  	BPF_LOADER_ERRNO__PROLOGUE2BIG,	/* Prologue too big for program */
>  	BPF_LOADER_ERRNO__PROLOGUEOOB,	/* Offset out of bound for prologue */
> +	BPF_LOADER_ERRNO__OBJCONF_OPT,	/* Invalid object config option */
> +	BPF_LOADER_ERRNO__OBJCONF_CONF,	/* Config value not set (lost '=')) */
> +	BPF_LOADER_ERRNO__OBJCONF_MAP_OPT,	/* Invalid object maps config option */
> +	BPF_LOADER_ERRNO__OBJCONF_MAP_NOTEXIST,	/* Target map not exist */
> +	BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE,	/* Incorrect value type for map */
> +	BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE,	/* Incorrect map type */
> +	BPF_LOADER_ERRNO__OBJCONF_MAP_KEYSIZE,	/* Incorrect map key size */
> +	BPF_LOADER_ERRNO__OBJCONF_MAP_VALUESIZE,/* Incorrect map value size */
>  	__BPF_LOADER_ERRNO__END,
>  };
>  
>  struct bpf_object;
> +struct parse_events_term;
>  #define PERF_BPF_PROBE_GROUP "perf_bpf_probe"
>  
>  typedef int (*bpf_prog_iter_callback_t)(struct probe_trace_event *tev,
> @@ -53,6 +63,14 @@ int bpf__strerror_load(struct bpf_object *obj, int err,
>  		       char *buf, size_t size);
>  int bpf__foreach_tev(struct bpf_object *obj,
>  		     bpf_prog_iter_callback_t func, void *arg);
> +
> +int bpf__config_obj(struct bpf_object *obj, struct parse_events_term *term,
> +		    struct perf_evlist *evlist, int *error_pos);
> +int bpf__strerror_config_obj(struct bpf_object *obj,
> +			     struct parse_events_term *term,
> +			     struct perf_evlist *evlist,
> +			     int *error_pos, int err, char *buf,
> +			     size_t size);
>  #else
>  static inline struct bpf_object *
>  bpf__prepare_load(const char *filename __maybe_unused,
> @@ -84,6 +102,15 @@ bpf__foreach_tev(struct bpf_object *obj __maybe_unused,
>  }
>  
>  static inline int
> +bpf__config_obj(struct bpf_object *obj __maybe_unused,
> +		struct parse_events_term *term __maybe_unused,
> +		struct perf_evlist *evlist __maybe_unused,
> +		int *error_pos __maybe_unused)
> +{
> +	return 0;
> +}
> +
> +static inline int
>  __bpf_strerror(char *buf, size_t size)
>  {
>  	if (!size)
> @@ -118,5 +145,16 @@ static inline int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
>  {
>  	return __bpf_strerror(buf, size);
>  }
> +
> +static inline int
> +bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused,
> +			 struct parse_events_term *term __maybe_unused,
> +			 struct perf_evlist *evlist __maybe_unused,
> +			 int *error_pos __maybe_unused,
> +			 int err __maybe_unused,
> +			 char *buf, size_t size)
> +{
> +	return __bpf_strerror(buf, size);
> +}
>  #endif
>  #endif
> -- 
> 1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279054 — Re: RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in bpf object

On 2015/11/28 9:10, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 27, 2015 at 08:47:38AM +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
>>
>> (maps.my_map.value looks pretty. However, there's a small but hard
>> to fixed problem related to flex's greedy matching. Please see [1].
>> Choose ':' to avoid it in a simpler way.)
> Understood the issues, but I would like to hear from Ingo, Jiri,
> Namhyung, Brian and others here, since we're setting up syntax, and yes,
> using:
>    
>    maps.my_map.value[0,3...6]=1234;
>
> or even:
>
>    maps->my_map->value[0,3...6]=1234;
>
> Looks more natural than:
>
>    maps:my_map:value[0,3...6]=1234;

You don't mention maps::my_map::value. C++ use '::' this way, so I think
it is also a possible choice.

Or we can use another character for example '|' to replace '/' in case we
have a map starting with 'c' or 'o'. For example:

./mybpf.c|maps.channel.value=1234|

and in normal case still support '/':

./mybpf.c/maps.channel.value=1234/

like 'sed':

# sed 's/a/b/g'
a
b
# sed 's+/+|+g'
/
|

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279065 — Re: RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in bpf object

On 2015/11/28 9:20, Wangnan (F) wrote:
>
>
> On 2015/11/28 9:10, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Nov 27, 2015 at 08:47:38AM +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
>>>
>>> (maps.my_map.value looks pretty. However, there's a small but hard
>>> to fixed problem related to flex's greedy matching. Please see [1].
>>> Choose ':' to avoid it in a simpler way.)
>> Understood the issues, but I would like to hear from Ingo, Jiri,
>> Namhyung, Brian and others here, since we're setting up syntax, and yes,
>> using:
>>       maps.my_map.value[0,3...6]=1234;
>>
>> or even:
>>
>>    maps->my_map->value[0,3...6]=1234;
>>
>> Looks more natural than:
>>
>>    maps:my_map:value[0,3...6]=1234;
>
> You don't mention maps::my_map::value. C++ use '::' this way, so I think
> it is also a possible choice.
>
> Or we can use another character for example '|' to replace '/' in case we
> have a map starting with 'c' or 'o'. For example:
>
> ./mybpf.c|maps.channel.value=1234|
>
> and in normal case still support '/':
>
> ./mybpf.c/maps.channel.value=1234/

Should be:

./mybpf.c/maps.mymap.value=1234/

>
> like 'sed':
>
> # sed 's/a/b/g'
> a
> b
> # sed 's+/+|+g'
> /
> |
>
> Thank you.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279371 — Re: RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in bpf object

Hi Arnaldo,

On Fri, Nov 27, 2015 at 10:10:54PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 27, 2015 at 08:47:38AM +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
> > 
> > (maps.my_map.value looks pretty. However, there's a small but hard
> > to fixed problem related to flex's greedy matching. Please see [1].
> > Choose ':' to avoid it in a simpler way.)
> 
> Understood the issues, but I would like to hear from Ingo, Jiri,
> Namhyung, Brian and others here, since we're setting up syntax, and yes,
> using:
>   
>   maps.my_map.value[0,3...6]=1234;
> 
> or even:
> 
>   maps->my_map->value[0,3...6]=1234;
> 
> Looks more natural than:
> 
>   maps:my_map:value[0,3...6]=1234;

Do we have to use only single separator?  What about this?

    maps:my_map.value[0,3...6]=1234;

Thanks,
Namhyung


> 
> I'll think harder about this, maybe we can find a way to use the more
> familiar dot notation somehow.
> 
> And I have to anyway test this more extensively, so I'll push the
> patches I have and try this as soon as I can.
> 
> - Arnaldo
> > 
> > This patch is more complex than the work it really does because the
> > consideration of extension. In designing of BPF map configuration,
> > following things should be considered:
> > 
> >  1. Array indics selection: perf should allow user setting different
> 
> indices :-)
> 
> >     value to different slots in an array, with syntax like:
> >     maps:my_map:value[0,3...6]=1234;
> > 
> >  2. A map can be config by different config terms, each for a part
> >     of it. For example, set each slot to pid of a thread;
> > 
> >  3. Type of value: integer is not the only valid value type. Perf
> >     event can also be put into a map after commit 35578d7984003097af2b1e3
> >     (bpf: Implement function bpf_perf_event_read() that get the selected
> >     hardware PMU conuter);
> > 
> >  4. For hash table, it is possible to use string or other as key;
> > 
> >  5. It is possible that map configuration is unable to be setup
> >     during parsing. Perf event is an example.
> > 
> > Therefore, this patch does following:
> > 
> >  1. Instead of updating map element during parsing, this patch stores
> >     map config options in 'struct bpf_map_priv'. Following patches
> >     would apply those configs at proper time;
> > 
> >  2. Link map operations to a list so a map can have multiple config
> >     terms attached, so different parts can be configured separately;
> > 
> >  3. Make 'struct bpf_map_priv' extensible so following patches can
> >     add new types of keys and operations;
> > 
> >  4. Use bpf_config_map_funcs array to support more maps config options.
> > 
> > Since the patch changing event parser to parse BPF object config is
> > relative large, I put in another commit. Code in this patch
> > could be tested after applying next patch.
> > 
> > [1] http://lkml.kernel.org/g/564ED621.4050500@huawei.com
>  
> > Signed-off-by: Wang Nan <wangnan0@huawei.com>
> > Signed-off-by: He Kuang <hekuang@huawei.com>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Zefan Li <lizefan@huawei.com>
> > Cc: pi3orama@163.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web