[dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[dpdk-dev] [PATCH v5] 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             |  61 ++++++--
 lib/librte_sched/rte_sched_version.map |   7 +
 test/test/test_red.c                   | 274 ++++++++++++++++++++++++++++++++-
 4 files changed, 373 insertions(+), 22 deletions(-)

diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
index ade57d1..52f8626 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;
+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;
+	rte_red_max_threshold = max_red_queue_length - 1;
+	return 0;
+}
+
+void
+__rte_red_reset(void)
+{
+	rte_red_init_done = 0;
+	rte_red_scaling = RTE_RED_SCALING;
+	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 6edf914..5fd17bb 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                     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,24 @@ 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 all RED global variable - only for use by RED unit-tests
+ */
+void
+__rte_red_reset(void);
+
+/**
  * @brief Generate random number for RED
  *
  * Implementation based on:
@@ -206,7 +243,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 +295,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 +404,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..262bece 100644
--- a/lib/librte_sched/rte_sched_version.map
+++ b/lib/librte_sched/rte_sched_version.map
@@ -29,3 +29,10 @@ DPDK_2.1 {
 	rte_sched_port_pkt_read_color;
 
 } DPDK_2.0;
+
+DPDK_17.08 {
+	global;
+
+	__rte_red_reset;
+	rte_red_set_scaling;
+} DPDK_2.1;
diff --git a/test/test/test_red.c b/test/test/test_red.c
index 348075d..2a8075d 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();
+	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.7.4

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dumitrescu, Cristian]

Hi Alan,

More issues and questions below:

...<snip>

> diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.h
I suggest we use uint32_t for rte_red_scaling and rte_red_max_threshold, even though their values can fit into uint8_t and uint16_t respectively.


> diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
> +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;

Is there a reason why we cannot simply reset the scaling here?

> +
> +	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++;
> +	}

This does not look right to me. I think you want to compute the log2 of max_red_queue_length here, but your result (count) is bigger by 1 than it should be, right?
When max_red_queue_length = RTE_RED_DEFAULT_QUEUE_LENGTH = 1024, the result should be: count = RTE_RED_SCALING = 10, not 11.

I suggest you use rte_bsf32() function for this purpose.

> +
> +	rte_red_scaling -= count - RTE_RED_SCALING;

Why not simply: rte_red_scaling = count?

> +	rte_red_max_threshold = max_red_queue_length - 1;
> +	return 0;
> +}
> +

> diff --git a/lib/librte_sched/rte_sched_version.map
> b/lib/librte_sched/rte_sched_version.map
> index 3aa159a..262bece 100644
> --- a/lib/librte_sched/rte_sched_version.map
> +++ b/lib/librte_sched/rte_sched_version.map
> @@ -29,3 +29,10 @@ DPDK_2.1 {
>  	rte_sched_port_pkt_read_color;
> 
>  } DPDK_2.0;
> +
> +DPDK_17.08 {
> +	global;
> +
> +	__rte_red_reset;
> +	rte_red_set_scaling;
> +} DPDK_2.1;

You need to put the correct DPDK release number here.

You also need to do more work to make sure the share library support  is not broken:
-need to increase LIBABIVER in Makefile
-need to update the library .so number in release notes
-need to check that build does not fail for shared libraries

(You can use this patch as example: https://dpdk.org/dev/patchwork/patch/33109/ )

> diff --git a/test/test/test_red.c b/test/test/test_red.c
The RED  test code fails to link, please fix:

export RTE_TARGET=x86_64-native-linuxapp-gcc

[ ]$ ~/git_dpdk_red/dpdk$ make -C test/test
make: Entering directory 'dpdk/test/test'
...<snip>
   LD test
test_red.o: In function `test_rte_red_init':
test_red.c:(.text+0x2cd): undefined reference to `__rte_red_reset'
test_red.c:(.text+0x2da): undefined reference to `rte_red_set_scaling'
test_red.c:(.text+0x2ed): undefined reference to `rte_red_max_threshold'
test_red.o: In function `increase_actual_qsize':
test_red.c:(.text+0x48c): undefined reference to `rte_red_scaling'
test_red.c:(.text+0x533): undefined reference to `rte_red_scaling'
test_red.o: In function `enqueue_dequeue_func':
test_red.c:(.text+0x70c): undefined reference to `rte_red_scaling'
test_red.c:(.text+0x7a5): undefined reference to `rte_red_scaling'
test_red.o: In function `increase_average_qsize':
test_red.c:(.text+0x95c): undefined reference to `rte_red_scaling'
test_red.o:test_red.c:(.text+0x9f1): more undefined references to `rte_red_scaling' follow
collect2: error: ld returned 1 exit status
dpdk/mk/rte.app.mk:306: recipe for target 'test' failed
make: *** [test] Error 1
make: Leaving directory 'dpdk/test/test'

Regards,
Cristian

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dewar, Alan]

Hi Cristian,

Responses inline below.

Regards
Alan

> -----Original Message-----
> From: Dumitrescu, Cristian [mailto:cristian.dumitrescu@intel.com] 
> Sent: Thursday, January 11, 2018 1:11 PM
> To: alangordondewar@gmail.com
> Cc: dev@dpdk.org; Alan Dewar <alan.dewar@att.com>; Kantecki, Tomasz <tomasz.kantecki@intel.com>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Subject: RE: [PATCH v5] sched: make RED scaling configurable
>
> Hi Alan,
>
> More issues and questions below:
>
>...<snip>
>
> > diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.h
> I suggest we use uint32_t for rte_red_scaling and rte_red_max_threshold, even though their values can fit into uint8_t and uint16_t respectively.

Will do.

> > diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
> > +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;
>
> Is there a reason why we cannot simply reset the scaling here?

Actually we could, but I was originally thinking that you might be happier keeping with a one-time RED initialization function, but then had to introduce the rte_reset_red_scaling function for the unit-tests. I'm happy to do RED reinitialization here, if you are.

> > +
> > +	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++;
> > +	}
>
> This does not look right to me. I think you want to compute the log2 of max_red_queue_length here, but your result (count) is bigger by 1 than it should be, right?
> When max_red_queue_length = RTE_RED_DEFAULT_QUEUE_LENGTH = 1024, the result should be: count = RTE_RED_SCALING = 10, not 11.
>

Well spotted.

> I suggest you use rte_bsf32() function for this purpose.

I wasn't aware of this function, it appears to do the necessary so will do.

> > +
> > +	rte_red_scaling -= count - RTE_RED_SCALING;
>
> Why not simply: rte_red_scaling = count?

Stupidity on my part! - Will do.

> > +	rte_red_max_threshold = max_red_queue_length - 1;
> > +	return 0;
> > +}
> > +
>
> > diff --git a/lib/librte_sched/rte_sched_version.map
> > b/lib/librte_sched/rte_sched_version.map
> > index 3aa159a..262bece 100644
> > --- a/lib/librte_sched/rte_sched_version.map
> > +++ b/lib/librte_sched/rte_sched_version.map
> > @@ -29,3 +29,10 @@ DPDK_2.1 {
> >  	rte_sched_port_pkt_read_color;
> > 
> >  } DPDK_2.0;
> > +
> > +DPDK_17.08 {
> > +	global;
> > +
> > +	__rte_red_reset;
> > +	rte_red_set_scaling;
> > +} DPDK_2.1;
>
> You need to put the correct DPDK release number here.

Will do.

> You also need to do more work to make sure the share library support  is not broken:
> -need to increase LIBABIVER in Makefile
> -need to update the library .so number in release notes -need to check that build does not fail for shared libraries
>
> (You can use this patch as example: https://urldefense.proofpoint.com/v2/url?u=https-3A__dpdk.org_dev_patchwork_patch_33109_&d=DwIFAg&c=LFYZ-o9_HUMeMTSQicvjIg&r=MSobi4bsl2plY_3b1jvY6kXt-SHzSxMj5YhuyxfTjJE&m=D7OTgDyX27pcg-xs8N7FQzX4lAkkcPqbPAhJp_IyrNI&s=mCvsqitq8Y6vD3UC-r1WEGtrRp17bsmJaEHGby97zYw&e=  )

Okay, I need to look into this stuff - I'll come back with some questions if anything isn't clear.

> > diff --git a/test/test/test_red.c b/test/test/test_red.c
> The RED  test code fails to link, please fix:

Yes - will do.

> export RTE_TARGET=x86_64-native-linuxapp-gcc
>
> [ ]$ ~/git_dpdk_red/dpdk$ make -C test/test
> make: Entering directory 'dpdk/test/test'
>...<snip>
>    LD test
> test_red.o: In function `test_rte_red_init':
> test_red.c:(.text+0x2cd): undefined reference to `__rte_red_reset'
> test_red.c:(.text+0x2da): undefined reference to `rte_red_set_scaling'
> test_red.c:(.text+0x2ed): undefined reference to `rte_red_max_threshold'
> test_red.o: In function `increase_actual_qsize':
> test_red.c:(.text+0x48c): undefined reference to `rte_red_scaling'
> test_red.c:(.text+0x533): undefined reference to `rte_red_scaling'
> test_red.o: In function `enqueue_dequeue_func':
> test_red.c:(.text+0x70c): undefined reference to `rte_red_scaling'
> test_red.c:(.text+0x7a5): undefined reference to `rte_red_scaling'
> test_red.o: In function `increase_average_qsize':
> test_red.c:(.text+0x95c): undefined reference to `rte_red_scaling'
> test_red.o:test_red.c:(.text+0x9f1): more undefined references to `rte_red_scaling' follow
> collect2: error: ld returned 1 exit status
> dpdk/mk/rte.app.mk:306: recipe for target 'test' failed
> make: *** [test] Error 1
> make: Leaving directory 'dpdk/test/test'
>
> Regards,
> Cristian

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dumitrescu, Cristian]

...<snip>
 
> > > diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
> > > +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;
> >
> > Is there a reason why we cannot simply reset the scaling here?
> 
> Actually we could, but I was originally thinking that you might be happier
> keeping with a one-time RED initialization function, but then had to
> introduce the rte_reset_red_scaling function for the unit-tests. I'm happy to
> do RED reinitialization here, if you are.
> 

Hi Alan,

What is the intention of the new rte_red_set_scaling() function?
	1. Is it to be called only once, before any RED object gets created?
	2. Is it possible to call it post-init, but in this case any RED object already created are not impacted (they continue to work)?

If the answer is 2, then yes, we could simply drop the __rte_red_reset() and do the RED globals reset as part of the rte_red_set_scaling() function transparently.

If the answer is 1, then we probably need to keep your approach: we need a global rte_red_init_done flag, and rte_red_set_scaling() could only be called at init time before any red objects are created.

I probably need to spend more time assessing all the code implications.

Regards,
Cristian

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu,
> Cristian
> Sent: Friday, January 12, 2018 11:09 AM
> To: Dewar, Alan <ad759e@intl.att.com>; 'alangordondewar@gmail.com'
> <alangordondewar@gmail.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>; 'Alan Dewar' <alan.dewar@att.com>;
> Kantecki, Tomasz <tomasz.kantecki@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable
> 
> ...<snip>
> 
> > > > diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
> > > > +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;
> > >
> > > Is there a reason why we cannot simply reset the scaling here?
> >
> > Actually we could, but I was originally thinking that you might be happier
> > keeping with a one-time RED initialization function, but then had to
> > introduce the rte_reset_red_scaling function for the unit-tests. I'm happy
> to
> > do RED reinitialization here, if you are.
> >
> 
> Hi Alan,
> 
> What is the intention of the new rte_red_set_scaling() function?
> 	1. Is it to be called only once, before any RED object gets created?
> 	2. Is it possible to call it post-init, but in this case any RED object
> already created are not impacted (they continue to work)?
> 
> If the answer is 2, then yes, we could simply drop the __rte_red_reset() and
> do the RED globals reset as part of the rte_red_set_scaling() function
> transparently.
> 
> If the answer is 1, then we probably need to keep your approach: we need a
> global rte_red_init_done flag, and rte_red_set_scaling() could only be called
> at init time before any red objects are created.
> 
> I probably need to spend more time assessing all the code implications.
> 
> Regards,
> Cristian

Hi Alan,

After talking to Tomasz, we agreed that 2. Is not an option, as any previously created red object will be broken.

So 1. Is the right answer, therefore this function can be called only once and only before any red objects are created. So, IMO we should do this:
-when rte_red_init_done is true, we need to return error
-when rte_red_init_done is false, we need to perform red initialization and set this flag

Agree?

Please also add a comment in the Doxygen description of rte_red_set_caling() in the rte_red.h file, stating that this function can be called only once and only before any red objects are created.

Regards,
Cristian

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dewar, Alan]

> > > > > diff --git a/lib/librte_sched/rte_red.c 
> > > > > b/lib/librte_sched/rte_red.c
> > > > > +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;
> > > >
> > > > Is there a reason why we cannot simply reset the scaling here?
> > >
> > > Actually we could, but I was originally thinking that you might be 
> > > happier keeping with a one-time RED initialization function, but 
> > > then had to introduce the rte_reset_red_scaling function for the 
> > > unit-tests. I'm happy
> > to
> > > do RED reinitialization here, if you are.
> > >
> > 
> > Hi Alan,
> > 
> > What is the intention of the new rte_red_set_scaling() function?
> > 	1. Is it to be called only once, before any RED object gets created?
> > 	2. Is it possible to call it post-init, but in this case any RED 
> > object already created are not impacted (they continue to work)?
> > 
> > If the answer is 2, then yes, we could simply drop the 
> > __rte_red_reset() and do the RED globals reset as part of the 
> > rte_red_set_scaling() function transparently.
> > 
> > If the answer is 1, then we probably need to keep your approach: we 
> > need a global rte_red_init_done flag, and rte_red_set_scaling() could 
> > only be called at init time before any red objects are created.
> > 
> > I probably need to spend more time assessing all the code implications.
> > 
> > Regards,
> > Cristian
>
> Hi Alan,
>
> After talking to Tomasz, we agreed that 2. Is not an option, as any previously created red object will be broken.
>
> So 1. Is the right answer, therefore this function can be called only once and only before any red objects are created. So, IMO we should do this:
-when rte_red_init_done is true, we need to return error -when rte_red_init_done is false, we need to perform red initialization and set this flag
>
> Agree?

Yes, as long as you're happy with keeping __rte_red_reset for the unit-tests.

> Please also add a comment in the Doxygen description of rte_red_set_caling() in the rte_red.h file, stating that this function can be called only once and only before any red objects are created.

Will do.

> Regards,
> Cristian

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> > > Hi Alan,
> > >
> > > What is the intention of the new rte_red_set_scaling() function?
> > > 	1. Is it to be called only once, before any RED object gets created?
> > > 	2. Is it possible to call it post-init, but in this case any RED
> > > object already created are not impacted (they continue to work)?
> > >
> > > If the answer is 2, then yes, we could simply drop the
> > > __rte_red_reset() and do the RED globals reset as part of the
> > > rte_red_set_scaling() function transparently.
> > >
> > > If the answer is 1, then we probably need to keep your approach: we
> > > need a global rte_red_init_done flag, and rte_red_set_scaling() could
> > > only be called at init time before any red objects are created.
> > >
> > > I probably need to spend more time assessing all the code implications.
> > >
> > > Regards,
> > > Cristian
> >
> > Hi Alan,
> >
> > After talking to Tomasz, we agreed that 2. Is not an option, as any
> previously created red object will be broken.
> >
> > So 1. Is the right answer, therefore this function can be called only once
> and only before any red objects are created. So, IMO we should do this:
> -when rte_red_init_done is true, we need to return error -when
> rte_red_init_done is false, we need to perform red initialization and set this
> flag
> >
> > Agree?
> 
> Yes, as long as you're happy with keeping __rte_red_reset for the unit-tests.
> 

OK

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dewar, Alan]

Hi Cristian,

> > +
> > +	rte_red_scaling -= count - RTE_RED_SCALING;
>
> Why not simply: rte_red_scaling = count?

The RED code stores the moving average in a uint32_t as a pseudo floating point number with a fixed sized fractional part of 10 bits.
This allows a maximum queue length of 1024.   To support larger queues, the size of the fixed size fractional part needs to be reduced.

To support a maximum queue length of 2048, we need to reduce the size of the fractional part to nine bits, for 4096 reduce the fractional part to eight bits etc.

Hence the "rte_red_scaling -= count - RTE_RED_SCALING;"

It is just coincidence that RTE_RED_SCALING is 10 and 1024 = 2**10.  

I hope that's clear, it took me a time to get my head around it again.

Regards
Alan

Re: [dpdk-dev] [PATCH v5] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Dewar, Alan [mailto:ad759e@intl.att.com]
> Sent: Friday, January 12, 2018 10:45 AM
> 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>;
> Kantecki, Tomasz <tomasz.kantecki@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Subject: RE: [PATCH v5] sched: make RED scaling configurable
> 
> Hi Cristian,
> 
> > > +
> > > +	rte_red_scaling -= count - RTE_RED_SCALING;
> >
> > Why not simply: rte_red_scaling = count?
> 
> The RED code stores the moving average in a uint32_t as a pseudo floating
> point number with a fixed sized fractional part of 10 bits.
> This allows a maximum queue length of 1024.   To support larger queues,
> the size of the fixed size fractional part needs to be reduced.
> 
> To support a maximum queue length of 2048, we need to reduce the size of
> the fractional part to nine bits, for 4096 reduce the fractional part to eight
> bits etc.
> 
> Hence the "rte_red_scaling -= count - RTE_RED_SCALING;"
> 
> It is just coincidence that RTE_RED_SCALING is 10 and 1024 = 2**10.
> 
> I hope that's clear, it took me a time to get my head around it again.
> 
> Regards
> Alan

Hi Alan,

After taking to Tomasz, I can also confirm you're right, so let's keep your initial proposal here.

This line of code will only work if this function is called once at most (this was my rationale to suggest the change), but this is fine, as this is enforced by testing the red_init flag when this function starts.

Regards,
Cristian

[dpdk-dev] [PATCH v6] 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>
---
 doc/guides/rel_notes/release_18_02.rst |   2 +-
 lib/librte_sched/Makefile              |   2 +-
 lib/librte_sched/rte_red.c             |  47 +++++-
 lib/librte_sched/rte_red.h             |  63 ++++++--
 lib/librte_sched/rte_sched_version.map |   7 +
 test/test/test_red.c                   | 274 ++++++++++++++++++++++++++++++++-
 6 files changed, 371 insertions(+), 24 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_02.rst b/doc/guides/rel_notes/release_18_02.rst
index 24b67bb..1bac48f 100644
--- a/doc/guides/rel_notes/release_18_02.rst
+++ b/doc/guides/rel_notes/release_18_02.rst
@@ -158,7 +158,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_power.so.1
      librte_reorder.so.1
      librte_ring.so.1
-     librte_sched.so.1
+     librte_sched.so.2
      librte_security.so.1
      librte_table.so.3
      librte_timer.so.1
diff --git a/lib/librte_sched/Makefile b/lib/librte_sched/Makefile
index 04785f7..db12d92 100644
--- a/lib/librte_sched/Makefile
+++ b/lib/librte_sched/Makefile
@@ -48,7 +48,7 @@ LDLIBS += -lrte_timer
 
 EXPORT_MAP := rte_sched_version.map
 
-LIBABIVER := 1
+LIBABIVER := 2
 
 #
 # all source are stored in SRCS-y
diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
index ade57d1..a47a2fa 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 */
+uint32_t rte_red_scaling = RTE_RED_SCALING;
+uint32_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,46 @@ 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)
+{
+	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;
+
+	rte_red_scaling = RTE_RED_SCALING - (rte_bsf32(max_red_queue_length) -
+					     RTE_RED_SCALING);
+	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;
+	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 6edf914..5e8c990 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                     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 uint32_t rte_red_scaling;
+extern uint32_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, this
+ *        function can only be called once and must be called before any
+ *        RED queues are initialised.
+ *
+ * @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 all RED global variables - only for use by RED unit-tests
+ */
+void
+__rte_red_reset_scaling(void);
+
+/**
  * @brief Generate random number for RED
  *
  * Implementation 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..56c4a52 100644
--- a/lib/librte_sched/rte_sched_version.map
+++ b/lib/librte_sched/rte_sched_version.map
@@ -29,3 +29,10 @@ DPDK_2.1 {
 	rte_sched_port_pkt_read_color;
 
 } DPDK_2.0;
+
+DPDK_18.02 {
+	global;
+
+	rte_red_set_scaling;
+	__rte_red_reset_scaling;
+} DPDK_2.1;
diff --git a/test/test/test_red.c b/test/test/test_red.c
index 348075d..0c95e84 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.7.4

Re: [dpdk-dev] [PATCH v6] sched: make RED scaling configurable

[Stephen Hemminger]

On Mon, 15 Jan 2018 16:16:09 +0000
alangordondewar@gmail.com wrote:

Looks like a good idea, minor editing feedback.


> -	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);

While you are at it remove unnecessary parenthesis here.

> +	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> +		rte_red_scaling;

It reads easier if the the shift operator on the next line

	red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv)
		 << rte_red_scaling;

Why do functional tests have to be in same file and clutter the code?

Re: [dpdk-dev] [PATCH v6] sched: make RED scaling configurable

[Alan Dewar]

On Mon, Jan 15, 2018 at 4:52 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 15 Jan 2018 16:16:09 +0000
> alangordondewar@gmail.com wrote:
>
> Looks like a good idea, minor editing feedback.
>
>
> > -     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);
>
> While you are at it remove unnecessary parenthesis here.
>

Okay will do.

> > +     red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> > +             rte_red_scaling;
>
> It reads easier if the the shift operator on the next line
>
>         red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv)
>                  << rte_red_scaling;
>

Okay will do.

> Why do functional tests have to be in same file and clutter the code?

Do you mean the same patch file or the same unit-test file?

I received feedback previously (might not have been this patch) where
I had split the functional changes and the new unit-tests into
separate patches and was asked to combine them into a single patch.  I
like the idea of if you have the fix you also have the new unit-tests.

As for having the new tests in the same file as existing tests, the
red tests are table driven so most of the changes to the unit-test
code are just adding new table entries to exercise the RED code with a
different scaling factor.

Re: [dpdk-dev] [PATCH v6] sched: make RED scaling configurable

[Alan Dewar]

On Tue, Jan 16, 2018 at 3:50 PM, Alan Dewar <alangordondewar@gmail.com> wrote:
> On Mon, Jan 15, 2018 at 4:52 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>>
>> On Mon, 15 Jan 2018 16:16:09 +0000
>> alangordondewar@gmail.com wrote:
>>
>> Looks like a good idea, minor editing feedback.
>>
>>
>> > -     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);
>>
>> While you are at it remove unnecessary parenthesis here.
>>
>
> Okay will do.

Ah - the compiler doesn't like it if I remove all the unnecessary
parenthesis. it gives the following error:

/home/adewar/git-repos/dpdk/lib/librte_sched/rte_red.c:153:49: error:
suggest parentheses around ‘+’ inside ‘<<’ [-Werror=parentheses]
  red_cfg->min_th = (uint32_t) min_th << wq_log2 + rte_red_scaling;

I'll reinstate the ones around the addition.

>
>> > +     red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
>> > +             rte_red_scaling;
>>
>> It reads easier if the the shift operator on the next line
>>
>>         red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv)
>>                  << rte_red_scaling;
>>
>
> Okay will do.
>
>> Why do functional tests have to be in same file and clutter the code?
>
> Do you mean the same patch file or the same unit-test file?
>
> I received feedback previously (might not have been this patch) where
> I had split the functional changes and the new unit-tests into
> separate patches and was asked to combine them into a single patch.  I
> like the idea of if you have the fix you also have the new unit-tests.
>
> As for having the new tests in the same file as existing tests, the
> red tests are table driven so most of the changes to the unit-test
> code are just adding new table entries to exercise the RED code with a
> different scaling factor.

Re: [dpdk-dev] [PATCH v6] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Alan Dewar [mailto:alangordondewar@gmail.com]
> Sent: Tuesday, January 16, 2018 3:58 PM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Kantecki, Tomasz
> <tomasz.kantecki@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; dev@dpdk.org; Alan Dewar
> <alan.dewar@att.com>
> Subject: Re: [dpdk-dev] [PATCH v6] sched: make RED scaling configurable
> 
> On Tue, Jan 16, 2018 at 3:50 PM, Alan Dewar
> <alangordondewar@gmail.com> wrote:
> > On Mon, Jan 15, 2018 at 4:52 PM, Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> >>
> >> On Mon, 15 Jan 2018 16:16:09 +0000
> >> alangordondewar@gmail.com wrote:
> >>
> >> Looks like a good idea, minor editing feedback.
> >>
> >>
> >> > -     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);
> >>
> >> While you are at it remove unnecessary parenthesis here.
> >>
> >
> > Okay will do.
> 
> Ah - the compiler doesn't like it if I remove all the unnecessary
> parenthesis. it gives the following error:
> 
> /home/adewar/git-repos/dpdk/lib/librte_sched/rte_red.c:153:49: error:
> suggest parentheses around ‘+’ inside ‘<<’ [-Werror=parentheses]
>   red_cfg->min_th = (uint32_t) min_th << wq_log2 + rte_red_scaling;
> 
> I'll reinstate the ones around the addition.

Alan,

I have a different view here, I suggest we keep the parenthesis as they are, as the code is easier to read and maintain. RED is not the code where you want to play with compiler typecasts and arithmetic.

Regards,
Cristian

[dpdk-dev] [PATCH v7] 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>
---
 doc/guides/rel_notes/release_18_02.rst |   2 +-
 lib/librte_sched/Makefile              |   2 +-
 lib/librte_sched/rte_red.c             |  47 +++++-
 lib/librte_sched/rte_red.h             |  63 ++++++--
 lib/librte_sched/rte_sched_version.map |   7 +
 test/test/test_red.c                   | 274 ++++++++++++++++++++++++++++++++-
 6 files changed, 371 insertions(+), 24 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_02.rst b/doc/guides/rel_notes/release_18_02.rst
index 24b67bb..1bac48f 100644
--- a/doc/guides/rel_notes/release_18_02.rst
+++ b/doc/guides/rel_notes/release_18_02.rst
@@ -158,7 +158,7 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_power.so.1
      librte_reorder.so.1
      librte_ring.so.1
-     librte_sched.so.1
+     librte_sched.so.2
      librte_security.so.1
      librte_table.so.3
      librte_timer.so.1
diff --git a/lib/librte_sched/Makefile b/lib/librte_sched/Makefile
index 04785f7..db12d92 100644
--- a/lib/librte_sched/Makefile
+++ b/lib/librte_sched/Makefile
@@ -48,7 +48,7 @@ LDLIBS += -lrte_timer
 
 EXPORT_MAP := rte_sched_version.map
 
-LIBABIVER := 1
+LIBABIVER := 2
 
 #
 # all source are stored in SRCS-y
diff --git a/lib/librte_sched/rte_red.c b/lib/librte_sched/rte_red.c
index ade57d1..a47a2fa 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 */
+uint32_t rte_red_scaling = RTE_RED_SCALING;
+uint32_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,46 @@ 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)
+{
+	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;
+
+	rte_red_scaling = RTE_RED_SCALING - (rte_bsf32(max_red_queue_length) -
+					     RTE_RED_SCALING);
+	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;
+	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 6edf914..5e8c990 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                     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 uint32_t rte_red_scaling;
+extern uint32_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, this
+ *        function can only be called once and must be called before any
+ *        RED queues are initialised.
+ *
+ * @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 all RED global variables - only for use by RED unit-tests
+ */
+void
+__rte_red_reset_scaling(void);
+
+/**
  * @brief Generate random number for RED
  *
  * Implementation 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..56c4a52 100644
--- a/lib/librte_sched/rte_sched_version.map
+++ b/lib/librte_sched/rte_sched_version.map
@@ -29,3 +29,10 @@ DPDK_2.1 {
 	rte_sched_port_pkt_read_color;
 
 } DPDK_2.0;
+
+DPDK_18.02 {
+	global;
+
+	rte_red_set_scaling;
+	__rte_red_reset_scaling;
+} DPDK_2.1;
diff --git a/test/test/test_red.c b/test/test/test_red.c
index 348075d..0c95e84 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.7.4

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Ferruh Yigit]

On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> 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>

Hi Cristian, Alan,

The v7 of this patch is sting without any comment for more than a year.
What is the status of this patch? Is it still valid? What is blocking it?

For reference patch:
https://patches.dpdk.org/patch/33837/

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Ferruh Yigit]

On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> 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>

Hi Cristian, Alan,

The v7 of this patch is sting without any comment for more than a year.
What is the status of this patch? Is it still valid? What is blocking it?

For reference patch:
https://patches.dpdk.org/patch/33837/

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Alan Dewar]

Hi Ferruh,

We are still using this patch against DPDK 17.11 and 18.11 as part of
the AT&T Vyatta NOS.   It is needed to make WRED queues longer than
1024 packets work correctly.  I'm afraid that I have no idea what is
holding it up from being merged.

Regards
Alan

On Fri, Apr 5, 2019 at 4:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> > 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>
>
> Hi Cristian, Alan,
>
> The v7 of this patch is sting without any comment for more than a year.
> What is the status of this patch? Is it still valid? What is blocking it?
>
> For reference patch:
> https://patches.dpdk.org/patch/33837/

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Alan Dewar]

Hi Ferruh,

We are still using this patch against DPDK 17.11 and 18.11 as part of
the AT&T Vyatta NOS.   It is needed to make WRED queues longer than
1024 packets work correctly.  I'm afraid that I have no idea what is
holding it up from being merged.

Regards
Alan

On Fri, Apr 5, 2019 at 4:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> > 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>
>
> Hi Cristian, Alan,
>
> The v7 of this patch is sting without any comment for more than a year.
> What is the status of this patch? Is it still valid? What is blocking it?
>
> For reference patch:
> https://patches.dpdk.org/patch/33837/

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Thomas Monjalon]

08/04/2019 10:24, Alan Dewar:
> On Fri, Apr 5, 2019 at 4:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> > > 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>
> >
> > Hi Cristian, Alan,
> >
> > The v7 of this patch is sting without any comment for more than a year.
> > What is the status of this patch? Is it still valid? What is blocking it?
> >
> > For reference patch:
> > https://patches.dpdk.org/patch/33837/
> 
> We are still using this patch against DPDK 17.11 and 18.11 as part of
> the AT&T Vyatta NOS.   It is needed to make WRED queues longer than
> 1024 packets work correctly.  I'm afraid that I have no idea what is
> holding it up from being merged.

It will be in a release when it will be merged in the git tree
dpdk-next-qos, managed by Cristian.

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Thomas Monjalon]

08/04/2019 10:24, Alan Dewar:
> On Fri, Apr 5, 2019 at 4:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> > > 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>
> >
> > Hi Cristian, Alan,
> >
> > The v7 of this patch is sting without any comment for more than a year.
> > What is the status of this patch? Is it still valid? What is blocking it?
> >
> > For reference patch:
> > https://patches.dpdk.org/patch/33837/
> 
> We are still using this patch against DPDK 17.11 and 18.11 as part of
> the AT&T Vyatta NOS.   It is needed to make WRED queues longer than
> 1024 packets work correctly.  I'm afraid that I have no idea what is
> holding it up from being merged.

It will be in a release when it will be merged in the git tree
dpdk-next-qos, managed by Cristian.

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, April 8, 2019 9:53 AM
> To: Alan Dewar <alangordondewar@gmail.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; Kantecki, Tomasz
> <tomasz.kantecki@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; dev@dpdk.org; Alan Dewar
> <alan.dewar@att.com>
> Subject: Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable
> 
> 08/04/2019 10:24, Alan Dewar:
> > On Fri, Apr 5, 2019 at 4:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> > > > 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>
> > >
> > > Hi Cristian, Alan,
> > >
> > > The v7 of this patch is sting without any comment for more than a year.
> > > What is the status of this patch? Is it still valid? What is blocking it?
> > >
> > > For reference patch:
> > > https://patches.dpdk.org/patch/33837/
> >
> > We are still using this patch against DPDK 17.11 and 18.11 as part of
> > the AT&T Vyatta NOS.   It is needed to make WRED queues longer than
> > 1024 packets work correctly.  I'm afraid that I have no idea what is
> > holding it up from being merged.
> 
> It will be in a release when it will be merged in the git tree
> dpdk-next-qos, managed by Cristian.
> 

I was hoping to get a review & ACK from Tomasz Kantecki, the author of the WRED code in DPDK, hence the lack of progress on this patch.

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, April 8, 2019 9:53 AM
> To: Alan Dewar <alangordondewar@gmail.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; Kantecki, Tomasz
> <tomasz.kantecki@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; dev@dpdk.org; Alan Dewar
> <alan.dewar@att.com>
> Subject: Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable
> 
> 08/04/2019 10:24, Alan Dewar:
> > On Fri, Apr 5, 2019 at 4:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> > > > 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>
> > >
> > > Hi Cristian, Alan,
> > >
> > > The v7 of this patch is sting without any comment for more than a year.
> > > What is the status of this patch? Is it still valid? What is blocking it?
> > >
> > > For reference patch:
> > > https://patches.dpdk.org/patch/33837/
> >
> > We are still using this patch against DPDK 17.11 and 18.11 as part of
> > the AT&T Vyatta NOS.   It is needed to make WRED queues longer than
> > 1024 packets work correctly.  I'm afraid that I have no idea what is
> > holding it up from being merged.
> 
> It will be in a release when it will be merged in the git tree
> dpdk-next-qos, managed by Cristian.
> 

I was hoping to get a review & ACK from Tomasz Kantecki, the author of the WRED code in DPDK, hence the lack of progress on this patch.

Re: [dpdk-dev] [PATCH v7] sched: make RED scaling configurable

[Thomas Monjalon]

08/04/2019 15:29, Dumitrescu, Cristian:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 08/04/2019 10:24, Alan Dewar:
> > > On Fri, Apr 5, 2019 at 4:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > > On 1/16/2018 4:07 PM, alangordondewar@gmail.com wrote:
> > > > > 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>
> > > >
> > > > Hi Cristian, Alan,
> > > >
> > > > The v7 of this patch is sting without any comment for more than a year.
> > > > What is the status of this patch? Is it still valid? What is blocking it?
> > > >
> > > > For reference patch:
> > > > https://patches.dpdk.org/patch/33837/
> > >
> > > We are still using this patch against DPDK 17.11 and 18.11 as part of
> > > the AT&T Vyatta NOS.   It is needed to make WRED queues longer than
> > > 1024 packets work correctly.  I'm afraid that I have no idea what is
> > > holding it up from being merged.
> > 
> > It will be in a release when it will be merged in the git tree
> > dpdk-next-qos, managed by Cristian.
> 
> I was hoping to get a review & ACK from Tomasz Kantecki, the author of the WRED code in DPDK, hence the lack of progress on this patch.

It seems nobody was able to provide an feedback after two years,
and it was never merged in the QoS git tree.
The handling of this patch is really a shame.

Alan, please rebase this patch.
If nothing is wrong in CI (including ABI check),
I will merge the next version.