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

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

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