diff mbox series

[v2,01/28] sched: update macros for flexible config

Message ID 20190625153217.24301-2-jasvinder.singh@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Cristian Dumitrescu
Headers
Series sched: feature enhancements |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Jasvinder Singh June 25, 2019, 3:31 p.m. UTC 
  Update macros to allow configuration flexiblity for pipe traffic
classes and queues, and subport level configuration of the pipe
parameters.

Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
Signed-off-by: Abraham Tovar <abrahamx.tovar@intel.com>
Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
---
 lib/librte_sched/rte_sched.h | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

Comments

Cristian Dumitrescu July 1, 2019, 7:04 p.m. UTC | #1 
> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Tuesday, June 25, 2019 4:32 PM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Tovar, AbrahamX
> <abrahamx.tovar@intel.com>; Krakowiak, LukaszX
> <lukaszx.krakowiak@intel.com>
> Subject: [PATCH v2 01/28] sched: update macros for flexible config
> 
> Update macros to allow configuration flexiblity for pipe traffic
> classes and queues, and subport level configuration of the pipe
> parameters.
> 
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> Signed-off-by: Abraham Tovar <abrahamx.tovar@intel.com>
> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> ---
>  lib/librte_sched/rte_sched.h | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
> index 9c55a787d..470a0036a 100644
> --- a/lib/librte_sched/rte_sched.h
> +++ b/lib/librte_sched/rte_sched.h
> @@ -52,7 +52,7 @@ extern "C" {
>   *	    multiple connections of same traffic class belonging to
>   *	    the same user;
>   *           - Weighted Round Robin (WRR) is used to service the
> - *	    queues within same pipe traffic class.
> + *	    queues within same pipe lowest priority traffic class (best-effort).
>   *
>   */
> 
> @@ -66,20 +66,32 @@ extern "C" {
>  #include "rte_red.h"
>  #endif
> 
> -/** Number of traffic classes per pipe (as well as subport).
> - * Cannot be changed.
> +/** Maximum number of queues per pipe.
> + * Note that the multiple queues (power of 2) can only be assigned to
> + * lowest priority (best-effort) traffic class. Other higher priority traffic
> + * classes can only have one queue.
> + * Can not change.
> + *
> + * @see struct rte_sched_subport_params
>   */
> -#define RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE    4
> +#define RTE_SCHED_QUEUES_PER_PIPE    16
> 
> -/** Number of queues per pipe traffic class. Cannot be changed. */
> -#define RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS    4
> +/** Number of WRR queues for best-effort traffic class per pipe.
> + *
> + * @see struct rte_sched_pipe_params
> + */
> +#define RTE_SCHED_BE_QUEUES_PER_PIPE    8

Should we have this as 8 or 4? I think we should limit this to 4, as 4 allows quick vectorization, while 8 is problematic.

We should also not have a run-time parameter for number of best effort queues, as this can be detected by checking the size of all best effort queues against 0. Of course, we should mandate that the enabled queues (with non-zero size) are contiguous.

> 
> -/** Number of queues per pipe. */
> -#define RTE_SCHED_QUEUES_PER_PIPE             \
> -	(RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE *     \
> -	RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS)
> +#define RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS    4
> +/** Number of traffic classes per pipe (as well as subport).
> + *
> + * @see struct rte_sched_subport_params
> + * @see struct rte_sched_pipe_params
> + */
> +#define RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE    \
> +(RTE_SCHED_QUEUES_PER_PIPE - RTE_SCHED_BE_QUEUES_PER_PIPE + 1)
> 
> -/** Maximum number of pipe profiles that can be defined per port.
> +/** Maximum number of pipe profiles that can be defined per subport.
>   * Compile-time configurable.
>   */
>  #ifndef RTE_SCHED_PIPE_PROFILES_PER_PORT
> @@ -95,6 +107,8 @@ extern "C" {
>   *
>   * The FCS is considered overhead only if not included in the packet
>   * length (field pkt_len of struct rte_mbuf).
> + *
> + * @see struct rte_sched_port_params
>   */
>  #ifndef RTE_SCHED_FRAME_OVERHEAD_DEFAULT
>  #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT      24
> --
> 2.21.0
Jasvinder Singh July 2, 2019, 1:26 p.m. UTC | #2 
> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Monday, July 1, 2019 8:05 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>; dev@dpdk.org
> Cc: Tovar, AbrahamX <abrahamx.tovar@intel.com>; Krakowiak, LukaszX
> <lukaszx.krakowiak@intel.com>
> Subject: RE: [PATCH v2 01/28] sched: update macros for flexible config
> 
> 
> 
> > -----Original Message-----
> > From: Singh, Jasvinder
> > Sent: Tuesday, June 25, 2019 4:32 PM
> > To: dev@dpdk.org
> > Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Tovar,
> > AbrahamX <abrahamx.tovar@intel.com>; Krakowiak, LukaszX
> > <lukaszx.krakowiak@intel.com>
> > Subject: [PATCH v2 01/28] sched: update macros for flexible config
> >
> > Update macros to allow configuration flexiblity for pipe traffic
> > classes and queues, and subport level configuration of the pipe
> > parameters.
> >
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > Signed-off-by: Abraham Tovar <abrahamx.tovar@intel.com>
> > Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> > ---
> >  lib/librte_sched/rte_sched.h | 36
> > +++++++++++++++++++++++++-----------
> >  1 file changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/librte_sched/rte_sched.h
> > b/lib/librte_sched/rte_sched.h index 9c55a787d..470a0036a 100644
> > --- a/lib/librte_sched/rte_sched.h
> > +++ b/lib/librte_sched/rte_sched.h
> > @@ -52,7 +52,7 @@ extern "C" {
> >   *	    multiple connections of same traffic class belonging to
> >   *	    the same user;
> >   *           - Weighted Round Robin (WRR) is used to service the
> > - *	    queues within same pipe traffic class.
> > + *	    queues within same pipe lowest priority traffic class (best-effort).
> >   *
> >   */
> >
> > @@ -66,20 +66,32 @@ extern "C" {
> >  #include "rte_red.h"
> >  #endif
> >
> > -/** Number of traffic classes per pipe (as well as subport).
> > - * Cannot be changed.
> > +/** Maximum number of queues per pipe.
> > + * Note that the multiple queues (power of 2) can only be assigned to
> > + * lowest priority (best-effort) traffic class. Other higher priority
> > +traffic
> > + * classes can only have one queue.
> > + * Can not change.
> > + *
> > + * @see struct rte_sched_subport_params
> >   */
> > -#define RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE    4
> > +#define RTE_SCHED_QUEUES_PER_PIPE    16
> >
> > -/** Number of queues per pipe traffic class. Cannot be changed. */
> > -#define RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS    4
> > +/** Number of WRR queues for best-effort traffic class per pipe.
> > + *
> > + * @see struct rte_sched_pipe_params
> > + */
> > +#define RTE_SCHED_BE_QUEUES_PER_PIPE    8
> 
> Should we have this as 8 or 4? I think we should limit this to 4, as 4 allows quick
> vectorization, while 8 is problematic.

The only reason to keep 8 queues for best effort TC is flexibility in reducing to 4 if needed, and moreover, it has very little impact on performance in out tests.    

> We should also not have a run-time parameter for number of best effort
> queues, as this can be detected by checking the size of all best effort queues
> against 0. Of course, we should mandate that the enabled queues (with non-
> zero size) are contiguous.

Yes, it is done that way without having any additional parameter in public structure. 
> >
> > -/** Number of queues per pipe. */
> > -#define RTE_SCHED_QUEUES_PER_PIPE             \
> > -	(RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE *     \
> > -	RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS)
> > +#define RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS    4
> > +/** Number of traffic classes per pipe (as well as subport).
> > + *
> > + * @see struct rte_sched_subport_params
> > + * @see struct rte_sched_pipe_params
> > + */
> > +#define RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE    \
> > +(RTE_SCHED_QUEUES_PER_PIPE - RTE_SCHED_BE_QUEUES_PER_PIPE + 1)
> >
> > -/** Maximum number of pipe profiles that can be defined per port.
> > +/** Maximum number of pipe profiles that can be defined per subport.
> >   * Compile-time configurable.
> >   */
> >  #ifndef RTE_SCHED_PIPE_PROFILES_PER_PORT @@ -95,6 +107,8 @@
> extern
> > "C" {
> >   *
> >   * The FCS is considered overhead only if not included in the packet
> >   * length (field pkt_len of struct rte_mbuf).
> > + *
> > + * @see struct rte_sched_port_params
> >   */
> >  #ifndef RTE_SCHED_FRAME_OVERHEAD_DEFAULT
> >  #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT      24
> > --
> > 2.21.0
diff mbox series

Patch

diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
index 9c55a787d..470a0036a 100644
--- a/lib/librte_sched/rte_sched.h
+++ b/lib/librte_sched/rte_sched.h
@@ -52,7 +52,7 @@  extern "C" {
  *	    multiple connections of same traffic class belonging to
  *	    the same user;
  *           - Weighted Round Robin (WRR) is used to service the
- *	    queues within same pipe traffic class.
+ *	    queues within same pipe lowest priority traffic class (best-effort).
  *
  */
 
@@ -66,20 +66,32 @@  extern "C" {
 #include "rte_red.h"
 #endif
 
-/** Number of traffic classes per pipe (as well as subport).
- * Cannot be changed.
+/** Maximum number of queues per pipe.
+ * Note that the multiple queues (power of 2) can only be assigned to
+ * lowest priority (best-effort) traffic class. Other higher priority traffic
+ * classes can only have one queue.
+ * Can not change.
+ *
+ * @see struct rte_sched_subport_params
  */
-#define RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE    4
+#define RTE_SCHED_QUEUES_PER_PIPE    16
 
-/** Number of queues per pipe traffic class. Cannot be changed. */
-#define RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS    4
+/** Number of WRR queues for best-effort traffic class per pipe.
+ *
+ * @see struct rte_sched_pipe_params
+ */
+#define RTE_SCHED_BE_QUEUES_PER_PIPE    8
 
-/** Number of queues per pipe. */
-#define RTE_SCHED_QUEUES_PER_PIPE             \
-	(RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE *     \
-	RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS)
+#define RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS    4
+/** Number of traffic classes per pipe (as well as subport).
+ *
+ * @see struct rte_sched_subport_params
+ * @see struct rte_sched_pipe_params
+ */
+#define RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE    \
+(RTE_SCHED_QUEUES_PER_PIPE - RTE_SCHED_BE_QUEUES_PER_PIPE + 1)
 
-/** Maximum number of pipe profiles that can be defined per port.
+/** Maximum number of pipe profiles that can be defined per subport.
  * Compile-time configurable.
  */
 #ifndef RTE_SCHED_PIPE_PROFILES_PER_PORT
@@ -95,6 +107,8 @@  extern "C" {
  *
  * The FCS is considered overhead only if not included in the packet
  * length (field pkt_len of struct rte_mbuf).
+ *
+ * @see struct rte_sched_port_params
  */
 #ifndef RTE_SCHED_FRAME_OVERHEAD_DEFAULT
 #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT      24