Re: [dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Apr 16, 2020 at 5:30 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-16 13:48, Jerin Jacob wrote:
> > On Thu, Apr 16, 2020 at 5:06 PM Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com> wrote:
> >> On 2020-04-16 08:07, Jerin Jacob wrote:
> >>> On Thu, Apr 16, 2020 at 4:42 AM Dan Gora <dg@adax.com> wrote:
> >>>> Instead of choosing to use getentropy() or the rdseed instruction for
> >>>> the random number generator entropy source using compilation flags,
> >>>> determine the best source at run time.
> >>>>
> >>>> This is accomplished by defining a weak symbol for getentropy(),
> >>>> checking that the compiler can generate the rdseed instruction even if
> >>>> the compilation platform does not natively support it, and checking for
> >>>> the availability of the rdseed instruction on the execution platform
> >>>> using rte_cpu_get_flag_enabled().
> >>>>
> >>>> If neither getentropy() or rdseed is available, rte_get_timer_cycles()
> >>>> will be continue to be used as the entropy source.
> >>>>
> >>>> This also allows non-Mason builds to use getentropy().
> >>>>
> >>>> Signed-off-by: Dan Gora <dg@adax.com>
> >>>> ---
> >>>>    config/x86/meson.build             |  7 +++++++
> >>>>    lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
> >>>>    lib/librte_eal/meson.build         |  3 ---
> >>>>    mk/rte.cpuflags.mk                 |  8 ++++++++
> >>>>    4 files changed, 39 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/config/x86/meson.build b/config/x86/meson.build
> >>>> index adc857ba2..214b16f2a 100644
> >>>> --- a/config/x86/meson.build
> >>>> +++ b/config/x86/meson.build
> >>>> @@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
> >>>>           machine_args += '-msse4'
> >>>>    endif
> >>>>
> >>>> +# set -mrdseed if necessary so _rdseed32_step compiles if the
> >>>> +# compilation host does not support the RDSEED instruction.
> >>>> +if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
> >>>> +       machine_args += '-mrdseed'
> >>>> +       message('RDSEED not enabled by default, explicitly setting -mrdseed')
> >>>> +endif
> >>>> +
> >>>>    base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
> >>>>    foreach f:base_flags
> >>>>           dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
> >>>> diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
> >>>> index 57ec8fb2b..40f8b5aab 100644
> >>>> --- a/lib/librte_eal/common/rte_random.c
> >>>> +++ b/lib/librte_eal/common/rte_random.c
> >>>> @@ -25,6 +25,8 @@ struct rte_rand_state {
> >>>>
> >>>>    static struct rte_rand_state rand_states[RTE_MAX_LCORE];
> >>>>
> >>>> +__rte_weak int getentropy(void *__buffer, size_t __length);
> >>>> +
> >>>>    static uint32_t
> >>>>    __rte_rand_lcg32(uint32_t *seed)
> >>>>    {
> >>>> @@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
> >>>>           return res;
> >>>>    }
> >>>>
> >>>> +/* Use rte_get_timer_cycles() if the system does not have
> >>>> + * genentropy() or the rdseed instruction.
> >>>> + */
> >>>> +__rte_weak int
> >>>> +getentropy(void *__buffer, size_t __length __rte_unused)
> >>>> +{
> >>>> +       uint64_t *ge_seed = __buffer;
> >>>> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
> >>>> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
> >>>> +               return -1;
> >>>> +#endif
> >>>> +       *ge_seed = rte_get_timer_cycles();
> >>>> +       return 0;
> >>> IMO, we need to create a new arch EAL abstraction to get uint64_t
> >>> random number. Reason being:
> >>> 1) ARMv8.5 supports similar instruction
> >>> https://developer.arm.com/docs/ddi0595/c/aarch64-system-registers/rndr
> >>
> >> How many ARMv8.5 system will there be running a libc older than 2.25 or
> >> a Linux kernel older than 3.17?
> > Probably none.
>
>
> In that case getentropy() will always available, and having ARMv8.5 rndr
> support would be pointless, as far as rte_random.c is concerned.

Yes. My initial comment was based on this patch context.
I just checked the lib/librte_eal/common/rte_random.c,
if this get activated only libc older than 2.25 or kernel < 3.17, it
is not a concern for me.


>
> >>
> >>> 2) Avoid #ifdef clutter in common code.
> >>>
> >>> Abstraction can return a random uint64_t number. Based on the arch
> >>> capabilities, it can hook
> >>> to specialized instruction or rte_get_timer_cycles()
>
>