* [dpdk-dev] [PATCH v2] Test/crypto: check valid test_stats before running test
@ 2020-05-15 7:32 Apeksha Gupta
2020-05-15 7:42 ` Ruifeng Wang
2020-05-15 7:56 ` [dpdk-dev] [PATCH v3] " Apeksha Gupta
0 siblings, 2 replies; 8+ messages in thread
From: Apeksha Gupta @ 2020-05-15 7:32 UTC (permalink / raw)
To: dev
Cc: Ruifeng.Wang, declan.doherty, asomalap, anoobj, roy.fan.zhang,
fiona.trahe, rnagadheeraj, adwivedi, jianjay.zhou,
pablo.de.lara.guarch, adamx.dybkowski, akhil.goyal,
Apeksha Gupta
Test_stats is an optional cryptodev op and if it is not defined by the
PMD, it should not run the test cases for it.
Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
---
app/test/test_cryptodev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index c624018ee..1ad650675 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -8801,6 +8801,10 @@ test_stats(void)
if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
return -ENOTSUP;
+ dev = &rte_cryptodevs[ts_params->valid_devs[0]];
+ if (dev->dev_ops->stats_get == 0)
+ return -ENOTSUP;
+
/* Verify the capabilities */
struct rte_cryptodev_sym_capability_idx cap_idx;
cap_idx.type = RTE_CRYPTO_SYM_XFORM_AUTH;
@@ -8820,7 +8824,6 @@ test_stats(void)
"rte_cryptodev_stats_get invalid dev failed");
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], 0) != 0),
"rte_cryptodev_stats_get invalid Param failed");
- dev = &rte_cryptodevs[ts_params->valid_devs[0]];
temp_pfn = dev->dev_ops->stats_get;
dev->dev_ops->stats_get = (cryptodev_stats_get_t)0;
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2] Test/crypto: check valid test_stats before running test
2020-05-15 7:32 [dpdk-dev] [PATCH v2] Test/crypto: check valid test_stats before running test Apeksha Gupta
@ 2020-05-15 7:42 ` Ruifeng Wang
2020-05-15 7:56 ` [dpdk-dev] [PATCH v3] " Apeksha Gupta
1 sibling, 0 replies; 8+ messages in thread
From: Ruifeng Wang @ 2020-05-15 7:42 UTC (permalink / raw)
To: Apeksha Gupta, dev
Cc: declan.doherty, asomalap, anoobj, roy.fan.zhang, fiona.trahe,
rnagadheeraj, adwivedi, jianjay.zhou, pablo.de.lara.guarch,
adamx.dybkowski, Akhil.goyal@nxp.com
> -----Original Message-----
> From: Apeksha Gupta <apeksha.gupta@nxp.com>
> Sent: Friday, May 15, 2020 3:33 PM
> To: dev@dpdk.org
> Cc: Ruifeng Wang <Ruifeng.Wang@arm.com>; declan.doherty@intel.com;
> asomalap@amd.com; anoobj@marvell.com; roy.fan.zhang@intel.com;
> fiona.trahe@intel.com; rnagadheeraj@marvell.com; adwivedi@marvell.com;
> jianjay.zhou@huawei.com; pablo.de.lara.guarch@intel.com;
> adamx.dybkowski@intel.com; Akhil.goyal@nxp.com; Apeksha Gupta
> <apeksha.gupta@nxp.com>
> Subject: [PATCH v2] Test/crypto: check valid test_stats before running test
>
> Test_stats is an optional cryptodev op and if it is not defined by the PMD, it
> should not run the test cases for it.
>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
> ---
> app/test/test_cryptodev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index
> c624018ee..1ad650675 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -8801,6 +8801,10 @@ test_stats(void)
> if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
> return -ENOTSUP;
>
> +dev = &rte_cryptodevs[ts_params->valid_devs[0]];
> +if (dev->dev_ops->stats_get == 0)
> +return -ENOTSUP;
> +
> /* Verify the capabilities */
> struct rte_cryptodev_sym_capability_idx cap_idx;
> cap_idx.type = RTE_CRYPTO_SYM_XFORM_AUTH; @@ -8820,7
> +8824,6 @@ test_stats(void)
> "rte_cryptodev_stats_get invalid dev failed");
> TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0],
> 0) != 0),
> "rte_cryptodev_stats_get invalid Param failed");
> -dev = &rte_cryptodevs[ts_params->valid_devs[0]];
> temp_pfn = dev->dev_ops->stats_get;
> dev->dev_ops->stats_get = (cryptodev_stats_get_t)0;
> TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0],
> &stats)
> --
> 2.17.1
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH v3] Test/crypto: check valid test_stats before running test
2020-05-15 7:32 [dpdk-dev] [PATCH v2] Test/crypto: check valid test_stats before running test Apeksha Gupta
2020-05-15 7:42 ` Ruifeng Wang
@ 2020-05-15 7:56 ` Apeksha Gupta
2020-05-15 8:03 ` Ruifeng Wang
` (2 more replies)
1 sibling, 3 replies; 8+ messages in thread
From: Apeksha Gupta @ 2020-05-15 7:56 UTC (permalink / raw)
To: dev
Cc: Ruifeng.Wang, declan.doherty, asomalap, anoobj, roy.fan.zhang,
fiona.trahe, rnagadheeraj, adwivedi, jianjay.zhou,
pablo.de.lara.guarch, adamx.dybkowski, akhil.goyal,
Apeksha Gupta
Test_stats is an optional cryptodev op and if it is not defined by the
PMD, it should not run the test cases for it.
Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
---
app/test/test_cryptodev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index c624018ee..1ad650675 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -8801,6 +8801,10 @@ test_stats(void)
if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
return -ENOTSUP;
+ dev = &rte_cryptodevs[ts_params->valid_devs[0]];
+ if (dev->dev_ops->stats_get == 0)
+ return -ENOTSUP;
+
/* Verify the capabilities */
struct rte_cryptodev_sym_capability_idx cap_idx;
cap_idx.type = RTE_CRYPTO_SYM_XFORM_AUTH;
@@ -8820,7 +8824,6 @@ test_stats(void)
"rte_cryptodev_stats_get invalid dev failed");
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], 0) != 0),
"rte_cryptodev_stats_get invalid Param failed");
- dev = &rte_cryptodevs[ts_params->valid_devs[0]];
temp_pfn = dev->dev_ops->stats_get;
dev->dev_ops->stats_get = (cryptodev_stats_get_t)0;
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v3] Test/crypto: check valid test_stats before running test
2020-05-15 7:56 ` [dpdk-dev] [PATCH v3] " Apeksha Gupta
@ 2020-05-15 8:03 ` Ruifeng Wang
2020-05-15 10:00 ` Ananyev, Konstantin
2020-05-16 1:55 ` [dpdk-dev] [PATCH v4] " Apeksha Gupta
2 siblings, 0 replies; 8+ messages in thread
From: Ruifeng Wang @ 2020-05-15 8:03 UTC (permalink / raw)
To: Apeksha Gupta, dev
Cc: declan.doherty, asomalap, anoobj, roy.fan.zhang, fiona.trahe,
rnagadheeraj, adwivedi, jianjay.zhou, pablo.de.lara.guarch,
adamx.dybkowski, Akhil.goyal@nxp.com, nd
> -----Original Message-----
> From: Apeksha Gupta <apeksha.gupta@nxp.com>
> Sent: Friday, May 15, 2020 3:56 PM
> To: dev@dpdk.org
> Cc: Ruifeng Wang <Ruifeng.Wang@arm.com>; declan.doherty@intel.com;
> asomalap@amd.com; anoobj@marvell.com; roy.fan.zhang@intel.com;
> fiona.trahe@intel.com; rnagadheeraj@marvell.com; adwivedi@marvell.com;
> jianjay.zhou@huawei.com; pablo.de.lara.guarch@intel.com;
> adamx.dybkowski@intel.com; Akhil.goyal@nxp.com; Apeksha Gupta
> <apeksha.gupta@nxp.com>
> Subject: [PATCH v3] Test/crypto: check valid test_stats before running test
>
> Test_stats is an optional cryptodev op and if it is not defined by the PMD, it
> should not run the test cases for it.
>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
> ---
> app/test/test_cryptodev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index
> c624018ee..1ad650675 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -8801,6 +8801,10 @@ test_stats(void)
> if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
> return -ENOTSUP;
>
> + dev = &rte_cryptodevs[ts_params->valid_devs[0]];
> + if (dev->dev_ops->stats_get == 0)
> + return -ENOTSUP;
> +
> /* Verify the capabilities */
> struct rte_cryptodev_sym_capability_idx cap_idx;
> cap_idx.type = RTE_CRYPTO_SYM_XFORM_AUTH; @@ -8820,7
> +8824,6 @@ test_stats(void)
> "rte_cryptodev_stats_get invalid dev failed");
> TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0],
> 0) != 0),
> "rte_cryptodev_stats_get invalid Param failed");
> - dev = &rte_cryptodevs[ts_params->valid_devs[0]];
> temp_pfn = dev->dev_ops->stats_get;
> dev->dev_ops->stats_get = (cryptodev_stats_get_t)0;
> TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0],
> &stats)
> --
> 2.17.1
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v3] Test/crypto: check valid test_stats before running test
2020-05-15 7:56 ` [dpdk-dev] [PATCH v3] " Apeksha Gupta
2020-05-15 8:03 ` Ruifeng Wang
@ 2020-05-15 10:00 ` Ananyev, Konstantin
2020-05-15 17:45 ` Akhil Goyal
2020-05-16 1:55 ` [dpdk-dev] [PATCH v4] " Apeksha Gupta
2 siblings, 1 reply; 8+ messages in thread
From: Ananyev, Konstantin @ 2020-05-15 10:00 UTC (permalink / raw)
To: Apeksha Gupta, dev
Cc: Ruifeng.Wang, Doherty, Declan, asomalap, anoobj, Zhang, Roy Fan,
Trahe, Fiona, rnagadheeraj, adwivedi, jianjay.zhou,
De Lara Guarch, Pablo, Dybkowski, AdamX, akhil.goyal
> Test_stats is an optional cryptodev op and if it is not defined by the
> PMD, it should not run the test cases for it.
>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
> ---
> app/test/test_cryptodev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index c624018ee..1ad650675 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -8801,6 +8801,10 @@ test_stats(void)
> if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
> return -ENOTSUP;
>
> + dev = &rte_cryptodevs[ts_params->valid_devs[0]];
> + if (dev->dev_ops->stats_get == 0)
> + return -ENOTSUP;
> +
>
rte_cryptodevs[] and dev_ops supposed to be internal for cryptodev
(though in practice they are not).
It is not a good practice for the app to access them directly.
/* Verify the capabilities */
> struct rte_cryptodev_sym_capability_idx cap_idx;
> cap_idx.type = RTE_CRYPTO_SYM_XFORM_AUTH;
> @@ -8820,7 +8824,6 @@ test_stats(void)
> "rte_cryptodev_stats_get invalid dev failed");
> TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], 0) != 0),
> "rte_cryptodev_stats_get invalid Param failed");
> - dev = &rte_cryptodevs[ts_params->valid_devs[0]];
> temp_pfn = dev->dev_ops->stats_get;
> dev->dev_ops->stats_get = (cryptodev_stats_get_t)0;
> TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats)
> --
> 2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v3] Test/crypto: check valid test_stats before running test
2020-05-15 10:00 ` Ananyev, Konstantin
@ 2020-05-15 17:45 ` Akhil Goyal
0 siblings, 0 replies; 8+ messages in thread
From: Akhil Goyal @ 2020-05-15 17:45 UTC (permalink / raw)
To: Ananyev, Konstantin, Apeksha Gupta, dev
Cc: Ruifeng.Wang, Doherty, Declan, asomalap, anoobj, Zhang, Roy Fan,
Trahe, Fiona, rnagadheeraj, adwivedi, jianjay.zhou,
De Lara Guarch, Pablo, Dybkowski, AdamX
Hi Konstantin,
>
> > Test_stats is an optional cryptodev op and if it is not defined by the
> > PMD, it should not run the test cases for it.
> >
> > Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
> > ---
> > app/test/test_cryptodev.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > index c624018ee..1ad650675 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -8801,6 +8801,10 @@ test_stats(void)
> > if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
> > return -ENOTSUP;
> >
> > + dev = &rte_cryptodevs[ts_params->valid_devs[0]];
> > + if (dev->dev_ops->stats_get == 0)
> > + return -ENOTSUP;
> > +
> >
>
> rte_cryptodevs[] and dev_ops supposed to be internal for cryptodev
> (though in practice they are not).
> It is not a good practice for the app to access them directly.
>
Yes, you are absolutely correct. The test case is not properly written.
App should not access the dev_ops directly.
Apeksha,
You can do the following change to fix this.
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 1ed2df898..aa14d42fa 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -8751,7 +8751,6 @@ test_stats(void)
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct rte_cryptodev_stats stats;
struct rte_cryptodev *dev;
- cryptodev_stats_get_t temp_pfn;
if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
return -ENOTSUP;
@@ -8769,19 +8768,16 @@ test_stats(void)
&cap_idx) == NULL)
return -ENOTSUP;
+ if (rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats)
+ == -ENOTSUP)
+ return -ENOTSUP;
+
rte_cryptodev_stats_reset(ts_params->valid_devs[0]);
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0] + 600,
&stats) == -ENODEV),
"rte_cryptodev_stats_get invalid dev failed");
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], 0) != 0),
"rte_cryptodev_stats_get invalid Param failed");
- dev = &rte_cryptodevs[ts_params->valid_devs[0]];
- temp_pfn = dev->dev_ops->stats_get;
- dev->dev_ops->stats_get = (cryptodev_stats_get_t)0;
- TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats)
- == -ENOTSUP),
- "rte_cryptodev_stats_get invalid Param failed");
- dev->dev_ops->stats_get = temp_pfn;
/* Test expected values */
ut_setup();
^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH v4] Test/crypto: check valid test_stats before running test
2020-05-15 7:56 ` [dpdk-dev] [PATCH v3] " Apeksha Gupta
2020-05-15 8:03 ` Ruifeng Wang
2020-05-15 10:00 ` Ananyev, Konstantin
@ 2020-05-16 1:55 ` Apeksha Gupta
2020-05-17 14:09 ` Akhil Goyal
2 siblings, 1 reply; 8+ messages in thread
From: Apeksha Gupta @ 2020-05-16 1:55 UTC (permalink / raw)
To: dev
Cc: Ruifeng.Wang, declan.doherty, asomalap, anoobj, roy.fan.zhang,
fiona.trahe, rnagadheeraj, adwivedi, jianjay.zhou,
pablo.de.lara.guarch, adamx.dybkowski, akhil.goyal,
Apeksha Gupta
Test_stats is an optional cryptodev op and if it is not defined by the
PMD, it should not run the test cases for it.
Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
---
app/test/test_cryptodev.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index c624018ee..83fee232d 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -8795,8 +8795,6 @@ test_stats(void)
{
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct rte_cryptodev_stats stats;
- struct rte_cryptodev *dev;
- cryptodev_stats_get_t temp_pfn;
if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
return -ENOTSUP;
@@ -8813,6 +8811,9 @@ test_stats(void)
if (rte_cryptodev_sym_capability_get(ts_params->valid_devs[0],
&cap_idx) == NULL)
return -ENOTSUP;
+ if (rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats)
+ == -ENOTSUP)
+ return -ENOTSUP;
rte_cryptodev_stats_reset(ts_params->valid_devs[0]);
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0] + 600,
@@ -8820,13 +8821,6 @@ test_stats(void)
"rte_cryptodev_stats_get invalid dev failed");
TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], 0) != 0),
"rte_cryptodev_stats_get invalid Param failed");
- dev = &rte_cryptodevs[ts_params->valid_devs[0]];
- temp_pfn = dev->dev_ops->stats_get;
- dev->dev_ops->stats_get = (cryptodev_stats_get_t)0;
- TEST_ASSERT((rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats)
- == -ENOTSUP),
- "rte_cryptodev_stats_get invalid Param failed");
- dev->dev_ops->stats_get = temp_pfn;
/* Test expected values */
ut_setup();
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v4] Test/crypto: check valid test_stats before running test
2020-05-16 1:55 ` [dpdk-dev] [PATCH v4] " Apeksha Gupta
@ 2020-05-17 14:09 ` Akhil Goyal
0 siblings, 0 replies; 8+ messages in thread
From: Akhil Goyal @ 2020-05-17 14:09 UTC (permalink / raw)
To: Apeksha Gupta, dev
Cc: Ruifeng.Wang, declan.doherty, asomalap, anoobj, roy.fan.zhang,
fiona.trahe, rnagadheeraj, adwivedi, jianjay.zhou,
pablo.de.lara.guarch, adamx.dybkowski, Apeksha Gupta
> Test_stats is an optional cryptodev op and if it is not defined by the
> PMD, it should not run the test cases for it.
>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
> ---
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
Title and description of the patch updated as
test/crypto: fix statistics case
The test case - test_stats is directly accessing the
cryptodev and its dev_ops which are internal to library
and should not be used directly by the application.
However, the test case is also missing to check for the
error ENOTSUP. It should skip the case if the API returns
ENOTSUP. This patch fixes these two issues.
Fixes: 202d375c60bc ("app/test: add cryptodev unit and performance tests")
Cc: stable@dpdk.org
Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
Applied to dpdk-next-crypto.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-17 14:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 7:32 [dpdk-dev] [PATCH v2] Test/crypto: check valid test_stats before running test Apeksha Gupta
2020-05-15 7:42 ` Ruifeng Wang
2020-05-15 7:56 ` [dpdk-dev] [PATCH v3] " Apeksha Gupta
2020-05-15 8:03 ` Ruifeng Wang
2020-05-15 10:00 ` Ananyev, Konstantin
2020-05-15 17:45 ` Akhil Goyal
2020-05-16 1:55 ` [dpdk-dev] [PATCH v4] " Apeksha Gupta
2020-05-17 14:09 ` Akhil Goyal
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).