[dpdk-dev] [PATCH v2 0/5] compress: add ZLIB compression PMD

[dpdk-dev] [PATCH v2 0/5] compress: add ZLIB compression PMD

[Shally Verma]

This patch series add software zlib based compression PMD
in DPDK compress drivers.
Application must need to install zlib prior to compile and
run this PMD to avail compression/decompression services.
Currently driver only tested for deflate, stateless
compression and decompression with direct memory buffers.

Changes in v2:
- removed unused variables
- corrected capability to reflect current support
- add lookup for internally maintained mempool during device_configure
- optimized core compression/decompression logic in enq/deq APIs
- updated documentation with correct feature support

v1 includes:
- build changes to build zlib PMD
- zlib PMD implementation
- zlib PMD documentation
- meson build support

This patchset is dependent upon compressdev API.

Sunila Sahu (5):
  compress/zlib: add ZLIB PMD support
  compress/zlib: add device setup PMD ops
  compress/zlib: add xform and stream create support
  compress/zlib: add enq deq apis
  doc: add ZLIB PMD documentation

 MAINTAINERS                                    |   5 +
 config/common_base                             |   5 +
 doc/guides/compressdevs/features/zlib.ini      |  22 ++
 doc/guides/compressdevs/zlib.rst               |  68 ++++
 drivers/compress/Makefile                      |   1 +
 drivers/compress/meson.build                   |   2 +-
 drivers/compress/zlib/Makefile                 |  29 ++
 drivers/compress/zlib/meson.build              |  14 +
 drivers/compress/zlib/rte_pmd_zlib_version.map |   3 +
 drivers/compress/zlib/zlib_pmd.c               | 412 +++++++++++++++++++++++++
 drivers/compress/zlib/zlib_pmd_ops.c           | 311 +++++++++++++++++++
 drivers/compress/zlib/zlib_pmd_private.h       |  71 +++++
 mk/rte.app.mk                                  |   2 +
 13 files changed, 944 insertions(+), 1 deletion(-)
 create mode 100644 doc/guides/compressdevs/features/zlib.ini
 create mode 100644 doc/guides/compressdevs/zlib.rst
 create mode 100644 drivers/compress/zlib/Makefile
 create mode 100644 drivers/compress/zlib/meson.build
 create mode 100644 drivers/compress/zlib/rte_pmd_zlib_version.map
 create mode 100644 drivers/compress/zlib/zlib_pmd.c
 create mode 100644 drivers/compress/zlib/zlib_pmd_ops.c
 create mode 100644 drivers/compress/zlib/zlib_pmd_private.h

-- 
2.9.5

[dpdk-dev] [PATCH v2 1/5] compress/zlib: add ZLIB PMD support

[Shally Verma]

From: Ashish Gupta <ashish.gupta@caviumnetworks.com>

Add sw zlib pmd support in compressdev driver.
Add device probe and remove support.
Add ZLIB build file support.

Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
---
 MAINTAINERS                                    |  3 +
 config/common_base                             |  5 ++
 drivers/compress/Makefile                      |  1 +
 drivers/compress/meson.build                   |  2 +-
 drivers/compress/zlib/Makefile                 | 28 +++++++++
 drivers/compress/zlib/meson.build              | 14 +++++
 drivers/compress/zlib/rte_pmd_zlib_version.map |  3 +
 drivers/compress/zlib/zlib_pmd.c               | 81 ++++++++++++++++++++++++++
 drivers/compress/zlib/zlib_pmd_private.h       | 33 +++++++++++
 mk/rte.app.mk                                  |  2 +
 10 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index dabb12d..448bbe1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -851,6 +851,9 @@ F: drivers/compress/isal/
 F: doc/guides/compressdevs/isal.rst
 F: doc/guides/compressdevs/features/isal.ini
 
+ZLIB
+M: Sunila Sahu <sunila.sahu@caviumnetworks.com>
+F: drivers/compress/zlib/
 
 Eventdev Drivers
 ----------------
diff --git a/config/common_base b/config/common_base
index 721e59b..dabc269 100644
--- a/config/common_base
+++ b/config/common_base
@@ -585,6 +585,11 @@ CONFIG_RTE_COMPRESSDEV_TEST=n
 CONFIG_RTE_LIBRTE_PMD_ISAL=n
 
 #
+# Compile PMD for ZLIB compression device
+#
+CONFIG_RTE_LIBRTE_PMD_ZLIB=n
+
+#
 # Compile generic event device library
 #
 CONFIG_RTE_LIBRTE_EVENTDEV=y
diff --git a/drivers/compress/Makefile b/drivers/compress/Makefile
index 592497f..1f159a5 100644
--- a/drivers/compress/Makefile
+++ b/drivers/compress/Makefile
@@ -4,5 +4,6 @@
 include $(RTE_SDK)/mk/rte.vars.mk
 
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += isal
+DIRS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += zlib
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/compress/meson.build b/drivers/compress/meson.build
index fb136e1..e4d5e5c 100644
--- a/drivers/compress/meson.build
+++ b/drivers/compress/meson.build
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Intel Corporation
 
-drivers = ['isal']
+drivers = ['isal','zlib']
 
 std_deps = ['compressdev'] # compressdev pulls in all other needed deps
 config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'
diff --git a/drivers/compress/zlib/Makefile b/drivers/compress/zlib/Makefile
new file mode 100644
index 0000000..bd322c9
--- /dev/null
+++ b/drivers/compress/zlib/Makefile
@@ -0,0 +1,28 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Cavium Networks
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_pmd_zlib.a
+
+# build flags
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
+# library version
+LIBABIVER := 1
+
+# versioning export map
+EXPORT_MAP := rte_pmd_zlib_version.map
+
+# external library dependencies
+LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring -lz
+LDLIBS += -lrte_compressdev
+LDLIBS += -lrte_bus_vdev
+
+# library source files
+SRCS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += zlib_pmd.c
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/compress/zlib/meson.build b/drivers/compress/zlib/meson.build
new file mode 100644
index 0000000..7748de2
--- /dev/null
+++ b/drivers/compress/zlib/meson.build
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Cavium Networks
+
+dep = dependency('zlib', required: false)
+if not dep.found()
+	build = false
+endif
+
+deps += 'bus_vdev'
+sources = files('zlib_pmd.c', 'zlib_pmd_ops.c')
+ext_deps += dep
+pkgconfig_extra_libs += '-lz'
+
+allow_experimental_apis = true
diff --git a/drivers/compress/zlib/rte_pmd_zlib_version.map b/drivers/compress/zlib/rte_pmd_zlib_version.map
new file mode 100644
index 0000000..1a99a33
--- /dev/null
+++ b/drivers/compress/zlib/rte_pmd_zlib_version.map
@@ -0,0 +1,3 @@
+18.08 {
+	local: *;
+};
diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
new file mode 100644
index 0000000..f667ccc
--- /dev/null
+++ b/drivers/compress/zlib/zlib_pmd.c
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Cavium Networks
+ */
+
+#include <rte_bus_vdev.h>
+#include <rte_common.h>
+#include "zlib_pmd_private.h"
+
+static int
+zlib_create(const char *name,
+		struct rte_vdev_device *vdev,
+		struct rte_compressdev_pmd_init_params *init_params)
+{
+	struct rte_compressdev *dev;
+
+	dev = rte_compressdev_pmd_create(name, &vdev->device,
+			sizeof(struct zlib_private), init_params);
+	if (dev == NULL) {
+		ZLIB_PMD_ERR("driver %s: create failed", init_params->name);
+		return -ENODEV;
+	}
+
+	dev->feature_flags = RTE_COMP_FF_NONCOMPRESSED_BLOCKS;
+
+	return 0;
+}
+
+static int
+zlib_probe(struct rte_vdev_device *vdev)
+{
+	struct rte_compressdev_pmd_init_params init_params = {
+		"",
+		rte_socket_id()
+	};
+	const char *name;
+	const char *input_args;
+
+	name = rte_vdev_device_name(vdev);
+
+	if (name == NULL)
+		return -EINVAL;
+	input_args = rte_vdev_device_args(vdev);
+	rte_compressdev_pmd_parse_input_args(&init_params, input_args);
+
+	return zlib_create(name, vdev, &init_params);
+}
+
+static int
+zlib_remove(struct rte_vdev_device *vdev)
+{
+	struct rte_compressdev *compressdev;
+	const char *name;
+
+	name = rte_vdev_device_name(vdev);
+	if (name == NULL)
+		return -EINVAL;
+
+	compressdev = rte_compressdev_pmd_get_named_dev(name);
+	if (compressdev == NULL)
+		return -ENODEV;
+
+	return rte_compressdev_pmd_destroy(compressdev);
+}
+
+static struct rte_vdev_driver zlib_pmd_drv = {
+	.probe = zlib_probe,
+	.remove = zlib_remove
+};
+
+RTE_PMD_REGISTER_VDEV(COMPRESSDEV_NAME_ZLIB_PMD, zlib_pmd_drv);
+RTE_PMD_REGISTER_ALIAS(COMPRESSDEV_NAME_ZLIB_PMD, compressdev_zlib_pmd);
+
+RTE_INIT(zlib_init_log);
+
+static void
+zlib_init_log(void)
+{
+	zlib_logtype_driver = rte_log_register("compress_zlib");
+	if (zlib_logtype_driver >= 0)
+		rte_log_set_level(zlib_logtype_driver, RTE_LOG_INFO);
+}
diff --git a/drivers/compress/zlib/zlib_pmd_private.h b/drivers/compress/zlib/zlib_pmd_private.h
new file mode 100644
index 0000000..d4c80b1
--- /dev/null
+++ b/drivers/compress/zlib/zlib_pmd_private.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Cavium Networks
+ */
+
+#ifndef _RTE_ZLIB_PMD_PRIVATE_H_
+#define _RTE_ZLIB_PMD_PRIVATE_H_
+
+#include <zlib.h>
+#include <rte_compressdev.h>
+#include <rte_compressdev_pmd.h>
+
+#define COMPRESSDEV_NAME_ZLIB_PMD	compress_zlib
+/**< ZLIB PMD device name */
+
+#define DEF_MEM_LEVEL			8
+
+int zlib_logtype_driver;
+#define ZLIB_PMD_LOG(level, fmt, args...) \
+	rte_log(RTE_LOG_ ## level, zlib_logtype_driver, "%s(): "fmt "\n", \
+			__func__, ##args)
+
+#define ZLIB_PMD_INFO(fmt, args...) \
+	ZLIB_PMD_LOG(INFO, fmt, ## args)
+#define ZLIB_PMD_ERR(fmt, args...) \
+	ZLIB_PMD_LOG(ERR, fmt, ## args)
+#define ZLIB_PMD_WARN(fmt, args...) \
+	ZLIB_PMD_LOG(WARNING, fmt, ## args)
+
+struct zlib_private {
+	char mp_name[RTE_MEMPOOL_NAMESIZE];
+};
+
+#endif /* _RTE_ZLIB_PMD_PRIVATE_H_ */
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 87a0c80..f884b8a 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -213,6 +213,8 @@ endif # CONFIG_RTE_LIBRTE_CRYPTODEV
 ifeq ($(CONFIG_RTE_LIBRTE_COMPRESSDEV),y)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += -lrte_pmd_isal_comp
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += -lisal
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += -lrte_pmd_zlib
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += -lz
 endif # CONFIG_RTE_LIBRTE_COMPRESSDEV
 
 ifeq ($(CONFIG_RTE_LIBRTE_EVENTDEV),y)
-- 
2.9.5

[dpdk-dev] [PATCH v2 2/5] compress/zlib: add device setup PMD ops

[Shally Verma]

From: Ashish Gupta <ashish.gupta@caviumnetworks.com>

Implement device configure and PMD ops

Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
---
 drivers/compress/zlib/Makefile           |   1 +
 drivers/compress/zlib/zlib_pmd.c         |   2 +
 drivers/compress/zlib/zlib_pmd_ops.c     | 236 +++++++++++++++++++++++++++++++
 drivers/compress/zlib/zlib_pmd_private.h |  34 +++++
 4 files changed, 273 insertions(+)

diff --git a/drivers/compress/zlib/Makefile b/drivers/compress/zlib/Makefile
index bd322c9..5cf8de6 100644
--- a/drivers/compress/zlib/Makefile
+++ b/drivers/compress/zlib/Makefile
@@ -24,5 +24,6 @@ LDLIBS += -lrte_bus_vdev
 
 # library source files
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += zlib_pmd.c
+SRCS-$(CONFIG_RTE_LIBRTE_PMD_ZLIB) += zlib_pmd_ops.c
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
index f667ccc..c4f67bb 100644
--- a/drivers/compress/zlib/zlib_pmd.c
+++ b/drivers/compress/zlib/zlib_pmd.c
@@ -20,6 +20,8 @@ zlib_create(const char *name,
 		return -ENODEV;
 	}
 
+	dev->dev_ops = rte_zlib_pmd_ops;
+
 	dev->feature_flags = RTE_COMP_FF_NONCOMPRESSED_BLOCKS;
 
 	return 0;
diff --git a/drivers/compress/zlib/zlib_pmd_ops.c b/drivers/compress/zlib/zlib_pmd_ops.c
new file mode 100644
index 0000000..03b6da5
--- /dev/null
+++ b/drivers/compress/zlib/zlib_pmd_ops.c
@@ -0,0 +1,236 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Cavium Networks
+ */
+
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_malloc.h>
+
+#include "zlib_pmd_private.h"
+
+static const struct rte_compressdev_capabilities zlib_pmd_capabilities[] = {
+	{   /* Deflate */
+		.algo = RTE_COMP_ALGO_DEFLATE,
+		.window_size = {
+			.min = 8,
+			.max = 15,
+			.increment = 1
+		},
+	},
+
+	RTE_COMP_END_OF_CAPABILITIES_LIST()
+
+};
+
+/** Configure device */
+static int
+zlib_pmd_config(struct rte_compressdev *dev,
+		struct rte_compressdev_config *config)
+{
+	struct rte_mempool *mp;
+
+	struct zlib_private *internals = dev->data->dev_private;
+	snprintf(internals->mp_name, RTE_MEMPOOL_NAMESIZE,
+			"stream_mp_%u", dev->data->dev_id);
+	mp = rte_mempool_lookup(internals->mp_name);
+	if (mp == NULL) {
+		mp = rte_mempool_create(internals->mp_name,
+				config->max_nb_priv_xforms +
+				config->max_nb_streams,
+				sizeof(struct zlib_priv_xform),
+				0, 0, NULL, NULL, NULL,
+				NULL, config->socket_id,
+				0);
+		if (mp == NULL) {
+			ZLIB_PMD_ERR("Cannot create private xform pool on "
+			"socket %d\n", config->socket_id);
+			return -ENOMEM;
+		}
+	}
+	return 0;
+}
+
+/** Start device */
+static int
+zlib_pmd_start(__rte_unused struct rte_compressdev *dev)
+{
+	return 0;
+}
+
+/** Stop device */
+static void
+zlib_pmd_stop(__rte_unused struct rte_compressdev *dev)
+{
+}
+
+/** Close device */
+static int
+zlib_pmd_close(struct rte_compressdev *dev)
+{
+	struct zlib_private *internals = dev->data->dev_private;
+	struct rte_mempool *mp = rte_mempool_lookup(internals->mp_name);
+	rte_mempool_free(mp);
+	return 0;
+}
+
+/** Get device statistics */
+static void
+zlib_pmd_stats_get(struct rte_compressdev *dev,
+		struct rte_compressdev_stats *stats)
+{
+	int qp_id;
+
+	for (qp_id = 0; qp_id < dev->data->nb_queue_pairs; qp_id++) {
+		struct zlib_qp *qp = dev->data->queue_pairs[qp_id];
+
+		stats->enqueued_count += qp->qp_stats.enqueued_count;
+		stats->dequeued_count += qp->qp_stats.dequeued_count;
+
+		stats->enqueue_err_count += qp->qp_stats.enqueue_err_count;
+		stats->dequeue_err_count += qp->qp_stats.dequeue_err_count;
+	}
+}
+
+/** Reset device statistics */
+static void
+zlib_pmd_stats_reset(struct rte_compressdev *dev)
+{
+	int qp_id;
+
+	for (qp_id = 0; qp_id < dev->data->nb_queue_pairs; qp_id++) {
+		struct zlib_qp *qp = dev->data->queue_pairs[qp_id];
+
+		memset(&qp->qp_stats, 0, sizeof(qp->qp_stats));
+	}
+}
+
+/** Get device info */
+static void
+zlib_pmd_info_get(struct rte_compressdev *dev,
+		struct rte_compressdev_info *dev_info)
+{
+	if (dev_info != NULL) {
+		dev_info->driver_name = dev->device->name;
+		dev_info->feature_flags = dev->feature_flags;
+		dev_info->capabilities = zlib_pmd_capabilities;
+	}
+}
+
+/** Release queue pair */
+static int
+zlib_pmd_qp_release(struct rte_compressdev *dev, uint16_t qp_id)
+{
+	struct zlib_qp *qp = dev->data->queue_pairs[qp_id];
+	struct rte_ring *r = NULL;
+
+	if (qp != NULL) {
+		r = rte_ring_lookup(qp->name);
+		if (r)
+			rte_ring_free(r);
+		rte_free(qp);
+		dev->data->queue_pairs[qp_id] = NULL;
+	}
+	return 0;
+}
+
+/** set a unique name for the queue pair based on it's name, dev_id and qp_id */
+static int
+zlib_pmd_qp_set_unique_name(struct rte_compressdev *dev,
+		struct zlib_qp *qp)
+{
+	unsigned int n = snprintf(qp->name, sizeof(qp->name),
+				"zlib_pmd_%u_qp_%u",
+				dev->data->dev_id, qp->id);
+
+	if (n >= sizeof(qp->name))
+		return -1;
+
+	return 0;
+}
+
+/** Create a ring to place process packets on */
+static struct rte_ring *
+zlib_pmd_qp_create_processed_pkts_ring(struct zlib_qp *qp,
+		unsigned int ring_size, int socket_id)
+{
+	struct rte_ring *r;
+
+	r = rte_ring_lookup(qp->name);
+	if (r) {
+		if (rte_ring_get_size(r) >= ring_size) {
+			ZLIB_PMD_INFO("Reusing existing ring %s for processed"
+					" packets", qp->name);
+			return r;
+		}
+
+		ZLIB_PMD_ERR("Unable to reuse existing ring %s for processed"
+				" packets", qp->name);
+		return NULL;
+	}
+
+	return rte_ring_create(qp->name, ring_size, socket_id,
+						RING_F_EXACT_SZ);
+}
+
+/** Setup a queue pair */
+static int
+zlib_pmd_qp_setup(struct rte_compressdev *dev, uint16_t qp_id,
+		uint32_t max_inflight_ops, int socket_id)
+{
+	struct zlib_qp *qp = NULL;
+
+	/* Free memory prior to re-allocation if needed. */
+	if (dev->data->queue_pairs[qp_id] != NULL)
+		zlib_pmd_qp_release(dev, qp_id);
+
+	/* Allocate the queue pair data structure. */
+	qp = rte_zmalloc_socket("ZLIB PMD Queue Pair", sizeof(*qp),
+					RTE_CACHE_LINE_SIZE, socket_id);
+	if (qp == NULL)
+		return (-ENOMEM);
+
+	qp->id = qp_id;
+	dev->data->queue_pairs[qp_id] = qp;
+
+	if (zlib_pmd_qp_set_unique_name(dev, qp))
+		goto qp_setup_cleanup;
+
+	qp->processed_pkts = zlib_pmd_qp_create_processed_pkts_ring(qp,
+			max_inflight_ops, socket_id);
+	if (qp->processed_pkts == NULL)
+		goto qp_setup_cleanup;
+
+	memset(&qp->qp_stats, 0, sizeof(qp->qp_stats));
+	return 0;
+
+qp_setup_cleanup:
+	if (qp) {
+		rte_free(qp);
+		qp = NULL;
+	}
+	return -1;
+}
+
+struct rte_compressdev_ops zlib_pmd_ops = {
+		.dev_configure		= zlib_pmd_config,
+		.dev_start		= zlib_pmd_start,
+		.dev_stop		= zlib_pmd_stop,
+		.dev_close		= zlib_pmd_close,
+
+		.stats_get		= zlib_pmd_stats_get,
+		.stats_reset		= zlib_pmd_stats_reset,
+
+		.dev_infos_get		= zlib_pmd_info_get,
+
+		.queue_pair_setup	= zlib_pmd_qp_setup,
+		.queue_pair_release	= zlib_pmd_qp_release,
+
+		.private_xform_create	= NULL,
+		.private_xform_free		= NULL,
+
+		.stream_create	= NULL,
+		.stream_free	= NULL
+};
+
+struct rte_compressdev_ops *rte_zlib_pmd_ops = &zlib_pmd_ops;
diff --git a/drivers/compress/zlib/zlib_pmd_private.h b/drivers/compress/zlib/zlib_pmd_private.h
index d4c80b1..dc83464 100644
--- a/drivers/compress/zlib/zlib_pmd_private.h
+++ b/drivers/compress/zlib/zlib_pmd_private.h
@@ -30,4 +30,38 @@ struct zlib_private {
 	char mp_name[RTE_MEMPOOL_NAMESIZE];
 };
 
+struct zlib_qp {
+	struct rte_ring *processed_pkts;
+	/**< Ring for placing process packets */
+	struct rte_compressdev_stats qp_stats;
+	/**< Queue pair statistics */
+	uint16_t id;
+	/**< Queue Pair Identifier */
+	char name[RTE_COMPRESSDEV_NAME_MAX_LEN];
+	/**< Unique Queue Pair Name */
+} __rte_cache_aligned;
+
+/* Algorithm handler function prototype */
+typedef void (*comp_func_t)(struct rte_comp_op *op, z_stream *strm);
+
+typedef int (*comp_free_t)(z_stream *strm);
+
+/** ZLIB Stream structure */
+struct zlib_stream {
+	z_stream strm;
+	/**< zlib stream structure */
+	comp_func_t comp;
+	/**< Operation (compression/decompression) */
+	comp_free_t free;
+	/**< Free Operation (compression/decompression) */
+} __rte_cache_aligned;
+
+/** ZLIB private xform structure */
+struct zlib_priv_xform {
+	struct zlib_stream stream;
+} __rte_cache_aligned;
+
+/** Device specific operations function pointer structure */
+extern struct rte_compressdev_ops *rte_zlib_pmd_ops;
+
 #endif /* _RTE_ZLIB_PMD_PRIVATE_H_ */
-- 
2.9.5

[dpdk-dev] [PATCH v2 3/5] compress/zlib: add xform and stream create support

[Shally Verma]

From: Sunila Sahu <ssahu@caviumnetworks.com>

Implement private xform and stream create ops

Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
---
 drivers/compress/zlib/zlib_pmd.c         | 93 ++++++++++++++++++++++++++++++++
 drivers/compress/zlib/zlib_pmd_ops.c     | 83 ++++++++++++++++++++++++++--
 drivers/compress/zlib/zlib_pmd_private.h |  4 ++
 3 files changed, 176 insertions(+), 4 deletions(-)

diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
index c4f67bb..7c2614e 100644
--- a/drivers/compress/zlib/zlib_pmd.c
+++ b/drivers/compress/zlib/zlib_pmd.c
@@ -6,6 +6,99 @@
 #include <rte_common.h>
 #include "zlib_pmd_private.h"
 
+/** Parse comp xform and set private xform/stream parameters */
+int
+zlib_set_stream_parameters(const struct rte_comp_xform *xform,
+		struct zlib_stream *stream)
+{
+	int strategy, level, wbits;
+	z_stream *strm = &stream->strm;
+
+	/* allocate deflate state */
+	strm->zalloc = Z_NULL;
+	strm->zfree = Z_NULL;
+	strm->opaque = Z_NULL;
+
+	switch (xform->type) {
+	case RTE_COMP_COMPRESS:
+		/** Compression window bits */
+		switch (xform->compress.algo) {
+		case RTE_COMP_ALGO_DEFLATE:
+			wbits = -(xform->compress.window_size);
+			break;
+		default:
+			ZLIB_PMD_ERR("Compression algorithm not supported\n");
+			return -1;
+		}
+		/** Compression Level */
+		switch (xform->compress.level) {
+		case RTE_COMP_LEVEL_PMD_DEFAULT:
+			level = Z_DEFAULT_COMPRESSION;
+			break;
+		case RTE_COMP_LEVEL_NONE:
+			level = Z_NO_COMPRESSION;
+			break;
+		case RTE_COMP_LEVEL_MIN:
+			level = Z_BEST_SPEED;
+			break;
+		case RTE_COMP_LEVEL_MAX:
+			level = Z_BEST_COMPRESSION;
+			break;
+		default:
+			level = xform->compress.level;
+			if (level < RTE_COMP_LEVEL_MIN ||
+					level > RTE_COMP_LEVEL_MAX) {
+				ZLIB_PMD_ERR("Compression level %d "
+						"not supported\n",
+						level);
+				return -1;
+			}
+		}
+		/** Compression strategy */
+		switch (xform->compress.deflate.huffman) {
+		case RTE_COMP_HUFFMAN_DEFAULT:
+			strategy = Z_DEFAULT_STRATEGY;
+			break;
+		case RTE_COMP_HUFFMAN_FIXED:
+			strategy = Z_FIXED;
+			break;
+		case RTE_COMP_HUFFMAN_DYNAMIC:
+			strategy = Z_HUFFMAN_ONLY;
+			break;
+		default:
+			ZLIB_PMD_ERR("Compression strategy not supported\n");
+			return -1;
+		}
+		if (deflateInit2(strm, level,
+					Z_DEFLATED, wbits,
+					DEF_MEM_LEVEL, strategy) != Z_OK) {
+			ZLIB_PMD_ERR("Deflate init failed\n");
+			return -1;
+		}
+	break;
+
+	case RTE_COMP_DECOMPRESS:
+		/** window bits */
+		switch (xform->decompress.algo) {
+		case RTE_COMP_ALGO_DEFLATE:
+			wbits = -(xform->decompress.window_size);
+			break;
+		default:
+			ZLIB_PMD_ERR("Compression algorithm not supported\n");
+			return -1;
+		}
+
+		if (inflateInit2(strm, wbits) != Z_OK) {
+			ZLIB_PMD_ERR("Inflate init failed\n");
+			return -1;
+		}
+		break;
+	default:
+		return -1;
+	}
+	return 0;
+}
+
 static int
 zlib_create(const char *name,
 		struct rte_vdev_device *vdev,
diff --git a/drivers/compress/zlib/zlib_pmd_ops.c b/drivers/compress/zlib/zlib_pmd_ops.c
index 03b6da5..812406b 100644
--- a/drivers/compress/zlib/zlib_pmd_ops.c
+++ b/drivers/compress/zlib/zlib_pmd_ops.c
@@ -212,6 +212,81 @@ zlib_pmd_qp_setup(struct rte_compressdev *dev, uint16_t qp_id,
 	return -1;
 }
 
+/** Configure stream */
+static int
+zlib_pmd_stream_create(struct rte_compressdev *dev,
+		const struct rte_comp_xform *xform,
+		void **zstream)
+{
+	int ret = 0;
+	struct zlib_stream *stream;
+	struct zlib_private *internals = dev->data->dev_private;
+
+	if (xform == NULL) {
+		ZLIB_PMD_ERR("invalid xform struct");
+		return -EINVAL;
+	}
+
+	struct rte_mempool *mp = rte_mempool_lookup(internals->mp_name);
+	if (rte_mempool_get(mp, zstream)) {
+		ZLIB_PMD_ERR(
+				"Couldn't get object from session mempool");
+		return -ENOMEM;
+	}
+	stream = *((struct zlib_stream **)zstream);
+
+	ret = zlib_set_stream_parameters(xform, stream);
+
+	if (ret < 0) {
+		ZLIB_PMD_ERR("failed configure session parameters");
+
+		memset(stream, 0, sizeof(struct zlib_stream));
+		/* Return session to mempool */
+		rte_mempool_put(mp, stream);
+		return ret;
+	}
+
+	return 0;
+}
+
+/** Configure private xform */
+static int
+zlib_pmd_private_xform_create(struct rte_compressdev *dev,
+		const struct rte_comp_xform *xform,
+		void **private_xform)
+{
+	int ret = 0;
+
+	ret = zlib_pmd_stream_create(dev, xform, private_xform);
+	return ret;
+}
+
+/** Clear the memory of stream so it doesn't leave key material behind */
+static int
+zlib_pmd_stream_free(__rte_unused struct rte_compressdev *dev,
+		void *zstream)
+{
+	struct zlib_stream *stream = (struct zlib_stream *)zstream;
+	if (!stream)
+		return -EINVAL;
+
+	stream->free(&stream->strm);
+	/* Zero out the whole structure */
+	memset(stream, 0, sizeof(struct zlib_stream));
+	struct rte_mempool *mp = rte_mempool_from_obj(stream);
+	rte_mempool_put(mp, stream);
+
+	return 0;
+}
+
+/** Clear the memory of stream so it doesn't leave key material behind */
+static int
+zlib_pmd_private_xform_free(struct rte_compressdev *dev,
+		void *private_xform)
+{
+	return zlib_pmd_stream_free(dev, private_xform);
+}
+
 struct rte_compressdev_ops zlib_pmd_ops = {
 		.dev_configure		= zlib_pmd_config,
 		.dev_start		= zlib_pmd_start,
@@ -226,11 +301,11 @@ struct rte_compressdev_ops zlib_pmd_ops = {
 		.queue_pair_setup	= zlib_pmd_qp_setup,
 		.queue_pair_release	= zlib_pmd_qp_release,
 
-		.private_xform_create	= NULL,
-		.private_xform_free		= NULL,
+		.private_xform_create	= zlib_pmd_private_xform_create,
+		.private_xform_free		= zlib_pmd_private_xform_free,
 
-		.stream_create	= NULL,
-		.stream_free	= NULL
+		.stream_create	= zlib_pmd_stream_create,
+		.stream_free	= zlib_pmd_stream_free
 };
 
 struct rte_compressdev_ops *rte_zlib_pmd_ops = &zlib_pmd_ops;
diff --git a/drivers/compress/zlib/zlib_pmd_private.h b/drivers/compress/zlib/zlib_pmd_private.h
index dc83464..cdfa866 100644
--- a/drivers/compress/zlib/zlib_pmd_private.h
+++ b/drivers/compress/zlib/zlib_pmd_private.h
@@ -61,6 +61,10 @@ struct zlib_priv_xform {
 	struct zlib_stream stream;
 } __rte_cache_aligned;
 
+int
+zlib_set_stream_parameters(const struct rte_comp_xform *xform,
+		struct zlib_stream *stream);
+
 /** Device specific operations function pointer structure */
 extern struct rte_compressdev_ops *rte_zlib_pmd_ops;
 
-- 
2.9.5

[dpdk-dev] [PATCH v2 4/5] compress/zlib: add enq deq apis

[Shally Verma]

From: Sunila Sahu <ssahu@caviumnetworks.com>

implement enqueue and dequeue apis

Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
---
 drivers/compress/zlib/zlib_pmd.c | 238 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 237 insertions(+), 1 deletion(-)

diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
index 7c2614e..aef23de 100644
--- a/drivers/compress/zlib/zlib_pmd.c
+++ b/drivers/compress/zlib/zlib_pmd.c
@@ -6,7 +6,198 @@
 #include <rte_common.h>
 #include "zlib_pmd_private.h"
 
-/** Parse comp xform and set private xform/stream parameters */
+/** Compute next mbuf in the list, assign data buffer and len */
+#define COMPUTE_BUF(mbuf, data, len)		\
+		((mbuf = mbuf->next) ?		\
+		(data = rte_pktmbuf_mtod(mbuf, uint8_t *)),	\
+		(len = rte_pktmbuf_data_len(mbuf)) : 0)
+
+/** Set op->status to appropriate flag if we run out of mbuf */
+#define COMPUTE_DST_BUF(mbuf, dst, dlen)		\
+		((COMPUTE_BUF(mbuf, dst, dlen)) ? 1 :	\
+		(!(op->status =				\
+		RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED)))
+
+static void
+process_zlib_deflate(struct rte_comp_op *op, z_stream *strm)
+{
+	int ret, flush, fin_flush;
+	struct rte_mbuf *mbuf_src = op->m_src;
+	struct rte_mbuf *mbuf_dst = op->m_dst;
+
+	switch (op->flush_flag) {
+	case RTE_COMP_FLUSH_FULL:
+	case RTE_COMP_FLUSH_FINAL:
+		fin_flush = Z_FINISH;
+		break;
+	default:
+		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
+		ZLIB_PMD_ERR("\nInvalid flush value");
+
+	}
+
+	if (unlikely(!strm)) {
+		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
+		ZLIB_PMD_ERR("\nInvalid z_stream");
+		return;
+	}
+	/* Update z_stream with the inputs provided by application */
+	strm->next_in = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *,
+			op->src.offset);
+
+	strm->avail_in = rte_pktmbuf_data_len(mbuf_src) - op->src.offset;
+
+	strm->next_out = rte_pktmbuf_mtod_offset(mbuf_dst, uint8_t *,
+			op->dst.offset);
+
+	strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
+
+	/* Set flush value to NO_FLUSH unless it is last mbuf */
+	flush = Z_NO_FLUSH;
+	/* Initialize status to SUCCESS */
+	op->status = RTE_COMP_OP_STATUS_SUCCESS;
+
+	do {
+		/* Set flush value to Z_FINISH for last block */
+		if ((op->src.length - strm->total_in) <= strm->avail_in) {
+			strm->avail_in = (op->src.length - strm->total_in);
+			flush = fin_flush;
+		}
+		do {
+			ret = deflate(strm, flush);
+			if (unlikely(ret == Z_STREAM_ERROR)) {
+				/* error return, do not process further */
+				op->status =  RTE_COMP_OP_STATUS_ERROR;
+				break;
+			}
+			/* Break if Z_STREAM_END is encountered */
+			if (ret == Z_STREAM_END)
+				goto def_end;
+
+		 /* Break if current input buffer is consumed or
+		  * there is no more space for compressed output
+		  */
+		} while ((strm->avail_out == 0) &&
+			COMPUTE_DST_BUF(mbuf_dst,  strm->next_out,
+				strm->avail_out));
+
+	/* Update source buffer to next mbuf
+	 * Exit if op->status is not SUCCESS or input buffers are fully consumed
+	 */
+	} while ((op->status == RTE_COMP_OP_STATUS_SUCCESS) &&
+		(COMPUTE_BUF(mbuf_src, strm->next_in, strm->avail_in)));
+
+def_end:
+	/* Update op stats */
+	switch (op->status) {
+	case RTE_COMP_OP_STATUS_SUCCESS:
+		op->consumed += strm->total_in;
+	case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
+		op->produced += strm->total_out;
+		break;
+	default:
+		ZLIB_PMD_ERR("stats not updated for status:%d\n",
+				op->status);
+	}
+
+	deflateReset(strm);
+}
+
+static void
+process_zlib_inflate(struct rte_comp_op *op, z_stream *strm)
+{
+	int ret, flush;
+	struct rte_mbuf *mbuf_src = op->m_src;
+	struct rte_mbuf *mbuf_dst = op->m_dst;
+
+	if (unlikely(!strm)) {
+		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
+		ZLIB_PMD_ERR("\nInvalid z_stream");
+		return;
+	}
+	strm->next_in = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *,
+			op->src.offset);
+
+	strm->avail_in = rte_pktmbuf_data_len(mbuf_src) - op->src.offset;
+
+	strm->next_out = rte_pktmbuf_mtod_offset(mbuf_dst, uint8_t *,
+			op->dst.offset);
+
+	strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
+
+	/** Ignoring flush value provided from application for decompression */
+	flush = Z_NO_FLUSH;
+	/* initialize status to SUCCESS */
+	op->status = RTE_COMP_OP_STATUS_SUCCESS;
+
+	do {
+		do {
+			ret = inflate(strm, flush);
+
+			switch (ret) {
+			case Z_NEED_DICT:
+				ret = Z_DATA_ERROR;     /* and fall through */
+			case Z_DATA_ERROR:
+			case Z_MEM_ERROR:
+			case Z_STREAM_ERROR:
+				op->status = RTE_COMP_OP_STATUS_ERROR;
+				break;
+			default:
+				/* Break if Z_STREAM_END is encountered */
+				if (ret == Z_STREAM_END)
+					goto inf_end;
+			}
+		/* Break if Z_STREAM_END is encountered or dst mbuf gets over */
+		} while ((strm->avail_out == 0) &&
+			COMPUTE_DST_BUF(mbuf_dst, strm->next_out,
+				strm->avail_out));
+
+		/* Exit if op->status is not SUCCESS.
+		 * Read next input buffer to be processed, exit if compressed
+		 * blocks are fully read
+		 */
+	} while ((op->status == RTE_COMP_OP_STATUS_SUCCESS) &&
+		(COMPUTE_BUF(mbuf_src, strm->next_in, strm->avail_in)));
+
+inf_end:
+	/* Update op stats */
+	switch (op->status) {
+	case RTE_COMP_OP_STATUS_SUCCESS:
+		op->consumed += strm->total_in;
+	case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
+		op->produced += strm->total_out;
+		break;
+	default:
+		ZLIB_PMD_ERR("stats not produced for status:%d\n",
+				op->status);
+	}
+
+	inflateReset(strm);
+}
+
+/** Process comp operation for mbuf */
+static inline int
+process_zlib_op(struct zlib_qp *qp, struct rte_comp_op *op)
+{
+	struct zlib_stream *stream;
+
+	if ((op->op_type == RTE_COMP_OP_STATEFUL) ||
+		op->src.offset > rte_pktmbuf_data_len(op->m_src) ||
+		op->dst.offset > rte_pktmbuf_data_len(op->m_dst)) {
+		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
+		ZLIB_PMD_ERR("\nInvalid source or destination buffers or "
+				"invalid Operation requested");
+	} else {
+		stream = &((struct zlib_priv_xform *)op->private_xform)->stream;
+		stream->comp(op, &stream->strm);
+	}
+	/* whatever is out of op, put it into completion queue with
+	 * its status
+	 */
+	return rte_ring_enqueue(qp->processed_pkts, (void *)op);
+}
+
+/** Parse comp xform and set private xform/Stream parameters */
 int
 zlib_set_stream_parameters(const struct rte_comp_xform *xform,
 		struct zlib_stream *stream)
@@ -21,6 +212,8 @@ zlib_set_stream_parameters(const struct rte_comp_xform *xform,
 
 	switch (xform->type) {
 	case RTE_COMP_COMPRESS:
+		stream->comp = process_zlib_deflate;
+		stream->free = deflateEnd;
 		/** Compression window bits */
 		switch (xform->compress.algo) {
 		case RTE_COMP_ALGO_DEFLATE:
@@ -78,6 +271,8 @@ zlib_set_stream_parameters(const struct rte_comp_xform *xform,
 	break;
 
 	case RTE_COMP_DECOMPRESS:
+		stream->comp = process_zlib_inflate;
+		stream->free = inflateEnd;
 		/** window bits */
 		switch (xform->decompress.algo) {
 		case RTE_COMP_ALGO_DEFLATE:
@@ -99,6 +294,43 @@ zlib_set_stream_parameters(const struct rte_comp_xform *xform,
 	return 0;
 }
 
+static uint16_t
+zlib_pmd_enqueue_burst(void *queue_pair,
+			struct rte_comp_op **ops, uint16_t nb_ops)
+{
+	struct zlib_qp *qp = queue_pair;
+	int ret, i;
+	int enqd = 0;
+	for (i = 0; i < nb_ops; i++) {
+		ret = process_zlib_op(qp, ops[i]);
+		if (unlikely(ret < 0)) {
+			/* increment count if failed to push to completion
+			 * queue
+			 */
+			qp->qp_stats.enqueue_err_count++;
+		} else {
+			qp->qp_stats.enqueued_count++;
+			enqd++;
+		}
+	}
+	return enqd;
+}
+
+static uint16_t
+zlib_pmd_dequeue_burst(void *queue_pair,
+			struct rte_comp_op **ops, uint16_t nb_ops)
+{
+	struct zlib_qp *qp = queue_pair;
+
+	unsigned int nb_dequeued = 0;
+
+	nb_dequeued = rte_ring_dequeue_burst(qp->processed_pkts,
+			(void **)ops, nb_ops, NULL);
+	qp->qp_stats.dequeued_count += nb_dequeued;
+
+	return nb_dequeued;
+}
+
 static int
 zlib_create(const char *name,
 		struct rte_vdev_device *vdev,
@@ -115,6 +347,10 @@ zlib_create(const char *name,
 
 	dev->dev_ops = rte_zlib_pmd_ops;
 
+	/* register rx/tx burst functions for data path */
+	dev->dequeue_burst = zlib_pmd_dequeue_burst;
+	dev->enqueue_burst = zlib_pmd_enqueue_burst;
+
 	dev->feature_flags = RTE_COMP_FF_NONCOMPRESSED_BLOCKS;
 
 	return 0;
-- 
2.9.5

[dpdk-dev] [PATCH v2 5/5] doc: add ZLIB PMD documentation

[Shally Verma]

add zlib pmd feature specification and overview documentation

Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
---
 MAINTAINERS                               |  2 +
 doc/guides/compressdevs/features/zlib.ini | 22 ++++++++++
 doc/guides/compressdevs/zlib.rst          | 68 +++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 448bbe1..1c217b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -854,6 +854,8 @@ F: doc/guides/compressdevs/features/isal.ini
 ZLIB
 M: Sunila Sahu <sunila.sahu@caviumnetworks.com>
 F: drivers/compress/zlib/
+F: doc/guides/compressdevs/zlib.rst
+F: doc/guides/compressdevs/features/zlib.ini
 
 Eventdev Drivers
 ----------------
diff --git a/doc/guides/compressdevs/features/zlib.ini b/doc/guides/compressdevs/features/zlib.ini
new file mode 100644
index 0000000..bdc0fc4
--- /dev/null
+++ b/doc/guides/compressdevs/features/zlib.ini
@@ -0,0 +1,22 @@
+;
+; Refer to default.ini for the full list of available PMD features.
+;
+; Supported features of 'ZLIB' compression driver.
+;
+[Features]
+HW Accelerated =
+CPU SSE        =
+CPU AVX        =
+CPU AVX2       =
+CPU AVX512     =
+CPU NEON       =
+Stateful       =
+By-Pass        =
+Chained mbufs  =
+Deflate        = Y
+LZS            =
+Adler32        =
+Crc32          =
+Adler32&Crc32  =
+Fixed          = Y
+Dynamic        = Y
diff --git a/doc/guides/compressdevs/zlib.rst b/doc/guides/compressdevs/zlib.rst
new file mode 100644
index 0000000..7dd5c74
--- /dev/null
+++ b/doc/guides/compressdevs/zlib.rst
@@ -0,0 +1,68 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+    Copyright(c) 2018 Cavium Networks.
+
+ZLIB Compression Poll Mode Driver
+==================================
+
+The ZLIB PMD (**librte_pmd_zlib**) provides poll mode compression &
+decompression driver based on SW zlib library,
+
+Features
+--------
+
+ZLIB PMD has support for:
+
+Compression/Decompression algorithm:
+
+* DEFLATE
+
+Huffman code type:
+
+* FIXED
+* DYNAMIC
+
+Window size support:
+
+* Min - 256 bytes
+* Max - 32K
+
+Limitations
+-----------
+
+* Chained mbufs are not supported.
+
+Installation
+------------
+
+* To build DPDK with ZLIB library, the user is required to download the ``libz`` library.
+* Use following command for installation.
+
+*  For Fedora users ::
+  yum install zlib-devel
+*  For ubuntu users ::
+  apt-get install zlib1g-dev
+
+* Once downloaded, the user needs to build the library.
+
+* make can  be used to install the library on their system, before building DPDK::
+
+    make
+    sudo make install
+
+Initialization
+--------------
+
+In order to enable this virtual compression PMD, user must:
+
+* Set ``CONFIG_RTE_LIBRTE_PMD_ZLIB=y`` in config/common_base.
+
+To use the PMD in an application, user must:
+
+* Call ``rte_vdev_init("compress_zlib")`` within the application.
+
+* Use ``--vdev="compress_zlib"`` in the EAL options, which will call ``rte_vdev_init()`` internally.
+
+The following parameter (optional) can be provided in the previous two calls:
+
+* ``socket_id:`` Specify the socket where the memory for the device is going to be allocated
+  (by default, socket_id will be the socket where the core that is creating the PMD is running on).
-- 
2.9.5

Re: [dpdk-dev] [PATCH v2 1/5] compress/zlib: add ZLIB PMD support

[De Lara Guarch, Pablo]

Hi Shally/Ashish,

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:57 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>
> Subject: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
> 

Remove "support" from title.

Also, add the PMD in devtools/test-build.sh.
Since it depends on ZLIB, add a "sed" line below test "$DPDK_DEP_ZLIB" != y.

More comments inline below.

> From: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> 
> Add sw zlib pmd support in compressdev driver.
> Add device probe and remove support.
> Add ZLIB build file support.
> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>

...

> +++ b/drivers/compress/zlib/meson.build
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2018 Cavium
> +Networks
> +
> +dep = dependency('zlib', required: false) if not dep.found()
> +	build = false
> +endif
> +
> +deps += 'bus_vdev'
> +sources = files('zlib_pmd.c', 'zlib_pmd_ops.c') ext_deps += dep

Zlib_pmd_ops.c is created in the next patch, so remove it from here.

> +pkgconfig_extra_libs += '-lz'
> +
> +allow_experimental_apis = true
> diff --git a/drivers/compress/zlib/rte_pmd_zlib_version.map
> b/drivers/compress/zlib/rte_pmd_zlib_version.map
> new file mode 100644
> index 0000000..1a99a33
> --- /dev/null
> +++ b/drivers/compress/zlib/rte_pmd_zlib_version.map
> @@ -0,0 +1,3 @@
> +18.08 {

DPDK_18.08.

> +	local: *;
> +};
> diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
> new file mode 100644
> index 0000000..f667ccc
> --- /dev/null
> +++ b/drivers/compress/zlib/zlib_pmd.c
> @@ -0,0 +1,81 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Cavium Networks
> + */
> +
> +#include <rte_bus_vdev.h>
> +#include <rte_common.h>

Leave space between rte_common and zlib_pmd_private.h

> +#include "zlib_pmd_private.h"
> +
> +static int
> +zlib_create(const char *name,
> +		struct rte_vdev_device *vdev,
> +		struct rte_compressdev_pmd_init_params *init_params) {
> +	struct rte_compressdev *dev;
> +
> +	dev = rte_compressdev_pmd_create(name, &vdev->device,
> +			sizeof(struct zlib_private), init_params);
> +	if (dev == NULL) {
> +		ZLIB_PMD_ERR("driver %s: create failed", init_params->name);
> +		return -ENODEV;
> +	}
> +
> +	dev->feature_flags = RTE_COMP_FF_NONCOMPRESSED_BLOCKS;

This is an algorithm feature flag, so it should go in capabilities.

> +
> +	return 0;
> +}
> +

...

> +RTE_PMD_REGISTER_VDEV(COMPRESSDEV_NAME_ZLIB_PMD, zlib_pmd_drv);
> +RTE_PMD_REGISTER_ALIAS(COMPRESSDEV_NAME_ZLIB_PMD,
> +compressdev_zlib_pmd);

No need to use an alias here. The convention now is driverType_driverName (e.g. compress_zlib).

> +
> +RTE_INIT(zlib_init_log);
> +
> +static void
> +zlib_init_log(void)
> +{
> +	zlib_logtype_driver = rte_log_register("compress_zlib");
> +	if (zlib_logtype_driver >= 0)
> +		rte_log_set_level(zlib_logtype_driver, RTE_LOG_INFO); }

Re: [dpdk-dev] [PATCH v2 2/5] compress/zlib: add device setup PMD ops

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:57 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>
> Subject: [PATCH v2 2/5] compress/zlib: add device setup PMD ops
> 
> From: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> 
> Implement device configure and PMD ops
> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>

...

> b/drivers/compress/zlib/zlib_pmd_ops.c
> new file mode 100644
> index 0000000..03b6da5
> --- /dev/null
> +++ b/drivers/compress/zlib/zlib_pmd_ops.c
> @@ -0,0 +1,236 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Cavium Networks
> + */
> +
> +#include <string.h>
> +
> +#include <rte_common.h>
> +#include <rte_malloc.h>
> +
> +#include "zlib_pmd_private.h"
> +
> +static const struct rte_compressdev_capabilities zlib_pmd_capabilities[] = {
> +	{   /* Deflate */
> +		.algo = RTE_COMP_ALGO_DEFLATE,
> +		.window_size = {
> +			.min = 8,
> +			.max = 15,
> +			.increment = 1
> +		},
> +	},

Add comp_feature_flags here, including the new FIXED and DYNAMIC flags.

> +
> +	RTE_COMP_END_OF_CAPABILITIES_LIST()
> +
> +};
> +

...

> +static int
> +zlib_pmd_qp_release(struct rte_compressdev *dev, uint16_t qp_id) {
> +	struct zlib_qp *qp = dev->data->queue_pairs[qp_id];
> +	struct rte_ring *r = NULL;
> +
> +	if (qp != NULL) {
> +		r = rte_ring_lookup(qp->name);
> +		if (r)
> +			rte_ring_free(r);

I think you can use rte_ring_free(qp->processed_pkts) here directly.

> +		rte_free(qp);
> +		dev->data->queue_pairs[qp_id] = NULL;
> +	}
> +	return 0;
> +}
> +
> +/** set a unique name for the queue pair based on it's name, dev_id and

Typo: "its"

...

> +
> +struct rte_compressdev_ops zlib_pmd_ops = {

...

> +
> +		.private_xform_create	= NULL,
> +		.private_xform_free		= NULL,

Remove extra indentation here.

> +
> +		.stream_create	= NULL,
> +		.stream_free	= NULL
> +};
> +
> +struct rte_compressdev_ops *rte_zlib_pmd_ops = &zlib_pmd_ops;

Re: [dpdk-dev] [PATCH v2 1/5] compress/zlib: add ZLIB PMD support

[De Lara Guarch, Pablo]

Hi,

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:57 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>
> Subject: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
> 
> From: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> 
> Add sw zlib pmd support in compressdev driver.
> Add device probe and remove support.
> Add ZLIB build file support.
> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>

...

> +++ b/drivers/compress/zlib/zlib_pmd_private.h

...

> +
> +struct zlib_private {
> +	char mp_name[RTE_MEMPOOL_NAMESIZE];

One extra comment. I think you can store the pointer to the mempool here,
instead of the name (it saves a lookup and space).

> +};

Re: [dpdk-dev] [PATCH v2 1/5] compress/zlib: add ZLIB PMD support

[De Lara Guarch, Pablo]

And the last comments, sorry for the multiple replies.

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:57 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>
> Subject: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
> 
> From: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> 
> Add sw zlib pmd support in compressdev driver.
> Add device probe and remove support.
> Add ZLIB build file support.
> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>

...

> +++ b/drivers/compress/zlib/zlib_pmd.c

...

> +static void
> +zlib_init_log(void)
> +{
> +	zlib_logtype_driver = rte_log_register("compress_zlib");

The standard for the name of the logtype for PMDs is "pmd.driverType.driverName",
so in this case it would be "pmd.compress.zlib".


> +	if (zlib_logtype_driver >= 0)
> +		rte_log_set_level(zlib_logtype_driver, RTE_LOG_INFO); }
> diff --git a/drivers/compress/zlib/zlib_pmd_private.h
> b/drivers/compress/zlib/zlib_pmd_private.h
> new file mode 100644
> index 0000000..d4c80b1
> --- /dev/null
> +++ b/drivers/compress/zlib/zlib_pmd_private.h

...

> +#define ZLIB_PMD_INFO(fmt, args...) \
> +	ZLIB_PMD_LOG(INFO, fmt, ## args)
> +#define ZLIB_PMD_ERR(fmt, args...) \
> +	ZLIB_PMD_LOG(ERR, fmt, ## args)
> +#define ZLIB_PMD_WARN(fmt, args...) \
> +	ZLIB_PMD_LOG(WARNING, fmt, ## args)

What do you think of having a single macro ZLIB_LOG(level, fmt, args...)?

Re: [dpdk-dev] [PATCH v2 3/5] compress/zlib: add xform and stream create support

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:57 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Sunila Sahu <ssahu@caviumnetworks.com>;
> Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>
> Subject: [PATCH v2 3/5] compress/zlib: add xform and stream create support

Retitle to "compress/zlib: support xform and stream creation"

About stateful, since it looks like it is not supported for the moment,
I think stream_create/stream_free functions should fail or should not be implemented.

More comments inline.

> 
> From: Sunila Sahu <ssahu@caviumnetworks.com>
> 
> Implement private xform and stream create ops
> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> ---
>  drivers/compress/zlib/zlib_pmd.c         | 93
> ++++++++++++++++++++++++++++++++
>  drivers/compress/zlib/zlib_pmd_ops.c     | 83 ++++++++++++++++++++++++++--
>  drivers/compress/zlib/zlib_pmd_private.h |  4 ++
>  3 files changed, 176 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
> index c4f67bb..7c2614e 100644
> --- a/drivers/compress/zlib/zlib_pmd.c
> +++ b/drivers/compress/zlib/zlib_pmd.c

...

> +		case RTE_COMP_HUFFMAN_DYNAMIC:
> +			strategy = Z_HUFFMAN_ONLY;

I don't think this is the suitable value for dynamic compression.
Z_HUFFMAN_ONLY does not do LZ77. I think the strategy for this case is Z_DEFAULT_STRATEGY.

> +			break;
> +		default:
> +			ZLIB_PMD_ERR("Compression strategy not
> supported\n");
> +			return -1;
> +		}

...

> +++ b/drivers/compress/zlib/zlib_pmd_ops.c

...

> +	struct rte_mempool *mp = rte_mempool_lookup(internals->mp_name);

As said in other email, you can directly get the mempool pointer from internals.

> +	if (rte_mempool_get(mp, zstream)) {
> +		ZLIB_PMD_ERR(
> +				"Couldn't get object from session mempool");

This can be in the same line.

...

> +/** Configure private xform */
> +static int
> +zlib_pmd_private_xform_create(struct rte_compressdev *dev,
> +		const struct rte_comp_xform *xform,
> +		void **private_xform)
> +{
> +	int ret = 0;
> +
> +	ret = zlib_pmd_stream_create(dev, xform, private_xform);
> +	return ret;

This can be written like "return zlib_pmd_stream_create(...);

...

> +		.private_xform_create	= zlib_pmd_private_xform_create,
> +		.private_xform_free		=
> zlib_pmd_private_xform_free,

Fix indentation here.

> 
> -		.stream_create	= NULL,
> -		.stream_free	= NULL
> +		.stream_create	= zlib_pmd_stream_create,
> +		.stream_free	= zlib_pmd_stream_free
>  };

Re: [dpdk-dev] [PATCH v2 1/5] compress/zlib: add ZLIB PMD support

[Verma, Shally]

Hi Pablo

>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>Sent: 11 July 2018 17:44
>To: Verma, Shally <Shally.Verma@cavium.com>
>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
><Mahipal.Challa@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>
>Subject: RE: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
>
>External Email
>
>And the last comments, sorry for the multiple replies.

No issues.

>
>> -----Original Message-----
>> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
>> Sent: Monday, July 2, 2018 5:57 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
>> mchalla@caviumnetworks.com; Ashish Gupta
>> <ashish.gupta@caviumnetworks.com>; Sunila Sahu
>> <sunila.sahu@caviumnetworks.com>
>> Subject: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
>>
>> From: Ashish Gupta <ashish.gupta@caviumnetworks.com>
>>
>> Add sw zlib pmd support in compressdev driver.
>> Add device probe and remove support.
>> Add ZLIB build file support.
>>
>> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
>> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
>
>...
>
>> +++ b/drivers/compress/zlib/zlib_pmd.c
>
>...
>
>> +static void
>> +zlib_init_log(void)
>> +{
>> +     zlib_logtype_driver = rte_log_register("compress_zlib");
>
>The standard for the name of the logtype for PMDs is "pmd.driverType.driverName",
>so in this case it would be "pmd.compress.zlib".
>
>
>> +     if (zlib_logtype_driver >= 0)
>> +             rte_log_set_level(zlib_logtype_driver, RTE_LOG_INFO); }
>> diff --git a/drivers/compress/zlib/zlib_pmd_private.h
>> b/drivers/compress/zlib/zlib_pmd_private.h
>> new file mode 100644
>> index 0000000..d4c80b1
>> --- /dev/null
>> +++ b/drivers/compress/zlib/zlib_pmd_private.h
>
>...
>
>> +#define ZLIB_PMD_INFO(fmt, args...) \
>> +     ZLIB_PMD_LOG(INFO, fmt, ## args)
>> +#define ZLIB_PMD_ERR(fmt, args...) \
>> +     ZLIB_PMD_LOG(ERR, fmt, ## args)
>> +#define ZLIB_PMD_WARN(fmt, args...) \
>> +     ZLIB_PMD_LOG(WARNING, fmt, ## args)
>
>What do you think of having a single macro ZLIB_LOG(level, fmt, args...)?
>
I find it simpler to use ZLIB_PMD_INFO/ERR?DEBUG version . So would prefer to stick to them.

Thanks for review.
Shally

Re: [dpdk-dev] [PATCH v2 4/5] compress/zlib: add enq deq apis

[De Lara Guarch, Pablo]

Hi,

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:57 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Sunila Sahu <ssahu@caviumnetworks.com>;
> Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>
> Subject: [PATCH v2 4/5] compress/zlib: add enq deq apis

Better retitle to "support burst enqueue/dequeue".

> 
> From: Sunila Sahu <ssahu@caviumnetworks.com>
> 
> implement enqueue and dequeue apis

This should start with capital letter, but this is probably not needed anyway.

> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> ---
>  drivers/compress/zlib/zlib_pmd.c | 238
> ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 237 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
> index 7c2614e..aef23de 100644
> --- a/drivers/compress/zlib/zlib_pmd.c
> +++ b/drivers/compress/zlib/zlib_pmd.c
> @@ -6,7 +6,198 @@
>  #include <rte_common.h>
>  #include "zlib_pmd_private.h"
> 
> -/** Parse comp xform and set private xform/stream parameters */
> +/** Compute next mbuf in the list, assign data buffer and len */
> +#define COMPUTE_BUF(mbuf, data, len)		\
> +		((mbuf = mbuf->next) ?		\
> +		(data = rte_pktmbuf_mtod(mbuf, uint8_t *)),	\
> +		(len = rte_pktmbuf_data_len(mbuf)) : 0)
> +
> +/** Set op->status to appropriate flag if we run out of mbuf */
> +#define COMPUTE_DST_BUF(mbuf, dst, dlen)		\
> +		((COMPUTE_BUF(mbuf, dst, dlen)) ? 1 :	\
> +		(!(op->status =				\

I see and build error here:

drivers/compress/zlib/zlib_pmd.c(81): error #187: use of "=" where "==" may have been intended
                        COMPUTE_DST_BUF(mbuf_dst,  strm->next_out,
It is a bit confusing macro, but afaik, you should pass op if you want to modify the status
(I suggest making this more readable).
 

> +		RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED)))
> +
> +static void
> +process_zlib_deflate(struct rte_comp_op *op, z_stream *strm) {
> +	int ret, flush, fin_flush;
> +	struct rte_mbuf *mbuf_src = op->m_src;
> +	struct rte_mbuf *mbuf_dst = op->m_dst;
> +
> +	switch (op->flush_flag) {
> +	case RTE_COMP_FLUSH_FULL:
> +	case RTE_COMP_FLUSH_FINAL:
> +		fin_flush = Z_FINISH;
> +		break;
> +	default:
> +		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> +		ZLIB_PMD_ERR("\nInvalid flush value");

Better to have "\n" at the end for consistency.

> +
> +	}
> +
> +	if (unlikely(!strm)) {
> +		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> +		ZLIB_PMD_ERR("\nInvalid z_stream");
> +		return;
> +	}
> +	/* Update z_stream with the inputs provided by application */
> +	strm->next_in = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *,
> +			op->src.offset);
> +
> +	strm->avail_in = rte_pktmbuf_data_len(mbuf_src) - op->src.offset;

Shouldn't this be "op->src.length"?

> +
> +	strm->next_out = rte_pktmbuf_mtod_offset(mbuf_dst, uint8_t *,
> +			op->dst.offset);
> +
> +	strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
> +
> +	/* Set flush value to NO_FLUSH unless it is last mbuf */
> +	flush = Z_NO_FLUSH;
> +	/* Initialize status to SUCCESS */
> +	op->status = RTE_COMP_OP_STATUS_SUCCESS;
> +

...

> +	/* Update source buffer to next mbuf
> +	 * Exit if op->status is not SUCCESS or input buffers are fully consumed
> +	 */
> +	} while ((op->status == RTE_COMP_OP_STATUS_SUCCESS) &&
> +		(COMPUTE_BUF(mbuf_src, strm->next_in, strm->avail_in)));

It looks like you support Scatter Gather here, but according to the documentation, you don't support it...

> +
> +def_end:
> +	/* Update op stats */
> +	switch (op->status) {
> +	case RTE_COMP_OP_STATUS_SUCCESS:
> +		op->consumed += strm->total_in;

I see a compilation error with gcc:

drivers/compress/zlib/zlib_pmd.c:166:16: error:
this statement may fall through [-Werror=implicit-fallthrough=]
   op->consumed += strm->total_in;
   ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
drivers/compress/zlib/zlib_pmd.c:167:2: note: here
  case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
  ^~~~

If the intention is to fall-through, you should add a comment before the next case.

> +	case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
> +		op->produced += strm->total_out;
> +		break;
> +	default:
> +		ZLIB_PMD_ERR("stats not updated for status:%d\n",
> +				op->status);
> +	}
> +
> +	deflateReset(strm);
> +}
> +

...

> +
> +/** Process comp operation for mbuf */
> +static inline int
> +process_zlib_op(struct zlib_qp *qp, struct rte_comp_op *op) {
> +	struct zlib_stream *stream;
> +
> +	if ((op->op_type == RTE_COMP_OP_STATEFUL) ||
> +		op->src.offset > rte_pktmbuf_data_len(op->m_src) ||
> +		op->dst.offset > rte_pktmbuf_data_len(op->m_dst)) {

An extra indentation is needed, so it is easy to distinguish between the if statement and the body.
You should also check if (src.offset + src.length > rte_pktmbuf_data_len(op->m_src))

> +		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> +		ZLIB_PMD_ERR("\nInvalid source or destination buffers or "
> +				"invalid Operation requested");

Better to include the "\n" at the end.

> +	} else {
> +		stream = &((struct zlib_priv_xform *)op->private_xform)-
> >stream;

I think it is more readable to have this line split into two:
First get the private xform and then get zlib_stream pointer.

> +		stream->comp(op, &stream->strm);
> +	}
> +	/* whatever is out of op, put it into completion queue with
> +	 * its status
> +	 */
> +	return rte_ring_enqueue(qp->processed_pkts, (void *)op); }
> +

...

> +static uint16_t
> +zlib_pmd_enqueue_burst(void *queue_pair,
> +			struct rte_comp_op **ops, uint16_t nb_ops) {
> +	struct zlib_qp *qp = queue_pair;
> +	int ret, i;
> +	int enqd = 0;

"i" and "enqd" variables should be uint16_t.

> +	for (i = 0; i < nb_ops; i++) {
> +		ret = process_zlib_op(qp, ops[i]);

I think you should check if there is enough space in the ring for all the operations, to save time.
If there is not, then the PMD would modify the operation unnecessarily and waste a lot of time.
Then, with that check, you can just process the operations that can fit, looping until minimum between
nb_ops and space available in the ring.

> +		if (unlikely(ret < 0)) {
> +			/* increment count if failed to push to completion
> +			 * queue
> +			 */
> +			qp->qp_stats.enqueue_err_count++;

I think this variable should be used when there was an error in the operation but
it was still be enqueued (because there was space in the ring).
So you can increase this count in process_zlib_op when there is an error.

> +		} else {
> +			qp->qp_stats.enqueued_count++;

I think this should be equal to all the operations enqueued (with and without error).

> +			enqd++;
> +		}
> +	}
> +	return enqd;
> +}

Re: [dpdk-dev] [PATCH v2 3/5] compress/zlib: add xform and stream create support

[Verma, Shally]

>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>Sent: 11 July 2018 17:57
>To: Verma, Shally <Shally.Verma@cavium.com>
>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
><Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish
><Ashish.Gupta@cavium.com>
>Subject: RE: [PATCH v2 3/5] compress/zlib: add xform and stream create support
>
>External Email
>
>> -----Original Message-----
>> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
>> Sent: Monday, July 2, 2018 5:57 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
>> mchalla@caviumnetworks.com; Sunila Sahu <ssahu@caviumnetworks.com>;
>> Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish Gupta
>> <ashish.gupta@caviumnetworks.com>
>> Subject: [PATCH v2 3/5] compress/zlib: add xform and stream create support
>
>Retitle to "compress/zlib: support xform and stream creation"
>
>About stateful, since it looks like it is not supported for the moment,
>I think stream_create/stream_free functions should fail or should not be implemented.
>
>More comments inline.
>
>>
>> From: Sunila Sahu <ssahu@caviumnetworks.com>
>>
>> Implement private xform and stream create ops
>>
>> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
>> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
>> ---
>>  drivers/compress/zlib/zlib_pmd.c         | 93
>> ++++++++++++++++++++++++++++++++
>>  drivers/compress/zlib/zlib_pmd_ops.c     | 83 ++++++++++++++++++++++++++--
>>  drivers/compress/zlib/zlib_pmd_private.h |  4 ++
>>  3 files changed, 176 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
>> index c4f67bb..7c2614e 100644
>> --- a/drivers/compress/zlib/zlib_pmd.c
>> +++ b/drivers/compress/zlib/zlib_pmd.c
>
>...
>
>> +             case RTE_COMP_HUFFMAN_DYNAMIC:
>> +                     strategy = Z_HUFFMAN_ONLY;
>
>I don't think this is the suitable value for dynamic compression.
>Z_HUFFMAN_ONLY does not do LZ77. I think the strategy for this case is Z_DEFAULT_STRATEGY.
Ya. you right. It will be changed to DEFAULT_STRATEGY.

Thanks
Shally

>
>> +                     break;
>> +             default:
>> +                     ZLIB_PMD_ERR("Compression strategy not
>> supported\n");
>> +                     return -1;
>> +             }
>
>...
>
>> +++ b/drivers/compress/zlib/zlib_pmd_ops.c
>
>...
>
>> +     struct rte_mempool *mp = rte_mempool_lookup(internals->mp_name);
>
>As said in other email, you can directly get the mempool pointer from internals.
>
>> +     if (rte_mempool_get(mp, zstream)) {
>> +             ZLIB_PMD_ERR(
>> +                             "Couldn't get object from session mempool");
>
>This can be in the same line.
>
>...
>
>> +/** Configure private xform */
>> +static int
>> +zlib_pmd_private_xform_create(struct rte_compressdev *dev,
>> +             const struct rte_comp_xform *xform,
>> +             void **private_xform)
>> +{
>> +     int ret = 0;
>> +
>> +     ret = zlib_pmd_stream_create(dev, xform, private_xform);
>> +     return ret;
>
>This can be written like "return zlib_pmd_stream_create(...);
>
>...
>
>> +             .private_xform_create   = zlib_pmd_private_xform_create,
>> +             .private_xform_free             =
>> zlib_pmd_private_xform_free,
>
>Fix indentation here.
>
>>
>> -             .stream_create  = NULL,
>> -             .stream_free    = NULL
>> +             .stream_create  = zlib_pmd_stream_create,
>> +             .stream_free    = zlib_pmd_stream_free
>>  };

Re: [dpdk-dev] [PATCH v2 5/5] doc: add ZLIB PMD documentation

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:57 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>
> Subject: [PATCH v2 5/5] doc: add ZLIB PMD documentation
> 

Better change title to "add ZLIB PMD guide", to avoid duplication.

> add zlib pmd feature specification and overview documentation
> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> ---
>  MAINTAINERS                               |  2 +
>  doc/guides/compressdevs/features/zlib.ini | 22 ++++++++++
>  doc/guides/compressdevs/zlib.rst          | 68
> +++++++++++++++++++++++++++++++
>  3 files changed, 92 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 448bbe1..1c217b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -854,6 +854,8 @@ F: doc/guides/compressdevs/features/isal.ini
>  ZLIB
>  M: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>  F: drivers/compress/zlib/
> +F: doc/guides/compressdevs/zlib.rst
> +F: doc/guides/compressdevs/features/zlib.ini
> 
>  Eventdev Drivers
>  ----------------
> diff --git a/doc/guides/compressdevs/features/zlib.ini
> b/doc/guides/compressdevs/features/zlib.ini
> new file mode 100644
> index 0000000..bdc0fc4
> --- /dev/null
> +++ b/doc/guides/compressdevs/features/zlib.ini
> @@ -0,0 +1,22 @@
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +; Supported features of 'ZLIB' compression driver.
> +;
> +[Features]
> +HW Accelerated =
> +CPU SSE        =
> +CPU AVX        =
> +CPU AVX2       =
> +CPU AVX512     =
> +CPU NEON       =
> +Stateful       =
> +By-Pass        =
> +Chained mbufs  =
> +Deflate        = Y
> +LZS            =
> +Adler32        =
> +Crc32          =
> +Adler32&Crc32  =
> +Fixed          = Y
> +Dynamic        = Y

No need to include the features that the PMD does not support
(plus, some of those names have been changed).

> diff --git a/doc/guides/compressdevs/zlib.rst
> b/doc/guides/compressdevs/zlib.rst
> new file mode 100644
> index 0000000..7dd5c74
> --- /dev/null
> +++ b/doc/guides/compressdevs/zlib.rst

...

> +
> +Installation
> +------------
> +
> +* To build DPDK with ZLIB library, the user is required to download the ``libz``

Zlib?

> library.
> +* Use following command for installation.
> +
> +*  For Fedora users ::
> +  yum install zlib-devel
> +*  For ubuntu users ::

Ubuntu

> +  apt-get install zlib1g-dev

I am seeing a build error because of these lines:

doc/guides/compressdevs/zlib.rst:41: WARNING: Literal block expected; none found.
doc/guides/compressdevs/zlib.rst:41: WARNING: Bullet list ends without a blank line; unexpected unindent.
doc/guides/compressdevs/zlib.rst:42: WARNING: Block quote ends without a blank line; unexpected unindent.
doc/guides/compressdevs/zlib.rst:43: WARNING: Literal block expected; none found.
doc/guides/compressdevs/zlib.rst:43: WARNING: Bullet list ends without a blank line; unexpected unindent.
doc/guides/compressdevs/zlib.rst: WARNING: document isn't included in any toctree

> +
> +* Once downloaded, the user needs to build the library.
> +
> +* make can  be used to install the library on their system, before building
> DPDK::
> +
> +    make
> +    sudo make install

This is not required for packages in distros. I assume that this is then for building from source code.
In that case, I think it would be good to differentiate between the two scenarios.

Re: [dpdk-dev] [PATCH v2 5/5] doc: add ZLIB PMD documentation

[Verma, Shally]

>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>Sent: 11 July 2018 19:32
>To: Verma, Shally <Shally.Verma@cavium.com>
>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
><Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>
>Subject: RE: [PATCH v2 5/5] doc: add ZLIB PMD documentation
>
>External Email
>
>> -----Original Message-----
>> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
>> Sent: Monday, July 2, 2018 5:57 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
>> mchalla@caviumnetworks.com; Sunila Sahu
>> <sunila.sahu@caviumnetworks.com>; Ashish Gupta
>> <ashish.gupta@caviumnetworks.com>
>> Subject: [PATCH v2 5/5] doc: add ZLIB PMD documentation
>>
>
>Better change title to "add ZLIB PMD guide", to avoid duplication.
>
>> add zlib pmd feature specification and overview documentation
>>
>> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
>> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
>> ---
>>  MAINTAINERS                               |  2 +
>>  doc/guides/compressdevs/features/zlib.ini | 22 ++++++++++
>>  doc/guides/compressdevs/zlib.rst          | 68
>> +++++++++++++++++++++++++++++++
>>  3 files changed, 92 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 448bbe1..1c217b0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -854,6 +854,8 @@ F: doc/guides/compressdevs/features/isal.ini
>>  ZLIB
>>  M: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>>  F: drivers/compress/zlib/
>> +F: doc/guides/compressdevs/zlib.rst
>> +F: doc/guides/compressdevs/features/zlib.ini
>>
>>  Eventdev Drivers
>>  ----------------
>> diff --git a/doc/guides/compressdevs/features/zlib.ini
>> b/doc/guides/compressdevs/features/zlib.ini
>> new file mode 100644
>> index 0000000..bdc0fc4
>> --- /dev/null
>> +++ b/doc/guides/compressdevs/features/zlib.ini
>> @@ -0,0 +1,22 @@
>> +;
>> +; Refer to default.ini for the full list of available PMD features.
>> +;
>> +; Supported features of 'ZLIB' compression driver.
>> +;
>> +[Features]
>> +HW Accelerated =
>> +CPU SSE        =
>> +CPU AVX        =
>> +CPU AVX2       =
>> +CPU AVX512     =
>> +CPU NEON       =
>> +Stateful       =
>> +By-Pass        =
>> +Chained mbufs  =
>> +Deflate        = Y
>> +LZS            =
>> +Adler32        =
>> +Crc32          =
>> +Adler32&Crc32  =
>> +Fixed          = Y
>> +Dynamic        = Y
>
>No need to include the features that the PMD does not support
>(plus, some of those names have been changed).
>
You mean remove entry with empty values like >> +Crc32          =

>> diff --git a/doc/guides/compressdevs/zlib.rst
>> b/doc/guides/compressdevs/zlib.rst
>> new file mode 100644
>> index 0000000..7dd5c74
>> --- /dev/null
>> +++ b/doc/guides/compressdevs/zlib.rst
>
>...
>
>> +
>> +Installation
>> +------------
>> +
>> +* To build DPDK with ZLIB library, the user is required to download the ``libz``
>
>Zlib?
Library is available with name libz .

Thanks
Shally
>
>> library.
>> +* Use following command for installation.
>> +
>> +*  For Fedora users ::
>> +  yum install zlib-devel
>> +*  For ubuntu users ::
>
>Ubuntu
>
>> +  apt-get install zlib1g-dev
>
>I am seeing a build error because of these lines:
>
>doc/guides/compressdevs/zlib.rst:41: WARNING: Literal block expected; none found.
>doc/guides/compressdevs/zlib.rst:41: WARNING: Bullet list ends without a blank line; unexpected unindent.
>doc/guides/compressdevs/zlib.rst:42: WARNING: Block quote ends without a blank line; unexpected unindent.
>doc/guides/compressdevs/zlib.rst:43: WARNING: Literal block expected; none found.
>doc/guides/compressdevs/zlib.rst:43: WARNING: Bullet list ends without a blank line; unexpected unindent.
>doc/guides/compressdevs/zlib.rst: WARNING: document isn't included in any toctree
>
>> +
>> +* Once downloaded, the user needs to build the library.
>> +
>> +* make can  be used to install the library on their system, before building
>> DPDK::
>> +
>> +    make
>> +    sudo make install
>
>This is not required for packages in distros. I assume that this is then for building from source code.
>In that case, I think it would be good to differentiate between the two scenarios.
>
>

Re: [dpdk-dev] [PATCH v2 5/5] doc: add ZLIB PMD documentation

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Wednesday, July 11, 2018 6:16 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; Athreya, Narayana Prasad
> <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
> <Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>;
> Gupta, Ashish <Ashish.Gupta@cavium.com>
> Subject: RE: [PATCH v2 5/5] doc: add ZLIB PMD documentation
> 
> 
> 
> >-----Original Message-----
> >From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
> >Sent: 11 July 2018 19:32
> >To: Verma, Shally <Shally.Verma@cavium.com>
> >Cc: dev@dpdk.org; Athreya, Narayana Prasad
> ><NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
> ><Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>;
> >Gupta, Ashish <Ashish.Gupta@cavium.com>
> >Subject: RE: [PATCH v2 5/5] doc: add ZLIB PMD documentation
> >
> >External Email
> >
> >> -----Original Message-----
> >> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> >> Sent: Monday, July 2, 2018 5:57 PM
> >> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> >> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> >> mchalla@caviumnetworks.com; Sunila Sahu
> >> <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> >> <ashish.gupta@caviumnetworks.com>
> >> Subject: [PATCH v2 5/5] doc: add ZLIB PMD documentation
> >>
> >
> >Better change title to "add ZLIB PMD guide", to avoid duplication.
> >
> >> add zlib pmd feature specification and overview documentation
> >>
> >> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> >> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> >> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> >> ---
> >>  MAINTAINERS                               |  2 +
> >>  doc/guides/compressdevs/features/zlib.ini | 22 ++++++++++
> >>  doc/guides/compressdevs/zlib.rst          | 68
> >> +++++++++++++++++++++++++++++++
> >>  3 files changed, 92 insertions(+)
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS index 448bbe1..1c217b0 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -854,6 +854,8 @@ F: doc/guides/compressdevs/features/isal.ini
> >>  ZLIB
> >>  M: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> >>  F: drivers/compress/zlib/
> >> +F: doc/guides/compressdevs/zlib.rst
> >> +F: doc/guides/compressdevs/features/zlib.ini
> >>
> >>  Eventdev Drivers
> >>  ----------------
> >> diff --git a/doc/guides/compressdevs/features/zlib.ini
> >> b/doc/guides/compressdevs/features/zlib.ini
> >> new file mode 100644
> >> index 0000000..bdc0fc4
> >> --- /dev/null
> >> +++ b/doc/guides/compressdevs/features/zlib.ini
> >> @@ -0,0 +1,22 @@
> >> +;
> >> +; Refer to default.ini for the full list of available PMD features.
> >> +;
> >> +; Supported features of 'ZLIB' compression driver.
> >> +;
> >> +[Features]
> >> +HW Accelerated =
> >> +CPU SSE        =
> >> +CPU AVX        =
> >> +CPU AVX2       =
> >> +CPU AVX512     =
> >> +CPU NEON       =
> >> +Stateful       =
> >> +By-Pass        =
> >> +Chained mbufs  =
> >> +Deflate        = Y
> >> +LZS            =
> >> +Adler32        =
> >> +Crc32          =
> >> +Adler32&Crc32  =
> >> +Fixed          = Y
> >> +Dynamic        = Y
> >
> >No need to include the features that the PMD does not support (plus,
> >some of those names have been changed).
> >
> You mean remove entry with empty values like >> +Crc32          =

Exactly.

Re: [dpdk-dev] [PATCH v2 4/5] compress/zlib: add enq deq apis

[Verma, Shally]

>-----Original Message-----
>From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
>Sent: 11 July 2018 18:56
>To: Verma, Shally <Shally.Verma@cavium.com>
>Cc: dev@dpdk.org; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
><Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish
><Ashish.Gupta@cavium.com>
>Subject: RE: [PATCH v2 4/5] compress/zlib: add enq deq apis
>
>External Email
>
>Hi,
>
>> -----Original Message-----
>> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
>> Sent: Monday, July 2, 2018 5:57 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
>> mchalla@caviumnetworks.com; Sunila Sahu <ssahu@caviumnetworks.com>;
>> Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish Gupta
>> <ashish.gupta@caviumnetworks.com>
>> Subject: [PATCH v2 4/5] compress/zlib: add enq deq apis
>
>Better retitle to "support burst enqueue/dequeue".
>
>>
>> From: Sunila Sahu <ssahu@caviumnetworks.com>
>>
>> implement enqueue and dequeue apis
>
>This should start with capital letter, but this is probably not needed anyway.
>
>>
>> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
>> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
>> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
>> ---
>>  drivers/compress/zlib/zlib_pmd.c | 238
>> ++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 237 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
>> index 7c2614e..aef23de 100644
>> --- a/drivers/compress/zlib/zlib_pmd.c
>> +++ b/drivers/compress/zlib/zlib_pmd.c
>> @@ -6,7 +6,198 @@
>>  #include <rte_common.h>
>>  #include "zlib_pmd_private.h"
>>
>> -/** Parse comp xform and set private xform/stream parameters */
>> +/** Compute next mbuf in the list, assign data buffer and len */
>> +#define COMPUTE_BUF(mbuf, data, len)         \
>> +             ((mbuf = mbuf->next) ?          \
>> +             (data = rte_pktmbuf_mtod(mbuf, uint8_t *)),     \
>> +             (len = rte_pktmbuf_data_len(mbuf)) : 0)
>> +
>> +/** Set op->status to appropriate flag if we run out of mbuf */
>> +#define COMPUTE_DST_BUF(mbuf, dst, dlen)             \
>> +             ((COMPUTE_BUF(mbuf, dst, dlen)) ? 1 :   \
>> +             (!(op->status =                         \
>
>I see and build error here:
>
>drivers/compress/zlib/zlib_pmd.c(81): error #187: use of "=" where "==" may have been intended
>                        COMPUTE_DST_BUF(mbuf_dst,  strm->next_out,
>It is a bit confusing macro, but afaik, you should pass op if you want to modify the status
>(I suggest making this more readable).
>
>
>> +             RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED)))
>> +
>> +static void
>> +process_zlib_deflate(struct rte_comp_op *op, z_stream *strm) {
>> +     int ret, flush, fin_flush;
>> +     struct rte_mbuf *mbuf_src = op->m_src;
>> +     struct rte_mbuf *mbuf_dst = op->m_dst;
>> +
>> +     switch (op->flush_flag) {
>> +     case RTE_COMP_FLUSH_FULL:
>> +     case RTE_COMP_FLUSH_FINAL:
>> +             fin_flush = Z_FINISH;
>> +             break;
>> +     default:
>> +             op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
>> +             ZLIB_PMD_ERR("\nInvalid flush value");
>
>Better to have "\n" at the end for consistency.
>
>> +
>> +     }
>> +
>> +     if (unlikely(!strm)) {
>> +             op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
>> +             ZLIB_PMD_ERR("\nInvalid z_stream");
>> +             return;
>> +     }
>> +     /* Update z_stream with the inputs provided by application */
>> +     strm->next_in = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *,
>> +                     op->src.offset);
>> +
>> +     strm->avail_in = rte_pktmbuf_data_len(mbuf_src) - op->src.offset;
>
>Shouldn't this be "op->src.length"?
If packet is segmented, then src.length will give wrong o/p. Though we are not claiming segmented packet support. But this is safer way to do.

>
>> +
>> +     strm->next_out = rte_pktmbuf_mtod_offset(mbuf_dst, uint8_t *,
>> +                     op->dst.offset);
>> +
>> +     strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
>> +
>> +     /* Set flush value to NO_FLUSH unless it is last mbuf */
>> +     flush = Z_NO_FLUSH;
>> +     /* Initialize status to SUCCESS */
>> +     op->status = RTE_COMP_OP_STATUS_SUCCESS;
>> +
>
>...
>
>> +     /* Update source buffer to next mbuf
>> +      * Exit if op->status is not SUCCESS or input buffers are fully consumed
>> +      */
>> +     } while ((op->status == RTE_COMP_OP_STATUS_SUCCESS) &&
>> +             (COMPUTE_BUF(mbuf_src, strm->next_in, strm->avail_in)));
>
>It looks like you support Scatter Gather here, but according to the documentation, you don't support it...
>
Yes. right now, we don't claim its support as we couldn't test it.

>> +
>> +def_end:
>> +     /* Update op stats */
>> +     switch (op->status) {
>> +     case RTE_COMP_OP_STATUS_SUCCESS:
>> +             op->consumed += strm->total_in;
>
>I see a compilation error with gcc:
>
>drivers/compress/zlib/zlib_pmd.c:166:16: error:
>this statement may fall through [-Werror=implicit-fallthrough=]
>   op->consumed += strm->total_in;
>   ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
>drivers/compress/zlib/zlib_pmd.c:167:2: note: here
>  case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
>  ^~~~
>
>If the intention is to fall-through, you should add a comment before the next case.
>
Ok. But then my view is it shouldn't be taken as error here. Is it we don't want fall-through model?

>> +     case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
>> +             op->produced += strm->total_out;
>> +             break;
>> +     default:
>> +             ZLIB_PMD_ERR("stats not updated for status:%d\n",
>> +                             op->status);
>> +     }
>> +
>> +     deflateReset(strm);
>> +}
>> +
>
>...
>
>> +
>> +/** Process comp operation for mbuf */
>> +static inline int
>> +process_zlib_op(struct zlib_qp *qp, struct rte_comp_op *op) {
>> +     struct zlib_stream *stream;
>> +
>> +     if ((op->op_type == RTE_COMP_OP_STATEFUL) ||
>> +             op->src.offset > rte_pktmbuf_data_len(op->m_src) ||
>> +             op->dst.offset > rte_pktmbuf_data_len(op->m_dst)) {
>
>An extra indentation is needed, so it is easy to distinguish between the if statement and the body.
>You should also check if (src.offset + src.length > rte_pktmbuf_data_len(op->m_src))

Should it be checked against pktmbuf_data_len() or pktmbuf_pkt_len()?!

>
>> +             op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
>> +             ZLIB_PMD_ERR("\nInvalid source or destination buffers or "
>> +                             "invalid Operation requested");
>
>Better to include the "\n" at the end.
>
>> +     } else {
>> +             stream = &((struct zlib_priv_xform *)op->private_xform)-
>> >stream;
>
>I think it is more readable to have this line split into two:
>First get the private xform and then get zlib_stream pointer.
>
>> +             stream->comp(op, &stream->strm);
>> +     }
>> +     /* whatever is out of op, put it into completion queue with
>> +      * its status
>> +      */
>> +     return rte_ring_enqueue(qp->processed_pkts, (void *)op); }
>> +
>
>...
>
>> +static uint16_t
>> +zlib_pmd_enqueue_burst(void *queue_pair,
>> +                     struct rte_comp_op **ops, uint16_t nb_ops) {
>> +     struct zlib_qp *qp = queue_pair;
>> +     int ret, i;
>> +     int enqd = 0;
>
>"i" and "enqd" variables should be uint16_t.
>
>> +     for (i = 0; i < nb_ops; i++) {
>> +             ret = process_zlib_op(qp, ops[i]);
>
>I think you should check if there is enough space in the ring for all the operations, to save time.
>If there is not, then the PMD would modify the operation unnecessarily and waste a lot of time.
>Then, with that check, you can just process the operations that can fit, looping until minimum between
>nb_ops and space available in the ring.
I doubt if I should do that. If there's an application with dequeue only thread, then at one point, we may see it 
full and another space available. Since this PMD is using lockless rings, it can be made flexible to produce as many it can.

>
>> +             if (unlikely(ret < 0)) {
>> +                     /* increment count if failed to push to completion
>> +                      * queue
>> +                      */
>> +                     qp->qp_stats.enqueue_err_count++;
>
>I think this variable should be used when there was an error in the operation but
>it was still be enqueued (because there was space in the ring).
>So you can increase this count in process_zlib_op when there is an error.
>
So what is exact purpose of this stat? is  enqueue_err_count supposed to give number of failed operations in processed queue?
Or, just number of operations that couldn't be enqueued/dequeued for processing?


>> +             } else {
>> +                     qp->qp_stats.enqueued_count++;
>
>I think this should be equal to all the operations enqueued (with and without error).
Yes. Right now it does that except for the cases, where op couldn't be pushed into as processing queue, and user cannot deque it.

Thanks
Shally
>
>> +                     enqd++;
>> +             }
>> +     }
>> +     return enqd;
>> +}

Re: [dpdk-dev] [PATCH v2 4/5] compress/zlib: add enq deq apis

[De Lara Guarch, Pablo]

Hi Shally,

Sorry for the delay. Comments inline.

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Thursday, July 12, 2018 6:46 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; Athreya, Narayana Prasad
> <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
> <Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Sahu,
> Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>
> Subject: RE: [PATCH v2 4/5] compress/zlib: add enq deq apis
> 
> 
> 
> >-----Original Message-----
> >From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
> >Sent: 11 July 2018 18:56
> >To: Verma, Shally <Shally.Verma@cavium.com>
> >Cc: dev@dpdk.org; Athreya, Narayana Prasad
> ><NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
> ><Mahipal.Challa@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>;
> >Sahu, Sunila <Sunila.Sahu@cavium.com>; Gupta, Ashish
> ><Ashish.Gupta@cavium.com>
> >Subject: RE: [PATCH v2 4/5] compress/zlib: add enq deq apis
> >
> >External Email
> >
> >Hi,
> >
> >> -----Original Message-----
> >> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> >> Sent: Monday, July 2, 2018 5:57 PM
> >> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> >> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> >> mchalla@caviumnetworks.com; Sunila Sahu <ssahu@caviumnetworks.com>;
> >> Sunila Sahu <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> >> <ashish.gupta@caviumnetworks.com>
> >> Subject: [PATCH v2 4/5] compress/zlib: add enq deq apis
> >
> >Better retitle to "support burst enqueue/dequeue".
> >
> >>
> >> From: Sunila Sahu <ssahu@caviumnetworks.com>
> >>
> >> implement enqueue and dequeue apis
> >
> >This should start with capital letter, but this is probably not needed anyway.
> >
> >>
> >> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> >> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> >> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> >> ---
> >>  drivers/compress/zlib/zlib_pmd.c | 238
> >> ++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 237 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/compress/zlib/zlib_pmd.c
> >> b/drivers/compress/zlib/zlib_pmd.c
> >> index 7c2614e..aef23de 100644
> >> --- a/drivers/compress/zlib/zlib_pmd.c
> >> +++ b/drivers/compress/zlib/zlib_pmd.c
> >> @@ -6,7 +6,198 @@
> >>  #include <rte_common.h>
> >>  #include "zlib_pmd_private.h"
> >>
> >> -/** Parse comp xform and set private xform/stream parameters */
> >> +/** Compute next mbuf in the list, assign data buffer and len */
> >> +#define COMPUTE_BUF(mbuf, data, len)         \
> >> +             ((mbuf = mbuf->next) ?          \
> >> +             (data = rte_pktmbuf_mtod(mbuf, uint8_t *)),     \
> >> +             (len = rte_pktmbuf_data_len(mbuf)) : 0)
> >> +
> >> +/** Set op->status to appropriate flag if we run out of mbuf */
> >> +#define COMPUTE_DST_BUF(mbuf, dst, dlen)             \
> >> +             ((COMPUTE_BUF(mbuf, dst, dlen)) ? 1 :   \
> >> +             (!(op->status =                         \
> >
> >I see and build error here:
> >
> >drivers/compress/zlib/zlib_pmd.c(81): error #187: use of "=" where "==" may
> have been intended
> >                        COMPUTE_DST_BUF(mbuf_dst,  strm->next_out, It
> >is a bit confusing macro, but afaik, you should pass op if you want to
> >modify the status (I suggest making this more readable).
> >
> >
> >> +             RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED)))
> >> +
> >> +static void
> >> +process_zlib_deflate(struct rte_comp_op *op, z_stream *strm) {
> >> +     int ret, flush, fin_flush;
> >> +     struct rte_mbuf *mbuf_src = op->m_src;
> >> +     struct rte_mbuf *mbuf_dst = op->m_dst;
> >> +
> >> +     switch (op->flush_flag) {
> >> +     case RTE_COMP_FLUSH_FULL:
> >> +     case RTE_COMP_FLUSH_FINAL:
> >> +             fin_flush = Z_FINISH;
> >> +             break;
> >> +     default:
> >> +             op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> >> +             ZLIB_PMD_ERR("\nInvalid flush value");
> >
> >Better to have "\n" at the end for consistency.
> >
> >> +
> >> +     }
> >> +
> >> +     if (unlikely(!strm)) {
> >> +             op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> >> +             ZLIB_PMD_ERR("\nInvalid z_stream");
> >> +             return;
> >> +     }
> >> +     /* Update z_stream with the inputs provided by application */
> >> +     strm->next_in = rte_pktmbuf_mtod_offset(mbuf_src, uint8_t *,
> >> +                     op->src.offset);
> >> +
> >> +     strm->avail_in = rte_pktmbuf_data_len(mbuf_src) -
> >> + op->src.offset;
> >
> >Shouldn't this be "op->src.length"?
> If packet is segmented, then src.length will give wrong o/p. Though we are not
> claiming segmented packet support. But this is safer way to do.

Right, I was assuming that only single segment buffers were supported.
Regardless you take into account single or multi segment buffers,
src.length needs to be taken into consideration.

> 
> >
> >> +
> >> +     strm->next_out = rte_pktmbuf_mtod_offset(mbuf_dst, uint8_t *,
> >> +                     op->dst.offset);
> >> +
> >> +     strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) -
> >> + op->dst.offset;
> >> +
> >> +     /* Set flush value to NO_FLUSH unless it is last mbuf */
> >> +     flush = Z_NO_FLUSH;
> >> +     /* Initialize status to SUCCESS */
> >> +     op->status = RTE_COMP_OP_STATUS_SUCCESS;
> >> +
> >
> >...
> >
> >> +     /* Update source buffer to next mbuf
> >> +      * Exit if op->status is not SUCCESS or input buffers are fully consumed
> >> +      */
> >> +     } while ((op->status == RTE_COMP_OP_STATUS_SUCCESS) &&
> >> +             (COMPUTE_BUF(mbuf_src, strm->next_in,
> >> + strm->avail_in)));
> >
> >It looks like you support Scatter Gather here, but according to the
> documentation, you don't support it...
> >
> Yes. right now, we don't claim its support as we couldn't test it.

Tests for this were sent in this release:
http://patches.dpdk.org/patch/42137/

If you think you support it, you should try the tests and set the flags.

> 
> >> +
> >> +def_end:
> >> +     /* Update op stats */
> >> +     switch (op->status) {
> >> +     case RTE_COMP_OP_STATUS_SUCCESS:
> >> +             op->consumed += strm->total_in;
> >
> >I see a compilation error with gcc:
> >
> >drivers/compress/zlib/zlib_pmd.c:166:16: error:
> >this statement may fall through [-Werror=implicit-fallthrough=]
> >   op->consumed += strm->total_in;
> >   ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> >drivers/compress/zlib/zlib_pmd.c:167:2: note: here
> >  case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
> >  ^~~~
> >
> >If the intention is to fall-through, you should add a comment before the next
> case.
> >
> Ok. But then my view is it shouldn't be taken as error here. Is it we don't want
> fall-through model?

Basically the compiler warns you that you could have forgotten a break statement.
If you want to fall-though, you should add "/* Fall-through */ before the next case.


> 
> >> +     case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
> >> +             op->produced += strm->total_out;
> >> +             break;
> >> +     default:
> >> +             ZLIB_PMD_ERR("stats not updated for status:%d\n",
> >> +                             op->status);
> >> +     }
> >> +
> >> +     deflateReset(strm);
> >> +}
> >> +
> >
> >...
> >
> >> +
> >> +/** Process comp operation for mbuf */ static inline int
> >> +process_zlib_op(struct zlib_qp *qp, struct rte_comp_op *op) {
> >> +     struct zlib_stream *stream;
> >> +
> >> +     if ((op->op_type == RTE_COMP_OP_STATEFUL) ||
> >> +             op->src.offset > rte_pktmbuf_data_len(op->m_src) ||
> >> +             op->dst.offset > rte_pktmbuf_data_len(op->m_dst)) {
> >
> >An extra indentation is needed, so it is easy to distinguish between the if
> statement and the body.
> >You should also check if (src.offset + src.length >
> >rte_pktmbuf_data_len(op->m_src))
> 
> Should it be checked against pktmbuf_data_len() or pktmbuf_pkt_len()?!

Pktlen if you consider multi-segment (which I thought you weren't supporting it).
For a single segment, these two values are the same.

> 
> >
> >> +             op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> >> +             ZLIB_PMD_ERR("\nInvalid source or destination buffers or "
> >> +                             "invalid Operation requested");
> >
> >Better to include the "\n" at the end.
> >
> >> +     } else {
> >> +             stream = &((struct zlib_priv_xform
> >> + *)op->private_xform)-
> >> >stream;
> >
> >I think it is more readable to have this line split into two:
> >First get the private xform and then get zlib_stream pointer.
> >
> >> +             stream->comp(op, &stream->strm);
> >> +     }
> >> +     /* whatever is out of op, put it into completion queue with
> >> +      * its status
> >> +      */
> >> +     return rte_ring_enqueue(qp->processed_pkts, (void *)op); }
> >> +
> >
> >...
> >
> >> +static uint16_t
> >> +zlib_pmd_enqueue_burst(void *queue_pair,
> >> +                     struct rte_comp_op **ops, uint16_t nb_ops) {
> >> +     struct zlib_qp *qp = queue_pair;
> >> +     int ret, i;
> >> +     int enqd = 0;
> >
> >"i" and "enqd" variables should be uint16_t.
> >
> >> +     for (i = 0; i < nb_ops; i++) {
> >> +             ret = process_zlib_op(qp, ops[i]);
> >
> >I think you should check if there is enough space in the ring for all the
> operations, to save time.
> >If there is not, then the PMD would modify the operation unnecessarily and
> waste a lot of time.
> >Then, with that check, you can just process the operations that can
> >fit, looping until minimum between nb_ops and space available in the ring.
> I doubt if I should do that. If there's an application with dequeue only thread,
> then at one point, we may see it full and another space available. Since this PMD
> is using lockless rings, it can be made flexible to produce as many it can.

Right, I see your point. My concern is that if there is no space in the ring,
you would have modified the operation. The expectation is that, if an operation cannot be enqueued,
you can retry and enqueue it again, but if you have modified it, that won't be possible.

An alternative is to move the processing into the dequeue function.

> 
> >
> >> +             if (unlikely(ret < 0)) {
> >> +                     /* increment count if failed to push to completion
> >> +                      * queue
> >> +                      */
> >> +                     qp->qp_stats.enqueue_err_count++;
> >
> >I think this variable should be used when there was an error in the
> >operation but it was still be enqueued (because there was space in the ring).
> >So you can increase this count in process_zlib_op when there is an error.
> >
> So what is exact purpose of this stat? is  enqueue_err_count supposed to give
> number of failed operations in processed queue?
> Or, just number of operations that couldn't be enqueued/dequeued for
> processing?

For me, it is the number of operations that were enqueued, but that contains an error.
This actually raises a concern. For software devices, operations with errors can be enqueued and not processed,
but for hardware devices, this might not work (arguments could be invalid) and then operation couldn't be "enqueued".
In that case, I believe that the faulty operations and the next ones won't be enqueued into the device,
making it inconsistent with software devices.

Any opinions on this?

> 
> 
> >> +             } else {
> >> +                     qp->qp_stats.enqueued_count++;
> >
> >I think this should be equal to all the operations enqueued (with and without
> error).
> Yes. Right now it does that except for the cases, where op couldn't be pushed
> into as processing queue, and user cannot deque it.
> 
> Thanks
> Shally
> >
> >> +                     enqd++;
> >> +             }
> >> +     }
> >> +     return enqd;
> >> +}

Re: [dpdk-dev] [PATCH v2 1/5] compress/zlib: add ZLIB PMD support

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Wednesday, July 11, 2018 1:41 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; Athreya, Narayana Prasad
> <NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
> <Mahipal.Challa@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>;
> Sahu, Sunila <Sunila.Sahu@cavium.com>
> Subject: RE: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
> 
> Hi Pablo
> 
> >-----Original Message-----
> >From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
> >Sent: 11 July 2018 17:44
> >To: Verma, Shally <Shally.Verma@cavium.com>
> >Cc: dev@dpdk.org; Athreya, Narayana Prasad
> ><NarayanaPrasad.Athreya@cavium.com>; Challa, Mahipal
> ><Mahipal.Challa@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>;
> >Sahu, Sunila <Sunila.Sahu@cavium.com>
> >Subject: RE: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
> >
> >External Email
> >
> >And the last comments, sorry for the multiple replies.
> 
> No issues.
> 
> >
> >> -----Original Message-----
> >> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> >> Sent: Monday, July 2, 2018 5:57 PM
> >> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> >> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> >> mchalla@caviumnetworks.com; Ashish Gupta
> >> <ashish.gupta@caviumnetworks.com>; Sunila Sahu
> >> <sunila.sahu@caviumnetworks.com>
> >> Subject: [PATCH v2 1/5] compress/zlib: add ZLIB PMD support
> >>
> >> From: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> >>
> >> Add sw zlib pmd support in compressdev driver.
> >> Add device probe and remove support.
> >> Add ZLIB build file support.
> >>
> >> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> >> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> >> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> >
> >...
> >
> >> +++ b/drivers/compress/zlib/zlib_pmd.c
> >
> >...
> >
> >> +static void
> >> +zlib_init_log(void)
> >> +{
> >> +     zlib_logtype_driver = rte_log_register("compress_zlib");
> >
> >The standard for the name of the logtype for PMDs is
> >"pmd.driverType.driverName", so in this case it would be "pmd.compress.zlib".
> >
> >
> >> +     if (zlib_logtype_driver >= 0)
> >> +             rte_log_set_level(zlib_logtype_driver, RTE_LOG_INFO); }
> >> diff --git a/drivers/compress/zlib/zlib_pmd_private.h
> >> b/drivers/compress/zlib/zlib_pmd_private.h
> >> new file mode 100644
> >> index 0000000..d4c80b1
> >> --- /dev/null
> >> +++ b/drivers/compress/zlib/zlib_pmd_private.h
> >
> >...
> >
> >> +#define ZLIB_PMD_INFO(fmt, args...) \
> >> +     ZLIB_PMD_LOG(INFO, fmt, ## args) #define ZLIB_PMD_ERR(fmt,
> >> +args...) \
> >> +     ZLIB_PMD_LOG(ERR, fmt, ## args) #define ZLIB_PMD_WARN(fmt,
> >> +args...) \
> >> +     ZLIB_PMD_LOG(WARNING, fmt, ## args)
> >
> >What do you think of having a single macro ZLIB_LOG(level, fmt, args...)?
> >
> I find it simpler to use ZLIB_PMD_INFO/ERR?DEBUG version . So would prefer to
> stick to them.

Ok for me then.

> 
> Thanks for review.
> Shally
>