Re: [dpdk-dev] [PATCH v2] crypto/aesni_mb: use architure independent marcos

["Zhang, Roy Fan" <roy.fan.zhang@intel.com>]

Hi Thomas,

Sorry the patch caused the problem.
I have tested the patch with intel-ipsec-mb 0.50/0.52 library with make and did not find problem.

Once switching between versions of ipsec-mb, a necessary "make uninstall" has to be done to clear old in /usr.

However I didn't test meson in 0.50, sorry about that.

The purpose of the patch is to fit the changes made to the latest intel-ipsec-mb code with newly introduced API.
Using the new APIs (macros) newly introduced have the benefit of way easier maintenance effort for different architectures and massively reduce the code size.

However using the new APIs only will cause the inconsistence compatibility to the user who uses older APIs. 

Ferruh and I had the talk and come with the solution to prepare both old and new code for 19.02, and make the compiler selects which files to be compiled based on the detected ipsec-mb version. We also planned to make a deprecation notice for 19.05 to drop the support of older version of ipsec-mb library, along with replacing the *_new* files with the existing one.

Regards,
Fan

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, December 19, 2018 1:09 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: dev@dpdk.org; akhil.goyal@nxp.com; Krakowiak, LukaszX
> <lukaszx.krakowiak@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] crypto/aesni_mb: use architure
> independent marcos
> 
> 11/12/2018 13:29, Fan Zhang:
> > From: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> >
> > This patch updates the aesni_mb to use IMB_* arch independent macros
> > to reduce the code size and future maintaining effort.
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> > ---
> > v2:
> > - making the PMD compatible with both new intel-ipsec-mb version 0.52
> > and older
> > - fixed a bug
> >
> >  drivers/crypto/aesni_mb/Makefile                   |   24 +-
> >  drivers/crypto/aesni_mb/meson.build                |   14 +-
> >  drivers/crypto/aesni_mb/rte_aesni_mb_pmd_next.c    | 1237
> ++++++++++++++++++++
> >  .../crypto/aesni_mb/rte_aesni_mb_pmd_ops_next.c    |  681
> +++++++++++
> >  drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h |   52 +-
> >  5 files changed, 1998 insertions(+), 10 deletions(-)  create mode
> > 100755 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_next.c
> >  create mode 100755
> > drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops_next.c
> >
> > diff --git a/drivers/crypto/aesni_mb/Makefile
> > b/drivers/crypto/aesni_mb/Makefile
> > index 806a95eb8..24630a6ca 100644
> > --- a/drivers/crypto/aesni_mb/Makefile
> > +++ b/drivers/crypto/aesni_mb/Makefile
> > @@ -22,8 +22,26 @@ LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool
> > -lrte_ring  LDLIBS += -lrte_cryptodev  LDLIBS += -lrte_bus_vdev
> >
> > -# library source files
> > -SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += rte_aesni_mb_pmd.c
> > -SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) +=
> rte_aesni_mb_pmd_ops.c
> > +IMB_HDR = /usr/include/intel-ipsec-mb.h
> > +
> > +# Detect library version
> > +IMB_VERSION = $(shell grep -e "IMB_VERSION_STR" $(IMB_HDR) | cut
> > +-d'"' -f2) IMB_VERSION_NUM = $(shell grep -e "IMB_VERSION_NUM"
> > +$(IMB_HDR) | cut -d' ' -f3)
> > +
> > +ifeq ($(IMB_VERSION),)
> > +	# files for older version of IMB
> > +	SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) +=
> rte_aesni_mb_pmd.c
> > +	SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) +=
> rte_aesni_mb_pmd_ops.c
> > +else
> > +	ifeq ($(shell expr $(IMB_VERSION_NUM) \>= 0x3400), 1)
> > +		# files for a new version of IMB
> > +		SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) +=
> rte_aesni_mb_pmd_next.c
> > +		SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) +=
> rte_aesni_mb_pmd_ops_next.c
> > +	else
> > +		# files for older version of IMB
> > +		SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) +=
> rte_aesni_mb_pmd.c
> > +		SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) +=
> rte_aesni_mb_pmd_ops.c
> > +	endif
> > +endif
> >
> >  include $(RTE_SDK)/mk/rte.lib.mk
> > diff --git a/drivers/crypto/aesni_mb/meson.build
> > b/drivers/crypto/aesni_mb/meson.build
> > index aae0995e5..490f68eaf 100644
> > --- a/drivers/crypto/aesni_mb/meson.build
> > +++ b/drivers/crypto/aesni_mb/meson.build
> > @@ -1,6 +1,6 @@
> >  # SPDX-License-Identifier: BSD-3-Clause  # Copyright(c) 2018 Intel
> > Corporation
> > -
> > +IPSec_MB_ver_0_52 = '0.52.0'
> >  lib = cc.find_library('IPSec_MB', required: false)  if not
> > lib.found()
> >  	build = false
> > @@ -8,5 +8,15 @@ else
> >  	ext_deps += lib
> >  endif
> >
> > -sources = files('rte_aesni_mb_pmd.c', 'rte_aesni_mb_pmd_ops.c')
> > +imb_version = cc.get_define('IMB_VERSION_STR',
> > +        prefix : '#include<intel-ipsec-mb.h>')
> > +
> > +if imb_version.version_compare('>=' + IPSec_MB_ver_0_52)
> > +	message('Build for a new version of library IPSec_MB[' + imb_version
> + ']')
> > +	sources = files('rte_aesni_mb_pmd_next.c',
> > +'rte_aesni_mb_pmd_ops_next.c') else
> > +	message('Build for older version of library IPSec_MB[' + imb_version
> + ']')
> > +	sources = files('rte_aesni_mb_pmd.c', 'rte_aesni_mb_pmd_ops.c')
> > +endif
> > +
> >  deps += ['bus_vdev']
> 
> I don't know what you are trying to do, but I know it is not explained.
> Adding files "*_next.c" looks to be a bad idea.
> And worst: it does not compile with meson:
> 	drivers/crypto/aesni_mb/meson.build:11:0: ERROR:  Could not get
> define 'IMB_VERSION_STR'
> 
> This patch is a total mess which must be explained, tested and split in several
> patches.
> I drop it from the merge to master and update all related AES patches to
> "Changes Requested" in patchwork.
> 
>