All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFT 0/2] pie: floating point fixes
@ 2022-05-24 18:46 Stephen Hemminger
  2022-05-24 18:46 ` [RFT 1/2] rte_pie: remove unnecessary floating point Stephen Hemminger
                   ` (6 more replies)
  0 siblings, 7 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-24 18:46 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

A couple of small untested changes to address some
issues found while reviewing usage of random in DPDK.

The PIE code should get rexamined in later release.
It should not be exposing internal algorithm, that makes
it brittle for ABI.

Also, no code in DPDK should ever be doing floating point
math in the data path!

Stephen Hemminger (2):
  rte_pie: remove unnecessary floating point
  rte_pie: fix incorrect floating point math

 lib/sched/rte_pie.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [RFT 1/2] rte_pie: remove unnecessary floating point
  2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
@ 2022-05-24 18:46 ` Stephen Hemminger
  2022-05-24 18:46 ` [RFT 2/2] rte_pie: fix incorrect floating point math Stephen Hemminger
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-24 18:46 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, wojciechx.liguzinski, Cristian Dumitrescu,
	Jasvinder Singh

The qdelay variable is derived from and compared to 64 bit
value so it doesn't have to be floating point.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Cc: wojciechx.liguzinski@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 02a987f54ad1..3e2c1ef46721 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -218,7 +218,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
 	uint64_t rand_value;
-	double qdelay = pie_cfg->qdelay_ref * 0.5;
+	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
 	if (((pie->qdelay_old < qdelay) && (pie->drop_prob < 0.2)) ||
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFT 2/2] rte_pie: fix incorrect floating point math
  2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
  2022-05-24 18:46 ` [RFT 1/2] rte_pie: remove unnecessary floating point Stephen Hemminger
@ 2022-05-24 18:46 ` Stephen Hemminger
  2022-05-24 19:31 ` [RFT 0/2] pie: floating point fixes Morten Brørup
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-24 18:46 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, wojciechx.liguzinski, Cristian Dumitrescu,
	Jasvinder Singh

The function rte_pie_drop was attempting to do a random probability
drop, but because of incorrect usage of fixed point divide
it would always return 1.

Change to use multiply to compute the probability.
Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Cc: wojciechx.liguzinski@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 3e2c1ef46721..82e6a05645a6 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -217,7 +217,6 @@ __rte_experimental
 _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
-	uint64_t rand_value;
 	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
@@ -240,9 +239,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	if (pie->accu_prob >= 8.5)
 		return 1;
 
-	rand_value = rte_rand()/RTE_RAND_MAX;
-
-	if ((double)rand_value < pie->drop_prob) {
+	if (rte_rand() < (uint64_t)RTE_RAND_MAX * pie->drop_prob) {
 		pie->accu_prob = 0;
 		return 1;
 	}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* RE: [RFT 0/2] pie: floating point fixes
  2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
  2022-05-24 18:46 ` [RFT 1/2] rte_pie: remove unnecessary floating point Stephen Hemminger
  2022-05-24 18:46 ` [RFT 2/2] rte_pie: fix incorrect floating point math Stephen Hemminger
@ 2022-05-24 19:31 ` Morten Brørup
  2022-05-24 22:18 ` [RFT v2 0/3] pie: fix random number issues Stephen Hemminger
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 36+ messages in thread
From: Morten Brørup @ 2022-05-24 19:31 UTC (permalink / raw)
  To: Stephen Hemminger, dev

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, 24 May 2022 20.46
> 
> A couple of small untested changes to address some
> issues found while reviewing usage of random in DPDK.
> 
> The PIE code should get rexamined in later release.

Untested fixes to seemingly untested code. What's not to like? ;-)

> It should not be exposing internal algorithm, that makes
> it brittle for ABI.

Agree. Just look at the mempool... an increasing amount of its internals are getting elevated to public API, so other code can understand and use its internals instead of going through its API. (My current pet peeve.)

> 
> Also, no code in DPDK should ever be doing floating point
> math in the data path!

+1

> 
> Stephen Hemminger (2):
>   rte_pie: remove unnecessary floating point
>   rte_pie: fix incorrect floating point math

This looks much better than what it replaces, so...

Series-acked-by: Morten Brørup <mb@smartsharesystems.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [RFT v2 0/3] pie: fix random number issues
  2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
                   ` (2 preceding siblings ...)
  2022-05-24 19:31 ` [RFT 0/2] pie: floating point fixes Morten Brørup
@ 2022-05-24 22:18 ` Stephen Hemminger
  2022-05-24 22:18   ` [RFT v2 1/3] random: add rte_rand_float() Stephen Hemminger
                     ` (2 more replies)
  2022-05-25 17:12 ` [PATCH v3 0/3] introduce random floating point function Stephen Hemminger
                   ` (2 subsequent siblings)
  6 siblings, 3 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-24 22:18 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Alternate way to fix PIE use of random by introducing a fast
way to get a random floating point number. It ends up
not needing floating point math.

Stephen Hemminger (3):
  random: add rte_rand_float()
  rte_pie: remove unnecessary floating point
  rte_pie: fix incorrect floating point math

 app/test/test_rand_perf.c              |  7 +++++++
 doc/guides/rel_notes/release_22_07.rst |  5 +++++
 lib/eal/common/rte_random.c            | 21 +++++++++++++++++++++
 lib/eal/include/rte_random.h           | 17 +++++++++++++++++
 lib/eal/version.map                    |  1 +
 lib/sched/rte_pie.h                    |  7 ++-----
 6 files changed, 53 insertions(+), 5 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [RFT v2 1/3] random: add rte_rand_float()
  2022-05-24 22:18 ` [RFT v2 0/3] pie: fix random number issues Stephen Hemminger
@ 2022-05-24 22:18   ` Stephen Hemminger
  2022-05-25 11:55     ` Ray Kinsella
  2022-05-25 14:45     ` Mattias Rönnblom
  2022-05-24 22:18   ` [RFT v2 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
  2022-05-24 22:18   ` [RFT v2 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
  2 siblings, 2 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-24 22:18 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Mattias Rönnblom, Ray Kinsella

The PIE code and other applications can benefit from having a
fast way to get a random floating point value. This new function
is equivalent to erand48_r in the standard library.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_rand_perf.c              |  7 +++++++
 doc/guides/rel_notes/release_22_07.rst |  5 +++++
 lib/eal/common/rte_random.c            | 21 +++++++++++++++++++++
 lib/eal/include/rte_random.h           | 17 +++++++++++++++++
 lib/eal/version.map                    |  1 +
 5 files changed, 51 insertions(+)

diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
index fe797ebfa1ca..f3602da5b2d4 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_float,
 	rand_type_bounded_best_case,
 	rand_type_bounded_worst_case
 };
@@ -30,6 +31,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_float:
+		return "Floating point [rte_rand_float()]";
 	case rand_type_bounded_best_case:
 		return "Bounded average best-case [rte_rand_max()]";
 	case rand_type_bounded_worst_case:
@@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
 		case rand_type_64:
 			sum += rte_rand();
 			break;
+		case rand_type_float:
+			sum += rte_rand_float() * UINT64_MAX;
+			break;
 		case rand_type_bounded_best_case:
 			sum += rte_rand_max(BEST_CASE_BOUND);
 			break;
@@ -83,6 +89,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_float);
 	test_rand_perf_type(rand_type_bounded_best_case);
 	test_rand_perf_type(rand_type_bounded_worst_case);
 
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index e49cacecefd4..128d4fca85b3 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -104,6 +104,11 @@ New Features
   * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
   * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
 
+* ** Added function get random floating point number.**
+
+  Added the function ``rte_rand_float()`` to provide a pseudo-random
+  floating point number.
+
 
 Removed Items
 -------------
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 4535cc980cec..024c3c41dc16 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -6,6 +6,7 @@
 #include <x86intrin.h>
 #endif
 #include <unistd.h>
+#include <ieee754.h>
 
 #include <rte_branch_prediction.h>
 #include <rte_cycles.h>
@@ -173,6 +174,26 @@ rte_rand_max(uint64_t upper_bound)
 	return res;
 }
 
+double
+rte_rand_float(void)
+{
+	struct rte_rand_state *state = __rte_rand_get_state();
+	union ieee754_double u = {
+		.ieee = {
+			.negative = 0,
+			.exponent = IEEE754_DOUBLE_BIAS,
+		},
+	};
+	uint64_t val;
+
+	/* Take 64 bit random value and put it into the mantissa */
+	val = __rte_rand_lfsr258(state);
+	u.ieee.mantissa0 = val >> 32;	/* only 20 bits used */
+	u.ieee.mantissa1 = (uint32_t)val;
+
+	return u.d - 1.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 29f5f1325a30..553beb2d9c6f 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -65,6 +65,23 @@ rte_rand(void);
 uint64_t
 rte_rand_max(uint64_t upper_bound);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Generates a pseudo-random floating point number.
+ *
+ * This function returns a nonnegative double-precison floating random
+ * number uniformly distributed over the interval [0.0, 1.0).
+ *
+ * If called from lcore threads, this function is thread-safe.
+ *
+ * @return
+ *   A pseudo-random value between 0 and 1.0.
+ */
+__rte_experimental
+double rte_rand_float(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/eal/version.map b/lib/eal/version.map
index d49e30bd042f..861906af2999 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -422,6 +422,7 @@ EXPERIMENTAL {
 	rte_intr_type_set;
 
 	# added in 22.07
+        rte_rand_float;
 	rte_thread_get_affinity_by_id;
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFT v2 2/3] rte_pie: remove unnecessary floating point
  2022-05-24 22:18 ` [RFT v2 0/3] pie: fix random number issues Stephen Hemminger
  2022-05-24 22:18   ` [RFT v2 1/3] random: add rte_rand_float() Stephen Hemminger
@ 2022-05-24 22:18   ` Stephen Hemminger
  2022-05-24 22:18   ` [RFT v2 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-24 22:18 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, wojciechx.liguzinski, Cristian Dumitrescu,
	Jasvinder Singh

The qdelay variable is derived from and compared to 64 bit
value so it doesn't have to be floating point.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Cc: wojciechx.liguzinski@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 02a987f54ad1..3e2c1ef46721 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -218,7 +218,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
 	uint64_t rand_value;
-	double qdelay = pie_cfg->qdelay_ref * 0.5;
+	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
 	if (((pie->qdelay_old < qdelay) && (pie->drop_prob < 0.2)) ||
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFT v2 3/3] rte_pie: fix incorrect floating point math
  2022-05-24 22:18 ` [RFT v2 0/3] pie: fix random number issues Stephen Hemminger
  2022-05-24 22:18   ` [RFT v2 1/3] random: add rte_rand_float() Stephen Hemminger
  2022-05-24 22:18   ` [RFT v2 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
@ 2022-05-24 22:18   ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-24 22:18 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, wojciechx.liguzinski, Cristian Dumitrescu,
	Jasvinder Singh

The function rte_pie_drop was attempting to do a random probability
drop, but because of incorrect usage of fixed point divide
it would always return 1.

Change to use new rte_rand_float() instead.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Cc: wojciechx.liguzinski@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 3e2c1ef46721..ed433300c2cf 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -217,7 +217,6 @@ __rte_experimental
 _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
-	uint64_t rand_value;
 	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
@@ -240,9 +239,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	if (pie->accu_prob >= 8.5)
 		return 1;
 
-	rand_value = rte_rand()/RTE_RAND_MAX;
-
-	if ((double)rand_value < pie->drop_prob) {
+	if (rte_rand_float() < pie->drop_prob) {
 		pie->accu_prob = 0;
 		return 1;
 	}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [RFT v2 1/3] random: add rte_rand_float()
  2022-05-24 22:18   ` [RFT v2 1/3] random: add rte_rand_float() Stephen Hemminger
@ 2022-05-25 11:55     ` Ray Kinsella
  2022-05-25 14:45     ` Mattias Rönnblom
  1 sibling, 0 replies; 36+ messages in thread
From: Ray Kinsella @ 2022-05-25 11:55 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Mattias Rönnblom


Stephen Hemminger <stephen@networkplumber.org> writes:

> The PIE code and other applications can benefit from having a
> fast way to get a random floating point value. This new function
> is equivalent to erand48_r in the standard library.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  app/test/test_rand_perf.c              |  7 +++++++
>  doc/guides/rel_notes/release_22_07.rst |  5 +++++
>  lib/eal/common/rte_random.c            | 21 +++++++++++++++++++++
>  lib/eal/include/rte_random.h           | 17 +++++++++++++++++
>  lib/eal/version.map                    |  1 +
>  5 files changed, 51 insertions(+)
>
> diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
> index fe797ebfa1ca..f3602da5b2d4 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_float,
>  	rand_type_bounded_best_case,
>  	rand_type_bounded_worst_case
>  };
> @@ -30,6 +31,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_float:
> +		return "Floating point [rte_rand_float()]";
>  	case rand_type_bounded_best_case:
>  		return "Bounded average best-case [rte_rand_max()]";
>  	case rand_type_bounded_worst_case:
> @@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
>  		case rand_type_64:
>  			sum += rte_rand();
>  			break;
> +		case rand_type_float:
> +			sum += rte_rand_float() * UINT64_MAX;
> +			break;
>  		case rand_type_bounded_best_case:
>  			sum += rte_rand_max(BEST_CASE_BOUND);
>  			break;
> @@ -83,6 +89,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_float);
>  	test_rand_perf_type(rand_type_bounded_best_case);
>  	test_rand_perf_type(rand_type_bounded_worst_case);
>  
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index e49cacecefd4..128d4fca85b3 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -104,6 +104,11 @@ New Features
>    * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
>    * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
>  
> +* ** Added function get random floating point number.**
> +
> +  Added the function ``rte_rand_float()`` to provide a pseudo-random
> +  floating point number.
> +
>  
>  Removed Items
>  -------------
> diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
> index 4535cc980cec..024c3c41dc16 100644
> --- a/lib/eal/common/rte_random.c
> +++ b/lib/eal/common/rte_random.c
> @@ -6,6 +6,7 @@
>  #include <x86intrin.h>
>  #endif
>  #include <unistd.h>
> +#include <ieee754.h>
>  
>  #include <rte_branch_prediction.h>
>  #include <rte_cycles.h>
> @@ -173,6 +174,26 @@ rte_rand_max(uint64_t upper_bound)
>  	return res;
>  }
>  
> +double
> +rte_rand_float(void)
> +{
> +	struct rte_rand_state *state = __rte_rand_get_state();
> +	union ieee754_double u = {
> +		.ieee = {
> +			.negative = 0,
> +			.exponent = IEEE754_DOUBLE_BIAS,
> +		},
> +	};
> +	uint64_t val;
> +
> +	/* Take 64 bit random value and put it into the mantissa */
> +	val = __rte_rand_lfsr258(state);
> +	u.ieee.mantissa0 = val >> 32;	/* only 20 bits used */
> +	u.ieee.mantissa1 = (uint32_t)val;
> +
> +	return u.d - 1.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 29f5f1325a30..553beb2d9c6f 100644
> --- a/lib/eal/include/rte_random.h
> +++ b/lib/eal/include/rte_random.h
> @@ -65,6 +65,23 @@ rte_rand(void);
>  uint64_t
>  rte_rand_max(uint64_t upper_bound);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Generates a pseudo-random floating point number.
> + *
> + * This function returns a nonnegative double-precison floating random
> + * number uniformly distributed over the interval [0.0, 1.0).
> + *
> + * If called from lcore threads, this function is thread-safe.
> + *
> + * @return
> + *   A pseudo-random value between 0 and 1.0.
> + */
> +__rte_experimental
> +double rte_rand_float(void);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index d49e30bd042f..861906af2999 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -422,6 +422,7 @@ EXPERIMENTAL {
>  	rte_intr_type_set;
>  
>  	# added in 22.07
> +        rte_rand_float;
Niggle - is there an alignment issue on the line above?

>  	rte_thread_get_affinity_by_id;
>  	rte_thread_self;
>  	rte_thread_set_affinity_by_id;


-- 
Regards, Ray K

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFT v2 1/3] random: add rte_rand_float()
  2022-05-24 22:18   ` [RFT v2 1/3] random: add rte_rand_float() Stephen Hemminger
  2022-05-25 11:55     ` Ray Kinsella
@ 2022-05-25 14:45     ` Mattias Rönnblom
  2022-05-25 15:26       ` Morten Brørup
                         ` (2 more replies)
  1 sibling, 3 replies; 36+ messages in thread
From: Mattias Rönnblom @ 2022-05-25 14:45 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Ray Kinsella

On 2022-05-25 00:18, Stephen Hemminger wrote:
> The PIE code and other applications can benefit from having a
> fast way to get a random floating point value. This new function
> is equivalent to erand48_r in the standard library.
> 

Seems like a good addition to the API.

> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>   app/test/test_rand_perf.c              |  7 +++++++
>   doc/guides/rel_notes/release_22_07.rst |  5 +++++
>   lib/eal/common/rte_random.c            | 21 +++++++++++++++++++++
>   lib/eal/include/rte_random.h           | 17 +++++++++++++++++
>   lib/eal/version.map                    |  1 +
>   5 files changed, 51 insertions(+)
> 
> diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
> index fe797ebfa1ca..f3602da5b2d4 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_float,
>   	rand_type_bounded_best_case,
>   	rand_type_bounded_worst_case
>   };
> @@ -30,6 +31,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_float:
> +		return "Floating point [rte_rand_float()]";
>   	case rand_type_bounded_best_case:
>   		return "Bounded average best-case [rte_rand_max()]";
>   	case rand_type_bounded_worst_case:
> @@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
>   		case rand_type_64:
>   			sum += rte_rand();
>   			break;
> +		case rand_type_float:
> +			sum += rte_rand_float() * UINT64_MAX;
> +			break;
>   		case rand_type_bounded_best_case:
>   			sum += rte_rand_max(BEST_CASE_BOUND);
>   			break;
> @@ -83,6 +89,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_float);
>   	test_rand_perf_type(rand_type_bounded_best_case);
>   	test_rand_perf_type(rand_type_bounded_worst_case);
>   
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index e49cacecefd4..128d4fca85b3 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -104,6 +104,11 @@ New Features
>     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
>     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
>   
> +* ** Added function get random floating point number.**
> +
> +  Added the function ``rte_rand_float()`` to provide a pseudo-random
> +  floating point number.
> +
>   
>   Removed Items
>   -------------
> diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
> index 4535cc980cec..024c3c41dc16 100644
> --- a/lib/eal/common/rte_random.c
> +++ b/lib/eal/common/rte_random.c
> @@ -6,6 +6,7 @@
>   #include <x86intrin.h>
>   #endif
>   #include <unistd.h>
> +#include <ieee754.h>
>   

Is this API available in FreeBSD? In Windows?

>   #include <rte_branch_prediction.h>
>   #include <rte_cycles.h>
> @@ -173,6 +174,26 @@ rte_rand_max(uint64_t upper_bound)
>   	return res;
>   }
>   
> +double
> +rte_rand_float(void)
> +{
> +	struct rte_rand_state *state = __rte_rand_get_state();
> +	union ieee754_double u = {
> +		.ieee = {
> +			.negative = 0,
> +			.exponent = IEEE754_DOUBLE_BIAS,
> +		},
> +	};
> +	uint64_t val;
> +
> +	/* Take 64 bit random value and put it into the mantissa */
> +	val = __rte_rand_lfsr258(state);
> +	u.ieee.mantissa0 = val >> 32;	/* only 20 bits used */
> +	u.ieee.mantissa1 = (uint32_t)val;

Why do you have a cast here, and not for mantissa0? Both will incur a 
64->32 conversion, assuming unsigned int is 32-bit (which it is on all 
platforms DPDK supports?).

The naive implementation (i.e., (double)rte_rand() / (double)UINT64_MAX) 
would cost a floating point multiplication. That's why you access the 
underlying IEEE754 bits directly. Correct?

> +
> +	return u.d - 1.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 29f5f1325a30..553beb2d9c6f 100644
> --- a/lib/eal/include/rte_random.h
> +++ b/lib/eal/include/rte_random.h
> @@ -65,6 +65,23 @@ rte_rand(void);
>   uint64_t
>   rte_rand_max(uint64_t upper_bound);
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Generates a pseudo-random floating point number.
> + *
> + * This function returns a nonnegative double-precison floating random
> + * number uniformly distributed over the interval [0.0, 1.0).
> + *
> + * If called from lcore threads, this function is thread-safe.
> + *
> + * @return
> + *   A pseudo-random value between 0 and 1.0.
> + */
> +__rte_experimental
> +double rte_rand_float(void);
> +

Newline after "double" missing.

I would call it something else than "float", in particular since it 
doesn't return "float" but a "double" type floating point value.

rte_drand() maybe? Short, but might be confused with rte_rand(), given 
the visual similarity. I still I would still prefer that over 
rte_rand_double().

>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index d49e30bd042f..861906af2999 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -422,6 +422,7 @@ EXPERIMENTAL {
>   	rte_intr_type_set;
>   
>   	# added in 22.07
> +        rte_rand_float;
>   	rte_thread_get_affinity_by_id;
>   	rte_thread_self;
>   	rte_thread_set_affinity_by_id;


^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [RFT v2 1/3] random: add rte_rand_float()
  2022-05-25 14:45     ` Mattias Rönnblom
@ 2022-05-25 15:26       ` Morten Brørup
  2022-05-25 15:45       ` Stephen Hemminger
  2022-05-25 15:47       ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Morten Brørup @ 2022-05-25 15:26 UTC (permalink / raw)
  To: Mattias Rönnblom, Stephen Hemminger, dev; +Cc: Ray Kinsella

> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
> Sent: Wednesday, 25 May 2022 16.46
> 
> On 2022-05-25 00:18, Stephen Hemminger wrote:
> > The PIE code and other applications can benefit from having a
> > fast way to get a random floating point value. This new function
> > is equivalent to erand48_r in the standard library.
> >

> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Generates a pseudo-random floating point number.
> > + *
> > + * This function returns a nonnegative double-precison floating
> random
> > + * number uniformly distributed over the interval [0.0, 1.0).
> > + *
> > + * If called from lcore threads, this function is thread-safe.
> > + *
> > + * @return
> > + *   A pseudo-random value between 0 and 1.0.
> > + */
> > +__rte_experimental
> > +double rte_rand_float(void);
> > +
> 
> Newline after "double" missing.
> 
> I would call it something else than "float", in particular since it
> doesn't return "float" but a "double" type floating point value.
> 
> rte_drand() maybe? Short, but might be confused with rte_rand(), given
> the visual similarity. I still I would still prefer that over
> rte_rand_double().

Although we use foo32() and foo64() for many functions returning uint32_t/uint64_t, I don't think we need a common (and preferably short) postfix for functions returning double. Such functions should be rare.

So:
+1 to rte_drand() - it resembles drand48() in stdlib.h.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFT v2 1/3] random: add rte_rand_float()
  2022-05-25 14:45     ` Mattias Rönnblom
  2022-05-25 15:26       ` Morten Brørup
@ 2022-05-25 15:45       ` Stephen Hemminger
  2022-05-25 15:47       ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 15:45 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev, Ray Kinsella

On Wed, 25 May 2022 14:45:37 +0000
Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:

> On 2022-05-25 00:18, Stephen Hemminger wrote:
> > The PIE code and other applications can benefit from having a
> > fast way to get a random floating point value. This new function
> > is equivalent to erand48_r in the standard library.
> >   
> 
> Seems like a good addition to the API.
> 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >   app/test/test_rand_perf.c              |  7 +++++++
> >   doc/guides/rel_notes/release_22_07.rst |  5 +++++
> >   lib/eal/common/rte_random.c            | 21 +++++++++++++++++++++
> >   lib/eal/include/rte_random.h           | 17 +++++++++++++++++
> >   lib/eal/version.map                    |  1 +
> >   5 files changed, 51 insertions(+)
> > 
> > diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
> > index fe797ebfa1ca..f3602da5b2d4 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_float,
> >   	rand_type_bounded_best_case,
> >   	rand_type_bounded_worst_case
> >   };
> > @@ -30,6 +31,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_float:
> > +		return "Floating point [rte_rand_float()]";
> >   	case rand_type_bounded_best_case:
> >   		return "Bounded average best-case [rte_rand_max()]";
> >   	case rand_type_bounded_worst_case:
> > @@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
> >   		case rand_type_64:
> >   			sum += rte_rand();
> >   			break;
> > +		case rand_type_float:
> > +			sum += rte_rand_float() * UINT64_MAX;
> > +			break;
> >   		case rand_type_bounded_best_case:
> >   			sum += rte_rand_max(BEST_CASE_BOUND);
> >   			break;
> > @@ -83,6 +89,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_float);
> >   	test_rand_perf_type(rand_type_bounded_best_case);
> >   	test_rand_perf_type(rand_type_bounded_worst_case);
> >   
> > diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> > index e49cacecefd4..128d4fca85b3 100644
> > --- a/doc/guides/rel_notes/release_22_07.rst
> > +++ b/doc/guides/rel_notes/release_22_07.rst
> > @@ -104,6 +104,11 @@ New Features
> >     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
> >     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
> >   
> > +* ** Added function get random floating point number.**
> > +
> > +  Added the function ``rte_rand_float()`` to provide a pseudo-random
> > +  floating point number.
> > +
> >   
> >   Removed Items
> >   -------------
> > diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
> > index 4535cc980cec..024c3c41dc16 100644
> > --- a/lib/eal/common/rte_random.c
> > +++ b/lib/eal/common/rte_random.c
> > @@ -6,6 +6,7 @@
> >   #include <x86intrin.h>
> >   #endif
> >   #include <unistd.h>
> > +#include <ieee754.h>
> >     
> 
> Is this API available in FreeBSD? In Windows?
> 
> >   #include <rte_branch_prediction.h>
> >   #include <rte_cycles.h>
> > @@ -173,6 +174,26 @@ rte_rand_max(uint64_t upper_bound)
> >   	return res;
> >   }
> >   
> > +double
> > +rte_rand_float(void)
> > +{
> > +	struct rte_rand_state *state = __rte_rand_get_state();
> > +	union ieee754_double u = {
> > +		.ieee = {
> > +			.negative = 0,
> > +			.exponent = IEEE754_DOUBLE_BIAS,
> > +		},
> > +	};
> > +	uint64_t val;
> > +
> > +	/* Take 64 bit random value and put it into the mantissa */
> > +	val = __rte_rand_lfsr258(state);
> > +	u.ieee.mantissa0 = val >> 32;	/* only 20 bits used */
> > +	u.ieee.mantissa1 = (uint32_t)val;  
> 
> Why do you have a cast here, and not for mantissa0? Both will incur a 
> 64->32 conversion, assuming unsigned int is 32-bit (which it is on all 
> platforms DPDK supports?).

Yes, cast there is unnecessary. A little concerned that some compiler
might complain about loss of precision.

> 
> The naive implementation (i.e., (double)rte_rand() / (double)UINT64_MAX) 
> would cost a floating point multiplication. That's why you access the 
> underlying IEEE754 bits directly. Correct?

Yes, avoiding floating point math, and directly setting bits keeps full precision.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFT v2 1/3] random: add rte_rand_float()
  2022-05-25 14:45     ` Mattias Rönnblom
  2022-05-25 15:26       ` Morten Brørup
  2022-05-25 15:45       ` Stephen Hemminger
@ 2022-05-25 15:47       ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 15:47 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev, Ray Kinsella

On Wed, 25 May 2022 14:45:37 +0000
Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:

> I would call it something else than "float", in particular since it 
> doesn't return "float" but a "double" type floating point value.
> 
> rte_drand() maybe? Short, but might be confused with rte_rand(), given 
> the visual similarity. I still I would still prefer that over 
> rte_rand_double().

The corresponding standard library routine is drand48().
With that convention it would be rte_drand().

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v3 0/3] introduce random floating point function
  2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
                   ` (3 preceding siblings ...)
  2022-05-24 22:18 ` [RFT v2 0/3] pie: fix random number issues Stephen Hemminger
@ 2022-05-25 17:12 ` Stephen Hemminger
  2022-05-25 17:12   ` [PATCH v3 1/3] random: add rte_drand() funciton Stephen Hemminger
                     ` (2 more replies)
  2022-05-25 20:31 ` [PATCH v4 0/3] introduce random floating point function Stephen Hemminger
  2022-05-26 20:26 ` [PATCH v5 " Stephen Hemminger
  6 siblings, 3 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 17:12 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Introduce a new random number function to get a floating
point value. Then use it to fix PIE scheduler.

v3
   - rename to rte_drand() and other review feedback
   - fix warnings with some compilers in test code

Stephen Hemminger (3):
  random: add rte_drand() funciton
  rte_pie: remove unnecessary floating point
  rte_pie: fix incorrect floating point math

 app/test/test_rand_perf.c              |  7 +++++++
 doc/guides/rel_notes/release_22_07.rst |  5 +++++
 lib/eal/common/rte_random.c            | 24 ++++++++++++++++++++++++
 lib/eal/include/rte_random.h           | 18 ++++++++++++++++++
 lib/eal/version.map                    |  1 +
 lib/sched/rte_pie.h                    |  7 ++-----
 6 files changed, 57 insertions(+), 5 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v3 1/3] random: add rte_drand() funciton
  2022-05-25 17:12 ` [PATCH v3 0/3] introduce random floating point function Stephen Hemminger
@ 2022-05-25 17:12   ` Stephen Hemminger
  2022-05-25 17:12   ` [PATCH v3 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
  2022-05-25 17:12   ` [PATCH v3 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 17:12 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Mattias Rönnblom, Ray Kinsella

The PIE code and other applications can benefit from having a
fast way to get a random floating point value. This new function
is equivalent to drand() in the standard library.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_rand_perf.c              |  7 +++++++
 doc/guides/rel_notes/release_22_07.rst |  5 +++++
 lib/eal/common/rte_random.c            | 24 ++++++++++++++++++++++++
 lib/eal/include/rte_random.h           | 18 ++++++++++++++++++
 lib/eal/version.map                    |  1 +
 5 files changed, 55 insertions(+)

diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
index fe797ebfa1ca..26fb1d9a586e 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_float,
 	rand_type_bounded_best_case,
 	rand_type_bounded_worst_case
 };
@@ -30,6 +31,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_float:
+		return "Floating point [rte_drand()]";
 	case rand_type_bounded_best_case:
 		return "Bounded average best-case [rte_rand_max()]";
 	case rand_type_bounded_worst_case:
@@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
 		case rand_type_64:
 			sum += rte_rand();
 			break;
+		case rand_type_float:
+			sum += 1000. * rte_drand();
+			break;
 		case rand_type_bounded_best_case:
 			sum += rte_rand_max(BEST_CASE_BOUND);
 			break;
@@ -83,6 +89,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_float);
 	test_rand_perf_type(rand_type_bounded_best_case);
 	test_rand_perf_type(rand_type_bounded_worst_case);
 
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index e49cacecefd4..128d4fca85b3 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -104,6 +104,11 @@ New Features
   * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
   * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
 
+* ** Added function get random floating point number.**
+
+  Added the function ``rte_drand()`` to provide a pseudo-random
+  floating point number.
+
 
 Removed Items
 -------------
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 4535cc980cec..a30bb104015b 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -6,6 +6,7 @@
 #include <x86intrin.h>
 #endif
 #include <unistd.h>
+#include <ieee754.h>
 
 #include <rte_branch_prediction.h>
 #include <rte_cycles.h>
@@ -173,6 +174,29 @@ rte_rand_max(uint64_t upper_bound)
 	return res;
 }
 
+double
+rte_drand(void)
+{
+	struct rte_rand_state *state = __rte_rand_get_state();
+	union ieee754_double u = {
+		.ieee = {
+			.negative = 0,
+			.exponent = IEEE754_DOUBLE_BIAS,
+		},
+	};
+	uint64_t val;
+
+	/* Take 64 bit random value and put it into the mantissa
+	 * This uses direct access to IEEE format to avoid doing
+	 * any direct floating point math here.
+	 */
+	val = __rte_rand_lfsr258(state);
+	u.ieee.mantissa0 = val >> 32;
+	u.ieee.mantissa1 = val;
+
+	return u.d - 1.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 29f5f1325a30..f6541c2b0f08 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -65,6 +65,24 @@ rte_rand(void);
 uint64_t
 rte_rand_max(uint64_t upper_bound);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Generates a pseudo-random floating point number.
+ *
+ * This function returns a nonnegative double-precision floating random
+ * number uniformly distributed over the interval [0.0, 1.0).
+ *
+ * The generator is not cryptographically secure.
+ * If called from lcore threads, this function is thread-safe.
+ *
+ * @return
+ *   A pseudo-random value between 0 and 1.0.
+ */
+__rte_experimental
+double rte_drand(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/eal/version.map b/lib/eal/version.map
index d49e30bd042f..cfbade9a33e9 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -422,6 +422,7 @@ EXPERIMENTAL {
 	rte_intr_type_set;
 
 	# added in 22.07
+	rte_drand;
 	rte_thread_get_affinity_by_id;
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 2/3] rte_pie: remove unnecessary floating point
  2022-05-25 17:12 ` [PATCH v3 0/3] introduce random floating point function Stephen Hemminger
  2022-05-25 17:12   ` [PATCH v3 1/3] random: add rte_drand() funciton Stephen Hemminger
@ 2022-05-25 17:12   ` Stephen Hemminger
  2022-05-25 17:12   ` [PATCH v3 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 17:12 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, wojciechx.liguzinski, Cristian Dumitrescu,
	Jasvinder Singh

The qdelay variable is derived from and compared to 64 bit
value so it doesn't have to be floating point.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Cc: wojciechx.liguzinski@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 02a987f54ad1..3e2c1ef46721 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -218,7 +218,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
 	uint64_t rand_value;
-	double qdelay = pie_cfg->qdelay_ref * 0.5;
+	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
 	if (((pie->qdelay_old < qdelay) && (pie->drop_prob < 0.2)) ||
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 3/3] rte_pie: fix incorrect floating point math
  2022-05-25 17:12 ` [PATCH v3 0/3] introduce random floating point function Stephen Hemminger
  2022-05-25 17:12   ` [PATCH v3 1/3] random: add rte_drand() funciton Stephen Hemminger
  2022-05-25 17:12   ` [PATCH v3 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
@ 2022-05-25 17:12   ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 17:12 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, wojciechx.liguzinski, Cristian Dumitrescu,
	Jasvinder Singh

The function rte_pie_drop was attempting to do a random probability
drop, but because of incorrect usage of fixed point divide
it would always return 1.

Change to use new rte_drand() instead.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Cc: wojciechx.liguzinski@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 3e2c1ef46721..528f2ea878e8 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -217,7 +217,6 @@ __rte_experimental
 _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
-	uint64_t rand_value;
 	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
@@ -240,9 +239,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	if (pie->accu_prob >= 8.5)
 		return 1;
 
-	rand_value = rte_rand()/RTE_RAND_MAX;
-
-	if ((double)rand_value < pie->drop_prob) {
+	if (rte_drand() < pie->drop_prob) {
 		pie->accu_prob = 0;
 		return 1;
 	}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 0/3] introduce random floating point function
  2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
                   ` (4 preceding siblings ...)
  2022-05-25 17:12 ` [PATCH v3 0/3] introduce random floating point function Stephen Hemminger
@ 2022-05-25 20:31 ` Stephen Hemminger
  2022-05-25 20:31   ` [PATCH v4 1/3] random: add rte_drand() function Stephen Hemminger
                     ` (3 more replies)
  2022-05-26 20:26 ` [PATCH v5 " Stephen Hemminger
  6 siblings, 4 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 20:31 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Introduce a new random number function to get a floating
point value. Then use it to fix PIE scheduler.

v4
   - use slower divide method in rte_drand() if library
     does not have ieee754 (FreeBSD and Windows?)
   - fix some typos

v3
   - rename to rte_drand() and other review feedback
   - fix warnings with some compilers in test code

Stephen Hemminger (3):
  random: add rte_drand() function
  rte_pie: remove unnecessary floating point
  rte_pie: fix incorrect floating point math

 app/test/test_rand_perf.c              |  7 +++++
 doc/guides/rel_notes/release_22_07.rst |  5 ++++
 lib/eal/common/rte_random.c            | 41 ++++++++++++++++++++++++++
 lib/eal/include/rte_random.h           | 18 +++++++++++
 lib/eal/meson.build                    |  3 ++
 lib/eal/version.map                    |  1 +
 lib/sched/rte_pie.h                    |  7 ++---
 7 files changed, 77 insertions(+), 5 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 1/3] random: add rte_drand() function
  2022-05-25 20:31 ` [PATCH v4 0/3] introduce random floating point function Stephen Hemminger
@ 2022-05-25 20:31   ` Stephen Hemminger
  2022-05-26  9:56     ` Ray Kinsella
  2022-05-26 13:20     ` Mattias Rönnblom
  2022-05-25 20:31   ` [PATCH v4 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 20:31 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Mattias Rönnblom, Ray Kinsella

The PIE code and other applications can benefit from having a
fast way to get a random floating point value. This new function
is equivalent to drand() in the standard library.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_rand_perf.c              |  7 +++++
 doc/guides/rel_notes/release_22_07.rst |  5 ++++
 lib/eal/common/rte_random.c            | 41 ++++++++++++++++++++++++++
 lib/eal/include/rte_random.h           | 18 +++++++++++
 lib/eal/meson.build                    |  3 ++
 lib/eal/version.map                    |  1 +
 6 files changed, 75 insertions(+)

diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
index fe797ebfa1ca..26fb1d9a586e 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_float,
 	rand_type_bounded_best_case,
 	rand_type_bounded_worst_case
 };
@@ -30,6 +31,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_float:
+		return "Floating point [rte_drand()]";
 	case rand_type_bounded_best_case:
 		return "Bounded average best-case [rte_rand_max()]";
 	case rand_type_bounded_worst_case:
@@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
 		case rand_type_64:
 			sum += rte_rand();
 			break;
+		case rand_type_float:
+			sum += 1000. * rte_drand();
+			break;
 		case rand_type_bounded_best_case:
 			sum += rte_rand_max(BEST_CASE_BOUND);
 			break;
@@ -83,6 +89,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_float);
 	test_rand_perf_type(rand_type_bounded_best_case);
 	test_rand_perf_type(rand_type_bounded_worst_case);
 
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index e49cacecefd4..b131ea577226 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -104,6 +104,11 @@ New Features
   * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
   * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
 
+* ** Added function get random floating point number.**
+
+  Added the function ``rte_drand()`` to provide a pseudo-random
+  floating point number.
+
 
 Removed Items
 -------------
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 4535cc980cec..3dc3484ee655 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -6,6 +6,9 @@
 #include <x86intrin.h>
 #endif
 #include <unistd.h>
+#ifdef RTE_LIBEAL_USE_IEEE754
+#include <ieee754.h>
+#endif
 
 #include <rte_branch_prediction.h>
 #include <rte_cycles.h>
@@ -173,6 +176,44 @@ rte_rand_max(uint64_t upper_bound)
 	return res;
 }
 
+double
+rte_drand(void)
+{
+	struct rte_rand_state *state = __rte_rand_get_state();
+	uint64_t rand64 = __rte_rand_lfsr258(state);
+#ifdef RTE_LIBEAL_USE_IEEE754
+	union ieee754_double u = {
+		.ieee = {
+			.negative = 0,
+			.exponent = IEEE754_DOUBLE_BIAS,
+		},
+	};
+
+	/* Take 64 bit random value and put it into the mantissa
+	 * This uses direct access to IEEE format to avoid doing
+	 * any direct floating point math here.
+	 */
+	u.ieee.mantissa0 = rand64 >> 32;
+	u.ieee.mantissa1 = rand64;
+
+	return u.d - 1.0;
+#else
+	/* Slower method requiring floating point divide
+	 *
+	 * The double mantissa only has 53 bits, so we uniformly mask off the
+	 * high 11 bits and then floating-point divide by 2^53 to achieve a
+	 * result in [0, 1).
+	 *
+	 * We are not allowed to emit 1.0, so denom must be one greater than
+	 * the possible range of the preceeding step.
+	 */
+	static const uint64_t denom = (uint64_t)1 << 53;
+
+	rand64 &= denom - 1;
+	return (double)rand64 / denom;
+#endif
+}
+
 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 29f5f1325a30..f6541c2b0f08 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -65,6 +65,24 @@ rte_rand(void);
 uint64_t
 rte_rand_max(uint64_t upper_bound);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Generates a pseudo-random floating point number.
+ *
+ * This function returns a nonnegative double-precision floating random
+ * number uniformly distributed over the interval [0.0, 1.0).
+ *
+ * The generator is not cryptographically secure.
+ * If called from lcore threads, this function is thread-safe.
+ *
+ * @return
+ *   A pseudo-random value between 0 and 1.0.
+ */
+__rte_experimental
+double rte_drand(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 056beb946119..e50524901c98 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -32,3 +32,6 @@ endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
+if cc.has_header_symbol('ieee754.h', 'union ieee754_double')
+    cflags += '-DRTE_LIBEAL_USE_IEEE754'
+endif
diff --git a/lib/eal/version.map b/lib/eal/version.map
index d49e30bd042f..cfbade9a33e9 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -422,6 +422,7 @@ EXPERIMENTAL {
 	rte_intr_type_set;
 
 	# added in 22.07
+	rte_drand;
 	rte_thread_get_affinity_by_id;
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 2/3] rte_pie: remove unnecessary floating point
  2022-05-25 20:31 ` [PATCH v4 0/3] introduce random floating point function Stephen Hemminger
  2022-05-25 20:31   ` [PATCH v4 1/3] random: add rte_drand() function Stephen Hemminger
@ 2022-05-25 20:31   ` Stephen Hemminger
  2022-05-25 20:31   ` [PATCH v4 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
  2022-05-26  7:06   ` [PATCH v4 0/3] introduce random floating point function Morten Brørup
  3 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 20:31 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Cristian Dumitrescu, Jasvinder Singh,
	Wojciech Liguzinski

The qdelay variable is derived from and compared to 64 bit
value so it doesn't have to be floating point.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 02a987f54ad1..3e2c1ef46721 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -218,7 +218,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
 	uint64_t rand_value;
-	double qdelay = pie_cfg->qdelay_ref * 0.5;
+	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
 	if (((pie->qdelay_old < qdelay) && (pie->drop_prob < 0.2)) ||
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 3/3] rte_pie: fix incorrect floating point math
  2022-05-25 20:31 ` [PATCH v4 0/3] introduce random floating point function Stephen Hemminger
  2022-05-25 20:31   ` [PATCH v4 1/3] random: add rte_drand() function Stephen Hemminger
  2022-05-25 20:31   ` [PATCH v4 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
@ 2022-05-25 20:31   ` Stephen Hemminger
  2022-05-26  7:06   ` [PATCH v4 0/3] introduce random floating point function Morten Brørup
  3 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-25 20:31 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Cristian Dumitrescu, Jasvinder Singh,
	Wojciech Liguzinski

The function rte_pie_drop was attempting to do a random probability
drop, but because of incorrect usage of fixed point divide
it would always return 1.

Change to use new rte_drand() instead.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 3e2c1ef46721..528f2ea878e8 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -217,7 +217,6 @@ __rte_experimental
 _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
-	uint64_t rand_value;
 	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
@@ -240,9 +239,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	if (pie->accu_prob >= 8.5)
 		return 1;
 
-	rand_value = rte_rand()/RTE_RAND_MAX;
-
-	if ((double)rand_value < pie->drop_prob) {
+	if (rte_drand() < pie->drop_prob) {
 		pie->accu_prob = 0;
 		return 1;
 	}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 0/3] introduce random floating point function
  2022-05-25 20:31 ` [PATCH v4 0/3] introduce random floating point function Stephen Hemminger
                     ` (2 preceding siblings ...)
  2022-05-25 20:31   ` [PATCH v4 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
@ 2022-05-26  7:06   ` Morten Brørup
  3 siblings, 0 replies; 36+ messages in thread
From: Morten Brørup @ 2022-05-26  7:06 UTC (permalink / raw)
  To: Stephen Hemminger, dev

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 25 May 2022 22.31
> 
> Introduce a new random number function to get a floating
> point value. Then use it to fix PIE scheduler.
> 
> v4
>    - use slower divide method in rte_drand() if library
>      does not have ieee754 (FreeBSD and Windows?)
>    - fix some typos
> 
> v3
>    - rename to rte_drand() and other review feedback
>    - fix warnings with some compilers in test code
> 
> Stephen Hemminger (3):
>   random: add rte_drand() function
>   rte_pie: remove unnecessary floating point
>   rte_pie: fix incorrect floating point math
> 
>  app/test/test_rand_perf.c              |  7 +++++
>  doc/guides/rel_notes/release_22_07.rst |  5 ++++
>  lib/eal/common/rte_random.c            | 41 ++++++++++++++++++++++++++
>  lib/eal/include/rte_random.h           | 18 +++++++++++
>  lib/eal/meson.build                    |  3 ++
>  lib/eal/version.map                    |  1 +
>  lib/sched/rte_pie.h                    |  7 ++---
>  7 files changed, 77 insertions(+), 5 deletions(-)
> 
> --
> 2.35.1
> 

Series-Acked-by: Morten Brørup <mb@smartsharesystems.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 1/3] random: add rte_drand() function
  2022-05-25 20:31   ` [PATCH v4 1/3] random: add rte_drand() function Stephen Hemminger
@ 2022-05-26  9:56     ` Ray Kinsella
  2022-05-26 13:20     ` Mattias Rönnblom
  1 sibling, 0 replies; 36+ messages in thread
From: Ray Kinsella @ 2022-05-26  9:56 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Mattias Rönnblom


Stephen Hemminger <stephen@networkplumber.org> writes:

> The PIE code and other applications can benefit from having a
> fast way to get a random floating point value. This new function
> is equivalent to drand() in the standard library.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  app/test/test_rand_perf.c              |  7 +++++
>  doc/guides/rel_notes/release_22_07.rst |  5 ++++
>  lib/eal/common/rte_random.c            | 41 ++++++++++++++++++++++++++
>  lib/eal/include/rte_random.h           | 18 +++++++++++
>  lib/eal/meson.build                    |  3 ++
>  lib/eal/version.map                    |  1 +
>  6 files changed, 75 insertions(+)
>
Acked-by: Ray Kinsella <mdr@ashoe.eu>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 1/3] random: add rte_drand() function
  2022-05-25 20:31   ` [PATCH v4 1/3] random: add rte_drand() function Stephen Hemminger
  2022-05-26  9:56     ` Ray Kinsella
@ 2022-05-26 13:20     ` Mattias Rönnblom
  2022-05-26 15:25       ` Stephen Hemminger
                         ` (2 more replies)
  1 sibling, 3 replies; 36+ messages in thread
From: Mattias Rönnblom @ 2022-05-26 13:20 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Mattias Rönnblom, Ray Kinsella

On 2022-05-25 22:31, Stephen Hemminger wrote:
> The PIE code and other applications can benefit from having a
> fast way to get a random floating point value. This new function
> is equivalent to drand() in the standard library.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>   app/test/test_rand_perf.c              |  7 +++++
>   doc/guides/rel_notes/release_22_07.rst |  5 ++++
>   lib/eal/common/rte_random.c            | 41 ++++++++++++++++++++++++++
>   lib/eal/include/rte_random.h           | 18 +++++++++++
>   lib/eal/meson.build                    |  3 ++
>   lib/eal/version.map                    |  1 +
>   6 files changed, 75 insertions(+)
> 
> diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
> index fe797ebfa1ca..26fb1d9a586e 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_float,
>   	rand_type_bounded_best_case,
>   	rand_type_bounded_worst_case
>   };
> @@ -30,6 +31,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_float:
> +		return "Floating point [rte_drand()]";
>   	case rand_type_bounded_best_case:
>   		return "Bounded average best-case [rte_rand_max()]";
>   	case rand_type_bounded_worst_case:
> @@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
>   		case rand_type_64:
>   			sum += rte_rand();
>   			break;
> +		case rand_type_float:
> +			sum += 1000. * rte_drand();

Including this floating point multiplication will lead to an 
overestimation of rte_drand() latency.

You could refactor this function to be a macro, and pass the return type 
to as a parameter to this macro. I did just that, and on both an AMD 
5900X and a Cortex-A72 it didn't add more than ~5%, so I don't think 
it's necessary.

> +			break;
>   		case rand_type_bounded_best_case:
>   			sum += rte_rand_max(BEST_CASE_BOUND);
>   			break;
> @@ -83,6 +89,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_float);
>   	test_rand_perf_type(rand_type_bounded_best_case);
>   	test_rand_perf_type(rand_type_bounded_worst_case);
>   
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index e49cacecefd4..b131ea577226 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -104,6 +104,11 @@ New Features
>     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
>     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
>   
> +* ** Added function get random floating point number.**
> +
> +  Added the function ``rte_drand()`` to provide a pseudo-random
> +  floating point number.
> +
>   
>   Removed Items
>   -------------
> diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
> index 4535cc980cec..3dc3484ee655 100644
> --- a/lib/eal/common/rte_random.c
> +++ b/lib/eal/common/rte_random.c
> @@ -6,6 +6,9 @@
>   #include <x86intrin.h>
>   #endif
>   #include <unistd.h>
> +#ifdef RTE_LIBEAL_USE_IEEE754
> +#include <ieee754.h>
> +#endif
>   
>   #include <rte_branch_prediction.h>
>   #include <rte_cycles.h>
> @@ -173,6 +176,44 @@ rte_rand_max(uint64_t upper_bound)
>   	return res;
>   }
>   
> +double
> +rte_drand(void)
> +{
> +	struct rte_rand_state *state = __rte_rand_get_state();
> +	uint64_t rand64 = __rte_rand_lfsr258(state);
> +#ifdef RTE_LIBEAL_USE_IEEE754
> +	union ieee754_double u = {
> +		.ieee = {
> +			.negative = 0,
> +			.exponent = IEEE754_DOUBLE_BIAS,
> +		},
> +	};
> +
> +	/* Take 64 bit random value and put it into the mantissa
> +	 * This uses direct access to IEEE format to avoid doing
> +	 * any direct floating point math here.
> +	 */
> +	u.ieee.mantissa0 = rand64 >> 32;
> +	u.ieee.mantissa1 = rand64;
> +
> +	return u.d - 1.0;
> +#else
> +	/* Slower method requiring floating point divide
> +	 *

Do you know how much slower? I ran rand_perf_test on two of my systems.

                       AMD 5900X     Pi4 (ARM Cortex-A72)
IEEE754 version          12              1.19
Non-IEEE754 version      11              1.16
Naive version*           24              1.16

* (double)rte_rand() / (double)UINT64_MAX

Numbers are TSC cycles/op.

Surprisingly, it seems like the IEEE754 version is slower on both of 
these machines.

Do you have a machine (or a different use case) where the supposedly 
more optimized version actually runs faster?

> +	 * The double mantissa only has 53 bits, so we uniformly mask off the
> +	 * high 11 bits and then floating-point divide by 2^53 to achieve a
> +	 * result in [0, 1).
> +	 *
> +	 * We are not allowed to emit 1.0, so denom must be one greater than
> +	 * the possible range of the preceeding step.
> +	 */
> +	static const uint64_t denom = (uint64_t)1 << 53;

Remove "static const". Surely, this can't make a difference (at least 
not in a positive direction).

> +
> +	rand64 &= denom - 1;
> +	return (double)rand64 / denom;
> +#endif
> +}
> +
>   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 29f5f1325a30..f6541c2b0f08 100644
> --- a/lib/eal/include/rte_random.h
> +++ b/lib/eal/include/rte_random.h
> @@ -65,6 +65,24 @@ rte_rand(void);
>   uint64_t
>   rte_rand_max(uint64_t upper_bound);
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Generates a pseudo-random floating point number.
> + *
> + * This function returns a nonnegative double-precision floating random
> + * number uniformly distributed over the interval [0.0, 1.0).
> + *
> + * The generator is not cryptographically secure.
> + * If called from lcore threads, this function is thread-safe.
> + *
> + * @return
> + *   A pseudo-random value between 0 and 1.0.
> + */
> +__rte_experimental
> +double rte_drand(void);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/eal/meson.build b/lib/eal/meson.build
> index 056beb946119..e50524901c98 100644
> --- a/lib/eal/meson.build
> +++ b/lib/eal/meson.build
> @@ -32,3 +32,6 @@ endif
>   if cc.has_function('getentropy', prefix : '#include <unistd.h>')
>       cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
>   endif
> +if cc.has_header_symbol('ieee754.h', 'union ieee754_double')
> +    cflags += '-DRTE_LIBEAL_USE_IEEE754'
> +endif
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index d49e30bd042f..cfbade9a33e9 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -422,6 +422,7 @@ EXPERIMENTAL {
>   	rte_intr_type_set;
>   
>   	# added in 22.07
> +	rte_drand;
>   	rte_thread_get_affinity_by_id;
>   	rte_thread_self;
>   	rte_thread_set_affinity_by_id;

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 1/3] random: add rte_drand() function
  2022-05-26 13:20     ` Mattias Rönnblom
@ 2022-05-26 15:25       ` Stephen Hemminger
  2022-05-26 15:28       ` Stephen Hemminger
  2022-05-26 20:19       ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-26 15:25 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev, Mattias Rönnblom, Ray Kinsella

On Thu, 26 May 2022 15:20:29 +0200
Mattias Rönnblom <hofors@lysator.liu.se> wrote:

> > @@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
> >   		case rand_type_64:
> >   			sum += rte_rand();
> >   			break;
> > +		case rand_type_float:
> > +			sum += 1000. * rte_drand();  
> 
> Including this floating point multiplication will lead to an 
> overestimation of rte_drand() latency.
> 
> You could refactor this function to be a macro, and pass the return type 
> to as a parameter to this macro. I did just that, and on both an AMD 
> 5900X and a Cortex-A72 it didn't add more than ~5%, so I don't think 
> it's necessary.

The test is not doing anything useful with the result.
It is just a way to exercise the code.

Macros are evil, have little or no typechecking and should be avoided.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 1/3] random: add rte_drand() function
  2022-05-26 13:20     ` Mattias Rönnblom
  2022-05-26 15:25       ` Stephen Hemminger
@ 2022-05-26 15:28       ` Stephen Hemminger
  2022-05-26 20:19       ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-26 15:28 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev, Mattias Rönnblom, Ray Kinsella

On Thu, 26 May 2022 15:20:29 +0200
Mattias Rönnblom <hofors@lysator.liu.se> wrote:

> > +#else
> > +	/* Slower method requiring floating point divide
> > +	 *  
> 
> Do you know how much slower? I ran rand_perf_test on two of my systems.
> 
>                        AMD 5900X     Pi4 (ARM Cortex-A72)
> IEEE754 version          12              1.19
> Non-IEEE754 version      11              1.16
> Naive version*           24              1.16
> 
> * (double)rte_rand() / (double)UINT64_MAX
> 
> Numbers are TSC cycles/op.
> 
> Surprisingly, it seems like the IEEE754 version is slower on both of 
> these machines.
> 
> Do you have a machine (or a different use case) where the supposedly 
> more optimized version actually runs faster?

The direct method is based off the concept used by glibc and others
and the divide (including spelling error) are from FreeBSD.

Be careful with micro benchmarks. A better one would be do
rte_drand() compared with something to check whether it is in range.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 1/3] random: add rte_drand() function
  2022-05-26 13:20     ` Mattias Rönnblom
  2022-05-26 15:25       ` Stephen Hemminger
  2022-05-26 15:28       ` Stephen Hemminger
@ 2022-05-26 20:19       ` Stephen Hemminger
  2 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-26 20:19 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev, Mattias Rönnblom, Ray Kinsella

On Thu, 26 May 2022 15:20:29 +0200
Mattias Rönnblom <hofors@lysator.liu.se> wrote:

> On 2022-05-25 22:31, Stephen Hemminger wrote:
> > The PIE code and other applications can benefit from having a
> > fast way to get a random floating point value. This new function
> > is equivalent to drand() in the standard library.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >   app/test/test_rand_perf.c              |  7 +++++
> >   doc/guides/rel_notes/release_22_07.rst |  5 ++++
> >   lib/eal/common/rte_random.c            | 41 ++++++++++++++++++++++++++
> >   lib/eal/include/rte_random.h           | 18 +++++++++++
> >   lib/eal/meson.build                    |  3 ++
> >   lib/eal/version.map                    |  1 +
> >   6 files changed, 75 insertions(+)
> > 
> > diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
> > index fe797ebfa1ca..26fb1d9a586e 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_float,
> >   	rand_type_bounded_best_case,
> >   	rand_type_bounded_worst_case
> >   };
> > @@ -30,6 +31,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_float:
> > +		return "Floating point [rte_drand()]";
> >   	case rand_type_bounded_best_case:
> >   		return "Bounded average best-case [rte_rand_max()]";
> >   	case rand_type_bounded_worst_case:
> > @@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
> >   		case rand_type_64:
> >   			sum += rte_rand();
> >   			break;
> > +		case rand_type_float:
> > +			sum += 1000. * rte_drand();  
> 
> Including this floating point multiplication will lead to an 
> overestimation of rte_drand() latency.
> 
> You could refactor this function to be a macro, and pass the return type 
> to as a parameter to this macro. I did just that, and on both an AMD 
> 5900X and a Cortex-A72 it didn't add more than ~5%, so I don't think 
> it's necessary.
> 
> > +			break;
> >   		case rand_type_bounded_best_case:
> >   			sum += rte_rand_max(BEST_CASE_BOUND);
> >   			break;
> > @@ -83,6 +89,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_float);
> >   	test_rand_perf_type(rand_type_bounded_best_case);
> >   	test_rand_perf_type(rand_type_bounded_worst_case);
> >   
> > diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> > index e49cacecefd4..b131ea577226 100644
> > --- a/doc/guides/rel_notes/release_22_07.rst
> > +++ b/doc/guides/rel_notes/release_22_07.rst
> > @@ -104,6 +104,11 @@ New Features
> >     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
> >     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
> >   
> > +* ** Added function get random floating point number.**
> > +
> > +  Added the function ``rte_drand()`` to provide a pseudo-random
> > +  floating point number.
> > +
> >   
> >   Removed Items
> >   -------------
> > diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
> > index 4535cc980cec..3dc3484ee655 100644
> > --- a/lib/eal/common/rte_random.c
> > +++ b/lib/eal/common/rte_random.c
> > @@ -6,6 +6,9 @@
> >   #include <x86intrin.h>
> >   #endif
> >   #include <unistd.h>
> > +#ifdef RTE_LIBEAL_USE_IEEE754
> > +#include <ieee754.h>
> > +#endif
> >   
> >   #include <rte_branch_prediction.h>
> >   #include <rte_cycles.h>
> > @@ -173,6 +176,44 @@ rte_rand_max(uint64_t upper_bound)
> >   	return res;
> >   }
> >   
> > +double
> > +rte_drand(void)
> > +{
> > +	struct rte_rand_state *state = __rte_rand_get_state();
> > +	uint64_t rand64 = __rte_rand_lfsr258(state);
> > +#ifdef RTE_LIBEAL_USE_IEEE754
> > +	union ieee754_double u = {
> > +		.ieee = {
> > +			.negative = 0,
> > +			.exponent = IEEE754_DOUBLE_BIAS,
> > +		},
> > +	};
> > +
> > +	/* Take 64 bit random value and put it into the mantissa
> > +	 * This uses direct access to IEEE format to avoid doing
> > +	 * any direct floating point math here.
> > +	 */
> > +	u.ieee.mantissa0 = rand64 >> 32;
> > +	u.ieee.mantissa1 = rand64;
> > +
> > +	return u.d - 1.0;
> > +#else
> > +	/* Slower method requiring floating point divide
> > +	 *  
> 
> Do you know how much slower? I ran rand_perf_test on two of my systems.
> 
>                        AMD 5900X     Pi4 (ARM Cortex-A72)
> IEEE754 version          12              1.19
> Non-IEEE754 version      11              1.16
> Naive version*           24              1.16
> 
> * (double)rte_rand() / (double)UINT64_MAX
> 
> Numbers are TSC cycles/op.

On AMD Ryzen 7 both versions take 9 cycles/op with the rand_perf_autotest
So it is a toss up.

The 754 version is:

        ubfx    r1, r1, #0, #20
        orr     r3, r1, #1069547520   << mantissa0
        mov     r2, r0
        orr     r3, r3, #3145728
        vmov.f64        d0, #1.0e+0
        vmov    d16, r2, r3
        vsub.f64        d0, d16, d0   << return u.d - 1.0

Note: the compiler is doing smart optimization on the divide version.
It knows that since denominator is fixed value it can use multiply.

        vmov    d16, r0, r1
        vmul.f64        d0, d16, d0

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v5 0/3] introduce random floating point function
  2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
                   ` (5 preceding siblings ...)
  2022-05-25 20:31 ` [PATCH v4 0/3] introduce random floating point function Stephen Hemminger
@ 2022-05-26 20:26 ` Stephen Hemminger
  2022-05-26 20:26   ` [PATCH v5 1/3] random: add rte_drand() function Stephen Hemminger
                     ` (3 more replies)
  6 siblings, 4 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-26 20:26 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Morten Brørup

Introduce a new random number function to get a floating
point value. Then use it to fix PIE scheduler.

v5
   - use divide method (similar to FreeBSD)
     it takes same number of cycles (after optimization)
     and works on all platforms.

Stephen Hemminger (3):
  random: add rte_drand() function
  rte_pie: remove unnecessary floating point
  rte_pie: fix incorrect floating point math

Stephen Hemminger (3):
  random: add rte_drand() function
  rte_pie: remove unnecessary floating point
  rte_pie: fix incorrect floating point math

 app/test/test_rand_perf.c              |  7 +++++++
 doc/guides/rel_notes/release_22_07.rst |  5 +++++
 lib/eal/common/rte_random.c            | 19 +++++++++++++++++++
 lib/eal/include/rte_random.h           | 18 ++++++++++++++++++
 lib/eal/version.map                    |  1 +
 lib/sched/rte_pie.h                    |  7 ++-----
 6 files changed, 52 insertions(+), 5 deletions(-)

Series-Acked-by: Morten Brørup <mb@smartsharesystems.com>

-- 
2.35.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v5 1/3] random: add rte_drand() function
  2022-05-26 20:26 ` [PATCH v5 " Stephen Hemminger
@ 2022-05-26 20:26   ` Stephen Hemminger
  2022-05-26 20:26   ` [PATCH v5 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-26 20:26 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Ray Kinsella, Mattias Rönnblom, Ray Kinsella

The PIE code and other applications can benefit from having a
fast way to get a random floating point value. This new function
is equivalent to drand() in the standard library.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Ray Kinsella <mdr@ashoe.eu>
---
 app/test/test_rand_perf.c              |  7 +++++++
 doc/guides/rel_notes/release_22_07.rst |  5 +++++
 lib/eal/common/rte_random.c            | 19 +++++++++++++++++++
 lib/eal/include/rte_random.h           | 18 ++++++++++++++++++
 lib/eal/version.map                    |  1 +
 5 files changed, 50 insertions(+)

diff --git a/app/test/test_rand_perf.c b/app/test/test_rand_perf.c
index fe797ebfa1ca..26fb1d9a586e 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_float,
 	rand_type_bounded_best_case,
 	rand_type_bounded_worst_case
 };
@@ -30,6 +31,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_float:
+		return "Floating point [rte_drand()]";
 	case rand_type_bounded_best_case:
 		return "Bounded average best-case [rte_rand_max()]";
 	case rand_type_bounded_worst_case:
@@ -55,6 +58,9 @@ test_rand_perf_type(enum rand_type rand_type)
 		case rand_type_64:
 			sum += rte_rand();
 			break;
+		case rand_type_float:
+			sum += 1000. * rte_drand();
+			break;
 		case rand_type_bounded_best_case:
 			sum += rte_rand_max(BEST_CASE_BOUND);
 			break;
@@ -83,6 +89,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_float);
 	test_rand_perf_type(rand_type_bounded_best_case);
 	test_rand_perf_type(rand_type_bounded_worst_case);
 
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index e49cacecefd4..b131ea577226 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -104,6 +104,11 @@ New Features
   * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
   * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
 
+* ** Added function get random floating point number.**
+
+  Added the function ``rte_drand()`` to provide a pseudo-random
+  floating point number.
+
 
 Removed Items
 -------------
diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 4535cc980cec..166b0d8921c9 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -173,6 +173,25 @@ rte_rand_max(uint64_t upper_bound)
 	return res;
 }
 
+double
+rte_drand(void)
+{
+	static const uint64_t denom = (uint64_t)1 << 53;
+	uint64_t rand64 = rte_rand();
+
+	/*
+	 * The double mantissa only has 53 bits, so we uniformly mask off the
+	 * high 11 bits and then floating-point divide by 2^53 to achieve a
+	 * result in [0, 1).
+	 *
+	 * We are not allowed to emit 1.0, so denom must be one greater than
+	 * the possible range of the preceding step.
+	 */
+
+	rand64 &= denom - 1;
+	return (double)rand64 / denom;
+}
+
 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 29f5f1325a30..f6541c2b0f08 100644
--- a/lib/eal/include/rte_random.h
+++ b/lib/eal/include/rte_random.h
@@ -65,6 +65,24 @@ rte_rand(void);
 uint64_t
 rte_rand_max(uint64_t upper_bound);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Generates a pseudo-random floating point number.
+ *
+ * This function returns a nonnegative double-precision floating random
+ * number uniformly distributed over the interval [0.0, 1.0).
+ *
+ * The generator is not cryptographically secure.
+ * If called from lcore threads, this function is thread-safe.
+ *
+ * @return
+ *   A pseudo-random value between 0 and 1.0.
+ */
+__rte_experimental
+double rte_drand(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/eal/version.map b/lib/eal/version.map
index d49e30bd042f..cfbade9a33e9 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -422,6 +422,7 @@ EXPERIMENTAL {
 	rte_intr_type_set;
 
 	# added in 22.07
+	rte_drand;
 	rte_thread_get_affinity_by_id;
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 2/3] rte_pie: remove unnecessary floating point
  2022-05-26 20:26 ` [PATCH v5 " Stephen Hemminger
  2022-05-26 20:26   ` [PATCH v5 1/3] random: add rte_drand() function Stephen Hemminger
@ 2022-05-26 20:26   ` Stephen Hemminger
  2022-05-30 11:50     ` Dumitrescu, Cristian
  2022-06-21  8:18     ` Singh, Jasvinder
  2022-05-26 20:26   ` [PATCH v5 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
  2022-06-22  9:21   ` [PATCH v5 0/3] introduce random floating point function Thomas Monjalon
  3 siblings, 2 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-26 20:26 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Cristian Dumitrescu, Jasvinder Singh,
	Wojciech Liguzinski

The qdelay variable is derived from and compared to 64 bit
value so it doesn't have to be floating point.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 02a987f54ad1..3e2c1ef46721 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -218,7 +218,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
 	uint64_t rand_value;
-	double qdelay = pie_cfg->qdelay_ref * 0.5;
+	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
 	if (((pie->qdelay_old < qdelay) && (pie->drop_prob < 0.2)) ||
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 3/3] rte_pie: fix incorrect floating point math
  2022-05-26 20:26 ` [PATCH v5 " Stephen Hemminger
  2022-05-26 20:26   ` [PATCH v5 1/3] random: add rte_drand() function Stephen Hemminger
  2022-05-26 20:26   ` [PATCH v5 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
@ 2022-05-26 20:26   ` Stephen Hemminger
  2022-05-30 11:50     ` Dumitrescu, Cristian
  2022-06-21  8:18     ` Singh, Jasvinder
  2022-06-22  9:21   ` [PATCH v5 0/3] introduce random floating point function Thomas Monjalon
  3 siblings, 2 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-05-26 20:26 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Cristian Dumitrescu, Jasvinder Singh,
	Wojciech Liguzinski

The function rte_pie_drop was attempting to do a random probability
drop, but because of incorrect usage of fixed point divide
it would always return 1.

Change to use new rte_drand() instead.

Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/sched/rte_pie.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
index 3e2c1ef46721..528f2ea878e8 100644
--- a/lib/sched/rte_pie.h
+++ b/lib/sched/rte_pie.h
@@ -217,7 +217,6 @@ __rte_experimental
 _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	struct rte_pie *pie)
 {
-	uint64_t rand_value;
 	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
 
 	/* PIE is active but the queue is not congested: return 0 */
@@ -240,9 +239,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
 	if (pie->accu_prob >= 8.5)
 		return 1;
 
-	rand_value = rte_rand()/RTE_RAND_MAX;
-
-	if ((double)rand_value < pie->drop_prob) {
+	if (rte_drand() < pie->drop_prob) {
 		pie->accu_prob = 0;
 		return 1;
 	}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 2/3] rte_pie: remove unnecessary floating point
  2022-05-26 20:26   ` [PATCH v5 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
@ 2022-05-30 11:50     ` Dumitrescu, Cristian
  2022-06-21  8:18     ` Singh, Jasvinder
  1 sibling, 0 replies; 36+ messages in thread
From: Dumitrescu, Cristian @ 2022-05-30 11:50 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Singh, Jasvinder, Wojciech Liguzinski



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, May 26, 2022 9:27 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Wojciech Liguzinski
> <wojciechx.liguzinski@intel.com>
> Subject: [PATCH v5 2/3] rte_pie: remove unnecessary floating point
> 
> The qdelay variable is derived from and compared to 64 bit
> value so it doesn't have to be floating point.
> 
> Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/sched/rte_pie.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
> index 02a987f54ad1..3e2c1ef46721 100644
> --- a/lib/sched/rte_pie.h
> +++ b/lib/sched/rte_pie.h
> @@ -218,7 +218,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
>  	struct rte_pie *pie)
>  {
>  	uint64_t rand_value;
> -	double qdelay = pie_cfg->qdelay_ref * 0.5;
> +	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
> 
>  	/* PIE is active but the queue is not congested: return 0 */
>  	if (((pie->qdelay_old < qdelay) && (pie->drop_prob < 0.2)) ||
> --
> 2.35.1

Hi Stephen,

Thanks for your proposed fix. It looks good to me, but since Jasvinder handled the PIE integration, I am going to defer this for his review once he comes back from vacation, just to make sure I am not missing anything here.

Regards,
Cristian

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 3/3] rte_pie: fix incorrect floating point math
  2022-05-26 20:26   ` [PATCH v5 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
@ 2022-05-30 11:50     ` Dumitrescu, Cristian
  2022-06-21  8:18     ` Singh, Jasvinder
  1 sibling, 0 replies; 36+ messages in thread
From: Dumitrescu, Cristian @ 2022-05-30 11:50 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Singh, Jasvinder, Wojciech Liguzinski



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, May 26, 2022 9:27 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Wojciech Liguzinski
> <wojciechx.liguzinski@intel.com>
> Subject: [PATCH v5 3/3] rte_pie: fix incorrect floating point math
> 
> The function rte_pie_drop was attempting to do a random probability
> drop, but because of incorrect usage of fixed point divide
> it would always return 1.
> 
> Change to use new rte_drand() instead.
> 
> Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/sched/rte_pie.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h
> index 3e2c1ef46721..528f2ea878e8 100644
> --- a/lib/sched/rte_pie.h
> +++ b/lib/sched/rte_pie.h
> @@ -217,7 +217,6 @@ __rte_experimental
>  _rte_pie_drop(const struct rte_pie_config *pie_cfg,
>  	struct rte_pie *pie)
>  {
> -	uint64_t rand_value;
>  	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
> 
>  	/* PIE is active but the queue is not congested: return 0 */
> @@ -240,9 +239,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
>  	if (pie->accu_prob >= 8.5)
>  		return 1;
> 
> -	rand_value = rte_rand()/RTE_RAND_MAX;
> -
> -	if ((double)rand_value < pie->drop_prob) {
> +	if (rte_drand() < pie->drop_prob) {
>  		pie->accu_prob = 0;
>  		return 1;
>  	}
> --
> 2.35.1

Hi Stephen,

Thanks for your proposed fix. It looks good to me, but since Jasvinder handled the PIE integration, I am going to defer this for his review once he comes back from vacation, just to make sure I am not missing anything here.

Regards,
Cristian

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 2/3] rte_pie: remove unnecessary floating point
  2022-05-26 20:26   ` [PATCH v5 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
  2022-05-30 11:50     ` Dumitrescu, Cristian
@ 2022-06-21  8:18     ` Singh, Jasvinder
  1 sibling, 0 replies; 36+ messages in thread
From: Singh, Jasvinder @ 2022-06-21  8:18 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Dumitrescu, Cristian, Wojciech Liguzinski



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, May 26, 2022 9:27 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Wojciech Liguzinski
> <wojciechx.liguzinski@intel.com>
> Subject: [PATCH v5 2/3] rte_pie: remove unnecessary floating point
> 
> The qdelay variable is derived from and compared to 64 bit value so it doesn't
> have to be floating point.
> 
> Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/sched/rte_pie.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h index
> 02a987f54ad1..3e2c1ef46721 100644
> --- a/lib/sched/rte_pie.h
> +++ b/lib/sched/rte_pie.h
> @@ -218,7 +218,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
>  	struct rte_pie *pie)
>  {
>  	uint64_t rand_value;
> -	double qdelay = pie_cfg->qdelay_ref * 0.5;
> +	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
> 
>  	/* PIE is active but the queue is not congested: return 0 */
>  	if (((pie->qdelay_old < qdelay) && (pie->drop_prob < 0.2)) ||
> --
> 2.35.1


Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 3/3] rte_pie: fix incorrect floating point math
  2022-05-26 20:26   ` [PATCH v5 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
  2022-05-30 11:50     ` Dumitrescu, Cristian
@ 2022-06-21  8:18     ` Singh, Jasvinder
  1 sibling, 0 replies; 36+ messages in thread
From: Singh, Jasvinder @ 2022-06-21  8:18 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Dumitrescu, Cristian, Wojciech Liguzinski



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, May 26, 2022 9:27 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Wojciech Liguzinski
> <wojciechx.liguzinski@intel.com>
> Subject: [PATCH v5 3/3] rte_pie: fix incorrect floating point math
> 
> The function rte_pie_drop was attempting to do a random probability drop,
> but because of incorrect usage of fixed point divide it would always return 1.
> 
> Change to use new rte_drand() instead.
> 
> Fixes: 44c730b0e379 ("sched: add PIE based congestion management")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/sched/rte_pie.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/lib/sched/rte_pie.h b/lib/sched/rte_pie.h index
> 3e2c1ef46721..528f2ea878e8 100644
> --- a/lib/sched/rte_pie.h
> +++ b/lib/sched/rte_pie.h
> @@ -217,7 +217,6 @@ __rte_experimental
>  _rte_pie_drop(const struct rte_pie_config *pie_cfg,
>  	struct rte_pie *pie)
>  {
> -	uint64_t rand_value;
>  	uint64_t qdelay = pie_cfg->qdelay_ref / 2;
> 
>  	/* PIE is active but the queue is not congested: return 0 */ @@ -
> 240,9 +239,7 @@ _rte_pie_drop(const struct rte_pie_config *pie_cfg,
>  	if (pie->accu_prob >= 8.5)
>  		return 1;
> 
> -	rand_value = rte_rand()/RTE_RAND_MAX;
> -
> -	if ((double)rand_value < pie->drop_prob) {
> +	if (rte_drand() < pie->drop_prob) {
>  		pie->accu_prob = 0;
>  		return 1;
>  	}
> --
> 2.35.1


Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 0/3] introduce random floating point function
  2022-05-26 20:26 ` [PATCH v5 " Stephen Hemminger
                     ` (2 preceding siblings ...)
  2022-05-26 20:26   ` [PATCH v5 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
@ 2022-06-22  9:21   ` Thomas Monjalon
  3 siblings, 0 replies; 36+ messages in thread
From: Thomas Monjalon @ 2022-06-22  9:21 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Morten Brørup

26/05/2022 22:26, Stephen Hemminger:
> Stephen Hemminger (3):
>   random: add rte_drand() function
>   rte_pie: remove unnecessary floating point
>   rte_pie: fix incorrect floating point math

Series applied, thanks.



^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2022-06-22  9:21 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-24 18:46 [RFT 0/2] pie: floating point fixes Stephen Hemminger
2022-05-24 18:46 ` [RFT 1/2] rte_pie: remove unnecessary floating point Stephen Hemminger
2022-05-24 18:46 ` [RFT 2/2] rte_pie: fix incorrect floating point math Stephen Hemminger
2022-05-24 19:31 ` [RFT 0/2] pie: floating point fixes Morten Brørup
2022-05-24 22:18 ` [RFT v2 0/3] pie: fix random number issues Stephen Hemminger
2022-05-24 22:18   ` [RFT v2 1/3] random: add rte_rand_float() Stephen Hemminger
2022-05-25 11:55     ` Ray Kinsella
2022-05-25 14:45     ` Mattias Rönnblom
2022-05-25 15:26       ` Morten Brørup
2022-05-25 15:45       ` Stephen Hemminger
2022-05-25 15:47       ` Stephen Hemminger
2022-05-24 22:18   ` [RFT v2 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
2022-05-24 22:18   ` [RFT v2 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
2022-05-25 17:12 ` [PATCH v3 0/3] introduce random floating point function Stephen Hemminger
2022-05-25 17:12   ` [PATCH v3 1/3] random: add rte_drand() funciton Stephen Hemminger
2022-05-25 17:12   ` [PATCH v3 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
2022-05-25 17:12   ` [PATCH v3 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
2022-05-25 20:31 ` [PATCH v4 0/3] introduce random floating point function Stephen Hemminger
2022-05-25 20:31   ` [PATCH v4 1/3] random: add rte_drand() function Stephen Hemminger
2022-05-26  9:56     ` Ray Kinsella
2022-05-26 13:20     ` Mattias Rönnblom
2022-05-26 15:25       ` Stephen Hemminger
2022-05-26 15:28       ` Stephen Hemminger
2022-05-26 20:19       ` Stephen Hemminger
2022-05-25 20:31   ` [PATCH v4 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
2022-05-25 20:31   ` [PATCH v4 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
2022-05-26  7:06   ` [PATCH v4 0/3] introduce random floating point function Morten Brørup
2022-05-26 20:26 ` [PATCH v5 " Stephen Hemminger
2022-05-26 20:26   ` [PATCH v5 1/3] random: add rte_drand() function Stephen Hemminger
2022-05-26 20:26   ` [PATCH v5 2/3] rte_pie: remove unnecessary floating point Stephen Hemminger
2022-05-30 11:50     ` Dumitrescu, Cristian
2022-06-21  8:18     ` Singh, Jasvinder
2022-05-26 20:26   ` [PATCH v5 3/3] rte_pie: fix incorrect floating point math Stephen Hemminger
2022-05-30 11:50     ` Dumitrescu, Cristian
2022-06-21  8:18     ` Singh, Jasvinder
2022-06-22  9:21   ` [PATCH v5 0/3] introduce random floating point function Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.