[PATCH] sched: make RED scaling configurable

[PATCH] sched: make RED scaling configurable

[alangordondewar]

From: Alan Dewar <alan.dewar@att.com>

The RED code stores the maximum threshold is a 32-bit integer as a
pseudo fixed-point floating number with 10 fractional bits.  Twelve
other bits are used to encode the filter weight, leaving just 10 bits
for the queue length.  This limits the maximum queue length supported
by RED queues as 1024 packets.

Move the "hard" definitions from red.h into config/common_base so that
RED scaling can be configured during build.

Signed-off-by: Alan Dewar <alan.dewar@att.com>
---
 config/common_base         | 2 ++
 lib/librte_sched/rte_red.h | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/config/common_base b/config/common_base
index 5e97a08..5501dfe 100644
--- a/config/common_base
+++ b/config/common_base
@@ -666,6 +666,8 @@ CONFIG_RTE_SCHED_COLLECT_STATS=n
 CONFIG_RTE_SCHED_SUBPORT_TC_OV=n
 CONFIG_RTE_SCHED_PORT_N_GRINDERS=8
 CONFIG_RTE_SCHED_VECTOR=n
+CONFIG_RTE_RED_SCALING=10
+CONFIG_RTE_RED_MAX_TH_MAX=1023
 
 #
 # Compile the distributor library
diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h
index ca12227..49d3379 100644
--- a/lib/librte_sched/rte_red.h
+++ b/lib/librte_sched/rte_red.h
@@ -51,10 +51,9 @@ extern "C" {
 #include <rte_debug.h>
 #include <rte_cycles.h>
 #include <rte_branch_prediction.h>
+#include <rte_config.h>
 
-#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-point */
 #define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied by number of leaf queues */
-#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold limit in fixed point format */
 #define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter weight value */
 #define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse filter weight value */
 #define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark probability value */
-- 
2.1.4

Re: [PATCH] sched: make RED scaling configurable

[Kantecki, Tomasz]

Hi Alan,

This looks like a very good idea but I think the patch must also cover adequate changes of test/test/test_red.c file.
What range of fraction/max_threshold values did you find to work correctly with no further code mods?

Thanks,
Tomasz

-----Original Message-----
From: alangordondewar@gmail.com [mailto:alangordondewar@gmail.com] 
Sent: Friday, September 1, 2017 2:42 PM
To: Kantecki, Tomasz <tomasz.kantecki@intel.com>
Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
Subject: [PATCH] sched: make RED scaling configurable

From: Alan Dewar <alan.dewar@att.com>

The RED code stores the maximum threshold is a 32-bit integer as a pseudo fixed-point floating number with 10 fractional bits.  Twelve other bits are used to encode the filter weight, leaving just 10 bits for the queue length.  This limits the maximum queue length supported by RED queues as 1024 packets.

Move the "hard" definitions from red.h into config/common_base so that RED scaling can be configured during build.

Signed-off-by: Alan Dewar <alan.dewar@att.com>
---
 config/common_base         | 2 ++
 lib/librte_sched/rte_red.h | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/config/common_base b/config/common_base index 5e97a08..5501dfe 100644
--- a/config/common_base
+++ b/config/common_base
@@ -666,6 +666,8 @@ CONFIG_RTE_SCHED_COLLECT_STATS=n  CONFIG_RTE_SCHED_SUBPORT_TC_OV=n
 CONFIG_RTE_SCHED_PORT_N_GRINDERS=8
 CONFIG_RTE_SCHED_VECTOR=n
+CONFIG_RTE_RED_SCALING=10
+CONFIG_RTE_RED_MAX_TH_MAX=1023
 
 #
 # Compile the distributor library
diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h index ca12227..49d3379 100644
--- a/lib/librte_sched/rte_red.h
+++ b/lib/librte_sched/rte_red.h
@@ -51,10 +51,9 @@ extern "C" {
 #include <rte_debug.h>
 #include <rte_cycles.h>
 #include <rte_branch_prediction.h>
+#include <rte_config.h>
 
-#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-point */
 #define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied by number of leaf queues */
-#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold limit in fixed point format */
 #define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter weight value */
 #define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse filter weight value */
 #define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark probability value */
--
2.1.4

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[PATCH v2] sched: make RED scaling configurable

[alangordondewar]

From: Alan Dewar <alan.dewar@att.com>

The RED code stores the maximum threshold is a 32-bit integer as a
pseudo fixed-point floating number with 10 fractional bits.  Twelve
other bits are used to encode the filter weight, leaving just 10 bits
for the queue length.  This limits the maximum queue length supported
by RED queues as 1024 packets.

Move the "hard" definitions from red.h into config/common_base so that
RED scaling can be configured during build.

Modified the RED unit-tests to use the new "soft" definition of
maximum-threshold from config/common_base in tests where it previously
used a hard coded limit of 1023.

The RED unit-tests all successfully pass when the maximum-threshold is
configured as 8191 and the RED scaling factor is dropped to seven.

Real-world testing has involved RED queue lengths of 8192 with multiple
different settings of the RED config parameters: min_th, max_th, wq_log2
and maxp_inv.

Signed-off-by: Alan Dewar <alan.dewar@att.com>
---
 config/common_base         |  2 ++
 lib/librte_sched/rte_red.h |  3 +--
 test/test/test_red.c       | 20 ++++++++++----------
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/config/common_base b/config/common_base
index 5e97a08..5501dfe 100644
--- a/config/common_base
+++ b/config/common_base
@@ -666,6 +666,8 @@ CONFIG_RTE_SCHED_COLLECT_STATS=n
 CONFIG_RTE_SCHED_SUBPORT_TC_OV=n
 CONFIG_RTE_SCHED_PORT_N_GRINDERS=8
 CONFIG_RTE_SCHED_VECTOR=n
+CONFIG_RTE_RED_SCALING=10
+CONFIG_RTE_RED_MAX_TH_MAX=1023
 
 #
 # Compile the distributor library
diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h
index ca12227..49d3379 100644
--- a/lib/librte_sched/rte_red.h
+++ b/lib/librte_sched/rte_red.h
@@ -51,10 +51,9 @@ extern "C" {
 #include <rte_debug.h>
 #include <rte_cycles.h>
 #include <rte_branch_prediction.h>
+#include <rte_config.h>
 
-#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-point */
 #define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied by number of leaf queues */
-#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold limit in fixed point format */
 #define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter weight value */
 #define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse filter weight value */
 #define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark probability value */
diff --git a/test/test/test_red.c b/test/test/test_red.c
index 348075d..70e8cfe 100644
--- a/test/test/test_red.c
+++ b/test/test/test_red.c
@@ -653,14 +653,14 @@ static enum test_result func_test2(struct test_config *tcfg)
 /**
  * Test F3: functional test 3
  */
-static uint32_t ft3_tlevel[] = {1022};
+static uint32_t ft3_tlevel[] = {RTE_RED_MAX_TH_MAX-1};
 
 static struct test_rte_red_config ft3_tconfig =  {
 	.rconfig = ft_wrconfig,
 	.num_cfg = RTE_DIM(ft_wrconfig),
 	.wq_log2 = ft_wq_log2,
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.maxp_inv = ft_maxp_inv,
 };
 
@@ -766,14 +766,14 @@ static enum test_result func_test3(struct test_config *tcfg)
 /**
  * Test F4: functional test 4
  */
-static uint32_t ft4_tlevel[] = {1022};
+static uint32_t ft4_tlevel[] = {RTE_RED_MAX_TH_MAX-1};
 static uint8_t ft4_wq_log2[] = {11};
 
 static struct test_rte_red_config ft4_tconfig =  {
 	.rconfig = ft_wrconfig,
 	.num_cfg = RTE_DIM(ft_wrconfig),
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.wq_log2 = ft4_wq_log2,
 	.maxp_inv = ft_maxp_inv,
 };
@@ -1048,7 +1048,7 @@ static enum test_result func_test5(struct test_config *tcfg)
 /**
  * Test F6: functional test 6
  */
-static uint32_t ft6_tlevel[] = {1022};
+static uint32_t ft6_tlevel[] = {RTE_RED_MAX_TH_MAX-1};
 static uint8_t ft6_wq_log2[] = {9, 8};
 static uint8_t ft6_maxp_inv[] = {10, 20};
 static struct rte_red_config ft6_config[2];
@@ -1059,7 +1059,7 @@ static struct test_rte_red_config ft6_tconfig =  {
 	.rconfig = ft6_config,
 	.num_cfg = RTE_DIM(ft6_config),
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.wq_log2 = ft6_wq_log2,
 	.maxp_inv = ft6_maxp_inv,
 };
@@ -1547,7 +1547,7 @@ static uint32_t ovfl_qconfig[] = {0, 0, 1, 1};
 static uint32_t ovfl_q[] ={0};
 static uint32_t ovfl_dropped[] ={0};
 static uint32_t ovfl_enqueued[] ={0};
-static uint32_t ovfl_tlevel[] = {1023};
+static uint32_t ovfl_tlevel[] = {RTE_RED_MAX_TH_MAX};
 static uint8_t ovfl_wq_log2[] = {12};
 
 static struct test_rte_red_config ovfl_tconfig =  {
@@ -1555,7 +1555,7 @@ static struct test_rte_red_config ovfl_tconfig =  {
 	.num_cfg = RTE_DIM(ovfl_wrconfig),
 	.wq_log2 = ovfl_wq_log2,
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.maxp_inv = ovfl_maxp_inv,
 };
 
@@ -1595,10 +1595,10 @@ static void ovfl_check_avg(uint32_t avg)
 }
 
 static struct test_config ovfl_test1_config = {
-	.ifname = "queue avergage overflow test interface",
+	.ifname = "queue average overflow test interface",
 	.msg = "overflow test 1 : use one RED configuration,\n"
 	"		  increase average queue size to target level,\n"
-	"		  check maximum number of bits requirte_red to represent avg_s\n\n",
+	"		  check maximum number of bits required to represent avg_s\n\n",
 	.htxt = "avg queue size  "
 	"wq_log2  "
 	"fraction bits  "
-- 
2.1.4

Re: [PATCH v2] sched: make RED scaling configurable

[Kantecki, Tomasz]

> -----Original Message-----
> From: alangordondewar@gmail.com [mailto:alangordondewar@gmail.com] 
> Sent: Wednesday, September 13, 2017 11:16 AM
> To: Kantecki, Tomasz <tomasz.kantecki@intel.com>
> Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
> Subject: [PATCH v2] sched: make RED scaling configurable
>
> From: Alan Dewar <alan.dewar@att.com>
>
> The RED code stores the maximum threshold is a 32-bit integer as a
> pseudo fixed-point floating number with 10 fractional bits.  Twelve
> other bits are used to encode the filter weight, leaving just 10 bits
> for the queue length.  This limits the maximum queue length supported
> by RED queues as 1024 packets.
>
> Move the "hard" definitions from red.h into config/common_base so that
> RED scaling can be configured during build.
>
> Modified the RED unit-tests to use the new "soft" definition of
> maximum-threshold from config/common_base in tests where it previously
> used a hard coded limit of 1023.
>
> The RED unit-tests all successfully pass when the maximum-threshold is
> configured as 8191 and the RED scaling factor is dropped to seven.
>
> Real-world testing has involved RED queue lengths of 8192 with multiple
> different settings of the RED config parameters: min_th, max_th, wq_log2
> and maxp_inv.
>
> Signed-off-by: Alan Dewar <alan.dewar@att.com>
> ---

Thanks! The patch looks good to me.
The only thing that potentially could be added is a brief description of the parameters in the common_base file.

Acked-by: Tomasz Kantecki <tomasz.kantecki@intel.com>

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[PATCH v3] sched: make RED scaling configurable

[alangordondewar]

From: Alan Dewar <alan.dewar@att.com>

The RED code stores the maximum threshold is a 32-bit integer as a
pseudo fixed-point floating number with 10 fractional bits.  Twelve
other bits are used to encode the filter weight, leaving just 10 bits
for the queue length.  This limits the maximum queue length supported
by RED queues as 1024 packets.

Move the "hard" definitions from red.h into config/common_base so that
RED scaling can be configured during build.

Modified the RED unit-tests to use the new "soft" definition of
maximum-threshold from config/common_base in tests where it previously
used a hard coded limit of 1023.

The RED unit-tests all successfully pass when the maximum-threshold is
configured as 8191 and the RED scaling factor is dropped to seven.

Real-world testing has involved RED queue lengths of 8192 with multiple
different settings of the RED config parameters: min_th, max_th, wq_log2
and maxp_inv.

Signed-off-by: Alan Dewar <alan.dewar@att.com>
Acked-by: Tomasz Kantecki <tomasz.kantecki@intel.com>
---
 config/common_base         |  7 +++++++
 lib/librte_sched/rte_red.h |  3 +--
 test/test/test_red.c       | 20 ++++++++++----------
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/config/common_base b/config/common_base
index 5e97a08..2626557 100644
--- a/config/common_base
+++ b/config/common_base
@@ -666,6 +666,13 @@ CONFIG_RTE_SCHED_COLLECT_STATS=n
 CONFIG_RTE_SCHED_SUBPORT_TC_OV=n
 CONFIG_RTE_SCHED_PORT_N_GRINDERS=8
 CONFIG_RTE_SCHED_VECTOR=n
+#
+# RTE_RED_SCALING - number of fractional bits used for RED's moving average
+# For every bit that RTE_RED_SCALING is reduced, the max-queue size can doubled
+# RTE_RED_MAX_TH_MAX = (max-queue size - 1), max-queue size must be power of two
+#
+CONFIG_RTE_RED_SCALING=10
+CONFIG_RTE_RED_MAX_TH_MAX=1023
 
 #
 # Compile the distributor library
diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h
index ca12227..49d3379 100644
--- a/lib/librte_sched/rte_red.h
+++ b/lib/librte_sched/rte_red.h
@@ -51,10 +51,9 @@ extern "C" {
 #include <rte_debug.h>
 #include <rte_cycles.h>
 #include <rte_branch_prediction.h>
+#include <rte_config.h>
 
-#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-point */
 #define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied by number of leaf queues */
-#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold limit in fixed point format */
 #define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter weight value */
 #define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse filter weight value */
 #define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark probability value */
diff --git a/test/test/test_red.c b/test/test/test_red.c
index 348075d..70e8cfe 100644
--- a/test/test/test_red.c
+++ b/test/test/test_red.c
@@ -653,14 +653,14 @@ static enum test_result func_test2(struct test_config *tcfg)
 /**
  * Test F3: functional test 3
  */
-static uint32_t ft3_tlevel[] = {1022};
+static uint32_t ft3_tlevel[] = {RTE_RED_MAX_TH_MAX-1};
 
 static struct test_rte_red_config ft3_tconfig =  {
 	.rconfig = ft_wrconfig,
 	.num_cfg = RTE_DIM(ft_wrconfig),
 	.wq_log2 = ft_wq_log2,
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.maxp_inv = ft_maxp_inv,
 };
 
@@ -766,14 +766,14 @@ static enum test_result func_test3(struct test_config *tcfg)
 /**
  * Test F4: functional test 4
  */
-static uint32_t ft4_tlevel[] = {1022};
+static uint32_t ft4_tlevel[] = {RTE_RED_MAX_TH_MAX-1};
 static uint8_t ft4_wq_log2[] = {11};
 
 static struct test_rte_red_config ft4_tconfig =  {
 	.rconfig = ft_wrconfig,
 	.num_cfg = RTE_DIM(ft_wrconfig),
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.wq_log2 = ft4_wq_log2,
 	.maxp_inv = ft_maxp_inv,
 };
@@ -1048,7 +1048,7 @@ static enum test_result func_test5(struct test_config *tcfg)
 /**
  * Test F6: functional test 6
  */
-static uint32_t ft6_tlevel[] = {1022};
+static uint32_t ft6_tlevel[] = {RTE_RED_MAX_TH_MAX-1};
 static uint8_t ft6_wq_log2[] = {9, 8};
 static uint8_t ft6_maxp_inv[] = {10, 20};
 static struct rte_red_config ft6_config[2];
@@ -1059,7 +1059,7 @@ static struct test_rte_red_config ft6_tconfig =  {
 	.rconfig = ft6_config,
 	.num_cfg = RTE_DIM(ft6_config),
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.wq_log2 = ft6_wq_log2,
 	.maxp_inv = ft6_maxp_inv,
 };
@@ -1547,7 +1547,7 @@ static uint32_t ovfl_qconfig[] = {0, 0, 1, 1};
 static uint32_t ovfl_q[] ={0};
 static uint32_t ovfl_dropped[] ={0};
 static uint32_t ovfl_enqueued[] ={0};
-static uint32_t ovfl_tlevel[] = {1023};
+static uint32_t ovfl_tlevel[] = {RTE_RED_MAX_TH_MAX};
 static uint8_t ovfl_wq_log2[] = {12};
 
 static struct test_rte_red_config ovfl_tconfig =  {
@@ -1555,7 +1555,7 @@ static struct test_rte_red_config ovfl_tconfig =  {
 	.num_cfg = RTE_DIM(ovfl_wrconfig),
 	.wq_log2 = ovfl_wq_log2,
 	.min_th = 32,
-	.max_th = 1023,
+	.max_th = RTE_RED_MAX_TH_MAX,
 	.maxp_inv = ovfl_maxp_inv,
 };
 
@@ -1595,10 +1595,10 @@ static void ovfl_check_avg(uint32_t avg)
 }
 
 static struct test_config ovfl_test1_config = {
-	.ifname = "queue avergage overflow test interface",
+	.ifname = "queue average overflow test interface",
 	.msg = "overflow test 1 : use one RED configuration,\n"
 	"		  increase average queue size to target level,\n"
-	"		  check maximum number of bits requirte_red to represent avg_s\n\n",
+	"		  check maximum number of bits required to represent avg_s\n\n",
 	.htxt = "avg queue size  "
 	"wq_log2  "
 	"fraction bits  "
-- 
2.1.4

Re: [PATCH v3] sched: make RED scaling configurable

[Dumitrescu, Cristian]

<snip>...
 
> diff --git a/config/common_base b/config/common_base
> index 5e97a08..2626557 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -666,6 +666,13 @@ CONFIG_RTE_SCHED_COLLECT_STATS=n
>  CONFIG_RTE_SCHED_SUBPORT_TC_OV=n
>  CONFIG_RTE_SCHED_PORT_N_GRINDERS=8
>  CONFIG_RTE_SCHED_VECTOR=n
> +#
> +# RTE_RED_SCALING - number of fractional bits used for RED's moving
> average
> +# For every bit that RTE_RED_SCALING is reduced, the max-queue size can
> doubled
> +# RTE_RED_MAX_TH_MAX = (max-queue size - 1), max-queue size must be
> power of two
> +#
> +CONFIG_RTE_RED_SCALING=10
> +CONFIG_RTE_RED_MAX_TH_MAX=1023
> 

Alan, Tomasz,

Thank you for your work!

We generally want to avoid build-time configuration in favour of run-time configuration. Can you please rework this as run-time configuration?

I suggest we add a new API function to configure these parameters globally for all the RED objects (as opposed to per RED object), with the default values as the current values when this function is not called.

So NACK for now.

Regards,
Cristian

Re: [PATCH v3] sched: make RED scaling configurable

[Dewar, Alan]

Hi Cristian,

No problem,  it doesn't sounds like it will be too difficult. 

Unfortunately I'm in the middle of another piece of work, so it will probably be a couple of weeks before I get around to it.

Regards
Alan

-----Original Message-----
From: Dumitrescu, Cristian [mailto:cristian.dumitrescu@intel.com] 
Sent: Monday, September 25, 2017 11:37 AM
To: alangordondewar@gmail.com; Kantecki, Tomasz <tomasz.kantecki@intel.com>
Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
Subject: RE: [dpdk-dev] [PATCH v3] sched: make RED scaling configurable

<snip>...
 
> diff --git a/config/common_base b/config/common_base index 
> 5e97a08..2626557 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -666,6 +666,13 @@ CONFIG_RTE_SCHED_COLLECT_STATS=n  
> CONFIG_RTE_SCHED_SUBPORT_TC_OV=n
>  CONFIG_RTE_SCHED_PORT_N_GRINDERS=8
>  CONFIG_RTE_SCHED_VECTOR=n
> +#
> +# RTE_RED_SCALING - number of fractional bits used for RED's moving
> average
> +# For every bit that RTE_RED_SCALING is reduced, the max-queue size 
> +can
> doubled
> +# RTE_RED_MAX_TH_MAX = (max-queue size - 1), max-queue size must be
> power of two
> +#
> +CONFIG_RTE_RED_SCALING=10
> +CONFIG_RTE_RED_MAX_TH_MAX=1023
> 

Alan, Tomasz,

Thank you for your work!

We generally want to avoid build-time configuration in favour of run-time configuration. Can you please rework this as run-time configuration?

I suggest we add a new API function to configure these parameters globally for all the RED objects (as opposed to per RED object), with the default values as the current values when this function is not called.

So NACK for now.

Regards,
Cristian

[PATCH v4] sched: make RED scaling configurable

[alangordondewar]

From: Alan Dewar <alan.dewar@att.com>

The RED code stores the weighted moving average in a 32-bit integer as
a pseudo fixed-point floating number with 10 fractional bits.  Twelve
other bits are used to encode the filter weight, leaving just 10 bits
for the queue length.  This limits the maximum queue length supported
by RED queues to 1024 packets.

Introduce a new API to allow the RED scaling factor to be configured
based upon maximum queue length.  If this API is not called, the RED
scaling factor remains at its default value.

Added some new RED scaling unit-tests to test with RED queue-lengths
up to 8192 packets long.

Signed-off-by: Alan Dewar <alan.dewar@att.com>
---
 lib/librte_sched/rte_red.c             |  53 ++++++-
 lib/librte_sched/rte_red.h             |  63 ++++++--
 lib/librte_sched/rte_sched_version.map |   6 +
 test/test/test_red.c                   | 274 ++++++++++++++++++++++++++++++++-
 4 files changed, 374 insertions(+), 22 deletions(-)

diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
index ade57d1..0dc8d28 100644
--- a/lib/librte_sched/rte_red.c
+++ b/lib/librte_sched/rte_red.c
@@ -43,6 +43,8 @@
 static int rte_red_init_done = 0;     /**< Flag to indicate that global initialisation is done */
 uint32_t rte_red_rand_val = 0;        /**< Random value cache */
 uint32_t rte_red_rand_seed = 0;       /**< Seed for random number generation */
+uint8_t rte_red_scaling = RTE_RED_SCALING_DEFAULT;
+uint16_t rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH - 1;
 
 /**
  * table[i] = log2(1-Wq) * Scale * -1
@@ -66,7 +68,7 @@ __rte_red_init_tables(void)
 	double scale = 0.0;
 	double table_size = 0.0;
 
-	scale = (double)(1 << RTE_RED_SCALING);
+	scale = (double)(1 << rte_red_scaling);
 	table_size = (double)(RTE_DIM(rte_red_pow2_frac_inv));
 
 	for (i = 0; i < RTE_DIM(rte_red_pow2_frac_inv); i++) {
@@ -119,7 +121,7 @@ rte_red_config_init(struct rte_red_config *red_cfg,
 	if (red_cfg == NULL) {
 		return -1;
 	}
-	if (max_th > RTE_RED_MAX_TH_MAX) {
+	if (max_th > rte_red_max_threshold) {
 		return -2;
 	}
 	if (min_th >= max_th) {
@@ -148,11 +150,52 @@ rte_red_config_init(struct rte_red_config *red_cfg,
 		rte_red_init_done = 1;
 	}
 
-	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 + RTE_RED_SCALING);
-	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 + RTE_RED_SCALING);
-	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) << RTE_RED_SCALING;
+	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 + rte_red_scaling);
+	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 + rte_red_scaling);
+	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
+		rte_red_scaling;
 	red_cfg->maxp_inv = maxp_inv;
 	red_cfg->wq_log2 = wq_log2;
 
 	return 0;
 }
+
+int
+rte_red_set_scaling(uint16_t max_red_queue_length)
+{
+	int8_t count;
+
+	if (rte_red_init_done)
+		/**
+		 * Can't change the scaling once the red table has been
+		 * computed.
+		 */
+		return -1;
+
+	if (max_red_queue_length < RTE_RED_MIN_QUEUE_LENGTH)
+		return -2;
+
+	if (max_red_queue_length > RTE_RED_MAX_QUEUE_LENGTH)
+		return -3;
+
+	if (!rte_is_power_of_2(max_red_queue_length))
+		return -4;
+
+	count = 0;
+	while (max_red_queue_length != 0) {
+		max_red_queue_length >>= 1;
+		count++;
+	}
+
+	rte_red_scaling -= count - RTE_RED_SCALING_DEFAULT;
+	rte_red_max_threshold = max_red_queue_length - 1;
+	return 0;
+}
+
+void
+rte_red_reset_scaling(void)
+{
+	rte_red_init_done = 0;
+	rte_red_scaling = RTE_RED_SCALING_DEFAULT;
+	rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH - 1;
+}
diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h
index ca12227..be1fb0f 100644
--- a/lib/librte_sched/rte_red.h
+++ b/lib/librte_sched/rte_red.h
@@ -52,14 +52,31 @@ extern "C" {
 #include <rte_cycles.h>
 #include <rte_branch_prediction.h>
 
-#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-point */
-#define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied by number of leaf queues */
-#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold limit in fixed point format */
-#define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter weight value */
-#define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse filter weight value */
-#define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark probability value */
-#define RTE_RED_MAXP_INV_MAX                255        /**< Max inverse mark probability value */
-#define RTE_RED_2POW16                      (1<<16)    /**< 2 power 16 */
+/**< Default fraction size for fixed-point */
+#define RTE_RED_SCALING_DEFAULT             10
+
+/**< Packet size multiplied by number of leaf queues */
+#define RTE_RED_S                           (1 << 22)
+
+/**< Minimum, default and maximum RED queue length */
+#define RTE_RED_MIN_QUEUE_LENGTH            64
+#define RTE_RED_DEFAULT_QUEUE_LENGTH        1024
+#define RTE_RED_MAX_QUEUE_LENGTH            8192
+
+/**< Min inverse filter weight value */
+#define RTE_RED_WQ_LOG2_MIN                 1
+
+/**< Max inverse filter weight value */
+#define RTE_RED_WQ_LOG2_MAX                 12
+
+/**< Min inverse mark probability value */
+#define RTE_RED_MAXP_INV_MIN                1
+
+/**< Max inverse mark probability value */
+#define RTE_RED_MAXP_INV_MAX                255
+
+/**< 2 power 16 */
+#define RTE_RED_2POW16                      (1<<16)
 #define RTE_RED_INT16_NBITS                 (sizeof(uint16_t) * CHAR_BIT)
 #define RTE_RED_WQ_LOG2_NUM                 (RTE_RED_WQ_LOG2_MAX - RTE_RED_WQ_LOG2_MIN + 1)
 
@@ -71,6 +88,8 @@ extern uint32_t rte_red_rand_val;
 extern uint32_t rte_red_rand_seed;
 extern uint16_t rte_red_log2_1_minus_Wq[RTE_RED_WQ_LOG2_NUM];
 extern uint16_t rte_red_pow2_frac_inv[16];
+extern uint8_t rte_red_scaling;
+extern uint16_t rte_red_max_threshold;
 
 /**
  * RED configuration parameters passed by user
@@ -137,6 +156,26 @@ rte_red_config_init(struct rte_red_config *red_cfg,
 	const uint16_t maxp_inv);
 
 /**
+ * @brief Configures the global setting for the RED scaling factor
+ *
+ * @param max_red_queue_length [in] must be a power of two
+ *
+ * @return Operation status
+ * @retval 0 success
+ * @retval !0 error
+ */
+int
+rte_red_set_scaling(uint16_t max_red_queue_length);
+
+/**
+ * @brief Reset the RED scaling factor - only for use by RED unit-tests
+ *
+ * @return Operation status
+ */
+void
+rte_red_reset_scaling(void);
+
+/**
  * @brief Generate random number for RED
  *
  * Implemenetation based on:
@@ -206,7 +245,7 @@ __rte_red_calc_qempty_factor(uint8_t wq_log2, uint16_t m)
 	f = (n >> 6) & 0xf;
 	n >>= 10;
 
-	if (n < RTE_RED_SCALING)
+	if (n < rte_red_scaling)
 		return (uint16_t) ((rte_red_pow2_frac_inv[f] + (1 << (n - 1))) >> n);
 
 	return 0;
@@ -258,7 +297,9 @@ rte_red_enqueue_empty(const struct rte_red_config *red_cfg,
 	if (m >= RTE_RED_2POW16) {
 		red->avg = 0;
 	} else {
-		red->avg = (red->avg >> RTE_RED_SCALING) * __rte_red_calc_qempty_factor(red_cfg->wq_log2, (uint16_t) m);
+		red->avg = (red->avg >> rte_red_scaling) *
+			__rte_red_calc_qempty_factor(red_cfg->wq_log2,
+						     (uint16_t) m);
 	}
 
 	return 0;
@@ -365,7 +406,7 @@ rte_red_enqueue_nonempty(const struct rte_red_config *red_cfg,
 	*/
 
 	/* avg update */
-	red->avg += (q << RTE_RED_SCALING) - (red->avg >> red_cfg->wq_log2);
+	red->avg += (q << rte_red_scaling) - (red->avg >> red_cfg->wq_log2);
 
 	/* avg < min_th: do not mark the packet  */
 	if (red->avg < red_cfg->min_th) {
diff --git a/lib/librte_sched/rte_sched_version.map b/lib/librte_sched/rte_sched_version.map
index 3aa159a..92e51f8 100644
--- a/lib/librte_sched/rte_sched_version.map
+++ b/lib/librte_sched/rte_sched_version.map
@@ -29,3 +29,9 @@ DPDK_2.1 {
 	rte_sched_port_pkt_read_color;
 
 } DPDK_2.0;
+
+DPDK_17.08 {
+	global;
+
+	rte_red_set_scaling;
+} DPDK_2.1;
diff --git a/test/test/test_red.c b/test/test/test_red.c
index 348075d..f2d50ef 100644
--- a/test/test/test_red.c
+++ b/test/test/test_red.c
@@ -67,6 +67,7 @@ struct test_rte_red_config {        /**< Test structure for RTE_RED config */
 	uint32_t min_th;                /**< Queue minimum threshold */
 	uint32_t max_th;                /**< Queue maximum threshold */
 	uint8_t *maxp_inv;              /**< Inverse mark probability */
+	uint16_t max_queue_len;         /**< Maximum queue length */
 };
 
 struct test_queue {                 /**< Test structure for RTE_RED Queues */
@@ -181,7 +182,7 @@ static uint32_t rte_red_get_avg_int(const struct rte_red_config *red_cfg,
 	/**
 	 * scale by 1/n and convert from fixed-point to integer
 	 */
-	return red->avg >> (RTE_RED_SCALING + red_cfg->wq_log2);
+	return red->avg >> (rte_red_scaling + red_cfg->wq_log2);
 }
 
 static double rte_red_get_avg_float(const struct rte_red_config *red_cfg,
@@ -190,7 +191,7 @@ static double rte_red_get_avg_float(const struct rte_red_config *red_cfg,
 	/**
 	 * scale by 1/n and convert from fixed-point to floating-point
 	 */
-	return ldexp((double)red->avg,  -(RTE_RED_SCALING + red_cfg->wq_log2));
+	return ldexp((double)red->avg,  -(rte_red_scaling + red_cfg->wq_log2));
 }
 
 static void rte_red_set_avg_int(const struct rte_red_config *red_cfg,
@@ -200,7 +201,7 @@ static void rte_red_set_avg_int(const struct rte_red_config *red_cfg,
 	/**
 	 * scale by n and convert from integer to fixed-point
 	 */
-	red->avg = avg << (RTE_RED_SCALING + red_cfg->wq_log2);
+	red->avg = avg << (rte_red_scaling + red_cfg->wq_log2);
 }
 
 static double calc_exp_avg_on_empty(double avg, uint32_t n, uint32_t time_diff)
@@ -280,10 +281,23 @@ static enum test_result
 test_rte_red_init(struct test_config *tcfg)
 {
 	unsigned i = 0;
+	int ret;
 
 	tcfg->tvar->clk_freq = rte_get_timer_hz();
 	init_port_ts( tcfg->tvar->clk_freq );
 
+	rte_red_reset_scaling();
+	ret = rte_red_set_scaling(tcfg->tconfig->max_queue_len);
+	if (ret) {
+		printf("rte_red_set_scaling failed: %d\n", ret);
+		return FAIL;
+	}
+	if (rte_red_max_threshold < tcfg->tconfig->max_th) {
+		printf("rte_red_max_th (%u) < tconfig->max_th (%u)\n",
+		       rte_red_max_threshold, tcfg->tconfig->max_th);
+		return FAIL;
+	}
+
 	for (i = 0; i < tcfg->tconfig->num_cfg; i++) {
 		if (rte_red_config_init(&tcfg->tconfig->rconfig[i],
 					(uint16_t)tcfg->tconfig->wq_log2[i],
@@ -385,6 +399,7 @@ static struct test_rte_red_config ft_tconfig =  {
 	.min_th = 32,
 	.max_th = 128,
 	.maxp_inv = ft_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_queue ft_tqueue = {
@@ -554,6 +569,7 @@ static struct test_rte_red_config ft2_tconfig =  {
 	.min_th = 32,
 	.max_th = 128,
 	.maxp_inv = ft2_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_config func_test2_config = {
@@ -576,6 +592,41 @@ static struct test_config func_test2_config = {
 	.tlevel = ft2_tlevel,
 };
 
+/**
+ * Test F2: functional test 2 - with long queues and smaller red-scaling factor
+ */
+static uint32_t ft2_tlevel_scaling[] = {8190};
+
+static struct test_rte_red_config ft2_tconfig_scaling =  {
+	.rconfig = ft2_rconfig,
+	.num_cfg = RTE_DIM(ft2_rconfig),
+	.wq_log2 = ft2_wq_log2,
+	.min_th = 32,
+	.max_th = 8191,
+	.maxp_inv = ft2_maxp_inv,
+	.max_queue_len = 8192,
+};
+
+static struct test_config func_test2_config_scaling = {
+	.ifname = "functional test 2 interface",
+	.msg = "functional test 2 : use several RED configurations and long queues,\n"
+	"		    increase average queue size to just below maximum threshold,\n"
+	"		    compare drop rate to drop probability\n\n",
+	.htxt = "RED config     "
+	"avg queue size "
+	"min threshold  "
+	"max threshold  "
+	"drop prob %    "
+	"drop rate %    "
+	"diff %         "
+	"tolerance %    "
+	"\n",
+	.tconfig = &ft2_tconfig_scaling,
+	.tqueue = &ft_tqueue,
+	.tvar = &ft_tvar,
+	.tlevel = ft2_tlevel_scaling,
+};
+
 static enum test_result func_test2(struct test_config *tcfg)
 {
 	enum test_result result = PASS;
@@ -662,6 +713,7 @@ static struct test_rte_red_config ft3_tconfig =  {
 	.min_th = 32,
 	.max_th = 1023,
 	.maxp_inv = ft_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_config func_test3_config = {
@@ -683,6 +735,40 @@ static struct test_config func_test3_config = {
 	.tlevel = ft3_tlevel,
 };
 
+/**
+ * Test F3: functional test 3 - with large queues and smaller red scaling factor
+ */
+static uint32_t ft3_tlevel_scaling[] = {8190};
+
+static struct test_rte_red_config ft3_tconfig_scaling =  {
+	.rconfig = ft_wrconfig,
+	.num_cfg = RTE_DIM(ft_wrconfig),
+	.wq_log2 = ft_wq_log2,
+	.min_th = 32,
+	.max_th = 8191,
+	.maxp_inv = ft_maxp_inv,
+	.max_queue_len = 8192,
+};
+
+static struct test_config func_test3_config_scaling = {
+	.ifname = "functional test 3 interface",
+	.msg = "functional test 3 : use one RED configuration and long queues,\n"
+	"		    increase average queue size to target level,\n"
+	"		    dequeue all packets until queue is empty,\n"
+	"		    confirm that average queue size is computed correctly while queue is empty\n\n",
+	.htxt = "q avg before   "
+	"q avg after    "
+	"expected       "
+	"difference %   "
+	"tolerance %    "
+	"result	 "
+	"\n",
+	.tconfig = &ft3_tconfig_scaling,
+	.tqueue = &ft_tqueue,
+	.tvar = &ft_tvar,
+	.tlevel = ft3_tlevel_scaling,
+};
+
 static enum test_result func_test3(struct test_config *tcfg)
 {
 	enum test_result result = PASS;
@@ -776,6 +862,7 @@ static struct test_rte_red_config ft4_tconfig =  {
 	.max_th = 1023,
 	.wq_log2 = ft4_wq_log2,
 	.maxp_inv = ft_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_queue ft4_tqueue = {
@@ -810,6 +897,42 @@ static struct test_config func_test4_config = {
 	.tlevel = ft4_tlevel,
 };
 
+/**
+ * Test F4: functional test 4
+ */
+static uint32_t ft4_tlevel_scaling[] = {8190};
+
+static struct test_rte_red_config ft4_tconfig_scaling =  {
+	.rconfig = ft_wrconfig,
+	.num_cfg = RTE_DIM(ft_wrconfig),
+	.min_th = 32,
+	.max_th = 8191,
+	.wq_log2 = ft4_wq_log2,
+	.maxp_inv = ft_maxp_inv,
+	.max_queue_len = 8192,
+};
+
+static struct test_config func_test4_config_scaling = {
+	.ifname = "functional test 4 interface",
+	.msg = "functional test 4 : use one RED configuration on long queue,\n"
+	"		    increase average queue size to target level,\n"
+	"		    dequeue all packets until queue is empty,\n"
+	"		    confirm that average queue size is computed correctly while\n"
+	"		    queue is empty for more than 50 sec,\n"
+	"		    (this test takes 52 sec to run)\n\n",
+	.htxt = "q avg before   "
+	"q avg after    "
+	"expected       "
+	"difference %   "
+	"tolerance %    "
+	"result	 "
+	"\n",
+	.tconfig = &ft4_tconfig_scaling,
+	.tqueue = &ft4_tqueue,
+	.tvar = &ft_tvar,
+	.tlevel = ft4_tlevel_scaling,
+};
+
 static enum test_result func_test4(struct test_config *tcfg)
 {
 	enum test_result result = PASS;
@@ -924,6 +1047,7 @@ static struct test_rte_red_config ft5_tconfig =  {
 	.max_th = 128,
 	.wq_log2 = ft5_wq_log2,
 	.maxp_inv = ft5_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_queue ft5_tqueue = {
@@ -970,6 +1094,45 @@ static struct test_config func_test5_config = {
 	.tlevel = ft5_tlevel,
 };
 
+
+/**
+ * Test F5: functional test 5
+ */
+static uint32_t ft5_tlevel_scaling[] = {8190};
+
+static struct test_rte_red_config ft5_tconfig_scaling =  {
+	.rconfig = ft5_config,
+	.num_cfg = RTE_DIM(ft5_config),
+	.min_th = 32,
+	.max_th = 8191,
+	.wq_log2 = ft5_wq_log2,
+	.maxp_inv = ft5_maxp_inv,
+	.max_queue_len = 8192,
+};
+
+static struct test_config func_test5_config_scaling = {
+	.ifname = "functional test 5 interface",
+	.msg = "functional test 5 : use several long queues (each with its own run-time data),\n"
+	"		    use several RED configurations (such that each configuration is shared by multiple queues),\n"
+	"		    increase average queue size to just below maximum threshold,\n"
+	"		    compare drop rate to drop probability,\n"
+	"		    (this is a larger scale version of functional test 2)\n\n",
+	.htxt = "queue          "
+	"config         "
+	"avg queue size "
+	"min threshold  "
+	"max threshold  "
+	"drop prob %    "
+	"drop rate %    "
+	"diff %         "
+	"tolerance %    "
+	"\n",
+	.tconfig = &ft5_tconfig_scaling,
+	.tqueue = &ft5_tqueue,
+	.tvar = &ft5_tvar,
+	.tlevel = ft5_tlevel_scaling,
+};
+
 static enum test_result func_test5(struct test_config *tcfg)
 {
 	enum test_result result = PASS;
@@ -1062,6 +1225,7 @@ static struct test_rte_red_config ft6_tconfig =  {
 	.max_th = 1023,
 	.wq_log2 = ft6_wq_log2,
 	.maxp_inv = ft6_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_queue ft6_tqueue = {
@@ -1078,7 +1242,7 @@ static struct test_queue ft6_tqueue = {
 static struct test_config func_test6_config = {
 	.ifname = "functional test 6 interface",
 	.msg = "functional test 6 : use several queues (each with its own run-time data),\n"
-	"		    use several RED configurations (such that each configuration is sharte_red by multiple queues),\n"
+	"		    use several RED configurations (such that each configuration is shared by multiple queues),\n"
 	"		    increase average queue size to target level,\n"
 	"		    dequeue all packets until queue is empty,\n"
 	"		    confirm that average queue size is computed correctly while queue is empty\n"
@@ -1097,6 +1261,44 @@ static struct test_config func_test6_config = {
 	.tlevel = ft6_tlevel,
 };
 
+/**
+ * Test F6: functional test 6
+ */
+static uint32_t ft6_tlevel_scaling[] = {8190};
+
+static struct test_rte_red_config ft6_tconfig_scaling =  {
+	.rconfig = ft6_config,
+	.num_cfg = RTE_DIM(ft6_config),
+	.min_th = 32,
+	.max_th = 8191,
+	.wq_log2 = ft6_wq_log2,
+	.maxp_inv = ft6_maxp_inv,
+	.max_queue_len = 8192,
+};
+
+static struct test_config func_test6_config_scaling = {
+	.ifname = "functional test 6 interface",
+	.msg = "functional test 6 : use several long queues (each with its own run-time data),\n"
+	"		    use several RED configurations (such that each configuration is shared by multiple queues),\n"
+	"		    increase average queue size to target level,\n"
+	"		    dequeue all packets until queue is empty,\n"
+	"		    confirm that average queue size is computed correctly while queue is empty\n"
+	"		    (this is a larger scale version of functional test 3)\n\n",
+	.htxt = "queue          "
+	"config         "
+	"q avg before   "
+	"q avg after    "
+	"expected       "
+	"difference %   "
+	"tolerance %    "
+	"result	 "
+	"\n",
+	.tconfig = &ft6_tconfig_scaling,
+	.tqueue = &ft6_tqueue,
+	.tvar = &ft_tvar,
+	.tlevel = ft6_tlevel_scaling,
+};
+
 static enum test_result func_test6(struct test_config *tcfg)
 {
 	enum test_result result = PASS;
@@ -1195,6 +1397,7 @@ static struct test_rte_red_config pt_tconfig =  {
 	.min_th = 32,
 	.max_th = 128,
 	.maxp_inv = pt_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_queue pt_tqueue = {
@@ -1557,6 +1760,7 @@ static struct test_rte_red_config ovfl_tconfig =  {
 	.min_th = 32,
 	.max_th = 1023,
 	.maxp_inv = ovfl_maxp_inv,
+	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
 };
 
 static struct test_queue ovfl_tqueue = {
@@ -1598,7 +1802,7 @@ static struct test_config ovfl_test1_config = {
 	.ifname = "queue avergage overflow test interface",
 	.msg = "overflow test 1 : use one RED configuration,\n"
 	"		  increase average queue size to target level,\n"
-	"		  check maximum number of bits requirte_red to represent avg_s\n\n",
+	"		  check maximum number of bits required to represent avg_s\n\n",
 	.htxt = "avg queue size  "
 	"wq_log2  "
 	"fraction bits  "
@@ -1615,6 +1819,39 @@ static struct test_config ovfl_test1_config = {
 	.tlevel = ovfl_tlevel,
 };
 
+static uint32_t ovfl_tlevel_scaling[] = {8191};
+
+static struct test_rte_red_config ovfl_tconfig_scaling =  {
+	.rconfig = ovfl_wrconfig,
+	.num_cfg = RTE_DIM(ovfl_wrconfig),
+	.wq_log2 = ovfl_wq_log2,
+	.min_th = 32,
+	.max_th = 8191,
+	.maxp_inv = ovfl_maxp_inv,
+	.max_queue_len = 8192,
+};
+
+static struct test_config ovfl_test1_config_scaling = {
+	.ifname = "queue average overflow test interface",
+	.msg = "overflow test 1 on long queue: use one RED configuration,\n"
+	"		  increase average queue size to target level,\n"
+	"		  check maximum number of bits required to represent avg_s\n\n",
+	.htxt = "avg queue size  "
+	"wq_log2  "
+	"fraction bits  "
+	"max queue avg  "
+	"num bits  "
+	"enqueued  "
+	"dropped   "
+	"drop prob %  "
+	"drop rate %  "
+	"\n",
+	.tconfig = &ovfl_tconfig_scaling,
+	.tqueue = &ovfl_tqueue,
+	.tvar = &ovfl_tvar,
+	.tlevel = ovfl_tlevel_scaling,
+};
+
 static enum test_result ovfl_test1(struct test_config *tcfg)
 {
 	enum test_result result = PASS;
@@ -1690,7 +1927,7 @@ static enum test_result ovfl_test1(struct test_config *tcfg)
 	printf("%s", tcfg->htxt);
 
 	printf("%-16u%-9u%-15u0x%08x     %-10u%-10u%-10u%-13.2lf%-13.2lf\n",
-	       avg, *tcfg->tconfig->wq_log2, RTE_RED_SCALING,
+	       avg, *tcfg->tconfig->wq_log2, rte_red_scaling,
 	       avg_max, avg_max_bits,
 	       *tcfg->tvar->enqueued, *tcfg->tvar->dropped,
 	       drop_prob * 100.0, drop_rate * 100.0);
@@ -1730,6 +1967,15 @@ struct tests perf_tests[] = {
 	{ &perf2_test6_config, perf2_test },
 };
 
+struct tests scale_tests[] = {
+	{ &func_test2_config_scaling, func_test2 },
+	{ &func_test3_config_scaling, func_test3 },
+	{ &func_test4_config_scaling, func_test4 },
+	{ &func_test5_config_scaling, func_test5 },
+	{ &func_test6_config_scaling, func_test6 },
+	{ &ovfl_test1_config_scaling, ovfl_test1 },
+};
+
 /**
  * function to execute the required_red tests
  */
@@ -1866,6 +2112,20 @@ test_red_perf(void)
 }
 
 static int
+test_red_scaling(void)
+{
+	uint32_t num_tests = 0;
+	uint32_t num_pass = 0;
+
+	if (test_invalid_parameters() < 0)
+		return -1;
+
+	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests, &num_pass);
+	show_stats(num_tests, num_pass);
+	return tell_the_result(num_tests, num_pass);
+}
+
+static int
 test_red_all(void)
 {
 	uint32_t num_tests = 0;
@@ -1876,10 +2136,12 @@ test_red_all(void)
 
 	run_tests(func_tests, RTE_DIM(func_tests), &num_tests, &num_pass);
 	run_tests(perf_tests, RTE_DIM(perf_tests), &num_tests, &num_pass);
+	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests, &num_pass);
 	show_stats(num_tests, num_pass);
 	return tell_the_result(num_tests, num_pass);
 }
 
 REGISTER_TEST_COMMAND(red_autotest, test_red);
 REGISTER_TEST_COMMAND(red_perf, test_red_perf);
+REGISTER_TEST_COMMAND(red_scaling, test_red_scaling);
 REGISTER_TEST_COMMAND(red_all, test_red_all);
-- 
2.1.4

Re: [PATCH v4] sched: make RED scaling configurable

[Dumitrescu, Cristian]

Adding Tomasz.

> -----Original Message-----
> From: alangordondewar@gmail.com [mailto:alangordondewar@gmail.com]
> Sent: Tuesday, October 3, 2017 10:22 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
> Subject: [PATCH v4] sched: make RED scaling configurable
> 
> From: Alan Dewar <alan.dewar@att.com>
> 
> The RED code stores the weighted moving average in a 32-bit integer as
> a pseudo fixed-point floating number with 10 fractional bits.  Twelve
> other bits are used to encode the filter weight, leaving just 10 bits
> for the queue length.  This limits the maximum queue length supported
> by RED queues to 1024 packets.
> 
> Introduce a new API to allow the RED scaling factor to be configured
> based upon maximum queue length.  If this API is not called, the RED
> scaling factor remains at its default value.
> 
> Added some new RED scaling unit-tests to test with RED queue-lengths
> up to 8192 packets long.
> 
> Signed-off-by: Alan Dewar <alan.dewar@att.com>
> ---
>  lib/librte_sched/rte_red.c             |  53 ++++++-
>  lib/librte_sched/rte_red.h             |  63 ++++++--
>  lib/librte_sched/rte_sched_version.map |   6 +
>  test/test/test_red.c                   | 274
> ++++++++++++++++++++++++++++++++-
>  4 files changed, 374 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
> index ade57d1..0dc8d28 100644
> --- a/lib/librte_sched/rte_red.c
> +++ b/lib/librte_sched/rte_red.c
> @@ -43,6 +43,8 @@
>  static int rte_red_init_done = 0;     /**< Flag to indicate that global
> initialisation is done */
>  uint32_t rte_red_rand_val = 0;        /**< Random value cache */
>  uint32_t rte_red_rand_seed = 0;       /**< Seed for random number
> generation */
> +uint8_t rte_red_scaling = RTE_RED_SCALING_DEFAULT;
> +uint16_t rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH -
> 1;
> 
>  /**
>   * table[i] = log2(1-Wq) * Scale * -1
> @@ -66,7 +68,7 @@ __rte_red_init_tables(void)
>  	double scale = 0.0;
>  	double table_size = 0.0;
> 
> -	scale = (double)(1 << RTE_RED_SCALING);
> +	scale = (double)(1 << rte_red_scaling);
>  	table_size = (double)(RTE_DIM(rte_red_pow2_frac_inv));
> 
>  	for (i = 0; i < RTE_DIM(rte_red_pow2_frac_inv); i++) {
> @@ -119,7 +121,7 @@ rte_red_config_init(struct rte_red_config *red_cfg,
>  	if (red_cfg == NULL) {
>  		return -1;
>  	}
> -	if (max_th > RTE_RED_MAX_TH_MAX) {
> +	if (max_th > rte_red_max_threshold) {
>  		return -2;
>  	}
>  	if (min_th >= max_th) {
> @@ -148,11 +150,52 @@ rte_red_config_init(struct rte_red_config
> *red_cfg,
>  		rte_red_init_done = 1;
>  	}
> 
> -	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> RTE_RED_SCALING);
> -	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> RTE_RED_SCALING);
> -	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> RTE_RED_SCALING;
> +	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> rte_red_scaling);
> +	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> rte_red_scaling);
> +	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> +		rte_red_scaling;
>  	red_cfg->maxp_inv = maxp_inv;
>  	red_cfg->wq_log2 = wq_log2;
> 
>  	return 0;
>  }
> +
> +int
> +rte_red_set_scaling(uint16_t max_red_queue_length)
> +{
> +	int8_t count;
> +
> +	if (rte_red_init_done)
> +		/**
> +		 * Can't change the scaling once the red table has been
> +		 * computed.
> +		 */
> +		return -1;
> +
> +	if (max_red_queue_length < RTE_RED_MIN_QUEUE_LENGTH)
> +		return -2;
> +
> +	if (max_red_queue_length > RTE_RED_MAX_QUEUE_LENGTH)
> +		return -3;
> +
> +	if (!rte_is_power_of_2(max_red_queue_length))
> +		return -4;
> +
> +	count = 0;
> +	while (max_red_queue_length != 0) {
> +		max_red_queue_length >>= 1;
> +		count++;
> +	}
> +
> +	rte_red_scaling -= count - RTE_RED_SCALING_DEFAULT;
> +	rte_red_max_threshold = max_red_queue_length - 1;
> +	return 0;
> +}
> +
> +void
> +rte_red_reset_scaling(void)
> +{
> +	rte_red_init_done = 0;
> +	rte_red_scaling = RTE_RED_SCALING_DEFAULT;
> +	rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH - 1;
> +}
> diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h
> index ca12227..be1fb0f 100644
> --- a/lib/librte_sched/rte_red.h
> +++ b/lib/librte_sched/rte_red.h
> @@ -52,14 +52,31 @@ extern "C" {
>  #include <rte_cycles.h>
>  #include <rte_branch_prediction.h>
> 
> -#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-
> point */
> -#define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied by
> number of leaf queues */
> -#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold limit
> in fixed point format */
> -#define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter
> weight value */
> -#define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse filter
> weight value */
> -#define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark
> probability value */
> -#define RTE_RED_MAXP_INV_MAX                255        /**< Max inverse mark
> probability value */
> -#define RTE_RED_2POW16                      (1<<16)    /**< 2 power 16 */
> +/**< Default fraction size for fixed-point */
> +#define RTE_RED_SCALING_DEFAULT             10
> +
> +/**< Packet size multiplied by number of leaf queues */
> +#define RTE_RED_S                           (1 << 22)
> +
> +/**< Minimum, default and maximum RED queue length */
> +#define RTE_RED_MIN_QUEUE_LENGTH            64
> +#define RTE_RED_DEFAULT_QUEUE_LENGTH        1024
> +#define RTE_RED_MAX_QUEUE_LENGTH            8192
> +
> +/**< Min inverse filter weight value */
> +#define RTE_RED_WQ_LOG2_MIN                 1
> +
> +/**< Max inverse filter weight value */
> +#define RTE_RED_WQ_LOG2_MAX                 12
> +
> +/**< Min inverse mark probability value */
> +#define RTE_RED_MAXP_INV_MIN                1
> +
> +/**< Max inverse mark probability value */
> +#define RTE_RED_MAXP_INV_MAX                255
> +
> +/**< 2 power 16 */
> +#define RTE_RED_2POW16                      (1<<16)
>  #define RTE_RED_INT16_NBITS                 (sizeof(uint16_t) * CHAR_BIT)
>  #define RTE_RED_WQ_LOG2_NUM                 (RTE_RED_WQ_LOG2_MAX -
> RTE_RED_WQ_LOG2_MIN + 1)
> 
> @@ -71,6 +88,8 @@ extern uint32_t rte_red_rand_val;
>  extern uint32_t rte_red_rand_seed;
>  extern uint16_t rte_red_log2_1_minus_Wq[RTE_RED_WQ_LOG2_NUM];
>  extern uint16_t rte_red_pow2_frac_inv[16];
> +extern uint8_t rte_red_scaling;
> +extern uint16_t rte_red_max_threshold;
> 
>  /**
>   * RED configuration parameters passed by user
> @@ -137,6 +156,26 @@ rte_red_config_init(struct rte_red_config *red_cfg,
>  	const uint16_t maxp_inv);
> 
>  /**
> + * @brief Configures the global setting for the RED scaling factor
> + *
> + * @param max_red_queue_length [in] must be a power of two
> + *
> + * @return Operation status
> + * @retval 0 success
> + * @retval !0 error
> + */
> +int
> +rte_red_set_scaling(uint16_t max_red_queue_length);
> +
> +/**
> + * @brief Reset the RED scaling factor - only for use by RED unit-tests
> + *
> + * @return Operation status
> + */
> +void
> +rte_red_reset_scaling(void);
> +
> +/**
>   * @brief Generate random number for RED
>   *
>   * Implemenetation based on:
> @@ -206,7 +245,7 @@ __rte_red_calc_qempty_factor(uint8_t wq_log2,
> uint16_t m)
>  	f = (n >> 6) & 0xf;
>  	n >>= 10;
> 
> -	if (n < RTE_RED_SCALING)
> +	if (n < rte_red_scaling)
>  		return (uint16_t) ((rte_red_pow2_frac_inv[f] + (1 << (n - 1)))
> >> n);
> 
>  	return 0;
> @@ -258,7 +297,9 @@ rte_red_enqueue_empty(const struct
> rte_red_config *red_cfg,
>  	if (m >= RTE_RED_2POW16) {
>  		red->avg = 0;
>  	} else {
> -		red->avg = (red->avg >> RTE_RED_SCALING) *
> __rte_red_calc_qempty_factor(red_cfg->wq_log2, (uint16_t) m);
> +		red->avg = (red->avg >> rte_red_scaling) *
> +			__rte_red_calc_qempty_factor(red_cfg->wq_log2,
> +						     (uint16_t) m);
>  	}
> 
>  	return 0;
> @@ -365,7 +406,7 @@ rte_red_enqueue_nonempty(const struct
> rte_red_config *red_cfg,
>  	*/
> 
>  	/* avg update */
> -	red->avg += (q << RTE_RED_SCALING) - (red->avg >> red_cfg-
> >wq_log2);
> +	red->avg += (q << rte_red_scaling) - (red->avg >> red_cfg-
> >wq_log2);
> 
>  	/* avg < min_th: do not mark the packet  */
>  	if (red->avg < red_cfg->min_th) {
> diff --git a/lib/librte_sched/rte_sched_version.map
> b/lib/librte_sched/rte_sched_version.map
> index 3aa159a..92e51f8 100644
> --- a/lib/librte_sched/rte_sched_version.map
> +++ b/lib/librte_sched/rte_sched_version.map
> @@ -29,3 +29,9 @@ DPDK_2.1 {
>  	rte_sched_port_pkt_read_color;
> 
>  } DPDK_2.0;
> +
> +DPDK_17.08 {
> +	global;
> +
> +	rte_red_set_scaling;
> +} DPDK_2.1;
> diff --git a/test/test/test_red.c b/test/test/test_red.c
> index 348075d..f2d50ef 100644
> --- a/test/test/test_red.c
> +++ b/test/test/test_red.c
> @@ -67,6 +67,7 @@ struct test_rte_red_config {        /**< Test structure for
> RTE_RED config */
>  	uint32_t min_th;                /**< Queue minimum threshold */
>  	uint32_t max_th;                /**< Queue maximum threshold */
>  	uint8_t *maxp_inv;              /**< Inverse mark probability */
> +	uint16_t max_queue_len;         /**< Maximum queue length */
>  };
> 
>  struct test_queue {                 /**< Test structure for RTE_RED Queues */
> @@ -181,7 +182,7 @@ static uint32_t rte_red_get_avg_int(const struct
> rte_red_config *red_cfg,
>  	/**
>  	 * scale by 1/n and convert from fixed-point to integer
>  	 */
> -	return red->avg >> (RTE_RED_SCALING + red_cfg->wq_log2);
> +	return red->avg >> (rte_red_scaling + red_cfg->wq_log2);
>  }
> 
>  static double rte_red_get_avg_float(const struct rte_red_config *red_cfg,
> @@ -190,7 +191,7 @@ static double rte_red_get_avg_float(const struct
> rte_red_config *red_cfg,
>  	/**
>  	 * scale by 1/n and convert from fixed-point to floating-point
>  	 */
> -	return ldexp((double)red->avg,  -(RTE_RED_SCALING + red_cfg-
> >wq_log2));
> +	return ldexp((double)red->avg,  -(rte_red_scaling + red_cfg-
> >wq_log2));
>  }
> 
>  static void rte_red_set_avg_int(const struct rte_red_config *red_cfg,
> @@ -200,7 +201,7 @@ static void rte_red_set_avg_int(const struct
> rte_red_config *red_cfg,
>  	/**
>  	 * scale by n and convert from integer to fixed-point
>  	 */
> -	red->avg = avg << (RTE_RED_SCALING + red_cfg->wq_log2);
> +	red->avg = avg << (rte_red_scaling + red_cfg->wq_log2);
>  }
> 
>  static double calc_exp_avg_on_empty(double avg, uint32_t n, uint32_t
> time_diff)
> @@ -280,10 +281,23 @@ static enum test_result
>  test_rte_red_init(struct test_config *tcfg)
>  {
>  	unsigned i = 0;
> +	int ret;
> 
>  	tcfg->tvar->clk_freq = rte_get_timer_hz();
>  	init_port_ts( tcfg->tvar->clk_freq );
> 
> +	rte_red_reset_scaling();
> +	ret = rte_red_set_scaling(tcfg->tconfig->max_queue_len);
> +	if (ret) {
> +		printf("rte_red_set_scaling failed: %d\n", ret);
> +		return FAIL;
> +	}
> +	if (rte_red_max_threshold < tcfg->tconfig->max_th) {
> +		printf("rte_red_max_th (%u) < tconfig->max_th (%u)\n",
> +		       rte_red_max_threshold, tcfg->tconfig->max_th);
> +		return FAIL;
> +	}
> +
>  	for (i = 0; i < tcfg->tconfig->num_cfg; i++) {
>  		if (rte_red_config_init(&tcfg->tconfig->rconfig[i],
>  					(uint16_t)tcfg->tconfig->wq_log2[i],
> @@ -385,6 +399,7 @@ static struct test_rte_red_config ft_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 128,
>  	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft_tqueue = {
> @@ -554,6 +569,7 @@ static struct test_rte_red_config ft2_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 128,
>  	.maxp_inv = ft2_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_config func_test2_config = {
> @@ -576,6 +592,41 @@ static struct test_config func_test2_config = {
>  	.tlevel = ft2_tlevel,
>  };
> 
> +/**
> + * Test F2: functional test 2 - with long queues and smaller red-scaling factor
> + */
> +static uint32_t ft2_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft2_tconfig_scaling =  {
> +	.rconfig = ft2_rconfig,
> +	.num_cfg = RTE_DIM(ft2_rconfig),
> +	.wq_log2 = ft2_wq_log2,
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.maxp_inv = ft2_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test2_config_scaling = {
> +	.ifname = "functional test 2 interface",
> +	.msg = "functional test 2 : use several RED configurations and long
> queues,\n"
> +	"		    increase average queue size to just below
> maximum threshold,\n"
> +	"		    compare drop rate to drop probability\n\n",
> +	.htxt = "RED config     "
> +	"avg queue size "
> +	"min threshold  "
> +	"max threshold  "
> +	"drop prob %    "
> +	"drop rate %    "
> +	"diff %         "
> +	"tolerance %    "
> +	"\n",
> +	.tconfig = &ft2_tconfig_scaling,
> +	.tqueue = &ft_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft2_tlevel_scaling,
> +};
> +
>  static enum test_result func_test2(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -662,6 +713,7 @@ static struct test_rte_red_config ft3_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 1023,
>  	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_config func_test3_config = {
> @@ -683,6 +735,40 @@ static struct test_config func_test3_config = {
>  	.tlevel = ft3_tlevel,
>  };
> 
> +/**
> + * Test F3: functional test 3 - with large queues and smaller red scaling factor
> + */
> +static uint32_t ft3_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft3_tconfig_scaling =  {
> +	.rconfig = ft_wrconfig,
> +	.num_cfg = RTE_DIM(ft_wrconfig),
> +	.wq_log2 = ft_wq_log2,
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test3_config_scaling = {
> +	.ifname = "functional test 3 interface",
> +	.msg = "functional test 3 : use one RED configuration and long
> queues,\n"
> +	"		    increase average queue size to target level,\n"
> +	"		    dequeue all packets until queue is empty,\n"
> +	"		    confirm that average queue size is computed
> correctly while queue is empty\n\n",
> +	.htxt = "q avg before   "
> +	"q avg after    "
> +	"expected       "
> +	"difference %   "
> +	"tolerance %    "
> +	"result	 "
> +	"\n",
> +	.tconfig = &ft3_tconfig_scaling,
> +	.tqueue = &ft_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft3_tlevel_scaling,
> +};
> +
>  static enum test_result func_test3(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -776,6 +862,7 @@ static struct test_rte_red_config ft4_tconfig =  {
>  	.max_th = 1023,
>  	.wq_log2 = ft4_wq_log2,
>  	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft4_tqueue = {
> @@ -810,6 +897,42 @@ static struct test_config func_test4_config = {
>  	.tlevel = ft4_tlevel,
>  };
> 
> +/**
> + * Test F4: functional test 4
> + */
> +static uint32_t ft4_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft4_tconfig_scaling =  {
> +	.rconfig = ft_wrconfig,
> +	.num_cfg = RTE_DIM(ft_wrconfig),
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.wq_log2 = ft4_wq_log2,
> +	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test4_config_scaling = {
> +	.ifname = "functional test 4 interface",
> +	.msg = "functional test 4 : use one RED configuration on long
> queue,\n"
> +	"		    increase average queue size to target level,\n"
> +	"		    dequeue all packets until queue is empty,\n"
> +	"		    confirm that average queue size is computed
> correctly while\n"
> +	"		    queue is empty for more than 50 sec,\n"
> +	"		    (this test takes 52 sec to run)\n\n",
> +	.htxt = "q avg before   "
> +	"q avg after    "
> +	"expected       "
> +	"difference %   "
> +	"tolerance %    "
> +	"result	 "
> +	"\n",
> +	.tconfig = &ft4_tconfig_scaling,
> +	.tqueue = &ft4_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft4_tlevel_scaling,
> +};
> +
>  static enum test_result func_test4(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -924,6 +1047,7 @@ static struct test_rte_red_config ft5_tconfig =  {
>  	.max_th = 128,
>  	.wq_log2 = ft5_wq_log2,
>  	.maxp_inv = ft5_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft5_tqueue = {
> @@ -970,6 +1094,45 @@ static struct test_config func_test5_config = {
>  	.tlevel = ft5_tlevel,
>  };
> 
> +
> +/**
> + * Test F5: functional test 5
> + */
> +static uint32_t ft5_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft5_tconfig_scaling =  {
> +	.rconfig = ft5_config,
> +	.num_cfg = RTE_DIM(ft5_config),
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.wq_log2 = ft5_wq_log2,
> +	.maxp_inv = ft5_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test5_config_scaling = {
> +	.ifname = "functional test 5 interface",
> +	.msg = "functional test 5 : use several long queues (each with its own
> run-time data),\n"
> +	"		    use several RED configurations (such that each
> configuration is shared by multiple queues),\n"
> +	"		    increase average queue size to just below
> maximum threshold,\n"
> +	"		    compare drop rate to drop probability,\n"
> +	"		    (this is a larger scale version of functional test
> 2)\n\n",
> +	.htxt = "queue          "
> +	"config         "
> +	"avg queue size "
> +	"min threshold  "
> +	"max threshold  "
> +	"drop prob %    "
> +	"drop rate %    "
> +	"diff %         "
> +	"tolerance %    "
> +	"\n",
> +	.tconfig = &ft5_tconfig_scaling,
> +	.tqueue = &ft5_tqueue,
> +	.tvar = &ft5_tvar,
> +	.tlevel = ft5_tlevel_scaling,
> +};
> +
>  static enum test_result func_test5(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -1062,6 +1225,7 @@ static struct test_rte_red_config ft6_tconfig =  {
>  	.max_th = 1023,
>  	.wq_log2 = ft6_wq_log2,
>  	.maxp_inv = ft6_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft6_tqueue = {
> @@ -1078,7 +1242,7 @@ static struct test_queue ft6_tqueue = {
>  static struct test_config func_test6_config = {
>  	.ifname = "functional test 6 interface",
>  	.msg = "functional test 6 : use several queues (each with its own run-
> time data),\n"
> -	"		    use several RED configurations (such that each
> configuration is sharte_red by multiple queues),\n"
> +	"		    use several RED configurations (such that each
> configuration is shared by multiple queues),\n"
>  	"		    increase average queue size to target level,\n"
>  	"		    dequeue all packets until queue is empty,\n"
>  	"		    confirm that average queue size is computed
> correctly while queue is empty\n"
> @@ -1097,6 +1261,44 @@ static struct test_config func_test6_config = {
>  	.tlevel = ft6_tlevel,
>  };
> 
> +/**
> + * Test F6: functional test 6
> + */
> +static uint32_t ft6_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft6_tconfig_scaling =  {
> +	.rconfig = ft6_config,
> +	.num_cfg = RTE_DIM(ft6_config),
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.wq_log2 = ft6_wq_log2,
> +	.maxp_inv = ft6_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test6_config_scaling = {
> +	.ifname = "functional test 6 interface",
> +	.msg = "functional test 6 : use several long queues (each with its own
> run-time data),\n"
> +	"		    use several RED configurations (such that each
> configuration is shared by multiple queues),\n"
> +	"		    increase average queue size to target level,\n"
> +	"		    dequeue all packets until queue is empty,\n"
> +	"		    confirm that average queue size is computed
> correctly while queue is empty\n"
> +	"		    (this is a larger scale version of functional test
> 3)\n\n",
> +	.htxt = "queue          "
> +	"config         "
> +	"q avg before   "
> +	"q avg after    "
> +	"expected       "
> +	"difference %   "
> +	"tolerance %    "
> +	"result	 "
> +	"\n",
> +	.tconfig = &ft6_tconfig_scaling,
> +	.tqueue = &ft6_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft6_tlevel_scaling,
> +};
> +
>  static enum test_result func_test6(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -1195,6 +1397,7 @@ static struct test_rte_red_config pt_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 128,
>  	.maxp_inv = pt_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue pt_tqueue = {
> @@ -1557,6 +1760,7 @@ static struct test_rte_red_config ovfl_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 1023,
>  	.maxp_inv = ovfl_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ovfl_tqueue = {
> @@ -1598,7 +1802,7 @@ static struct test_config ovfl_test1_config = {
>  	.ifname = "queue avergage overflow test interface",
>  	.msg = "overflow test 1 : use one RED configuration,\n"
>  	"		  increase average queue size to target level,\n"
> -	"		  check maximum number of bits requirte_red to
> represent avg_s\n\n",
> +	"		  check maximum number of bits required to
> represent avg_s\n\n",
>  	.htxt = "avg queue size  "
>  	"wq_log2  "
>  	"fraction bits  "
> @@ -1615,6 +1819,39 @@ static struct test_config ovfl_test1_config = {
>  	.tlevel = ovfl_tlevel,
>  };
> 
> +static uint32_t ovfl_tlevel_scaling[] = {8191};
> +
> +static struct test_rte_red_config ovfl_tconfig_scaling =  {
> +	.rconfig = ovfl_wrconfig,
> +	.num_cfg = RTE_DIM(ovfl_wrconfig),
> +	.wq_log2 = ovfl_wq_log2,
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.maxp_inv = ovfl_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config ovfl_test1_config_scaling = {
> +	.ifname = "queue average overflow test interface",
> +	.msg = "overflow test 1 on long queue: use one RED
> configuration,\n"
> +	"		  increase average queue size to target level,\n"
> +	"		  check maximum number of bits required to
> represent avg_s\n\n",
> +	.htxt = "avg queue size  "
> +	"wq_log2  "
> +	"fraction bits  "
> +	"max queue avg  "
> +	"num bits  "
> +	"enqueued  "
> +	"dropped   "
> +	"drop prob %  "
> +	"drop rate %  "
> +	"\n",
> +	.tconfig = &ovfl_tconfig_scaling,
> +	.tqueue = &ovfl_tqueue,
> +	.tvar = &ovfl_tvar,
> +	.tlevel = ovfl_tlevel_scaling,
> +};
> +
>  static enum test_result ovfl_test1(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -1690,7 +1927,7 @@ static enum test_result ovfl_test1(struct
> test_config *tcfg)
>  	printf("%s", tcfg->htxt);
> 
>  	printf("%-16u%-9u%-15u0x%08x     %-10u%-10u%-10u%-13.2lf%-
> 13.2lf\n",
> -	       avg, *tcfg->tconfig->wq_log2, RTE_RED_SCALING,
> +	       avg, *tcfg->tconfig->wq_log2, rte_red_scaling,
>  	       avg_max, avg_max_bits,
>  	       *tcfg->tvar->enqueued, *tcfg->tvar->dropped,
>  	       drop_prob * 100.0, drop_rate * 100.0);
> @@ -1730,6 +1967,15 @@ struct tests perf_tests[] = {
>  	{ &perf2_test6_config, perf2_test },
>  };
> 
> +struct tests scale_tests[] = {
> +	{ &func_test2_config_scaling, func_test2 },
> +	{ &func_test3_config_scaling, func_test3 },
> +	{ &func_test4_config_scaling, func_test4 },
> +	{ &func_test5_config_scaling, func_test5 },
> +	{ &func_test6_config_scaling, func_test6 },
> +	{ &ovfl_test1_config_scaling, ovfl_test1 },
> +};
> +
>  /**
>   * function to execute the required_red tests
>   */
> @@ -1866,6 +2112,20 @@ test_red_perf(void)
>  }
> 
>  static int
> +test_red_scaling(void)
> +{
> +	uint32_t num_tests = 0;
> +	uint32_t num_pass = 0;
> +
> +	if (test_invalid_parameters() < 0)
> +		return -1;
> +
> +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> &num_pass);
> +	show_stats(num_tests, num_pass);
> +	return tell_the_result(num_tests, num_pass);
> +}
> +
> +static int
>  test_red_all(void)
>  {
>  	uint32_t num_tests = 0;
> @@ -1876,10 +2136,12 @@ test_red_all(void)
> 
>  	run_tests(func_tests, RTE_DIM(func_tests), &num_tests,
> &num_pass);
>  	run_tests(perf_tests, RTE_DIM(perf_tests), &num_tests,
> &num_pass);
> +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> &num_pass);
>  	show_stats(num_tests, num_pass);
>  	return tell_the_result(num_tests, num_pass);
>  }
> 
>  REGISTER_TEST_COMMAND(red_autotest, test_red);
>  REGISTER_TEST_COMMAND(red_perf, test_red_perf);
> +REGISTER_TEST_COMMAND(red_scaling, test_red_scaling);
>  REGISTER_TEST_COMMAND(red_all, test_red_all);
> --
> 2.1.4

Re: [PATCH v4] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Tuesday, October 3, 2017 6:16 PM
> To: alangordondewar@gmail.com; Kantecki, Tomasz
> <tomasz.kantecki@intel.com>
> Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
> Subject: RE: [PATCH v4] sched: make RED scaling configurable
> 
> Adding Tomasz.
> 
> > -----Original Message-----
> > From: alangordondewar@gmail.com
> [mailto:alangordondewar@gmail.com]
> > Sent: Tuesday, October 3, 2017 10:22 AM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
> > Subject: [PATCH v4] sched: make RED scaling configurable
> >
> > From: Alan Dewar <alan.dewar@att.com>
> >
> > The RED code stores the weighted moving average in a 32-bit integer as
> > a pseudo fixed-point floating number with 10 fractional bits.  Twelve
> > other bits are used to encode the filter weight, leaving just 10 bits
> > for the queue length.  This limits the maximum queue length supported
> > by RED queues to 1024 packets.
> >
> > Introduce a new API to allow the RED scaling factor to be configured
> > based upon maximum queue length.  If this API is not called, the RED
> > scaling factor remains at its default value.
> >
> > Added some new RED scaling unit-tests to test with RED queue-lengths
> > up to 8192 packets long.
> >
> > Signed-off-by: Alan Dewar <alan.dewar@att.com>
> > ---
> >  lib/librte_sched/rte_red.c             |  53 ++++++-
> >  lib/librte_sched/rte_red.h             |  63 ++++++--
> >  lib/librte_sched/rte_sched_version.map |   6 +
> >  test/test/test_red.c                   | 274
> > ++++++++++++++++++++++++++++++++-
> >  4 files changed, 374 insertions(+), 22 deletions(-)
> >
> > diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
> > index ade57d1..0dc8d28 100644
> > --- a/lib/librte_sched/rte_red.c
> > +++ b/lib/librte_sched/rte_red.c
> > @@ -43,6 +43,8 @@
> >  static int rte_red_init_done = 0;     /**< Flag to indicate that global
> > initialisation is done */
> >  uint32_t rte_red_rand_val = 0;        /**< Random value cache */
> >  uint32_t rte_red_rand_seed = 0;       /**< Seed for random number
> > generation */
> > +uint8_t rte_red_scaling = RTE_RED_SCALING_DEFAULT;
> > +uint16_t rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH
> -
> > 1;
> >
> >  /**
> >   * table[i] = log2(1-Wq) * Scale * -1
> > @@ -66,7 +68,7 @@ __rte_red_init_tables(void)
> >  	double scale = 0.0;
> >  	double table_size = 0.0;
> >
> > -	scale = (double)(1 << RTE_RED_SCALING);
> > +	scale = (double)(1 << rte_red_scaling);
> >  	table_size = (double)(RTE_DIM(rte_red_pow2_frac_inv));
> >
> >  	for (i = 0; i < RTE_DIM(rte_red_pow2_frac_inv); i++) {
> > @@ -119,7 +121,7 @@ rte_red_config_init(struct rte_red_config
> *red_cfg,
> >  	if (red_cfg == NULL) {
> >  		return -1;
> >  	}
> > -	if (max_th > RTE_RED_MAX_TH_MAX) {
> > +	if (max_th > rte_red_max_threshold) {
> >  		return -2;
> >  	}
> >  	if (min_th >= max_th) {
> > @@ -148,11 +150,52 @@ rte_red_config_init(struct rte_red_config
> > *red_cfg,
> >  		rte_red_init_done = 1;
> >  	}
> >
> > -	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> > RTE_RED_SCALING);
> > -	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> > RTE_RED_SCALING);
> > -	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> > RTE_RED_SCALING;
> > +	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> > rte_red_scaling);
> > +	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> > rte_red_scaling);
> > +	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> > +		rte_red_scaling;
> >  	red_cfg->maxp_inv = maxp_inv;
> >  	red_cfg->wq_log2 = wq_log2;
> >
> >  	return 0;
> >  }
> > +
> > +int
> > +rte_red_set_scaling(uint16_t max_red_queue_length)
> > +{
> > +	int8_t count;
> > +
> > +	if (rte_red_init_done)
> > +		/**
> > +		 * Can't change the scaling once the red table has been
> > +		 * computed.
> > +		 */
> > +		return -1;
> > +
> > +	if (max_red_queue_length < RTE_RED_MIN_QUEUE_LENGTH)
> > +		return -2;
> > +
> > +	if (max_red_queue_length > RTE_RED_MAX_QUEUE_LENGTH)
> > +		return -3;
> > +
> > +	if (!rte_is_power_of_2(max_red_queue_length))
> > +		return -4;
> > +
> > +	count = 0;
> > +	while (max_red_queue_length != 0) {
> > +		max_red_queue_length >>= 1;
> > +		count++;
> > +	}
> > +
> > +	rte_red_scaling -= count - RTE_RED_SCALING_DEFAULT;
> > +	rte_red_max_threshold = max_red_queue_length - 1;
> > +	return 0;
> > +}
> > +
> > +void
> > +rte_red_reset_scaling(void)
> > +{
> > +	rte_red_init_done = 0;
> > +	rte_red_scaling = RTE_RED_SCALING_DEFAULT;
> > +	rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH - 1;
> > +}
> > diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h
> > index ca12227..be1fb0f 100644
> > --- a/lib/librte_sched/rte_red.h
> > +++ b/lib/librte_sched/rte_red.h
> > @@ -52,14 +52,31 @@ extern "C" {
> >  #include <rte_cycles.h>
> >  #include <rte_branch_prediction.h>
> >
> > -#define RTE_RED_SCALING                     10         /**< Fraction size for
> fixed-
> > point */
> > -#define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied
> by
> > number of leaf queues */
> > -#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold
> limit
> > in fixed point format */
> > -#define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse
> filter
> > weight value */
> > -#define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse
> filter
> > weight value */
> > -#define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse
> mark
> > probability value */
> > -#define RTE_RED_MAXP_INV_MAX                255        /**< Max inverse
> mark
> > probability value */
> > -#define RTE_RED_2POW16                      (1<<16)    /**< 2 power 16 */
> > +/**< Default fraction size for fixed-point */
> > +#define RTE_RED_SCALING_DEFAULT             10
> > +
> > +/**< Packet size multiplied by number of leaf queues */
> > +#define RTE_RED_S                           (1 << 22)
> > +
> > +/**< Minimum, default and maximum RED queue length */
> > +#define RTE_RED_MIN_QUEUE_LENGTH            64
> > +#define RTE_RED_DEFAULT_QUEUE_LENGTH        1024
> > +#define RTE_RED_MAX_QUEUE_LENGTH            8192
> > +
> > +/**< Min inverse filter weight value */
> > +#define RTE_RED_WQ_LOG2_MIN                 1
> > +
> > +/**< Max inverse filter weight value */
> > +#define RTE_RED_WQ_LOG2_MAX                 12
> > +
> > +/**< Min inverse mark probability value */
> > +#define RTE_RED_MAXP_INV_MIN                1
> > +
> > +/**< Max inverse mark probability value */
> > +#define RTE_RED_MAXP_INV_MAX                255
> > +
> > +/**< 2 power 16 */
> > +#define RTE_RED_2POW16                      (1<<16)
> >  #define RTE_RED_INT16_NBITS                 (sizeof(uint16_t) * CHAR_BIT)
> >  #define RTE_RED_WQ_LOG2_NUM                 (RTE_RED_WQ_LOG2_MAX
> -
> > RTE_RED_WQ_LOG2_MIN + 1)
> >
> > @@ -71,6 +88,8 @@ extern uint32_t rte_red_rand_val;
> >  extern uint32_t rte_red_rand_seed;
> >  extern uint16_t rte_red_log2_1_minus_Wq[RTE_RED_WQ_LOG2_NUM];
> >  extern uint16_t rte_red_pow2_frac_inv[16];
> > +extern uint8_t rte_red_scaling;
> > +extern uint16_t rte_red_max_threshold;
> >
> >  /**
> >   * RED configuration parameters passed by user
> > @@ -137,6 +156,26 @@ rte_red_config_init(struct rte_red_config
> *red_cfg,
> >  	const uint16_t maxp_inv);
> >
> >  /**
> > + * @brief Configures the global setting for the RED scaling factor
> > + *
> > + * @param max_red_queue_length [in] must be a power of two
> > + *
> > + * @return Operation status
> > + * @retval 0 success
> > + * @retval !0 error
> > + */
> > +int
> > +rte_red_set_scaling(uint16_t max_red_queue_length);
> > +
> > +/**
> > + * @brief Reset the RED scaling factor - only for use by RED unit-tests
> > + *
> > + * @return Operation status
> > + */
> > +void
> > +rte_red_reset_scaling(void);
> > +
> > +/**
> >   * @brief Generate random number for RED
> >   *
> >   * Implemenetation based on:
> > @@ -206,7 +245,7 @@ __rte_red_calc_qempty_factor(uint8_t wq_log2,
> > uint16_t m)
> >  	f = (n >> 6) & 0xf;
> >  	n >>= 10;
> >
> > -	if (n < RTE_RED_SCALING)
> > +	if (n < rte_red_scaling)
> >  		return (uint16_t) ((rte_red_pow2_frac_inv[f] + (1 << (n - 1)))
> > >> n);
> >
> >  	return 0;
> > @@ -258,7 +297,9 @@ rte_red_enqueue_empty(const struct
> > rte_red_config *red_cfg,
> >  	if (m >= RTE_RED_2POW16) {
> >  		red->avg = 0;
> >  	} else {
> > -		red->avg = (red->avg >> RTE_RED_SCALING) *
> > __rte_red_calc_qempty_factor(red_cfg->wq_log2, (uint16_t) m);
> > +		red->avg = (red->avg >> rte_red_scaling) *
> > +			__rte_red_calc_qempty_factor(red_cfg->wq_log2,
> > +						     (uint16_t) m);
> >  	}
> >
> >  	return 0;
> > @@ -365,7 +406,7 @@ rte_red_enqueue_nonempty(const struct
> > rte_red_config *red_cfg,
> >  	*/
> >
> >  	/* avg update */
> > -	red->avg += (q << RTE_RED_SCALING) - (red->avg >> red_cfg-
> > >wq_log2);
> > +	red->avg += (q << rte_red_scaling) - (red->avg >> red_cfg-
> > >wq_log2);
> >
> >  	/* avg < min_th: do not mark the packet  */
> >  	if (red->avg < red_cfg->min_th) {
> > diff --git a/lib/librte_sched/rte_sched_version.map
> > b/lib/librte_sched/rte_sched_version.map
> > index 3aa159a..92e51f8 100644
> > --- a/lib/librte_sched/rte_sched_version.map
> > +++ b/lib/librte_sched/rte_sched_version.map
> > @@ -29,3 +29,9 @@ DPDK_2.1 {
> >  	rte_sched_port_pkt_read_color;
> >
> >  } DPDK_2.0;
> > +
> > +DPDK_17.08 {
> > +	global;
> > +
> > +	rte_red_set_scaling;
> > +} DPDK_2.1;
> > diff --git a/test/test/test_red.c b/test/test/test_red.c
> > index 348075d..f2d50ef 100644
> > --- a/test/test/test_red.c
> > +++ b/test/test/test_red.c
> > @@ -67,6 +67,7 @@ struct test_rte_red_config {        /**< Test structure
> for
> > RTE_RED config */
> >  	uint32_t min_th;                /**< Queue minimum threshold */
> >  	uint32_t max_th;                /**< Queue maximum threshold */
> >  	uint8_t *maxp_inv;              /**< Inverse mark probability */
> > +	uint16_t max_queue_len;         /**< Maximum queue length */
> >  };
> >
> >  struct test_queue {                 /**< Test structure for RTE_RED Queues */
> > @@ -181,7 +182,7 @@ static uint32_t rte_red_get_avg_int(const struct
> > rte_red_config *red_cfg,
> >  	/**
> >  	 * scale by 1/n and convert from fixed-point to integer
> >  	 */
> > -	return red->avg >> (RTE_RED_SCALING + red_cfg->wq_log2);
> > +	return red->avg >> (rte_red_scaling + red_cfg->wq_log2);
> >  }
> >
> >  static double rte_red_get_avg_float(const struct rte_red_config *red_cfg,
> > @@ -190,7 +191,7 @@ static double rte_red_get_avg_float(const struct
> > rte_red_config *red_cfg,
> >  	/**
> >  	 * scale by 1/n and convert from fixed-point to floating-point
> >  	 */
> > -	return ldexp((double)red->avg,  -(RTE_RED_SCALING + red_cfg-
> > >wq_log2));
> > +	return ldexp((double)red->avg,  -(rte_red_scaling + red_cfg-
> > >wq_log2));
> >  }
> >
> >  static void rte_red_set_avg_int(const struct rte_red_config *red_cfg,
> > @@ -200,7 +201,7 @@ static void rte_red_set_avg_int(const struct
> > rte_red_config *red_cfg,
> >  	/**
> >  	 * scale by n and convert from integer to fixed-point
> >  	 */
> > -	red->avg = avg << (RTE_RED_SCALING + red_cfg->wq_log2);
> > +	red->avg = avg << (rte_red_scaling + red_cfg->wq_log2);
> >  }
> >
> >  static double calc_exp_avg_on_empty(double avg, uint32_t n, uint32_t
> > time_diff)
> > @@ -280,10 +281,23 @@ static enum test_result
> >  test_rte_red_init(struct test_config *tcfg)
> >  {
> >  	unsigned i = 0;
> > +	int ret;
> >
> >  	tcfg->tvar->clk_freq = rte_get_timer_hz();
> >  	init_port_ts( tcfg->tvar->clk_freq );
> >
> > +	rte_red_reset_scaling();
> > +	ret = rte_red_set_scaling(tcfg->tconfig->max_queue_len);
> > +	if (ret) {
> > +		printf("rte_red_set_scaling failed: %d\n", ret);
> > +		return FAIL;
> > +	}
> > +	if (rte_red_max_threshold < tcfg->tconfig->max_th) {
> > +		printf("rte_red_max_th (%u) < tconfig->max_th (%u)\n",
> > +		       rte_red_max_threshold, tcfg->tconfig->max_th);
> > +		return FAIL;
> > +	}
> > +
> >  	for (i = 0; i < tcfg->tconfig->num_cfg; i++) {
> >  		if (rte_red_config_init(&tcfg->tconfig->rconfig[i],
> >  					(uint16_t)tcfg->tconfig->wq_log2[i],
> > @@ -385,6 +399,7 @@ static struct test_rte_red_config ft_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 128,
> >  	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_queue ft_tqueue = {
> > @@ -554,6 +569,7 @@ static struct test_rte_red_config ft2_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 128,
> >  	.maxp_inv = ft2_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_config func_test2_config = {
> > @@ -576,6 +592,41 @@ static struct test_config func_test2_config = {
> >  	.tlevel = ft2_tlevel,
> >  };
> >
> > +/**
> > + * Test F2: functional test 2 - with long queues and smaller red-scaling
> factor
> > + */
> > +static uint32_t ft2_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft2_tconfig_scaling =  {
> > +	.rconfig = ft2_rconfig,
> > +	.num_cfg = RTE_DIM(ft2_rconfig),
> > +	.wq_log2 = ft2_wq_log2,
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.maxp_inv = ft2_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test2_config_scaling = {
> > +	.ifname = "functional test 2 interface",
> > +	.msg = "functional test 2 : use several RED configurations and long
> > queues,\n"
> > +	"		    increase average queue size to just below
> > maximum threshold,\n"
> > +	"		    compare drop rate to drop probability\n\n",
> > +	.htxt = "RED config     "
> > +	"avg queue size "
> > +	"min threshold  "
> > +	"max threshold  "
> > +	"drop prob %    "
> > +	"drop rate %    "
> > +	"diff %         "
> > +	"tolerance %    "
> > +	"\n",
> > +	.tconfig = &ft2_tconfig_scaling,
> > +	.tqueue = &ft_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft2_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test2(struct test_config *tcfg)
> >  {
> >  	enum test_result result = PASS;
> > @@ -662,6 +713,7 @@ static struct test_rte_red_config ft3_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 1023,
> >  	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_config func_test3_config = {
> > @@ -683,6 +735,40 @@ static struct test_config func_test3_config = {
> >  	.tlevel = ft3_tlevel,
> >  };
> >
> > +/**
> > + * Test F3: functional test 3 - with large queues and smaller red scaling
> factor
> > + */
> > +static uint32_t ft3_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft3_tconfig_scaling =  {
> > +	.rconfig = ft_wrconfig,
> > +	.num_cfg = RTE_DIM(ft_wrconfig),
> > +	.wq_log2 = ft_wq_log2,
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test3_config_scaling = {
> > +	.ifname = "functional test 3 interface",
> > +	.msg = "functional test 3 : use one RED configuration and long
> > queues,\n"
> > +	"		    increase average queue size to target level,\n"
> > +	"		    dequeue all packets until queue is empty,\n"
> > +	"		    confirm that average queue size is computed
> > correctly while queue is empty\n\n",
> > +	.htxt = "q avg before   "
> > +	"q avg after    "
> > +	"expected       "
> > +	"difference %   "
> > +	"tolerance %    "
> > +	"result	 "
> > +	"\n",
> > +	.tconfig = &ft3_tconfig_scaling,
> > +	.tqueue = &ft_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft3_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test3(struct test_config *tcfg)
> >  {
> >  	enum test_result result = PASS;
> > @@ -776,6 +862,7 @@ static struct test_rte_red_config ft4_tconfig =  {
> >  	.max_th = 1023,
> >  	.wq_log2 = ft4_wq_log2,
> >  	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_queue ft4_tqueue = {
> > @@ -810,6 +897,42 @@ static struct test_config func_test4_config = {
> >  	.tlevel = ft4_tlevel,
> >  };
> >
> > +/**
> > + * Test F4: functional test 4
> > + */
> > +static uint32_t ft4_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft4_tconfig_scaling =  {
> > +	.rconfig = ft_wrconfig,
> > +	.num_cfg = RTE_DIM(ft_wrconfig),
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.wq_log2 = ft4_wq_log2,
> > +	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test4_config_scaling = {
> > +	.ifname = "functional test 4 interface",
> > +	.msg = "functional test 4 : use one RED configuration on long
> > queue,\n"
> > +	"		    increase average queue size to target level,\n"
> > +	"		    dequeue all packets until queue is empty,\n"
> > +	"		    confirm that average queue size is computed
> > correctly while\n"
> > +	"		    queue is empty for more than 50 sec,\n"
> > +	"		    (this test takes 52 sec to run)\n\n",
> > +	.htxt = "q avg before   "
> > +	"q avg after    "
> > +	"expected       "
> > +	"difference %   "
> > +	"tolerance %    "
> > +	"result	 "
> > +	"\n",
> > +	.tconfig = &ft4_tconfig_scaling,
> > +	.tqueue = &ft4_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft4_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test4(struct test_config *tcfg)
> >  {
> >  	enum test_result result = PASS;
> > @@ -924,6 +1047,7 @@ static struct test_rte_red_config ft5_tconfig =  {
> >  	.max_th = 128,
> >  	.wq_log2 = ft5_wq_log2,
> >  	.maxp_inv = ft5_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_queue ft5_tqueue = {
> > @@ -970,6 +1094,45 @@ static struct test_config func_test5_config = {
> >  	.tlevel = ft5_tlevel,
> >  };
> >
> > +
> > +/**
> > + * Test F5: functional test 5
> > + */
> > +static uint32_t ft5_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft5_tconfig_scaling =  {
> > +	.rconfig = ft5_config,
> > +	.num_cfg = RTE_DIM(ft5_config),
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.wq_log2 = ft5_wq_log2,
> > +	.maxp_inv = ft5_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test5_config_scaling = {
> > +	.ifname = "functional test 5 interface",
> > +	.msg = "functional test 5 : use several long queues (each with its
> own
> > run-time data),\n"
> > +	"		    use several RED configurations (such that each
> > configuration is shared by multiple queues),\n"
> > +	"		    increase average queue size to just below
> > maximum threshold,\n"
> > +	"		    compare drop rate to drop probability,\n"
> > +	"		    (this is a larger scale version of functional test
> > 2)\n\n",
> > +	.htxt = "queue          "
> > +	"config         "
> > +	"avg queue size "
> > +	"min threshold  "
> > +	"max threshold  "
> > +	"drop prob %    "
> > +	"drop rate %    "
> > +	"diff %         "
> > +	"tolerance %    "
> > +	"\n",
> > +	.tconfig = &ft5_tconfig_scaling,
> > +	.tqueue = &ft5_tqueue,
> > +	.tvar = &ft5_tvar,
> > +	.tlevel = ft5_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test5(struct test_config *tcfg)
> >  {
> >  	enum test_result result = PASS;
> > @@ -1062,6 +1225,7 @@ static struct test_rte_red_config ft6_tconfig =  {
> >  	.max_th = 1023,
> >  	.wq_log2 = ft6_wq_log2,
> >  	.maxp_inv = ft6_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_queue ft6_tqueue = {
> > @@ -1078,7 +1242,7 @@ static struct test_queue ft6_tqueue = {
> >  static struct test_config func_test6_config = {
> >  	.ifname = "functional test 6 interface",
> >  	.msg = "functional test 6 : use several queues (each with its own
> run-
> > time data),\n"
> > -	"		    use several RED configurations (such that each
> > configuration is sharte_red by multiple queues),\n"
> > +	"		    use several RED configurations (such that each
> > configuration is shared by multiple queues),\n"
> >  	"		    increase average queue size to target level,\n"
> >  	"		    dequeue all packets until queue is empty,\n"
> >  	"		    confirm that average queue size is computed
> > correctly while queue is empty\n"
> > @@ -1097,6 +1261,44 @@ static struct test_config func_test6_config = {
> >  	.tlevel = ft6_tlevel,
> >  };
> >
> > +/**
> > + * Test F6: functional test 6
> > + */
> > +static uint32_t ft6_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft6_tconfig_scaling =  {
> > +	.rconfig = ft6_config,
> > +	.num_cfg = RTE_DIM(ft6_config),
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.wq_log2 = ft6_wq_log2,
> > +	.maxp_inv = ft6_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test6_config_scaling = {
> > +	.ifname = "functional test 6 interface",
> > +	.msg = "functional test 6 : use several long queues (each with its
> own
> > run-time data),\n"
> > +	"		    use several RED configurations (such that each
> > configuration is shared by multiple queues),\n"
> > +	"		    increase average queue size to target level,\n"
> > +	"		    dequeue all packets until queue is empty,\n"
> > +	"		    confirm that average queue size is computed
> > correctly while queue is empty\n"
> > +	"		    (this is a larger scale version of functional test
> > 3)\n\n",
> > +	.htxt = "queue          "
> > +	"config         "
> > +	"q avg before   "
> > +	"q avg after    "
> > +	"expected       "
> > +	"difference %   "
> > +	"tolerance %    "
> > +	"result	 "
> > +	"\n",
> > +	.tconfig = &ft6_tconfig_scaling,
> > +	.tqueue = &ft6_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft6_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test6(struct test_config *tcfg)
> >  {
> >  	enum test_result result = PASS;
> > @@ -1195,6 +1397,7 @@ static struct test_rte_red_config pt_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 128,
> >  	.maxp_inv = pt_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_queue pt_tqueue = {
> > @@ -1557,6 +1760,7 @@ static struct test_rte_red_config ovfl_tconfig =
> {
> >  	.min_th = 32,
> >  	.max_th = 1023,
> >  	.maxp_inv = ovfl_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> >
> >  static struct test_queue ovfl_tqueue = {
> > @@ -1598,7 +1802,7 @@ static struct test_config ovfl_test1_config = {
> >  	.ifname = "queue avergage overflow test interface",
> >  	.msg = "overflow test 1 : use one RED configuration,\n"
> >  	"		  increase average queue size to target level,\n"
> > -	"		  check maximum number of bits requirte_red to
> > represent avg_s\n\n",
> > +	"		  check maximum number of bits required to
> > represent avg_s\n\n",
> >  	.htxt = "avg queue size  "
> >  	"wq_log2  "
> >  	"fraction bits  "
> > @@ -1615,6 +1819,39 @@ static struct test_config ovfl_test1_config = {
> >  	.tlevel = ovfl_tlevel,
> >  };
> >
> > +static uint32_t ovfl_tlevel_scaling[] = {8191};
> > +
> > +static struct test_rte_red_config ovfl_tconfig_scaling =  {
> > +	.rconfig = ovfl_wrconfig,
> > +	.num_cfg = RTE_DIM(ovfl_wrconfig),
> > +	.wq_log2 = ovfl_wq_log2,
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.maxp_inv = ovfl_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config ovfl_test1_config_scaling = {
> > +	.ifname = "queue average overflow test interface",
> > +	.msg = "overflow test 1 on long queue: use one RED
> > configuration,\n"
> > +	"		  increase average queue size to target level,\n"
> > +	"		  check maximum number of bits required to
> > represent avg_s\n\n",
> > +	.htxt = "avg queue size  "
> > +	"wq_log2  "
> > +	"fraction bits  "
> > +	"max queue avg  "
> > +	"num bits  "
> > +	"enqueued  "
> > +	"dropped   "
> > +	"drop prob %  "
> > +	"drop rate %  "
> > +	"\n",
> > +	.tconfig = &ovfl_tconfig_scaling,
> > +	.tqueue = &ovfl_tqueue,
> > +	.tvar = &ovfl_tvar,
> > +	.tlevel = ovfl_tlevel_scaling,
> > +};
> > +
> >  static enum test_result ovfl_test1(struct test_config *tcfg)
> >  {
> >  	enum test_result result = PASS;
> > @@ -1690,7 +1927,7 @@ static enum test_result ovfl_test1(struct
> > test_config *tcfg)
> >  	printf("%s", tcfg->htxt);
> >
> >  	printf("%-16u%-9u%-15u0x%08x     %-10u%-10u%-10u%-13.2lf%-
> > 13.2lf\n",
> > -	       avg, *tcfg->tconfig->wq_log2, RTE_RED_SCALING,
> > +	       avg, *tcfg->tconfig->wq_log2, rte_red_scaling,
> >  	       avg_max, avg_max_bits,
> >  	       *tcfg->tvar->enqueued, *tcfg->tvar->dropped,
> >  	       drop_prob * 100.0, drop_rate * 100.0);
> > @@ -1730,6 +1967,15 @@ struct tests perf_tests[] = {
> >  	{ &perf2_test6_config, perf2_test },
> >  };
> >
> > +struct tests scale_tests[] = {
> > +	{ &func_test2_config_scaling, func_test2 },
> > +	{ &func_test3_config_scaling, func_test3 },
> > +	{ &func_test4_config_scaling, func_test4 },
> > +	{ &func_test5_config_scaling, func_test5 },
> > +	{ &func_test6_config_scaling, func_test6 },
> > +	{ &ovfl_test1_config_scaling, ovfl_test1 },
> > +};
> > +
> >  /**
> >   * function to execute the required_red tests
> >   */
> > @@ -1866,6 +2112,20 @@ test_red_perf(void)
> >  }
> >
> >  static int
> > +test_red_scaling(void)
> > +{
> > +	uint32_t num_tests = 0;
> > +	uint32_t num_pass = 0;
> > +
> > +	if (test_invalid_parameters() < 0)
> > +		return -1;
> > +
> > +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> > &num_pass);
> > +	show_stats(num_tests, num_pass);
> > +	return tell_the_result(num_tests, num_pass);
> > +}
> > +
> > +static int
> >  test_red_all(void)
> >  {
> >  	uint32_t num_tests = 0;
> > @@ -1876,10 +2136,12 @@ test_red_all(void)
> >
> >  	run_tests(func_tests, RTE_DIM(func_tests), &num_tests,
> > &num_pass);
> >  	run_tests(perf_tests, RTE_DIM(perf_tests), &num_tests,
> > &num_pass);
> > +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> > &num_pass);
> >  	show_stats(num_tests, num_pass);
> >  	return tell_the_result(num_tests, num_pass);
> >  }
> >
> >  REGISTER_TEST_COMMAND(red_autotest, test_red);
> >  REGISTER_TEST_COMMAND(red_perf, test_red_perf);
> > +REGISTER_TEST_COMMAND(red_scaling, test_red_scaling);
> >  REGISTER_TEST_COMMAND(red_all, test_red_all);
> > --
> > 2.1.4

Tomasz,

Any feedback from you?

Regards,
Cristian

Re: [PATCH v4] sched: make RED scaling configurable

[Dumitrescu, Cristian]

Hi Alan,

Thanks for your work!

I do have some comments (see below), but generally looks good to me.

> -----Original Message-----
> From: alangordondewar@gmail.com [mailto:alangordondewar@gmail.com]
> Sent: Tuesday, October 3, 2017 10:22 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
> Subject: [PATCH v4] sched: make RED scaling configurable
> 
> From: Alan Dewar <alan.dewar@att.com>
> 
> The RED code stores the weighted moving average in a 32-bit integer as
> a pseudo fixed-point floating number with 10 fractional bits.  Twelve
> other bits are used to encode the filter weight, leaving just 10 bits
> for the queue length.  This limits the maximum queue length supported
> by RED queues to 1024 packets.
> 
> Introduce a new API to allow the RED scaling factor to be configured
> based upon maximum queue length.  If this API is not called, the RED
> scaling factor remains at its default value.
> 
> Added some new RED scaling unit-tests to test with RED queue-lengths
> up to 8192 packets long.
> 
> Signed-off-by: Alan Dewar <alan.dewar@att.com>
> ---
>  lib/librte_sched/rte_red.c             |  53 ++++++-
>  lib/librte_sched/rte_red.h             |  63 ++++++--
>  lib/librte_sched/rte_sched_version.map |   6 +
>  test/test/test_red.c                   | 274
> ++++++++++++++++++++++++++++++++-
>  4 files changed, 374 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
> index ade57d1..0dc8d28 100644
> --- a/lib/librte_sched/rte_red.c
> +++ b/lib/librte_sched/rte_red.c
> @@ -43,6 +43,8 @@
>  static int rte_red_init_done = 0;     /**< Flag to indicate that global
> initialisation is done */
>  uint32_t rte_red_rand_val = 0;        /**< Random value cache */
>  uint32_t rte_red_rand_seed = 0;       /**< Seed for random number
> generation */
> +uint8_t rte_red_scaling = RTE_RED_SCALING_DEFAULT;
> +uint16_t rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH -
> 1;
> 
>  /**
>   * table[i] = log2(1-Wq) * Scale * -1
> @@ -66,7 +68,7 @@ __rte_red_init_tables(void)
>  	double scale = 0.0;
>  	double table_size = 0.0;
> 
> -	scale = (double)(1 << RTE_RED_SCALING);
> +	scale = (double)(1 << rte_red_scaling);
>  	table_size = (double)(RTE_DIM(rte_red_pow2_frac_inv));
> 
>  	for (i = 0; i < RTE_DIM(rte_red_pow2_frac_inv); i++) {
> @@ -119,7 +121,7 @@ rte_red_config_init(struct rte_red_config *red_cfg,
>  	if (red_cfg == NULL) {
>  		return -1;
>  	}
> -	if (max_th > RTE_RED_MAX_TH_MAX) {
> +	if (max_th > rte_red_max_threshold) {
>  		return -2;
>  	}
>  	if (min_th >= max_th) {
> @@ -148,11 +150,52 @@ rte_red_config_init(struct rte_red_config
> *red_cfg,
>  		rte_red_init_done = 1;
>  	}
> 
> -	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> RTE_RED_SCALING);
> -	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> RTE_RED_SCALING);
> -	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> RTE_RED_SCALING;
> +	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> rte_red_scaling);
> +	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> rte_red_scaling);
> +	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> +		rte_red_scaling;
>  	red_cfg->maxp_inv = maxp_inv;
>  	red_cfg->wq_log2 = wq_log2;
> 
>  	return 0;
>  }
> +
> +int
> +rte_red_set_scaling(uint16_t max_red_queue_length)
> +{
> +	int8_t count;
> +
> +	if (rte_red_init_done)
> +		/**
> +		 * Can't change the scaling once the red table has been
> +		 * computed.
> +		 */
> +		return -1;
> +
> +	if (max_red_queue_length < RTE_RED_MIN_QUEUE_LENGTH)
> +		return -2;
> +
> +	if (max_red_queue_length > RTE_RED_MAX_QUEUE_LENGTH)
> +		return -3;
> +
> +	if (!rte_is_power_of_2(max_red_queue_length))
> +		return -4;
> +
> +	count = 0;
> +	while (max_red_queue_length != 0) {
> +		max_red_queue_length >>= 1;
> +		count++;
> +	}
> +
> +	rte_red_scaling -= count - RTE_RED_SCALING_DEFAULT;
> +	rte_red_max_threshold = max_red_queue_length - 1;
> +	return 0;
> +}
> +
> +void
> +rte_red_reset_scaling(void)
> +{
> +	rte_red_init_done = 0;
> +	rte_red_scaling = RTE_RED_SCALING_DEFAULT;
> +	rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH - 1;
> +}

Why do we need this function? These global variables are already initialized at the top of the file.

My vote is to remove it.


> diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h
> index ca12227..be1fb0f 100644
> --- a/lib/librte_sched/rte_red.h
> +++ b/lib/librte_sched/rte_red.h
> @@ -52,14 +52,31 @@ extern "C" {
>  #include <rte_cycles.h>
>  #include <rte_branch_prediction.h>
> 
> -#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-
> point */
> -#define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied
> by number of leaf queues */
> -#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold
> limit in fixed point format */
> -#define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter
> weight value */
> -#define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse
> filter weight value */
> -#define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark
> probability value */
> -#define RTE_RED_MAXP_INV_MAX                255        /**< Max inverse
> mark probability value */
> -#define RTE_RED_2POW16                      (1<<16)    /**< 2 power 16 */
> +/**< Default fraction size for fixed-point */
> +#define RTE_RED_SCALING_DEFAULT             10

Let's keep the name of this macro undchanges, i.e. RTE_RED_SCALING. IMO it is ckear enough this is the default value (also stated in the comment).

> +
> +/**< Packet size multiplied by number of leaf queues */
> +#define RTE_RED_S                           (1 << 22)
> +
> +/**< Minimum, default and maximum RED queue length */
> +#define RTE_RED_MIN_QUEUE_LENGTH            64
> +#define RTE_RED_DEFAULT_QUEUE_LENGTH        1024
> +#define RTE_RED_MAX_QUEUE_LENGTH            8192
> +
> +/**< Min inverse filter weight value */
> +#define RTE_RED_WQ_LOG2_MIN                 1
> +
> +/**< Max inverse filter weight value */
> +#define RTE_RED_WQ_LOG2_MAX                 12
> +
> +/**< Min inverse mark probability value */
> +#define RTE_RED_MAXP_INV_MIN                1
> +
> +/**< Max inverse mark probability value */
> +#define RTE_RED_MAXP_INV_MAX                255
> +
> +/**< 2 power 16 */
> +#define RTE_RED_2POW16                      (1<<16)
>  #define RTE_RED_INT16_NBITS                 (sizeof(uint16_t) * CHAR_BIT)
>  #define RTE_RED_WQ_LOG2_NUM                 (RTE_RED_WQ_LOG2_MAX -
> RTE_RED_WQ_LOG2_MIN + 1)
> 

All the above Doxygen comments are incorrectly formatted: as you want to change their behavior to refer to the variable _after_ the comment, they have to start with /**, not with /**< (which should be used if the comment refers to the variable _before_ the comment). Please fix.


> @@ -71,6 +88,8 @@ extern uint32_t rte_red_rand_val;
>  extern uint32_t rte_red_rand_seed;
>  extern uint16_t rte_red_log2_1_minus_Wq[RTE_RED_WQ_LOG2_NUM];
>  extern uint16_t rte_red_pow2_frac_inv[16];
> +extern uint8_t rte_red_scaling;
> +extern uint16_t rte_red_max_threshold;
> 
>  /**
>   * RED configuration parameters passed by user
> @@ -137,6 +156,26 @@ rte_red_config_init(struct rte_red_config *red_cfg,
>  	const uint16_t maxp_inv);
> 
>  /**
> + * @brief Configures the global setting for the RED scaling factor
> + *
> + * @param max_red_queue_length [in] must be a power of two
> + *
> + * @return Operation status
> + * @retval 0 success
> + * @retval !0 error
> + */
> +int
> +rte_red_set_scaling(uint16_t max_red_queue_length);
> +
> +/**
> + * @brief Reset the RED scaling factor - only for use by RED unit-tests
> + *
> + * @return Operation status
> + */
> +void
> +rte_red_reset_scaling(void);

As stated above, this function is probably not useful and my vote is to remove it.

> +
> +/**
>   * @brief Generate random number for RED
>   *
>   * Implemenetation based on:
> @@ -206,7 +245,7 @@ __rte_red_calc_qempty_factor(uint8_t wq_log2,
> uint16_t m)
>  	f = (n >> 6) & 0xf;
>  	n >>= 10;
> 
> -	if (n < RTE_RED_SCALING)
> +	if (n < rte_red_scaling)
>  		return (uint16_t) ((rte_red_pow2_frac_inv[f] + (1 << (n - 1)))
> >> n);
> 
>  	return 0;
> @@ -258,7 +297,9 @@ rte_red_enqueue_empty(const struct rte_red_config
> *red_cfg,
>  	if (m >= RTE_RED_2POW16) {
>  		red->avg = 0;
>  	} else {
> -		red->avg = (red->avg >> RTE_RED_SCALING) *
> __rte_red_calc_qempty_factor(red_cfg->wq_log2, (uint16_t) m);
> +		red->avg = (red->avg >> rte_red_scaling) *
> +			__rte_red_calc_qempty_factor(red_cfg->wq_log2,
> +						     (uint16_t) m);
>  	}
> 
>  	return 0;
> @@ -365,7 +406,7 @@ rte_red_enqueue_nonempty(const struct
> rte_red_config *red_cfg,
>  	*/
> 
>  	/* avg update */
> -	red->avg += (q << RTE_RED_SCALING) - (red->avg >> red_cfg-
> >wq_log2);
> +	red->avg += (q << rte_red_scaling) - (red->avg >> red_cfg-
> >wq_log2);
> 
>  	/* avg < min_th: do not mark the packet  */
>  	if (red->avg < red_cfg->min_th) {
> diff --git a/lib/librte_sched/rte_sched_version.map
> b/lib/librte_sched/rte_sched_version.map
> index 3aa159a..92e51f8 100644
> --- a/lib/librte_sched/rte_sched_version.map
> +++ b/lib/librte_sched/rte_sched_version.map
> @@ -29,3 +29,9 @@ DPDK_2.1 {
>  	rte_sched_port_pkt_read_color;
> 
>  } DPDK_2.0;
> +
> +DPDK_17.08 {
> +	global;
> +
> +	rte_red_set_scaling;
> +} DPDK_2.1;
> diff --git a/test/test/test_red.c b/test/test/test_red.c
> index 348075d..f2d50ef 100644
> --- a/test/test/test_red.c
> +++ b/test/test/test_red.c
> @@ -67,6 +67,7 @@ struct test_rte_red_config {        /**< Test structure
> for RTE_RED config */
>  	uint32_t min_th;                /**< Queue minimum threshold */
>  	uint32_t max_th;                /**< Queue maximum threshold */
>  	uint8_t *maxp_inv;              /**< Inverse mark probability */
> +	uint16_t max_queue_len;         /**< Maximum queue length */
>  };
> 
>  struct test_queue {                 /**< Test structure for RTE_RED Queues */
> @@ -181,7 +182,7 @@ static uint32_t rte_red_get_avg_int(const struct
> rte_red_config *red_cfg,
>  	/**
>  	 * scale by 1/n and convert from fixed-point to integer
>  	 */
> -	return red->avg >> (RTE_RED_SCALING + red_cfg->wq_log2);
> +	return red->avg >> (rte_red_scaling + red_cfg->wq_log2);
>  }
> 
>  static double rte_red_get_avg_float(const struct rte_red_config *red_cfg,
> @@ -190,7 +191,7 @@ static double rte_red_get_avg_float(const struct
> rte_red_config *red_cfg,
>  	/**
>  	 * scale by 1/n and convert from fixed-point to floating-point
>  	 */
> -	return ldexp((double)red->avg,  -(RTE_RED_SCALING + red_cfg-
> >wq_log2));
> +	return ldexp((double)red->avg,  -(rte_red_scaling + red_cfg-
> >wq_log2));
>  }
> 
>  static void rte_red_set_avg_int(const struct rte_red_config *red_cfg,
> @@ -200,7 +201,7 @@ static void rte_red_set_avg_int(const struct
> rte_red_config *red_cfg,
>  	/**
>  	 * scale by n and convert from integer to fixed-point
>  	 */
> -	red->avg = avg << (RTE_RED_SCALING + red_cfg->wq_log2);
> +	red->avg = avg << (rte_red_scaling + red_cfg->wq_log2);
>  }
> 
>  static double calc_exp_avg_on_empty(double avg, uint32_t n, uint32_t
> time_diff)
> @@ -280,10 +281,23 @@ static enum test_result
>  test_rte_red_init(struct test_config *tcfg)
>  {
>  	unsigned i = 0;
> +	int ret;
> 
>  	tcfg->tvar->clk_freq = rte_get_timer_hz();
>  	init_port_ts( tcfg->tvar->clk_freq );
> 
> +	rte_red_reset_scaling();
> +	ret = rte_red_set_scaling(tcfg->tconfig->max_queue_len);
> +	if (ret) {
> +		printf("rte_red_set_scaling failed: %d\n", ret);
> +		return FAIL;
> +	}
> +	if (rte_red_max_threshold < tcfg->tconfig->max_th) {
> +		printf("rte_red_max_th (%u) < tconfig->max_th (%u)\n",
> +		       rte_red_max_threshold, tcfg->tconfig->max_th);
> +		return FAIL;
> +	}
> +
>  	for (i = 0; i < tcfg->tconfig->num_cfg; i++) {
>  		if (rte_red_config_init(&tcfg->tconfig->rconfig[i],
>  					(uint16_t)tcfg->tconfig->wq_log2[i],
> @@ -385,6 +399,7 @@ static struct test_rte_red_config ft_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 128,
>  	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft_tqueue = {
> @@ -554,6 +569,7 @@ static struct test_rte_red_config ft2_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 128,
>  	.maxp_inv = ft2_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_config func_test2_config = {
> @@ -576,6 +592,41 @@ static struct test_config func_test2_config = {
>  	.tlevel = ft2_tlevel,
>  };
> 
> +/**
> + * Test F2: functional test 2 - with long queues and smaller red-scaling
> factor
> + */
> +static uint32_t ft2_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft2_tconfig_scaling =  {
> +	.rconfig = ft2_rconfig,
> +	.num_cfg = RTE_DIM(ft2_rconfig),
> +	.wq_log2 = ft2_wq_log2,
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.maxp_inv = ft2_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test2_config_scaling = {
> +	.ifname = "functional test 2 interface",
> +	.msg = "functional test 2 : use several RED configurations and long
> queues,\n"
> +	"		    increase average queue size to just below
> maximum threshold,\n"
> +	"		    compare drop rate to drop probability\n\n",
> +	.htxt = "RED config     "
> +	"avg queue size "
> +	"min threshold  "
> +	"max threshold  "
> +	"drop prob %    "
> +	"drop rate %    "
> +	"diff %         "
> +	"tolerance %    "
> +	"\n",
> +	.tconfig = &ft2_tconfig_scaling,
> +	.tqueue = &ft_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft2_tlevel_scaling,
> +};
> +
>  static enum test_result func_test2(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -662,6 +713,7 @@ static struct test_rte_red_config ft3_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 1023,
>  	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_config func_test3_config = {
> @@ -683,6 +735,40 @@ static struct test_config func_test3_config = {
>  	.tlevel = ft3_tlevel,
>  };
> 
> +/**
> + * Test F3: functional test 3 - with large queues and smaller red scaling
> factor
> + */
> +static uint32_t ft3_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft3_tconfig_scaling =  {
> +	.rconfig = ft_wrconfig,
> +	.num_cfg = RTE_DIM(ft_wrconfig),
> +	.wq_log2 = ft_wq_log2,
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test3_config_scaling = {
> +	.ifname = "functional test 3 interface",
> +	.msg = "functional test 3 : use one RED configuration and long
> queues,\n"
> +	"		    increase average queue size to target level,\n"
> +	"		    dequeue all packets until queue is empty,\n"
> +	"		    confirm that average queue size is computed
> correctly while queue is empty\n\n",
> +	.htxt = "q avg before   "
> +	"q avg after    "
> +	"expected       "
> +	"difference %   "
> +	"tolerance %    "
> +	"result	 "
> +	"\n",
> +	.tconfig = &ft3_tconfig_scaling,
> +	.tqueue = &ft_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft3_tlevel_scaling,
> +};
> +
>  static enum test_result func_test3(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -776,6 +862,7 @@ static struct test_rte_red_config ft4_tconfig =  {
>  	.max_th = 1023,
>  	.wq_log2 = ft4_wq_log2,
>  	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft4_tqueue = {
> @@ -810,6 +897,42 @@ static struct test_config func_test4_config = {
>  	.tlevel = ft4_tlevel,
>  };
> 
> +/**
> + * Test F4: functional test 4
> + */
> +static uint32_t ft4_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft4_tconfig_scaling =  {
> +	.rconfig = ft_wrconfig,
> +	.num_cfg = RTE_DIM(ft_wrconfig),
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.wq_log2 = ft4_wq_log2,
> +	.maxp_inv = ft_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test4_config_scaling = {
> +	.ifname = "functional test 4 interface",
> +	.msg = "functional test 4 : use one RED configuration on long
> queue,\n"
> +	"		    increase average queue size to target level,\n"
> +	"		    dequeue all packets until queue is empty,\n"
> +	"		    confirm that average queue size is computed
> correctly while\n"
> +	"		    queue is empty for more than 50 sec,\n"
> +	"		    (this test takes 52 sec to run)\n\n",
> +	.htxt = "q avg before   "
> +	"q avg after    "
> +	"expected       "
> +	"difference %   "
> +	"tolerance %    "
> +	"result	 "
> +	"\n",
> +	.tconfig = &ft4_tconfig_scaling,
> +	.tqueue = &ft4_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft4_tlevel_scaling,
> +};
> +
>  static enum test_result func_test4(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -924,6 +1047,7 @@ static struct test_rte_red_config ft5_tconfig =  {
>  	.max_th = 128,
>  	.wq_log2 = ft5_wq_log2,
>  	.maxp_inv = ft5_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft5_tqueue = {
> @@ -970,6 +1094,45 @@ static struct test_config func_test5_config = {
>  	.tlevel = ft5_tlevel,
>  };
> 
> +
> +/**
> + * Test F5: functional test 5
> + */
> +static uint32_t ft5_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft5_tconfig_scaling =  {
> +	.rconfig = ft5_config,
> +	.num_cfg = RTE_DIM(ft5_config),
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.wq_log2 = ft5_wq_log2,
> +	.maxp_inv = ft5_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test5_config_scaling = {
> +	.ifname = "functional test 5 interface",
> +	.msg = "functional test 5 : use several long queues (each with its
> own run-time data),\n"
> +	"		    use several RED configurations (such that each
> configuration is shared by multiple queues),\n"
> +	"		    increase average queue size to just below
> maximum threshold,\n"
> +	"		    compare drop rate to drop probability,\n"
> +	"		    (this is a larger scale version of functional test
> 2)\n\n",
> +	.htxt = "queue          "
> +	"config         "
> +	"avg queue size "
> +	"min threshold  "
> +	"max threshold  "
> +	"drop prob %    "
> +	"drop rate %    "
> +	"diff %         "
> +	"tolerance %    "
> +	"\n",
> +	.tconfig = &ft5_tconfig_scaling,
> +	.tqueue = &ft5_tqueue,
> +	.tvar = &ft5_tvar,
> +	.tlevel = ft5_tlevel_scaling,
> +};
> +
>  static enum test_result func_test5(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -1062,6 +1225,7 @@ static struct test_rte_red_config ft6_tconfig =  {
>  	.max_th = 1023,
>  	.wq_log2 = ft6_wq_log2,
>  	.maxp_inv = ft6_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ft6_tqueue = {
> @@ -1078,7 +1242,7 @@ static struct test_queue ft6_tqueue = {
>  static struct test_config func_test6_config = {
>  	.ifname = "functional test 6 interface",
>  	.msg = "functional test 6 : use several queues (each with its own
> run-time data),\n"
> -	"		    use several RED configurations (such that each
> configuration is sharte_red by multiple queues),\n"
> +	"		    use several RED configurations (such that each
> configuration is shared by multiple queues),\n"
>  	"		    increase average queue size to target level,\n"
>  	"		    dequeue all packets until queue is empty,\n"
>  	"		    confirm that average queue size is computed
> correctly while queue is empty\n"
> @@ -1097,6 +1261,44 @@ static struct test_config func_test6_config = {
>  	.tlevel = ft6_tlevel,
>  };
> 
> +/**
> + * Test F6: functional test 6
> + */
> +static uint32_t ft6_tlevel_scaling[] = {8190};
> +
> +static struct test_rte_red_config ft6_tconfig_scaling =  {
> +	.rconfig = ft6_config,
> +	.num_cfg = RTE_DIM(ft6_config),
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.wq_log2 = ft6_wq_log2,
> +	.maxp_inv = ft6_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config func_test6_config_scaling = {
> +	.ifname = "functional test 6 interface",
> +	.msg = "functional test 6 : use several long queues (each with its
> own run-time data),\n"
> +	"		    use several RED configurations (such that each
> configuration is shared by multiple queues),\n"
> +	"		    increase average queue size to target level,\n"
> +	"		    dequeue all packets until queue is empty,\n"
> +	"		    confirm that average queue size is computed
> correctly while queue is empty\n"
> +	"		    (this is a larger scale version of functional test
> 3)\n\n",
> +	.htxt = "queue          "
> +	"config         "
> +	"q avg before   "
> +	"q avg after    "
> +	"expected       "
> +	"difference %   "
> +	"tolerance %    "
> +	"result	 "
> +	"\n",
> +	.tconfig = &ft6_tconfig_scaling,
> +	.tqueue = &ft6_tqueue,
> +	.tvar = &ft_tvar,
> +	.tlevel = ft6_tlevel_scaling,
> +};
> +
>  static enum test_result func_test6(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -1195,6 +1397,7 @@ static struct test_rte_red_config pt_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 128,
>  	.maxp_inv = pt_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue pt_tqueue = {
> @@ -1557,6 +1760,7 @@ static struct test_rte_red_config ovfl_tconfig =  {
>  	.min_th = 32,
>  	.max_th = 1023,
>  	.maxp_inv = ovfl_maxp_inv,
> +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
>  };
> 
>  static struct test_queue ovfl_tqueue = {
> @@ -1598,7 +1802,7 @@ static struct test_config ovfl_test1_config = {
>  	.ifname = "queue avergage overflow test interface",
>  	.msg = "overflow test 1 : use one RED configuration,\n"
>  	"		  increase average queue size to target level,\n"
> -	"		  check maximum number of bits requirte_red to
> represent avg_s\n\n",
> +	"		  check maximum number of bits required to
> represent avg_s\n\n",
>  	.htxt = "avg queue size  "
>  	"wq_log2  "
>  	"fraction bits  "
> @@ -1615,6 +1819,39 @@ static struct test_config ovfl_test1_config = {
>  	.tlevel = ovfl_tlevel,
>  };
> 
> +static uint32_t ovfl_tlevel_scaling[] = {8191};
> +
> +static struct test_rte_red_config ovfl_tconfig_scaling =  {
> +	.rconfig = ovfl_wrconfig,
> +	.num_cfg = RTE_DIM(ovfl_wrconfig),
> +	.wq_log2 = ovfl_wq_log2,
> +	.min_th = 32,
> +	.max_th = 8191,
> +	.maxp_inv = ovfl_maxp_inv,
> +	.max_queue_len = 8192,
> +};
> +
> +static struct test_config ovfl_test1_config_scaling = {
> +	.ifname = "queue average overflow test interface",
> +	.msg = "overflow test 1 on long queue: use one RED
> configuration,\n"
> +	"		  increase average queue size to target level,\n"
> +	"		  check maximum number of bits required to
> represent avg_s\n\n",
> +	.htxt = "avg queue size  "
> +	"wq_log2  "
> +	"fraction bits  "
> +	"max queue avg  "
> +	"num bits  "
> +	"enqueued  "
> +	"dropped   "
> +	"drop prob %  "
> +	"drop rate %  "
> +	"\n",
> +	.tconfig = &ovfl_tconfig_scaling,
> +	.tqueue = &ovfl_tqueue,
> +	.tvar = &ovfl_tvar,
> +	.tlevel = ovfl_tlevel_scaling,
> +};
> +
>  static enum test_result ovfl_test1(struct test_config *tcfg)
>  {
>  	enum test_result result = PASS;
> @@ -1690,7 +1927,7 @@ static enum test_result ovfl_test1(struct
> test_config *tcfg)
>  	printf("%s", tcfg->htxt);
> 
>  	printf("%-16u%-9u%-15u0x%08x     %-10u%-10u%-10u%-13.2lf%-
> 13.2lf\n",
> -	       avg, *tcfg->tconfig->wq_log2, RTE_RED_SCALING,
> +	       avg, *tcfg->tconfig->wq_log2, rte_red_scaling,
>  	       avg_max, avg_max_bits,
>  	       *tcfg->tvar->enqueued, *tcfg->tvar->dropped,
>  	       drop_prob * 100.0, drop_rate * 100.0);
> @@ -1730,6 +1967,15 @@ struct tests perf_tests[] = {
>  	{ &perf2_test6_config, perf2_test },
>  };
> 
> +struct tests scale_tests[] = {
> +	{ &func_test2_config_scaling, func_test2 },
> +	{ &func_test3_config_scaling, func_test3 },
> +	{ &func_test4_config_scaling, func_test4 },
> +	{ &func_test5_config_scaling, func_test5 },
> +	{ &func_test6_config_scaling, func_test6 },
> +	{ &ovfl_test1_config_scaling, ovfl_test1 },
> +};
> +
>  /**
>   * function to execute the required_red tests
>   */
> @@ -1866,6 +2112,20 @@ test_red_perf(void)
>  }
> 
>  static int
> +test_red_scaling(void)
> +{
> +	uint32_t num_tests = 0;
> +	uint32_t num_pass = 0;
> +
> +	if (test_invalid_parameters() < 0)
> +		return -1;
> +
> +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> &num_pass);
> +	show_stats(num_tests, num_pass);
> +	return tell_the_result(num_tests, num_pass);
> +}
> +
> +static int
>  test_red_all(void)
>  {
>  	uint32_t num_tests = 0;
> @@ -1876,10 +2136,12 @@ test_red_all(void)
> 
>  	run_tests(func_tests, RTE_DIM(func_tests), &num_tests,
> &num_pass);
>  	run_tests(perf_tests, RTE_DIM(perf_tests), &num_tests,
> &num_pass);
> +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> &num_pass);
>  	show_stats(num_tests, num_pass);
>  	return tell_the_result(num_tests, num_pass);
>  }
> 
>  REGISTER_TEST_COMMAND(red_autotest, test_red);
>  REGISTER_TEST_COMMAND(red_perf, test_red_perf);
> +REGISTER_TEST_COMMAND(red_scaling, test_red_scaling);
>  REGISTER_TEST_COMMAND(red_all, test_red_all);
> --
> 2.1.4

Regards,
Cristian

Re: [PATCH v4] sched: make RED scaling configurable

[Dewar, Alan]

Hi Cristian,

> -----Original Message-----
> From: Dumitrescu, Cristian [mailto:cristian.dumitrescu@intel.com] 
> Sent: Tuesday, January 02, 2018 4:44 PM
> To: alangordondewar@gmail.com
> Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
> Subject: RE: [PATCH v4] sched: make RED scaling configurable
>
> Hi Alan,
>
> Thanks for your work!

Thanks for the review ;^) - my responses are in-line below.

Regards
Alan

> I do have some comments (see below), but generally looks good to me.
>
> > -----Original Message-----
> > From: alangordondewar@gmail.com [mailto:alangordondewar@gmail.com]
> > Sent: Tuesday, October 3, 2017 10:22 AM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>
> > Subject: [PATCH v4] sched: make RED scaling configurable
> > 
> > From: Alan Dewar <alan.dewar@att.com>
> > 
> > The RED code stores the weighted moving average in a 32-bit integer as 
> > a pseudo fixed-point floating number with 10 fractional bits.  Twelve 
> > other bits are used to encode the filter weight, leaving just 10 bits 
> > for the queue length.  This limits the maximum queue length supported 
> > by RED queues to 1024 packets.
> > 
> > Introduce a new API to allow the RED scaling factor to be configured 
> > based upon maximum queue length.  If this API is not called, the RED 
> > scaling factor remains at its default value.
> > 
> > Added some new RED scaling unit-tests to test with RED queue-lengths 
> > up to 8192 packets long.
> > 
> > Signed-off-by: Alan Dewar <alan.dewar@att.com>
> > ---
> >  lib/librte_sched/rte_red.c             |  53 ++++++-
> >  lib/librte_sched/rte_red.h             |  63 ++++++--
> >  lib/librte_sched/rte_sched_version.map |   6 +
> >  test/test/test_red.c                   | 274
> > ++++++++++++++++++++++++++++++++-
> >  4 files changed, 374 insertions(+), 22 deletions(-)
> > 
> > diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c 
> > index ade57d1..0dc8d28 100644
> > --- a/lib/librte_sched/rte_red.c
> > +++ b/lib/librte_sched/rte_red.c
> > @@ -43,6 +43,8 @@
> >  static int rte_red_init_done = 0;     /**< Flag to indicate that global
> > initialisation is done */
> >  uint32_t rte_red_rand_val = 0;        /**< Random value cache */
> >  uint32_t rte_red_rand_seed = 0;       /**< Seed for random number
> > generation */
> > +uint8_t rte_red_scaling = RTE_RED_SCALING_DEFAULT; uint16_t 
> > +rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH -
> > 1;
> > 
> >  /**
> >   * table[i] = log2(1-Wq) * Scale * -1 @@ -66,7 +68,7 @@ 
> > __rte_red_init_tables(void)
> >  	double scale = 0.0;
> >  	double table_size = 0.0;
> > 
> > -	scale = (double)(1 << RTE_RED_SCALING);
> > +	scale = (double)(1 << rte_red_scaling);
> >  	table_size = (double)(RTE_DIM(rte_red_pow2_frac_inv));
> > 
> >  	for (i = 0; i < RTE_DIM(rte_red_pow2_frac_inv); i++) { @@ -119,7 
> > +121,7 @@ rte_red_config_init(struct rte_red_config *red_cfg,
> >  	if (red_cfg == NULL) {
> >  		return -1;
> >  	}
> > -	if (max_th > RTE_RED_MAX_TH_MAX) {
> > +	if (max_th > rte_red_max_threshold) {
> >  		return -2;
> >  	}
> >  	if (min_th >= max_th) {
> > @@ -148,11 +150,52 @@ rte_red_config_init(struct rte_red_config 
> > *red_cfg,
> >  		rte_red_init_done = 1;
> >  	}
> > 
> > -	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> > RTE_RED_SCALING);
> > -	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> > RTE_RED_SCALING);
> > -	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> > RTE_RED_SCALING;
> > +	red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 +
> > rte_red_scaling);
> > +	red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 +
> > rte_red_scaling);
> > +	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> > +		rte_red_scaling;
> >  	red_cfg->maxp_inv = maxp_inv;
> >  	red_cfg->wq_log2 = wq_log2;
> > 
> >  	return 0;
> >  }
> > +
> > +int
> > +rte_red_set_scaling(uint16_t max_red_queue_length) {
> > +	int8_t count;
> > +
> > +	if (rte_red_init_done)
> > +		/**
> > +		 * Can't change the scaling once the red table has been
> > +		 * computed.
> > +		 */
> > +		return -1;
> > +
> > +	if (max_red_queue_length < RTE_RED_MIN_QUEUE_LENGTH)
> > +		return -2;
> > +
> > +	if (max_red_queue_length > RTE_RED_MAX_QUEUE_LENGTH)
> > +		return -3;
> > +
> > +	if (!rte_is_power_of_2(max_red_queue_length))
> > +		return -4;
> > +
> > +	count = 0;
> > +	while (max_red_queue_length != 0) {
> > +		max_red_queue_length >>= 1;
> > +		count++;
> > +	}
> > +
> > +	rte_red_scaling -= count - RTE_RED_SCALING_DEFAULT;
> > +	rte_red_max_threshold = max_red_queue_length - 1;
> > +	return 0;
> > +}
> > +
> > +void
> > +rte_red_reset_scaling(void)
> > +{
> > +	rte_red_init_done = 0;
> > +	rte_red_scaling = RTE_RED_SCALING_DEFAULT;
> > +	rte_red_max_threshold = RTE_RED_DEFAULT_QUEUE_LENGTH - 1; }
>
> Why do we need this function? These global variables are already initialized at the top of the file. 
>
> My vote is to remove it. 

It is needed by the revised unit-test program that has to reinitialize the RED code for each sub-test.

> > diff --git a/lib/librte_sched/rte_red.h b/lib/librte_sched/rte_red.h 
> > index ca12227..be1fb0f 100644
> > --- a/lib/librte_sched/rte_red.h
> > +++ b/lib/librte_sched/rte_red.h
> > @@ -52,14 +52,31 @@ extern "C" {
> >  #include <rte_cycles.h>
> >  #include <rte_branch_prediction.h>
> > 
> > -#define RTE_RED_SCALING                     10         /**< Fraction size for fixed-
> > point */
> > -#define RTE_RED_S                           (1 << 22)  /**< Packet size multiplied
> > by number of leaf queues */
> > -#define RTE_RED_MAX_TH_MAX                  1023       /**< Max threshold
> > limit in fixed point format */
> > -#define RTE_RED_WQ_LOG2_MIN                 1          /**< Min inverse filter
> > weight value */
> > -#define RTE_RED_WQ_LOG2_MAX                 12         /**< Max inverse
> > filter weight value */
> > -#define RTE_RED_MAXP_INV_MIN                1          /**< Min inverse mark
> > probability value */
> > -#define RTE_RED_MAXP_INV_MAX                255        /**< Max inverse
> > mark probability value */
> > -#define RTE_RED_2POW16                      (1<<16)    /**< 2 power 16 */
> > +/**< Default fraction size for fixed-point */
> > +#define RTE_RED_SCALING_DEFAULT             10
>
> Let's keep the name of this macro undchanges, i.e. RTE_RED_SCALING. IMO it is ckear enough this is the default value (also > stated in the comment).

Okay will do.

> > +
> > +/**< Packet size multiplied by number of leaf queues */
> > +#define RTE_RED_S                           (1 << 22)
> > +
> > +/**< Minimum, default and maximum RED queue length */
> > +#define RTE_RED_MIN_QUEUE_LENGTH            64
> > +#define RTE_RED_DEFAULT_QUEUE_LENGTH        1024
> > +#define RTE_RED_MAX_QUEUE_LENGTH            8192
> > +
> > +/**< Min inverse filter weight value */
> > +#define RTE_RED_WQ_LOG2_MIN                 1
> > +
> > +/**< Max inverse filter weight value */
> > +#define RTE_RED_WQ_LOG2_MAX                 12
> > +
> > +/**< Min inverse mark probability value */
> > +#define RTE_RED_MAXP_INV_MIN                1
> > +
> > +/**< Max inverse mark probability value */
> > +#define RTE_RED_MAXP_INV_MAX                255
> > +
> > +/**< 2 power 16 */
> > +#define RTE_RED_2POW16                      (1<<16)
> >  #define RTE_RED_INT16_NBITS                 (sizeof(uint16_t) * CHAR_BIT)
> >  #define RTE_RED_WQ_LOG2_NUM                 (RTE_RED_WQ_LOG2_MAX -
> > RTE_RED_WQ_LOG2_MIN + 1)
> > 
>
> All the above Doxygen comments are incorrectly formatted: as you want to change their behavior to refer to the variable _after_ the comment, they have to start with /**, not with /**< (which should be used if the comment refers to the variable _before_ the comment). Please fix.

Will do.

> > @@ -71,6 +88,8 @@ extern uint32_t rte_red_rand_val;  extern uint32_t 
> > rte_red_rand_seed;  extern uint16_t 
> > rte_red_log2_1_minus_Wq[RTE_RED_WQ_LOG2_NUM];
> >  extern uint16_t rte_red_pow2_frac_inv[16];
> > +extern uint8_t rte_red_scaling;
> > +extern uint16_t rte_red_max_threshold;
> > 
> >  /**
> >   * RED configuration parameters passed by user @@ -137,6 +156,26 @@ 
> > rte_red_config_init(struct rte_red_config *red_cfg,
> >  	const uint16_t maxp_inv);
> > 
> >  /**
> > + * @brief Configures the global setting for the RED scaling factor
> > + *
> > + * @param max_red_queue_length [in] must be a power of two
> > + *
> > + * @return Operation status
> > + * @retval 0 success
> > + * @retval !0 error
> > + */
> > +int
> > +rte_red_set_scaling(uint16_t max_red_queue_length);
> > +
> > +/**
> > + * @brief Reset the RED scaling factor - only for use by RED 
> > +unit-tests
> > + *
> > + * @return Operation status
> > + */
> > +void
> > +rte_red_reset_scaling(void);
>
> As stated above, this function is probably not useful and my vote is to remove it. 
>

It is needed by the revised unit-test program.  This function can't be moved into the unit-test program because it needs to reset variables that are statically declared within rte_red.c


> > +
> > +/**
> >   * @brief Generate random number for RED
> >   *
> >   * Implemenetation based on:
> > @@ -206,7 +245,7 @@ __rte_red_calc_qempty_factor(uint8_t wq_log2, 
> > uint16_t m)
> >  	f = (n >> 6) & 0xf;
> >  	n >>= 10;
> > 
> > -	if (n < RTE_RED_SCALING)
> > +	if (n < rte_red_scaling)
> >  		return (uint16_t) ((rte_red_pow2_frac_inv[f] + (1 << (n - 1)))
> > >> n);
> > 
> >  	return 0;
> > @@ -258,7 +297,9 @@ rte_red_enqueue_empty(const struct rte_red_config 
> > *red_cfg,
> >  	if (m >= RTE_RED_2POW16) {
> >  		red->avg = 0;
> >  	} else {
> > -		red->avg = (red->avg >> RTE_RED_SCALING) *
> > __rte_red_calc_qempty_factor(red_cfg->wq_log2, (uint16_t) m);
> > +		red->avg = (red->avg >> rte_red_scaling) *
> > +			__rte_red_calc_qempty_factor(red_cfg->wq_log2,
> > +						     (uint16_t) m);
> >  	}
> > 
> >  	return 0;
> > @@ -365,7 +406,7 @@ rte_red_enqueue_nonempty(const struct 
> > rte_red_config *red_cfg,
> >  	*/
> > 
> >  	/* avg update */
> > -	red->avg += (q << RTE_RED_SCALING) - (red->avg >> red_cfg-
> > >wq_log2);
> > +	red->avg += (q << rte_red_scaling) - (red->avg >> red_cfg-
> > >wq_log2);
> > 
> >  	/* avg < min_th: do not mark the packet  */
> >  	if (red->avg < red_cfg->min_th) {
> > diff --git a/lib/librte_sched/rte_sched_version.map
> > b/lib/librte_sched/rte_sched_version.map
> > index 3aa159a..92e51f8 100644
> > --- a/lib/librte_sched/rte_sched_version.map
> > +++ b/lib/librte_sched/rte_sched_version.map
> > @@ -29,3 +29,9 @@ DPDK_2.1 {
> >  	rte_sched_port_pkt_read_color;
> > 
> >  } DPDK_2.0;
> > +
> > +DPDK_17.08 {
> > +	global;
> > +
> > +	rte_red_set_scaling;
> > +} DPDK_2.1;
> > diff --git a/test/test/test_red.c b/test/test/test_red.c index 
> > 348075d..f2d50ef 100644
> > --- a/test/test/test_red.c
> > +++ b/test/test/test_red.c
> > @@ -67,6 +67,7 @@ struct test_rte_red_config {        /**< Test structure
> > for RTE_RED config */
> >  	uint32_t min_th;                /**< Queue minimum threshold */
> >  	uint32_t max_th;                /**< Queue maximum threshold */
> >  	uint8_t *maxp_inv;              /**< Inverse mark probability */
> > +	uint16_t max_queue_len;         /**< Maximum queue length */
> >  };
> > 
> >  struct test_queue {                 /**< Test structure for RTE_RED Queues */
> > @@ -181,7 +182,7 @@ static uint32_t rte_red_get_avg_int(const struct 
> > rte_red_config *red_cfg,
> >  	/**
> >  	 * scale by 1/n and convert from fixed-point to integer
> >  	 */
> > -	return red->avg >> (RTE_RED_SCALING + red_cfg->wq_log2);
> > +	return red->avg >> (rte_red_scaling + red_cfg->wq_log2);
> >  }
> > 
> >  static double rte_red_get_avg_float(const struct rte_red_config 
> > *red_cfg, @@ -190,7 +191,7 @@ static double 
> > rte_red_get_avg_float(const struct rte_red_config *red_cfg,
> >  	/**
> >  	 * scale by 1/n and convert from fixed-point to floating-point
> >  	 */
> > -	return ldexp((double)red->avg,  -(RTE_RED_SCALING + red_cfg-
> > >wq_log2));
> > +	return ldexp((double)red->avg,  -(rte_red_scaling + red_cfg-
> > >wq_log2));
> >  }
> > 
> >  static void rte_red_set_avg_int(const struct rte_red_config *red_cfg, 
> > @@ -200,7 +201,7 @@ static void rte_red_set_avg_int(const struct 
> > rte_red_config *red_cfg,
> >  	/**
> >  	 * scale by n and convert from integer to fixed-point
> >  	 */
> > -	red->avg = avg << (RTE_RED_SCALING + red_cfg->wq_log2);
> > +	red->avg = avg << (rte_red_scaling + red_cfg->wq_log2);
> >  }
> > 
> >  static double calc_exp_avg_on_empty(double avg, uint32_t n, uint32_t
> > time_diff)
> > @@ -280,10 +281,23 @@ static enum test_result  
> > test_rte_red_init(struct test_config *tcfg)  {
> >  	unsigned i = 0;
> > +	int ret;
> > 
> >  	tcfg->tvar->clk_freq = rte_get_timer_hz();
> >  	init_port_ts( tcfg->tvar->clk_freq );
> > 
> > +	rte_red_reset_scaling();
> > +	ret = rte_red_set_scaling(tcfg->tconfig->max_queue_len);
> > +	if (ret) {
> > +		printf("rte_red_set_scaling failed: %d\n", ret);
> > +		return FAIL;
> > +	}
> > +	if (rte_red_max_threshold < tcfg->tconfig->max_th) {
> > +		printf("rte_red_max_th (%u) < tconfig->max_th (%u)\n",
> > +		       rte_red_max_threshold, tcfg->tconfig->max_th);
> > +		return FAIL;
> > +	}
> > +
> >  	for (i = 0; i < tcfg->tconfig->num_cfg; i++) {
> >  		if (rte_red_config_init(&tcfg->tconfig->rconfig[i],
> >  					(uint16_t)tcfg->tconfig->wq_log2[i],
> > @@ -385,6 +399,7 @@ static struct test_rte_red_config ft_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 128,
> >  	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_queue ft_tqueue = { @@ -554,6 +569,7 @@ static 
> > struct test_rte_red_config ft2_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 128,
> >  	.maxp_inv = ft2_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_config func_test2_config = { @@ -576,6 +592,41 @@ 
> > static struct test_config func_test2_config = {
> >  	.tlevel = ft2_tlevel,
> >  };
> > 
> > +/**
> > + * Test F2: functional test 2 - with long queues and smaller 
> > +red-scaling
> > factor
> > + */
> > +static uint32_t ft2_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft2_tconfig_scaling =  {
> > +	.rconfig = ft2_rconfig,
> > +	.num_cfg = RTE_DIM(ft2_rconfig),
> > +	.wq_log2 = ft2_wq_log2,
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.maxp_inv = ft2_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test2_config_scaling = {
> > +	.ifname = "functional test 2 interface",
> > +	.msg = "functional test 2 : use several RED configurations and long
> > queues,\n"
> > +	"		    increase average queue size to just below
> > maximum threshold,\n"
> > +	"		    compare drop rate to drop probability\n\n",
> > +	.htxt = "RED config     "
> > +	"avg queue size "
> > +	"min threshold  "
> > +	"max threshold  "
> > +	"drop prob %    "
> > +	"drop rate %    "
> > +	"diff %         "
> > +	"tolerance %    "
> > +	"\n",
> > +	.tconfig = &ft2_tconfig_scaling,
> > +	.tqueue = &ft_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft2_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test2(struct test_config *tcfg)  {
> >  	enum test_result result = PASS;
> > @@ -662,6 +713,7 @@ static struct test_rte_red_config ft3_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 1023,
> >  	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_config func_test3_config = { @@ -683,6 +735,40 @@ 
> > static struct test_config func_test3_config = {
> >  	.tlevel = ft3_tlevel,
> >  };
> > 
> > +/**
> > + * Test F3: functional test 3 - with large queues and smaller red 
> > +scaling
> > factor
> > + */
> > +static uint32_t ft3_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft3_tconfig_scaling =  {
> > +	.rconfig = ft_wrconfig,
> > +	.num_cfg = RTE_DIM(ft_wrconfig),
> > +	.wq_log2 = ft_wq_log2,
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test3_config_scaling = {
> > +	.ifname = "functional test 3 interface",
> > +	.msg = "functional test 3 : use one RED configuration and long
> > queues,\n"
> > +	"		    increase average queue size to target level,\n"
> > +	"		    dequeue all packets until queue is empty,\n"
> > +	"		    confirm that average queue size is computed
> > correctly while queue is empty\n\n",
> > +	.htxt = "q avg before   "
> > +	"q avg after    "
> > +	"expected       "
> > +	"difference %   "
> > +	"tolerance %    "
> > +	"result	 "
> > +	"\n",
> > +	.tconfig = &ft3_tconfig_scaling,
> > +	.tqueue = &ft_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft3_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test3(struct test_config *tcfg)  {
> >  	enum test_result result = PASS;
> > @@ -776,6 +862,7 @@ static struct test_rte_red_config ft4_tconfig =  {
> >  	.max_th = 1023,
> >  	.wq_log2 = ft4_wq_log2,
> >  	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_queue ft4_tqueue = { @@ -810,6 +897,42 @@ static 
> > struct test_config func_test4_config = {
> >  	.tlevel = ft4_tlevel,
> >  };
> > 
> > +/**
> > + * Test F4: functional test 4
> > + */
> > +static uint32_t ft4_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft4_tconfig_scaling =  {
> > +	.rconfig = ft_wrconfig,
> > +	.num_cfg = RTE_DIM(ft_wrconfig),
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.wq_log2 = ft4_wq_log2,
> > +	.maxp_inv = ft_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test4_config_scaling = {
> > +	.ifname = "functional test 4 interface",
> > +	.msg = "functional test 4 : use one RED configuration on long
> > queue,\n"
> > +	"		    increase average queue size to target level,\n"
> > +	"		    dequeue all packets until queue is empty,\n"
> > +	"		    confirm that average queue size is computed
> > correctly while\n"
> > +	"		    queue is empty for more than 50 sec,\n"
> > +	"		    (this test takes 52 sec to run)\n\n",
> > +	.htxt = "q avg before   "
> > +	"q avg after    "
> > +	"expected       "
> > +	"difference %   "
> > +	"tolerance %    "
> > +	"result	 "
> > +	"\n",
> > +	.tconfig = &ft4_tconfig_scaling,
> > +	.tqueue = &ft4_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft4_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test4(struct test_config *tcfg)  {
> >  	enum test_result result = PASS;
> > @@ -924,6 +1047,7 @@ static struct test_rte_red_config ft5_tconfig =  {
> >  	.max_th = 128,
> >  	.wq_log2 = ft5_wq_log2,
> >  	.maxp_inv = ft5_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_queue ft5_tqueue = { @@ -970,6 +1094,45 @@ static 
> > struct test_config func_test5_config = {
> >  	.tlevel = ft5_tlevel,
> >  };
> > 
> > +
> > +/**
> > + * Test F5: functional test 5
> > + */
> > +static uint32_t ft5_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft5_tconfig_scaling =  {
> > +	.rconfig = ft5_config,
> > +	.num_cfg = RTE_DIM(ft5_config),
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.wq_log2 = ft5_wq_log2,
> > +	.maxp_inv = ft5_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test5_config_scaling = {
> > +	.ifname = "functional test 5 interface",
> > +	.msg = "functional test 5 : use several long queues (each with its
> > own run-time data),\n"
> > +	"		    use several RED configurations (such that each
> > configuration is shared by multiple queues),\n"
> > +	"		    increase average queue size to just below
> > maximum threshold,\n"
> > +	"		    compare drop rate to drop probability,\n"
> > +	"		    (this is a larger scale version of functional test
> > 2)\n\n",
> > +	.htxt = "queue          "
> > +	"config         "
> > +	"avg queue size "
> > +	"min threshold  "
> > +	"max threshold  "
> > +	"drop prob %    "
> > +	"drop rate %    "
> > +	"diff %         "
> > +	"tolerance %    "
> > +	"\n",
> > +	.tconfig = &ft5_tconfig_scaling,
> > +	.tqueue = &ft5_tqueue,
> > +	.tvar = &ft5_tvar,
> > +	.tlevel = ft5_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test5(struct test_config *tcfg)  {
> >  	enum test_result result = PASS;
> > @@ -1062,6 +1225,7 @@ static struct test_rte_red_config ft6_tconfig =  {
> >  	.max_th = 1023,
> >  	.wq_log2 = ft6_wq_log2,
> >  	.maxp_inv = ft6_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_queue ft6_tqueue = { @@ -1078,7 +1242,7 @@ static 
> > struct test_queue ft6_tqueue = {  static struct test_config 
> > func_test6_config = {
> >  	.ifname = "functional test 6 interface",
> >  	.msg = "functional test 6 : use several queues (each with its own 
> > run-time data),\n"
> > -	"		    use several RED configurations (such that each
> > configuration is sharte_red by multiple queues),\n"
> > +	"		    use several RED configurations (such that each
> > configuration is shared by multiple queues),\n"
> >  	"		    increase average queue size to target level,\n"
> >  	"		    dequeue all packets until queue is empty,\n"
> >  	"		    confirm that average queue size is computed
> > correctly while queue is empty\n"
> > @@ -1097,6 +1261,44 @@ static struct test_config func_test6_config = {
> >  	.tlevel = ft6_tlevel,
> >  };
> > 
> > +/**
> > + * Test F6: functional test 6
> > + */
> > +static uint32_t ft6_tlevel_scaling[] = {8190};
> > +
> > +static struct test_rte_red_config ft6_tconfig_scaling =  {
> > +	.rconfig = ft6_config,
> > +	.num_cfg = RTE_DIM(ft6_config),
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.wq_log2 = ft6_wq_log2,
> > +	.maxp_inv = ft6_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config func_test6_config_scaling = {
> > +	.ifname = "functional test 6 interface",
> > +	.msg = "functional test 6 : use several long queues (each with its
> > own run-time data),\n"
> > +	"		    use several RED configurations (such that each
> > configuration is shared by multiple queues),\n"
> > +	"		    increase average queue size to target level,\n"
> > +	"		    dequeue all packets until queue is empty,\n"
> > +	"		    confirm that average queue size is computed
> > correctly while queue is empty\n"
> > +	"		    (this is a larger scale version of functional test
> > 3)\n\n",
> > +	.htxt = "queue          "
> > +	"config         "
> > +	"q avg before   "
> > +	"q avg after    "
> > +	"expected       "
> > +	"difference %   "
> > +	"tolerance %    "
> > +	"result	 "
> > +	"\n",
> > +	.tconfig = &ft6_tconfig_scaling,
> > +	.tqueue = &ft6_tqueue,
> > +	.tvar = &ft_tvar,
> > +	.tlevel = ft6_tlevel_scaling,
> > +};
> > +
> >  static enum test_result func_test6(struct test_config *tcfg)  {
> >  	enum test_result result = PASS;
> > @@ -1195,6 +1397,7 @@ static struct test_rte_red_config pt_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 128,
> >  	.maxp_inv = pt_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_queue pt_tqueue = { @@ -1557,6 +1760,7 @@ static 
> > struct test_rte_red_config ovfl_tconfig =  {
> >  	.min_th = 32,
> >  	.max_th = 1023,
> >  	.maxp_inv = ovfl_maxp_inv,
> > +	.max_queue_len = RTE_RED_DEFAULT_QUEUE_LENGTH,
> >  };
> > 
> >  static struct test_queue ovfl_tqueue = { @@ -1598,7 +1802,7 @@ static 
> > struct test_config ovfl_test1_config = {
> >  	.ifname = "queue avergage overflow test interface",
> >  	.msg = "overflow test 1 : use one RED configuration,\n"
> >  	"		  increase average queue size to target level,\n"
> > -	"		  check maximum number of bits requirte_red to
> > represent avg_s\n\n",
> > +	"		  check maximum number of bits required to
> > represent avg_s\n\n",
> >  	.htxt = "avg queue size  "
> >  	"wq_log2  "
> >  	"fraction bits  "
> > @@ -1615,6 +1819,39 @@ static struct test_config ovfl_test1_config = {
> >  	.tlevel = ovfl_tlevel,
> >  };
> > 
> > +static uint32_t ovfl_tlevel_scaling[] = {8191};
> > +
> > +static struct test_rte_red_config ovfl_tconfig_scaling =  {
> > +	.rconfig = ovfl_wrconfig,
> > +	.num_cfg = RTE_DIM(ovfl_wrconfig),
> > +	.wq_log2 = ovfl_wq_log2,
> > +	.min_th = 32,
> > +	.max_th = 8191,
> > +	.maxp_inv = ovfl_maxp_inv,
> > +	.max_queue_len = 8192,
> > +};
> > +
> > +static struct test_config ovfl_test1_config_scaling = {
> > +	.ifname = "queue average overflow test interface",
> > +	.msg = "overflow test 1 on long queue: use one RED
> > configuration,\n"
> > +	"		  increase average queue size to target level,\n"
> > +	"		  check maximum number of bits required to
> > represent avg_s\n\n",
> > +	.htxt = "avg queue size  "
> > +	"wq_log2  "
> > +	"fraction bits  "
> > +	"max queue avg  "
> > +	"num bits  "
> > +	"enqueued  "
> > +	"dropped   "
> > +	"drop prob %  "
> > +	"drop rate %  "
> > +	"\n",
> > +	.tconfig = &ovfl_tconfig_scaling,
> > +	.tqueue = &ovfl_tqueue,
> > +	.tvar = &ovfl_tvar,
> > +	.tlevel = ovfl_tlevel_scaling,
> > +};
> > +
> >  static enum test_result ovfl_test1(struct test_config *tcfg)  {
> >  	enum test_result result = PASS;
> > @@ -1690,7 +1927,7 @@ static enum test_result ovfl_test1(struct 
> > test_config *tcfg)
> >  	printf("%s", tcfg->htxt);
> > 
> >  	printf("%-16u%-9u%-15u0x%08x     %-10u%-10u%-10u%-13.2lf%-
> > 13.2lf\n",
> > -	       avg, *tcfg->tconfig->wq_log2, RTE_RED_SCALING,
> > +	       avg, *tcfg->tconfig->wq_log2, rte_red_scaling,
> >  	       avg_max, avg_max_bits,
> >  	       *tcfg->tvar->enqueued, *tcfg->tvar->dropped,
> >  	       drop_prob * 100.0, drop_rate * 100.0); @@ -1730,6 +1967,15 @@ 
> > struct tests perf_tests[] = {
> >  	{ &perf2_test6_config, perf2_test },  };
> > 
> > +struct tests scale_tests[] = {
> > +	{ &func_test2_config_scaling, func_test2 },
> > +	{ &func_test3_config_scaling, func_test3 },
> > +	{ &func_test4_config_scaling, func_test4 },
> > +	{ &func_test5_config_scaling, func_test5 },
> > +	{ &func_test6_config_scaling, func_test6 },
> > +	{ &ovfl_test1_config_scaling, ovfl_test1 }, };
> > +
> >  /**
> >   * function to execute the required_red tests
> >   */
> > @@ -1866,6 +2112,20 @@ test_red_perf(void)  }
> > 
> >  static int
> > +test_red_scaling(void)
> > +{
> > +	uint32_t num_tests = 0;
> > +	uint32_t num_pass = 0;
> > +
> > +	if (test_invalid_parameters() < 0)
> > +		return -1;
> > +
> > +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> > &num_pass);
> > +	show_stats(num_tests, num_pass);
> > +	return tell_the_result(num_tests, num_pass); }
> > +
> > +static int
> >  test_red_all(void)
> >  {
> >  	uint32_t num_tests = 0;
> > @@ -1876,10 +2136,12 @@ test_red_all(void)
> > 
> >  	run_tests(func_tests, RTE_DIM(func_tests), &num_tests, &num_pass);
> >  	run_tests(perf_tests, RTE_DIM(perf_tests), &num_tests, &num_pass);
> > +	run_tests(scale_tests, RTE_DIM(scale_tests), &num_tests,
> > &num_pass);
> >  	show_stats(num_tests, num_pass);
> >  	return tell_the_result(num_tests, num_pass);  }
> > 
> >  REGISTER_TEST_COMMAND(red_autotest, test_red);  
> > REGISTER_TEST_COMMAND(red_perf, test_red_perf);
> > +REGISTER_TEST_COMMAND(red_scaling, test_red_scaling);
> >  REGISTER_TEST_COMMAND(red_all, test_red_all);
> > --
> > 2.1.4
>
> Regards,
> Cristian

Re: [PATCH v4] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> > > +int
> > > +rte_red_set_scaling(uint16_t max_red_queue_length);
> > > +
> > > +/**
> > > + * @brief Reset the RED scaling factor - only for use by RED
> > > +unit-tests
> > > + *
> > > + * @return Operation status
> > > + */
> > > +void
> > > +rte_red_reset_scaling(void);
> >
> > As stated above, this function is probably not useful and my vote is to
> remove it.
> >
> 
> It is needed by the revised unit-test program.  This function can't be moved
> into the unit-test program because it needs to reset variables that are
> statically declared within rte_red.c
> 
> 

Hi Alan,

We only put API that makes sense for a real app, not for unit test.

About unit tests: Each test in the unit test suite should start from scratch, i.e. create a RED object from scratch, configure it, use it, free it rather than use RED objects created by previous tests. We need to avoid the latter approach, as it is creating fake dependencies between tests that alter the overall test results. Each test should be independent, and not rely on previous tests. Makes sense?

Regards,
Cristian

Re: [PATCH v4] sched: make RED scaling configurable

[Dewar, Alan]

> > > > +int
> > > > +rte_red_set_scaling(uint16_t max_red_queue_length);
> > > > +
> > > > +/**
> > > > + * @brief Reset the RED scaling factor - only for use by RED 
> > > > +unit-tests
> > > > + *
> > > > + * @return Operation status
> > > > + */
> > > > +void
> > > > +rte_red_reset_scaling(void);
> > >
> > > As stated above, this function is probably not useful and my vote is 
> > > to
> > remove it.
> > >
> > 
> > It is needed by the revised unit-test program.  This function can't be 
> > moved into the unit-test program because it needs to reset variables 
> > that are statically declared within rte_red.c
> > 
> > 
>
> Hi Alan,
>
> We only put API that makes sense for a real app, not for unit test.

I didn't add rte_red_reset_scaling function to lib/librte_sched/rte_sched_version.map, so doesn't that mean that this function wouldn't be part of the rte_sched library's external API?
 
> About unit tests: Each test in the unit test suite should start from scratch, i.e. create a RED object from scratch, configure it, use it, free it rather than use RED objects created by previous tests. We need to avoid the latter approach, as it is creating fake dependencies between tests that alter the overall test results. Each test should be independent, and not rely on previous tests. Makes sense?

I can appreciate what you are suggesting, but rte_red.c does some one-time initialization of a module static array called rte_red_log2_1_minus_Wq.  The values assigned to elements in this array depend upon what the RED scaling factor is set to.   This means that the first RED sub-test that calls rte_red_config_init imposes its scaling factor for all subsequent RED sub-tests.    The new RED unit-tests need to use different scaling factors.

The only way to make these RED sub-tests completely independent would be to create and execute separate test images for each sub-test, rather than building all the sub-tests into a single image and using the test-harness's command-line to select which sub-test to execute. 

As an alternative I'm guessing that we could change how rte_red.c does its initialization.

Regards
Alan


>
> Regards,
> Cristian

Re: [PATCH v4] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Dewar, Alan [mailto:ad759e@intl.att.com]
> Sent: Thursday, January 4, 2018 1:35 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> 'alangordondewar@gmail.com' <alangordondewar@gmail.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>; 'Alan Dewar' <alan.dewar@att.com>
> Subject: RE: [PATCH v4] sched: make RED scaling configurable
> 
> > > > > +int
> > > > > +rte_red_set_scaling(uint16_t max_red_queue_length);
> > > > > +
> > > > > +/**
> > > > > + * @brief Reset the RED scaling factor - only for use by RED
> > > > > +unit-tests
> > > > > + *
> > > > > + * @return Operation status
> > > > > + */
> > > > > +void
> > > > > +rte_red_reset_scaling(void);
> > > >
> > > > As stated above, this function is probably not useful and my vote is
> > > > to
> > > remove it.
> > > >
> > >
> > > It is needed by the revised unit-test program.  This function can't be
> > > moved into the unit-test program because it needs to reset variables
> > > that are statically declared within rte_red.c
> > >
> > >
> >
> > Hi Alan,
> >
> > We only put API that makes sense for a real app, not for unit test.
> 
> I didn't add rte_red_reset_scaling function to
> lib/librte_sched/rte_sched_version.map, so doesn't that mean that this
> function wouldn't be part of the rte_sched library's external API?

Nope, it does not work this way.

> 
> > About unit tests: Each test in the unit test suite should start from scratch,
> i.e. create a RED object from scratch, configure it, use it, free it rather than
> use RED objects created by previous tests. We need to avoid the latter
> approach, as it is creating fake dependencies between tests that alter the
> overall test results. Each test should be independent, and not rely on
> previous tests. Makes sense?
> 
> I can appreciate what you are suggesting, but rte_red.c does some one-time
> initialization of a module static array called rte_red_log2_1_minus_Wq.  The
> values assigned to elements in this array depend upon what the RED scaling
> factor is set to.   This means that the first RED sub-test that calls
> rte_red_config_init imposes its scaling factor for all subsequent RED sub-
> tests.    The new RED unit-tests need to use different scaling factors.
> 
> The only way to make these RED sub-tests completely independent would be
> to create and execute separate test images for each sub-test, rather than
> building all the sub-tests into a single image and using the test-harness's
> command-line to select which sub-test to execute.
> 
> As an alternative I'm guessing that we could change how rte_red.c does its
> initialization.
> 

OK, makes sense to me. Let's create an API function with the updated name of __rte_red_reset() which resets all globals and forces retrain of the module. It should be part of the API map.

> Regards
> Alan
> 
> 
> >
> > Regards,
> > Cristian