From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id 19DE8A0679
	for <public@inbox.dpdk.org>; Fri,  5 Apr 2019 18:57:47 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 26F791B572;
	Fri,  5 Apr 2019 18:57:44 +0200 (CEST)
Received: from mail-pf1-f193.google.com (mail-pf1-f193.google.com
 [209.85.210.193]) by dpdk.org (Postfix) with ESMTP id 737A31B56B
 for <dev@dpdk.org>; Fri,  5 Apr 2019 18:57:42 +0200 (CEST)
Received: by mail-pf1-f193.google.com with SMTP id 10so3600182pfo.5
 for <dev@dpdk.org>; Fri, 05 Apr 2019 09:57:42 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=networkplumber-org.20150623.gappssmtp.com; s=20150623;
 h=date:from:to:cc:subject:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding;
 bh=O27mfYd8rsuduxVRC+rpmXS1f8sZvtYKOrZs71Ey8/g=;
 b=PViXlmKUKyfq6+toCYW0cCLxvLcwI0bHKupKAo0J4gVuzpLx4FvEGGS0B4pGtJaKEL
 pbQ3CUWQIzwpTo8VvzbMBMPePS0FsntkEExHg26uqnTFLS+usNMXfkRm/UQ1tHKj8qil
 etB9Tslop7cFIVArHcGtI+qTpNbBp7AGHNrYw3rq82bCwKkFv8+fYvz6sSENf2vTKn81
 VMI5YJIR6Vb6VCxeuv4dxTagNV89ofeFWWoH8dc/K5fEyl+aGLno50CHkt6ZJz5nbgvE
 pm9Qdzn9rBVIc+4WG8etJMQjjwqGlqnb9hsKRRiPyJ0CDvV6Cwz4/w6U6SAPWmHw80By
 MWoQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to
 :references:mime-version:content-transfer-encoding;
 bh=O27mfYd8rsuduxVRC+rpmXS1f8sZvtYKOrZs71Ey8/g=;
 b=W26nQNMN8CO8wmNAPuKvDSQ7ZdBgHkDgSqoTlP7GOhGl+YKvEFvB5HMdAE48LcG9Yr
 6C0y7WNtxMedHT7sGteYjZ/iuVqJixr9n50mIW+QuiLsWFO8/PpsNH7s81MachOR28Wv
 ASB4xRJJ9b0SHWi0ND2l2kHsvLPtIXWfOlL5jVQsNNWqoSBaqM4Lk0ddinjMU+zDJn9Z
 v9TYOUoBOxmSjaG3VoKhfI0av7VveahPIJcACs1e3DiXIMMeYjrDEz71jtG9+3W5qbDd
 XJWLtXhK4n5R3pc2NNrj6gCVyaGum1vGa52xgjt+9dvvuw2jnm7ym+Vg4E7SjYyIrcQ3
 /b/g==
X-Gm-Message-State: APjAAAWUJvWgcj0S165+uTZ//gTuGhM8njWfTWNhVBJ7iOVvSNV46cPr
 xHamWIUaG1icLbagHoiXgc/SSg==
X-Google-Smtp-Source: APXvYqxGN45yn7iMLjXCuHRerPPxPcbWVmtVWrboOmAp613Q0pqtI7Y5fYt0zkZh8tUSJWWacBgneQ==
X-Received: by 2002:a63:5b0a:: with SMTP id p10mr13284741pgb.282.1554483461340; 
 Fri, 05 Apr 2019 09:57:41 -0700 (PDT)
Received: from shemminger-XPS-13-9360 ([167.220.56.255])
 by smtp.gmail.com with ESMTPSA id e123sm35587869pgc.14.2019.04.05.09.57.41
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Fri, 05 Apr 2019 09:57:41 -0700 (PDT)
Date: Fri, 5 Apr 2019 09:57:38 -0700
From: Stephen Hemminger <stephen@networkplumber.org>
To: Mattias =?UTF-8?B?UsO2bm5ibG9t?= <mattias.ronnblom@ericsson.com>
Cc: <dev@dpdk.org>
Message-ID: <20190405095738.145a622d@shemminger-XPS-13-9360>
In-Reply-To: <20190405134542.28618-1-mattias.ronnblom@ericsson.com>
References: <20190405134542.28618-1-mattias.ronnblom@ericsson.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [dpdk-dev] [RFC] eal: make rte_rand() MT safe
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190405165738.Jd-oBpE0QIJO_PEK7P2067yLWLX6Mzs8-bfnCE-TQQo@z>

On Fri, 5 Apr 2019 15:45:42 +0200
Mattias R=C3=B6nnblom <mattias.ronnblom@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.
>=20
> 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.
>=20
> 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.
>=20
> The assumption is that the random number generators will be seeded
> once, during startup.
>=20
> Bugzilla ID: 114
>=20
> Signed-off-by: Mattias R=C3=B6nnblom <mattias.ronnblom@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
>=20
> 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
> =20
>  #include <stdint.h>
> -#include <stdlib.h>
> =20
>  /**
>   * 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);
> =20
>  /**
>   * 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 =3D (uint64_t)lrand48();
> -	val <<=3D 32;
> -	val +=3D (uint64_t)lrand48();
> -	return val;
> -}
> +uint64_t
> +rte_rand(void);
> =20
>  #ifdef __cplusplus
>  }
> diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/me=
son.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 =3D 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/r=
te_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 =3D 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 =3D rte_lcore_id();
> +
> +	if (unlikely(lcore_id =3D=3D LCORE_ID_ANY))
> +		lcore_id =3D rte_get_master_lcore();
> +
> +	data =3D &rand_data[lcore_id].data;
> +
> +	val =3D __rte_rand48(data);
> +	val <<=3D 32;
> +	val +=3D __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