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]

Date:   Wed, 25 Nov 2020 00:02:13 +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 3:43 PM, Song Liu <songliubraving@...com> wrote:
> 
> 
> 
>> 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?

Which callbacks do you mean here? I would like to try that as
well. 

Thanks,
Song

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