[dpdk-dev] [PATCH] examples/l3fw-power: do not exit on power lib init failure

[dpdk-dev] [PATCH] examples/l3fw-power: do not exit on power lib init failure

[Radu Nicolau]

Do not exit the application if power library fails to initialize
or high performance cores configuration cannot be used.

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 examples/l3fwd-power/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d15cd52..42518fe 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1683,10 +1683,10 @@ main(int argc, char **argv)
 		rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n");
 
 	if (init_power_library())
-		rte_exit(EXIT_FAILURE, "init_power_library failed\n");
+		RTE_LOG(ERR, POWER, "init_power_library failed\n");
 
 	if (update_lcore_params() < 0)
-		rte_exit(EXIT_FAILURE, "update_lcore_params failed\n");
+		RTE_LOG(ERR, POWER, "update_lcore_params failed\n");
 
 	if (check_lcore_params() < 0)
 		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
-- 
2.7.5

Re: [dpdk-dev] [PATCH] examples/l3fw-power: do not exit on power lib init failure

[De Lara Guarch, Pablo]

Hi Radu,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Radu Nicolau
> Sent: Wednesday, July 18, 2018 2:50 PM
> To: dev@dpdk.org
> Cc: Hunt, David <david.hunt@intel.com>; Yao, Lei A <lei.a.yao@intel.com>;
> Nicolau, Radu <radu.nicolau@intel.com>
> Subject: [dpdk-dev] [PATCH] examples/l3fw-power: do not exit on power lib init
> failure
> 
> Do not exit the application if power library fails to initialize or high performance
> cores configuration cannot be used.

I would say this deserves more explanation. It doesn't look correct to me to keep log an error that
the power library couldn't be initialized and still the sample application keeps running.

Actually, the way init_power_library is coded, it returns the output of the call of rte_power_init on the last core
(ret is overwritten for each core), which looks wrong to me.

Thanks,
Pablo
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---

[dpdk-dev] [PATCH v2] examples/l3fw-power: do not exit on power lib init failure

[Radu Nicolau]

Do not exit the application if power library fails to initialize
or high performance cores configuration cannot be used.

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
v2: updated init_power_library to return error code if any core init fails


 examples/l3fwd-power/main.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d15cd52..e73b853 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1638,11 +1638,13 @@ init_power_library(void)
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		if (rte_lcore_is_enabled(lcore_id)) {
 			/* init power management library */
-			ret = rte_power_init(lcore_id);
-			if (ret)
+			int cpi_ret = rte_power_init(lcore_id);
+			if (cpi_ret) {
 				RTE_LOG(ERR, POWER,
 				"Library initialization failed on core %u\n",
 				lcore_id);
+				ret = -1;
+			}
 		}
 	}
 	return ret;
@@ -1683,10 +1685,10 @@ main(int argc, char **argv)
 		rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n");
 
 	if (init_power_library())
-		rte_exit(EXIT_FAILURE, "init_power_library failed\n");
+		RTE_LOG(ERR, POWER, "init_power_library failed\n");
 
 	if (update_lcore_params() < 0)
-		rte_exit(EXIT_FAILURE, "update_lcore_params failed\n");
+		RTE_LOG(ERR, POWER, "update_lcore_params failed\n");
 
 	if (check_lcore_params() < 0)
 		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
-- 
2.7.5

Re: [dpdk-dev] [PATCH] examples/l3fw-power: do not exit on power lib init failure

[Radu Nicolau]

On 7/26/2018 6:39 PM, De Lara Guarch, Pablo wrote:
> Hi Radu,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Radu Nicolau
>> Sent: Wednesday, July 18, 2018 2:50 PM
>> To: dev@dpdk.org
>> Cc: Hunt, David <david.hunt@intel.com>; Yao, Lei A <lei.a.yao@intel.com>;
>> Nicolau, Radu <radu.nicolau@intel.com>
>> Subject: [dpdk-dev] [PATCH] examples/l3fw-power: do not exit on power lib init
>> failure
>>
>> Do not exit the application if power library fails to initialize or high performance
>> cores configuration cannot be used.
> I would say this deserves more explanation. It doesn't look correct to me to keep log an error that
> the power library couldn't be initialized and still the sample application keeps running.
The application can also be used without the power library features, and 
this was also the previous behavior, before high/regular performance 
core feature was added.
>
> Actually, the way init_power_library is coded, it returns the output of the call of rte_power_init on the last core
> (ret is overwritten for each core), which looks wrong to me.
I will update init_power_library to return -1 if rte_power_init fails 
for at least one core.
>
> Thanks,
> Pablo
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---

Re: [dpdk-dev] [PATCH v2] examples/l3fw-power: do not exit on power lib init failure

[Thomas Monjalon]

No review?

27/07/2018 11:53, Radu Nicolau:
> Do not exit the application if power library fails to initialize
> or high performance cores configuration cannot be used.
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> v2: updated init_power_library to return error code if any core init fails
> 
> 
>  examples/l3fwd-power/main.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> index d15cd52..e73b853 100644
> --- a/examples/l3fwd-power/main.c
> +++ b/examples/l3fwd-power/main.c
> @@ -1638,11 +1638,13 @@ init_power_library(void)
>  	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
>  		if (rte_lcore_is_enabled(lcore_id)) {
>  			/* init power management library */
> -			ret = rte_power_init(lcore_id);
> -			if (ret)
> +			int cpi_ret = rte_power_init(lcore_id);
> +			if (cpi_ret) {
>  				RTE_LOG(ERR, POWER,
>  				"Library initialization failed on core %u\n",
>  				lcore_id);
> +				ret = -1;
> +			}
>  		}
>  	}
>  	return ret;
> @@ -1683,10 +1685,10 @@ main(int argc, char **argv)
>  		rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n");
>  
>  	if (init_power_library())
> -		rte_exit(EXIT_FAILURE, "init_power_library failed\n");
> +		RTE_LOG(ERR, POWER, "init_power_library failed\n");
>  
>  	if (update_lcore_params() < 0)
> -		rte_exit(EXIT_FAILURE, "update_lcore_params failed\n");
> +		RTE_LOG(ERR, POWER, "update_lcore_params failed\n");
>  
>  	if (check_lcore_params() < 0)
>  		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
> 

Re: [dpdk-dev] [PATCH v2] examples/l3fw-power: do not exit on power lib init failure

[Hunt, David]

On 27/7/2018 10:53 AM, Radu Nicolau wrote:
> Do not exit the application if power library fails to initialize
> or high performance cores configuration cannot be used.
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> v2: updated init_power_library to return error code if any core init fails
>
>
>   examples/l3fwd-power/main.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> index d15cd52..e73b853 100644
> --- a/examples/l3fwd-power/main.c
> +++ b/examples/l3fwd-power/main.c
> @@ -1638,11 +1638,13 @@ init_power_library(void)
>   	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
>   		if (rte_lcore_is_enabled(lcore_id)) {
>   			/* init power management library */
> -			ret = rte_power_init(lcore_id);
> -			if (ret)
> +			int cpi_ret = rte_power_init(lcore_id);
> +			if (cpi_ret) {
>   				RTE_LOG(ERR, POWER,
>   				"Library initialization failed on core %u\n",
>   				lcore_id);
> +				ret = -1;
> +			}
>   		}
>   	}
>   	return ret;
> @@ -1683,10 +1685,10 @@ main(int argc, char **argv)
>   		rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n");
>   
>   	if (init_power_library())
> -		rte_exit(EXIT_FAILURE, "init_power_library failed\n");
> +		RTE_LOG(ERR, POWER, "init_power_library failed\n");
>   
>   	if (update_lcore_params() < 0)
> -		rte_exit(EXIT_FAILURE, "update_lcore_params failed\n");
> +		RTE_LOG(ERR, POWER, "update_lcore_params failed\n");
>   
>   	if (check_lcore_params() < 0)
>   		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");

Hi Radu,

I think this patch needs to be seen as a Fix instead of a patch.
The patch that introduced the API to query the power capabilities of a 
core moved around some
functionality that now causes the application to exit earlier than 
previously was the case, making
problems for the test framework.
I think if you added some explanation around this, and the following 
line, it would be fine:

Fixes: f88e7c175a68 ("examples/l3fwd-power: add high/regular perf cores 
options")

Regards,
Dave.

[dpdk-dev] [PATCH v3] examples/l3fw-power: do not exit on power lib init failure

[Radu Nicolau]

Do not exit the application if power library fails to initialize
or high performance cores configuration cannot be used - this was the
behavior of the application before the patch that added the high/regular
performance option and also this is the normally expected behavior, the
application can be used with no power options, i.e. test framework.

Fixes: f88e7c175a68 ("examples/l3fwd-power: add high/regular perf cores options")

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
v3: updated commit msg
v2: updated init_power_library to return error code if any core init fails


 examples/l3fwd-power/main.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d15cd52..e73b853 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1638,11 +1638,13 @@ init_power_library(void)
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		if (rte_lcore_is_enabled(lcore_id)) {
 			/* init power management library */
-			ret = rte_power_init(lcore_id);
-			if (ret)
+			int cpi_ret = rte_power_init(lcore_id);
+			if (cpi_ret) {
 				RTE_LOG(ERR, POWER,
 				"Library initialization failed on core %u\n",
 				lcore_id);
+				ret = -1;
+			}
 		}
 	}
 	return ret;
@@ -1683,10 +1685,10 @@ main(int argc, char **argv)
 		rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n");
 
 	if (init_power_library())
-		rte_exit(EXIT_FAILURE, "init_power_library failed\n");
+		RTE_LOG(ERR, POWER, "init_power_library failed\n");
 
 	if (update_lcore_params() < 0)
-		rte_exit(EXIT_FAILURE, "update_lcore_params failed\n");
+		RTE_LOG(ERR, POWER, "update_lcore_params failed\n");
 
 	if (check_lcore_params() < 0)
 		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
-- 
2.7.5

Re: [dpdk-dev] [PATCH v3] examples/l3fw-power: do not exit on power lib init failure

[Hunt, David]

On 7/8/2018 2:24 PM, Radu Nicolau wrote:
> Do not exit the application if power library fails to initialize
> or high performance cores configuration cannot be used - this was the
> behavior of the application before the patch that added the high/regular
> performance option and also this is the normally expected behavior, the
> application can be used with no power options, i.e. test framework.
>
> Fixes: f88e7c175a68 ("examples/l3fwd-power: add high/regular perf cores options")
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---

  Acked-by: David Hunt <david.hunt@intel.com>

Re: [dpdk-dev] [PATCH v3] examples/l3fw-power: do not exit on power lib init failure

[Radu Nicolau]

On 8/7/2018 4:45 PM, Hunt, David wrote:
> On 7/8/2018 2:24 PM, Radu Nicolau wrote:
>> Do not exit the application if power library fails to initialize
>> or high performance cores configuration cannot be used - this was the
>> behavior of the application before the patch that added the high/regular
>> performance option and also this is the normally expected behavior, the
>> application can be used with no power options, i.e. test framework.
>>
>> Fixes: f88e7c175a68 ("examples/l3fwd-power: add high/regular perf 
>> cores options")
>>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>
>  Acked-by: David Hunt <david.hunt@intel.com>
After further internal discussions we will revise the behavior of the 
app when exiting due to failed init.
We will try to get a new patch for 18.11.