From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id AE1465B2C for ; Fri, 12 Apr 2019 12:25:10 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Apr 2019 03:25:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,340,1549958400"; d="scan'208,217";a="160648153" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by fmsmga002.fm.intel.com with ESMTP; 12 Apr 2019 03:25:07 -0700 Received: from irsmsx156.ger.corp.intel.com (10.108.20.68) by IRSMSX151.ger.corp.intel.com (163.33.192.59) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 12 Apr 2019 11:25:06 +0100 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.115]) by IRSMSX156.ger.corp.intel.com ([169.254.3.53]) with mapi id 14.03.0415.000; Fri, 12 Apr 2019 11:25:06 +0100 From: "Trahe, Fiona" To: Ayuj Verma , "akhil.goyal@nxp.com" , "Kusztal, ArkadiuszX" , "De Lara Guarch, Pablo" CC: Shally Verma , Sunila Sahu , Kanaka Durga Kotamarthy , Arvind Desai , "dev@dpdk.org" , "Trahe, Fiona" Thread-Topic: [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP Thread-Index: AQHU7sgdjnB7ftUqtUGurkGwlNYoR6Yz7yswgAQffgCAAEL6UA== Date: Fri, 12 Apr 2019 10:25:05 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B4358973F4C4@IRSMSX101.ger.corp.intel.com> References: <1554809619-21164-1-git-send-email-ayverma@marvell.com> <1554809619-21164-2-git-send-email-ayverma@marvell.com>, <348A99DA5F5B7549AA880327E580B4358973CC8B@IRSMSX101.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjFkNzkyMTUtOGUxMy00YzM5LWFjOTgtMGQ4ODMxZjRjZDlhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibXg1RGV6cnlLY1dJUjRDbkJRak9LVnB4XC95S2x6cWVuUGNhdTV5a2lPOGVMQVpORXloNlp4NXZCWEJYYzR6SWoifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.181] MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Apr 2019 10:25:16 -0000 Hi Ayuj From: Ayuj Verma [mailto:ayverma@marvell.com] Sent: Friday, April 12, 2019 8:08 AM To: Trahe, Fiona ; akhil.goyal@nxp.com; Kusztal, Ark= adiuszX ; De Lara Guarch, Pablo Cc: Shally Verma ; Sunila Sahu ; Ka= naka Durga Kotamarthy ; Arvind Desai ; 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 > Sent: 09 April 2019 20:47 To: Ayuj Verma; akhil.goyal@nxp.com; Kusztal, A= rkadiuszX; De Lara Guarch, Pablo Cc: Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@d= pdk.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; Trahe, Fiona >; Kusztal, ArkadiuszX > >; De L= ara Guarch, Pablo > > Cc: shallyv@marvell.com; ssahu@marvell.com; kkotamarthy@marvell.com; adesai@marvell.com; > dev@dpdk.org; Ayuj Verma > > Subject: [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP > > Return -ENOTSUP for unsupported tests > > Signed-off-by: Ayuj Verma > > Signed-off-by: Shally Verma > > --- > 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_asy= m.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 =3D 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 =3D 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 - AS= YNC should be ASYMM [Ayuj] Each test execute if device supports that algorithm else it is skip= ped. Thus, here it checks if modinv is not supported in capability then ski= p 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 MOD= INV" 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 =3D rte_cryptodev_asym_capability_get(dev_id, &cap_id= x); But actually is not checked. It should have a NULL check before moving on t= o 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 00D76A0096 for ; Fri, 12 Apr 2019 12:25:20 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 58A2A5B2E; Fri, 12 Apr 2019 12:25:17 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id AE1465B2C for ; Fri, 12 Apr 2019 12:25:10 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Apr 2019 03:25:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,340,1549958400"; d="scan'208,217";a="160648153" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by fmsmga002.fm.intel.com with ESMTP; 12 Apr 2019 03:25:07 -0700 Received: from irsmsx156.ger.corp.intel.com (10.108.20.68) by IRSMSX151.ger.corp.intel.com (163.33.192.59) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 12 Apr 2019 11:25:06 +0100 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.115]) by IRSMSX156.ger.corp.intel.com ([169.254.3.53]) with mapi id 14.03.0415.000; Fri, 12 Apr 2019 11:25:06 +0100 From: "Trahe, Fiona" To: Ayuj Verma , "akhil.goyal@nxp.com" , "Kusztal, ArkadiuszX" , "De Lara Guarch, Pablo" CC: Shally Verma , Sunila Sahu , Kanaka Durga Kotamarthy , Arvind Desai , "dev@dpdk.org" , "Trahe, Fiona" Thread-Topic: [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP Thread-Index: AQHU7sgdjnB7ftUqtUGurkGwlNYoR6Yz7yswgAQffgCAAEL6UA== Date: Fri, 12 Apr 2019 10:25:05 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B4358973F4C4@IRSMSX101.ger.corp.intel.com> References: <1554809619-21164-1-git-send-email-ayverma@marvell.com> <1554809619-21164-2-git-send-email-ayverma@marvell.com>, <348A99DA5F5B7549AA880327E580B4358973CC8B@IRSMSX101.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjFkNzkyMTUtOGUxMy00YzM5LWFjOTgtMGQ4ODMxZjRjZDlhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibXg1RGV6cnlLY1dJUjRDbkJRak9LVnB4XC95S2x6cWVuUGNhdTV5a2lPOGVMQVpORXloNlp4NXZCWEJYYzR6SWoifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.181] MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190412102505.iKIb5iytsQQEV-w3-ZIaZhII9iKHy7XR9Z733myN4oo@z> Hi Ayuj From: Ayuj Verma [mailto:ayverma@marvell.com] Sent: Friday, April 12, 2019 8:08 AM To: Trahe, Fiona ; akhil.goyal@nxp.com; Kusztal, Ark= adiuszX ; De Lara Guarch, Pablo Cc: Shally Verma ; Sunila Sahu ; Ka= naka Durga Kotamarthy ; Arvind Desai ; 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 > Sent: 09 April 2019 20:47 To: Ayuj Verma; akhil.goyal@nxp.com; Kusztal, A= rkadiuszX; De Lara Guarch, Pablo Cc: Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@d= pdk.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; Trahe, Fiona >; Kusztal, ArkadiuszX > >; De L= ara Guarch, Pablo > > Cc: shallyv@marvell.com; ssahu@marvell.com; kkotamarthy@marvell.com; adesai@marvell.com; > dev@dpdk.org; Ayuj Verma > > Subject: [PATCH v2] app/test: replace TEST_SKIPPED with -ENOTSUP > > Return -ENOTSUP for unsupported tests > > Signed-off-by: Ayuj Verma > > Signed-off-by: Shally Verma > > --- > 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_asy= m.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 =3D 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 =3D 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 - AS= YNC should be ASYMM [Ayuj] Each test execute if device supports that algorithm else it is skip= ped. Thus, here it checks if modinv is not supported in capability then ski= p 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 MOD= INV" 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 =3D rte_cryptodev_asym_capability_get(dev_id, &cap_id= x); But actually is not checked. It should have a NULL check before moving on t= o 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.