RE: [PATCH] sched: Cleanup qos scheduler defines from rte_config

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Friday, January 21, 2022 6:15 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> thomas@monjalon.net
> Subject: [PATCH] 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   | 7 -------
>  lib/sched/rte_sched.c | 4 ++++
>  2 files changed, 4 insertions(+), 7 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/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

Hi Thomas,

Any issues with merging this patch?

What this patch is fixing is removing these flags from rte_config.h, which I understand from your previous email is the "most problematic" issue from your perspective, hence my recommendation to have this patch merged. But this is definitely not the final step.

Here are some next steps to make these options run-time configurable as opposed to build time options:

1. RTE_SCHED_COLLECT_STATS: Always turn on the stats, ignore the modest performance impact, enable the code path with this flag defined and remove this flag completely
2. RTE_SCHED_TC_OV: Always enable the oversubscription feature, enable the code path with RTE_SCHED_TC_OV defined and remove this flag completely.
3. RTE_SCHED_VECTOR: Keep the code path with RTE_SCHED_VECTOR disabled and remove code path with the RTE_SCHED_VECTOR enabled and also remove this knob completely. The code currently under this flag is no longer useful.
4. RTE_SCHED_PORT_N_GRINDERS: Keep the RTE_SCHED_PORT_N_GRINDERS defined in rte_sched.c (as introduced by this new patch), this is not a configuration knob for the user.
5. RTE_SCHED_CMAN: Check the latest performance impact of having the congestion management code enabled at build-time when not actually used at run-time; in the absence of no significant impact, simply remove this knob and have the congestion management always enabled at build-time with run-time options to enable/disable it.

Does it sound like a good plan? Thanks for your help!

Regards,
Cristian