On Fri, 5 Apr 2019 15:45:42 +0200
Mattias Rönnblom <mattias.ronnb...@ericsson.com> wrote:
> The rte_rand() documentation left it unspecified if the rte_rand() was
> multi-thread safe or not, and the implementation (based on lrand48())
> was not.
>
> This commit makes rte_rand() safe to use from any lcore thread by
> using lrand48_r() and per-lcore random state structs. Besides the
> obvious improvement in terms of correctness (for concurrent users),
> this also much improves rte_rand() performance, since the threads no
> longer shares state. For the single-threaded case, this patch causes
> ~10% rte_rand() performance degradation.
>
> rte_srand() is left multi-thread unsafe, and external synchronization
> is required to serialize rte_sand() calls from different lcore
> threads, and a rte_srand() call with rte_rand() calls made by other
> lcore threads.
>
> The assumption is that the random number generators will be seeded
> once, during startup.
>
> Bugzilla ID: 114
>
> Signed-off-by: Mattias Rönnblom <mattias.ronnb...@ericsson.com>
> ---
> lib/librte_eal/common/include/rte_random.h | 25 ++++-----
> lib/librte_eal/common/meson.build | 1 +
> lib/librte_eal/common/rte_random.c | 65 ++++++++++++++++++++++
> lib/librte_eal/freebsd/eal/Makefile | 1 +
> lib/librte_eal/freebsd/eal/eal.c | 2 -
> lib/librte_eal/linux/eal/Makefile | 1 +
> lib/librte_eal/linux/eal/eal.c | 2 -
> lib/librte_eal/rte_eal_version.map | 2 +
> 8 files changed, 80 insertions(+), 19 deletions(-)
> create mode 100644 lib/librte_eal/common/rte_random.c
>
> diff --git a/lib/librte_eal/common/include/rte_random.h
> b/lib/librte_eal/common/include/rte_random.h
> index b2ca1c209..bca85a672 100644
> --- a/lib/librte_eal/common/include/rte_random.h
> +++ b/lib/librte_eal/common/include/rte_random.h
> @@ -16,7 +16,6 @@ extern "C" {
> #endif
>
> #include <stdint.h>
> -#include <stdlib.h>
>
> /**
> * Seed the pseudo-random generator.
> @@ -25,14 +24,15 @@ extern "C" {
> * value. It may need to be re-seeded by the user with a real random
> * value.
> *
> + * This function is not multi-thread safe in regards to other
> + * rte_srand() calls, nor is it in relation to concurrent rte_rand()
> + * calls.
> + *
> * @param seedval
> * The value of the seed.
> */
> -static inline void
> -rte_srand(uint64_t seedval)
> -{
> - srand48((long)seedval);
> -}
> +void
> +rte_srand(uint64_t seedval);
>
> /**
> * Get a pseudo-random value.
> @@ -41,18 +41,13 @@ rte_srand(uint64_t seedval)
> * congruential algorithm and 48-bit integer arithmetic, called twice
> * to generate a 64-bit value.
> *
> + * If called from lcore threads, this function is thread-safe.
> + *
> * @return
> * A pseudo-random value between 0 and (1<<64)-1.
> */
> -static inline uint64_t
> -rte_rand(void)
> -{
> - uint64_t val;
> - val = (uint64_t)lrand48();
> - val <<= 32;
> - val += (uint64_t)lrand48();
> - return val;
> -}
> +uint64_t
> +rte_rand(void);
>
> #ifdef __cplusplus
> }
> diff --git a/lib/librte_eal/common/meson.build
> b/lib/librte_eal/common/meson.build
> index 0670e4102..bafd23207 100644
> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> @@ -35,6 +35,7 @@ common_sources = files(
> 'rte_keepalive.c',
> 'rte_malloc.c',
> 'rte_option.c',
> + 'rte_random.c',
> 'rte_reciprocal.c',
> 'rte_service.c'
> )
> diff --git a/lib/librte_eal/common/rte_random.c
> b/lib/librte_eal/common/rte_random.c
> new file mode 100644
> index 000000000..9d519d03b
> --- /dev/null
> +++ b/lib/librte_eal/common/rte_random.c
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2014 Intel Corporation
> + * Copyright(c) 2019 Ericsson AB
> + */
> +
> +#include <stdlib.h>
> +
> +#include <rte_cycles.h>
> +#include <rte_eal.h>
> +#include <rte_lcore.h>
> +#include <rte_random.h>
> +
> +struct rte_rand_data
> +{
> + struct drand48_data data;
> +} __rte_cache_aligned;
> +
> +static struct rte_rand_data rand_data[RTE_MAX_LCORE];
> +
> +void
> +rte_srand(uint64_t seedval)
> +{
> + unsigned i;
> +
> + /* give the different lcores a different seed, to avoid a
> + situation where they generate the same sequence */
> + for (i = 0; i < RTE_MAX_LCORE; i++)
> + srand48_r((long)seedval + i, &rand_data[i].data);
> +}
> +
> +static inline uint32_t
> +__rte_rand48(struct drand48_data *data)
> +{
> + long res;
> +
> + lrand48_r(data, &res);
> +
> + return (uint32_t)res;
> +}
> +
> +uint64_t
> +rte_rand(void)
> +{
> + unsigned lcore_id;
> + struct drand48_data *data;
> + uint64_t val;
> +
> + lcore_id = rte_lcore_id();
> +
> + if (unlikely(lcore_id == LCORE_ID_ANY))
> + lcore_id = rte_get_master_lcore();
> +
> + data = &rand_data[lcore_id].data;
> +
> + val = __rte_rand48(data);
> + val <<= 32;
> + val += __rte_rand48(data);
> +
> + return val;
> +}
> +
> +RTE_INIT(rte_rand_init)
> +{
> + rte_srand(rte_get_timer_cycles());
> +}
rand48 is a terrible PRNG, why not use something better?
Similar discussion in Linux kernel pointed at:
http://www.pcg-random.org/posts/some-prng-implementations.html
Mail thread here:
https://www.spinics.net/lists/netdev/msg560231.html
