From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 54F6CA0471 for ; Thu, 18 Jul 2019 14:45:04 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B55D51041; Thu, 18 Jul 2019 14:45:02 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id E915EDE3 for ; Thu, 18 Jul 2019 14:45:00 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jul 2019 05:44:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,278,1559545200"; d="scan'208";a="367342330" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by fmsmga006.fm.intel.com with ESMTP; 18 Jul 2019 05:44:58 -0700 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by IRSMSX102.ger.corp.intel.com (163.33.3.155) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 18 Jul 2019 13:44:57 +0100 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.88]) by irsmsx155.ger.corp.intel.com ([169.254.14.201]) with mapi id 14.03.0439.000; Thu, 18 Jul 2019 13:44:57 +0100 From: "Trahe, Fiona" To: Shally Verma , "Kusztal, ArkadiuszX" , "dev@dpdk.org" CC: "akhil.goyal@nxp.com" , "Trahe, Fiona" Thread-Topic: [EXT] [PATCH v3 04/11] test: add cipher field to RSA test Thread-Index: AQHVPAfDvOcGM5ZdQkGAUhcRpnP0XKbOO8oAgAA7OtCAABgJYIAAJbcAgAGJWRA= Date: Thu, 18 Jul 2019 12:44:57 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B435897C7371@IRSMSX101.ger.corp.intel.com> References: <20190716185304.12592-1-arkadiuszx.kusztal@intel.com> <20190716185304.12592-5-arkadiuszx.kusztal@intel.com> <06EE24DD0B19E248B53F6DC8657831551B2808CE@hasmsx109.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: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDk4ZDZiMWItZGY1Yy00NThlLTg3ZTMtOTI3YjE4Y2YzNzI2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoid2xEQ3lwMVo1ejBKamFtZ0ZVTjVhbEh0NXJ0SFhMU1ZoZllXdjFxSzI2NTVCWE5pOFBJaGlmRjJta3NyQkJqbSJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [EXT] [PATCH v3 04/11] test: add cipher field to RSA test 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" Hi Shally, Arek, > > > > > > > > > -----Original Message----- > > > > > From: Arek Kusztal > > > > > Sent: Wednesday, July 17, 2019 12:23 AM > > > > > To: dev@dpdk.org > > > > > Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com; Shally Verma > > > > > ; Arek Kusztal > > > > > Subject: [EXT] [PATCH v3 04/11] test: add cipher field to RSA tes= t > > > > > > > > > > External Email > > > > > > > > > > -----------------------------------------------------------------= - > > > > > -- > > > > > -- This patch adds cipher field to RSA test cases > > > > > > > > > > Signed-off-by: Arek Kusztal > > > > > --- > > > > > app/test/test_cryptodev_asym.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/app/test/test_cryptodev_asym.c > > > > > b/app/test/test_cryptodev_asym.c index 4dee164..8391545 100644 > > > > > --- a/app/test/test_cryptodev_asym.c > > > > > +++ b/app/test/test_cryptodev_asym.c > > > > > @@ -164,6 +164,7 @@ queue_ops_rsa_enc_dec(struct > > > > > rte_cryptodev_asym_session *sess) > > > > > uint8_t dev_id =3D ts_params->valid_devs[0]; > > > > > struct rte_crypto_op *op, *result_op; > > > > > struct rte_crypto_asym_op *asym_op; > > > > > + uint8_t cipher_buf[TEST_DATA_SIZE] =3D {0}; > > > > > int ret, status =3D TEST_SUCCESS; > > > > > > > > > > /* Set up crypto op data structure */ @@ -180,6 +181,8 @@ > > > > > queue_ops_rsa_enc_dec(struct rte_cryptodev_asym_session *sess) > > > > > asym_op->rsa.op_type =3D RTE_CRYPTO_ASYM_OP_ENCRYPT; > > > > > > > > > > asym_op->rsa.message.data =3D rsaplaintext.data; > > > > > + asym_op->rsa.cipher.data =3D cipher_buf; > > > > > + asym_op->rsa.cipher.length =3D 0; > > > > [Shally] I think this should be initialized to length of buffer > > > > available i.e. RSA Key size? PMD can override it with length of > > > > actual data written at output, which has to be less than , equal to > > RSA_key size. > > > [AK] - its because API comments are ambiguous in this case and we hav= e > > > only one field describing array length. > > > I would suggest to rephrase cipher field API comments from "length in > > bytes > > > * of this field needs to be greater or equal to the length of > > > * corresponding RSA key in bytes" > > > To "underlying array should have allocated enough memory to hold > > > cipher output (bigger or equal to RSA key size". Then length could an= d > > > I think should be zero or unspecified at this point. > > > What do you think? > > > > [AK2] Something like that: > > * When RTE_CRYPTO_ASYM_OP_ENCRYPT op_type used underlying > > array > > * should have been allocated with enough memory to hold cipher > > * output (bigger or equal to RSA key size). > > The same for message field. > [Shally] This description is okay. But still I would assume app to set le= ngth field of cipher buffer to actual > allocated than 0. But I look forward to more feedback on this from others [Fiona] I think the important thing is to be clear on when it's an input fi= eld and when an output and what the appl or PMD does in each case. So my understanding is in ENCRYPT case it's an output field and DECRYPT it'= s an input. SO how about - combining this with the changes already suggested to avoid r= epetition in patch 2: Comment under rte_crypto_rsa_op_param.message: Pointer to input data * - to be encrypted for RSA public encrypt. * - to be signed for RSA sign generation. * - to be authenticated for RSA sign verification. Pointer to output data * - for RSA private decrypt. In this case the underlying array should have been all= ocated with enough memory to hold plaintext output (i.e. must be a= t least RSA key size). The message.length field should be 0 and will be overw= ritten by the PMD with the decrypted length. All data is in Octet-string network byte order format. Note 1: If API allows a length on decrypt, then what would the PMD use it f= or? Would it have to handle the case where it's less than key-size? In whic= h case the appl is breaking the API and ignoring the previous comment. Or m= ore than key-size - what does the PMD care - it just needs key-size. IF the= re was a case where PMD could produce more than keysize and would need to k= now if the buffer is big enough then we should allow this and say it's both= an input (buffer-len) and an output (decrypted-message-len). But I don't t= hink there's such a case.=20 Note 2 : it's good practice for apps to zero all fields in all API structs,= except those explicitly set, to allow for future API extensions without AB= I breakage. Comment under rte_crypto_rsa_op_param.cipher: Pointer to input data * - to be decrypted for RSA private decrypt. =20 Pointer to output data * - for RSA public encrypt. In this case the underlying array should have been all= ocated with enough memory to hold ciphertext output (i.e. must be = at least RSA key size). The message.length field should be 0 and will be overw= ritten by the PMD with the encrypted length. All data is in Octet-string network byte order format. @Shally - does above make sense? If so we can update patches 2, 3 and 4 based on above.