From: Thomas Monjalon <thomas@monjalon.net>
To: Fan Zhang <roy.fan.zhang@intel.com>
Cc: dev@dpdk.org, akhil.goyal@nxp.com,
Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2] crypto/aesni_mb: use architure independent marcos
Date: Wed, 19 Dec 2018 14:08:30 +0100 [thread overview]
Message-ID: <2126399.7T9Zi8FxJ9@xps> (raw)
In-Reply-To: <20181211122917.18713-1-roy.fan.zhang@intel.com>
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.
next prev parent reply other threads:[~2018-12-19 13:08 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-23 14:04 [dpdk-dev] [PATCH] crypto/aesni_mb: use of archtecture independent macros Fan Zhang
2018-12-11 12:29 ` [dpdk-dev] [PATCH v2] crypto/aesni_mb: use architure independent marcos Fan Zhang
2018-12-18 10:26 ` Akhil Goyal
2018-12-19 13:08 ` Thomas Monjalon [this message]
2018-12-19 13:48 ` Zhang, Roy Fan
2018-12-19 20:16 ` [dpdk-dev] [PATCH v3 0/4] use architecure independent macros Fan Zhang
2018-12-19 20:16 ` [dpdk-dev] [PATCH v3 1/4] crypto/aesni_mb: rename files to compatible Fan Zhang
2018-12-19 20:16 ` [dpdk-dev] [PATCH v3 2/4] crypto/aesni_mb: use architecture independent macros Fan Zhang
2018-12-19 20:16 ` [dpdk-dev] [PATCH v3 3/4] doc: update library support version Fan Zhang
2018-12-19 20:16 ` [dpdk-dev] [PATCH v3 4/4] doc: update deprecation notice Fan Zhang
2018-12-20 11:56 ` [dpdk-dev] [PATCH v4 0/3] use architecure independent macros Fan Zhang
2018-12-20 11:56 ` [dpdk-dev] [PATCH v4 1/3] crypto/aesni_mb: rename files Fan Zhang
2018-12-20 11:56 ` [dpdk-dev] [PATCH v4 2/3] crypto/aesni_mb: use architecture independent macros Fan Zhang
2018-12-20 11:56 ` [dpdk-dev] [PATCH v4 3/3] doc: update documentation Fan Zhang
2019-01-09 22:09 ` [dpdk-dev] [PATCH v4 0/3] use architecure independent macros De Lara Guarch, Pablo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2126399.7T9Zi8FxJ9@xps \
--to=thomas@monjalon.net \
--cc=akhil.goyal@nxp.com \
--cc=dev@dpdk.org \
--cc=lukaszx.krakowiak@intel.com \
--cc=roy.fan.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).