| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20151113154629.GL7160@kernel.org>
Date: Fri, 13 Nov 2015 12:46:29 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: masami.hiramatsu.pt@...achi.com, ast@...nel.org,
lizefan@...wei.com, pi3orama@....com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/12] perf tools: Allow BPF program config probing
options
Em Fri, Nov 13, 2015 at 12:29:14PM +0000, Wang Nan escreveu:
> By extending the syntax of BPF object section names, this patch allows
> user to config probing options like what they can do in 'perf probe'.
>
> Test result:
>
> For following BPF file bpf.c:
>
> SEC("inlines=no\n"
> "func=SyS_dup?")
> int func(void *ctx)
> {
> return 1;
> }
>
> Cmdline:
>
> # ./perf record -e ./test_probe_glob.c ls /
> ...
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.013 MB perf.data ]
> # ./perf evlist
> perf_bpf_probe:func_1
> perf_bpf_probe:func
>
> Change "inlines=no" to "inlines=yes":
>
> Cmdline:
>
> # ./perf record -e ./test_probe_glob.c ls /
> ...
> [ perf record: Woken up 2 times to write data ]
> [ perf record: Captured and wrote 0.013 MB perf.data ]
> # ./perf evlist
> perf_bpf_probe:func_3
> perf_bpf_probe:func_2
> perf_bpf_probe:func_1
> perf_bpf_probe:func
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Zefan Li <lizefan@...wei.com>
> Cc: pi3orama@....com
> ---
> tools/perf/util/bpf-loader.c | 50 +++++++++++++++++++++++++++++++++++++++++++-
> tools/perf/util/config.c | 9 ++++----
> tools/perf/util/util.c | 18 ++++++++++++++++
> tools/perf/util/util.h | 2 ++
> 4 files changed, 74 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 8d78785..a368ead 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -125,6 +125,38 @@ config__module(const char *value, struct perf_probe_event *pev)
> return 0;
> }
>
> +static int
> +config__bool(const char *value,
> + bool *pbool, bool invert)
> +{
> + int err;
> + bool bool_value;
> +
> + if (!pbool)
> + return -EINVAL;
> +
> + err = convert_str_to_bool(value, &bool_value);
> + if (err)
> + return err;
> +
> + *pbool = invert ? !bool_value : bool_value;
> + return 0;
if (!err)
*pbool = invert ? !bool_value : bool_value;
return err;
But again, no strict requirement, just a more compact form :)
> +}
> +
> +static int
> +config__inlines(const char *value,
> + struct perf_probe_event *pev __maybe_unused)
> +{
> + return config__bool(value, &probe_conf.no_inlines, true);
> +}
> +
> +static int
> +config__force(const char *value,
> + struct perf_probe_event *pev __maybe_unused)
> +{
> + return config__bool(value, &probe_conf.force_add, false);
> +}
> +
> static struct {
> const char *key;
> const char *usage;
> @@ -142,7 +174,19 @@ static struct {
> "module=<module name> ",
> "Set kprobe module",
> config__module,
> - }
> + },
> + {
> + "inlines",
> + "inlines=[yes|no] ",
> + "Probe at inline symbol",
> + config__inlines,
> + },
> + {
> + "force",
> + "force=[yes|no] ",
> + "Forcibly add events with existing name",
> + config__force,
> + },
Named initializers for both, please
> };
>
> static int
> @@ -240,6 +284,10 @@ config_bpf_program(struct bpf_program *prog)
> const char *config_str;
> int err;
>
> + /* Initialize per-program probing setting */
> + probe_conf.no_inlines = false;
> + probe_conf.force_add = false;
> +
> config_str = bpf_program__title(prog, false);
> if (IS_ERR(config_str)) {
> pr_debug("bpf: unable to get title for program\n");
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 2e452ac..8219798 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -351,15 +351,16 @@ int perf_config_int(const char *name, const char *value)
>
> static int perf_config_bool_or_int(const char *name, const char *value, int *is_bool)
> {
> + bool str_bool;
> +
> *is_bool = 1;
> if (!value)
> return 1;
> if (!*value)
> return 0;
> - if (!strcasecmp(value, "true") || !strcasecmp(value, "yes") || !strcasecmp(value, "on"))
> - return 1;
> - if (!strcasecmp(value, "false") || !strcasecmp(value, "no") || !strcasecmp(value, "off"))
> - return 0;
> +
> + if (convert_str_to_bool(value, &str_bool) == 0)
> + return str_bool ? 1 : 0;
> *is_bool = 0;
> return perf_config_int(name, value);
> }
> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> index 47b1e36..55785d5 100644
> --- a/tools/perf/util/util.c
> +++ b/tools/perf/util/util.c
> @@ -695,3 +695,21 @@ fetch_kernel_version(unsigned int *puint, char *str,
> *puint = (version << 16) + (patchlevel << 8) + sublevel;
> return 0;
> }
> +
> +int convert_str_to_bool(const char *str, bool *result)
strtobool() should be more compact and convey the same idea....
Hey, I googled for that name and guess what, the kernel has exactly this
function:
lib/string.c
/**
* strtobool - convert common user inputs into boolean values
* @s: input string
* @res: result
*
* This routine returns 0 iff the first character is one of 'Yy1Nn0'.
* Otherwise it will return -EINVAL. Value pointed to by res is
* updated upon finding a match.
*/
include/linux/string.h
So, please add it to tools/include/linux/string.h and
tools/lib/util/string.c, this way we use the same code as the kernel,
with the same function signature, etc.
Also this looks like a good thing to have on a separate patch, one that
introduces strtobool(), then the rest of this patch.
- Arnaldo
> +{
> + if (!result || !str)
> + return -EINVAL;
> +
> + if (!strcasecmp(str, "true") || !strcasecmp(str, "yes") || !strcasecmp(str, "on")) {
> + *result = true;
> + return 0;
> + }
> +
> + if (!strcasecmp(str, "false") || !strcasecmp(str, "no") || !strcasecmp(str, "off")) {
> + *result = false;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index dcc6590..be90932 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -358,4 +358,6 @@ int fetch_kernel_version(unsigned int *puint,
> #define KVER_FMT "%d.%d.%d"
> #define KVER_PARAM(x) KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
>
> +int convert_str_to_bool(const char *str, bool *result);
> +
> #endif /* GIT_COMPAT_UTIL_H */
> --
> 1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists