linux-kernel - Re: [PATCH 05/12] perf tools: Allow BPF program config probing options

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 16 Nov 2015 17:11:27 +0800
From:	"Wangnan (F)" <wangnan0@...wei.com>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>
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



On 2015/11/13 23:46, Arnaldo Carvalho de Melo wrote:
> 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
>> ---

[SNIP]

>> 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.

Good suggestion. One small inconvenience: kernel's strtobool
only support 'yes|no', it doesn't match perf's config, which
requires parsing yes/no/on/off/true/false. In addition, it
accept '0/1' but perf config not.

It is very easy for me to change strtobool() to match them.
However, do you think we can provide a function with same
name with kernel but have different behavior?

However I think we can first clone that function and use it in
BPF without touching perf config, then decide whether we need
to change strtobool() or not.

Thank you.

--
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/