Re: [PATCH v4 08/10] compress/isal: add ISA-L compression functionality

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

Hi Lee,

> -----Original Message-----
> From: Daly, Lee
> Sent: Saturday, April 28, 2018 12:39 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Tucker, Greg B
> <greg.b.tucker@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>; Trahe,
> Fiona <fiona.trahe@intel.com>; Daly, Lee <lee.daly@intel.com>
> Subject: [PATCH v4 08/10] compress/isal: add ISA-L compression functionality
> 
> Adds compression functionality, this sets internal ISA-L structures, provides input
> & output mbuf addresses, executes compression, which ISA-L calls deflate, and
> finally error checks.
> 
> Signed-off-by: Lee Daly <lee.daly@intel.com>
> ---
>  drivers/compress/isal/isal_compress_pmd.c         | 93
> ++++++++++++++++++++++-
>  drivers/compress/isal/isal_compress_pmd_ops.c     | 28 +++++++
>  drivers/compress/isal/isal_compress_pmd_private.h |  2 +
>  3 files changed, 121 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/compress/isal/isal_compress_pmd.c
> b/drivers/compress/isal/isal_compress_pmd.c
> index b13822a..e09fd3f 100644
> --- a/drivers/compress/isal/isal_compress_pmd.c
> +++ b/drivers/compress/isal/isal_compress_pmd.c
> @@ -6,6 +6,7 @@
>  #include <rte_bus_vdev.h>
>  #include <rte_common.h>
>  #include <rte_malloc.h>
> +#include <rte_mbuf.h>
>  #include <rte_compressdev_pmd.h>
> 
>  #include "isal_compress_pmd_private.h"
> @@ -208,14 +209,102 @@ isal_comp_set_priv_xform_parameters(struct
> isal_priv_xform *priv_xform,
>  	return 0;
>  }
> 
> +/* Stateless Compression Function */
> +static int
> +process_isal_deflate(struct rte_comp_op *op, struct isal_comp_qp *qp,
> +		struct isal_priv_xform *priv_xform)
> +{
> +	int ret = 0;
> +	op->status = RTE_COMP_OP_STATUS_SUCCESS;
> +
> +	/* Required due to init clearing level_buf */
> +	uint8_t *temp_level_buf = qp->stream->level_buf;
> +
> +	/* Initialize compression stream */
> +	isal_deflate_stateless_init(qp->stream);
> +
> +	qp->stream->level_buf = temp_level_buf;
> +
> +	qp->stream->flush = NO_FLUSH;

Worth putting a comment here saying why you are not using the flush flag from op.

> +
> +	/* Set op checksum, none by default */
> +	qp->stream->gzip_flag = priv_xform->compress.chksum;
> +

...

> +	}
> +
> +	if (ret != COMP_OK) {
> +		op->status = RTE_COMP_OP_STATUS_ERROR;
> +		return ret;
> +	}
> +
> +	op->consumed = op->src.length - qp->stream->avail_in;

Could you use qp->stream->total_in here? Looks simpler.