RE: [PATCH v2] sched: enable CMAN at runtime

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

> -----Original Message-----
> From: Danilewicz, MarcinX <marcinx.danilewicz@intel.com>
> Sent: Thursday, May 12, 2022 2:11 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: Ajmera, Megha <megha.ajmera@intel.com>
> Subject: [PATCH v2] sched: enable CMAN at runtime
> 
> Added changes to enable CMAN (RED or PIE) at init
> from profile configuration file.
> 
> By default CMAN code is enable but not in use, when
> there is no RED or PIE profile configured.
> 
> Log: v2 change in rte_sched.h to avoid ABI breakage.
> 
> Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com>
> ---
>  config/rte_config.h                      |   3 -
>  drivers/net/softnic/rte_eth_softnic_tm.c |  12 --
>  examples/ip_pipeline/tmgr.c              |   4 -
>  examples/qos_sched/cfg_file.c            |  14 +--
>  examples/qos_sched/cfg_file.h            |   2 -
>  examples/qos_sched/init.c                |   4 -
>  examples/qos_sched/main.h                |   2 -
>  examples/qos_sched/profile.cfg           | 130 ++++++++++-----------
>  examples/qos_sched/profile_pie.cfg       | 142 ++++++++++++++++++++++
>  examples/qos_sched/profile_red.cfg       | 143 +++++++++++++++++++++++
>  lib/sched/rte_sched.c                    |  53 ++-------
>  lib/sched/rte_sched.h                    |   2 +
>  12 files changed, 371 insertions(+), 140 deletions(-)
>  create mode 100644 examples/qos_sched/profile_pie.cfg
>  create mode 100644 examples/qos_sched/profile_red.cfg
> 
> diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h
> index 5ece64e527..82aa73ff5b 100644
> --- a/lib/sched/rte_sched.h
> +++ b/lib/sched/rte_sched.h
> @@ -128,6 +128,8 @@ extern "C" {
>  enum rte_sched_cman_mode {
>  	RTE_SCHED_CMAN_RED, /**< Random Early Detection (RED) */
>  	RTE_SCHED_CMAN_PIE, /**< Proportional Integral Controller
> Enhanced (PIE) */
> +	/* New enum RTE_SCHED_CMAN_NONE added at the end to avoid
> ABI breakage */
> +	RTE_SCHED_CMAN_NONE, /**< no RED|PIE cfg available */
>  };

Can you please explain why you need to add the new RTE_SCHED_CMAN_NONE value to this enumeration and what is the ABI breakage that you are trying to avoid?

The library expects the struct rte_sched_subport_params:: cman_params to be NULL when the CMAN is disabled, why is this not enough to detect that CMAN is disabled? I suggest to keep the current approach.
> 
>  /*
> --
> 2.25.1