netdev - Re: [PATCH net-next 2/9] bpf: introduce percpu

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 7 Mar 2016 10:26:38 -0800
From:	Alexei Starovoitov <ast@...com>
To:	Daniel Borkmann <daniel@...earbox.net>,
	"David S . Miller" <davem@...emloft.net>
CC:	Daniel Wagner <daniel.wagner@...-carit.de>,
	Tom Zanussi <tom.zanussi@...ux.intel.com>,
	Wang Nan <wangnan0@...wei.com>, He Kuang <hekuang@...wei.com>,
	Martin KaFai Lau <kafai@...com>,
	Brendan Gregg <brendan.d.gregg@...il.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<kernel-team@...com>
Subject: Re: [PATCH net-next 2/9] bpf: introduce percpu_freelist

On 3/7/16 2:33 AM, Daniel Borkmann wrote:
> On 03/07/2016 02:58 AM, Alexei Starovoitov wrote:
>> Introduce simple percpu_freelist to keep single list of elements
>> spread across per-cpu singly linked lists.
>>
>> /* push element into the list */
>> void pcpu_freelist_push(struct pcpu_freelist *, struct
>> pcpu_freelist_node *);
>>
>> /* pop element from the list */
>> struct pcpu_freelist_node *pcpu_freelist_pop(struct pcpu_freelist *);
>>
>> The object is pushed to the current cpu list.
>> Pop first trying to get the object from the current cpu list,
>> if it's empty goes to the neigbour cpu list.
>>
>> For bpf program usage pattern the collision rate is very low,
>> since programs push and pop the objects typically on the same cpu.
>>
>> Signed-off-by: Alexei Starovoitov <ast@...nel.org>
>
> These bits and their usage in combination with preallocation of objects
> in patch 3/9 look very useful to me!
>
> This code seems generic enough and doesn't contain any BPF specifics,
> other subsystems could potentially utilize it as well, I'd suggest that
> this should better be placed under lib/ so it's exposed/visible for other
> developers too.

I thought about that but the code is bpf usage pattern specific.
It only looks generic. If it's in the lib/ the patches will start
piling in to make it actually generic and adding features that not
only may slow it down, but will risk breaking bpf usage in subtle ways.
It being in kernel/bpf/ is a clear sign that it's for bpf maps only.
When it's in lib/ it's like an invitation to use it for something else
which is the opposite of the trade-offs made here. It's specific to
bpf usage pattern. That's why it performs better than all other
alternatives I've tried.