netdev - Re: [RFC perf,bpf 5/5] perf util: generate bpf_prog_info

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: <27fc8327-3390-ba5a-6063-89c9e7165e7b@fb.com>
Date:   Tue, 6 Nov 2018 23:29:07 +0000
From:   Alexei Starovoitov <ast@...com>
To:     Song Liu <songliubraving@...com>, David Ahern <dsahern@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "acme@...nel.org" <acme@...nel.org>
Subject: Re: [RFC perf,bpf 5/5] perf util: generate bpf_prog_info_event for
 short living bpf programs

On 11/6/18 3:17 PM, Song Liu wrote:
> 
> 
>> On Nov 6, 2018, at 1:54 PM, David Ahern <dsahern@...il.com> wrote:
>>
>> On 11/6/18 1:52 PM, Song Liu wrote:
>>> +
>>> static int record__mmap_read_all(struct record *rec)
>>> {
>>> 	int err;
>>>
>>> +	err = record__mmap_process_vip_events(rec);
>>> +	if (err)
>>> +		return err;
>>> +
>>> 	err = record__mmap_read_evlist(rec, rec->evlist, false);
>>> 	if (err)
>>> 		return err;
>>
>> Seems to me that is going to increase the overhead of perf on any system
>> doing BPF updates. The BPF events cause a wakeup every load and unload,
>> and perf processes not only the VIP events but then walks all of the
>> other maps.
> 
> BPF prog load/unload events should be rare events in real world use cases.
> So I think the overhead is OK. Also, I don't see an easy way to improve
> this.
> 
>>
>>> @@ -1686,6 +1734,8 @@ static struct option __record_options[] = {
>>> 			  "signal"),
>>> 	OPT_BOOLEAN(0, "dry-run", &dry_run,
>>> 		    "Parse options then exit"),
>>> +	OPT_BOOLEAN(0, "no-bpf-event", &record.no_bpf_event,
>>> +		    "do not record event on bpf program load/unload"),
>>
>> Why should this default on? If am recording FIB events, I don't care
>> about BPF events.
>>
> 
> I am OK with default off if that's the preferred way.

I think concerns with perf overhead from collecting bpf events
are unfounded.
I would prefer for this flag to be on by default.