From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 95E5B961B for ; Tue, 21 Jun 2016 16:05:06 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 21 Jun 2016 07:05:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,504,1459839600"; d="scan'208";a="1002316326" Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28]) by orsmga002.jf.intel.com with ESMTP; 21 Jun 2016 07:05:03 -0700 Received: from irsmsx111.ger.corp.intel.com (10.108.20.4) by irsmsx105.ger.corp.intel.com (163.33.3.28) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 21 Jun 2016 15:05:02 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.51]) by irsmsx111.ger.corp.intel.com ([169.254.2.182]) with mapi id 14.03.0248.002; Tue, 21 Jun 2016 15:05:02 +0100 From: "Griffin, John" To: Thomas Monjalon CC: "dev@dpdk.org" , "Jain, Deepak K" , "De Lara Guarch, Pablo" Thread-Topic: [dpdk-dev] [PATCH] qat: addition of optimized content descriptor for AES128-SHA1-HMAC Thread-Index: AQHRy7xOQ8wyDo4hh0qHLtZ23vPiqJ/z31AAgAASRWA= Date: Tue, 21 Jun 2016 14:05:01 +0000 Message-ID: References: <1466513759-55842-1-git-send-email-john.griffin@intel.com> <3195917.C7veDrCJq7@xps13> In-Reply-To: <3195917.C7veDrCJq7@xps13> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGI0MWI3OTgtZGQ4Zi00ZjUwLTlmNjUtODc4NDQxNzhmODllIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ik9pbkNQN2FHREVcL3RLQkVsMXU3UUxvWTFQNWI5akRrc280N0U4N1RkVFU4PSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] qat: addition of optimized content descriptor for AES128-SHA1-HMAC X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Jun 2016 14:05:07 -0000 Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Tuesday, June 21, 2016 2:50 PM > To: Griffin, John > Cc: dev@dpdk.org; Jain, Deepak K ; De Lara > Guarch, Pablo > Subject: Re: [dpdk-dev] [PATCH] qat: addition of optimized content > descriptor for AES128-SHA1-HMAC >=20 > Hi, >=20 > I'm not used to review crypto patches but I think this patch can be impro= ved. >=20 > 2016-06-21 13:55, John Griffin: > > Adding an optimized content descriptor for AES128-SHA1-HMAC to > improve > > thoughput performance. >=20 > Maybe you can explain how it improves the performance. [JFG] Can add some text- it's though a reduction in the amount of the sessi= on information which DMA'ed to the device - helps to reduce PCIe b/w. I'll add some text. >=20 > > +/* > > + * Function which will return if it's possible to use the > > + * optimised content descriptor. > > + */ > > +int qat_crypto_sym_use_optimized_alg(struct qat_session *session) > [...] > > +/* > > + * Function to create an optimised content descriptor for AES128 SHA1. > > + */ > > +int qat_crypto_create_optimzed_session(struct qat_session *session, >=20 > These function are very specific with a generic name. > Maybe that CBC AES128 SHA1 or something like that must be part of the > function name. > Otherwise you will come with yet another crypto refactoring patch in few > weeks. [JFG]=20 [JFG] Ok - I might not rename, but I'll make the implementation more generi= c to all it cope better with=20 the potential addition of other algorithms. >=20 > > case ICP_QAT_FW_LA_CMD_CIPHER: > > - session =3D qat_crypto_sym_configure_session_cipher(dev, xform, > session); > > + session =3D qat_crypto_sym_configure_session_cipher(dev, > xform, > > + session); > > break; > > case ICP_QAT_FW_LA_CMD_AUTH: > > - session =3D qat_crypto_sym_configure_session_auth(dev, xform, > session); > > + session =3D qat_crypto_sym_configure_session_auth(dev, > xform, > > + session); > > break; > > case ICP_QAT_FW_LA_CMD_CIPHER_HASH: > > - session =3D qat_crypto_sym_configure_session_cipher(dev, xform, > session); > > - session =3D qat_crypto_sym_configure_session_auth(dev, xform, > session); > > - break; > > + session =3D qat_crypto_sym_configure_session_cipher(dev, > xform, > > + session); > > + session =3D qat_crypto_sym_configure_session_auth(dev, > xform, > > + session); > > + if (qat_crypto_sym_use_optimized_alg(session)) > > + qat_crypto_sym_configure_optimized_session(dev, > xform, > > + session); > > + break; > > case ICP_QAT_FW_LA_CMD_HASH_CIPHER: > > - session =3D qat_crypto_sym_configure_session_auth(dev, xform, > session); > > - session =3D qat_crypto_sym_configure_session_cipher(dev, xform, > session); > > - break; > > + session =3D qat_crypto_sym_configure_session_auth(dev, > xform, > > + session); > > + session =3D qat_crypto_sym_configure_session_cipher(dev, > xform, > > + session); > > + if (qat_crypto_sym_use_optimized_alg(session)) > > + qat_crypto_sym_configure_optimized_session(dev, > xform, > > + session); > > + break; >=20 > There is a lot of indent fixing mixed with the addition here. > 2 patches would make things easier to understand. [JFG]=20 [JFG] Ok let me remove from this patch. >=20 > > @@ -551,11 +591,11 @@ qat_crypto_sym_configure_session_auth(struct > rte_cryptodev *dev, > > auth_xform->algo); > > goto error_out; > > } > > - cipher_xform =3D qat_get_cipher_xform(xform); > > > > if ((session->qat_hash_alg =3D=3D > ICP_QAT_HW_AUTH_ALGO_GALOIS_128) || > > (session->qat_hash_alg =3D=3D > > ICP_QAT_HW_AUTH_ALGO_GALOIS_64)) { > > + cipher_xform =3D qat_get_cipher_xform(xform); > > if > (qat_alg_aead_session_create_content_desc_auth(session, >=20 > How this move is related to the patch? [JFG] It's not tbh - I'll remove from this patch.