[dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[Dan Gora]

Instead of choosing to use getentropy() or the rdseed instruction for
the random number generator entropy source using compilation flags,
determine the best source at run time.

This is accomplished by defining a weak symbol for getentropy(),
checking that the compiler can generate the rdseed instruction even if
the compilation platform does not natively support it, and checking for
the availability of the rdseed instruction on the execution platform
using rte_cpu_get_flag_enabled().

If neither getentropy() or rdseed is available, rte_get_timer_cycles()
will be continue to be used as the entropy source.

This also allows non-Mason builds to use getentropy().

Signed-off-by: Dan Gora <dg@adax.com>
---
 config/x86/meson.build             |  7 +++++++
 lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
 lib/librte_eal/meson.build         |  3 ---
 mk/rte.cpuflags.mk                 |  8 ++++++++
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/config/x86/meson.build b/config/x86/meson.build
index adc857ba2..214b16f2a 100644
--- a/config/x86/meson.build
+++ b/config/x86/meson.build
@@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
 	machine_args += '-msse4'
 endif
 
+# set -mrdseed if necessary so _rdseed32_step compiles if the
+# compilation host does not support the RDSEED instruction.
+if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
+	machine_args += '-mrdseed'
+	message('RDSEED not enabled by default, explicitly setting -mrdseed')
+endif
+
 base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
 foreach f:base_flags
 	dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
index 57ec8fb2b..40f8b5aab 100644
--- a/lib/librte_eal/common/rte_random.c
+++ b/lib/librte_eal/common/rte_random.c
@@ -25,6 +25,8 @@ struct rte_rand_state {
 
 static struct rte_rand_state rand_states[RTE_MAX_LCORE];
 
+__rte_weak int getentropy(void *__buffer, size_t __length);
+
 static uint32_t
 __rte_rand_lcg32(uint32_t *seed)
 {
@@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
 	return res;
 }
 
+/* Use rte_get_timer_cycles() if the system does not have
+ * genentropy() or the rdseed instruction.
+ */
+__rte_weak int
+getentropy(void *__buffer, size_t __length __rte_unused)
+{
+	uint64_t *ge_seed = __buffer;
+#ifdef RTE_MACHINE_CPUFLAG_RDSEED
+	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
+		return -1;
+#endif
+	*ge_seed = rte_get_timer_cycles();
+	return 0;
+}
+
 static uint64_t
 __rte_random_initial_seed(void)
 {
-#ifdef RTE_LIBEAL_USE_GETENTROPY
 	int ge_rc;
 	uint64_t ge_seed;
 
@@ -187,15 +203,18 @@ __rte_random_initial_seed(void)
 
 	if (ge_rc == 0)
 		return ge_seed;
-#endif
+
 #ifdef RTE_MACHINE_CPUFLAG_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);
+	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED)) {
+		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 */
 	return rte_get_timer_cycles();
diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
index 4be5118ce..8382efbb8 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -17,9 +17,6 @@ deps += 'kvargs'
 if dpdk_conf.has('RTE_USE_LIBBSD')
 	ext_deps += libbsd
 endif
-if cc.has_function('getentropy', prefix : '#include <unistd.h>')
-	cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
-endif
 sources = common_sources + env_sources
 objs = common_objs + env_objs
 headers = common_headers + env_headers
diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
index fa8753531..fb7bf8a53 100644
--- a/mk/rte.cpuflags.mk
+++ b/mk/rte.cpuflags.mk
@@ -53,6 +53,14 @@ endif
 
 ifneq ($(filter $(AUTO_CPUFLAGS),__RDSEED__),)
 CPUFLAGS += RDSEED
+else
+# If the native environment doesn't define __RDSEED__, see
+# if the compiler supports -mrdseed.
+RDSEED_CPUFLAGS := $(shell $(CC) $(MACHINE_CFLAGS) $(WERROR_FLAGS) $(EXTRA_CFLAGS) -mrdseed -dM -E - < /dev/null)
+ifneq ($(filter $(RDSEED_CPUFLAGS),__RDSEED__),)
+CPUFLAGS += RDSEED
+MACHINE_CFLAGS += -mrdseed
+endif
 endif
 
 ifneq ($(filter $(AUTO_CPUFLAGS),__FSGSBASE__),)
-- 
2.24.1.425.g7034cd094b

[dpdk-dev] [PATCH v2] eal: choose initial PRNG seed source at runtime

[Dan Gora]

Instead of choosing to use getentropy() or the rdseed instruction for
the random number generator entropy source using compilation flags,
determine the best source at run time.

This is accomplished by defining a weak symbol for getentropy(),
checking that the compiler can generate the rdseed instruction even if
the compilation platform does not natively support it, and checking for
the availability of the rdseed instruction on the execution platform
using rte_cpu_get_flag_enabled().

If neither getentropy() or rdseed is available, rte_get_timer_cycles()
will be continue to be used as the entropy source.

This also allows non-meson builds to use getentropy().

Signed-off-by: Dan Gora <dg@adax.com>
---
v2:
* Rebase to latest master.
* Fix spelling of "meson".

 config/x86/meson.build             |  7 +++++++
 lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
 lib/librte_eal/meson.build         |  3 ---
 mk/rte.cpuflags.mk                 |  8 ++++++++
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/config/x86/meson.build b/config/x86/meson.build
index adc857ba2..214b16f2a 100644
--- a/config/x86/meson.build
+++ b/config/x86/meson.build
@@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
 	machine_args += '-msse4'
 endif
 
+# set -mrdseed if necessary so _rdseed32_step compiles if the
+# compilation host does not support the RDSEED instruction.
+if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
+	machine_args += '-mrdseed'
+	message('RDSEED not enabled by default, explicitly setting -mrdseed')
+endif
+
 base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
 foreach f:base_flags
 	dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
index 57ec8fb2b..40f8b5aab 100644
--- a/lib/librte_eal/common/rte_random.c
+++ b/lib/librte_eal/common/rte_random.c
@@ -25,6 +25,8 @@ struct rte_rand_state {
 
 static struct rte_rand_state rand_states[RTE_MAX_LCORE];
 
+__rte_weak int getentropy(void *__buffer, size_t __length);
+
 static uint32_t
 __rte_rand_lcg32(uint32_t *seed)
 {
@@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
 	return res;
 }
 
+/* Use rte_get_timer_cycles() if the system does not have
+ * genentropy() or the rdseed instruction.
+ */
+__rte_weak int
+getentropy(void *__buffer, size_t __length __rte_unused)
+{
+	uint64_t *ge_seed = __buffer;
+#ifdef RTE_MACHINE_CPUFLAG_RDSEED
+	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
+		return -1;
+#endif
+	*ge_seed = rte_get_timer_cycles();
+	return 0;
+}
+
 static uint64_t
 __rte_random_initial_seed(void)
 {
-#ifdef RTE_LIBEAL_USE_GETENTROPY
 	int ge_rc;
 	uint64_t ge_seed;
 
@@ -187,15 +203,18 @@ __rte_random_initial_seed(void)
 
 	if (ge_rc == 0)
 		return ge_seed;
-#endif
+
 #ifdef RTE_MACHINE_CPUFLAG_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);
+	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED)) {
+		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 */
 	return rte_get_timer_cycles();
diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
index 0267c3b9d..748359b8c 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -15,9 +15,6 @@ deps += 'kvargs'
 if dpdk_conf.has('RTE_USE_LIBBSD')
 	ext_deps += libbsd
 endif
-if cc.has_function('getentropy', prefix : '#include <unistd.h>')
-	cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
-endif
 if cc.has_header('getopt.h')
 	cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG']
 endif
diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
index fa8753531..fb7bf8a53 100644
--- a/mk/rte.cpuflags.mk
+++ b/mk/rte.cpuflags.mk
@@ -53,6 +53,14 @@ endif
 
 ifneq ($(filter $(AUTO_CPUFLAGS),__RDSEED__),)
 CPUFLAGS += RDSEED
+else
+# If the native environment doesn't define __RDSEED__, see
+# if the compiler supports -mrdseed.
+RDSEED_CPUFLAGS := $(shell $(CC) $(MACHINE_CFLAGS) $(WERROR_FLAGS) $(EXTRA_CFLAGS) -mrdseed -dM -E - < /dev/null)
+ifneq ($(filter $(RDSEED_CPUFLAGS),__RDSEED__),)
+CPUFLAGS += RDSEED
+MACHINE_CFLAGS += -mrdseed
+endif
 endif
 
 ifneq ($(filter $(AUTO_CPUFLAGS),__FSGSBASE__),)
-- 
2.24.1.425.g7034cd094b

Re: [dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[Jerin Jacob]

On Thu, Apr 16, 2020 at 4:42 AM Dan Gora <dg@adax.com> wrote:
>
> Instead of choosing to use getentropy() or the rdseed instruction for
> the random number generator entropy source using compilation flags,
> determine the best source at run time.
>
> This is accomplished by defining a weak symbol for getentropy(),
> checking that the compiler can generate the rdseed instruction even if
> the compilation platform does not natively support it, and checking for
> the availability of the rdseed instruction on the execution platform
> using rte_cpu_get_flag_enabled().
>
> If neither getentropy() or rdseed is available, rte_get_timer_cycles()
> will be continue to be used as the entropy source.
>
> This also allows non-Mason builds to use getentropy().
>
> Signed-off-by: Dan Gora <dg@adax.com>
> ---
>  config/x86/meson.build             |  7 +++++++
>  lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
>  lib/librte_eal/meson.build         |  3 ---
>  mk/rte.cpuflags.mk                 |  8 ++++++++
>  4 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/config/x86/meson.build b/config/x86/meson.build
> index adc857ba2..214b16f2a 100644
> --- a/config/x86/meson.build
> +++ b/config/x86/meson.build
> @@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
>         machine_args += '-msse4'
>  endif
>
> +# set -mrdseed if necessary so _rdseed32_step compiles if the
> +# compilation host does not support the RDSEED instruction.
> +if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
> +       machine_args += '-mrdseed'
> +       message('RDSEED not enabled by default, explicitly setting -mrdseed')
> +endif
> +
>  base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
>  foreach f:base_flags
>         dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
> diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
> index 57ec8fb2b..40f8b5aab 100644
> --- a/lib/librte_eal/common/rte_random.c
> +++ b/lib/librte_eal/common/rte_random.c
> @@ -25,6 +25,8 @@ struct rte_rand_state {
>
>  static struct rte_rand_state rand_states[RTE_MAX_LCORE];
>
> +__rte_weak int getentropy(void *__buffer, size_t __length);
> +
>  static uint32_t
>  __rte_rand_lcg32(uint32_t *seed)
>  {
> @@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
>         return res;
>  }
>
> +/* Use rte_get_timer_cycles() if the system does not have
> + * genentropy() or the rdseed instruction.
> + */
> +__rte_weak int
> +getentropy(void *__buffer, size_t __length __rte_unused)
> +{
> +       uint64_t *ge_seed = __buffer;
> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
> +               return -1;
> +#endif
> +       *ge_seed = rte_get_timer_cycles();
> +       return 0;

IMO, we need to create a new arch EAL abstraction to get uint64_t
random number. Reason being:
1) ARMv8.5 supports similar instruction
https://developer.arm.com/docs/ddi0595/c/aarch64-system-registers/rndr
2) Avoid #ifdef clutter in common code.

Abstraction can return a random uint64_t number. Based on the arch
capabilities, it can hook
to specialized instruction or rte_get_timer_cycles()

Re: [dpdk-dev] [PATCH v2] eal: choose initial PRNG seed source at runtime

[Mattias Rönnblom]

On 2020-04-16 01:59, Dan Gora wrote:
> Instead of choosing to use getentropy() or the rdseed instruction for
> the random number generator entropy source using compilation flags,
> determine the best source at run time.


I like this idea.


> This is accomplished by defining a weak symbol for getentropy(),
> checking that the compiler can generate the rdseed instruction even if
> the compilation platform does not natively support it, and checking for
> the availability of the rdseed instruction on the execution platform
> using rte_cpu_get_flag_enabled().
>
> If neither getentropy() or rdseed is available, rte_get_timer_cycles()
> will be continue to be used as the entropy source.
>
> This also allows non-meson builds to use getentropy().
>
> Signed-off-by: Dan Gora <dg@adax.com>
> ---
> v2:
> * Rebase to latest master.
> * Fix spelling of "meson".
>
>   config/x86/meson.build             |  7 +++++++
>   lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
>   lib/librte_eal/meson.build         |  3 ---
>   mk/rte.cpuflags.mk                 |  8 ++++++++
>   4 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/config/x86/meson.build b/config/x86/meson.build
> index adc857ba2..214b16f2a 100644
> --- a/config/x86/meson.build
> +++ b/config/x86/meson.build
> @@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
>   	machine_args += '-msse4'
>   endif
>   
> +# set -mrdseed if necessary so _rdseed32_step compiles if the
> +# compilation host does not support the RDSEED instruction.
> +if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
> +	machine_args += '-mrdseed'
> +	message('RDSEED not enabled by default, explicitly setting -mrdseed')
> +endif
> +
>   base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
>   foreach f:base_flags
>   	dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
> diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
> index 57ec8fb2b..40f8b5aab 100644
> --- a/lib/librte_eal/common/rte_random.c
> +++ b/lib/librte_eal/common/rte_random.c
> @@ -25,6 +25,8 @@ struct rte_rand_state {
>   
>   static struct rte_rand_state rand_states[RTE_MAX_LCORE];
>   
> +__rte_weak int getentropy(void *__buffer, size_t __length);
> +
>   static uint32_t
>   __rte_rand_lcg32(uint32_t *seed)
>   {
> @@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
>   	return res;
>   }
>   
> +/* Use rte_get_timer_cycles() if the system does not have
> + * genentropy() or the rdseed instruction.
> + */
> +__rte_weak int
> +getentropy(void *__buffer, size_t __length __rte_unused)
> +{


Just make the weak getentropy() to return -1, always.


> +	uint64_t *ge_seed = __buffer;
> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
> +	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
> +		return -1;
> +#endif
> +	*ge_seed = rte_get_timer_cycles();
> +	return 0;
> +}
> +
>   static uint64_t
>   __rte_random_initial_seed(void)
>   {
> -#ifdef RTE_LIBEAL_USE_GETENTROPY
>   	int ge_rc;
>   	uint64_t ge_seed;
>   
> @@ -187,15 +203,18 @@ __rte_random_initial_seed(void)
>   
>   	if (ge_rc == 0)
>   		return ge_seed;
> -#endif
> +
>   #ifdef RTE_MACHINE_CPUFLAG_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);
> +	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED)) {


Move the variable declarations into this scope.


> +		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 */
>   	return rte_get_timer_cycles();
> diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
> index 0267c3b9d..748359b8c 100644
> --- a/lib/librte_eal/meson.build
> +++ b/lib/librte_eal/meson.build
> @@ -15,9 +15,6 @@ deps += 'kvargs'
>   if dpdk_conf.has('RTE_USE_LIBBSD')
>   	ext_deps += libbsd
>   endif
> -if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> -	cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> -endif
>   if cc.has_header('getopt.h')
>   	cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG']
>   endif
> diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
> index fa8753531..fb7bf8a53 100644
> --- a/mk/rte.cpuflags.mk
> +++ b/mk/rte.cpuflags.mk
> @@ -53,6 +53,14 @@ endif
>   
>   ifneq ($(filter $(AUTO_CPUFLAGS),__RDSEED__),)
>   CPUFLAGS += RDSEED
> +else
> +# If the native environment doesn't define __RDSEED__, see
> +# if the compiler supports -mrdseed.


For which environments is this true?


> +RDSEED_CPUFLAGS := $(shell $(CC) $(MACHINE_CFLAGS) $(WERROR_FLAGS) $(EXTRA_CFLAGS) -mrdseed -dM -E - < /dev/null)
> +ifneq ($(filter $(RDSEED_CPUFLAGS),__RDSEED__),)


There are no better ways to achieve this? It seems like a bit of a hack.


> +CPUFLAGS += RDSEED
> +MACHINE_CFLAGS += -mrdseed
> +endif
>   endif
>   
>   ifneq ($(filter $(AUTO_CPUFLAGS),__FSGSBASE__),)

Re: [dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[Mattias Rönnblom]

On 2020-04-16 08:07, Jerin Jacob wrote:
> On Thu, Apr 16, 2020 at 4:42 AM Dan Gora <dg@adax.com> wrote:
>> Instead of choosing to use getentropy() or the rdseed instruction for
>> the random number generator entropy source using compilation flags,
>> determine the best source at run time.
>>
>> This is accomplished by defining a weak symbol for getentropy(),
>> checking that the compiler can generate the rdseed instruction even if
>> the compilation platform does not natively support it, and checking for
>> the availability of the rdseed instruction on the execution platform
>> using rte_cpu_get_flag_enabled().
>>
>> If neither getentropy() or rdseed is available, rte_get_timer_cycles()
>> will be continue to be used as the entropy source.
>>
>> This also allows non-Mason builds to use getentropy().
>>
>> Signed-off-by: Dan Gora <dg@adax.com>
>> ---
>>   config/x86/meson.build             |  7 +++++++
>>   lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
>>   lib/librte_eal/meson.build         |  3 ---
>>   mk/rte.cpuflags.mk                 |  8 ++++++++
>>   4 files changed, 39 insertions(+), 8 deletions(-)
>>
>> diff --git a/config/x86/meson.build b/config/x86/meson.build
>> index adc857ba2..214b16f2a 100644
>> --- a/config/x86/meson.build
>> +++ b/config/x86/meson.build
>> @@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
>>          machine_args += '-msse4'
>>   endif
>>
>> +# set -mrdseed if necessary so _rdseed32_step compiles if the
>> +# compilation host does not support the RDSEED instruction.
>> +if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
>> +       machine_args += '-mrdseed'
>> +       message('RDSEED not enabled by default, explicitly setting -mrdseed')
>> +endif
>> +
>>   base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
>>   foreach f:base_flags
>>          dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
>> diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
>> index 57ec8fb2b..40f8b5aab 100644
>> --- a/lib/librte_eal/common/rte_random.c
>> +++ b/lib/librte_eal/common/rte_random.c
>> @@ -25,6 +25,8 @@ struct rte_rand_state {
>>
>>   static struct rte_rand_state rand_states[RTE_MAX_LCORE];
>>
>> +__rte_weak int getentropy(void *__buffer, size_t __length);
>> +
>>   static uint32_t
>>   __rte_rand_lcg32(uint32_t *seed)
>>   {
>> @@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
>>          return res;
>>   }
>>
>> +/* Use rte_get_timer_cycles() if the system does not have
>> + * genentropy() or the rdseed instruction.
>> + */
>> +__rte_weak int
>> +getentropy(void *__buffer, size_t __length __rte_unused)
>> +{
>> +       uint64_t *ge_seed = __buffer;
>> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
>> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
>> +               return -1;
>> +#endif
>> +       *ge_seed = rte_get_timer_cycles();
>> +       return 0;
> IMO, we need to create a new arch EAL abstraction to get uint64_t
> random number. Reason being:
> 1) ARMv8.5 supports similar instruction
> https://developer.arm.com/docs/ddi0595/c/aarch64-system-registers/rndr


How many ARMv8.5 system will there be running a libc older than 2.25 or 
a Linux kernel older than 3.17?


> 2) Avoid #ifdef clutter in common code.
>
> Abstraction can return a random uint64_t number. Based on the arch
> capabilities, it can hook
> to specialized instruction or rte_get_timer_cycles()

Re: [dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[Jerin Jacob]

On Thu, Apr 16, 2020 at 5:06 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-16 08:07, Jerin Jacob wrote:
> > On Thu, Apr 16, 2020 at 4:42 AM Dan Gora <dg@adax.com> wrote:
> >> Instead of choosing to use getentropy() or the rdseed instruction for
> >> the random number generator entropy source using compilation flags,
> >> determine the best source at run time.
> >>
> >> This is accomplished by defining a weak symbol for getentropy(),
> >> checking that the compiler can generate the rdseed instruction even if
> >> the compilation platform does not natively support it, and checking for
> >> the availability of the rdseed instruction on the execution platform
> >> using rte_cpu_get_flag_enabled().
> >>
> >> If neither getentropy() or rdseed is available, rte_get_timer_cycles()
> >> will be continue to be used as the entropy source.
> >>
> >> This also allows non-Mason builds to use getentropy().
> >>
> >> Signed-off-by: Dan Gora <dg@adax.com>
> >> ---
> >>   config/x86/meson.build             |  7 +++++++
> >>   lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
> >>   lib/librte_eal/meson.build         |  3 ---
> >>   mk/rte.cpuflags.mk                 |  8 ++++++++
> >>   4 files changed, 39 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/config/x86/meson.build b/config/x86/meson.build
> >> index adc857ba2..214b16f2a 100644
> >> --- a/config/x86/meson.build
> >> +++ b/config/x86/meson.build
> >> @@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
> >>          machine_args += '-msse4'
> >>   endif
> >>
> >> +# set -mrdseed if necessary so _rdseed32_step compiles if the
> >> +# compilation host does not support the RDSEED instruction.
> >> +if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
> >> +       machine_args += '-mrdseed'
> >> +       message('RDSEED not enabled by default, explicitly setting -mrdseed')
> >> +endif
> >> +
> >>   base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
> >>   foreach f:base_flags
> >>          dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
> >> diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
> >> index 57ec8fb2b..40f8b5aab 100644
> >> --- a/lib/librte_eal/common/rte_random.c
> >> +++ b/lib/librte_eal/common/rte_random.c
> >> @@ -25,6 +25,8 @@ struct rte_rand_state {
> >>
> >>   static struct rte_rand_state rand_states[RTE_MAX_LCORE];
> >>
> >> +__rte_weak int getentropy(void *__buffer, size_t __length);
> >> +
> >>   static uint32_t
> >>   __rte_rand_lcg32(uint32_t *seed)
> >>   {
> >> @@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
> >>          return res;
> >>   }
> >>
> >> +/* Use rte_get_timer_cycles() if the system does not have
> >> + * genentropy() or the rdseed instruction.
> >> + */
> >> +__rte_weak int
> >> +getentropy(void *__buffer, size_t __length __rte_unused)
> >> +{
> >> +       uint64_t *ge_seed = __buffer;
> >> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
> >> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
> >> +               return -1;
> >> +#endif
> >> +       *ge_seed = rte_get_timer_cycles();
> >> +       return 0;
> > IMO, we need to create a new arch EAL abstraction to get uint64_t
> > random number. Reason being:
> > 1) ARMv8.5 supports similar instruction
> > https://developer.arm.com/docs/ddi0595/c/aarch64-system-registers/rndr
>
>
> How many ARMv8.5 system will there be running a libc older than 2.25 or
> a Linux kernel older than 3.17?

Probably none.

>
>
> > 2) Avoid #ifdef clutter in common code.
> >
> > Abstraction can return a random uint64_t number. Based on the arch
> > capabilities, it can hook
> > to specialized instruction or rte_get_timer_cycles()

Re: [dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[Mattias Rönnblom]

On 2020-04-16 13:48, Jerin Jacob wrote:
> On Thu, Apr 16, 2020 at 5:06 PM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>> On 2020-04-16 08:07, Jerin Jacob wrote:
>>> On Thu, Apr 16, 2020 at 4:42 AM Dan Gora <dg@adax.com> wrote:
>>>> Instead of choosing to use getentropy() or the rdseed instruction for
>>>> the random number generator entropy source using compilation flags,
>>>> determine the best source at run time.
>>>>
>>>> This is accomplished by defining a weak symbol for getentropy(),
>>>> checking that the compiler can generate the rdseed instruction even if
>>>> the compilation platform does not natively support it, and checking for
>>>> the availability of the rdseed instruction on the execution platform
>>>> using rte_cpu_get_flag_enabled().
>>>>
>>>> If neither getentropy() or rdseed is available, rte_get_timer_cycles()
>>>> will be continue to be used as the entropy source.
>>>>
>>>> This also allows non-Mason builds to use getentropy().
>>>>
>>>> Signed-off-by: Dan Gora <dg@adax.com>
>>>> ---
>>>>    config/x86/meson.build             |  7 +++++++
>>>>    lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
>>>>    lib/librte_eal/meson.build         |  3 ---
>>>>    mk/rte.cpuflags.mk                 |  8 ++++++++
>>>>    4 files changed, 39 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/config/x86/meson.build b/config/x86/meson.build
>>>> index adc857ba2..214b16f2a 100644
>>>> --- a/config/x86/meson.build
>>>> +++ b/config/x86/meson.build
>>>> @@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
>>>>           machine_args += '-msse4'
>>>>    endif
>>>>
>>>> +# set -mrdseed if necessary so _rdseed32_step compiles if the
>>>> +# compilation host does not support the RDSEED instruction.
>>>> +if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
>>>> +       machine_args += '-mrdseed'
>>>> +       message('RDSEED not enabled by default, explicitly setting -mrdseed')
>>>> +endif
>>>> +
>>>>    base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
>>>>    foreach f:base_flags
>>>>           dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
>>>> diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
>>>> index 57ec8fb2b..40f8b5aab 100644
>>>> --- a/lib/librte_eal/common/rte_random.c
>>>> +++ b/lib/librte_eal/common/rte_random.c
>>>> @@ -25,6 +25,8 @@ struct rte_rand_state {
>>>>
>>>>    static struct rte_rand_state rand_states[RTE_MAX_LCORE];
>>>>
>>>> +__rte_weak int getentropy(void *__buffer, size_t __length);
>>>> +
>>>>    static uint32_t
>>>>    __rte_rand_lcg32(uint32_t *seed)
>>>>    {
>>>> @@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
>>>>           return res;
>>>>    }
>>>>
>>>> +/* Use rte_get_timer_cycles() if the system does not have
>>>> + * genentropy() or the rdseed instruction.
>>>> + */
>>>> +__rte_weak int
>>>> +getentropy(void *__buffer, size_t __length __rte_unused)
>>>> +{
>>>> +       uint64_t *ge_seed = __buffer;
>>>> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
>>>> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
>>>> +               return -1;
>>>> +#endif
>>>> +       *ge_seed = rte_get_timer_cycles();
>>>> +       return 0;
>>> IMO, we need to create a new arch EAL abstraction to get uint64_t
>>> random number. Reason being:
>>> 1) ARMv8.5 supports similar instruction
>>> https://developer.arm.com/docs/ddi0595/c/aarch64-system-registers/rndr
>>
>> How many ARMv8.5 system will there be running a libc older than 2.25 or
>> a Linux kernel older than 3.17?
> Probably none.


In that case getentropy() will always available, and having ARMv8.5 rndr 
support would be pointless, as far as rte_random.c is concerned.


>>
>>> 2) Avoid #ifdef clutter in common code.
>>>
>>> Abstraction can return a random uint64_t number. Based on the arch
>>> capabilities, it can hook
>>> to specialized instruction or rte_get_timer_cycles()

Re: [dpdk-dev] [PATCH] eal: choose initial PRNG seed source at runtime

[Jerin Jacob]

On Thu, Apr 16, 2020 at 5:30 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2020-04-16 13:48, Jerin Jacob wrote:
> > On Thu, Apr 16, 2020 at 5:06 PM Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com> wrote:
> >> On 2020-04-16 08:07, Jerin Jacob wrote:
> >>> On Thu, Apr 16, 2020 at 4:42 AM Dan Gora <dg@adax.com> wrote:
> >>>> Instead of choosing to use getentropy() or the rdseed instruction for
> >>>> the random number generator entropy source using compilation flags,
> >>>> determine the best source at run time.
> >>>>
> >>>> This is accomplished by defining a weak symbol for getentropy(),
> >>>> checking that the compiler can generate the rdseed instruction even if
> >>>> the compilation platform does not natively support it, and checking for
> >>>> the availability of the rdseed instruction on the execution platform
> >>>> using rte_cpu_get_flag_enabled().
> >>>>
> >>>> If neither getentropy() or rdseed is available, rte_get_timer_cycles()
> >>>> will be continue to be used as the entropy source.
> >>>>
> >>>> This also allows non-Mason builds to use getentropy().
> >>>>
> >>>> Signed-off-by: Dan Gora <dg@adax.com>
> >>>> ---
> >>>>    config/x86/meson.build             |  7 +++++++
> >>>>    lib/librte_eal/common/rte_random.c | 29 ++++++++++++++++++++++++-----
> >>>>    lib/librte_eal/meson.build         |  3 ---
> >>>>    mk/rte.cpuflags.mk                 |  8 ++++++++
> >>>>    4 files changed, 39 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/config/x86/meson.build b/config/x86/meson.build
> >>>> index adc857ba2..214b16f2a 100644
> >>>> --- a/config/x86/meson.build
> >>>> +++ b/config/x86/meson.build
> >>>> @@ -20,6 +20,13 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
> >>>>           machine_args += '-msse4'
> >>>>    endif
> >>>>
> >>>> +# set -mrdseed if necessary so _rdseed32_step compiles if the
> >>>> +# compilation host does not support the RDSEED instruction.
> >>>> +if cc.get_define('__RDSEED__', args: machine_args) == '' and cc.has_argument('-mrdseed')
> >>>> +       machine_args += '-mrdseed'
> >>>> +       message('RDSEED not enabled by default, explicitly setting -mrdseed')
> >>>> +endif
> >>>> +
> >>>>    base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
> >>>>    foreach f:base_flags
> >>>>           dpdk_conf.set('RTE_MACHINE_CPUFLAG_' + f, 1)
> >>>> diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/rte_random.c
> >>>> index 57ec8fb2b..40f8b5aab 100644
> >>>> --- a/lib/librte_eal/common/rte_random.c
> >>>> +++ b/lib/librte_eal/common/rte_random.c
> >>>> @@ -25,6 +25,8 @@ struct rte_rand_state {
> >>>>
> >>>>    static struct rte_rand_state rand_states[RTE_MAX_LCORE];
> >>>>
> >>>> +__rte_weak int getentropy(void *__buffer, size_t __length);
> >>>> +
> >>>>    static uint32_t
> >>>>    __rte_rand_lcg32(uint32_t *seed)
> >>>>    {
> >>>> @@ -176,10 +178,24 @@ rte_rand_max(uint64_t upper_bound)
> >>>>           return res;
> >>>>    }
> >>>>
> >>>> +/* Use rte_get_timer_cycles() if the system does not have
> >>>> + * genentropy() or the rdseed instruction.
> >>>> + */
> >>>> +__rte_weak int
> >>>> +getentropy(void *__buffer, size_t __length __rte_unused)
> >>>> +{
> >>>> +       uint64_t *ge_seed = __buffer;
> >>>> +#ifdef RTE_MACHINE_CPUFLAG_RDSEED
> >>>> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_RDSEED))
> >>>> +               return -1;
> >>>> +#endif
> >>>> +       *ge_seed = rte_get_timer_cycles();
> >>>> +       return 0;
> >>> IMO, we need to create a new arch EAL abstraction to get uint64_t
> >>> random number. Reason being:
> >>> 1) ARMv8.5 supports similar instruction
> >>> https://developer.arm.com/docs/ddi0595/c/aarch64-system-registers/rndr
> >>
> >> How many ARMv8.5 system will there be running a libc older than 2.25 or
> >> a Linux kernel older than 3.17?
> > Probably none.
>
>
> In that case getentropy() will always available, and having ARMv8.5 rndr
> support would be pointless, as far as rte_random.c is concerned.

Yes. My initial comment was based on this patch context.
I just checked the lib/librte_eal/common/rte_random.c,
if this get activated only libc older than 2.25 or kernel < 3.17, it
is not a concern for me.


>
> >>
> >>> 2) Avoid #ifdef clutter in common code.
> >>>
> >>> Abstraction can return a random uint64_t number. Based on the arch
> >>> capabilities, it can hook
> >>> to specialized instruction or rte_get_timer_cycles()
>
>

Re: [dpdk-dev] [PATCH v2] eal: choose initial PRNG seed source at runtime

[Dan Gora]

On Thu, Apr 16, 2020 at 8:30 AM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
> > diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
> > index 0267c3b9d..748359b8c 100644
> > --- a/lib/librte_eal/meson.build
> > +++ b/lib/librte_eal/meson.build
> > @@ -15,9 +15,6 @@ deps += 'kvargs'
> >   if dpdk_conf.has('RTE_USE_LIBBSD')
> >       ext_deps += libbsd
> >   endif
> > -if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> > -     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> > -endif
> >   if cc.has_header('getopt.h')
> >       cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG']
> >   endif
> > diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
> > index fa8753531..fb7bf8a53 100644
> > --- a/mk/rte.cpuflags.mk
> > +++ b/mk/rte.cpuflags.mk
> > @@ -53,6 +53,14 @@ endif
> >
> >   ifneq ($(filter $(AUTO_CPUFLAGS),__RDSEED__),)
> >   CPUFLAGS += RDSEED
> > +else
> > +# If the native environment doesn't define __RDSEED__, see
> > +# if the compiler supports -mrdseed.
>
>
> For which environments is this true?

If you compile on a machine which does not have the RDSEED cpu flag,
gcc will not define __RDSEED__ nor compile the _rdseed32_step()
functions unless you explicitly set -mrdseed on the gcc/clang command
line..

We have a HP DL380 machine which do not have rdseed:
model name      : Intel(R) Xeon(R) CPU E5-2630 v3 @ 2.40GHz

And another which does:
model name      : Intel(R) Xeon(R) CPU E5-2640 v4 @ 2.40GHz

_rdseed32_step() will only compile on the v3 if we explicitly set -mrdseed.

> > +RDSEED_CPUFLAGS := $(shell $(CC) $(MACHINE_CFLAGS) $(WERROR_FLAGS) $(EXTRA_CFLAGS) -mrdseed -dM -E - < /dev/null)
> > +ifneq ($(filter $(RDSEED_CPUFLAGS),__RDSEED__),)
>
>
> There are no better ways to achieve this? It seems like a bit of a hack.

eh.. I don't know of any really.. It's the same command that is used
to set AUTO_CPUFLAGS a few lines before, just with -mrdseed set.  It
doesn't seem that bad to my mind, but if there is a better way, I'm
open to suggestions.

That said, this patch does not work.  We cannot set
RTE_MACHINE_CPUFLAG_RDSEED because rte_eal_init() will fail on a
machine which does not have rdseed because rte_cpu_is_supported()
checks all of these cpu flags set at compile time.  Since we're
detecting this flag at run time, we have to remove it from this list
of "required" CPU flags.

I'm working on a new patch.. I should have something tomorrow.

There is also still a problem with getting getentropy() to work when
the binary is compiled on a system with a glibc < 2.25 but run on a
system with glibc >= 2.25.  It doesn't resolve the weak symbol to the
glibc version.. I think that it's because getentropy() is versioned
within glibc.  I'm still working on how to fix this.. It might come
down to using dlopen(), dlsym()... :(

I'm not really sure if it's worth it.

thanks
dan

Re: [dpdk-dev] [PATCH v2] eal: choose initial PRNG seed source at runtime

[Mattias Rönnblom]

On 2020-04-17 01:50, Dan Gora wrote:
> On Thu, Apr 16, 2020 at 8:30 AM Mattias Rönnblom
> <mattias.ronnblom@ericsson.com> wrote:
>>> diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
>>> index 0267c3b9d..748359b8c 100644
>>> --- a/lib/librte_eal/meson.build
>>> +++ b/lib/librte_eal/meson.build
>>> @@ -15,9 +15,6 @@ deps += 'kvargs'
>>>    if dpdk_conf.has('RTE_USE_LIBBSD')
>>>        ext_deps += libbsd
>>>    endif
>>> -if cc.has_function('getentropy', prefix : '#include <unistd.h>')
>>> -     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
>>> -endif
>>>    if cc.has_header('getopt.h')
>>>        cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG']
>>>    endif
>>> diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
>>> index fa8753531..fb7bf8a53 100644
>>> --- a/mk/rte.cpuflags.mk
>>> +++ b/mk/rte.cpuflags.mk
>>> @@ -53,6 +53,14 @@ endif
>>>
>>>    ifneq ($(filter $(AUTO_CPUFLAGS),__RDSEED__),)
>>>    CPUFLAGS += RDSEED
>>> +else
>>> +# If the native environment doesn't define __RDSEED__, see
>>> +# if the compiler supports -mrdseed.
>>
>> For which environments is this true?
> If you compile on a machine which does not have the RDSEED cpu flag,
> gcc will not define __RDSEED__ nor compile the _rdseed32_step()
> functions unless you explicitly set -mrdseed on the gcc/clang command
> line..
>
> We have a HP DL380 machine which do not have rdseed:
> model name      : Intel(R) Xeon(R) CPU E5-2630 v3 @ 2.40GHz
>
> And another which does:
> model name      : Intel(R) Xeon(R) CPU E5-2640 v4 @ 2.40GHz
>
> _rdseed32_step() will only compile on the v3 if we explicitly set -mrdseed.
>
>>> +RDSEED_CPUFLAGS := $(shell $(CC) $(MACHINE_CFLAGS) $(WERROR_FLAGS) $(EXTRA_CFLAGS) -mrdseed -dM -E - < /dev/null)
>>> +ifneq ($(filter $(RDSEED_CPUFLAGS),__RDSEED__),)
>>
>> There are no better ways to achieve this? It seems like a bit of a hack.
> eh.. I don't know of any really.. It's the same command that is used
> to set AUTO_CPUFLAGS a few lines before, just with -mrdseed set.  It
> doesn't seem that bad to my mind, but if there is a better way, I'm
> open to suggestions.
>
> That said, this patch does not work.  We cannot set
> RTE_MACHINE_CPUFLAG_RDSEED because rte_eal_init() will fail on a
> machine which does not have rdseed because rte_cpu_is_supported()
> checks all of these cpu flags set at compile time.  Since we're
> detecting this flag at run time, we have to remove it from this list
> of "required" CPU flags.
>
> I'm working on a new patch.. I should have something tomorrow.
>
> There is also still a problem with getting getentropy() to work when
> the binary is compiled on a system with a glibc < 2.25 but run on a
> system with glibc >= 2.25.  It doesn't resolve the weak symbol to the
> glibc version.. I think that it's because getentropy() is versioned
> within glibc.  I'm still working on how to fix this.. It might come
> down to using dlopen(), dlsym()... :(


One way would be to include a getrandom syscall wrapper, instead of 
going via libc, in case libc is too old.


> I'm not really sure if it's worth it.


I personally don't think having high-quality initial, automatic, seeding 
is important-enough to warrant much complexity.


> thanks
> dan