Fix unit tests execution for ENA PMD
cover Fix unit tests execution for ENA PMD
Hi,
ENA PMD uses timer service to implement various periodic device status check routine. Because of that, it already initializes the timer subsystem. As unit tests also initalizes the timer subsystem, return value is equal to -EAGAIN, as it was already done by the ENA PMD.
This patch set adds missing documentation that this kind of return code may appear and this is not a failure and also changes unit tests tool, to do not fail if timer subsystem was already initialized.
Stanislaw Kardach (2):
timer: clarify subsystem_init return value
test: proceed if timer subsystem was initialized
app/test/test.c | 11 ++++++----- lib/librte_timer/rte_timer.h | 1 + 2 files changed, 7 insertions(+), 5 deletions(-)
Signature · 1 line
2.25.1
PATCH 1/2 timer: clarify subsystem_init return value
From: Stanislaw Kardach <[email protected]>
rte_timer_subsystem_init() may return -EALREADY if it has been already initialized. Therefore put explicitly into doxygen that this is not a failure for the application.
--- lib/librte_timer/rte_timer.h | 1 + 1 file changed, 1 insertion(+)
lib/librte_timer/rte_timer.h +1 -0
@@ -171,6 +171,7 @@ int rte_timer_data_dealloc(uint32_t id);* - 0: Success * - -ENOMEM: Unable to allocate memory needed to initialize timer * subsystem +* - -EALREADY: timer subsystem was already initialized. Not an error. */ int rte_timer_subsystem_init (void );
Signature · 1 line
2.25.1
Re: [dpdk-dev] [PATCH 1/2] timer: clarify subsystem_init return value
Quoted 11 lines
-----Original Message-----
From: Michal Krawczyk <[email protected]>
Sent: Friday, March 26, 2021 5:48 AM
To: [email protected]; Robert Sanford <[email protected]>; Carrillo, Erik G <[email protected]>
Cc: Stanislaw Kardach <[email protected]>; Michal Krawczyk <[email protected]>
Subject: [PATCH 1/2] timer: clarifysubsystem_initreturn valueFrom: Stanislaw Kardach <[email protected]>
Quoted 7 lines
rte_timer_subsystem_init()may return-EALREADYif it has been already initialized. Therefore put explicitly into doxygen that this is not a failure for the application.Signed-off-by Stanislaw Kardach <[email protected]>Reviewed-by Michal Krawczyk <[email protected]>---
Thanks, Michal.
PATCH 2/2 test: proceed if timer subsystem was initialized
In reply to Michal KrawczykFrom: Stanislaw Kardach <[email protected]>
rte_timer_subsystem_init() may return -EALREADY if the timer subsystem was already initialized. This can happen i.e. in PMD code (see eth_ena_dev_init). This is not an error, rather a notification as the initialization function simply returns without any action taken.
--- app/test/test.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
app/test/test.c +6 -5
@@ -134,8 +134,13 @@ main(int argc, char **argv) goto out; } + argv += ret; + + prgname = argv[0 ]; +#ifdef RTE_LIB_TIMER -if (rte_timer_subsystem_init () <0 ) { + ret =rte_timer_subsystem_init (); +if (ret <0 && ret != -EALREADY ) { ret =-1 ; goto out; }
@@ -146,10 +151,6 @@ main(int argc, char **argv) goto out; } - argv += ret; - - prgname = argv[0 ]; - recursive_call =getenv (RECURSIVE_ENV_VAR );if (recursive_call !=NULL ) { ret =do_recursive_call ();
Signature · 1 line
2.25.1
Re: [dpdk-dev] [PATCH 2/2] test: proceed if timer subsystem was initialized
26/03/2021 11:47, Michal Krawczyk:
Quoted 6 lines
From: Stanislaw Kardach <[email protected]>
rte_timer_subsystem_init()may return-EALREADYif the timer subsystem was already initialized. This can happen i.e. in PMD code (seeeth_ena_dev_init). This is not an error, rather a notification as the initialization function simply returns without any action taken.
Missing these lines:
Quoted 26 lines
Signed-off-by Stanislaw Kardach <[email protected]>Reviewed-by Michal Krawczyk <[email protected]>--- app/test/test.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)app/test/test.c +6
-1@@ -134,8 +134,13 @@ main(int argc, char **argv) goto out; } + argv += ret; + + prgname = argv[0 ]; +#ifdef RTE_LIB_TIMER -if (rte_timer_subsystem_init () <0 ) { + ret =rte_timer_subsystem_init (); +if (ret <0 && ret != -EALREADY ) { ret =-1 ; goto out; }
app/test/test.c main(int argc, char **argv) · line 152
Quoted 7 lines
app/test/test.c +0
-4@@ -146,10 +151,6 @@ main(int argc, char **argv) goto out; } - argv += ret; - - prgname = argv[0 ]; -
How this change for argv/prgname is related to the fix?
Re: [dpdk-dev] [PATCH 2/2] test: proceed if timer subsystem was initialized
Hi Thomas,
Thanks for the review.
Quoted 8 lines · Thomas Monjalon
26/03/2021 11:47, Michal Krawczyk:
Quoted 6 lines
From: Stanislaw Kardach <[email protected]>
rte_timer_subsystem_init()may return-EALREADYif the timer subsystem was already initialized. This can happen i.e. in PMD code (seeeth_ena_dev_init). This is not an error, rather a notification as the initialization function simply returns without any action taken.
Quoted 4 lines
Missing these lines:
Fixes 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")
Will add in V2.
Quoted 31 lines
Quoted 31 lines
Signed-off-by Stanislaw Kardach <[email protected]>Reviewed-by Michal Krawczyk <[email protected]>--- app/test/test.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)app/test/test.c +6
-1@@ -134,8 +134,13 @@ main(int argc, char **argv) goto out; } + argv += ret; + + prgname = argv[0 ]; +#ifdef RTE_LIB_TIMER -if (rte_timer_subsystem_init () <0 ) { + ret =rte_timer_subsystem_init (); +if (ret <0 && ret != -EALREADY ) { ret =-1 ; goto out; }@@ -146,10 +151,6 @@ main(int argc, char **argv) goto out; }
Quoted 7 lines
app/test/test.c main(int argc, char **argv) · line 152
Quoted 4 lines
app/test/test.c +0
-4- argv += ret; - - prgname = argv[0 ]; -How this change for argv/prgname is related to the fix?
This patch saves the return value of rte_timer_subsystem_init() in ret which overwrites the previous ret that held the number of arguments consumed by rte_eal_init(). Similarly because rte_eal_init() reshuffles argv, the prgname is effectively at argv[ret]. So I need to move this logic before the timer subsystem check.
Re: [dpdk-dev] [PATCH 2/2] test: proceed if timer subsystem was initialized
06/04/2021 17:31, Stanisław Kardach:
Quoted 59 lines
Hi Thomas,
Thanks for the review.
Quoted 8 lines · Thomas Monjalon
26/03/2021 11:47, Michal Krawczyk:
Quoted 6 lines
From: Stanislaw Kardach <[email protected]>
rte_timer_subsystem_init()may return-EALREADYif the timer subsystem was already initialized. This can happen i.e. in PMD code (seeeth_ena_dev_init). This is not an error, rather a notification as the initialization function simply returns without any action taken.Quoted 4 lines
Missing these lines:
Fixes 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")Will add in V2.
Quoted 31 lines
Quoted 31 lines
Signed-off-by Stanislaw Kardach <[email protected]>Reviewed-by Michal Krawczyk <[email protected]>--- app/test/test.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)app/test/test.c +6
-1@@ -134,8 +134,13 @@ main(int argc, char **argv) goto out; } + argv += ret; + + prgname = argv[0 ]; +#ifdef RTE_LIB_TIMER -if (rte_timer_subsystem_init () <0 ) { + ret =rte_timer_subsystem_init (); +if (ret <0 && ret != -EALREADY ) { ret =-1 ; goto out; }@@ -146,10 +151,6 @@ main(int argc, char **argv) goto out; }Quoted 7 lines
app/test/test.c main(int argc, char **argv) · line 152
Quoted 4 lines
app/test/test.c +0
-4- argv += ret; - - prgname = argv[0 ]; -How this change for argv/prgname is related to the fix?
Quoted 5 lines
This patch saves the return value of
rte_timer_subsystem_init()in ret which overwrites the previous ret that held the number of arguments consumed byrte_eal_init(). Similarly becauserte_eal_init()reshuffles argv, the prgname is effectively at argv[ret]. So I need to move this logic before the timer subsystem check.
OK I didn't see the consequence on ret variable.
In this case I can merge with the added lines.
No need of v2.
Thomas Monjalon
Applied26/03/2021 11:47, Michal Krawczyk:
Quoted 5 lines
ENA PMD uses timer service to implement various periodic device status check routine. Because of that, it already initializes the timer subsystem. As unit tests also initalizes the timer subsystem, return value is equal to
-EAGAIN, as it was already done by the ENA PMD.
Quoted 7 lines
This patch set adds missing documentation that this kind of return code may appear and this is not a failure and also changes unit tests tool, to do not fail if timer subsystem was already initialized.
Stanislaw Kardach (2):
timer: clarifysubsystem_initreturn value
test: proceed if timer subsystem was initialized
Applied, thanks