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

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

On 2020-04-17 01:50, Dan Gora wrote:
> On Thu, Apr 16, 2020 at 8:30 AM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>>> diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
>>> index 0267c3b9d..748359b8c 100644
>>> --- a/lib/librte_eal/meson.build
>>> +++ b/lib/librte_eal/meson.build
>>> @@ -15,9 +15,6 @@ deps += 'kvargs'
>>>    if dpdk_conf.has('RTE_USE_LIBBSD')
>>>        ext_deps += libbsd
>>>    endif
>>> -if cc.has_function('getentropy', prefix : '#include <unistd.h>')
>>> -     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
>>> -endif
>>>    if cc.has_header('getopt.h')
>>>        cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG']
>>>    endif
>>> diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
>>> index fa8753531..fb7bf8a53 100644
>>> --- a/mk/rte.cpuflags.mk
>>> +++ b/mk/rte.cpuflags.mk
>>> @@ -53,6 +53,14 @@ endif
>>>
>>>    ifneq ($(filter $(AUTO_CPUFLAGS),__RDSEED__),)
>>>    CPUFLAGS += RDSEED
>>> +else
>>> +# If the native environment doesn't define __RDSEED__, see
>>> +# if the compiler supports -mrdseed.
>>
>> For which environments is this true?
> If you compile on a machine which does not have the RDSEED cpu flag,
> gcc will not define __RDSEED__ nor compile the _rdseed32_step()
> functions unless you explicitly set -mrdseed on the gcc/clang command
> line..
>
> We have a HP DL380 machine which do not have rdseed:
> model name      : Intel(R) Xeon(R) CPU E5-2630 v3 @ 2.40GHz
>
> And another which does:
> model name      : Intel(R) Xeon(R) CPU E5-2640 v4 @ 2.40GHz
>
> _rdseed32_step() will only compile on the v3 if we explicitly set -mrdseed.
>
>>> +RDSEED_CPUFLAGS := $(shell $(CC) $(MACHINE_CFLAGS) $(WERROR_FLAGS) $(EXTRA_CFLAGS) -mrdseed -dM -E - < /dev/null)
>>> +ifneq ($(filter $(RDSEED_CPUFLAGS),__RDSEED__),)
>>
>> There are no better ways to achieve this? It seems like a bit of a hack.
> eh.. I don't know of any really.. It's the same command that is used
> to set AUTO_CPUFLAGS a few lines before, just with -mrdseed set.  It
> doesn't seem that bad to my mind, but if there is a better way, I'm
> open to suggestions.
>
> That said, this patch does not work.  We cannot set
> RTE_MACHINE_CPUFLAG_RDSEED because rte_eal_init() will fail on a
> machine which does not have rdseed because rte_cpu_is_supported()
> checks all of these cpu flags set at compile time.  Since we're
> detecting this flag at run time, we have to remove it from this list
> of "required" CPU flags.
>
> I'm working on a new patch.. I should have something tomorrow.
>
> There is also still a problem with getting getentropy() to work when
> the binary is compiled on a system with a glibc < 2.25 but run on a
> system with glibc >= 2.25.  It doesn't resolve the weak symbol to the
> glibc version.. I think that it's because getentropy() is versioned
> within glibc.  I'm still working on how to fix this.. It might come
> down to using dlopen(), dlsym()... :(


One way would be to include a getrandom syscall wrapper, instead of 
going via libc, in case libc is too old.


> I'm not really sure if it's worth it.


I personally don't think having high-quality initial, automatic, seeding 
is important-enough to warrant much complexity.


> thanks
> dan