[v4,3/5] eal: improve entropy for initial PRNG seed

Message ID 20190628090124.16849-4-mattias.ronnblom@ericsson.com (mailing list archive)
State Accepted, archived
Headers
Series Pseudo-random number generation improvements |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Mattias Rönnblom June 28, 2019, 9:01 a.m. UTC
  Replace the use of rte_get_timer_cycles() with getentropy() for
seeding the pseudo-random number generator. getentropy() provides a
more truly random value.

getentropy() requires glibc 2.25 and Linux kernel 3.17. In case
getentropy() is not found at compile time, or the relevant syscall
fails in runtime, the rdseed machine instruction will be used as a
fallback.

rdseed is only available on x86 (Broadwell or later). In case it is
not present, rte_get_timer_cycles() will be used as a second fallback.

On non-Meson builds, getentropy() will not be used.

Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/common/rte_random.c | 36 +++++++++++++++++++++++++++++-
 lib/librte_eal/meson.build         |  3 +++
 2 files changed, 38 insertions(+), 1 deletion(-)
  

Comments

Ferruh Yigit June 28, 2019, 7:01 p.m. UTC | #1
On 6/28/2019 10:01 AM, Mattias Rönnblom wrote:
> Replace the use of rte_get_timer_cycles() with getentropy() for
> seeding the pseudo-random number generator. getentropy() provides a
> more truly random value.
> 
> getentropy() requires glibc 2.25 and Linux kernel 3.17. In case
> getentropy() is not found at compile time, or the relevant syscall
> fails in runtime, the rdseed machine instruction will be used as a
> fallback.
> 
> rdseed is only available on x86 (Broadwell or later). In case it is
> not present, rte_get_timer_cycles() will be used as a second fallback.
> 
> On non-Meson builds, getentropy() will not be used.
> 
> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

<...>

> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
> +	unsigned int rdseed_rc;
> +	unsigned long long rdseed_seed;
> +
> +	/* first fallback: rdseed instruction, if available */
> +	rdseed_rc = _rdseed64_step(&rdseed_seed);

This is causing build error for 32-bit [1] and ICC [2], can you please check?

[1]
.../dpdk/lib/librte_eal/common/rte_random.c: In function
‘__rte_random_initial_seed’:
.../dpdk/lib/librte_eal/common/rte_random.c:196:14: error: implicit declaration
of function ‘_rdseed64_step’; did you mean ‘_rdseed32_step’?
[-Werror=implicit-function-declaration]
  196 |  rdseed_rc = _rdseed64_step(&rdseed_seed);
      |              ^~~~~~~~~~~~~~
      |              _rdseed32_step
.../dpdk/lib/librte_eal/common/rte_random.c:196:14: error: nested extern
declaration of ‘_rdseed64_step’ [-Werror=nested-externs]



[2]
Building x86_64-native-linuxapp-icc ...
.../dpdk/lib/librte_eal/common/rte_random.c(196): error #167: argument of type
"unsigned long long *" is incompatible with parameter of type "unsigned long *"
        rdseed_rc = _rdseed64_step(&rdseed_seed);
                                   ^
  
Mattias Rönnblom June 28, 2019, 8:58 p.m. UTC | #2
On 2019-06-28 21:01, Ferruh Yigit wrote:
> On 6/28/2019 10:01 AM, Mattias Rönnblom wrote:
>> Replace the use of rte_get_timer_cycles() with getentropy() for
>> seeding the pseudo-random number generator. getentropy() provides a
>> more truly random value.
>>
>> getentropy() requires glibc 2.25 and Linux kernel 3.17. In case
>> getentropy() is not found at compile time, or the relevant syscall
>> fails in runtime, the rdseed machine instruction will be used as a
>> fallback.
>>
>> rdseed is only available on x86 (Broadwell or later). In case it is
>> not present, rte_get_timer_cycles() will be used as a second fallback.
>>
>> On non-Meson builds, getentropy() will not be used.
>>
>> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
>> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> <...>
> 
>> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
>> +	unsigned int rdseed_rc;
>> +	unsigned long long rdseed_seed;
>> +
>> +	/* first fallback: rdseed instruction, if available */
>> +	rdseed_rc = _rdseed64_step(&rdseed_seed);
> 
> This is causing build error for 32-bit [1] and ICC [2], can you please check?
> 
> [1]
> .../dpdk/lib/librte_eal/common/rte_random.c: In function
> ‘__rte_random_initial_seed’:
> .../dpdk/lib/librte_eal/common/rte_random.c:196:14: error: implicit declaration
> of function ‘_rdseed64_step’; did you mean ‘_rdseed32_step’?
> [-Werror=implicit-function-declaration]
>    196 |  rdseed_rc = _rdseed64_step(&rdseed_seed);
>        |              ^~~~~~~~~~~~~~
>        |              _rdseed32_step
> .../dpdk/lib/librte_eal/common/rte_random.c:196:14: error: nested extern
> declaration of ‘_rdseed64_step’ [-Werror=nested-externs]
> 
> 

My suggestion would be to just replace _rdseed64_step() with two 
_rdseed32_step() calls on both 32-bit and 64-bit x86. I'll submit a patch.

> 
> [2]
> Building x86_64-native-linuxapp-icc ...
> .../dpdk/lib/librte_eal/common/rte_random.c(196): error #167: argument of type
> "unsigned long long *" is incompatible with parameter of type "unsigned long *"
>          rdseed_rc = _rdseed64_step(&rdseed_seed);
>                                     ^

I'll fix this as well.

Thanks.
  

Patch

diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
index 4d3cf5226..e53d96d18 100644
--- a/lib/librte_eal/common/rte_random.c
+++ b/lib/librte_eal/common/rte_random.c
@@ -2,7 +2,11 @@ 
  * Copyright(c) 2019 Ericsson AB
  */
 
+#ifdef RTE_MACHINE_CPUFLAG_RDSEED
+#include <x86intrin.h>
+#endif
 #include <stdlib.h>
+#include <unistd.h>
 
 #include <rte_branch_prediction.h>
 #include <rte_cycles.h>
@@ -133,7 +137,37 @@  rte_rand(void)
 	return __rte_rand_lfsr258(state);
 }
 
+static uint64_t
+__rte_random_initial_seed(void)
+{
+#ifdef RTE_LIBEAL_USE_GETENTROPY
+	int ge_rc;
+	uint64_t ge_seed;
+
+	ge_rc = getentropy(&ge_seed, sizeof(ge_seed));
+
+	if (ge_rc == 0)
+		return ge_seed;
+#endif
+#ifdef RTE_MACHINE_CPUFLAG_RDSEED
+	unsigned int rdseed_rc;
+	unsigned long long rdseed_seed;
+
+	/* first fallback: rdseed instruction, if available */
+	rdseed_rc = _rdseed64_step(&rdseed_seed);
+
+	if (rdseed_rc == 1)
+		return (uint64_t)rdseed_seed;
+#endif
+	/* second fallback: seed using rdtsc */
+	return rte_get_timer_cycles();
+}
+
 RTE_INIT(rte_rand_init)
 {
-	rte_srand(rte_get_timer_cycles());
+	uint64_t seed;
+
+	seed = __rte_random_initial_seed();
+
+	rte_srand(seed);
 }
diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
index fa36b20e0..ccd5b85b8 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -18,6 +18,9 @@  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
 sources = common_sources + env_sources
 objs = common_objs + env_objs
 headers = common_headers + env_headers