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:tho...@monjalon.net]
> Sent: Wednesday, December 19, 2018 1:09 PM
> To: Zhang, Roy Fan <roy.fan.zh...@intel.com>
> Cc: dev@dpdk.org; akhil.go...@nxp.com; Krakowiak, LukaszX
> <lukaszx.krakow...@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.krakow...@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.zh...@intel.com>
> > Signed-off-by: Lukasz Krakowiak <lukaszx.krakow...@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.
> 
> 


Reply via email to