Re: [dpdk-dev] [EXT] [PATCH v3 01/10] drivers/crypto: introduce IPsec-mb framework

["Power, Ciara" <ciara.power@intel.com>]

Hi Akhil,

>-----Original Message-----
>From: Akhil Goyal <gakhil@marvell.com>
>Sent: Wednesday 6 October 2021 14:51
>To: Power, Ciara <ciara.power@intel.com>; dev@dpdk.org
>Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Bronowski, PiotrX
><piotrx.bronowski@intel.com>; Thomas Monjalon <thomas@monjalon.net>; De
>Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Ray Kinsella
><mdr@ashroe.eu>
>Subject: RE: [EXT] [PATCH v3 01/10] drivers/crypto: introduce IPsec-mb
>framework
>
>> From: Fan Zhang <roy.fan.zhang@intel.com>
>>
>> This patch introduces the new framework to share common code between
>> the SW crypto PMDs that depend on the intel-ipsec-mb library.
>> This change helps to reduce future effort on the code maintenance and
>> feature updates.
>>
>> The PMDs that will be added to this framework in subsequent patches are:
>>   - AESNI MB
>>   - AESNI GCM
>>   - KASUMI
>>   - SNOW3G
>>   - ZUC
>>
>> The use of these PMDs will not change, they will still be supported
>> for x86, and will use the same EAL args as before.
>>
>> The minimum required version for the intel-ipsec-mb library is now v1.0.
>>
>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
>> Signed-off-by: Ciara Power <ciara.power@intel.com>
>>
>> ---
>> v3:
>>   - Updated intel-ipsec-mb macros.
>>   - Added use of auto init function for IMB_MGR.
>>   - Added detail to commit log.
>> v2:
>>   - Added qp NULL check in get stats function.
>>   - Added maintainers file entry.
>>   - Replaced strlcpy with rte_strlcpy.
>> ---
>>  MAINTAINERS                                   |   4 +
>>  drivers/crypto/ipsec_mb/meson.build           |  27 ++
>>  drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd.c    | 169 ++++++++++
>>  .../crypto/ipsec_mb/rte_ipsec_mb_pmd_ops.c    | 291 ++++++++++++++++++
>>  .../ipsec_mb/rte_ipsec_mb_pmd_private.h       | 275 +++++++++++++++++
>>  drivers/crypto/ipsec_mb/version.map           |   3 +
>>  drivers/crypto/meson.build                    |   1 +
>>  7 files changed, 770 insertions(+)
>>  create mode 100644 drivers/crypto/ipsec_mb/meson.build
>>  create mode 100644 drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd.c
>>  create mode 100644 drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_ops.c
>>  create mode 100644 drivers/crypto/ipsec_mb/rte_ipsec_mb_pmd_private.h
>>  create mode 100644 drivers/crypto/ipsec_mb/version.map
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS index 1e0d303394..f1aaf7d408
>> 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1065,6 +1065,10 @@ F: drivers/common/qat/
>>  F: doc/guides/cryptodevs/qat.rst
>>  F: doc/guides/cryptodevs/features/qat.ini
>>
>> +IPsec MB
>
>Not sure if the name ipsec_mb is appropriate for a crypto PMD which also
>support algos which are not specified for IPsec like ZUC/SNOW/KASUMI.
>Moreover, this is a crypto PMD and not IPsec PMD.
>
<snip>
>> +/** Get device statistics */
>> +void
>> +ipsec_mb_pmd_stats_get(struct rte_cryptodev *dev,
>> +		struct rte_cryptodev_stats *stats)
>
>I believe 1 instance of the ipsec_mb PMD will support only one kind of
>operation(aesni_mb/aesni_gcm/zuc/snow/kasumi).
>This cannot be changed during the lifetime of the process. Right?
>
<snip>

Yes - please treat the ipsec_mb_pmd.c/pmd_ops.c as the shared code base for these existing SW crypto PMDs based on intel-ipsec-mb library. There is no new PMD called ipsec_mb as such.

Maybe to make it less misleading we could rename the file names like so:
Ipsec_mb_pmd.c -> ipsec_mb_private.c
And similar change for ipsec_mb_pmd_ops.c and ipsec_mb_pmd_private.h
What do you think?

Thanks,
Ciara