linux-kernel - Re: [RFC 2/2] perf-stat: enable counting events for BPF programs

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]

Message-ID: <3331CA57-F1EE-4C66-8E79-E5A7DA91F880@fb.com>
Date:   Tue, 24 Nov 2020 23:43:40 +0000
From:   Song Liu <songliubraving@...com>
To:     Jiri Olsa <jolsa@...hat.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "acme@...nel.org" <acme@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "alexander.shishkin@...ux.intel.com" 
        <alexander.shishkin@...ux.intel.com>,
        "namhyung@...nel.org" <namhyung@...nel.org>
Subject: Re: [RFC 2/2] perf-stat: enable counting events for BPF programs



> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <jolsa@...hat.com> wrote:
> 
> On Wed, Nov 18, 2020 at 08:50:46PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> +static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
>> +					    int fd)
>> +{
>> +	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
>> +
>> +	return bpf_map_update_elem(bpf_map__fd(skel->maps.events),
>> +				   &cpu, &fd, BPF_ANY);
>> +}
>> +
>> +struct bpf_counter_ops bpf_program_profiler_ops = {
>> +	.load       = bpf_program_profiler__load,
>> +	.enable	    = bpf_program_profiler__enable,
>> +	.read       = bpf_program_profiler__read,
>> +	.destroy    = bpf_program_profiler__destroy,
>> +	.install_pe = bpf_program_profiler__install_pe,
>> +};
> 
> hum, what's the point of this ops? you plan some other ops?
> we could just define stat callbacks right?

I do have other ideas using BPF program to aggregate perf event 
counts. This ops provides common interface for different BPF 
extensions to evsel. 

> 
>> +SEC("fentry/XXX")
>> +int BPF_PROG(fentry_XXX)
>> +{
>> +	u32 key = bpf_get_smp_processor_id();
>> +	struct bpf_perf_event_value reading;
>> +	struct bpf_perf_event_value *ptr;
>> +	u32 zero = 0;
>> +	long err;
>> +
>> +	/* look up before reading, to reduce error */
>> +	ptr = bpf_map_lookup_elem(&fentry_readings, &zero);
>> +	if (!ptr)
>> +		return 0;
>> +
>> +	err = bpf_perf_event_read_value(&events, key, &reading,
>> +					sizeof(reading));
>> +	if (err)
>> +		return 0;
>> +
>> +	*ptr = reading;
>> +	return 0;
>> +}
> 
> so currently it's one extra bpf program for each event,
> but it seems bad for multiple events stat.. could we
> just have one program that would read and process all events?

Multiple fentry programs should not be too expensive. Current design
extends evsel, so it is a cleaner implementation. We can evaluate the 
difference of these two designs by comparing this with 
"bpftool prog profile".

Thanks,
Song