[PATCH 00/18] random number generator improvements

[PATCH 00/18] random number generator improvements

[Stephen Hemminger]

DPDK random number generator has some issues that are worth addressing
in this release. There are seeding related issues, and the current
random number generator is weak and seed could be derived.

The seeding issue is caused by build detection of getentropy()
not working; which results in use of TSC on many systems.
The fix is to always use getentropy().

The security sensitive values (RSS and Toeplitz hash keys,
the IPsec SAD hash seed, an IPsec salt and random MAC addresses)
were generated with rte_rand(), whose internal state can be recovered
from a handful of outputs. The fix is to introduce a DPDK
wrapper around the more secure kernel getrandom() API.

Also found that the sched RED was rolling its own very
simplistic PRNG. The fix is to just use existing rte_rand().
This does drop an inline function in header (rte_fast_rand)
which should never have been exposed.

For Windows build, shims for both getentropy() and getrandom()
are provided.

A couple of other improvements to rte_rand:
  - seeding uses the whole 64 bits passed to rte_srand().
  - helper function for when only 32 bits are needed rte_rand32().
  - test infrastructure now supports seeding for repeatability.
  - unit tests for rte_random_bytes() and rte_rand32().

Stephen Hemminger (18):
  eal/windows: add getentropy shim
  random: always use getentropy
  eal: add 32 bit random number function
  sched: fix data race in RED random number generation
  eal/windows: add getrandom shim
  eal: add function to get random bytes
  hash: use secure random for Toeplitz hash key
  pipeline: use secure random for IPsec salt
  net: use secure random for random MAC address
  ipsec: use secure random for SAD hash seed
  net/intel: use secure random for default RSS key
  random: use all 64 bits of the seed
  drivers/net: use secure random for RSS key
  drivers/net: use common random MAC address helper
  random: remove RDSEED fallback for initial seed
  test: allow setting random seed for tests
  test: add random number generator tests
  doc: recommend secure random for unpredictable values

 AGENTS.md                              |   6 +-
 app/test/meson.build                   |   1 +
 app/test/test.c                        |  48 ++++
 app/test/test_random.c                 | 297 +++++++++++++++++++++++++
 doc/guides/contributing/unit_test.rst  |  15 ++
 doc/guides/rel_notes/release_26_11.rst |  29 ++-
 drivers/net/axgbe/axgbe_dev.c          |  16 +-
 drivers/net/bnx2x/bnx2x.c              |  11 +-
 drivers/net/enetc/enetc4_vf.c          |  22 +-
 drivers/net/enetfec/enet_ethdev.c      |  16 +-
 drivers/net/intel/cpfl/cpfl_ethdev.c   |   5 +-
 drivers/net/intel/iavf/iavf_ethdev.c   |   8 +-
 drivers/net/intel/ice/ice_dcf.c        |  10 +-
 drivers/net/intel/ice/ice_ethdev.c     |  11 +-
 drivers/net/intel/idpf/idpf_ethdev.c   |   5 +-
 lib/eal/common/rte_random.c            | 148 +++++++-----
 lib/eal/include/rte_random.h           |  89 ++++++--
 lib/eal/meson.build                    |   3 -
 lib/eal/windows/include/rte_os_shim.h  |  64 ++++++
 lib/eal/windows/meson.build            |   6 +-
 lib/hash/rte_thash.c                   |  46 +++-
 lib/hash/rte_thash.h                   |   4 +-
 lib/ipsec/ipsec_sad.c                  |   7 +-
 lib/net/rte_ether.c                    |  13 +-
 lib/net/rte_ether.h                    |   5 +
 lib/pipeline/rte_swx_ipsec.c           |   3 +-
 lib/sched/rte_red.c                    |   9 +-
 lib/sched/rte_red.h                    |  26 +--
 28 files changed, 753 insertions(+), 170 deletions(-)
 create mode 100644 app/test/test_random.c

-- 
2.53.0

[PATCH 01/18] eal/windows: add getentropy shim

[Stephen Hemminger]

Provide getentropy() in the Windows OS shim using BCryptGenRandom()
with the system preferred generator, which needs no algorithm
provider handle. The POSIX 256 byte limit is kept so that callers
see the same behaviour on all platforms.

This is needed EAL change that always uses getentropy().

The next patch is tagged for stable, and it does not build on
Windows without this shim, so this one is tagged as well.

Bugzilla ID: 2035
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/rte_random.c           |  1 +
 lib/eal/windows/include/rte_os_shim.h | 30 +++++++++++++++++++++++++++
 lib/eal/windows/meson.build           |  6 +++++-
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 576a32a46c..537fa035ee 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -17,6 +17,7 @@
 #include <rte_random.h>
 
 #include <eal_export.h>
+#include <rte_os_shim.h>
 #include "eal_private.h"
 
 struct __rte_cache_aligned rte_rand_state {
diff --git a/lib/eal/windows/include/rte_os_shim.h b/lib/eal/windows/include/rte_os_shim.h
index 44664a5062..4f9611d642 100644
--- a/lib/eal/windows/include/rte_os_shim.h
+++ b/lib/eal/windows/include/rte_os_shim.h
@@ -3,12 +3,16 @@
 #ifndef _RTE_OS_SHIM_
 #define _RTE_OS_SHIM_
 
+#include <errno.h>
 #include <time.h>
 
 #include <rte_os.h>
 #include <rte_windows.h>
 #include <getline.h>
 
+/* Needs Windows SDK types, so must come after rte_windows.h. */
+#include <bcrypt.h>
+
 /**
  * @file
  * @internal
@@ -124,4 +128,30 @@ rte_localtime_r(const time_t *timep, struct tm *result)
 }
 #define localtime_r(timep, result) rte_localtime_r(timep, result)
 
+/*
+ * Windows has no getentropy(), use the system preferred random
+ * generator which does not require a provider handle.
+ */
+static inline int
+rte_getentropy(void *buffer, size_t length)
+{
+	NTSTATUS status;
+
+	/* Match the POSIX limit so callers behave the same everywhere. */
+	if (length > 256) {
+		errno = EIO;
+		return -1;
+	}
+
+	status = BCryptGenRandom(NULL, (PUCHAR)buffer, (ULONG)length,
+				 BCRYPT_USE_SYSTEM_PREFERRED_RNG);
+	if (!BCRYPT_SUCCESS(status)) {
+		errno = EIO;
+		return -1;
+	}
+
+	return 0;
+}
+#define getentropy(buffer, length) rte_getentropy(buffer, length)
+
 #endif /* _RTE_OS_SHIM_ */
diff --git a/lib/eal/windows/meson.build b/lib/eal/windows/meson.build
index 6f909c2131..6387786530 100644
--- a/lib/eal/windows/meson.build
+++ b/lib/eal/windows/meson.build
@@ -37,7 +37,11 @@ ws2_32_dep = cc.find_library('ws2_32')
 if not cc.links(min_c_code, dependencies: ws2_32_dep)
     error('broken dependency, "ws2_32"')
 endif
-ext_deps += [dbghelp_dep, setupapi_dep, ws2_32_dep]
+bcrypt_dep = cc.find_library('bcrypt')
+if not cc.links(min_c_code, dependencies: bcrypt_dep)
+    error('broken dependency, "bcrypt"')
+endif
+ext_deps += [dbghelp_dep, setupapi_dep, ws2_32_dep, bcrypt_dep]
 if is_ms_linker
         # Contrary to docs, VirtualAlloc2() is exported by mincore.lib.
         mincore_dep = cc.find_library('mincore')
-- 
2.53.0

[PATCH 02/18] random: always use getentropy

[Stephen Hemminger]

The cc.has_function() check compiled its test program under strict
-std=c11, where glibc hides the getentropy() declaration behind
_DEFAULT_SOURCE. The probe therefore always failed on Linux and
RTE_LIBEAL_USE_GETENTROPY was never defined, silently falling back
to RDSEED or TSC for the initial PRNG seed.

Now that a getentropy() wrapper exists for Windows it can always be
enabled. Removing the conditional means any future regression is a
link error rather than a silent downgrade of seed quality.

Falling back to the TSC now logs at ERR level, including the errno
from getentropy() so that a blocked syscall can be told apart from
other failures. A TSC derived seed only varies by process start time,
which a co-resident attacker can bound.

This requires glibc >= 2.25, which all supported distributions meet.

A backport needs the preceding patch which adds the Windows
getentropy() shim, otherwise the Windows build breaks.

Bugzilla ID: 2035
Fixes: faf8fd252785 ("eal: improve entropy for initial PRNG seed")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_11.rst | 4 ++++
 lib/eal/common/rte_random.c            | 6 ++++--
 lib/eal/include/rte_random.h           | 7 ++++---
 lib/eal/meson.build                    | 3 ---
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 87c7e81bde..5852f7acfb 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -55,6 +55,10 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Updated random number generation.**
+
+  * The initial seed is now always taken from ``getentropy()``.
+
 
 Removed Items
 -------------
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 537fa035ee..b036c50349 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -7,6 +7,8 @@
 #include <x86intrin.h>
 #endif
 #endif
+#include <errno.h>
+#include <string.h>
 #include <unistd.h>
 
 #include <rte_bitops.h>
@@ -219,7 +221,6 @@ rte_drand(void)
 static uint64_t
 __rte_random_initial_seed(void)
 {
-#ifdef RTE_LIBEAL_USE_GETENTROPY
 	int ge_rc;
 	uint64_t ge_seed;
 
@@ -227,7 +228,6 @@ __rte_random_initial_seed(void)
 
 	if (ge_rc == 0)
 		return ge_seed;
-#endif
 #ifdef __RDSEED__
 	unsigned int rdseed_low;
 	unsigned int rdseed_high;
@@ -238,6 +238,8 @@ __rte_random_initial_seed(void)
 		return (uint64_t)rdseed_low | ((uint64_t)rdseed_high << 32);
 #endif
 	/* second fallback: seed using rdtsc */
+	EAL_LOG(ERR, "getentropy() failed (%s), seeding PRNG from TSC: seed has low entropy",
+		strerror(errno));
 	return rte_get_tsc_cycles();
 }
 
diff --git a/lib/eal/include/rte_random.h b/lib/eal/include/rte_random.h
index 15cbe6215a..978ecda203 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -20,9 +20,10 @@ extern "C" {
 /**
  * Seed the pseudo-random generator.
  *
- * The generator is automatically seeded by the EAL init with a timer
- * value. It may need to be re-seeded by the user with a real random
- * value.
+ * The generator is automatically seeded by the EAL init from the
+ * random source provided by the operating system, so there is no need
+ * to re-seed it to get unpredictable values. Seeding it explicitly is
+ * useful to make a run repeatable.
  *
  * This function is not multi-thread safe in regards to other
  * rte_srand() calls, nor is it in relation to concurrent rte_rand(),
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index f9fcee24ee..2bf3c67439 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -24,9 +24,6 @@ endif
 if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
     ext_deps += libarchive
 endif
-if cc.has_function('getentropy', prefix : '#include <unistd.h>')
-    cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
-endif
 
 if is_freebsd
     annotate_locks = false
-- 
2.53.0

[PATCH 03/18] eal: add 32 bit random number function

[Stephen Hemminger]

Add a function returning a 32 bit random value. Callers that only
need 32 bits currently have to truncate the result of rte_rand(),
which assumes that the low bits of the generator are as good as the
high ones. That happens to hold for the current generator but is not
a property callers should have to know about.

Returning the high bits keeps the function correct for the common
case where a generator has weaker low order bits.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_11.rst |  2 ++
 lib/eal/common/rte_random.c            | 10 +++++++
 lib/eal/include/rte_random.h           | 39 +++++++++++++++++++-------
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 5852f7acfb..1dc634d504 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -58,6 +58,8 @@ New Features
 * **Updated random number generation.**
 
   * The initial seed is now always taken from ``getentropy()``.
+  * Added ``rte_rand32()`` for callers which only need 32 bits and would
+    otherwise have to truncate the result of ``rte_rand()``.
 
 
 Removed Items
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index b036c50349..1368b70e27 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -158,6 +158,16 @@ rte_rand(void)
 	return __rte_rand_lfsr258(state);
 }
 
+RTE_EXPORT_SYMBOL(rte_rand32)
+uint32_t
+rte_rand32(void)
+{
+	/* Use the high bits, they are the ones that stay good if the
+	 * underlying generator is ever changed.
+	 */
+	return (uint32_t)(rte_rand() >> 32);
+}
+
 RTE_EXPORT_SYMBOL(rte_rand_max)
 uint64_t
 rte_rand_max(uint64_t upper_bound)
diff --git a/lib/eal/include/rte_random.h b/lib/eal/include/rte_random.h
index 978ecda203..9d3ecc596e 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -13,6 +13,8 @@
 
 #include <stdint.h>
 
+#include <rte_compat.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -27,7 +29,7 @@ extern "C" {
  *
  * This function is not multi-thread safe in regards to other
  * rte_srand() calls, nor is it in relation to concurrent rte_rand(),
- * rte_rand_max() or rte_drand() calls.
+ * rte_rand32(), rte_rand_max() or rte_drand() calls.
  *
  * @param seedval
  *   The value of the seed.
@@ -40,9 +42,9 @@ rte_srand(uint64_t seedval);
  *
  * The generator is not cryptographically secure.
  *
- * rte_rand(), rte_rand_max() and rte_drand() are multi-thread safe,
- * with the exception that they may not be called by multiple
- * _unregistered_ non-EAL threads in parallel.
+ * rte_rand(), rte_rand32(), rte_rand_max() and rte_drand() are
+ * multi-thread safe, with the exception that they may not be called
+ * by multiple _unregistered_ non-EAL threads in parallel.
  *
  * @return
  *   A pseudo-random value between 0 and (1<<64)-1.
@@ -50,15 +52,32 @@ rte_srand(uint64_t seedval);
 uint64_t
 rte_rand(void);
 
+/**
+ * Get a 32 bit pseudo-random value.
+ *
+ * Prefer this over truncating the result of rte_rand() since not
+ * every generator produces equally good values in all bit positions.
+ *
+ * The generator is not cryptographically secure.
+ *
+ * This function is multi-thread safe, with the exception that it may
+ * not be called by multiple _unregistered_ non-EAL threads in parallel.
+ *
+ * @return
+ *   A pseudo-random value between 0 and (1<<32)-1.
+ */
+uint32_t
+rte_rand32(void);
+
 /**
  * Generates a pseudo-random number with an upper bound.
  *
  * This function returns an uniformly distributed (unbiased) random
  * number less than a user-specified maximum value.
  *
- * rte_rand(), rte_rand_max() and rte_drand() are multi-thread safe,
- * with the exception that they may not be called by multiple
- * _unregistered_ non-EAL threads in parallel.
+ * rte_rand(), rte_rand32(), rte_rand_max() and rte_drand() are
+ * multi-thread safe, with the exception that they may not be called
+ * by multiple _unregistered_ non-EAL threads in parallel.
  *
  * @param upper_bound
  *   The upper bound of the generated number.
@@ -76,9 +95,9 @@ rte_rand_max(uint64_t upper_bound);
  *
  * The generator is not cryptographically secure.
  *
- * rte_rand(), rte_rand_max() and rte_drand() are multi-thread safe,
- * with the exception that they may not be called by multiple
- * _unregistered_ non-EAL threads in parallel.
+ * rte_rand(), rte_rand32(), rte_rand_max() and rte_drand() are
+ * multi-thread safe, with the exception that they may not be called
+ * by multiple _unregistered_ non-EAL threads in parallel.
  *
  * @return
  *   A pseudo-random value between 0 and 1.0.
-- 
2.53.0

[PATCH 04/18] sched: fix data race in RED random number generation

[Stephen Hemminger]

RED used a private LCG whose seed and cached value were globals shared
by all lcores without synchronisation. The QoS framework documents
running different ports on different threads, so this is a data race.
The values do not tear, but updates are lost and drop decisions become
correlated across queues.

Use rte_rand32() in the drop test instead. Its state is per lcore, so
nothing is shared and no cache is needed. PIE already does this with
rte_drand().

This removes rte_fast_rand(), rte_red_rand_seed and rte_red_rand_val,
which were only used by RED itself. Removing exported symbols breaks
ABI, so this cannot be backported.

There is no prior deprecation notice for these symbols. They were
never meant to be part of the API: rte_fast_rand() is the internal
generator of the dropper and the two variables are its state, all
exposed only because the drop test is inline in rte_red.h. Keeping
them for a release would mean keeping the data race, which is the
bug being fixed.

Fixes: de3cfa2c9823 ("sched: initial import")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_11.rst | 12 +++++++++++-
 lib/sched/rte_red.c                    |  9 +--------
 lib/sched/rte_red.h                    | 26 ++------------------------
 3 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 1dc634d504..c1742b3c60 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -85,6 +85,12 @@ Removed Items
     ``rte_rib6_is_equal``
   * table: ``RTE_LPM_IPV6_ADDR_SIZE``
 
+* sched: Removed the private random number generator used by RED: the
+  inline function ``rte_fast_rand()`` in ``rte_red.h`` and its global
+  state ``rte_red_rand_seed`` and ``rte_red_rand_val``. Its state was
+  shared by all lcores without synchronisation. RED now uses
+  ``rte_rand32()``, which has per lcore state.
+
 
 API Changes
 -----------
@@ -101,7 +107,6 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
-
 ABI Changes
 -----------
 
@@ -117,6 +122,11 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* sched: Removed the exported variables ``rte_red_rand_seed`` and
+  ``rte_red_rand_val``. They held the state of the private random
+  number generator used by RED, which has been replaced by
+  ``rte_rand32()``.
+
 
 Known Issues
 ------------
diff --git a/lib/sched/rte_red.c b/lib/sched/rte_red.c
index d7534d0bee..6d20afa9dc 100644
--- a/lib/sched/rte_red.c
+++ b/lib/sched/rte_red.c
@@ -5,14 +5,9 @@
 #include <math.h>
 #include <eal_export.h>
 #include "rte_red.h"
-#include <rte_random.h>
 #include <rte_common.h>
 
-static int rte_red_init_done = 0;     /**< Flag to indicate that global initialisation is done */
-RTE_EXPORT_SYMBOL(rte_red_rand_val)
-uint32_t rte_red_rand_val = 0;        /**< Random value cache */
-RTE_EXPORT_SYMBOL(rte_red_rand_seed)
-uint32_t rte_red_rand_seed = 0;       /**< Seed for random number generation */
+static int rte_red_init_done;         /**< Flag to indicate that global initialisation is done */
 
 /**
  * table[i] = log2(1-Wq) * Scale * -1
@@ -116,8 +111,6 @@ rte_red_config_init(struct rte_red_config *red_cfg,
 	 *  Initialize the RED module if not already done
 	 */
 	if (!rte_red_init_done) {
-		rte_red_rand_seed = rte_rand();
-		rte_red_rand_val = rte_fast_rand();
 		__rte_red_init_tables();
 		rte_red_init_done = 1;
 	}
diff --git a/lib/sched/rte_red.h b/lib/sched/rte_red.h
index 3b90cc46a9..8c9d86f6f5 100644
--- a/lib/sched/rte_red.h
+++ b/lib/sched/rte_red.h
@@ -15,6 +15,7 @@
 #include <rte_debug.h>
 #include <rte_cycles.h>
 #include <rte_branch_prediction.h>
+#include <rte_random.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -34,8 +35,6 @@ extern "C" {
 /**
  * Externs
  */
-extern uint32_t rte_red_rand_val;
-extern uint32_t rte_red_rand_seed;
 extern uint16_t rte_red_log2_1_minus_Wq[RTE_RED_WQ_LOG2_NUM];
 extern uint16_t rte_red_pow2_frac_inv[16];
 
@@ -102,23 +101,6 @@ rte_red_config_init(struct rte_red_config *red_cfg,
 	const uint16_t max_th,
 	const uint16_t maxp_inv);
 
-/**
- * @brief Generate random number for RED
- *
- * Implementation based on:
- * http://software.intel.com/en-us/articles/fast-random-number-generator-on-the-intel-pentiumr-4-processor/
- *
- * 10 bit shift has been found through empirical tests (was 16).
- *
- * @return Random number between 0 and (2^22 - 1)
- */
-static inline uint32_t
-rte_fast_rand(void)
-{
-	rte_red_rand_seed = (214013 * rte_red_rand_seed) + 2531011;
-	return rte_red_rand_seed >> 10;
-}
-
 /**
  * @brief calculate factor to scale average queue size when queue
  *        becomes empty
@@ -293,12 +275,8 @@ __rte_red_drop(const struct rte_red_config *red_cfg, struct rte_red *red)
 
 	pa_den = red_cfg->pa_const - pa_num_count;
 
-	/* If drop, generate and save random number to be used next time */
-	if (unlikely((rte_red_rand_val % pa_den) < pa_num)) {
-		rte_red_rand_val = rte_fast_rand();
-
+	if (unlikely((rte_rand32() % pa_den) < pa_num))
 		return 1;
-	}
 
 	/* No drop */
 	return 0;
-- 
2.53.0

[PATCH 05/18] eal/windows: add getrandom shim

[Stephen Hemminger]

Provide getrandom() in the Windows OS shim using BCryptGenRandom()
with the system preferred generator, the same primitive as the
getentropy() shim.

BCryptGenRandom() either fills the entire buffer or fails, so the
shim never returns a short count and has no 256 byte limit. The
flags argument is ignored since not needed.

This is needed by the coming rte_random_bytes() change.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/windows/include/rte_os_shim.h | 34 +++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/lib/eal/windows/include/rte_os_shim.h b/lib/eal/windows/include/rte_os_shim.h
index 4f9611d642..87819045e8 100644
--- a/lib/eal/windows/include/rte_os_shim.h
+++ b/lib/eal/windows/include/rte_os_shim.h
@@ -4,6 +4,7 @@
 #define _RTE_OS_SHIM_
 
 #include <errno.h>
+#include <limits.h>
 #include <time.h>
 
 #include <rte_os.h>
@@ -154,4 +155,37 @@ rte_getentropy(void *buffer, size_t length)
 }
 #define getentropy(buffer, length) rte_getentropy(buffer, length)
 
+/*
+ * Windows has no getrandom(), use the same system preferred random
+ * generator as the getentropy() shim above.
+ *
+ * BCryptGenRandom() either fills the whole buffer or fails, so unlike
+ * the system call this never returns a short count. The flags argument
+ * is ignored, there is no Windows equivalent of GRND_NONBLOCK and the
+ * system preferred generator does not block.
+ */
+static inline ssize_t
+rte_getrandom(void *buffer, size_t length, unsigned int flags)
+{
+	NTSTATUS status;
+
+	RTE_SET_USED(flags);
+
+	/* The count is a ULONG, a longer request is served short like
+	 * the system call is allowed to do.
+	 */
+	if (length > ULONG_MAX)
+		length = ULONG_MAX;
+
+	status = BCryptGenRandom(NULL, (PUCHAR)buffer, (ULONG)length,
+				 BCRYPT_USE_SYSTEM_PREFERRED_RNG);
+	if (!BCRYPT_SUCCESS(status)) {
+		errno = EIO;
+		return -1;
+	}
+
+	return (ssize_t)length;
+}
+#define getrandom(buffer, length, flags) rte_getrandom(buffer, length, flags)
+
 #endif /* _RTE_OS_SHIM_ */
-- 
2.53.0

[PATCH 06/18] eal: add function to get random bytes

[Stephen Hemminger]

Add a function that fills a buffer from the random generator provided
by the operating system, for callers that need values which must not
be predictable, such as keys, hash seeds and MAC addresses.

rte_rand() is a fast pseudo-random generator whose internal state can
be recovered from a handful of outputs, so it is not suitable for
those uses.

The implementation uses getrandom(), which is now available on all
supported platforms. Requests are split internally to stay within the
size that is guaranteed to be returned in full, so that callers do not
have to care about the limit. If the system random generator is not
available an error is returned rather than falling back to the
pseudo-random generator, since a guessable value is worse than a
failure for the intended uses.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_11.rst |  5 +++
 lib/eal/common/rte_random.c            | 40 ++++++++++++++++++++++++
 lib/eal/include/rte_random.h           | 43 +++++++++++++++++++++++++-
 3 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index c1742b3c60..f25e497729 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -60,6 +60,11 @@ New Features
   * The initial seed is now always taken from ``getentropy()``.
   * Added ``rte_rand32()`` for callers which only need 32 bits and would
     otherwise have to truncate the result of ``rte_rand()``.
+  * Added ``rte_random_bytes()`` to fill a buffer from the random
+    generator provided by the operating system. It is intended for
+    values which must not be predictable, such as keys, hash seeds and
+    MAC addresses, where the pseudo-random ``rte_rand()`` is not
+    suitable.
 
 
 Removed Items
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 1368b70e27..4b777f69f4 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -10,9 +10,13 @@
 #include <errno.h>
 #include <string.h>
 #include <unistd.h>
+#ifndef RTE_EXEC_ENV_WINDOWS
+#include <sys/random.h>
+#endif
 
 #include <rte_bitops.h>
 #include <rte_branch_prediction.h>
+#include <rte_common.h>
 #include <rte_cycles.h>
 #include <rte_lcore.h>
 #include <rte_lcore_var.h>
@@ -228,6 +232,42 @@ rte_drand(void)
 	return (double)rand64 / denom;
 }
 
+/* Requests of at most this size are guaranteed to return in full
+ * once the random source has been initialized. Larger requests are
+ * split so that callers do not have to care about the limit.
+ */
+#define RANDOM_BYTES_CHUNK 256
+
+RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_random_bytes, 26.11)
+int
+rte_random_bytes(void *buf, size_t len)
+{
+	uint8_t *ptr = buf;
+
+	while (len > 0) {
+		size_t chunk = RTE_MIN(len, (size_t)RANDOM_BYTES_CHUNK);
+		ssize_t ret;
+
+		ret = getrandom(ptr, chunk, 0);
+		if (ret < 0) {
+			if (errno == EINTR)
+				continue;
+			return -errno;
+		}
+
+		/* Should not happen, a bounded request either blocks
+		 * until it can be satisfied in full or fails.
+		 */
+		if (ret == 0)
+			return -EIO;
+
+		ptr += ret;
+		len -= ret;
+	}
+
+	return 0;
+}
+
 static uint64_t
 __rte_random_initial_seed(void)
 {
diff --git a/lib/eal/include/rte_random.h b/lib/eal/include/rte_random.h
index 9d3ecc596e..7c5030ecab 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -8,9 +8,14 @@
 /**
  * @file
  *
- * Pseudo-random Generators in RTE
+ * Random number generation.
+ *
+ * A fast pseudo-random generator for general use, and access to the
+ * random source of the operating system for values which must not be
+ * predictable.
  */
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include <rte_compat.h>
@@ -104,6 +109,42 @@ rte_rand_max(uint64_t upper_bound);
  */
 double rte_drand(void);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Fill a buffer with random bytes from the system random generator.
+ *
+ * The bytes are drawn from the same source as the urandom device and
+ * are suitable for cryptographic purposes such as keys, hash seeds and
+ * MAC addresses. Unlike rte_rand() the generator state is not
+ * recoverable from the output.
+ *
+ * If the system random source has not been initialized yet this call
+ * blocks until enough entropy is available. Once initialized it never
+ * blocks.
+ *
+ * It is several orders of magnitude slower than rte_rand() because it
+ * may enter the kernel on every call, and is not meant to be used on
+ * the datapath.
+ *
+ * This function is multi-thread safe.
+ *
+ * @param buf
+ *   Buffer to fill with random bytes.
+ * @param len
+ *   Number of bytes to write. There is no upper limit, larger requests
+ *   are split internally. A length of zero succeeds without doing
+ *   anything.
+ * @return
+ *   0 on success and the buffer is filled completely.
+ *   A negative errno if the system random generator failed, the
+ *   contents of the buffer are then undefined.
+ */
+__rte_experimental
+int
+rte_random_bytes(void *buf, size_t len);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.53.0

[PATCH 07/18] hash: use secure random for Toeplitz hash key

[Stephen Hemminger]

When no key is supplied by the application, rte_thash_init_ctx()
generated the Toeplitz hash key with rte_rand().

That generator is not meant to be unpredictable and its internal state
can be recovered from a small number of outputs, so an attacker able to
observe other random values can derive the RSS key and craft flows that
all land on a single receive queue.

Use rte_random_bytes() for the hash key and for the LFSR state.

The hash key is generated once at context creation. It is now drawn
before the tailq lock is taken, since the system random generator can
block until the kernel pool is initialized and that should not be
done while holding a global lock.

The LFSR state is also set from alloc_lfsr(), which besides context
creation is reached from rte_thash_add_helper() and
rte_thash_adjust_tuple(). Those paths report a failure of the random
source as -ENOMEM, which is what they already return when alloc_lfsr()
fails.

No longer use rte_memcpy() just use memcpy().

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/hash/rte_thash.c | 46 +++++++++++++++++++++++++++++++++++---------
 lib/hash/rte_thash.h |  4 +++-
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
index fcd41248b9..7730ee053b 100644
--- a/lib/hash/rte_thash.c
+++ b/lib/hash/rte_thash.c
@@ -9,7 +9,6 @@
 #include <rte_thash.h>
 #include <rte_tailq.h>
 #include <rte_random.h>
-#include <rte_memcpy.h>
 #include <rte_errno.h>
 #include <rte_eal_memconfig.h>
 #include <rte_log.h>
@@ -176,7 +175,17 @@ alloc_lfsr(uint32_t poly_degree)
 	lfsr->deg = poly_degree;
 	lfsr->poly = thash_get_rand_poly(lfsr->deg);
 	do {
-		lfsr->state = rte_rand() & ((1 << lfsr->deg) - 1);
+		uint32_t rnd;
+
+		if (rte_random_bytes(&rnd, sizeof(rnd)) != 0) {
+			rte_free(lfsr);
+			return NULL;
+		}
+
+		/* deg can be 32, so the mask must not be computed by
+		 * shifting an int by the degree.
+		 */
+		lfsr->state = rnd & RTE_GENMASK32(lfsr->deg - 1, 0);
 	} while (lfsr->state == 0);
 	/* init reverse order polynomial */
 	lfsr->rev_poly = get_rev_poly(lfsr->poly, lfsr->deg);
@@ -215,13 +224,35 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, uint32_t reta_sz,
 	struct rte_thash_ctx *ctx;
 	struct rte_tailq_entry *te;
 	struct rte_thash_list *thash_list;
-	uint32_t i;
+	uint8_t *rand_key = NULL;
 
 	if ((name == NULL) || (key_len == 0) || !RETA_SZ_IN_RANGE(reta_sz)) {
 		rte_errno = EINVAL;
 		return NULL;
 	}
 
+	/* Draw the random key before taking the tailq lock, the system
+	 * random generator can block until it is initialized.
+	 */
+	if (key == NULL) {
+		rand_key = rte_zmalloc(NULL, key_len, 0);
+		if (rand_key == NULL) {
+			HASH_LOG(ERR, "thash ctx %s key allocation failed",
+				name);
+			rte_errno = ENOMEM;
+			return NULL;
+		}
+
+		if (rte_random_bytes(rand_key, key_len) != 0) {
+			HASH_LOG(ERR,
+				"Cannot generate hash key for thash context %s",
+				name);
+			rte_free(rand_key);
+			rte_errno = EIO;
+			return NULL;
+		}
+	}
+
 	thash_list = RTE_TAILQ_CAST(rte_thash_tailq.head, rte_thash_list);
 
 	rte_mcfg_tailq_write_lock();
@@ -262,12 +293,7 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, uint32_t reta_sz,
 	LIST_INIT(&ctx->head);
 	ctx->flags = flags;
 
-	if (key)
-		rte_memcpy(ctx->hash_key, key, key_len);
-	else {
-		for (i = 0; i < key_len; i++)
-			ctx->hash_key[i] = rte_rand();
-	}
+	memcpy(ctx->hash_key, key ? key : rand_key, key_len);
 
 	if (rte_thash_gfni_supported()) {
 		ctx->matrices = rte_zmalloc(NULL, key_len * sizeof(uint64_t),
@@ -286,6 +312,7 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, uint32_t reta_sz,
 	TAILQ_INSERT_TAIL(thash_list, te, next);
 
 	rte_mcfg_tailq_write_unlock();
+	rte_free(rand_key);
 
 	return ctx;
 
@@ -295,6 +322,7 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, uint32_t reta_sz,
 	rte_free(te);
 exit:
 	rte_mcfg_tailq_write_unlock();
+	rte_free(rand_key);
 	return NULL;
 }
 
diff --git a/lib/hash/rte_thash.h b/lib/hash/rte_thash.h
index 8bca430663..4387eb4a07 100644
--- a/lib/hash/rte_thash.h
+++ b/lib/hash/rte_thash.h
@@ -295,7 +295,9 @@ struct rte_thash_subtuple_helper;
  *  the reta entry.
  * @param key
  *  Pointer to the key used to init an internal key state.
- *  Could be NULL, in this case internal key will be inited with random.
+ *  Could be NULL, in this case the internal key is generated from the
+ *  random source of the operating system, and context creation fails
+ *  if that source is unavailable.
  * @param flags
  *  Supported flags are:
  *   RTE_THASH_IGNORE_PERIOD_OVERFLOW
-- 
2.53.0

[PATCH 08/18] pipeline: use secure random for IPsec salt

[Stephen Hemminger]

The salt for AES-CBC and 3DES-CBC was generated with rte_rand().
Unlike the other cipher cases, where the salt comes from the key
material supplied by the application, this value is generated
internally and ends up in the IPsec transform.

rte_rand() is a fast pseudo-random generator whose state can be
recovered from a few outputs, so use rte_random_bytes() instead.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pipeline/rte_swx_ipsec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/pipeline/rte_swx_ipsec.c b/lib/pipeline/rte_swx_ipsec.c
index 553056fad2..9959009aab 100644
--- a/lib/pipeline/rte_swx_ipsec.c
+++ b/lib/pipeline/rte_swx_ipsec.c
@@ -1460,7 +1460,8 @@ crypto_xform_get(struct rte_swx_ipsec_sa_params *p,
 		switch (p->crypto.cipher_auth.cipher.alg) {
 		case RTE_CRYPTO_CIPHER_AES_CBC:
 		case RTE_CRYPTO_CIPHER_3DES_CBC:
-			salt = rte_rand();
+			if (rte_random_bytes(&salt, sizeof(salt)) != 0)
+				return NULL;
 			break;
 
 		case RTE_CRYPTO_CIPHER_AES_CTR:
-- 
2.53.0

[PATCH 09/18] net: use secure random for random MAC address

[Stephen Hemminger]

rte_eth_random_addr() returned 48 bits of raw rte_rand() output on
every call. That both makes the generated address predictable and
leaks enough generator output to help recover the internal state.

Use rte_random_bytes() instead. The function cannot report an error
to its callers, so fall back to rte_rand() if the system random
generator is unavailable, which is no worse than the previous
behaviour.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/net/rte_ether.c | 13 ++++++++++---
 lib/net/rte_ether.h |  5 +++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/lib/net/rte_ether.c b/lib/net/rte_ether.c
index 6703145fc5..4b2932c00b 100644
--- a/lib/net/rte_ether.c
+++ b/lib/net/rte_ether.c
@@ -3,6 +3,7 @@
  */
 
 #include <stdbool.h>
+#include <string.h>
 
 #include <eal_export.h>
 #include <rte_ether.h>
@@ -12,10 +13,16 @@ RTE_EXPORT_SYMBOL(rte_eth_random_addr)
 void
 rte_eth_random_addr(uint8_t *addr)
 {
-	uint64_t rand = rte_rand();
-	uint8_t *p = (uint8_t *)&rand;
+	/* Prefer the system random generator so that the address can not
+	 * be predicted from other random values. Fall back to rte_rand()
+	 * if it is unavailable, which is what was always used before.
+	 */
+	if (rte_random_bytes(addr, RTE_ETHER_ADDR_LEN) != 0) {
+		uint64_t rand = rte_rand();
+
+		memcpy(addr, &rand, RTE_ETHER_ADDR_LEN);
+	}
 
-	rte_memcpy(addr, p, RTE_ETHER_ADDR_LEN);
 	addr[0] &= (uint8_t)~RTE_ETHER_GROUP_ADDR;	/* clear multicast bit */
 	addr[0] |= RTE_ETHER_LOCAL_ADMIN_ADDR;	/* set local assignment bit */
 }
diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
index c9a0b536c3..b0dc227362 100644
--- a/lib/net/rte_ether.h
+++ b/lib/net/rte_ether.h
@@ -221,6 +221,11 @@ static inline int rte_is_valid_assigned_ether_addr(const struct rte_ether_addr *
 /**
  * Generate a random Ethernet address that is locally administered
  * and not multicast.
+ *
+ * The address is taken from the random source of the operating system
+ * so that it cannot be predicted from other random values. If that
+ * source is unavailable it falls back to the pseudo-random rte_rand().
+ *
  * @param addr
  *   A pointer to Ethernet address.
  */
-- 
2.53.0

[PATCH 10/18] ipsec: use secure random for SAD hash seed

[Stephen Hemminger]

The hash initial value for the security association database was
taken from rte_rand(). A predictable hash seed lets an attacker pick
SPI values that collide in the same bucket and degrade lookups.

Use rte_random_bytes(), which is done once when the database is
created.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ipsec/ipsec_sad.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/ipsec/ipsec_sad.c b/lib/ipsec/ipsec_sad.c
index 15ea868f77..0ea797604e 100644
--- a/lib/ipsec/ipsec_sad.c
+++ b/lib/ipsec/ipsec_sad.c
@@ -300,7 +300,12 @@ rte_ipsec_sad_create(const char *name, const struct rte_ipsec_sad_conf *conf)
 	memcpy(sad->name, sad_name, sizeof(sad_name));
 
 	hash_params.hash_func = DEFAULT_HASH_FUNC;
-	hash_params.hash_func_init_val = rte_rand();
+	if (rte_random_bytes(&hash_params.hash_func_init_val,
+			     sizeof(hash_params.hash_func_init_val)) != 0) {
+		rte_free(sad);
+		rte_errno = EIO;
+		return NULL;
+	}
 	sad->init_val = hash_params.hash_func_init_val;
 	hash_params.socket_id = conf->socket_id;
 	hash_params.name = hash_name;
-- 
2.53.0

[PATCH 11/18] net/intel: use secure random for default RSS key

[Stephen Hemminger]

When the application does not supply an RSS key, these drivers built
the default key a byte at a time from rte_rand(). That generator is a
fast non-cryptographic PRNG whose state is recoverable from its output,
so the resulting key is predictable. Linux fills its default RSS key
from get_random_bytes() for the same reason.

Use rte_random_bytes() which takes randomness from the operating
system, and propagate the error where the caller can report it.

ice_get_default_rss_key() has no way to return an error to its caller,
so it keeps rte_rand() as a fallback if the system random generator
is unavailable.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/intel/cpfl/cpfl_ethdev.c |  5 +++--
 drivers/net/intel/iavf/iavf_ethdev.c |  8 +++++---
 drivers/net/intel/ice/ice_dcf.c      | 10 ++++++----
 drivers/net/intel/ice/ice_ethdev.c   | 11 ++++++++---
 drivers/net/intel/idpf/idpf_ethdev.c |  5 +++--
 5 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/net/intel/cpfl/cpfl_ethdev.c b/drivers/net/intel/cpfl/cpfl_ethdev.c
index 4315adb68c..c3f18686bb 100644
--- a/drivers/net/intel/cpfl/cpfl_ethdev.c
+++ b/drivers/net/intel/cpfl/cpfl_ethdev.c
@@ -478,8 +478,9 @@ cpfl_init_rss(struct idpf_vport *vport)
 	nb_q = dev_data->nb_rx_queues;
 
 	if (rss_conf->rss_key == NULL) {
-		for (i = 0; i < vport->rss_key_size; i++)
-			vport->rss_key[i] = (uint8_t)rte_rand();
+		ret = rte_random_bytes(vport->rss_key, vport->rss_key_size);
+		if (ret != 0)
+			return ret;
 	} else if (rss_conf->rss_key_len != vport->rss_key_size) {
 		PMD_INIT_LOG(ERR, "Invalid RSS key length in RSS configuration, should be %d",
 			     vport->rss_key_size);
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index d601ec3b6a..fcef1eb26a 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -565,12 +565,14 @@ iavf_init_rss(struct iavf_adapter *adapter)
 	/* configure RSS key */
 	if (!rss_conf->rss_key) {
 		/* Calculate the default hash key */
-		for (i = 0; i < vf->vf_res->rss_key_size; i++)
-			vf->rss_key[i] = (uint8_t)rte_rand();
-	} else
+		ret = rte_random_bytes(vf->rss_key, vf->vf_res->rss_key_size);
+		if (ret != 0)
+			return ret;
+	} else {
 		memcpy(vf->rss_key, rss_conf->rss_key,
 			   RTE_MIN(rss_conf->rss_key_len,
 				   vf->vf_res->rss_key_size));
+	}
 
 	/* init RSS LUT table */
 	for (i = 0, j = 0; i < vf->vf_res->rss_lut_size; i++, j++) {
diff --git a/drivers/net/intel/ice/ice_dcf.c b/drivers/net/intel/ice/ice_dcf.c
index 3b635c0822..b73af13d5b 100644
--- a/drivers/net/intel/ice/ice_dcf.c
+++ b/drivers/net/intel/ice/ice_dcf.c
@@ -1125,14 +1125,16 @@ ice_dcf_init_rss(struct ice_dcf_hw *hw)
 	 */
 
 	/* configure RSS key */
-	if (!rss_conf->rss_key)
+	if (!rss_conf->rss_key) {
 		/* Calculate the default hash key */
-		for (i = 0; i < hw->vf_res->rss_key_size; i++)
-			hw->rss_key[i] = (uint8_t)rte_rand();
-	else
+		ret = rte_random_bytes(hw->rss_key, hw->vf_res->rss_key_size);
+		if (ret != 0)
+			return ret;
+	} else {
 		memcpy(hw->rss_key, rss_conf->rss_key,
 			   RTE_MIN(rss_conf->rss_key_len,
 				   hw->vf_res->rss_key_size));
+	}
 
 	/* init RSS LUT table */
 	for (i = 0, j = 0; i < hw->vf_res->rss_lut_size; i++, j++) {
diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 76b8ff0a72..105a6636c0 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -3778,9 +3778,14 @@ ice_get_default_rss_key(uint8_t *rss_key, uint32_t rss_key_size)
 	}
 
 	if (!default_key_done) {
-		/* Calculate the default hash key */
-		for (i = 0; i < sizeof(default_key); i++)
-			key[i] = (uint8_t)rte_rand();
+		/* Calculate the default hash key. Fall back to rte_rand()
+		 * if the system random generator is not available, this
+		 * function cannot report an error.
+		 */
+		if (rte_random_bytes(key, sizeof(default_key)) != 0) {
+			for (i = 0; i < sizeof(default_key); i++)
+				key[i] = (uint8_t)rte_rand();
+		}
 		default_key_done = true;
 	}
 	memcpy(rss_key, key, RTE_MIN(rss_key_size, sizeof(default_key)));
diff --git a/drivers/net/intel/idpf/idpf_ethdev.c b/drivers/net/intel/idpf/idpf_ethdev.c
index c13505416a..ab4d007a22 100644
--- a/drivers/net/intel/idpf/idpf_ethdev.c
+++ b/drivers/net/intel/idpf/idpf_ethdev.c
@@ -432,8 +432,9 @@ idpf_init_rss(struct idpf_vport *vport)
 	nb_q = dev_data->nb_rx_queues;
 
 	if (rss_conf->rss_key == NULL) {
-		for (i = 0; i < vport->rss_key_size; i++)
-			vport->rss_key[i] = (uint8_t)rte_rand();
+		ret = rte_random_bytes(vport->rss_key, vport->rss_key_size);
+		if (ret != 0)
+			return ret;
 	} else if (rss_conf->rss_key_len != vport->rss_key_size) {
 		PMD_INIT_LOG(ERR, "Invalid RSS key length in RSS configuration, should be %d",
 			     vport->rss_key_size);
-- 
2.53.0

[PATCH 12/18] random: use all 64 bits of the seed

[Stephen Hemminger]

The generator state is five 64 bit words, expanded from the seed by a
32 bit linear congruential generator. The two halves of the seed were
folded together first, so only 32 bits of it had any effect, and the
low order bits of such a generator are weak, leaving the five words
with correlated low bits.

Expand the seed with SplitMix64 instead, so every bit of the seed
affects every word of the state. It is described in "Fast Splittable
Pseudorandom Number Generators" by Steele, Lea and Flood,
https://doi.org/10.1145/2714064.2660195 and is what the authors of
the xoshiro and xoroshiro generators recommend for seeding.

The per-lcore seeds were formed by adding the lcore id to the seed,
leaving neighbouring lcores with nearly identical state. Pass that
sum through SplitMix64 as well.

Fixes: 3f002f069612 ("eal: replace libc-based random generation with LFSR")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_11.rst |  4 ++
 lib/eal/common/rte_random.c            | 74 +++++++++++++-------------
 2 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index f25e497729..9d1238d6f3 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -65,6 +65,10 @@ New Features
     values which must not be predictable, such as keys, hash seeds and
     MAC addresses, where the pseudo-random ``rte_rand()`` is not
     suitable.
+  * Improved the expansion of the seed into the generator state. All
+    64 bits of the value passed to ``rte_srand()`` now affect the state,
+    so a given seed produces a different sequence than in previous
+    releases.
 
 
 Removed Items
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 4b777f69f4..db4e19b414 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -39,55 +39,45 @@ static RTE_LCORE_VAR_HANDLE(struct rte_rand_state, rand_state);
 /* instance to be shared by all unregistered non-EAL threads */
 static struct rte_rand_state unregistered_rand_state;
 
-static uint32_t
-__rte_rand_lcg32(uint32_t *seed)
-{
-	*seed = 1103515245U * *seed + 12345U;
-
-	return *seed;
-}
-
+/* SplitMix64, used to expand the seed into the generator state.
+ * It has a full 64 bit period and good avalanche, so all the bits
+ * of the seed affect every word of the resulting state.
+ *
+ * See "Fast Splittable Pseudorandom Number Generators" by Steele,
+ * Lea and Flood, https://doi.org/10.1145/2714064.2660195
+ */
 static uint64_t
-__rte_rand_lcg64(uint32_t *seed)
+__rte_rand_splitmix64(uint64_t *state)
 {
-	uint64_t low;
-	uint64_t high;
-
-	/* A 64-bit LCG would have been much cleaner, but good
-	 * multiplier/increments for such seem hard to come by.
-	 */
+	uint64_t z;
 
-	low = __rte_rand_lcg32(seed);
-	high = __rte_rand_lcg32(seed);
+	z = (*state += 0x9E3779B97F4A7C15ULL);
+	z = (z ^ (z >> 30)) * 0xBF58476D1CE4E5B9ULL;
+	z = (z ^ (z >> 27)) * 0x94D049BB133111EBULL;
 
-	return low | (high << 32);
+	return z ^ (z >> 31);
 }
 
 static uint64_t
-__rte_rand_lfsr258_gen_seed(uint32_t *seed, uint64_t min_value)
+__rte_rand_lfsr258_gen_seed(uint64_t *state, uint64_t min_value)
 {
-	uint64_t res;
-
-	res = __rte_rand_lcg64(seed);
-
-	if (res < min_value)
-		res += min_value;
-
-	return res;
+	/* LFSR258 degenerates unless each word exceeds its threshold.
+	 * All thresholds are powers of two, so a bitwise or is enough
+	 * and keeps the remaining bits untouched.
+	 */
+	return __rte_rand_splitmix64(state) | min_value;
 }
 
 static void
 __rte_srand_lfsr258(uint64_t seed, struct rte_rand_state *state)
 {
-	uint32_t lcg_seed;
-
-	lcg_seed = (uint32_t)(seed ^ (seed >> 32));
+	uint64_t mix_state = seed;
 
-	state->z1 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 2UL);
-	state->z2 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 512UL);
-	state->z3 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 4096UL);
-	state->z4 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 131072UL);
-	state->z5 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 8388608UL);
+	state->z1 = __rte_rand_lfsr258_gen_seed(&mix_state, 2UL);
+	state->z2 = __rte_rand_lfsr258_gen_seed(&mix_state, 512UL);
+	state->z3 = __rte_rand_lfsr258_gen_seed(&mix_state, 4096UL);
+	state->z4 = __rte_rand_lfsr258_gen_seed(&mix_state, 131072UL);
+	state->z5 = __rte_rand_lfsr258_gen_seed(&mix_state, 8388608UL);
 }
 
 RTE_EXPORT_SYMBOL(rte_srand)
@@ -95,16 +85,24 @@ void
 rte_srand(uint64_t seed)
 {
 	unsigned int lcore_id;
+	uint64_t mix_state;
 
-	/* add lcore_id to seed to avoid having the same sequence */
+	/* Mix in the lcore id so that each lcore gets an unrelated
+	 * sequence. Adding it to the seed would leave neighbouring
+	 * lcores with nearly identical generator state.
+	 */
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		struct rte_rand_state *lcore_state =
 			RTE_LCORE_VAR_LCORE(lcore_id, rand_state);
 
-		__rte_srand_lfsr258(seed + lcore_id, lcore_state);
+		mix_state = seed + lcore_id;
+		__rte_srand_lfsr258(__rte_rand_splitmix64(&mix_state),
+				    lcore_state);
 	}
 
-	__rte_srand_lfsr258(seed + lcore_id, &unregistered_rand_state);
+	mix_state = seed + lcore_id;
+	__rte_srand_lfsr258(__rte_rand_splitmix64(&mix_state),
+			    &unregistered_rand_state);
 }
 
 static __rte_always_inline uint64_t
-- 
2.53.0

[PATCH 13/18] drivers/net: use secure random for RSS key

[Stephen Hemminger]

Both drivers built the default RSS key a word at a time from rte_rand().
That generator is a fast non-cryptographic PRNG whose state is
recoverable from its output, so the resulting key is predictable.

Use rte_random_bytes() which takes randomness from the operating
system, and report the error to the caller.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_dev.c | 16 +++++++++++-----
 drivers/net/bnx2x/bnx2x.c     | 11 +++++++----
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index 482d3d8062..37856fddd9 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -3,6 +3,8 @@
  *   Copyright(c) 2018 Synopsys, Inc. All rights reserved.
  */
 
+#include <rte_random.h>
+
 #include "axgbe_ethdev.h"
 #include "axgbe_common.h"
 #include "axgbe_phy.h"
@@ -931,13 +933,15 @@ static void axgbe_rss_options(struct axgbe_port *pdata)
 static int axgbe_config_rss(struct axgbe_port *pdata)
 {
 	uint32_t i;
+	int ret;
 
 	if (pdata->rss_enable) {
 		/* Initialize RSS hash key and lookup table */
-		uint32_t *key = (uint32_t *)pdata->rss_key;
-
-		for (i = 0; i < sizeof(pdata->rss_key) / 4; i++)
-			*key++ = (uint32_t)rte_rand();
+		ret = rte_random_bytes(pdata->rss_key, sizeof(pdata->rss_key));
+		if (ret != 0) {
+			PMD_DRV_LOG_LINE(ERR, "Error generating RSS hash key");
+			return ret;
+		}
 		for (i = 0; i < AXGBE_RSS_MAX_TABLE_SIZE; i++)
 			AXGMAC_SET_BITS(pdata->rss_table[i], MAC_RSSDR, DMCH,
 					i % pdata->eth_dev->data->nb_rx_queues);
@@ -1393,7 +1397,9 @@ static int axgbe_init(struct axgbe_port *pdata)
 	axgbe_config_tx_pbl_val(pdata);
 	axgbe_config_rx_pbl_val(pdata);
 	axgbe_config_rx_buffer_size(pdata);
-	axgbe_config_rss(pdata);
+	ret = axgbe_config_rss(pdata);
+	if (ret)
+		return ret;
 	axgbe_config_tso_mode(pdata);
 	wrapper_tx_desc_init(pdata);
 	ret = wrapper_rx_desc_init(pdata);
diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 8790c858d5..97ee95a1be 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -28,6 +28,7 @@
 #include <zlib.h>
 
 #include <rte_bitops.h>
+#include <rte_random.h>
 #include <rte_string_fns.h>
 
 #include "eal_firmware.h"
@@ -6643,7 +6644,7 @@ bnx2x_config_rss_pf(struct bnx2x_softc *sc, struct ecore_rss_config_obj *rss_obj
 		  uint8_t config_hash)
 {
 	struct ecore_config_rss_params params = { NULL };
-	uint32_t i;
+	int rc;
 
 	/*
 	 * Although RSS is meaningless when there is a single HW queue we
@@ -6675,9 +6676,11 @@ bnx2x_config_rss_pf(struct bnx2x_softc *sc, struct ecore_rss_config_obj *rss_obj
 			 sizeof(params.ind_table));
 
 	if (config_hash) {
-/* RSS keys */
-		for (i = 0; i < sizeof(params.rss_key) / 4; i++) {
-			params.rss_key[i] = (uint32_t) rte_rand();
+		/* RSS keys */
+		rc = rte_random_bytes(params.rss_key, sizeof(params.rss_key));
+		if (rc != 0) {
+			PMD_DRV_LOG(ERR, sc, "Error generating RSS hash key");
+			return rc;
 		}
 
 		rte_bit_relaxed_set32(ECORE_RSS_SET_SRCH, &params.rss_flags);
-- 
2.53.0

[PATCH 14/18] drivers/net: use common random MAC address helper

[Stephen Hemminger]

Both drivers open coded a random MAC address: take 32 bits from
rte_rand(), clear the multicast bit and set the local assignment bit.
That is what rte_eth_random_addr() does, and it now takes its
randomness from the operating system rather than rte_rand().

The enetc version also wrote past the end of the six byte address.
After incrementing a uint32_t pointer to the last two bytes it stored
a full uint32_t there, overrunning hw->mac.addr by two bytes. The
address bytes are now read explicitly instead of through a wider
pointer, which also makes the value written to PMAR1 independent of
the host byte order.

Note that the read path at the top of the same function has the same
overrun. It is left alone here, this patch only changes the code that
generates the address.

Fixes: a7fc52fcadb1 ("net/enetc: introduce ENETC4 PMD")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/enetc/enetc4_vf.c     | 22 +++++++++++-----------
 drivers/net/enetfec/enet_ethdev.c | 16 ++--------------
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/net/enetc/enetc4_vf.c b/drivers/net/enetc/enetc4_vf.c
index ef5f1e6d66..df3b4bd89b 100644
--- a/drivers/net/enetc/enetc4_vf.c
+++ b/drivers/net/enetc/enetc4_vf.c
@@ -4,7 +4,6 @@
 
 #include <stdbool.h>
 #include <rte_kvargs.h>
-#include <rte_random.h>
 #include <dpaax_iova_table.h>
 #include "enetc_logs.h"
 #include "enetc.h"
@@ -1252,18 +1251,19 @@ enetc4_vf_mac_init(struct enetc_eth_hw *hw, struct rte_eth_dev *eth_dev)
 	low_mac = (uint16_t)*mac;
 
 	if ((high_mac | low_mac) == 0) {
-		char *first_byte;
+		const uint8_t *addr = hw->mac.addr;
+
 		ENETC_PMD_NOTICE("MAC is not available for this SI, "
 				 "set random MAC");
-		mac = (uint32_t *)hw->mac.addr;
-		*mac = (uint32_t)rte_rand();
-		first_byte = (char *)mac;
-		*first_byte &= 0xfe;    /* clear multicast bit */
-		*first_byte |= 0x02;    /* set local assignment bit (IEEE802) */
-		enetc4_port_wr(enetc_hw, ENETC4_PMAR0, *mac);
-		mac++;
-		*mac = (uint16_t)rte_rand();
-		enetc4_port_wr(enetc_hw, ENETC4_PMAR1, *mac);
+		rte_eth_random_addr(hw->mac.addr);
+		/* The address is six bytes, read the last two explicitly
+		 * rather than through a wider pointer.
+		 */
+		enetc4_port_wr(enetc_hw, ENETC4_PMAR0,
+			       addr[0] | addr[1] << 8 |
+			       addr[2] << 16 | (uint32_t)addr[3] << 24);
+		enetc4_port_wr(enetc_hw, ENETC4_PMAR1,
+			       addr[4] | addr[5] << 8);
 		enetc_print_ethaddr("New address: ",
 			(const struct rte_ether_addr *)hw->mac.addr);
 	}
diff --git a/drivers/net/enetfec/enet_ethdev.c b/drivers/net/enetfec/enet_ethdev.c
index c7ef097875..691f634d80 100644
--- a/drivers/net/enetfec/enet_ethdev.c
+++ b/drivers/net/enetfec/enet_ethdev.c
@@ -637,20 +637,8 @@ pmd_enetfec_probe(struct rte_vdev_device *vdev)
 	*mac = (uint16_t)(tmac >> ENETFEC_MAC_SHIFT);
 	high_mac = (uint16_t)(*mac);
 
-	if ((high_mac | low_mac) == 0 || (high_mac | low_mac) == ENETFEC_MAC_RESET) {
-		uint8_t *first_byte;
-
-		mac = (uint16_t *)addr.addr_bytes;
-		tmac = (uint32_t)rte_rand();
-		first_byte = (uint8_t *)&tmac;
-		*first_byte &= (uint8_t)~RTE_ETHER_GROUP_ADDR; /* clear multicast bit */
-		*first_byte |= RTE_ETHER_LOCAL_ADMIN_ADDR; /* set local assignment bit (IEEE802) */
-		*mac = (uint16_t)tmac;
-		mac++;
-		*mac = (uint16_t)(tmac >> ENETFEC_MAC_SHIFT);
-		mac++;
-		*mac = (uint16_t)rte_rand();
-	}
+	if ((high_mac | low_mac) == 0 || (high_mac | low_mac) == ENETFEC_MAC_RESET)
+		rte_eth_random_addr(addr.addr_bytes);
 
 	enetfec_set_mac_address(dev, &addr);
 
-- 
2.53.0

[PATCH 15/18] random: remove RDSEED fallback for initial seed

[Stephen Hemminger]

RDSEED is an x86 instruction that returns random bits from a hardware
source. It has well known reliability problems: it can fail
persistently when many cores use it at once, and several processors
have errata affecting it. Since commit bfb3c5b9ffe7 ("random: always
use getentropy") it was only reachable if getentropy() failed, which
does not happen in practice.

Nothing is lost by removing it. The kernel already mixes RDSEED into
its own entropy pool as one of several sources, so what it provides is
present in the getentropy() result anyway, combined with other sources
rather than trusted on its own.

The remaining fallback, the timestamp counter, still logs an error so
that a low entropy seed is reported.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/rte_random.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index db4e19b414..c6f3136408 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -2,11 +2,6 @@
  * Copyright(c) 2019 Ericsson AB
  */
 
-#ifdef __RDSEED__
-#ifndef RTE_TOOLCHAIN_MSVC
-#include <x86intrin.h>
-#endif
-#endif
 #include <errno.h>
 #include <string.h>
 #include <unistd.h>
@@ -276,16 +271,8 @@ __rte_random_initial_seed(void)
 
 	if (ge_rc == 0)
 		return ge_seed;
-#ifdef __RDSEED__
-	unsigned int rdseed_low;
-	unsigned int rdseed_high;
-
-	/* first fallback: rdseed instruction, if available */
-	if (_rdseed32_step(&rdseed_low) == 1 &&
-	    _rdseed32_step(&rdseed_high) == 1)
-		return (uint64_t)rdseed_low | ((uint64_t)rdseed_high << 32);
-#endif
-	/* second fallback: seed using rdtsc */
+
+	/* fallback: seed using rdtsc */
 	EAL_LOG(ERR, "getentropy() failed (%s), seeding PRNG from TSC: seed has low entropy",
 		strerror(errno));
 	return rte_get_tsc_cycles();
-- 
2.53.0

[PATCH 16/18] test: allow setting random seed for tests

[Stephen Hemminger]

Many tests build their input data with rte_rand(), which is seeded
from the system entropy source and therefore produces a different
sequence on every run. Varying the data is good for coverage, but it
adds noise when comparing performance results between two builds.

Add a DPDK_TEST_SEED environment variable to the test application that
seeds the generator with a fixed value. The generator is reseeded
before each test is dispatched, so a test gets the same input data no
matter what ran before it in the same invocation. When the variable is
not set the behaviour is unchanged and each run gets a random seed.

Note that rand_perf_autotest calls rte_srand() itself. It measures
generator latency rather than any value derived from the sequence, so
it is left alone.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test.c                        | 48 ++++++++++++++++++++++++++
 doc/guides/contributing/unit_test.rst  | 15 ++++++++
 doc/guides/rel_notes/release_26_11.rst |  2 ++
 3 files changed, 65 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index c610c3588e..176e4fa1f5 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <stdint.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <ctype.h>
@@ -21,6 +22,7 @@ extern cmdline_parse_ctx_t main_ctx[];
 #include <rte_eal.h>
 #include <rte_cycles.h>
 #include <rte_log.h>
+#include <rte_random.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TIMER
 #include <rte_timer.h>
@@ -107,6 +109,33 @@ do_recursive_call(void)
 
 int last_test_result;
 
+/* Tests that use rte_rand() get a different sequence on every run, which
+ * is good for coverage but makes it hard to compare performance results.
+ * Setting DPDK_TEST_SEED gives a repeatable sequence instead.
+ */
+static bool use_test_seed;
+static uint64_t test_seed;
+
+static int
+get_test_seed(void)
+{
+	const char *env = getenv("DPDK_TEST_SEED");
+	char *end;
+
+	if (env == NULL || *env == '\0')
+		return 0;
+
+	errno = 0;
+	test_seed = strtoull(env, &end, 0);
+	if (errno != 0 || end == env || *end != '\0') {
+		fprintf(stderr, "Invalid DPDK_TEST_SEED: '%s'\n", env);
+		return -1;
+	}
+
+	use_test_seed = true;
+	return 0;
+}
+
 #define MAX_EXTRA_ARGS 32
 
 int
@@ -175,8 +204,18 @@ main(int argc, char **argv)
 		goto out;
 	}
 
+	if (get_test_seed() < 0) {
+		ret = -1;
+		goto out;
+	}
+
 	recursive_call = getenv(RECURSIVE_ENV_VAR);
 	if (recursive_call != NULL) {
+		/* Child instances inherit the environment, so seed them
+		 * too rather than leaving them randomly seeded.
+		 */
+		if (use_test_seed)
+			rte_srand(test_seed);
 		ret = do_recursive_call();
 		goto out;
 	}
@@ -233,6 +272,12 @@ main(int argc, char **argv)
 				}
 			}
 
+			/* Reseed before each test so that a test gets the same
+			 * sequence no matter what ran before it.
+			 */
+			if (use_test_seed)
+				rte_srand(test_seed);
+
 			snprintf(buf, sizeof(buf), "%s\n", tests[i]);
 			if (cmdline_parse_check(cl, buf) < 0) {
 				printf("Error: invalid test command: '%s'\n", tests[i]);
@@ -260,6 +305,9 @@ main(int argc, char **argv)
 			goto out;
 		}
 
+		if (use_test_seed)
+			rte_srand(test_seed);
+
 		cmdline_interact(cl);
 		cmdline_stdin_exit(cl);
 	}
diff --git a/doc/guides/contributing/unit_test.rst b/doc/guides/contributing/unit_test.rst
index f13ddff291..5d08fe1792 100644
--- a/doc/guides/contributing/unit_test.rst
+++ b/doc/guides/contributing/unit_test.rst
@@ -114,6 +114,21 @@ via the ``DPDK_TEST_PARAMS`` argument,
 in case some tests need additional configuration.
 This is not currently used in the Meson test suites.
 
+Many tests build their input data with ``rte_rand()``,
+which produces a different sequence on every run.
+This is useful for coverage, but it adds noise when comparing results.
+Setting ``DPDK_TEST_SEED`` to an integer seeds the generator with that
+value before each test command is run, so that a test gets the same input
+data no matter what ran before it::
+
+   $ DPDK_TEST_SEED=42 ./build/app/test/dpdk-test lpm_perf_autotest
+
+This also works when running the tests via Meson::
+
+   $ DPDK_TEST_SEED=42 meson test -C build --suite perf-tests
+
+When the variable is not set, the generator keeps its default random seeding.
+
 
 Running test cases via Meson
 ----------------------------
diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 9d1238d6f3..d315d426f4 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -69,6 +69,8 @@ New Features
     64 bits of the value passed to ``rte_srand()`` now affect the state,
     so a given seed produces a different sequence than in previous
     releases.
+  * The test application now accepts a ``DPDK_TEST_SEED`` environment
+    variable to make runs using ``rte_rand()`` repeatable.
 
 
 Removed Items
-- 
2.53.0

[PATCH 17/18] test: add random number generator tests

[Stephen Hemminger]

Add functional tests for rte_random_bytes() and rte_rand32(). The
rte_random_bytes() cases cover a zero length request, requests smaller
than the internal chunk size, a request large enough to be split into
several chunks, and that two calls do not return the same bytes. Each
case also checks that nothing is written past the requested length.

The rte_rand32() cases check that every bit position is both set and
clear across a number of draws, and that rte_srand() makes the
sequence repeatable.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/meson.build   |   1 +
 app/test/test_random.c | 297 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 298 insertions(+)
 create mode 100644 app/test/test_random.c

diff --git a/app/test/meson.build b/app/test/meson.build
index 51abeeb732..794e4e5962 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -158,6 +158,7 @@ source_file_deps = {
     'test_prefetch.c': [],
     'test_ptr_compress.c': ['ptr_compress'],
     'test_rand_perf.c': [],
+    'test_random.c': [],
     'test_rawdev.c': ['rawdev', 'bus_vdev', 'raw_skeleton'],
     'test_rcu_qsbr.c': ['rcu', 'hash'],
     'test_rcu_qsbr_perf.c': ['rcu', 'hash'],
diff --git a/app/test/test_random.c b/app/test/test_random.c
new file mode 100644
index 0000000000..f0084f5abb
--- /dev/null
+++ b/app/test/test_random.c
@@ -0,0 +1,297 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Stephen Hemminger
+ */
+
+#include <inttypes.h>
+#include <stdbool.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_random.h>
+
+#include "test.h"
+
+/* Larger than the chunk size rte_random_bytes() splits requests into,
+ * and not a multiple of it, so the loop is exercised.
+ */
+#define BIG_LEN (3 * 256 + 7)
+
+/* Fill pattern, any byte left untouched by the call is visible. */
+#define FILL_BYTE 0xa5
+
+static bool
+all_equal(const uint8_t *buf, size_t len, uint8_t val)
+{
+	size_t i;
+
+	for (i = 0; i < len; i++)
+		if (buf[i] != val)
+			return false;
+
+	return true;
+}
+
+static int
+test_random_bytes_zero(void)
+{
+	uint8_t buf[8];
+
+	memset(buf, FILL_BYTE, sizeof(buf));
+
+	TEST_ASSERT(rte_random_bytes(buf, 0) == 0,
+		    "zero length request failed");
+	TEST_ASSERT(all_equal(buf, sizeof(buf), FILL_BYTE),
+		    "zero length request wrote to the buffer");
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_random_bytes_small(void)
+{
+	uint8_t buf[32];
+	unsigned int i;
+
+	/* A single byte is worth checking on its own, it is the
+	 * smallest request that must still be filled.
+	 */
+	for (i = 1; i <= sizeof(buf); i++) {
+		memset(buf, FILL_BYTE, sizeof(buf));
+
+		TEST_ASSERT(rte_random_bytes(buf, i) == 0,
+			    "request of %u bytes failed", i);
+		TEST_ASSERT(all_equal(buf + i, sizeof(buf) - i, FILL_BYTE),
+			    "request of %u bytes wrote past the end", i);
+	}
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_random_bytes_large(void)
+{
+	uint8_t buf[BIG_LEN + 8];
+
+	memset(buf, FILL_BYTE, sizeof(buf));
+
+	/* Must be split internally and still filled completely. */
+	TEST_ASSERT(rte_random_bytes(buf, BIG_LEN) == 0,
+		    "request of %d bytes failed", BIG_LEN);
+	TEST_ASSERT(all_equal(buf + BIG_LEN, sizeof(buf) - BIG_LEN, FILL_BYTE),
+		    "request of %d bytes wrote past the end", BIG_LEN);
+	TEST_ASSERT(!all_equal(buf, BIG_LEN, 0),
+		    "request of %d bytes returned all zero", BIG_LEN);
+	TEST_ASSERT(!all_equal(buf, BIG_LEN, FILL_BYTE),
+		    "request of %d bytes left the buffer untouched", BIG_LEN);
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_random_bytes_distinct(void)
+{
+	uint8_t a[64], b[64];
+
+	TEST_ASSERT(rte_random_bytes(a, sizeof(a)) == 0, "first call failed");
+	TEST_ASSERT(rte_random_bytes(b, sizeof(b)) == 0, "second call failed");
+
+	/* The chance of a false failure here is 2^-512. */
+	TEST_ASSERT(memcmp(a, b, sizeof(a)) != 0,
+		    "two calls returned the same bytes");
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_rand(void)
+{
+	unsigned int i;
+	uint64_t any_set = 0;
+	uint64_t always_set = UINT64_MAX;
+
+	/* Every bit position should be both set and clear at some
+	 * point, which catches a generator that is stuck or that only
+	 * ever fills part of the word.
+	 */
+	for (i = 0; i < 100; i++) {
+		uint64_t v = rte_rand();
+
+		any_set |= v;
+		always_set &= v;
+	}
+
+	TEST_ASSERT(any_set == UINT64_MAX,
+		    "some bits were never set: %#" PRIx64, any_set);
+	TEST_ASSERT(always_set == 0,
+		    "some bits were always set: %#" PRIx64, always_set);
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_rand32(void)
+{
+	unsigned int i;
+	uint32_t any_set = 0;
+	uint32_t always_set = UINT32_MAX;
+
+	/* Every bit position should be both set and clear at some
+	 * point, which catches a generator that is stuck or that only
+	 * ever fills part of the word.
+	 */
+	for (i = 0; i < 100; i++) {
+		uint32_t v = rte_rand32();
+
+		any_set |= v;
+		always_set &= v;
+	}
+
+	TEST_ASSERT(any_set == UINT32_MAX,
+		    "some bits were never set: %#x", any_set);
+	TEST_ASSERT(always_set == 0,
+		    "some bits were always set: %#x", always_set);
+
+	return TEST_SUCCESS;
+}
+
+/* Bounds worth covering: the power of two case is handled by masking,
+ * everything else goes through the rejection loop.
+ */
+static const uint64_t rand_max_bounds[] = {
+	2, 3, 17, 64, 1000, 1ULL << 32, (1ULL << 63) + 1, UINT64_MAX,
+};
+
+static int
+test_rand_max(void)
+{
+	unsigned int i, j;
+
+	/* A bound of one leaves only a single legal value. */
+	for (i = 0; i < 100; i++)
+		TEST_ASSERT(rte_rand_max(1) == 0,
+			    "upper bound of 1 returned a non-zero value");
+
+	for (i = 0; i < RTE_DIM(rand_max_bounds); i++) {
+		uint64_t bound = rand_max_bounds[i];
+
+		for (j = 0; j < 10000; j++) {
+			uint64_t v = rte_rand_max(bound);
+
+			TEST_ASSERT(v < bound,
+				    "value %" PRIu64 " is not less than the upper bound %" PRIu64,
+				    v, bound);
+		}
+	}
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_rand_max_spread(void)
+{
+	unsigned int i;
+	bool seen[3] = { false, false, false };
+
+	/* With a small bound every value has to show up quickly. The
+	 * chance of missing one in 100 draws is 3 * (2/3)^100.
+	 */
+	for (i = 0; i < 100; i++)
+		seen[rte_rand_max(RTE_DIM(seen))] = true;
+
+	for (i = 0; i < RTE_DIM(seen); i++)
+		TEST_ASSERT(seen[i], "value %u was never generated", i);
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_drand(void)
+{
+	unsigned int i;
+	bool low = false, high = false;
+
+	for (i = 0; i < 100; i++) {
+		double v = rte_drand();
+
+		TEST_ASSERT(v >= 0.0 && v < 1.0,
+			    "value %f is outside of [0.0, 1.0)", v);
+
+		if (v < 0.5)
+			low = true;
+		else
+			high = true;
+	}
+
+	/* Both halves of the interval have to be used, which catches a
+	 * generator stuck on a constant. Missing one has a probability
+	 * of 2^-100.
+	 */
+	TEST_ASSERT(low, "no value below 0.5 was generated");
+	TEST_ASSERT(high, "no value of 0.5 or above was generated");
+
+	return TEST_SUCCESS;
+}
+
+/* Record a sequence that uses every generator function, so that a
+ * function left out of the shared state would show up here.
+ */
+static void
+record_sequence(uint64_t seed, uint64_t *out, unsigned int n)
+{
+	unsigned int i;
+
+	rte_srand(seed);
+
+	for (i = 0; i < n; i += 4) {
+		out[i] = rte_rand();
+		out[i + 1] = rte_rand32();
+		out[i + 2] = rte_rand_max(1000);
+		out[i + 3] = (uint64_t)(rte_drand() * (1ULL << 53));
+	}
+}
+
+static int
+test_srand_repeatable(void)
+{
+	uint64_t first[16], second[16];
+
+	record_sequence(42, first, RTE_DIM(first));
+	record_sequence(42, second, RTE_DIM(second));
+
+	TEST_ASSERT(memcmp(first, second, sizeof(first)) == 0,
+		    "same seed produced a different sequence");
+
+	record_sequence(43, second, RTE_DIM(second));
+
+	TEST_ASSERT(memcmp(first, second, sizeof(first)) != 0,
+		    "different seed produced the same sequence");
+
+	return TEST_SUCCESS;
+}
+
+static struct unit_test_suite random_test_suite = {
+	.suite_name = "random autotest",
+	.setup = NULL,
+	.teardown = NULL,
+	.unit_test_cases = {
+		TEST_CASE(test_random_bytes_zero),
+		TEST_CASE(test_random_bytes_small),
+		TEST_CASE(test_random_bytes_large),
+		TEST_CASE(test_random_bytes_distinct),
+		TEST_CASE(test_rand),
+		TEST_CASE(test_rand32),
+		TEST_CASE(test_rand_max),
+		TEST_CASE(test_rand_max_spread),
+		TEST_CASE(test_drand),
+		TEST_CASE(test_srand_repeatable),
+		TEST_CASES_END()
+	}
+};
+
+static int
+test_random(void)
+{
+	return unit_test_suite_runner(&random_test_suite);
+}
+
+REGISTER_FAST_TEST(random_autotest, NOHUGE_OK, ASAN_OK, test_random);
-- 
2.53.0

[PATCH 18/18] doc: recommend secure random for unpredictable values

[Stephen Hemminger]

The AI review checks say not to generate keys, IVs and nonces with
rte_rand(), but do not say what to use instead. Name
rte_random_bytes() as the replacement, and extend the rule to the
other values which must not be predictable.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 AGENTS.md | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/AGENTS.md b/AGENTS.md
index 813584352b..df4c37f62e 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -544,6 +544,9 @@ or authentication digests. Stays silent on all other patches.
 - **Insecure RNG for keys/IVs** (Error):
   rte_rand()/rand()/random() are not cryptographically secure;
   do not use them to generate keys, IVs, or nonces in crypto/security code.
+  Use rte_random_bytes(), which draws from the random source of the
+  operating system. The same applies to other values that must not be
+  predictable, such as RSS and hash keys, salts and random MAC addresses.
 
 Do NOT flag:
 - IV/nonce reuse -- a runtime property, not determinable from a patch.
@@ -1680,7 +1683,7 @@ Checked by `devtools/checkpatches.sh` -- not duplicated here.
 - [ ] Memory ordering is the weakest correct choice (`relaxed` for counters, `acquire`/`release` for publish/consume)
 - [ ] Auth tag/digest comparisons use rte_memeq_timingsafe(), not memcmp
 - [ ] Key material zeroed before free (rte_free_sensitive / rte_memzero_explicit)
-- [ ] Keys/IVs/nonces not generated with rte_rand()/rand()/random()
+- [ ] Keys/IVs/nonces not generated with rte_rand()/rand()/random(), use rte_random_bytes()
 
 ### API Tags
 
@@ -1795,6 +1798,7 @@ devtools/get-maintainer.sh <patch-file>
 - Non-constant-time comparison of auth tag/digest/MAC/ICV (timing side channel)
 - Key material or session secrets freed without zeroing (rte_free_sensitive/rte_memzero_explicit)
 - Non-cryptographic RNG (rte_rand/rand/random) used to generate keys, IVs, or nonces
+  instead of rte_random_bytes()
 
 *Process and format errors:*
 - Forbidden tokens in code
-- 
2.53.0

Re: [PATCH 15/18] random: remove RDSEED fallback for initial seed

[Bruce Richardson]

On Tue, Sep 15, 2026 at 12:21:13PM -0700, Stephen Hemminger wrote:
> RDSEED is an x86 instruction that returns random bits from a hardware
> source. It has well known reliability problems: it can fail
> persistently when many cores use it at once, and several processors
> have errata affecting it. Since commit bfb3c5b9ffe7 ("random: always
> use getentropy") it was only reachable if getentropy() failed, which
> does not happen in practice.
> 
> Nothing is lost by removing it. The kernel already mixes RDSEED into
> its own entropy pool as one of several sources, so what it provides is
> present in the getentropy() result anyway, combined with other sources
> rather than trusted on its own.
> 
> The remaining fallback, the timestamp counter, still logs an error so
> that a low entropy seed is reported.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH 18/18] doc: recommend secure random for unpredictable values

[Aaron Conole]

Stephen Hemminger <stephen@networkplumber.org> writes:

> The AI review checks say not to generate keys, IVs and nonces with
> rte_rand(), but do not say what to use instead. Name
> rte_random_bytes() as the replacement, and extend the rule to the
> other values which must not be predictable.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

LGTM,

Acked-by: Aaron Conole <aconole@redhat.com>

[PATCH v2 0/5] random: fix initial seed entropy

[Stephen Hemminger]

This series is about how the random number generator is seeded: where
the seed comes from, how it is expanded into the generator state, and
being able to pin it for repeatable test runs.

The build detection of getentropy() does not work, so DPDK has been
seeding its PRNG from RDSEED or the TSC on systems that do have
getentropy(). cc.has_function() probes with -std=c11, but glibc hides
getentropy() behind _DEFAULT_SOURCE, so the probe always fails.

A TSC derived seed only varies by process start time, which a local
attacker can determine.

  1/5 adds a Windows getentropy() shim over BCryptGenRandom(), so the
      EAL can call getentropy() unconditionally.
  2/5 drops the broken probe and always uses getentropy(). The TSC
      fallback remains but now logs at ERR with the errno, so a
      blocked syscall can be told apart from other failures.
  3/5 expands the seed with SplitMix64. The two halves of the seed
      were folded together and run through a 32 bit LCG, so only 32
      bits of the seed had any effect and the five state words had
      correlated low bits. Per-lcore seeds were the seed plus the
      lcore id, leaving neighbouring lcores with nearly identical
      state; that sum now goes through SplitMix64 as well. A given
      seed therefore produces a different sequence than in previous
      releases.
  4/5 removes the RDSEED fallback. It is unreachable once getentropy()
      is always used, and RDSEED has known reliability problems and
      errata. The kernel already mixes RDSEED into the pool that
      getentropy() draws from.
  5/5 adds a DPDK_TEST_SEED environment variable to the test
      application, so runs that build input data with rte_rand() can
      be made repeatable. Unset behaves as before.

Bugzilla ID: 2035

v2: - rebased
    - reduced patch series to only seedinr related changes

Two checks warn deliberately:

  1/5 carries Cc: stable@dpdk.org without a Fixes: tag. 2/5 is the
  stable candidate and does not build on Windows without the shim, so
  1/5 has to go back with it. The commit message says so.

  3/5 has a Fixes: tag and no Cc: stable. It changes the sequence a
  given seed produces, so a backport would change output for anyone
  relying on rte_srand() reproducibility on a stable branch.

Stephen Hemminger (5):
  eal/windows: add getentropy shim
  random: always use getentropy
  random: use all 64 bits of the seed
  random: remove RDSEED fallback for initial seed
  test: allow setting random seed for tests

 app/test/test.c                        | 48 +++++++++++++
 doc/guides/contributing/unit_test.rst  | 15 ++++
 doc/guides/rel_notes/release_26_11.rst | 10 +++
 lib/eal/common/rte_random.c            | 98 +++++++++++---------------
 lib/eal/include/rte_random.h           |  7 +-
 lib/eal/meson.build                    |  3 -
 lib/eal/windows/include/rte_os_shim.h  | 30 ++++++++
 lib/eal/windows/meson.build            |  6 +-
 8 files changed, 155 insertions(+), 62 deletions(-)

-- 
2.53.0

[PATCH v2 1/5] eal/windows: add getentropy shim

[Stephen Hemminger]

Provide getentropy() in the Windows OS shim using BCryptGenRandom()
with the system preferred generator, which needs no algorithm
provider handle. The POSIX 256 byte limit is kept so that callers
see the same behaviour on all platforms.

This is needed EAL change that always uses getentropy().

The next patch is tagged for stable, and it does not build on
Windows without this shim, so this one is tagged as well.

Bugzilla ID: 2035
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/rte_random.c           |  1 +
 lib/eal/windows/include/rte_os_shim.h | 30 +++++++++++++++++++++++++++
 lib/eal/windows/meson.build           |  6 +++++-
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 576a32a46c..537fa035ee 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -17,6 +17,7 @@
 #include <rte_random.h>
 
 #include <eal_export.h>
+#include <rte_os_shim.h>
 #include "eal_private.h"
 
 struct __rte_cache_aligned rte_rand_state {
diff --git a/lib/eal/windows/include/rte_os_shim.h b/lib/eal/windows/include/rte_os_shim.h
index 44664a5062..4f9611d642 100644
--- a/lib/eal/windows/include/rte_os_shim.h
+++ b/lib/eal/windows/include/rte_os_shim.h
@@ -3,12 +3,16 @@
 #ifndef _RTE_OS_SHIM_
 #define _RTE_OS_SHIM_
 
+#include <errno.h>
 #include <time.h>
 
 #include <rte_os.h>
 #include <rte_windows.h>
 #include <getline.h>
 
+/* Needs Windows SDK types, so must come after rte_windows.h. */
+#include <bcrypt.h>
+
 /**
  * @file
  * @internal
@@ -124,4 +128,30 @@ rte_localtime_r(const time_t *timep, struct tm *result)
 }
 #define localtime_r(timep, result) rte_localtime_r(timep, result)
 
+/*
+ * Windows has no getentropy(), use the system preferred random
+ * generator which does not require a provider handle.
+ */
+static inline int
+rte_getentropy(void *buffer, size_t length)
+{
+	NTSTATUS status;
+
+	/* Match the POSIX limit so callers behave the same everywhere. */
+	if (length > 256) {
+		errno = EIO;
+		return -1;
+	}
+
+	status = BCryptGenRandom(NULL, (PUCHAR)buffer, (ULONG)length,
+				 BCRYPT_USE_SYSTEM_PREFERRED_RNG);
+	if (!BCRYPT_SUCCESS(status)) {
+		errno = EIO;
+		return -1;
+	}
+
+	return 0;
+}
+#define getentropy(buffer, length) rte_getentropy(buffer, length)
+
 #endif /* _RTE_OS_SHIM_ */
diff --git a/lib/eal/windows/meson.build b/lib/eal/windows/meson.build
index 6f909c2131..6387786530 100644
--- a/lib/eal/windows/meson.build
+++ b/lib/eal/windows/meson.build
@@ -37,7 +37,11 @@ ws2_32_dep = cc.find_library('ws2_32')
 if not cc.links(min_c_code, dependencies: ws2_32_dep)
     error('broken dependency, "ws2_32"')
 endif
-ext_deps += [dbghelp_dep, setupapi_dep, ws2_32_dep]
+bcrypt_dep = cc.find_library('bcrypt')
+if not cc.links(min_c_code, dependencies: bcrypt_dep)
+    error('broken dependency, "bcrypt"')
+endif
+ext_deps += [dbghelp_dep, setupapi_dep, ws2_32_dep, bcrypt_dep]
 if is_ms_linker
         # Contrary to docs, VirtualAlloc2() is exported by mincore.lib.
         mincore_dep = cc.find_library('mincore')
-- 
2.53.0

[PATCH v2 2/5] random: always use getentropy

[Stephen Hemminger]

The current meson build check for getentropy support is broken
and getentropy never gets used!

The cc.has_function() checks with -std=c11 but glibc hides
getentropy behind _DEFAULT_SOURCE. Therefore the probe always
failed, and code would fallback to RDSEED or TSC
for the initial PRNG seed.

Now that a getentropy() wrapper exists for Windows it can always be used.

Falling back to the TSC now logs at ERR level, including the errno
from getentropy() so that a blocked syscall can be told apart from
other failures. A TSC derived seed only varies by process start time,
which is easily determined by local attacker.

A backport needs the preceding patch which adds the Windows
getentropy() shim, otherwise the Windows build breaks.

Bugzilla ID: 2035
Fixes: faf8fd252785 ("eal: improve entropy for initial PRNG seed")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_11.rst | 4 ++++
 lib/eal/common/rte_random.c            | 6 ++++--
 lib/eal/include/rte_random.h           | 7 ++++---
 lib/eal/meson.build                    | 3 ---
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 88c06025b0..9bef8b4774 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -158,6 +158,10 @@ New Features
   Added ``rte_bbdev_queue_stats_get()`` function to retrieve statistics
   for a specific queue, complementing the existing device-level statistics API.
 
+* **Updated random number generation.**
+
+  * The initial seed is now always taken from ``getentropy()``.
+
 
 Removed Items
 -------------
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 537fa035ee..b036c50349 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -7,6 +7,8 @@
 #include <x86intrin.h>
 #endif
 #endif
+#include <errno.h>
+#include <string.h>
 #include <unistd.h>
 
 #include <rte_bitops.h>
@@ -219,7 +221,6 @@ rte_drand(void)
 static uint64_t
 __rte_random_initial_seed(void)
 {
-#ifdef RTE_LIBEAL_USE_GETENTROPY
 	int ge_rc;
 	uint64_t ge_seed;
 
@@ -227,7 +228,6 @@ __rte_random_initial_seed(void)
 
 	if (ge_rc == 0)
 		return ge_seed;
-#endif
 #ifdef __RDSEED__
 	unsigned int rdseed_low;
 	unsigned int rdseed_high;
@@ -238,6 +238,8 @@ __rte_random_initial_seed(void)
 		return (uint64_t)rdseed_low | ((uint64_t)rdseed_high << 32);
 #endif
 	/* second fallback: seed using rdtsc */
+	EAL_LOG(ERR, "getentropy() failed (%s), seeding PRNG from TSC: seed has low entropy",
+		strerror(errno));
 	return rte_get_tsc_cycles();
 }
 
diff --git a/lib/eal/include/rte_random.h b/lib/eal/include/rte_random.h
index 15cbe6215a..978ecda203 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -20,9 +20,10 @@ extern "C" {
 /**
  * Seed the pseudo-random generator.
  *
- * The generator is automatically seeded by the EAL init with a timer
- * value. It may need to be re-seeded by the user with a real random
- * value.
+ * The generator is automatically seeded by the EAL init from the
+ * random source provided by the operating system, so there is no need
+ * to re-seed it to get unpredictable values. Seeding it explicitly is
+ * useful to make a run repeatable.
  *
  * This function is not multi-thread safe in regards to other
  * rte_srand() calls, nor is it in relation to concurrent rte_rand(),
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 092f5c5261..a261bdd48b 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -22,9 +22,6 @@ endif
 if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
     ext_deps += libarchive
 endif
-if cc.has_function('getentropy', prefix : '#include <unistd.h>')
-    cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
-endif
 
 if is_freebsd
     annotate_locks = false
-- 
2.53.0

[PATCH v2 3/5] random: use all 64 bits of the seed

[Stephen Hemminger]

The generator state is five 64 bit words, expanded from the seed by a
32 bit linear congruential generator. The two halves of the seed were
folded together first, so only 32 bits of it had any effect, and the
low order bits of such a generator are weak, leaving the five words
with correlated low bits.

Expand the seed with SplitMix64 instead, so every bit of the seed
affects every word of the state. It is described in "Fast Splittable
Pseudorandom Number Generators" by Steele, Lea and Flood,
https://doi.org/10.1145/2714064.2660195 and is what the authors of
the xoshiro and xoroshiro generators recommend for seeding.

The per-lcore seeds were formed by adding the lcore id to the seed,
leaving neighbouring lcores with nearly identical state. Pass that
sum through SplitMix64 as well.

Fixes: 3f002f069612 ("eal: replace libc-based random generation with LFSR")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_11.rst |  4 ++
 lib/eal/common/rte_random.c            | 74 +++++++++++++-------------
 2 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 9bef8b4774..2d12eb7949 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -161,6 +161,10 @@ New Features
 * **Updated random number generation.**
 
   * The initial seed is now always taken from ``getentropy()``.
+  * Improved the expansion of the seed into the generator state. All
+    64 bits of the value passed to ``rte_srand()`` now affect the state,
+    so a given seed produces a different sequence than in previous
+    releases.
 
 
 Removed Items
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index b036c50349..3c5b36aa2c 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -35,55 +35,45 @@ static RTE_LCORE_VAR_HANDLE(struct rte_rand_state, rand_state);
 /* instance to be shared by all unregistered non-EAL threads */
 static struct rte_rand_state unregistered_rand_state;
 
-static uint32_t
-__rte_rand_lcg32(uint32_t *seed)
-{
-	*seed = 1103515245U * *seed + 12345U;
-
-	return *seed;
-}
-
+/* SplitMix64, used to expand the seed into the generator state.
+ * It has a full 64 bit period and good avalanche, so all the bits
+ * of the seed affect every word of the resulting state.
+ *
+ * See "Fast Splittable Pseudorandom Number Generators" by Steele,
+ * Lea and Flood, https://doi.org/10.1145/2714064.2660195
+ */
 static uint64_t
-__rte_rand_lcg64(uint32_t *seed)
+__rte_rand_splitmix64(uint64_t *state)
 {
-	uint64_t low;
-	uint64_t high;
-
-	/* A 64-bit LCG would have been much cleaner, but good
-	 * multiplier/increments for such seem hard to come by.
-	 */
+	uint64_t z;
 
-	low = __rte_rand_lcg32(seed);
-	high = __rte_rand_lcg32(seed);
+	z = (*state += 0x9E3779B97F4A7C15ULL);
+	z = (z ^ (z >> 30)) * 0xBF58476D1CE4E5B9ULL;
+	z = (z ^ (z >> 27)) * 0x94D049BB133111EBULL;
 
-	return low | (high << 32);
+	return z ^ (z >> 31);
 }
 
 static uint64_t
-__rte_rand_lfsr258_gen_seed(uint32_t *seed, uint64_t min_value)
+__rte_rand_lfsr258_gen_seed(uint64_t *state, uint64_t min_value)
 {
-	uint64_t res;
-
-	res = __rte_rand_lcg64(seed);
-
-	if (res < min_value)
-		res += min_value;
-
-	return res;
+	/* LFSR258 degenerates unless each word exceeds its threshold.
+	 * All thresholds are powers of two, so a bitwise or is enough
+	 * and keeps the remaining bits untouched.
+	 */
+	return __rte_rand_splitmix64(state) | min_value;
 }
 
 static void
 __rte_srand_lfsr258(uint64_t seed, struct rte_rand_state *state)
 {
-	uint32_t lcg_seed;
-
-	lcg_seed = (uint32_t)(seed ^ (seed >> 32));
+	uint64_t mix_state = seed;
 
-	state->z1 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 2UL);
-	state->z2 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 512UL);
-	state->z3 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 4096UL);
-	state->z4 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 131072UL);
-	state->z5 = __rte_rand_lfsr258_gen_seed(&lcg_seed, 8388608UL);
+	state->z1 = __rte_rand_lfsr258_gen_seed(&mix_state, 2UL);
+	state->z2 = __rte_rand_lfsr258_gen_seed(&mix_state, 512UL);
+	state->z3 = __rte_rand_lfsr258_gen_seed(&mix_state, 4096UL);
+	state->z4 = __rte_rand_lfsr258_gen_seed(&mix_state, 131072UL);
+	state->z5 = __rte_rand_lfsr258_gen_seed(&mix_state, 8388608UL);
 }
 
 RTE_EXPORT_SYMBOL(rte_srand)
@@ -91,16 +81,24 @@ void
 rte_srand(uint64_t seed)
 {
 	unsigned int lcore_id;
+	uint64_t mix_state;
 
-	/* add lcore_id to seed to avoid having the same sequence */
+	/* Mix in the lcore id so that each lcore gets an unrelated
+	 * sequence. Adding it to the seed would leave neighbouring
+	 * lcores with nearly identical generator state.
+	 */
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		struct rte_rand_state *lcore_state =
 			RTE_LCORE_VAR_LCORE(lcore_id, rand_state);
 
-		__rte_srand_lfsr258(seed + lcore_id, lcore_state);
+		mix_state = seed + lcore_id;
+		__rte_srand_lfsr258(__rte_rand_splitmix64(&mix_state),
+				    lcore_state);
 	}
 
-	__rte_srand_lfsr258(seed + lcore_id, &unregistered_rand_state);
+	mix_state = seed + lcore_id;
+	__rte_srand_lfsr258(__rte_rand_splitmix64(&mix_state),
+			    &unregistered_rand_state);
 }
 
 static __rte_always_inline uint64_t
-- 
2.53.0

[PATCH v2 4/5] random: remove RDSEED fallback for initial seed

[Stephen Hemminger]

RDSEED is an x86 instruction that returns random bits from a hardware
source. It has well known reliability problems: it can fail
persistently when many cores use it at once, and several processors
have errata affecting it. Since "random: always use getentropy"
earlier in this series it was only reachable if getentropy() failed,
which does not happen in practice.

Nothing is lost by removing it. The kernel already mixes RDSEED into
its own entropy pool as one of several sources, so what it provides is
present in the getentropy() result anyway, combined with other sources
rather than trusted on its own.

The remaining fallback, the timestamp counter, still logs an error so
that a low entropy seed is reported.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/rte_random.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 3c5b36aa2c..c14df2cce7 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -2,11 +2,6 @@
  * Copyright(c) 2019 Ericsson AB
  */
 
-#ifdef __RDSEED__
-#ifndef RTE_TOOLCHAIN_MSVC
-#include <x86intrin.h>
-#endif
-#endif
 #include <errno.h>
 #include <string.h>
 #include <unistd.h>
@@ -226,16 +221,8 @@ __rte_random_initial_seed(void)
 
 	if (ge_rc == 0)
 		return ge_seed;
-#ifdef __RDSEED__
-	unsigned int rdseed_low;
-	unsigned int rdseed_high;
-
-	/* first fallback: rdseed instruction, if available */
-	if (_rdseed32_step(&rdseed_low) == 1 &&
-	    _rdseed32_step(&rdseed_high) == 1)
-		return (uint64_t)rdseed_low | ((uint64_t)rdseed_high << 32);
-#endif
-	/* second fallback: seed using rdtsc */
+
+	/* fallback: seed using rdtsc */
 	EAL_LOG(ERR, "getentropy() failed (%s), seeding PRNG from TSC: seed has low entropy",
 		strerror(errno));
 	return rte_get_tsc_cycles();
-- 
2.53.0

[PATCH v2 5/5] test: allow setting random seed for tests

[Stephen Hemminger]

Many tests build their input data with rte_rand(), which is seeded
from the system entropy source and therefore produces a different
sequence on every run. Varying the data is good for coverage, but it
adds noise when comparing performance results between two builds.

Add a DPDK_TEST_SEED environment variable to the test application that
seeds the generator with a fixed value. The generator is reseeded
before each test is dispatched, so a test gets the same input data no
matter what ran before it in the same invocation. When the variable is
not set the behaviour is unchanged and each run gets a random seed.

Note that rand_perf_autotest calls rte_srand() itself. It measures
generator latency rather than any value derived from the sequence, so
it is left alone.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test.c                        | 48 ++++++++++++++++++++++++++
 doc/guides/contributing/unit_test.rst  | 15 ++++++++
 doc/guides/rel_notes/release_26_11.rst |  2 ++
 3 files changed, 65 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index c610c3588e..176e4fa1f5 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <stdint.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <ctype.h>
@@ -21,6 +22,7 @@ extern cmdline_parse_ctx_t main_ctx[];
 #include <rte_eal.h>
 #include <rte_cycles.h>
 #include <rte_log.h>
+#include <rte_random.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TIMER
 #include <rte_timer.h>
@@ -107,6 +109,33 @@ do_recursive_call(void)
 
 int last_test_result;
 
+/* Tests that use rte_rand() get a different sequence on every run, which
+ * is good for coverage but makes it hard to compare performance results.
+ * Setting DPDK_TEST_SEED gives a repeatable sequence instead.
+ */
+static bool use_test_seed;
+static uint64_t test_seed;
+
+static int
+get_test_seed(void)
+{
+	const char *env = getenv("DPDK_TEST_SEED");
+	char *end;
+
+	if (env == NULL || *env == '\0')
+		return 0;
+
+	errno = 0;
+	test_seed = strtoull(env, &end, 0);
+	if (errno != 0 || end == env || *end != '\0') {
+		fprintf(stderr, "Invalid DPDK_TEST_SEED: '%s'\n", env);
+		return -1;
+	}
+
+	use_test_seed = true;
+	return 0;
+}
+
 #define MAX_EXTRA_ARGS 32
 
 int
@@ -175,8 +204,18 @@ main(int argc, char **argv)
 		goto out;
 	}
 
+	if (get_test_seed() < 0) {
+		ret = -1;
+		goto out;
+	}
+
 	recursive_call = getenv(RECURSIVE_ENV_VAR);
 	if (recursive_call != NULL) {
+		/* Child instances inherit the environment, so seed them
+		 * too rather than leaving them randomly seeded.
+		 */
+		if (use_test_seed)
+			rte_srand(test_seed);
 		ret = do_recursive_call();
 		goto out;
 	}
@@ -233,6 +272,12 @@ main(int argc, char **argv)
 				}
 			}
 
+			/* Reseed before each test so that a test gets the same
+			 * sequence no matter what ran before it.
+			 */
+			if (use_test_seed)
+				rte_srand(test_seed);
+
 			snprintf(buf, sizeof(buf), "%s\n", tests[i]);
 			if (cmdline_parse_check(cl, buf) < 0) {
 				printf("Error: invalid test command: '%s'\n", tests[i]);
@@ -260,6 +305,9 @@ main(int argc, char **argv)
 			goto out;
 		}
 
+		if (use_test_seed)
+			rte_srand(test_seed);
+
 		cmdline_interact(cl);
 		cmdline_stdin_exit(cl);
 	}
diff --git a/doc/guides/contributing/unit_test.rst b/doc/guides/contributing/unit_test.rst
index f13ddff291..5d08fe1792 100644
--- a/doc/guides/contributing/unit_test.rst
+++ b/doc/guides/contributing/unit_test.rst
@@ -114,6 +114,21 @@ via the ``DPDK_TEST_PARAMS`` argument,
 in case some tests need additional configuration.
 This is not currently used in the Meson test suites.
 
+Many tests build their input data with ``rte_rand()``,
+which produces a different sequence on every run.
+This is useful for coverage, but it adds noise when comparing results.
+Setting ``DPDK_TEST_SEED`` to an integer seeds the generator with that
+value before each test command is run, so that a test gets the same input
+data no matter what ran before it::
+
+   $ DPDK_TEST_SEED=42 ./build/app/test/dpdk-test lpm_perf_autotest
+
+This also works when running the tests via Meson::
+
+   $ DPDK_TEST_SEED=42 meson test -C build --suite perf-tests
+
+When the variable is not set, the generator keeps its default random seeding.
+
 
 Running test cases via Meson
 ----------------------------
diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 2d12eb7949..5d5db46190 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -165,6 +165,8 @@ New Features
     64 bits of the value passed to ``rte_srand()`` now affect the state,
     so a given seed produces a different sequence than in previous
     releases.
+  * The test application now accepts a ``DPDK_TEST_SEED`` environment
+    variable to make runs using ``rte_rand()`` repeatable.
 
 
 Removed Items
-- 
2.53.0