Re: [PATCH] eal: add support for TRNG with Arm RNG feature

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 24 Jul 2024 17:07:49 +0200
Mattias Rönnblom <hofors@lysator.liu.se> wrote:

> On 2024-07-24 16:35, Stephen Hemminger wrote:
> > On Wed, 24 Jul 2024 08:40:39 +0200
> > Mattias Rönnblom <hofors@lysator.liu.se> wrote:
> >   
> >> On 2024-07-23 23:27, Shunzhi Wen wrote:> True Random Number Generator
> >> (TRNG) is capable of  
> >>   > generating random numbers from a physical entropy source.
> >>   > TRNG is enabled when compiled on Arm arch that supports
> >>   > FEAT_RNG introduced in Armv8.5-A flagged by __ARM_FEATURE_RNG.
> >>   >  
> >>
> >> I'm missing a rationale here. Why is this useful?
> >>  
> >>   > Signed-off-by: Shunzhi Wen <shunzhi.wen@arm.com>
> >>   > Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
> >>   > Reviewed-by: Jack Bond-Preston <jack.bond-preston@arm.com>
> >>   > Reviewed-by: Dhruv Tripathi <dhruv.tripathi@arm.com>
> >>   > ---
> >>   >   .mailmap                                      |  2 ++
> >>   >   app/test/test_rand_perf.c                     | 25 +++++++++++++++++--
> >>   >   config/arm/meson.build                        |  2 +-
> >>   >   lib/eal/arm/include/rte_cpuflags_64.h         |  3 +++
> >>   >   lib/eal/arm/meson.build                       |  1 +
> >>   >   lib/eal/arm/rte_cpuflags.c                    |  1 +
> >>   >   lib/eal/arm/rte_random.c                      | 20 +++++++++++++++
> >>   >   .../{rte_random.c => eal_common_random.c}     |  0
> >>   >   lib/eal/common/meson.build                    |  2 +-
> >>   >   lib/eal/include/rte_random.h                  | 17 +++++++++++++
> >>   >   lib/eal/loongarch/meson.build                 |  1 +
> >>   >   lib/eal/loongarch/rte_random.c                | 14 +++++++++++
> >>   >   lib/eal/ppc/meson.build                       |  1 +
> >>   >   lib/eal/ppc/rte_random.c                      | 14 +++++++++++
> >>   >   lib/eal/riscv/meson.build                     |  1 +
> >>   >   lib/eal/riscv/rte_random.c                    | 14 +++++++++++
> >>   >   lib/eal/version.map                           |  1 +
> >>   >   lib/eal/x86/meson.build                       |  1 +
> >>   >   lib/eal/x86/rte_random.c                      | 14 +++++++++++
> >>   >   19 files changed, 130 insertions(+), 4 deletions(-)
> >>   >   create mode 100644 lib/eal/arm/rte_random.c
> >>   >   rename lib/eal/common/{rte_random.c => eal_common_random.c} (100%)
> >>   >   create mode 100644 lib/eal/loongarch/rte_random.c
> >>   >   create mode 100644 lib/eal/ppc/rte_random.c
> >>   >   create mode 100644 lib/eal/riscv/rte_random.c
> >>   >   create mode 100644 lib/eal/x86/rte_random.c
> >>   >
> >>   > diff --git a/.mailmap b/.mailmap
> >>   > index ac06962e82..23209edfd2 100644
> >>   > --- a/.mailmap
> >>   > +++ b/.mailmap
> >>   > @@ -338,6 +338,7 @@ Dexia Li <dexia.li@jaguarmicro.com>
> >>   >   Dexuan Cui <decui@microsoft.com>
> >>   >   Dharmik Thakkar <dharmikjayesh.thakkar@arm.com>  
> >> <dharmik.thakkar@arm.com>  
> >>   >   Dheemanth Mallikarjun <dheemanthm@vmware.com>
> >>   > +Dhruv Tripathi <dhruv.tripathi@arm.com>
> >>   >   Diana Wang <na.wang@corigine.com>
> >>   >   Didier Pallard <didier.pallard@6wind.com>
> >>   >   Dilshod Urazov <dilshod.urazov@oktetlabs.ru>
> >>   > @@ -1353,6 +1354,7 @@ Shuki Katzenelson <shuki@lightbitslabs.com>
> >>   >   Shun Hao <shunh@nvidia.com>
> >>   >   Shu Shen <shu.shen@radisys.com>
> >>   >   Shujing Dong <shujing.dong@corigine.com>
> >>   > +Shunzhi Wen <shunzhi.wen@arm.com>
> >>   >   Shweta Choudaha <shweta.choudaha@att.com>
> >>   >   Shyam Kumar Shrivastav <shrivastav.shyam@gmail.com>
> >>   >   Shy Shyman <shys@nvidia.com> <shys@mellanox.com>
> >>   > diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
> >>   > index 30204e12c0..b61cc75014 100644
> >>   > --- a/app/test/test_rand_perf.c
> >>   > +++ b/app/test/test_rand_perf.c
> >>   > @@ -20,6 +20,7 @@ static volatile uint64_t vsum;
> >>   >
> >>   >   enum rand_type {
> >>   >   	rand_type_64,
> >>   > +	rand_type_true_rand_64,
> >>   >   	rand_type_float,
> >>   >   	rand_type_bounded_best_case,
> >>   >   	rand_type_bounded_worst_case
> >>   > @@ -31,6 +32,8 @@ rand_type_desc(enum rand_type rand_type)
> >>   >   	switch (rand_type) {
> >>   >   	case rand_type_64:
> >>   >   		return "Full 64-bit [rte_rand()]";
> >>   > +	case rand_type_true_rand_64:
> >>   > +		return "Full 64-bit True Random [rte_trand()]";
> >>   >   	case rand_type_float:
> >>   >   		return "Floating point [rte_drand()]";
> >>   >   	case rand_type_bounded_best_case:
> >>   > @@ -50,6 +53,9 @@ test_rand_perf_type(enum rand_type rand_type)
> >>   >   	uint64_t end;
> >>   >   	uint64_t sum = 0;
> >>   >   	uint64_t op_latency;
> >>   > +	int ret;
> >>   > +	uint64_t val;
> >>   > +	uint32_t fail_count = 0;
> >>   >
> >>   >   	start = rte_rdtsc();
> >>   >
> >>   > @@ -58,6 +64,13 @@ test_rand_perf_type(enum rand_type rand_type)
> >>   >   		case rand_type_64:
> >>   >   			sum += rte_rand();
> >>   >   			break;
> >>   > +		case rand_type_true_rand_64:
> >>   > +			ret = rte_trand(&val);
> >>   > +			if (ret == 0)
> >>   > +				sum += val;
> >>   > +			else
> >>   > +				fail_count++;
> >>   > +			break;
> >>   >   		case rand_type_float:
> >>   >   			sum += 1000. * rte_drand();
> >>   >   			break;
> >>   > @@ -77,8 +90,15 @@ test_rand_perf_type(enum rand_type rand_type)
> >>   >
> >>   >   	op_latency = (end - start) / ITERATIONS;
> >>   >
> >>   > -	printf("%s: %"PRId64" TSC cycles/op\n", rand_type_desc(rand_type),
> >>   > -	       op_latency);
> >>   > +	if (!fail_count)
> >>   > +		printf("%s: %"PRId64" TSC cycles/op\n",
> >>   > +		       rand_type_desc(rand_type),
> >>   > +		       op_latency);
> >>   > +	else
> >>   > +		printf("%s: %"PRId64" TSC cycles/op (failed %d time(s))\n",
> >>   > +		       rand_type_desc(rand_type),
> >>   > +		       op_latency,
> >>   > +			   fail_count);
> >>   >   }
> >>   >
> >>   >   static int
> >>   > @@ -89,6 +109,7 @@ test_rand_perf(void)
> >>   >   	printf("Pseudo-random number generation latencies:\n");
> >>   >
> >>   >   	test_rand_perf_type(rand_type_64);
> >>   > +	test_rand_perf_type(rand_type_true_rand_64);
> >>   >   	test_rand_perf_type(rand_type_float);
> >>   >   	test_rand_perf_type(rand_type_bounded_best_case);
> >>   >   	test_rand_perf_type(rand_type_bounded_worst_case);
> >>   > diff --git a/config/arm/meson.build b/config/arm/meson.build
> >>   > index 012935d5d7..13be94254e 100644
> >>   > --- a/config/arm/meson.build
> >>   > +++ b/config/arm/meson.build
> >>   > @@ -95,7 +95,7 @@ part_number_config_arm = {
> >>   >       },
> >>   >       '0xd49': {
> >>   >           'march': 'armv9-a',
> >>   > -        'march_features': ['sve2'],
> >>   > +        'march_features': ['sve2', 'rng'],
> >>   >           'fallback_march': 'armv8.5-a',
> >>   >           'mcpu': 'neoverse-n2',
> >>   >           'flags': [
> >>   > diff --git a/lib/eal/arm/include/rte_cpuflags_64.h  
> >> b/lib/eal/arm/include/rte_cpuflags_64.h  
> >>   > index afe70209c3..6aa067339f 100644
> >>   > --- a/lib/eal/arm/include/rte_cpuflags_64.h
> >>   > +++ b/lib/eal/arm/include/rte_cpuflags_64.h
> >>   > @@ -36,6 +36,9 @@ enum rte_cpu_flag_t {
> >>   >   	RTE_CPUFLAG_SVEF64MM,
> >>   >   	RTE_CPUFLAG_SVEBF16,
> >>   >   	RTE_CPUFLAG_AARCH64,
> >>   > +
> >>   > +	/* RNDR, RNDRRS instructions */
> >>   > +	RTE_CPUFLAG_RNG,
> >>   >   };
> >>   >
> >>   >   #include "generic/rte_cpuflags.h"
> >>   > diff --git a/lib/eal/arm/meson.build b/lib/eal/arm/meson.build
> >>   > index 6fba3d6ba7..e9e468cbf0 100644
> >>   > --- a/lib/eal/arm/meson.build
> >>   > +++ b/lib/eal/arm/meson.build
> >>   > @@ -9,4 +9,5 @@ sources += files(
> >>   >           'rte_hypervisor.c',
> >>   >           'rte_mmu.c',
> >>   >           'rte_power_intrinsics.c',
> >>   > +        'rte_random.c',
> >>   >   )
> >>   > diff --git a/lib/eal/arm/rte_cpuflags.c b/lib/eal/arm/rte_cpuflags.c
> >>   > index 7ba4f8ba97..56074f0c6a 100644
> >>   > --- a/lib/eal/arm/rte_cpuflags.c
> >>   > +++ b/lib/eal/arm/rte_cpuflags.c
> >>   > @@ -116,6 +116,7 @@ const struct feature_entry  
> >> rte_cpu_feature_table[] = {  
> >>   >   	FEAT_DEF(SVEF64MM,	REG_HWCAP2,   11)
> >>   >   	FEAT_DEF(SVEBF16,	REG_HWCAP2,   12)
> >>   >   	FEAT_DEF(AARCH64,	REG_PLATFORM,  0)
> >>   > +	FEAT_DEF(RNG,		REG_HWCAP2,   16)
> >>   >   };
> >>   >   #endif /* RTE_ARCH */
> >>   >
> >>   > diff --git a/lib/eal/arm/rte_random.c b/lib/eal/arm/rte_random.c
> >>   > new file mode 100644
> >>   > index 0000000000..74c8fa733b
> >>   > --- /dev/null
> >>   > +++ b/lib/eal/arm/rte_random.c
> >>   > @@ -0,0 +1,20 @@
> >>   > +/* SPDX-License-Identifier: BSD-3-Clause
> >>   > + * Copyright(c) 2024 Arm Limited
> >>   > + */
> >>   > +
> >>   > +#include "arm_acle.h"
> >>   > +#include "rte_common.h"
> >>   > +#include "rte_random.h"
> >>   > +#include <errno.h>
> >>   > +
> >>   > +int
> >>   > +rte_trand(uint64_t *val)
> >>   > +{
> >>   > +#if defined __ARM_FEATURE_RNG
> >>   > +	int ret = __rndr(val);
> >>   > +	return (ret == 0) ? 0 : -ENODATA;
> >>   > +#else
> >>   > +	RTE_SET_USED(val);
> >>   > +	return -ENOTSUP;
> >>   > +#endif
> >>   > +}
> >>   > diff --git a/lib/eal/common/rte_random.c  
> >> b/lib/eal/common/eal_common_random.c  
> >>   > similarity index 100%
> >>   > rename from lib/eal/common/rte_random.c
> >>   > rename to lib/eal/common/eal_common_random.c
> >>   > diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
> >>   > index 22a626ba6f..c4405aa48b 100644
> >>   > --- a/lib/eal/common/meson.build
> >>   > +++ b/lib/eal/common/meson.build
> >>   > @@ -32,7 +32,6 @@ sources += files(
> >>   >           'malloc_elem.c',
> >>   >           'malloc_heap.c',
> >>   >           'rte_malloc.c',
> >>   > -        'rte_random.c',
> >>   >           'rte_reciprocal.c',
> >>   >           'rte_service.c',
> >>   >           'rte_version.c',
> >>   > @@ -48,6 +47,7 @@ if not is_windows
> >>   >               'eal_common_trace.c',
> >>   >               'eal_common_trace_ctf.c',
> >>   >               'eal_common_trace_utils.c',
> >>   > +            'eal_common_random.c',
> >>   >               'hotplug_mp.c',
> >>   >               'malloc_mp.c',
> >>   >               'rte_keepalive.c',
> >>   > diff --git a/lib/eal/include/rte_random.h b/lib/eal/include/rte_random.h
> >>   > index 5031c6fe5f..e6b5ac46ed 100644
> >>   > --- a/lib/eal/include/rte_random.h
> >>   > +++ b/lib/eal/include/rte_random.h
> >>   > @@ -15,6 +15,7 @@
> >>   >   extern "C" {
> >>   >   #endif
> >>   >
> >>   > +#include <rte_compat.h>
> >>   >   #include <stdint.h>
> >>   >
> >>   >   /**
> >>   > @@ -84,6 +85,22 @@ rte_rand_max(uint64_t upper_bound);
> >>   >    */
> >>   >   double rte_drand(void);
> >>   >
> >>   > +/**
> >>   > + * Get a true random value.
> >>   > + *
> >>   > + * The generator is cryptographically secure.  
> >>
> >> If you want to extend <rte_random.h> with a cryptographically secure
> >> random number generator, that's fine.
> >>
> >> To have an API that's only available on certain ARM CPUs is not.
> >>
> >> NAK
> >>
> >> A new function should be called something with "secure", rather than
> >> "true" (which is a bit silly, since we might well live in a completely
> >> deterministic universe). "secure" would more clearly communicate the
> >> intent, and also doesn't imply any particular implementation.  
> > 
> > Agree, with Mattias. What constitutes a secure random number generator
> > is a matter of much debate. Most of the HW random generators are taking
> > diode (Schottky noise) and doing transforms on it to get something uniform.
> > 
> > If a key generation type API was added, why not just use existing and more
> > researched kernel get_random()?
> >   
> 
> Ideally, you want to avoid system calls on lcore workers doing packet 
> processing. If you have to do system calls (which I believe is the case 
> here), it's better to a simple call, not so often.
> 
> getentropy() seems to need about 800 core clock cycles on my x86_64, on 
> average. (rte_rand() needs ~11 cc/call.) 800 cc is not too horrible, but 
> system calls tend to have some pretty bad tail latencies.
> 
> To improve efficiency, one could do a getentropy() on a relatively large 
> buffer, and cache the result on a per-lcore basis, amortizing the system 
> call overhead over many calls.
> 
> You still have the tail latency issue to deal with. We could have a 
> control thread providing entropy for the lcores, but that seems like 
> massive overkill.


Getrandom is a vsyscall on current kernels, and it manages use of entropy across
multiple sources. If you are doing lots of key generation, you don't want to
hit the hardware every time.

https://lwn.net/Articles/974468/