Re: [PATCH v5 17/19] crypto/ccp: support cpu based md5 and sha2 family authentication algo

["Kumar, Ravi1" <Ravi1.Kumar@amd.com>]

>Hi,
>
>I am doing some late comments because I have a quick look when trying to pull 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 <Ravi1.kumar@amd.com>
>> ---
>>  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=n
>
>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 after the patch were applied to dpdk-next-crypto, Pablo requested detailed commit messages for patches and just not to populate the mail-chain unnecessarily, 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=y, 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 performs faster on CPU as compared to their performance on CCP.

Regards,
Ravi