Hi Anoob,
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com]
> Sent: Thursday, June 14, 2018 3:57 AM
> To: Anoob Joseph <anoob.jos...@caviumnetworks.com>
> Cc: Akhil Goyal <akhil.go...@nxp.com>; De Lara Guarch, Pablo
> <pablo.de.lara.gua...@intel.com>; Thomas Monjalon
> <tho...@monjalon.net>; Nithin Dabilpuram <nithin.dabilpu...@cavium.com>;
> Ankur Dwivedi <ankur.dwiv...@cavium.com>; Murthy NSSR
> <nidadavolu.mur...@cavium.com>; Narayana Prasad
> <narayanaprasad.athr...@caviumnetworks.com>; Ragothaman Jayaraman
> <ragothaman.jayara...@cavium.com>; Srisivasubramanian Srinivasan
> <srisivasubramanian.sriniva...@cavium.com>; dev@dpdk.org
> Subject: Re: [PATCH 01/16] config: add Cavium CPT PMD skeleton
>
> -----Original Message-----
> > Date: Fri, 8 Jun 2018 22:15:10 +0530
> > From: Anoob Joseph <anoob.jos...@caviumnetworks.com>
> > To: Akhil Goyal <akhil.go...@nxp.com>, Pablo de Lara
> > <pablo.de.lara.gua...@intel.com>, Thomas Monjalon
> > <tho...@monjalon.net>
> > Cc: Nithin Dabilpuram <nithin.dabilpu...@cavium.com>, Ankur Dwivedi
> > <ankur.dwiv...@cavium.com>, Jerin Jacob
> > <jerin.ja...@caviumnetworks.com>, Murthy NSSR
> > <nidadavolu.mur...@cavium.com>, Narayana Prasad
> > <narayanaprasad.athr...@caviumnetworks.com>, Ragothaman Jayaraman
> > <ragothaman.jayara...@cavium.com>, Srisivasubramanian Srinivasan
> > <srisivasubramanian.sriniva...@cavium.com>, dev@dpdk.org
> > Subject: [PATCH 01/16] config: add Cavium CPT PMD skeleton
> > X-Mailer: git-send-email 2.7.4
> >
> > From: Nithin Dabilpuram <nithin.dabilpu...@cavium.com>
> >
> > Add makefile and config file options.
> > Also add version map file and maintainers file to claim
> > responsibility.
> >
> > Signed-off-by: Ankur Dwivedi <ankur.dwiv...@cavium.com>
> > Signed-off-by: Murthy NSSR <nidadavolu.mur...@cavium.com>
> > Signed-off-by: Nithin Dabilpuram <nithin.dabilpu...@cavium.com>
> > Signed-off-by: Ragothaman Jayaraman
> <ragothaman.jayara...@cavium.com>
> > Signed-off-by: Srisivasubramanian Srinivasan
> > <srisivasubramanian.sriniva...@cavium.com>
> > ---
> > MAINTAINERS | 7 ++++++
> > config/common_base | 8 ++++++
> > drivers/crypto/Makefile | 1 +
> > drivers/crypto/cpt/Makefile | 40
> ++++++++++++++++++++++++++++++
> > drivers/crypto/cpt/rte_pmd_cpt_version.map | 4 +++
> > mk/rte.app.mk | 2 +-
> > 6 files changed, 61 insertions(+), 1 deletion(-) create mode 100644
> > drivers/crypto/cpt/Makefile create mode 100644
> > drivers/crypto/cpt/rte_pmd_cpt_version.map
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index 4667fa7..1b4a8eb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -504,6 +504,13 @@ F: drivers/net/octeontx/
> > F: doc/guides/nics/octeontx.rst
> > F: doc/guides/nics/features/octeontx.ini
> >
> > +Cavium CPT
> > +M: Ankur Dwivedi <ankur.dwiv...@cavium.com>
> > +M: Nithin Dabilpuram <nithin.dabilpu...@cavium.com>
> > +M: Murthy NSSR <nidadavolu.mur...@cavium.com>
> > +F: drivers/crypto/cpt/
> > +F: doc/guides/cryptodevs/cpt.rst
Add the documentation file line in the patch where you actually add it.
>
> Move this under crypto drivers section if it is not already the case.
>
> > +
> > Chelsio cxgbe
> > M: Rahul Lakkireddy <rahul.lakkire...@chelsio.com>
> > F: drivers/net/cxgbe/
> > diff --git a/config/common_base b/config/common_base index
> > 6b0d1cb..85e03a8 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -631,6 +631,14 @@ CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV=n
> > CONFIG_RTE_LIBRTE_PMD_DPAA2_EVENTDEV=n
> >
> > #
> > +# Compile PMD for Cavium CPT Crypto device #
> > +CONFIG_RTE_LIBRTE_PMD_CPT=n
>
> Please enable this option by default, If there are arm64 specific usage then
> please stub it out so that it can compile on all architectures. It will help
> to verify
> the API changes across the architecture by author.(i.e author should not
> depend
> arm64 box to verify the compilation changes) and enable distribution OS
> support
> like Ubuntu as they building with default arm64 config.
Agreed. Actually, I thought this was disabled because of the dependency with
libcrypto
(based on the Makefile below), but actually it is not needed.
Also, move this where the other crypto drivers are.
>
>
> > +CONFIG_RTE_LIBRTE_PMD_CPT_DEBUG_INIT=n
> > +CONFIG_RTE_LIBRTE_PMD_CPT_DEBUG_RX=n
> > +CONFIG_RTE_LIBRTE_PMD_CPT_DEBUG_TX=n
>
> This config option is not required when we are using dynamic debugging
>
> > +
> > +#
> > # Compile raw device support
> > # EXPERIMENTAL: API may change without prior notice # diff --git
> > a/drivers/crypto/Makefile b/drivers/crypto/Makefile index
> > 1d0c88e..a0515f3 100644
> > --- a/drivers/crypto/Makefile
> > +++ b/drivers/crypto/Makefile
> > @@ -22,5 +22,6 @@ ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
> > DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_SEC) += dpaa_sec endif
> > DIRS-$(CONFIG_RTE_LIBRTE_PMD_VIRTIO_CRYPTO) += virtio
> > +DIRS-$(CONFIG_RTE_LIBRTE_PMD_CPT) += cpt
> >
> > include $(RTE_SDK)/mk/rte.subdir.mk
> > diff --git a/drivers/crypto/cpt/Makefile b/drivers/crypto/cpt/Makefile
> > new file mode 100644 index 0000000..b2d950d
> > --- /dev/null
> > +++ b/drivers/crypto/cpt/Makefile
> > @@ -0,0 +1,40 @@
> > +
> > +include $(RTE_SDK)/mk/rte.vars.mk
> > +
> > +# library name
> > +LIB = librte_pmd_cptvf.a
> > +
> > +# library version
> > +LIBABIVER := 1
> > +
> > +# build flags
> > +CFLAGS += $(WERROR_FLAGS)
> > +
> > +# external library include paths
> > +CFLAGS += -I$(LIBCRYPTO_THUNDERX_PATH)/include
> > +LDLIBS += -L$(LIBCRYPTO_THUNDERX_PATH) -lcrypto
>
> What is the external library dependency here? Does look like it is documented
> in
> doc patch.
>
> If there is something, I think, better to change LIBCRYPTO_THUNDERX_PATH to
> LIBCRYPTO_OCTEONTX_PATH
Is this needed? I don't see any dependency with libcrypto and I don't see this
variable
documented in the driver guide.
>
> > +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring LDLIBS +=
> > +-lrte_cryptodev LDLIBS += -lrte_pci -lrte_bus_pci
> > +
> > +VPATH += $(RTE_SDK)/drivers/crypto/cpt/base
> > +
> > +CFLAGS += -O3
> > +#CFLAGS += -DAUTH_SOFT_COMPUTE_IPAD_OPAD #CFLAGS += -
> DCPT_DEBUG
>
> Remove commented out stuff.
>
> > +
> > +SRCS-$(CONFIG_RTE_LIBRTE_PMD_CPT) +=
> > +
> > +# export include files
> > +SYMLINK-y-include +=
> > +
> > +# versioning export map
> > +EXPORT_MAP := rte_pmd_cpt_version.map
> > +
> > +# library dependencies
> > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_CPT) += lib/librte_eal
> > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_CPT) += lib/librte_cryptodev
> > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_CPT) += lib/librte_mempool
> > +lib/librte_mbuf
> > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_CPT) += lib/librte_malloc
> > +
> > +include $(RTE_SDK)/mk/rte.lib.mk
> > diff --git a/drivers/crypto/cpt/rte_pmd_cpt_version.map
> > b/drivers/crypto/cpt/rte_pmd_cpt_version.map
> > new file mode 100644
> > index 0000000..9b9ab1a
> > --- /dev/null
> > +++ b/drivers/crypto/cpt/rte_pmd_cpt_version.map
> > @@ -0,0 +1,4 @@
> > +DPDK_18.05 {
> > +
DPDK_18.08.
> > + local: *;
> > +};
> > diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 1e32c83..158066d
> > 100644
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -209,7 +209,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_SEC) +=
> -lrte_pmd_dpaa_sec
> > endif # CONFIG_RTE_LIBRTE_DPAA_BUS
> > _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_VIRTIO_CRYPTO) +=
> > -lrte_pmd_virtio_crypto endif # CONFIG_RTE_LIBRTE_CRYPTODEV
> > -
> > +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_CPT) += -lrte_pmd_cptvf
This should be inside ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y).
> > ifeq ($(CONFIG_RTE_LIBRTE_COMPRESSDEV),y)
> > _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += -lrte_pmd_isal_comp
> > _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += -lisal
>
>
> You can add meson build as well here on the same patch.
>
>
> > --
> > 1.9.3
> >
