DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Trahe, Fiona" <fiona.trahe@intel.com>
To: Ayuj Verma <ayverma@marvell.com>,
	"akhil.goyal@nxp.com" <akhil.goyal@nxp.com>,
	"Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>,
	"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
Cc: Shally Verma <shallyv@marvell.com>,
	Sunila Sahu <ssahu@marvell.com>,
	Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>,
	Arvind Desai <adesai@marvell.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"Trahe, Fiona" <fiona.trahe@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP
Date: Fri, 12 Apr 2019 10:25:05 +0000	[thread overview]
Message-ID: <348A99DA5F5B7549AA880327E580B4358973F4C4@IRSMSX101.ger.corp.intel.com> (raw)
In-Reply-To: <MN2PR18MB25425D5901DB11DFC65ECDFFAD280@MN2PR18MB2542.namprd18.prod.outlook.com>

Hi Ayuj

From: Ayuj Verma [mailto:ayverma@marvell.com]
Sent: Friday, April 12, 2019 8:08 AM
To: Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>; Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>; dev@dpdk.org
Subject: Re: [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP


Hi Fiona,



Please see inline.



Thanks and regards

Ayuj Verma

________________________________
From: Trahe, Fiona <fiona.trahe@intel.com<mailto:fiona.trahe@intel.com>>
Sent: 09 April 2019 20:47
To: Ayuj Verma; akhil.goyal@nxp.com<mailto:akhil.goyal@nxp.com>; Kusztal, ArkadiuszX; De Lara Guarch, Pablo
Cc: Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org<mailto:dev@dpdk.org>; Trahe, Fiona
Subject: RE: [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP

Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Tuesday, April 9, 2019 12:34 PM
> To: akhil.goyal@nxp.com<mailto:akhil.goyal@nxp.com>; Trahe, Fiona <fiona.trahe@intel.com<mailto:fiona.trahe@intel.com>>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com<mailto:pablo.de.lara.guarch@intel.com>>
> Cc: shallyv@marvell.com<mailto:shallyv@marvell.com>; ssahu@marvell.com<mailto:ssahu@marvell.com>; kkotamarthy@marvell.com<mailto:kkotamarthy@marvell.com>; adesai@marvell.com<mailto:adesai@marvell.com>;
> dev@dpdk.org<mailto:dev@dpdk.org>; Ayuj Verma <ayverma@marvell.com<mailto:ayverma@marvell.com>>
> Subject: [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP
>
> Return -ENOTSUP for unsupported tests
>
> Signed-off-by: Ayuj Verma <ayverma@marvell.com<mailto:ayverma@marvell.com>>
> Signed-off-by: Shally Verma <shallyv@marvell.com<mailto:shallyv@marvell.com>>
> ---
>  app/test/test_cryptodev_asym.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
> index d2efce9..feed3a8 100644
> --- a/app/test/test_cryptodev_asym.c
> +++ b/app/test/test_cryptodev_asym.c
> @@ -352,7 +352,7 @@ struct test_cases_array {
>                RTE_LOG(INFO, USER1,
>                                "Device doesn't support sign op with "
>                                "exponent key type. Test Skipped\n");
> -             return TEST_SKIPPED;
> +             return -ENOTSUP;
>        }
>
>        sess = rte_cryptodev_asym_session_create(sess_mpool);
> @@ -498,7 +498,7 @@ struct test_cases_array {
>                RTE_LOG(INFO, USER1,
>                                "Device doesn't support sign op with "
>                                "exponent key type. Test Skipped\n");
> -             return TEST_SKIPPED;
> +             return -ENOTSUP;
>        }
>
>        sess = rte_cryptodev_asym_session_create(sess_mpool);
> @@ -1261,7 +1261,7 @@ static inline void print_asym_capa(
>                &modinv_xform.xform_type, "modinv") < 0) {
>                RTE_LOG(ERR, USER1,
>                                 "Invalid ASYNC algorithm specified\n");
> -             return -1;
> +             return -ENOTSUP;
[Fiona] this looks more like a test code bug rather than an indication that the device doesn't support modinv. SO should still return -1.
Also - while you're updating, can you please fix the typo in the trace - ASYNC should be ASYMM

[Ayuj]  Each test execute if device supports that algorithm else it is skipped. Thus, here it checks if modinv is not supported in capability then skip the test, which looks okay to me.

So, why do you say it is a bug?
Probably message is not proper should have been "Device doesn't support MODINV"

Will update typo.

[Fiona] This is checking that the "modinv" string matches an element in the API enum. It's not checking anything to do with the device.

The device capability is retrieved by the following line:
capability = rte_cryptodev_asym_capability_get(dev_id,

                                                                    &cap_idx);

But actually is not checked. It should have a NULL check before moving on to the len check,

Else there can be a segfault in the len check function.

Can you add that NULL check please - and that should return -ENOTSUP.

Same with the modex test.

  parent reply	other threads:[~2019-04-12 10:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-09 11:33 [dpdk-dev] [PATCH v2] fix return value for skipped tests Ayuj Verma
2019-04-09 11:33 ` Ayuj Verma
2019-04-09 11:33 ` [dpdk-dev] [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP Ayuj Verma
2019-04-09 11:33   ` Ayuj Verma
2019-04-09 14:55   ` Bruce Richardson
2019-04-09 14:55     ` Bruce Richardson
2019-04-10 12:33     ` [dpdk-dev] [EXT] " Ayuj Verma
2019-04-10 12:33       ` Ayuj Verma
2019-04-10 12:48       ` Bruce Richardson
2019-04-10 12:48         ` Bruce Richardson
2019-04-09 15:17   ` [dpdk-dev] " Trahe, Fiona
2019-04-09 15:17     ` Trahe, Fiona
2019-04-12  7:08     ` Ayuj Verma
2019-04-12  7:08       ` Ayuj Verma
2019-04-12 10:25       ` Trahe, Fiona [this message]
2019-04-12 10:25         ` Trahe, Fiona

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=348A99DA5F5B7549AA880327E580B4358973F4C4@IRSMSX101.ger.corp.intel.com \
    --to=fiona.trahe@intel.com \
    --cc=adesai@marvell.com \
    --cc=akhil.goyal@nxp.com \
    --cc=arkadiuszx.kusztal@intel.com \
    --cc=ayverma@marvell.com \
    --cc=dev@dpdk.org \
    --cc=kkotamarthy@marvell.com \
    --cc=pablo.de.lara.guarch@intel.com \
    --cc=shallyv@marvell.com \
    --cc=ssahu@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).