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

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

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); }