[PATCH v2 0/4] sched: HQoS Library cleanup

[PATCH v2 0/4] sched: HQoS Library cleanup

[Megha Ajmera]

v2:
* Rebased with latest main branch code resolved conflicts.

v1:
This patchset involves the cleanup of HQoS Library:

* Removed unused HQoS #defines from rte_config.
  RTE_SCHED_CMAN, RTE_SCHED_COLLECT_STATS, RTE_SCHED_SUBPORT_TC_OV, RTE_SCHED_VECTOR

* RTE_SCHED_COLLECT_STATS flag is removed from the code.
  Stats collection is now always enabled.

* RTE_SCHED_SUBPORT_TC_OV flag is removed.
  TC subscription for best effort queues is always enabled in HQoS library.

* RTE_SCHED_VECTOR flag is removed from HQoS library as the code under this
  flag is no longer useful.


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                        |   8 +-
 doc/guides/sample_app_ug/qos_scheduler.rst |   3 +-
 lib/sched/rte_sched.c                      | 156 +--------------------
 3 files changed, 5 insertions(+), 162 deletions(-)

-- 
2.25.1

[PATCH v2 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                        | 8 ++------
 doc/guides/sample_app_ug/qos_scheduler.rst | 3 +--
 lib/sched/rte_sched.c                      | 4 ++++
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index 91d96eeecb..917097630e 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -88,12 +88,8 @@
 /* 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
 
 /* rte_graph defines */
 #define RTE_GRAPH_BURST_SIZE 256
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

[PATCH v2 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 v2 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 v2 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 v2 1/4] sched: Cleanup qos scheduler defines from rte_config

[Thomas Monjalon]

18/02/2022 10:36, 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                        | 8 ++------
>  doc/guides/sample_app_ug/qos_scheduler.rst | 3 +--
>  lib/sched/rte_sched.c                      | 4 ++++
>  3 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/config/rte_config.h b/config/rte_config.h
> index 91d96eeecb..917097630e 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -88,12 +88,8 @@
>  /* rte_power defines */
>  #define RTE_MAX_LCORE_FREQS 64
>  
> -/* rte_sched defines */
> -#undef RTE_SCHED_CMAN

So what is the purpose of the code under RTE_SCHED_CMAN #ifdef?
Is it a dead code? Should it be enabled with a hidden option?

> -#undef RTE_SCHED_COLLECT_STATS

RTE_SCHED_COLLECT_STATS should be removed from config
in the same patch removing the #ifdef in the code.

> -#undef RTE_SCHED_SUBPORT_TC_OV

RTE_SCHED_SUBPORT_TC_OV should be removed from config
in the same patch removing the #ifdef in the code.

> -#define RTE_SCHED_PORT_N_GRINDERS 8
> -#undef RTE_SCHED_VECTOR

RTE_SCHED_VECTOR should be removed while removing code in patch 4.
Maybe start the series with patch 4.
A good cleanup series starts with removing useless code.
While removing, you should justify in the commit log why it is useless.

> +/* KNI defines */
> +#define RTE_KNI_PREEMPT_DEFAULT 1

The KNI addition is unrelated.
>  
>  /* rte_graph defines */
>  #define RTE_GRAPH_BURST_SIZE 256
> 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.

No we should not modify rte_config.h
It should be modified via CFLAGS.

> +#ifndef RTE_SCHED_PORT_N_GRINDERS
> +#define RTE_SCHED_PORT_N_GRINDERS 8
> +#endif

Do you expect users to modify it?
Given the #ifndef, it seems yes.
So you should document it with CFLAGS.

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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Friday, February 18, 2022 9:37 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net; david.marchand@redhat.com
> Subject: [PATCH v2 0/4] sched: HQoS Library cleanup
> 
> v2:
> * Rebased with latest main branch code resolved conflicts.
> 
> v1:
> This patchset involves the cleanup of HQoS Library:
> 
> * Removed unused HQoS #defines from rte_config.
>   RTE_SCHED_CMAN, RTE_SCHED_COLLECT_STATS,
> RTE_SCHED_SUBPORT_TC_OV, RTE_SCHED_VECTOR
> 
> * RTE_SCHED_COLLECT_STATS flag is removed from the code.
>   Stats collection is now always enabled.
> 
> * RTE_SCHED_SUBPORT_TC_OV flag is removed.
>   TC subscription for best effort queues is always enabled in HQoS library.
> 
> * RTE_SCHED_VECTOR flag is removed from HQoS library as the code under
> this
>   flag is no longer useful.
> 
> 
> 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                        |   8 +-
>  doc/guides/sample_app_ug/qos_scheduler.rst |   3 +-
>  lib/sched/rte_sched.c                      | 156 +--------------------
>  3 files changed, 5 insertions(+), 162 deletions(-)
> 
> --
> 2.25.1

Do people feel the need to have a cover letter for this patchset? IMO it does not add any useful info, so my vote is to discard the cover letter.

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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Friday, February 18, 2022 9:37 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net; david.marchand@redhat.com
> Subject: [PATCH v2 2/4] sched: Always enable stats in HQoS library.
> 
> 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

Please adjust the patch title to meet the requirements:
-start with a verb
-do not start with an upper letter

And also please do not mention HQoS anywhere (title, body, ...), the name of the library is sched, not HQoS.

Title proposal:
	sched: enable statistics unconditionally

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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Friday, February 18, 2022 9:37 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net; david.marchand@redhat.com
> Subject: [PATCH v2 3/4] sched: Always enable best effort TC oversubscription
> in HQoS library.
> 
> 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

Same comments on the title:
	sched: enable traffic class oversubscription unconditionally

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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Friday, February 18, 2022 9:37 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net; david.marchand@redhat.com
> Subject: [PATCH v2 4/4] sched: Removed code defined under VECTOR
> Defines.
> 
> 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

Same title comments:
	sched: remove code no longer needed

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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Friday, February 18, 2022 9:37 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net; david.marchand@redhat.com
> Subject: [PATCH v2 1/4] sched: Cleanup qos scheduler defines from
> rte_config
> 
> 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                        | 8 ++------
>  doc/guides/sample_app_ug/qos_scheduler.rst | 3 +--
>  lib/sched/rte_sched.c                      | 4 ++++
>  3 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/config/rte_config.h b/config/rte_config.h
> index 91d96eeecb..917097630e 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -88,12 +88,8 @@
>  /* 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
> 
>  /* rte_graph defines */
>  #define RTE_GRAPH_BURST_SIZE 256
> 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

I agree with Thomas's suggestion melt this patch into the other patches.

You would need to have an additional patch for RTE_SCHED_PORT_N_GRINDERS.

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

[Dumitrescu, Cristian]

> > +/* KNI defines */
> > +#define RTE_KNI_PREEMPT_DEFAULT 1
> 
> The KNI addition is unrelated.
> >

Why these KNI lines, hopefully it is not intentional and it will get removed in the next version?

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

[Dumitrescu, Cristian]

> > -/* rte_sched defines */
> > -#undef RTE_SCHED_CMAN
> 
> So what is the purpose of the code under RTE_SCHED_CMAN #ifdef?
> Is it a dead code? Should it be enabled with a hidden option?
> 

We want to keep congestion management enabled through CFLAGS for now, as we have a bit more homework to do on understanding the performance impact to enable the feature at build-time; we'll follow-up on this for next release. Right, Megha?

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

[Thomas Monjalon]

18/02/2022 11:58, Dumitrescu, Cristian:
> 
> > -----Original Message-----
> > From: Ajmera, Megha <megha.ajmera@intel.com>
> > Sent: Friday, February 18, 2022 9:37 AM
> > To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> > thomas@monjalon.net; david.marchand@redhat.com
> > Subject: [PATCH v2 0/4] sched: HQoS Library cleanup
> > 
> > v2:
> > * Rebased with latest main branch code resolved conflicts.
> > 
> > v1:
> > This patchset involves the cleanup of HQoS Library:
> > 
> > * Removed unused HQoS #defines from rte_config.
> >   RTE_SCHED_CMAN, RTE_SCHED_COLLECT_STATS,
> > RTE_SCHED_SUBPORT_TC_OV, RTE_SCHED_VECTOR
> > 
> > * RTE_SCHED_COLLECT_STATS flag is removed from the code.
> >   Stats collection is now always enabled.
> > 
> > * RTE_SCHED_SUBPORT_TC_OV flag is removed.
> >   TC subscription for best effort queues is always enabled in HQoS library.
> > 
> > * RTE_SCHED_VECTOR flag is removed from HQoS library as the code under
> > this
> >   flag is no longer useful.
> > 
> > 
> > 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                        |   8 +-
> >  doc/guides/sample_app_ug/qos_scheduler.rst |   3 +-
> >  lib/sched/rte_sched.c                      | 156 +--------------------
> >  3 files changed, 5 insertions(+), 162 deletions(-)
> > 
> > --
> > 2.25.1
> 
> Do people feel the need to have a cover letter for this patchset? IMO it does not add any useful info, so my vote is to discard the cover letter.

Yes cover letter is needed.
It is good to have a summary,
and it helps to have a clean threading in emails.
The cover letter should be a reply to the cover letter of the first version.

[PATCH v3 0/4] sched: cleanup of sched library

[Megha Ajmera]

v3:
This patchset involves the cleanup of sched library:

* Addresses review comments on v2 patchset.

* RTE_SCHED_CMAN is left unmodified in rte_config.h.
  Cleanup of this will be taken up later.

* Removed unused flag RTE_SCHED_VECTOR from arm/meson.build. Only
  scalar version is now supported.

* Added grinder configuration in docs. The configuration is moved from
  rte_config.h into sched library.  Default number of grinders is 8.
  To override the default, specify RTE_SCHED_PORT_N_GRINDERS=N in CFLAGS
  before compiling sched library.

* Sample app is updated to always collect statistics as this flag is
  removed.

* Updated softnic library by enabling TC oversubscription.
  This flag is now removed from sched.

v2:
This patchset involves the cleanup of sched Library:

* Removed unused sched #defines from rte_config.
  RTE_SCHED_CMAN, RTE_SCHED_COLLECT_STATS, RTE_SCHED_SUBPORT_TC_OV and
  RTE_SCHED_VECTOR.

* RTE_SCHED_COLLECT_STATS flag is removed from the code.
  Stats collection is now always enabled.

* RTE_SCHED_SUBPORT_TC_OV flag is removed.
  TC over subscription for best effort queues is now always enabled.

* RTE_SCHED_VECTOR flag is removed from sched library as the code under this
  flag is no longer useful. Only scalar version is supported.

* Rebased with latest main branch code.

Megha Ajmera (4):
  sched: remove code no longer needed
  sched: move grinder configuration flag
  sched: enable statistics unconditionally
  sched: enable traffic class oversubscription unconditionally

 config/arm/meson.build                     |   1 -
 config/rte_config.h                        |   4 -
 doc/guides/sample_app_ug/qos_scheduler.rst |   5 +-
 drivers/net/softnic/rte_eth_softnic_tm.c   |  18 ---
 examples/qos_sched/init.c                  |   2 -
 lib/sched/rte_sched.c                      | 156 +--------------------
 6 files changed, 4 insertions(+), 182 deletions(-)

-- 
2.25.1

[PATCH v3 1/4] sched: remove code no longer needed

[Megha Ajmera]

Remove RTE_SCHED_VECTOR flag from rte_config.h.
This flag is no longer useful. Only scalar version is supported.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 config/arm/meson.build |  1 -
 config/rte_config.h    |  1 -
 lib/sched/rte_sched.c  | 54 ------------------------------------------
 3 files changed, 56 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 48b88a84f2..8aead74086 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -17,7 +17,6 @@ flags_common = [
         #    ['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
         #    ['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
 
-        ['RTE_SCHED_VECTOR', false],
         ['RTE_ARM_USE_WFE', false],
         ['RTE_ARCH_ARM64', true],
         ['RTE_CACHE_LINE_SIZE', 128]
diff --git a/config/rte_config.h b/config/rte_config.h
index 91d96eeecb..7a7da2f4e5 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -93,7 +93,6 @@
 #undef RTE_SCHED_COLLECT_STATS
 #undef RTE_SCHED_SUBPORT_TC_OV
 #define RTE_SCHED_PORT_N_GRINDERS 8
-#undef RTE_SCHED_VECTOR
 
 /* rte_graph defines */
 #define RTE_GRAPH_BURST_SIZE 256
diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 62b3d2e315..1d3051cc0f 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -24,17 +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
-
 #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
@@ -2545,47 +2534,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)
 {
@@ -2599,8 +2547,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

[PATCH v3 2/4] sched: move grinder configuration flag

[Megha Ajmera]

Grinder configuration is now moved to sched library.

Number of grinders can also modified by specifying
RTE_SCHED_PORT_N_GRINDERS=N in CFLAGS, where N is number of grinders.

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

diff --git a/config/rte_config.h b/config/rte_config.h
index 7a7da2f4e5..d449af4810 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -92,7 +92,6 @@
 #undef RTE_SCHED_CMAN
 #undef RTE_SCHED_COLLECT_STATS
 #undef RTE_SCHED_SUBPORT_TC_OV
-#define RTE_SCHED_PORT_N_GRINDERS 8
 
 /* rte_graph defines */
 #define RTE_GRAPH_BURST_SIZE 256
diff --git a/doc/guides/sample_app_ug/qos_scheduler.rst b/doc/guides/sample_app_ug/qos_scheduler.rst
index 49c14a00da..0782e41ee7 100644
--- a/doc/guides/sample_app_ug/qos_scheduler.rst
+++ b/doc/guides/sample_app_ug/qos_scheduler.rst
@@ -45,6 +45,11 @@ The application is located in the ``qos_sched`` sub-directory.
     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.
 
+.. note::
+
+    Number of grinders is currently set to 8. This can be modified by specifying RTE_SCHED_PORT_N_GRINDERS=N in
+    CFLAGS, where N is number of grinders.
+
 Running the Application
 -----------------------
 
diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 1d3051cc0f..9c85edb4cc 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -24,6 +24,10 @@
 #pragma warning(disable:2259) /* conversion may lose significant bits */
 #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

[PATCH v3 3/4] sched: enable statistics unconditionally

[Megha Ajmera]

Removed RTE_SCHED_COLLECT_STATS flag from rte_config.h.
Stats collection is always enabled.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 config/rte_config.h                        |  1 -
 doc/guides/sample_app_ug/qos_scheduler.rst |  6 ------
 lib/sched/rte_sched.c                      | 12 ------------
 3 files changed, 19 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index d449af4810..de6fea5b67 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -90,7 +90,6 @@
 
 /* rte_sched defines */
 #undef RTE_SCHED_CMAN
-#undef RTE_SCHED_COLLECT_STATS
 #undef RTE_SCHED_SUBPORT_TC_OV
 
 /* rte_graph defines */
diff --git a/doc/guides/sample_app_ug/qos_scheduler.rst b/doc/guides/sample_app_ug/qos_scheduler.rst
index 0782e41ee7..34b662b230 100644
--- a/doc/guides/sample_app_ug/qos_scheduler.rst
+++ b/doc/guides/sample_app_ug/qos_scheduler.rst
@@ -39,12 +39,6 @@ The application is located in the ``qos_sched`` sub-directory.
 
         This application is intended as a linux only.
 
-.. 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.
-
 .. note::
 
     Number of grinders is currently set to 8. This can be modified by specifying RTE_SCHED_PORT_N_GRINDERS=N in
diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 9c85edb4cc..8a051049de 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1779,8 +1779,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,
@@ -1838,8 +1836,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
@@ -1978,18 +1974,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;
 }
@@ -2031,12 +2023,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;
 	}
 
@@ -2048,10 +2038,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 v3 4/4] sched: enable traffic class oversubscription unconditionally

[Megha Ajmera]

Removed RTE_SCHED_SUBPORT_TC_OV from rte_config.h.
Best effort traffic class oversubscription is always enabled.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 config/rte_config.h                      |  1 -
 drivers/net/softnic/rte_eth_softnic_tm.c | 18 -----
 examples/qos_sched/init.c                |  2 -
 lib/sched/rte_sched.c                    | 90 ------------------------
 4 files changed, 111 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index de6fea5b67..8eb29c1525 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -90,7 +90,6 @@
 
 /* rte_sched defines */
 #undef RTE_SCHED_CMAN
-#undef RTE_SCHED_SUBPORT_TC_OV
 
 /* rte_graph defines */
 #define RTE_GRAPH_BURST_SIZE 256
diff --git a/drivers/net/softnic/rte_eth_softnic_tm.c b/drivers/net/softnic/rte_eth_softnic_tm.c
index e74092ce7f..6a7766ba1c 100644
--- a/drivers/net/softnic/rte_eth_softnic_tm.c
+++ b/drivers/net/softnic/rte_eth_softnic_tm.c
@@ -595,15 +595,9 @@ static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.sched_sp_n_priorities_max = 1,
 			.sched_wfq_n_children_per_group_max = UINT32_MAX,
 			.sched_wfq_n_groups_max = 1,
-#ifdef RTE_SCHED_SUBPORT_TC_OV
 			.sched_wfq_weight_max = UINT32_MAX,
 			.sched_wfq_packet_mode_supported = 0,
 			.sched_wfq_byte_mode_supported = 1,
-#else
-			.sched_wfq_weight_max = 1,
-			.sched_wfq_packet_mode_supported = 0,
-			.sched_wfq_byte_mode_supported = 1,
-#endif
 
 			.stats_mask = STATS_MASK_DEFAULT,
 		} },
@@ -2828,8 +2822,6 @@ pmd_tm_hierarchy_commit(struct rte_eth_dev *dev,
 	return 0;
 }
 
-#ifdef RTE_SCHED_SUBPORT_TC_OV
-
 static int
 update_pipe_weight(struct rte_eth_dev *dev, struct tm_node *np, uint32_t weight)
 {
@@ -2867,8 +2859,6 @@ update_pipe_weight(struct rte_eth_dev *dev, struct tm_node *np, uint32_t weight)
 	return 0;
 }
 
-#endif
-
 static int
 update_queue_weight(struct rte_eth_dev *dev,
 	struct tm_node *nq, uint32_t weight)
@@ -2983,7 +2973,6 @@ pmd_tm_node_parent_update(struct rte_eth_dev *dev,
 			rte_strerror(EINVAL));
 		/* fall-through */
 	case TM_NODE_LEVEL_PIPE:
-#ifdef RTE_SCHED_SUBPORT_TC_OV
 		if (update_pipe_weight(dev, n, weight))
 			return -rte_tm_error_set(error,
 				EINVAL,
@@ -2991,13 +2980,6 @@ pmd_tm_node_parent_update(struct rte_eth_dev *dev,
 				NULL,
 				rte_strerror(EINVAL));
 		return 0;
-#else
-		return -rte_tm_error_set(error,
-			EINVAL,
-			RTE_TM_ERROR_TYPE_NODE_WEIGHT,
-			NULL,
-			rte_strerror(EINVAL));
-#endif
 		/* fall-through */
 	case TM_NODE_LEVEL_TC:
 		return -rte_tm_error_set(error,
diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
index 3c1f0bc680..8a0fb8a374 100644
--- a/examples/qos_sched/init.c
+++ b/examples/qos_sched/init.c
@@ -183,9 +183,7 @@ static struct rte_sched_pipe_params pipe_profiles[MAX_SCHED_PIPE_PROFILES] = {
 		.tc_rate = {305175, 305175, 305175, 305175, 305175, 305175,
 			305175, 305175, 305175, 305175, 305175, 305175, 305175},
 		.tc_period = 40,
-#ifdef RTE_SCHED_SUBPORT_TC_OV
 		.tc_ov_weight = 1,
-#endif
 
 		.wrr_weights = {1, 1, 1, 1},
 	},
diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index 8a051049de..328b82c051 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1318,14 +1318,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
 	}
 
 	{
@@ -1345,11 +1343,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;
 
 	}
@@ -2256,50 +2252,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)
@@ -2394,46 +2346,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)
@@ -2480,8 +2392,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,
-- 
2.25.1

RE: [PATCH v3 0/4] sched: cleanup of sched library

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Tuesday, February 22, 2022 12:58 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net; david.marchand@redhat.com; Thakur, Sham Singh
> <sham.singh.thakur@intel.com>
> Subject: [PATCH v3 0/4] sched: cleanup of sched library
> 
> v3:
> This patchset involves the cleanup of sched library:
> 
> * Addresses review comments on v2 patchset.
> 
> * RTE_SCHED_CMAN is left unmodified in rte_config.h.
>   Cleanup of this will be taken up later.
> 
> * Removed unused flag RTE_SCHED_VECTOR from arm/meson.build. Only
>   scalar version is now supported.
> 
> * Added grinder configuration in docs. The configuration is moved from
>   rte_config.h into sched library.  Default number of grinders is 8.
>   To override the default, specify RTE_SCHED_PORT_N_GRINDERS=N in
> CFLAGS
>   before compiling sched library.
> 
> * Sample app is updated to always collect statistics as this flag is
>   removed.
> 
> * Updated softnic library by enabling TC oversubscription.
>   This flag is now removed from sched.
> 
> v2:
> This patchset involves the cleanup of sched Library:
> 
> * Removed unused sched #defines from rte_config.
>   RTE_SCHED_CMAN, RTE_SCHED_COLLECT_STATS,
> RTE_SCHED_SUBPORT_TC_OV and
>   RTE_SCHED_VECTOR.
> 
> * RTE_SCHED_COLLECT_STATS flag is removed from the code.
>   Stats collection is now always enabled.
> 
> * RTE_SCHED_SUBPORT_TC_OV flag is removed.
>   TC over subscription for best effort queues is now always enabled.
> 
> * RTE_SCHED_VECTOR flag is removed from sched library as the code under
> this
>   flag is no longer useful. Only scalar version is supported.
> 
> * Rebased with latest main branch code.
> 
> Megha Ajmera (4):
>   sched: remove code no longer needed
>   sched: move grinder configuration flag
>   sched: enable statistics unconditionally
>   sched: enable traffic class oversubscription unconditionally
> 
>  config/arm/meson.build                     |   1 -
>  config/rte_config.h                        |   4 -
>  doc/guides/sample_app_ug/qos_scheduler.rst |   5 +-
>  drivers/net/softnic/rte_eth_softnic_tm.c   |  18 ---
>  examples/qos_sched/init.c                  |   2 -
>  lib/sched/rte_sched.c                      | 156 +--------------------
>  6 files changed, 4 insertions(+), 182 deletions(-)
> 
> --
> 2.25.1

Series-acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Re: [PATCH v3 0/4] sched: cleanup of sched library

[Thomas Monjalon]

> > Megha Ajmera (4):
> >   sched: remove code no longer needed
> >   sched: move grinder configuration flag
> >   sched: enable statistics unconditionally
> >   sched: enable traffic class oversubscription unconditionally
> 
> Series-acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied, thanks.