Hi Anoob,

-----Original Message-----
From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
Sent: Thursday, June 14, 2018 3:57 AM
To: Anoob Joseph <redacted>
Cc: Akhil Goyal <redacted>; De Lara Guarch, Pablo
[off-list ref]; Thomas Monjalon
[off-list ref]; Nithin Dabilpuram [off-list ref];
Ankur Dwivedi [off-list ref]; Murthy NSSR
[off-list ref]; Narayana Prasad
[off-list ref]; Ragothaman Jayaraman
[off-list ref]; Srisivasubramanian Srinivasan
[off-list ref]; dev@dpdk.org
Subject: Re: [PATCH 01/16] config: add Cavium CPT PMD skeleton

-----Original Message-----

quoted

Date: Fri,  8 Jun 2018 22:15:10 +0530
From: Anoob Joseph <redacted>
To: Akhil Goyal <redacted>, Pablo de Lara
[off-list ref], Thomas Monjalon
[off-list ref]
Cc: Nithin Dabilpuram <redacted>, Ankur Dwivedi
[off-list ref], Jerin Jacob
[off-list ref],  Murthy NSSR
[off-list ref], Narayana Prasad
[off-list ref], Ragothaman Jayaraman
[off-list ref], Srisivasubramanian Srinivasan
[off-list ref], dev@dpdk.org
Subject: [PATCH 01/16] config: add Cavium CPT PMD skeleton
X-Mailer: git-send-email 2.7.4

From: Nithin Dabilpuram <redacted>

Add makefile and config file options.
Also add version map file and maintainers file to claim
responsibility.

Signed-off-by: Ankur Dwivedi <redacted>
Signed-off-by: Murthy NSSR <redacted>
Signed-off-by: Nithin Dabilpuram <redacted>
Signed-off-by: Ragothaman Jayaraman

[off-list ref]

quoted

Signed-off-by: Srisivasubramanian Srinivasan
[off-list ref]
---
 MAINTAINERS                                |  7 ++++++
 config/common_base                         |  8 ++++++
 drivers/crypto/Makefile                    |  1 +
 drivers/crypto/cpt/Makefile                | 40

++++++++++++++++++++++++++++++

quoted

 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.dwivedi@cavium.com>
+M: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>
+M: Murthy NSSR <Nidadavolu.Murthy@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.

quoted

+
 Chelsio cxgbe
 M: Rahul Lakkireddy [off-list ref]
 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.

quoted

+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

quoted

+
+#
 # 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.

quoted

+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.

quoted

+
+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.

quoted

+	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

quoted

 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).

quoted

 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.

quoted

--
1.9.3