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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 2 Oct 2022 00:50:38 +0200 From: "Jason A. Donenfeld" <Jason@...c4.com> To: Willy Tarreau <w@....eu> Cc: Eric Dumazet <eric.dumazet@...il.com>, "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev <netdev@...r.kernel.org>, Eric Dumazet <edumazet@...gle.com>, Christophe Leroy <christophe.leroy@...roup.eu> Subject: Re: [PATCH net-next] once: add DO_ONCE_SLOW() for sleepable contexts On Sat, Oct 01, 2022 at 11:15:29PM +0200, Willy Tarreau wrote: > Hi Eric, > > On Sat, Oct 01, 2022 at 01:51:02PM -0700, Eric Dumazet wrote: > > From: Eric Dumazet <edumazet@...gle.com> > > > > Christophe Leroy reported a ~80ms latency spike > > happening at first TCP connect() time. > > Seeing Christophe's message also made me wonder if we didn't break > something back then :-/ > > > This is because __inet_hash_connect() uses get_random_once() > > to populate a perturbation table which became quite big > > after commit 4c2c8f03a5ab ("tcp: increase source port perturb table to 2^16") > > > > get_random_once() uses DO_ONCE(), which block hard irqs for the duration > > of the operation. > > > > This patch adds DO_ONCE_SLOW() which uses a mutex instead of a spinlock > > for operations where we prefer to stay in process context. > > That's a nice improvement I think. I was wondering if, for this special > case, we *really* need an exclusive DO_ONCE(). I mean, we're getting > random bytes, we really do not care if two CPUs change them in parallel > provided that none uses them before the table is entirely filled. Thus > that could probably end up as something like: > > if (!atomic_read(&done)) { > get_random_bytes(array); > atomic_set(&done, 1); > } If you don't care about the tables being consistent between CPUs, then yea, sure, that seems like a reasonable approach, and I like not polluting once.{c,h} with some _SLOW() special cases. If you don't want the atomic read in there you could also do the same pattern with a static branch, like what DO_ONCE() does: if (static_branch_unlikely(&need_bytes)) { get_random_bytes(array); static_branch_disable(&need_bytes); } Anyway, same thing as your suggestion more or less. Jason
Powered by blists - more mailing lists