patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH v2] sched: Fix subport profile id not set correctly.
@ 2022-10-05 17:22 Megha Ajmera
  2022-10-05 19:19 ` Dumitrescu, Cristian
  2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
  0 siblings, 2 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-05 17:22 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu; +Cc: stable

In rte_sched_subport_config() API, subport_profile_id is not set correctly.

Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
Cc: cristian.dumitrescu@intel.com

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

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index c5fa9e4582..c91697131d 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1257,8 +1257,6 @@ rte_sched_subport_config(struct rte_sched_port *port,
 
 		n_subports++;
 
-		subport_profile_id = 0;
-
 		/* Port */
 		port->subports[subport_id] = s;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v2] sched: Fix subport profile id not set correctly.
  2022-10-05 17:22 [PATCH v2] sched: Fix subport profile id not set correctly Megha Ajmera
@ 2022-10-05 19:19 ` Dumitrescu, Cristian
  2022-10-11  4:59   ` Ajmera, Megha
  2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
  1 sibling, 1 reply; 27+ messages in thread
From: Dumitrescu, Cristian @ 2022-10-05 19:19 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder; +Cc: stable



> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Wednesday, October 5, 2022 6:23 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: stable@dpdk.org
> Subject: [PATCH v2] sched: Fix subport profile id not set correctly.
> 
> In rte_sched_subport_config() API, subport_profile_id is not set correctly.
> 
> Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
> Cc: cristian.dumitrescu@intel.com
> 
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
>  lib/sched/rte_sched.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
> index c5fa9e4582..c91697131d 100644
> --- a/lib/sched/rte_sched.c
> +++ b/lib/sched/rte_sched.c
> @@ -1257,8 +1257,6 @@ rte_sched_subport_config(struct rte_sched_port
> *port,
> 
>  		n_subports++;
> 
> -		subport_profile_id = 0;
> -
>  		/* Port */
>  		port->subports[subport_id] = s;
> 
> --
> 2.25.1

Hi Megha,

I noticed several patches from you that all fix small things, can you please put all of them into a series as opposed to isolated patches? This is to avoid apply issues due to dependency order. No need for a cover letter in this case.

Can you please also pay attention to the details in the patch title and description:
-title needs to have a lower case letter after "sched: "
-title needs to start with a verb
-title needs to be short and clear
-description needs to make sense

Thanks,
Cristian

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 1/3] sched: fix subport profile id not set correctly
  2022-10-05 17:22 [PATCH v2] sched: Fix subport profile id not set correctly Megha Ajmera
  2022-10-05 19:19 ` Dumitrescu, Cristian
@ 2022-10-06 19:00 ` Megha Ajmera
  2022-10-06 19:00   ` [PATCH 2/3] sched: removed unused subport field in hqos profile Megha Ajmera
                     ` (4 more replies)
  1 sibling, 5 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-06 19:00 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu; +Cc: stable

In rte_sched_subport_config() API, subport_profile_id is not set correctly.

Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
Cc: cristian.dumitrescu@intel.com

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

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index c5fa9e4582..c91697131d 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1257,8 +1257,6 @@ rte_sched_subport_config(struct rte_sched_port *port,
 
 		n_subports++;
 
-		subport_profile_id = 0;
-
 		/* Port */
 		port->subports[subport_id] = s;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 2/3] sched: removed unused subport field in hqos profile
  2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
@ 2022-10-06 19:00   ` Megha Ajmera
  2022-10-11 14:26     ` Dumitrescu, Cristian
  2022-10-06 19:00   ` [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 27+ messages in thread
From: Megha Ajmera @ 2022-10-06 19:00 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu; +Cc: stable

Removed unused subport field from profile.cfg
Correctly using subport profile id in subport config load.

Fixes: 802d214dc880 ("examples/qos_sched: update subport rate dynamically")
Cc: cristian.dumitrescu@intel.com

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 examples/qos_sched/cfg_file.c  | 2 +-
 examples/qos_sched/profile.cfg | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index 3d5d75fcf0..ca871d3287 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -157,7 +157,7 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 
 	profiles = rte_cfgfile_num_sections(cfg, "subport profile",
 					   sizeof("subport profile") - 1);
-	subport_params[0].n_pipe_profiles = profiles;
+	port_params.n_subport_profiles = profiles;
 
 	for (i = 0; i < profiles; i++) {
 		char sec_name[32];
diff --git a/examples/qos_sched/profile.cfg b/examples/qos_sched/profile.cfg
index c9ec187c93..e8de101b6c 100644
--- a/examples/qos_sched/profile.cfg
+++ b/examples/qos_sched/profile.cfg
@@ -26,8 +26,6 @@ number of subports per port = 1
 number of pipes per subport = 4096
 queue sizes = 64 64 64 64 64 64 64 64 64 64 64 64 64
 
-subport 0-8 = 0                ; These subports are configured with subport profile 0
-
 [subport profile 0]
 tb rate = 1250000000           ; Bytes per second
 tb size = 1000000              ; Bytes
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
  2022-10-06 19:00   ` [PATCH 2/3] sched: removed unused subport field in hqos profile Megha Ajmera
@ 2022-10-06 19:00   ` Megha Ajmera
  2022-10-11 13:09     ` Dumitrescu, Cristian
  2022-10-11 14:22   ` [PATCH 1/3] sched: fix subport profile id not set correctly Dumitrescu, Cristian
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 27+ messages in thread
From: Megha Ajmera @ 2022-10-06 19:00 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu; +Cc: stable

Config load functions updated to support 100G rates
for subport and pipes.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 examples/qos_sched/cfg_file.c | 64 +++++++++++++++++------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index ca871d3287..ca60d616a1 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -64,67 +64,67 @@ cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe_params
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb rate");
 		if (entry)
-			pipe_params[j].tb_rate = (uint64_t)atoi(entry);
+			pipe_params[j].tb_rate = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb size");
 		if (entry)
-			pipe_params[j].tb_size = (uint64_t)atoi(entry);
+			pipe_params[j].tb_size = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc period");
 		if (entry)
-			pipe_params[j].tc_period = (uint64_t)atoi(entry);
+			pipe_params[j].tc_period = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 0 rate");
 		if (entry)
-			pipe_params[j].tc_rate[0] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[0] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 1 rate");
 		if (entry)
-			pipe_params[j].tc_rate[1] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[1] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 2 rate");
 		if (entry)
-			pipe_params[j].tc_rate[2] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[2] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 3 rate");
 		if (entry)
-			pipe_params[j].tc_rate[3] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[3] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 4 rate");
 		if (entry)
-			pipe_params[j].tc_rate[4] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[4] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 5 rate");
 		if (entry)
-			pipe_params[j].tc_rate[5] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[5] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 6 rate");
 		if (entry)
-			pipe_params[j].tc_rate[6] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[6] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 7 rate");
 		if (entry)
-			pipe_params[j].tc_rate[7] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[7] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 8 rate");
 		if (entry)
-			pipe_params[j].tc_rate[8] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[8] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 9 rate");
 		if (entry)
-			pipe_params[j].tc_rate[9] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[9] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 10 rate");
 		if (entry)
-			pipe_params[j].tc_rate[10] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[10] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 11 rate");
 		if (entry)
-			pipe_params[j].tc_rate[11] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[11] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 rate");
 		if (entry)
-			pipe_params[j].tc_rate[12] = (uint64_t)atoi(entry);
+			pipe_params[j].tc_rate[12] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 oversubscription weight");
 		if (entry)
@@ -165,67 +165,67 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb rate");
 		if (entry)
-			subport_profile[i].tb_rate = (uint64_t)atoi(entry);
+			subport_profile[i].tb_rate = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb size");
 		if (entry)
-			subport_profile[i].tb_size = (uint64_t)atoi(entry);
+			subport_profile[i].tb_size = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc period");
 		if (entry)
-			subport_profile[i].tc_period = (uint64_t)atoi(entry);
+			subport_profile[i].tc_period = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 0 rate");
 		if (entry)
-			subport_profile[i].tc_rate[0] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[0] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 1 rate");
 		if (entry)
-			subport_profile[i].tc_rate[1] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[1] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 2 rate");
 		if (entry)
-			subport_profile[i].tc_rate[2] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[2] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 3 rate");
 		if (entry)
-			subport_profile[i].tc_rate[3] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[3] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 4 rate");
 		if (entry)
-			subport_profile[i].tc_rate[4] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[4] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 5 rate");
 		if (entry)
-			subport_profile[i].tc_rate[5] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[5] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 6 rate");
 		if (entry)
-			subport_profile[i].tc_rate[6] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[6] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 7 rate");
 		if (entry)
-			subport_profile[i].tc_rate[7] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[7] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 8 rate");
 		if (entry)
-			subport_profile[i].tc_rate[8] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[8] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 9 rate");
 		if (entry)
-			subport_profile[i].tc_rate[9] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[9] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 10 rate");
 		if (entry)
-			subport_profile[i].tc_rate[10] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[10] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 11 rate");
 		if (entry)
-			subport_profile[i].tc_rate[11] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[11] = atol(entry);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
 		if (entry)
-			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
+			subport_profile[i].tc_rate[12] = atol(entry);
 	}
 
 	return 0;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v2] sched: Fix subport profile id not set correctly.
  2022-10-05 19:19 ` Dumitrescu, Cristian
@ 2022-10-11  4:59   ` Ajmera, Megha
  0 siblings, 0 replies; 27+ messages in thread
From: Ajmera, Megha @ 2022-10-11  4:59 UTC (permalink / raw)
  To: Dumitrescu, Cristian, dev, Singh, Jasvinder; +Cc: stable

> 
> Hi Megha,
> 
> I noticed several patches from you that all fix small things, can you please put all
> of them into a series as opposed to isolated patches? This is to avoid apply
> issues due to dependency order. No need for a cover letter in this case.
> 
> Can you please also pay attention to the details in the patch title and
> description:
> -title needs to have a lower case letter after "sched: "
> -title needs to start with a verb
> -title needs to be short and clear
> -description needs to make sense

Hi Cristian,

I have incorporated review comments in recent patch-set series. Can you please help review the same?

Regards,
Megha

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-06 19:00   ` [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
@ 2022-10-11 13:09     ` Dumitrescu, Cristian
  2022-10-18  5:40       ` Ajmera, Megha
  0 siblings, 1 reply; 27+ messages in thread
From: Dumitrescu, Cristian @ 2022-10-11 13:09 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder; +Cc: stable



> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Thursday, October 6, 2022 8:01 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: stable@dpdk.org
> Subject: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config
> 
> Config load functions updated to support 100G rates
> for subport and pipes.
> 
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
>  examples/qos_sched/cfg_file.c | 64 +++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
> index ca871d3287..ca60d616a1 100644
> --- a/examples/qos_sched/cfg_file.c
> +++ b/examples/qos_sched/cfg_file.c
> @@ -64,67 +64,67 @@ cfg_load_pipe(struct rte_cfgfile *cfg, struct
> rte_sched_pipe_params *pipe_params
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb rate");
>  		if (entry)
> -			pipe_params[j].tb_rate = (uint64_t)atoi(entry);
> +			pipe_params[j].tb_rate = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb size");
>  		if (entry)
> -			pipe_params[j].tb_size = (uint64_t)atoi(entry);
> +			pipe_params[j].tb_size = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc period");
>  		if (entry)
> -			pipe_params[j].tc_period = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_period = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 0 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[0] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[0] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 1 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[1] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[1] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 2 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[2] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[2] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 3 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[3] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[3] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 4 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[4] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[4] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 5 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[5] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[5] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 6 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[6] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[6] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 7 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[7] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[7] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 8 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[8] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[8] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 9 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[9] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[9] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 10 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[10] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[10] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 11 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[11] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[11] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 rate");
>  		if (entry)
> -			pipe_params[j].tc_rate[12] = (uint64_t)atoi(entry);
> +			pipe_params[j].tc_rate[12] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12
> oversubscription weight");
>  		if (entry)
> @@ -165,67 +165,67 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb rate");
>  		if (entry)
> -			subport_profile[i].tb_rate = (uint64_t)atoi(entry);
> +			subport_profile[i].tb_rate = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb size");
>  		if (entry)
> -			subport_profile[i].tb_size = (uint64_t)atoi(entry);
> +			subport_profile[i].tb_size = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc period");
>  		if (entry)
> -			subport_profile[i].tc_period = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_period = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 0 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[0] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[0] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 1 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[1] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[1] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 2 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[2] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[2] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 3 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[3] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[3] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 4 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[4] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[4] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 5 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[5] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[5] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 6 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[6] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[6] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 7 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[7] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[7] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 8 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[8] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[8] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 9 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[9] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[9] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 10 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[10] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[10] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 11 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[11] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[11] = atol(entry);
> 
>  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
>  		if (entry)
> -			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
> +			subport_profile[i].tc_rate[12] = atol(entry);
>  	}
> 
>  	return 0;
> --
> 2.25.1

Hi Megha,

Maybe you can explain how removing this typecast can provide support for 100+G rates?

The atoi() function returns a 32-bit value, while the subport and pipe rates are 64-bit values; this typecast can at most remove a compiler warning ...

Regards,
Cristian

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH 1/3] sched: fix subport profile id not set correctly
  2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
  2022-10-06 19:00   ` [PATCH 2/3] sched: removed unused subport field in hqos profile Megha Ajmera
  2022-10-06 19:00   ` [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
@ 2022-10-11 14:22   ` Dumitrescu, Cristian
  2022-10-28  8:09   ` [PATCH v5 1/3] sched: fix subport profile ID Megha Ajmera
  2022-10-28  9:55   ` [PATCH v6 1/3] sched: fix subport profile ID Megha Ajmera
  4 siblings, 0 replies; 27+ messages in thread
From: Dumitrescu, Cristian @ 2022-10-11 14:22 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder; +Cc: stable



> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Thursday, October 6, 2022 8:01 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: stable@dpdk.org
> Subject: [PATCH 1/3] sched: fix subport profile id not set correctly
> 
> In rte_sched_subport_config() API, subport_profile_id is not set correctly.
> 
> Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
> Cc: cristian.dumitrescu@intel.com
> 
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
>  lib/sched/rte_sched.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
> index c5fa9e4582..c91697131d 100644
> --- a/lib/sched/rte_sched.c
> +++ b/lib/sched/rte_sched.c
> @@ -1257,8 +1257,6 @@ rte_sched_subport_config(struct rte_sched_port
> *port,
> 
>  		n_subports++;
> 
> -		subport_profile_id = 0;
> -
>  		/* Port */
>  		port->subports[subport_id] = s;
> 
> --
> 2.25.1

Hi Megha,

Please fix the title to: "sched: fix subport profile ID".

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

Regards,
Cristian

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH 2/3] sched: removed unused subport field in hqos profile
  2022-10-06 19:00   ` [PATCH 2/3] sched: removed unused subport field in hqos profile Megha Ajmera
@ 2022-10-11 14:26     ` Dumitrescu, Cristian
  0 siblings, 0 replies; 27+ messages in thread
From: Dumitrescu, Cristian @ 2022-10-11 14:26 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder; +Cc: stable



> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Thursday, October 6, 2022 8:01 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: stable@dpdk.org
> Subject: [PATCH 2/3] sched: removed unused subport field in hqos profile
> 
> Removed unused subport field from profile.cfg
> Correctly using subport profile id in subport config load.
> 
> Fixes: 802d214dc880 ("examples/qos_sched: update subport rate
> dynamically")
> Cc: cristian.dumitrescu@intel.com
> 
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
>  examples/qos_sched/cfg_file.c  | 2 +-
>  examples/qos_sched/profile.cfg | 2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
> index 3d5d75fcf0..ca871d3287 100644
> --- a/examples/qos_sched/cfg_file.c
> +++ b/examples/qos_sched/cfg_file.c
> @@ -157,7 +157,7 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
> 
>  	profiles = rte_cfgfile_num_sections(cfg, "subport profile",
>  					   sizeof("subport profile") - 1);
> -	subport_params[0].n_pipe_profiles = profiles;
> +	port_params.n_subport_profiles = profiles;
> 
>  	for (i = 0; i < profiles; i++) {
>  		char sec_name[32];
> diff --git a/examples/qos_sched/profile.cfg
> b/examples/qos_sched/profile.cfg
> index c9ec187c93..e8de101b6c 100644
> --- a/examples/qos_sched/profile.cfg
> +++ b/examples/qos_sched/profile.cfg
> @@ -26,8 +26,6 @@ number of subports per port = 1
>  number of pipes per subport = 4096
>  queue sizes = 64 64 64 64 64 64 64 64 64 64 64 64 64
> 
> -subport 0-8 = 0                ; These subports are configured with subport profile
> 0
> -
>  [subport profile 0]
>  tb rate = 1250000000           ; Bytes per second
>  tb size = 1000000              ; Bytes
> --
> 2.25.1

Hi Megha,

Please fix:
-the title: "sched: fix number of subport profiles".
-add Cc: stable@dpdk.org, not Cc to me; you need to add me in the "To:" line of the email

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

Regards,
Cristian

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-11 13:09     ` Dumitrescu, Cristian
@ 2022-10-18  5:40       ` Ajmera, Megha
  2022-10-18 13:12         ` Dumitrescu, Cristian
  0 siblings, 1 reply; 27+ messages in thread
From: Ajmera, Megha @ 2022-10-18  5:40 UTC (permalink / raw)
  To: Dumitrescu, Cristian, dev, Singh, Jasvinder; +Cc: stable

> >  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
> >  		if (entry)
> > -			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
> > +			subport_profile[i].tc_rate[12] = atol(entry);
> >  	}
> >
> >  	return 0;
> > --
> > 2.25.1
> 
> Hi Megha,
> 
> Maybe you can explain how removing this typecast can provide support for
> 100+G rates?
> 
> The atoi() function returns a 32-bit value, while the subport and pipe rates are
> 64-bit values; this typecast can at most remove a compiler warning ...

Hi Cristian,

We have now changed 'atoi' to 'atol' which will return 64-bit value so it will take care of 100G+ port speeds. However, I noticed that 'atol' will return signed-64-bit so typecast may still be needed to assign it to unsigned-64-bit variable. Will send updated patch today.

Regards,
Megha

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-18  5:40       ` Ajmera, Megha
@ 2022-10-18 13:12         ` Dumitrescu, Cristian
  2022-10-19 18:37           ` Stephen Hemminger
  0 siblings, 1 reply; 27+ messages in thread
From: Dumitrescu, Cristian @ 2022-10-18 13:12 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder; +Cc: stable



> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Tuesday, October 18, 2022 6:40 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org;
> Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: stable@dpdk.org
> Subject: RE: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe
> config
> 
> > >  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
> > >  		if (entry)
> > > -			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
> > > +			subport_profile[i].tc_rate[12] = atol(entry);
> > >  	}
> > >
> > >  	return 0;
> > > --
> > > 2.25.1
> >
> > Hi Megha,
> >
> > Maybe you can explain how removing this typecast can provide support for
> > 100+G rates?
> >
> > The atoi() function returns a 32-bit value, while the subport and pipe rates
> are
> > 64-bit values; this typecast can at most remove a compiler warning ...
> 
> Hi Cristian,
> 
> We have now changed 'atoi' to 'atol' which will return 64-bit value so it will
> take care of 100G+ port speeds. However, I noticed that 'atol' will return
> signed-64-bit so typecast may still be needed to assign it to unsigned-64-bit
> variable. Will send updated patch today.
> 
> Regards,
> Megha

You need to use 'atoll', not 'atol'.  And yes, typecast is still required.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-18 13:12         ` Dumitrescu, Cristian
@ 2022-10-19 18:37           ` Stephen Hemminger
  2022-10-20  9:47             ` [PATCH v4 1/3] sched: fix subport profile ID Megha Ajmera
  2022-10-20  9:55             ` [PATCH " Ajmera, Megha
  0 siblings, 2 replies; 27+ messages in thread
From: Stephen Hemminger @ 2022-10-19 18:37 UTC (permalink / raw)
  To: Dumitrescu, Cristian; +Cc: Ajmera, Megha, dev, Singh, Jasvinder, stable

On Tue, 18 Oct 2022 13:12:10 +0000
"Dumitrescu, Cristian" <cristian.dumitrescu@intel.com> wrote:

> > -----Original Message-----
> > From: Ajmera, Megha <megha.ajmera@intel.com>
> > Sent: Tuesday, October 18, 2022 6:40 AM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org;
> > Singh, Jasvinder <jasvinder.singh@intel.com>
> > Cc: stable@dpdk.org
> > Subject: RE: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe
> > config
> >   
> > > >  		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
> > > >  		if (entry)
> > > > -			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
> > > > +			subport_profile[i].tc_rate[12] = atol(entry);
> > > >  	}
> > > >
> > > >  	return 0;
> > > > --
> > > > 2.25.1  
> > >
> > > Hi Megha,
> > >
> > > Maybe you can explain how removing this typecast can provide support for
> > > 100+G rates?
> > >
> > > The atoi() function returns a 32-bit value, while the subport and pipe rates  
> > are  
> > > 64-bit values; this typecast can at most remove a compiler warning ...  
> > 
> > Hi Cristian,
> > 
> > We have now changed 'atoi' to 'atol' which will return 64-bit value so it will
> > take care of 100G+ port speeds. However, I noticed that 'atol' will return
> > signed-64-bit so typecast may still be needed to assign it to unsigned-64-bit
> > variable. Will send updated patch today.
> > 
> > Regards,
> > Megha  
> 
> You need to use 'atoll', not 'atol'.  And yes, typecast is still required.

The code should be using strtoull() and checking for errors.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v4 1/3] sched: fix subport profile ID
  2022-10-19 18:37           ` Stephen Hemminger
@ 2022-10-20  9:47             ` Megha Ajmera
  2022-10-20  9:47               ` [PATCH v4 2/3] sched: fix number of subport profiles Megha Ajmera
  2022-10-20  9:47               ` [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
  2022-10-20  9:55             ` [PATCH " Ajmera, Megha
  1 sibling, 2 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-20  9:47 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable

In rte_sched_subport_config() API, subport_profile_id is not set correctly.

Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
Cc: cristian.dumitrescu@intel.com

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

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index c5fa9e4582..c91697131d 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1257,8 +1257,6 @@ rte_sched_subport_config(struct rte_sched_port *port,
 
 		n_subports++;
 
-		subport_profile_id = 0;
-
 		/* Port */
 		port->subports[subport_id] = s;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v4 2/3] sched: fix number of subport profiles
  2022-10-20  9:47             ` [PATCH v4 1/3] sched: fix subport profile ID Megha Ajmera
@ 2022-10-20  9:47               ` Megha Ajmera
  2022-10-20  9:47               ` [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
  1 sibling, 0 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-20  9:47 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable

Removed unused subport field from profile.cfg
Correctly using subport profile id in subport config load.

Fixes: 802d214dc880 ("examples/qos_sched: update subport rate dynamically")
Cc: stable@dpdk.org

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 examples/qos_sched/cfg_file.c  | 2 +-
 examples/qos_sched/profile.cfg | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index 3d5d75fcf0..ca871d3287 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -157,7 +157,7 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 
 	profiles = rte_cfgfile_num_sections(cfg, "subport profile",
 					   sizeof("subport profile") - 1);
-	subport_params[0].n_pipe_profiles = profiles;
+	port_params.n_subport_profiles = profiles;
 
 	for (i = 0; i < profiles; i++) {
 		char sec_name[32];
diff --git a/examples/qos_sched/profile.cfg b/examples/qos_sched/profile.cfg
index c9ec187c93..e8de101b6c 100644
--- a/examples/qos_sched/profile.cfg
+++ b/examples/qos_sched/profile.cfg
@@ -26,8 +26,6 @@ number of subports per port = 1
 number of pipes per subport = 4096
 queue sizes = 64 64 64 64 64 64 64 64 64 64 64 64 64
 
-subport 0-8 = 0                ; These subports are configured with subport profile 0
-
 [subport profile 0]
 tb rate = 1250000000           ; Bytes per second
 tb size = 1000000              ; Bytes
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-20  9:47             ` [PATCH v4 1/3] sched: fix subport profile ID Megha Ajmera
  2022-10-20  9:47               ` [PATCH v4 2/3] sched: fix number of subport profiles Megha Ajmera
@ 2022-10-20  9:47               ` Megha Ajmera
  2022-10-24 18:57                 ` Dumitrescu, Cristian
  1 sibling, 1 reply; 27+ messages in thread
From: Megha Ajmera @ 2022-10-20  9:47 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable

Config load functions updated to support 100G rates
for subport and pipes.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 examples/qos_sched/cfg_file.c | 294 ++++++++++++++++++++++++++--------
 1 file changed, 230 insertions(+), 64 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index ca871d3287..9fcdf5eeb6 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -60,71 +60,154 @@ cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe_params
 
 	for (j = 0; j < profiles; j++) {
 		char pipe_name[32];
+		/* Convert to decimal */
+		int base = 10;
+
 		snprintf(pipe_name, sizeof(pipe_name), "pipe profile %d", j);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb rate");
-		if (entry)
-			pipe_params[j].tb_rate = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tb_rate = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb size");
-		if (entry)
-			pipe_params[j].tb_size = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tb_size = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc period");
-		if (entry)
-			pipe_params[j].tc_period = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_period = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 0 rate");
-		if (entry)
-			pipe_params[j].tc_rate[0] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[0] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 1 rate");
-		if (entry)
-			pipe_params[j].tc_rate[1] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[1] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 2 rate");
-		if (entry)
-			pipe_params[j].tc_rate[2] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[2] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 3 rate");
-		if (entry)
-			pipe_params[j].tc_rate[3] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[3] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 4 rate");
-		if (entry)
-			pipe_params[j].tc_rate[4] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[4] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 5 rate");
-		if (entry)
-			pipe_params[j].tc_rate[5] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[5] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 6 rate");
-		if (entry)
-			pipe_params[j].tc_rate[6] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[6] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 7 rate");
-		if (entry)
-			pipe_params[j].tc_rate[7] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[7] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 8 rate");
-		if (entry)
-			pipe_params[j].tc_rate[8] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[8] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 9 rate");
-		if (entry)
-			pipe_params[j].tc_rate[9] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[9] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 10 rate");
-		if (entry)
-			pipe_params[j].tc_rate[10] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[10] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 11 rate");
-		if (entry)
-			pipe_params[j].tc_rate[11] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[11] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 rate");
-		if (entry)
-			pipe_params[j].tc_rate[12] = (uint64_t)atoi(entry);
+		if (entry) {
+			pipe_params[j].tc_rate[12] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 oversubscription weight");
 		if (entry)
@@ -161,71 +244,154 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 
 	for (i = 0; i < profiles; i++) {
 		char sec_name[32];
+		/* convert to decimal */
+		int base = 10;
+
 		snprintf(sec_name, sizeof(sec_name), "subport profile %d", i);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb rate");
-		if (entry)
-			subport_profile[i].tb_rate = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tb_rate = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb size");
-		if (entry)
-			subport_profile[i].tb_size = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tb_size = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc period");
-		if (entry)
-			subport_profile[i].tc_period = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_period = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 0 rate");
-		if (entry)
-			subport_profile[i].tc_rate[0] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[0] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 1 rate");
-		if (entry)
-			subport_profile[i].tc_rate[1] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[1] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 2 rate");
-		if (entry)
-			subport_profile[i].tc_rate[2] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[2] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 3 rate");
-		if (entry)
-			subport_profile[i].tc_rate[3] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[3] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 4 rate");
-		if (entry)
-			subport_profile[i].tc_rate[4] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[4] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 5 rate");
-		if (entry)
-			subport_profile[i].tc_rate[5] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[5] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 6 rate");
-		if (entry)
-			subport_profile[i].tc_rate[6] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[6] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 7 rate");
-		if (entry)
-			subport_profile[i].tc_rate[7] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[7] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 8 rate");
-		if (entry)
-			subport_profile[i].tc_rate[8] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[8] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 9 rate");
-		if (entry)
-			subport_profile[i].tc_rate[9] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[9] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 10 rate");
-		if (entry)
-			subport_profile[i].tc_rate[10] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[10] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 11 rate");
-		if (entry)
-			subport_profile[i].tc_rate[11] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[11] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
-		if (entry)
-			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
+		if (entry) {
+			subport_profile[i].tc_rate[12] = (uint64_t)strtoull(entry, NULL, base);
+			if (errno == EINVAL || errno == ERANGE) {
+				/* cannot convert string to unsigned long long */
+				return -1;
+			}
+		}
 	}
 
 	return 0;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-19 18:37           ` Stephen Hemminger
  2022-10-20  9:47             ` [PATCH v4 1/3] sched: fix subport profile ID Megha Ajmera
@ 2022-10-20  9:55             ` Ajmera, Megha
  1 sibling, 0 replies; 27+ messages in thread
From: Ajmera, Megha @ 2022-10-20  9:55 UTC (permalink / raw)
  To: Stephen Hemminger, Dumitrescu, Cristian; +Cc: dev, Singh, Jasvinder, stable

> >
> > You need to use 'atoll', not 'atol'.  And yes, typecast is still required.
> 
> The code should be using strtoull() and checking for errors.

Thanks Cristian and Stephen. I have sent updated patch-series v4 for review and commit.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-20  9:47               ` [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
@ 2022-10-24 18:57                 ` Dumitrescu, Cristian
  0 siblings, 0 replies; 27+ messages in thread
From: Dumitrescu, Cristian @ 2022-10-24 18:57 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder, stephen; +Cc: stable

Hi Megha,

Comments inline below.

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Thursday, October 20, 2022 10:48 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> stephen@networkplumber.org
> Cc: stable@dpdk.org
> Subject: [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe
> config
> 
> Config load functions updated to support 100G rates
> for subport and pipes.
> 
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
>  examples/qos_sched/cfg_file.c | 294 ++++++++++++++++++++++++++------
> --
>  1 file changed, 230 insertions(+), 64 deletions(-)
> 
> diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
> index ca871d3287..9fcdf5eeb6 100644
> --- a/examples/qos_sched/cfg_file.c
> +++ b/examples/qos_sched/cfg_file.c
> @@ -60,71 +60,154 @@ cfg_load_pipe(struct rte_cfgfile *cfg, struct
> rte_sched_pipe_params *pipe_params
> 
>  	for (j = 0; j < profiles; j++) {
>  		char pipe_name[32];
> +		/* Convert to decimal */
> +		int base = 10;

We should set base to 0 to allow multiple basis (8, 10, 16), I see no reason to limit to base 10, let's take the benefits out of strtoull. I suggest we use the value 0 directly in the function, no need to have a local variable just for this.
> +
>  		snprintf(pipe_name, sizeof(pipe_name), "pipe profile %d",
> j);
> 
>  		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb rate");
> -		if (entry)
> -			pipe_params[j].tb_rate = (uint64_t)atoi(entry);
> +		if (entry) {
> +			pipe_params[j].tb_rate = (uint64_t)strtoull(entry,
> NULL, base);
> +			if (errno == EINVAL || errno == ERANGE) {
> +				/* cannot convert string to unsigned long
> long */
> +				return -1;
> +			}
> +		}

Some comments for this recurring pattern:

1. Why are you still doing conversion to uint64_t, as strtoull already returns a 64-bit unsigned integer?
2. If you are checking errno after the call, you should first set errno to 0 before the call, right? See man strtoull, please.
3. The errno check does not fully handle the error cases, you should also use the second argument (as opposed to setting it to NULL) to check that after the call the value referenced by the second argument is 0. This ensures that there are no illegal characters after the digits, such as in the case of "1234qwerty".
4. The comment just before the return does not bring any value, as it is obvious that an error took place if you're returning an error.
5. I suggest you wrap the strtoull() usage into local function, such as parse_u64() or similar.


<snip>

Regards,
Cristian

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v5 1/3] sched: fix subport profile ID
  2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
                     ` (2 preceding siblings ...)
  2022-10-11 14:22   ` [PATCH 1/3] sched: fix subport profile id not set correctly Dumitrescu, Cristian
@ 2022-10-28  8:09   ` Megha Ajmera
  2022-10-28  8:09     ` [PATCH v5 2/3] sched: fix number of subport profiles Megha Ajmera
  2022-10-28  8:09     ` [PATCH v5 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
  2022-10-28  9:55   ` [PATCH v6 1/3] sched: fix subport profile ID Megha Ajmera
  4 siblings, 2 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-28  8:09 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable, Dumitrescu

In rte_sched_subport_config() API, subport_profile_id is not set correctly.

Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
Cc: cristian.dumitrescu@intel.com

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

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index c5fa9e4582..c91697131d 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1257,8 +1257,6 @@ rte_sched_subport_config(struct rte_sched_port *port,
 
 		n_subports++;
 
-		subport_profile_id = 0;
-
 		/* Port */
 		port->subports[subport_id] = s;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v5 2/3] sched: fix number of subport profiles
  2022-10-28  8:09   ` [PATCH v5 1/3] sched: fix subport profile ID Megha Ajmera
@ 2022-10-28  8:09     ` Megha Ajmera
  2022-10-28  8:09     ` [PATCH v5 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
  1 sibling, 0 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-28  8:09 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable

Removed unused subport field from profile.cfg
Correctly using subport profile id in subport config load.

Fixes: 802d214dc880 ("examples/qos_sched: update subport rate dynamically")
Cc: stable@dpdk.org

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 examples/qos_sched/cfg_file.c  | 2 +-
 examples/qos_sched/profile.cfg | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index 3d5d75fcf0..ca871d3287 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -157,7 +157,7 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 
 	profiles = rte_cfgfile_num_sections(cfg, "subport profile",
 					   sizeof("subport profile") - 1);
-	subport_params[0].n_pipe_profiles = profiles;
+	port_params.n_subport_profiles = profiles;
 
 	for (i = 0; i < profiles; i++) {
 		char sec_name[32];
diff --git a/examples/qos_sched/profile.cfg b/examples/qos_sched/profile.cfg
index c9ec187c93..e8de101b6c 100644
--- a/examples/qos_sched/profile.cfg
+++ b/examples/qos_sched/profile.cfg
@@ -26,8 +26,6 @@ number of subports per port = 1
 number of pipes per subport = 4096
 queue sizes = 64 64 64 64 64 64 64 64 64 64 64 64 64
 
-subport 0-8 = 0                ; These subports are configured with subport profile 0
-
 [subport profile 0]
 tb rate = 1250000000           ; Bytes per second
 tb size = 1000000              ; Bytes
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v5 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-28  8:09   ` [PATCH v5 1/3] sched: fix subport profile ID Megha Ajmera
  2022-10-28  8:09     ` [PATCH v5 2/3] sched: fix number of subport profiles Megha Ajmera
@ 2022-10-28  8:09     ` Megha Ajmera
  1 sibling, 0 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-28  8:09 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable

Config load functions updated to support 100G rates
for subport and pipes.
Added new parse function to convert string to unsigned
long long.
Added error checks.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 examples/qos_sched/cfg_file.c | 180 +++++++++++++++++++++-------------
 examples/qos_sched/cfg_file.h |   2 +
 examples/qos_sched/init.c     |  16 ++-
 3 files changed, 128 insertions(+), 70 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index ca871d3287..d203621fa4 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -25,6 +25,21 @@ uint32_t n_active_queues;
 
 struct rte_sched_cman_params cman_params;
 
+int parse_u64(const char *entry, uint64_t *val)
+{
+	char *endptr;
+	if(!entry || !val)
+		return -EINVAL;
+
+	errno = 0;
+
+	*val = strtoull(entry, &endptr, 0);
+	if (errno == EINVAL || errno == ERANGE || *endptr != '\0') {
+		return -EINVAL;
+	}
+	return 0;
+}
+
 int
 cfg_load_port(struct rte_cfgfile *cfg, struct rte_sched_port_params *port_params)
 {
@@ -47,7 +62,7 @@ cfg_load_port(struct rte_cfgfile *cfg, struct rte_sched_port_params *port_params
 int
 cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe_params)
 {
-	int i, j;
+	int i, j, ret = 0;
 	char *next;
 	const char *entry;
 	int profiles;
@@ -63,68 +78,84 @@ cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe_params
 		snprintf(pipe_name, sizeof(pipe_name), "pipe profile %d", j);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb rate");
-		if (entry)
-			pipe_params[j].tb_rate = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tb_rate);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb size");
-		if (entry)
-			pipe_params[j].tb_size = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tb_size);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc period");
-		if (entry)
-			pipe_params[j].tc_period = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_period);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 0 rate");
-		if (entry)
-			pipe_params[j].tc_rate[0] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[0]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 1 rate");
-		if (entry)
-			pipe_params[j].tc_rate[1] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[1]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 2 rate");
-		if (entry)
-			pipe_params[j].tc_rate[2] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[2]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 3 rate");
-		if (entry)
-			pipe_params[j].tc_rate[3] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[3]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 4 rate");
-		if (entry)
-			pipe_params[j].tc_rate[4] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[4]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 5 rate");
-		if (entry)
-			pipe_params[j].tc_rate[5] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[5]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 6 rate");
-		if (entry)
-			pipe_params[j].tc_rate[6] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[6]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 7 rate");
-		if (entry)
-			pipe_params[j].tc_rate[7] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[7]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 8 rate");
-		if (entry)
-			pipe_params[j].tc_rate[8] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[8]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 9 rate");
-		if (entry)
-			pipe_params[j].tc_rate[9] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[9]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 10 rate");
-		if (entry)
-			pipe_params[j].tc_rate[10] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[10]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 11 rate");
-		if (entry)
-			pipe_params[j].tc_rate[11] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[11]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 rate");
-		if (entry)
-			pipe_params[j].tc_rate[12] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[12]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 oversubscription weight");
 		if (entry)
@@ -148,7 +179,7 @@ int
 cfg_load_subport_profile(struct rte_cfgfile *cfg,
 	struct rte_sched_subport_profile_params *subport_profile)
 {
-	int i;
+	int i, ret = 0;
 	const char *entry;
 	int profiles;
 
@@ -164,68 +195,85 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 		snprintf(sec_name, sizeof(sec_name), "subport profile %d", i);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb rate");
-		if (entry)
-			subport_profile[i].tb_rate = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tb_rate);
+		if (ret) {
+			return ret;
+		}
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb size");
-		if (entry)
-			subport_profile[i].tb_size = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tb_size);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc period");
-		if (entry)
-			subport_profile[i].tc_period = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_period);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 0 rate");
-		if (entry)
-			subport_profile[i].tc_rate[0] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[0]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 1 rate");
-		if (entry)
-			subport_profile[i].tc_rate[1] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[1]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 2 rate");
-		if (entry)
-			subport_profile[i].tc_rate[2] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[2]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 3 rate");
-		if (entry)
-			subport_profile[i].tc_rate[3] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[3]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 4 rate");
-		if (entry)
-			subport_profile[i].tc_rate[4] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[4]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 5 rate");
-		if (entry)
-			subport_profile[i].tc_rate[5] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[5]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 6 rate");
-		if (entry)
-			subport_profile[i].tc_rate[6] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[6]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 7 rate");
-		if (entry)
-			subport_profile[i].tc_rate[7] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[7]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 8 rate");
-		if (entry)
-			subport_profile[i].tc_rate[8] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[8]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 9 rate");
-		if (entry)
-			subport_profile[i].tc_rate[9] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[9]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 10 rate");
-		if (entry)
-			subport_profile[i].tc_rate[10] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[10]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 11 rate");
-		if (entry)
-			subport_profile[i].tc_rate[11] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[11]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
-		if (entry)
-			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[12]);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
diff --git a/examples/qos_sched/cfg_file.h b/examples/qos_sched/cfg_file.h
index 0dc458aa71..71d280718f 100644
--- a/examples/qos_sched/cfg_file.h
+++ b/examples/qos_sched/cfg_file.h
@@ -8,6 +8,8 @@
 #include <rte_sched.h>
 #include <rte_cfgfile.h>
 
+int parse_u64(const char *entry, uint64_t *val);
+
 int cfg_load_port(struct rte_cfgfile *cfg, struct rte_sched_port_params *port);
 
 int cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe);
diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
index d897fd378b..e3b0930dd0 100644
--- a/examples/qos_sched/init.c
+++ b/examples/qos_sched/init.c
@@ -286,10 +286,18 @@ app_load_cfg_profile(const char *profile)
 	if (file == NULL)
 		rte_exit(EXIT_FAILURE, "Cannot load configuration profile %s\n", profile);
 
-	cfg_load_port(file, &port_params);
-	cfg_load_subport(file, subport_params);
-	cfg_load_subport_profile(file, subport_profile);
-	cfg_load_pipe(file, pipe_profiles);
+	if (cfg_load_port(file, &port_params)) {
+		rte_exit(EXIT_FAILURE, "Invalid port configuration\n");
+	}
+	if (cfg_load_subport(file, subport_params)) {
+		rte_exit (EXIT_FAILURE, "Invalid subport configuration\n");
+	}
+	if (cfg_load_subport_profile(file, subport_profile)) {
+		rte_exit(EXIT_FAILURE, "Invalid subport profile configuration\n");
+	}
+	if (cfg_load_pipe(file, pipe_profiles)) {
+		rte_exit(EXIT_FAILURE, "Invalid pipe profile configuration\n");
+	}
 
 	rte_cfgfile_close(file);
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 1/3] sched: fix subport profile ID
  2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
                     ` (3 preceding siblings ...)
  2022-10-28  8:09   ` [PATCH v5 1/3] sched: fix subport profile ID Megha Ajmera
@ 2022-10-28  9:55   ` Megha Ajmera
  2022-10-28  9:55     ` [PATCH v6 2/3] sched: fix number of subport profiles Megha Ajmera
                       ` (2 more replies)
  4 siblings, 3 replies; 27+ messages in thread
From: Megha Ajmera @ 2022-10-28  9:55 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable, Dumitrescu

In rte_sched_subport_config() API, subport_profile_id is not set correctly.

Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
Cc: cristian.dumitrescu@intel.com

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

diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
index c5fa9e4582..c91697131d 100644
--- a/lib/sched/rte_sched.c
+++ b/lib/sched/rte_sched.c
@@ -1257,8 +1257,6 @@ rte_sched_subport_config(struct rte_sched_port *port,
 
 		n_subports++;
 
-		subport_profile_id = 0;
-
 		/* Port */
 		port->subports[subport_id] = s;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 2/3] sched: fix number of subport profiles
  2022-10-28  9:55   ` [PATCH v6 1/3] sched: fix subport profile ID Megha Ajmera
@ 2022-10-28  9:55     ` Megha Ajmera
  2022-10-28 14:02       ` David Marchand
  2022-10-28  9:55     ` [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
  2022-10-28 14:01     ` [PATCH v6 1/3] sched: fix subport profile ID David Marchand
  2 siblings, 1 reply; 27+ messages in thread
From: Megha Ajmera @ 2022-10-28  9:55 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable

Removed unused subport field from profile.cfg
Correctly using subport profile id in subport config load.

Fixes: 802d214dc880 ("examples/qos_sched: update subport rate dynamically")
Cc: stable@dpdk.org

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 examples/qos_sched/cfg_file.c  | 2 +-
 examples/qos_sched/profile.cfg | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index 3d5d75fcf0..ca871d3287 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -157,7 +157,7 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 
 	profiles = rte_cfgfile_num_sections(cfg, "subport profile",
 					   sizeof("subport profile") - 1);
-	subport_params[0].n_pipe_profiles = profiles;
+	port_params.n_subport_profiles = profiles;
 
 	for (i = 0; i < profiles; i++) {
 		char sec_name[32];
diff --git a/examples/qos_sched/profile.cfg b/examples/qos_sched/profile.cfg
index c9ec187c93..e8de101b6c 100644
--- a/examples/qos_sched/profile.cfg
+++ b/examples/qos_sched/profile.cfg
@@ -26,8 +26,6 @@ number of subports per port = 1
 number of pipes per subport = 4096
 queue sizes = 64 64 64 64 64 64 64 64 64 64 64 64 64
 
-subport 0-8 = 0                ; These subports are configured with subport profile 0
-
 [subport profile 0]
 tb rate = 1250000000           ; Bytes per second
 tb size = 1000000              ; Bytes
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-28  9:55   ` [PATCH v6 1/3] sched: fix subport profile ID Megha Ajmera
  2022-10-28  9:55     ` [PATCH v6 2/3] sched: fix number of subport profiles Megha Ajmera
@ 2022-10-28  9:55     ` Megha Ajmera
  2022-10-28 10:57       ` Dumitrescu, Cristian
  2022-10-28 14:01     ` [PATCH v6 1/3] sched: fix subport profile ID David Marchand
  2 siblings, 1 reply; 27+ messages in thread
From: Megha Ajmera @ 2022-10-28  9:55 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu, stephen; +Cc: stable

- Config load functions updated to support 100G rates
for subport and pipes.
- Added new parse function to convert string to unsigned
long long.
- Added error checks.
- Fixed format warnings.

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
 examples/qos_sched/cfg_file.c | 179 +++++++++++++++++++++-------------
 examples/qos_sched/cfg_file.h |   2 +
 examples/qos_sched/init.c     |  23 ++++-
 3 files changed, 133 insertions(+), 71 deletions(-)

diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.c
index ca871d3287..c75cf9db2e 100644
--- a/examples/qos_sched/cfg_file.c
+++ b/examples/qos_sched/cfg_file.c
@@ -25,6 +25,21 @@ uint32_t n_active_queues;
 
 struct rte_sched_cman_params cman_params;
 
+int parse_u64(const char *entry, uint64_t *val)
+{
+	char *endptr;
+	if (!entry || !val)
+		return -EINVAL;
+
+	errno = 0;
+
+	*val = strtoull(entry, &endptr, 0);
+	if (errno == EINVAL || errno == ERANGE || *endptr != '\0')
+		return -EINVAL;
+
+	return 0;
+}
+
 int
 cfg_load_port(struct rte_cfgfile *cfg, struct rte_sched_port_params *port_params)
 {
@@ -47,7 +62,7 @@ cfg_load_port(struct rte_cfgfile *cfg, struct rte_sched_port_params *port_params
 int
 cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe_params)
 {
-	int i, j;
+	int i, j, ret = 0;
 	char *next;
 	const char *entry;
 	int profiles;
@@ -63,68 +78,84 @@ cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe_params
 		snprintf(pipe_name, sizeof(pipe_name), "pipe profile %d", j);
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb rate");
-		if (entry)
-			pipe_params[j].tb_rate = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tb_rate);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tb size");
-		if (entry)
-			pipe_params[j].tb_size = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tb_size);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc period");
-		if (entry)
-			pipe_params[j].tc_period = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_period);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 0 rate");
-		if (entry)
-			pipe_params[j].tc_rate[0] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[0]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 1 rate");
-		if (entry)
-			pipe_params[j].tc_rate[1] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[1]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 2 rate");
-		if (entry)
-			pipe_params[j].tc_rate[2] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[2]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 3 rate");
-		if (entry)
-			pipe_params[j].tc_rate[3] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[3]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 4 rate");
-		if (entry)
-			pipe_params[j].tc_rate[4] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[4]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 5 rate");
-		if (entry)
-			pipe_params[j].tc_rate[5] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[5]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 6 rate");
-		if (entry)
-			pipe_params[j].tc_rate[6] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[6]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 7 rate");
-		if (entry)
-			pipe_params[j].tc_rate[7] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[7]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 8 rate");
-		if (entry)
-			pipe_params[j].tc_rate[8] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[8]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 9 rate");
-		if (entry)
-			pipe_params[j].tc_rate[9] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[9]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 10 rate");
-		if (entry)
-			pipe_params[j].tc_rate[10] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[10]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 11 rate");
-		if (entry)
-			pipe_params[j].tc_rate[11] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[11]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 rate");
-		if (entry)
-			pipe_params[j].tc_rate[12] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &pipe_params[j].tc_rate[12]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, pipe_name, "tc 12 oversubscription weight");
 		if (entry)
@@ -148,7 +179,7 @@ int
 cfg_load_subport_profile(struct rte_cfgfile *cfg,
 	struct rte_sched_subport_profile_params *subport_profile)
 {
-	int i;
+	int i, ret = 0;
 	const char *entry;
 	int profiles;
 
@@ -164,68 +195,84 @@ cfg_load_subport_profile(struct rte_cfgfile *cfg,
 		snprintf(sec_name, sizeof(sec_name), "subport profile %d", i);
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb rate");
-		if (entry)
-			subport_profile[i].tb_rate = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tb_rate);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tb size");
-		if (entry)
-			subport_profile[i].tb_size = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tb_size);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc period");
-		if (entry)
-			subport_profile[i].tc_period = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_period);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 0 rate");
-		if (entry)
-			subport_profile[i].tc_rate[0] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[0]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 1 rate");
-		if (entry)
-			subport_profile[i].tc_rate[1] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[1]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 2 rate");
-		if (entry)
-			subport_profile[i].tc_rate[2] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[2]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 3 rate");
-		if (entry)
-			subport_profile[i].tc_rate[3] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[3]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 4 rate");
-		if (entry)
-			subport_profile[i].tc_rate[4] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[4]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 5 rate");
-		if (entry)
-			subport_profile[i].tc_rate[5] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[5]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 6 rate");
-		if (entry)
-			subport_profile[i].tc_rate[6] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[6]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 7 rate");
-		if (entry)
-			subport_profile[i].tc_rate[7] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[7]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 8 rate");
-		if (entry)
-			subport_profile[i].tc_rate[8] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[8]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 9 rate");
-		if (entry)
-			subport_profile[i].tc_rate[9] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[9]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 10 rate");
-		if (entry)
-			subport_profile[i].tc_rate[10] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[10]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 11 rate");
-		if (entry)
-			subport_profile[i].tc_rate[11] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[11]);
+		if (ret)
+			return ret;
 
 		entry = rte_cfgfile_get_entry(cfg, sec_name, "tc 12 rate");
-		if (entry)
-			subport_profile[i].tc_rate[12] = (uint64_t)atoi(entry);
+		ret = parse_u64(entry, &subport_profile[i].tc_rate[12]);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
diff --git a/examples/qos_sched/cfg_file.h b/examples/qos_sched/cfg_file.h
index 0dc458aa71..71d280718f 100644
--- a/examples/qos_sched/cfg_file.h
+++ b/examples/qos_sched/cfg_file.h
@@ -8,6 +8,8 @@
 #include <rte_sched.h>
 #include <rte_cfgfile.h>
 
+int parse_u64(const char *entry, uint64_t *val);
+
 int cfg_load_port(struct rte_cfgfile *cfg, struct rte_sched_port_params *port);
 
 int cfg_load_pipe(struct rte_cfgfile *cfg, struct rte_sched_pipe_params *pipe);
diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
index d897fd378b..0709aec10c 100644
--- a/examples/qos_sched/init.c
+++ b/examples/qos_sched/init.c
@@ -280,20 +280,33 @@ app_init_sched_port(uint32_t portid, uint32_t socketid)
 static int
 app_load_cfg_profile(const char *profile)
 {
+	int ret  = 0;
 	if (profile == NULL)
 		return 0;
 	struct rte_cfgfile *file = rte_cfgfile_load(profile, 0);
 	if (file == NULL)
 		rte_exit(EXIT_FAILURE, "Cannot load configuration profile %s\n", profile);
 
-	cfg_load_port(file, &port_params);
-	cfg_load_subport(file, subport_params);
-	cfg_load_subport_profile(file, subport_profile);
-	cfg_load_pipe(file, pipe_profiles);
+	ret = cfg_load_port(file, &port_params);
+	if (ret)
+		goto _app_load_cfg_profile_error_return;
 
+	ret = cfg_load_subport(file, subport_params);
+	if (ret)
+		goto _app_load_cfg_profile_error_return;
+
+	ret = cfg_load_subport_profile(file, subport_profile);
+	if (ret)
+		goto _app_load_cfg_profile_error_return;
+
+	ret = cfg_load_pipe(file, pipe_profiles);
+	if (ret)
+		goto _app_load_cfg_profile_error_return;
+
+_app_load_cfg_profile_error_return:
 	rte_cfgfile_close(file);
 
-	return 0;
+	return ret;
 }
 
 int app_init(void)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-28  9:55     ` [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
@ 2022-10-28 10:57       ` Dumitrescu, Cristian
  2022-10-28 14:04         ` David Marchand
  0 siblings, 1 reply; 27+ messages in thread
From: Dumitrescu, Cristian @ 2022-10-28 10:57 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder, stephen; +Cc: stable



> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Friday, October 28, 2022 10:56 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> stephen@networkplumber.org
> Cc: stable@dpdk.org
> Subject: [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe
> config
> 
> - Config load functions updated to support 100G rates
> for subport and pipes.
> - Added new parse function to convert string to unsigned
> long long.
> - Added error checks.
> - Fixed format warnings.
> 
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
>  examples/qos_sched/cfg_file.c | 179 +++++++++++++++++++++-------------
>  examples/qos_sched/cfg_file.h |   2 +
>  examples/qos_sched/init.c     |  23 ++++-
>  3 files changed, 133 insertions(+), 71 deletions(-)
> 

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


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 1/3] sched: fix subport profile ID
  2022-10-28  9:55   ` [PATCH v6 1/3] sched: fix subport profile ID Megha Ajmera
  2022-10-28  9:55     ` [PATCH v6 2/3] sched: fix number of subport profiles Megha Ajmera
  2022-10-28  9:55     ` [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
@ 2022-10-28 14:01     ` David Marchand
  2 siblings, 0 replies; 27+ messages in thread
From: David Marchand @ 2022-10-28 14:01 UTC (permalink / raw)
  To: Megha Ajmera, cristian.dumitrescu; +Cc: dev, jasvinder.singh, stephen, stable

On Fri, Oct 28, 2022 at 11:58 AM Megha Ajmera <megha.ajmera@intel.com> wrote:
>
> In rte_sched_subport_config() API, subport_profile_id is not set correctly.
>
> Fixes: ac6fcb841b0f ("sched: update subport rate dynamically")
> Cc: cristian.dumitrescu@intel.com

Cc: stable@dpdk.org

>
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> Acked-by: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>

It should be Cristian Dumitrescu.

And there is no "Dumitrescu@dpdk.org".
I removed it before replying.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 2/3] sched: fix number of subport profiles
  2022-10-28  9:55     ` [PATCH v6 2/3] sched: fix number of subport profiles Megha Ajmera
@ 2022-10-28 14:02       ` David Marchand
  0 siblings, 0 replies; 27+ messages in thread
From: David Marchand @ 2022-10-28 14:02 UTC (permalink / raw)
  To: Megha Ajmera; +Cc: dev, jasvinder.singh, cristian.dumitrescu, stephen, stable

On Fri, Oct 28, 2022 at 11:59 AM Megha Ajmera <megha.ajmera@intel.com> wrote:
>

Wrong prefix in the title, it should be examples/qos_sched:.


> Removed unused subport field from profile.cfg
> Correctly using subport profile id in subport config load.

This reads odd... but I'll keep untouched.


>
> Fixes: 802d214dc880 ("examples/qos_sched: update subport rate dynamically")
> Cc: stable@dpdk.org
>
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe config
  2022-10-28 10:57       ` Dumitrescu, Cristian
@ 2022-10-28 14:04         ` David Marchand
  0 siblings, 0 replies; 27+ messages in thread
From: David Marchand @ 2022-10-28 14:04 UTC (permalink / raw)
  To: Dumitrescu, Cristian, Ajmera, Megha
  Cc: dev, Singh, Jasvinder, stephen, stable

On Fri, Oct 28, 2022 at 12:57 PM Dumitrescu, Cristian
<cristian.dumitrescu@intel.com> wrote:
> > Subject: [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe
> > config

As mentionned in 2/3, the title prefix is incorrect.

> >
> > - Config load functions updated to support 100G rates
> > for subport and pipes.
> > - Added new parse function to convert string to unsigned
> > long long.
> > - Added error checks.
> > - Fixed format warnings.

I reformated this a bit as I don't understand why this patch deserves a list.


> >
> > Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> > ---
> >  examples/qos_sched/cfg_file.c | 179 +++++++++++++++++++++-------------
> >  examples/qos_sched/cfg_file.h |   2 +
> >  examples/qos_sched/init.c     |  23 ++++-
> >  3 files changed, 133 insertions(+), 71 deletions(-)
> >
>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>

Series applied with fixes.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-10-28 14:04 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-05 17:22 [PATCH v2] sched: Fix subport profile id not set correctly Megha Ajmera
2022-10-05 19:19 ` Dumitrescu, Cristian
2022-10-11  4:59   ` Ajmera, Megha
2022-10-06 19:00 ` [PATCH 1/3] sched: fix " Megha Ajmera
2022-10-06 19:00   ` [PATCH 2/3] sched: removed unused subport field in hqos profile Megha Ajmera
2022-10-11 14:26     ` Dumitrescu, Cristian
2022-10-06 19:00   ` [PATCH 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
2022-10-11 13:09     ` Dumitrescu, Cristian
2022-10-18  5:40       ` Ajmera, Megha
2022-10-18 13:12         ` Dumitrescu, Cristian
2022-10-19 18:37           ` Stephen Hemminger
2022-10-20  9:47             ` [PATCH v4 1/3] sched: fix subport profile ID Megha Ajmera
2022-10-20  9:47               ` [PATCH v4 2/3] sched: fix number of subport profiles Megha Ajmera
2022-10-20  9:47               ` [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
2022-10-24 18:57                 ` Dumitrescu, Cristian
2022-10-20  9:55             ` [PATCH " Ajmera, Megha
2022-10-11 14:22   ` [PATCH 1/3] sched: fix subport profile id not set correctly Dumitrescu, Cristian
2022-10-28  8:09   ` [PATCH v5 1/3] sched: fix subport profile ID Megha Ajmera
2022-10-28  8:09     ` [PATCH v5 2/3] sched: fix number of subport profiles Megha Ajmera
2022-10-28  8:09     ` [PATCH v5 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
2022-10-28  9:55   ` [PATCH v6 1/3] sched: fix subport profile ID Megha Ajmera
2022-10-28  9:55     ` [PATCH v6 2/3] sched: fix number of subport profiles Megha Ajmera
2022-10-28 14:02       ` David Marchand
2022-10-28  9:55     ` [PATCH v6 3/3] sched: support for 100G+ rates in subport/pipe config Megha Ajmera
2022-10-28 10:57       ` Dumitrescu, Cristian
2022-10-28 14:04         ` David Marchand
2022-10-28 14:01     ` [PATCH v6 1/3] sched: fix subport profile ID David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).