[PATCH v1 0/4] sched: HQoS Library cleanup.

[PATCH v1 0/4] sched: HQoS Library cleanup.

[Megha Ajmera]

v1: 
* Removed unused HQoS #defines from rte_config.
* Enabled stats in HQoS by default.
* TC subscription for best effort queues is always enabled in HQoS library.
* VECTOR defines are removed from HQoS library.


Megha Ajmera (4):
  sched: Cleanup qos scheduler defines from rte_config
  sched: Always enable stats in HQoS library.
  sched: Always enable best effort TC oversubscription in HQoS library.
  sched: Removed code defined under VECTOR #Defines.

 config/rte_config.h                        |   7 -
 doc/guides/sample_app_ug/qos_scheduler.rst |   3 +-
 lib/sched/rte_sched.c                      | 156 +--------------------
 3 files changed, 3 insertions(+), 163 deletions(-)

-- 
2.25.1

[PATCH v1 1/4] sched: Cleanup qos scheduler defines from rte_config

[Megha Ajmera]

Cleanup of sched config options those are by-default not defined.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 config/rte_config.h                        | 7 -------
 doc/guides/sample_app_ug/qos_scheduler.rst | 3 +--
 lib/sched/rte_sched.c                      | 4 ++++
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index cab4390a97..917097630e 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -88,13 +88,6 @@
 /* rte_power defines */
 #define RTE_MAX_LCORE_FREQS 64
 
-/* rte_sched defines */
-#undef RTE_SCHED_CMAN
-#undef RTE_SCHED_COLLECT_STATS
-#undef RTE_SCHED_SUBPORT_TC_OV
-#define RTE_SCHED_PORT_N_GRINDERS 8
-#undef RTE_SCHED_VECTOR
-
 /* KNI defines */
 #define RTE_KNI_PREEMPT_DEFAULT 1
 
diff --git a/doc/guides/sample_app_ug/qos_scheduler.rst b/doc/guides/sample_app_ug/qos_scheduler.rst
index 49c14a00da..7016ca4078 100644
--- a/doc/guides/sample_app_ug/qos_scheduler.rst
+++ b/doc/guides/sample_app_ug/qos_scheduler.rst
@@ -42,8 +42,7 @@ The application is located in the ``qos_sched`` sub-directory.
 .. note::
 
     To get statistics on the sample app using the command line interface as described in the next section,
-    DPDK must be compiled defining *RTE_SCHED_COLLECT_STATS*, which can be done by changing the relevant
-    entry in the ``config/rte_config.h`` file.
+    DPDK must be compiled after defining *RTE_SCHED_COLLECT_STATS* in the ``config/rte_config.h`` file.
 
 Running the Application
 -----------------------
diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 62b3d2e315..6c3e3bb0bf 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -35,6 +35,10 @@
 
 #endif
 
+#ifndef RTE_SCHED_PORT_N_GRINDERS
+#define RTE_SCHED_PORT_N_GRINDERS 8
+#endif
+
 #define RTE_SCHED_TB_RATE_CONFIG_ERR          (1e-7)
 #define RTE_SCHED_WRR_SHIFT                   3
 #define RTE_SCHED_MAX_QUEUES_PER_TC           RTE_SCHED_BE_QUEUES_PER_PIPE
-- 
2.25.1

Re: [PATCH v1 1/4] sched: Cleanup qos scheduler defines from rte_config

[Stephen Hemminger]

On Fri, 18 Feb 2022 07:41:45 +0000
Megha Ajmera <megha.ajmera@intel.com> wrote:

> diff --git a/doc/guides/sample_app_ug/qos_scheduler.rst b/doc/guides/sample_app_ug/qos_scheduler.rst
> index 49c14a00da..7016ca4078 100644
> --- a/doc/guides/sample_app_ug/qos_scheduler.rst
> +++ b/doc/guides/sample_app_ug/qos_scheduler.rst
> @@ -42,8 +42,7 @@ The application is located in the ``qos_sched`` sub-directory.
>  .. note::
>  
>      To get statistics on the sample app using the command line interface as described in the next section,
> -    DPDK must be compiled defining *RTE_SCHED_COLLECT_STATS*, which can be done by changing the relevant
> -    entry in the ``config/rte_config.h`` file.
> +    DPDK must be compiled after defining *RTE_SCHED_COLLECT_STATS* in the ``config/rte_config.h`` file.

Please don't have a process that requires users to edit rte_config.h
If necessary use meson options?

[PATCH v1 2/4] sched: Always enable stats in HQoS library.

[Megha Ajmera]

Removed "RTE_SCHED_COLLECT_STATS" flag from HQoS.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 lib/sched/rte_sched.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 6c3e3bb0bf..6f2d85edc0 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1790,8 +1790,6 @@ rte_sched_port_queue_is_empty(struct rte_sched_subport *subport,
 
 #endif /* RTE_SCHED_DEBUG */
 
-#ifdef RTE_SCHED_COLLECT_STATS
-
 static inline void
 rte_sched_port_update_subport_stats(struct rte_sched_port *port,
 	struct rte_sched_subport *subport,
@@ -1849,8 +1847,6 @@ rte_sched_port_update_queue_stats_on_drop(struct rte_sched_subport *subport,
 #endif
 }
 
-#endif /* RTE_SCHED_COLLECT_STATS */
-
 #ifdef RTE_SCHED_CMAN
 
 static inline int
@@ -1989,18 +1985,14 @@ rte_sched_port_enqueue_qptrs_prefetch0(struct rte_sched_subport *subport,
 	struct rte_mbuf *pkt, uint32_t subport_qmask)
 {
 	struct rte_sched_queue *q;
-#ifdef RTE_SCHED_COLLECT_STATS
 	struct rte_sched_queue_extra *qe;
-#endif
 	uint32_t qindex = rte_mbuf_sched_queue_get(pkt);
 	uint32_t subport_queue_id = subport_qmask & qindex;
 
 	q = subport->queue + subport_queue_id;
 	rte_prefetch0(q);
-#ifdef RTE_SCHED_COLLECT_STATS
 	qe = subport->queue_extra + subport_queue_id;
 	rte_prefetch0(qe);
-#endif
 
 	return subport_queue_id;
 }
@@ -2042,12 +2034,10 @@ rte_sched_port_enqueue_qwa(struct rte_sched_port *port,
 	if (unlikely(rte_sched_port_cman_drop(port, subport, pkt, qindex, qlen) ||
 		     (qlen >= qsize))) {
 		rte_pktmbuf_free(pkt);
-#ifdef RTE_SCHED_COLLECT_STATS
 		rte_sched_port_update_subport_stats_on_drop(port, subport,
 			qindex, pkt, qlen < qsize);
 		rte_sched_port_update_queue_stats_on_drop(subport, qindex, pkt,
 			qlen < qsize);
-#endif
 		return 0;
 	}
 
@@ -2059,10 +2049,8 @@ rte_sched_port_enqueue_qwa(struct rte_sched_port *port,
 	rte_bitmap_set(subport->bmp, qindex);
 
 	/* Statistics */
-#ifdef RTE_SCHED_COLLECT_STATS
 	rte_sched_port_update_subport_stats(port, subport, qindex, pkt);
 	rte_sched_port_update_queue_stats(subport, qindex, pkt);
-#endif
 
 	return 1;
 }
-- 
2.25.1

[PATCH v1 3/4] sched: Always enable best effort TC oversubscription in HQoS library.

[Megha Ajmera]

Removed "RTE_SCHED_SUBPORT_TC_OV" flag from HQoS.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 lib/sched/rte_sched.c | 91 -------------------------------------------
 1 file changed, 91 deletions(-)

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 6f2d85edc0..807134b48d 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1329,14 +1329,12 @@ rte_sched_subport_config(struct rte_sched_port *port,
 		for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++)
 			s->grinder_base_bmp_pos[i] = RTE_SCHED_PIPE_INVALID;
 
-#ifdef RTE_SCHED_SUBPORT_TC_OV
 		/* TC oversubscription */
 		s->tc_ov_wm_min = port->mtu;
 		s->tc_ov_period_id = 0;
 		s->tc_ov = 0;
 		s->tc_ov_n = 0;
 		s->tc_ov_rate = 0;
-#endif
 	}
 
 	{
@@ -1356,11 +1354,9 @@ rte_sched_subport_config(struct rte_sched_port *port,
 			else
 				profile->tc_credits_per_period[i] = 0;
 
-#ifdef RTE_SCHED_SUBPORT_TC_OV
 		s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile->tc_period,
 							s->pipe_tc_be_rate_max);
 		s->tc_ov_wm = s->tc_ov_wm_max;
-#endif
 		s->profile = subport_profile_id;
 
 	}
@@ -2267,50 +2263,6 @@ rte_sched_port_enqueue(struct rte_sched_port *port, struct rte_mbuf **pkts,
 	return result;
 }
 
-#ifndef RTE_SCHED_SUBPORT_TC_OV
-
-static inline void
-grinder_credits_update(struct rte_sched_port *port,
-	struct rte_sched_subport *subport, uint32_t pos)
-{
-	struct rte_sched_grinder *grinder = subport->grinder + pos;
-	struct rte_sched_pipe *pipe = grinder->pipe;
-	struct rte_sched_pipe_profile *params = grinder->pipe_params;
-	struct rte_sched_subport_profile *sp = grinder->subport_params;
-	uint64_t n_periods;
-	uint32_t i;
-
-	/* Subport TB */
-	n_periods = (port->time - subport->tb_time) / sp->tb_period;
-	subport->tb_credits += n_periods * sp->tb_credits_per_period;
-	subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size);
-	subport->tb_time += n_periods * sp->tb_period;
-
-	/* Pipe TB */
-	n_periods = (port->time - pipe->tb_time) / params->tb_period;
-	pipe->tb_credits += n_periods * params->tb_credits_per_period;
-	pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size);
-	pipe->tb_time += n_periods * params->tb_period;
-
-	/* Subport TCs */
-	if (unlikely(port->time >= subport->tc_time)) {
-		for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++)
-			subport->tc_credits[i] = sp->tc_credits_per_period[i];
-
-		subport->tc_time = port->time + sp->tc_period;
-	}
-
-	/* Pipe TCs */
-	if (unlikely(port->time >= pipe->tc_time)) {
-		for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++)
-			pipe->tc_credits[i] = params->tc_credits_per_period[i];
-
-		pipe->tc_time = port->time + params->tc_period;
-	}
-}
-
-#else
-
 static inline uint64_t
 grinder_tc_ov_credits_update(struct rte_sched_port *port,
 	struct rte_sched_subport *subport, uint32_t pos)
@@ -2405,46 +2357,6 @@ grinder_credits_update(struct rte_sched_port *port,
 	}
 }
 
-#endif /* RTE_SCHED_TS_CREDITS_UPDATE, RTE_SCHED_SUBPORT_TC_OV */
-
-
-#ifndef RTE_SCHED_SUBPORT_TC_OV
-
-static inline int
-grinder_credits_check(struct rte_sched_port *port,
-	struct rte_sched_subport *subport, uint32_t pos)
-{
-	struct rte_sched_grinder *grinder = subport->grinder + pos;
-	struct rte_sched_pipe *pipe = grinder->pipe;
-	struct rte_mbuf *pkt = grinder->pkt;
-	uint32_t tc_index = grinder->tc_index;
-	uint64_t pkt_len = pkt->pkt_len + port->frame_overhead;
-	uint64_t subport_tb_credits = subport->tb_credits;
-	uint64_t subport_tc_credits = subport->tc_credits[tc_index];
-	uint64_t pipe_tb_credits = pipe->tb_credits;
-	uint64_t pipe_tc_credits = pipe->tc_credits[tc_index];
-	int enough_credits;
-
-	/* Check queue credits */
-	enough_credits = (pkt_len <= subport_tb_credits) &&
-		(pkt_len <= subport_tc_credits) &&
-		(pkt_len <= pipe_tb_credits) &&
-		(pkt_len <= pipe_tc_credits);
-
-	if (!enough_credits)
-		return 0;
-
-	/* Update port credits */
-	subport->tb_credits -= pkt_len;
-	subport->tc_credits[tc_index] -= pkt_len;
-	pipe->tb_credits -= pkt_len;
-	pipe->tc_credits[tc_index] -= pkt_len;
-
-	return 1;
-}
-
-#else
-
 static inline int
 grinder_credits_check(struct rte_sched_port *port,
 	struct rte_sched_subport *subport, uint32_t pos)
@@ -2491,9 +2403,6 @@ grinder_credits_check(struct rte_sched_port *port,
 	return 1;
 }
 
-#endif /* RTE_SCHED_SUBPORT_TC_OV */
-
-
 static inline int
 grinder_schedule(struct rte_sched_port *port,
 	struct rte_sched_subport *subport, uint32_t pos)
-- 
2.25.1

[PATCH v1 4/4] sched: Removed code defined under VECTOR Defines.

[Megha Ajmera]

Removed "RTE_SCHED_VECTOR" flag from HQoS.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 lib/sched/rte_sched.c | 53 -------------------------------------------
 1 file changed, 53 deletions(-)

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 807134b48d..8ad5ca7e05 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -24,16 +24,6 @@
 #pragma warning(disable:2259) /* conversion may lose significant bits */
 #endif
 
-#ifdef RTE_SCHED_VECTOR
-#include <rte_vect.h>
-
-#ifdef RTE_ARCH_X86
-#define SCHED_VECTOR_SSE4
-#elif defined(__ARM_NEON)
-#define SCHED_VECTOR_NEON
-#endif
-
-#endif
 
 #ifndef RTE_SCHED_PORT_N_GRINDERS
 #define RTE_SCHED_PORT_N_GRINDERS 8
@@ -2446,47 +2436,6 @@ grinder_schedule(struct rte_sched_port *port,
 	return 1;
 }
 
-#ifdef SCHED_VECTOR_SSE4
-
-static inline int
-grinder_pipe_exists(struct rte_sched_subport *subport, uint32_t base_pipe)
-{
-	__m128i index = _mm_set1_epi32(base_pipe);
-	__m128i pipes = _mm_load_si128((__m128i *)subport->grinder_base_bmp_pos);
-	__m128i res = _mm_cmpeq_epi32(pipes, index);
-
-	pipes = _mm_load_si128((__m128i *)(subport->grinder_base_bmp_pos + 4));
-	pipes = _mm_cmpeq_epi32(pipes, index);
-	res = _mm_or_si128(res, pipes);
-
-	if (_mm_testz_si128(res, res))
-		return 0;
-
-	return 1;
-}
-
-#elif defined(SCHED_VECTOR_NEON)
-
-static inline int
-grinder_pipe_exists(struct rte_sched_subport *subport, uint32_t base_pipe)
-{
-	uint32x4_t index, pipes;
-	uint32_t *pos = (uint32_t *)subport->grinder_base_bmp_pos;
-
-	index = vmovq_n_u32(base_pipe);
-	pipes = vld1q_u32(pos);
-	if (!vminvq_u32(veorq_u32(pipes, index)))
-		return 1;
-
-	pipes = vld1q_u32(pos + 4);
-	if (!vminvq_u32(veorq_u32(pipes, index)))
-		return 1;
-
-	return 0;
-}
-
-#else
-
 static inline int
 grinder_pipe_exists(struct rte_sched_subport *subport, uint32_t base_pipe)
 {
@@ -2500,8 +2449,6 @@ grinder_pipe_exists(struct rte_sched_subport *subport, uint32_t base_pipe)
 	return 0;
 }
 
-#endif /* RTE_SCHED_OPTIMIZATIONS */
-
 static inline void
 grinder_pcache_populate(struct rte_sched_subport *subport,
 	uint32_t pos, uint32_t bmp_pos, uint64_t bmp_slab)
-- 
2.25.1

Re: [PATCH v1 0/4] sched: HQoS Library cleanup.

[David Marchand]

On Fri, Feb 18, 2022 at 8:42 AM Megha Ajmera <megha.ajmera@intel.com> wrote:
>
> v1:
> * Removed unused HQoS #defines from rte_config.
> * Enabled stats in HQoS by default.
> * TC subscription for best effort queues is always enabled in HQoS library.
> * VECTOR defines are removed from HQoS library.

This series does not apply on main, so we won't get tests from CI.

Is this a followup of patches?
https://patchwork.dpdk.org/project/dpdk/patch/20220121181459.1599739-1-megha.ajmera@intel.com/
(<-- which should be marked superseded..)
https://patchwork.dpdk.org/project/dpdk/patch/20220125102105.1719667-1-megha.ajmera@intel.com/

I prefer to ask because I did not see much public discussion.
Commitlogs are poor/nonexistent and the cover letter does not explain
the rationale.


-- 
David Marchand

RE: [PATCH v1 0/4] sched: HQoS Library cleanup.

[Ajmera, Megha]

Hi David,

I have rebased patch-set with latest main branch code resolving the conflicts. Also updated cover letter.

Regards,
Megha

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Friday, February 18, 2022 1:42 PM
To: Ajmera, Megha <megha.ajmera@intel.com>
Cc: dev <dev@dpdk.org>; Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v1 0/4] sched: HQoS Library cleanup.

On Fri, Feb 18, 2022 at 8:42 AM Megha Ajmera <megha.ajmera@intel.com> wrote:
>
> v1:
> * Removed unused HQoS #defines from rte_config.
> * Enabled stats in HQoS by default.
> * TC subscription for best effort queues is always enabled in HQoS library.
> * VECTOR defines are removed from HQoS library.

This series does not apply on main, so we won't get tests from CI.

Is this a followup of patches?
https://patchwork.dpdk.org/project/dpdk/patch/20220121181459.1599739-1-megha.ajmera@intel.com/
(<-- which should be marked superseded..) https://patchwork.dpdk.org/project/dpdk/patch/20220125102105.1719667-1-megha.ajmera@intel.com/

I prefer to ask because I did not see much public discussion.
Commitlogs are poor/nonexistent and the cover letter does not explain the rationale.


--
David Marchand