Re: [PATCH v1 1/2] random-bits: Factor out entropy generating function

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Mon, Apr 4, 2022 at 2:20 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 04/04/2022 15:52, Noah Goldstein wrote:
> > On Mon, Apr 4, 2022 at 1:38 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 04/04/2022 15:23, Noah Goldstein wrote:
> >>> On Mon, Apr 4, 2022 at 12:42 PM Adhemerval Zanella
> >>> <adhemerval.zanella@linaro.org> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 01/04/2022 15:01, Cristian Rodríguez wrote:
> >>>>> On Thu, Mar 31, 2022 at 8:05 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >>>>>
> >>>>>> AFAIK our goal is entropy more so than security. For example
> >>>>>> if this is used to generate jiffies to stagger threads its not a security
> >>>>>> issue in any sense, it's just not ideal for performance.
> >>>>>
> >>>>> In any case this should be more than fast enough for the other use
> >>>>> cases of random_bits() .. maybe one new random_bits_fast() function
> >>>>> foe edge cases where even this is too slow?
> >>>>
> >>>> I think we are bike-shedding in the same issue OpenBSD guys stumbled
> >>>> and which they have solved 10 years ago [1].  Essentially, we need to
> >>>> come  up with a internal PRNG interface that can be used internally and
> >>>> externally, instead of reinventing cleaver ways to use the timer as
> >>>> entropy source.
> >>>>
> >>>> The issue is not really the cypher used, ideally it could be replace if
> >>>> we find out that it does not fit.  The main issue is to glue together
> >>>> all the requirements to have a concise internal interface, taking in
> >>>> consideration the glibc constrains to work with multiple kernel version
> >>>> and environments (where we can't assume we have access to a source or
> >>>> reliable entropy like getrandom syscall).
> >>>>
> >>>> My plan to rehearse Florian arc4random proposal to have some simpler
> >>>> to where we might improve upon (a simpler fork detection for kernels
> >>>> without MADV_WIPEONFORK that just issue a atfork handle, maybe using
> >>>> ChaCha20 as virtually all other systems do, no per-thread state).
> >>>
> >>> Why no per-thread / per-cpu? It seems otherwise there will need to be
> >>> some explicit synchronization on the stream.
> >>
> >> Mainly because it simplifies a lot the *initial* implementation. I would
> >> prefer to incremental add per-thread optimization than dump a large patch
> >> so we can review in integrate the code more easier.
> >>
> >>>
> >>>
> >>> Regarding this patch, do we want to skip it and just wait on arc4random
> >>> interface in kernel/glibc or should I go forward with it and some arch
> >>> specific entropy functions in the mean-time?
> >>
> >> I don't have a strong opinion on this patch, it does improve x86_64
> >> latency on random_bits although currently internal usage are far from
> >> latency sensitive.  It is really a microoptimization without much real
> >> work gain for current code.
> >
> > That's fair. The motivation is so random_bits can be used for lightweight
> > jitter i.e in cases like:
> >
> > [v2] nptl: Add backoff mechanism to spinlock loop
> >
> > The x86 backend with `rdtsc` is just because it's a simple improvement,
> > other arch will hopefully be able to get off syscall if they don't have vdso
> > gettime.
> > But agree the improvement gains a minimal so if people don't feel its worth
> > the added complexity we can wait on a strong arch-random interface.
>
> I think for mutex optimization it would be better to just add a arch-specific
> jitter code and use the backoff optimization iff the arch-specific code is
> used. And maybe not tied to random_bits(), since I am not sure if an interface
> like arc4random would be good to use in such scenario (since a possible state
> reschedule might call getentropy with my add unexpected latency).

So you propose drop this patch in favor of some new internal interface like
get_fast_jitter() and a define such as 'HAS_FAST_JITTER'?