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

[Akhil Goyal <gakhil@marvell.com>]

> Hi Akhil,
> 
> >> 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?
> 
Ok that would be better.