From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Kumar, Ravi1" Subject: Re: [PATCH v5 17/19] crypto/ccp: support cpu based md5 and sha2 family authentication algo Date: Mon, 23 Apr 2018 06:41:16 +0000 Message-ID: References: <1520584520-130522-1-git-send-email-Ravi1.kumar@amd.com> <1521462233-13590-1-git-send-email-Ravi1.kumar@amd.com> <1521462233-13590-17-git-send-email-Ravi1.kumar@amd.com> <14946114.ASIPbkMQcp@xps> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "hemant.agrawal@nxp.com" To: Thomas Monjalon , "pablo.de.lara.guarch@intel.com" Return-path: Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on0042.outbound.protection.outlook.com [104.47.42.42]) by dpdk.org (Postfix) with ESMTP id F0BF910BD for ; Mon, 23 Apr 2018 08:41:18 +0200 (CEST) In-Reply-To: <14946114.ASIPbkMQcp@xps> Content-Language: en-US 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, > >I am doing some late comments because I have a quick look when trying to p= ull next-crypto in master branch. >Unfortunately, it doesn't met the basic quality criterias. > > >19/03/2018 13:23, Ravi Kumar: >> Signed-off-by: Ravi Kumar >> --- >> config/common_base | 1 + >> drivers/crypto/ccp/ccp_crypto.c | 282 ++++++++++++++++++++++++++++= ++++++- >> drivers/crypto/ccp/ccp_crypto.h | 5 +- >> drivers/crypto/ccp/ccp_pmd_ops.c | 23 +++ >> drivers/crypto/ccp/ccp_pmd_private.h | 10 ++ >> 5 files changed, 316 insertions(+), 5 deletions(-) >[...] >> +CONFIG_RTE_LIBRTE_PMD_CCP_CPU_AUTH=3Dn > >Why introducing a compile-time option? >Can it be a run-time option of the device? >We must not add compile-time device option if not well justified. > >Talking about justification, there is 0 explanation in the commit messages= . >But there are some in next-crypto tree. Where do they come from? > Hi Thomas, The detailed commit messages were missing from the earlier patches. Later a= fter the patch were applied to dpdk-next-crypto, Pablo requested detailed c= ommit messages for patches and just not to populate the mail-chain unnecess= arily, the messages were later squashed in the commits offline. Here is the explanation of the patch: By default, all the crypto operations (cipher + auth) are offloaded to CCP = engines. When user enables CONFIG_RTE_LIBRTE_PMD_CCP_CPU_AUTH=3Dy, the auth= operations are not offloaded to CCP and rather performed over CPU. We kept= this feature as a compile time option in order to let user decide whether = to run auth operations on CCP or CPU as some of the auth operations perform= s faster on CPU as compared to their performance on CCP. Regards, Ravi