RE: [EXTERNAL] [PATCH v2 2/3] compress/zlib: support for dictionaries and PDCP checksum

[Akhil Goyal <gakhil@marvell.com>]

> Support to calculate PDCP checksums. Includes changes to
> pass in dictionaries. Adds support for sync flush
> 
> Signed-off-by: Sameer Vaze <svaze@qti.qualcomm.com>
> ---
> v2:
> * Moved dictionary params to deflate struct
> * Added inflate params struct to union
> 
>  drivers/compress/zlib/zlib_pmd.c         | 173 +++++++++++++++++++++--
>  drivers/compress/zlib/zlib_pmd_private.h |   8 ++
>  2 files changed, 172 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/compress/zlib/zlib_pmd.c b/drivers/compress/zlib/zlib_pmd.c
> index 92e808e78c..931dd39a4c 100644
> --- a/drivers/compress/zlib/zlib_pmd.c
> +++ b/drivers/compress/zlib/zlib_pmd.c
> @@ -4,6 +4,7 @@
> 
>  #include <bus_vdev_driver.h>
>  #include <rte_common.h>
> +#include <stdlib.h>
> 
>  #include "zlib_pmd_private.h"
> 
> @@ -15,6 +16,120 @@
>  		(data = rte_pktmbuf_mtod(mbuf, uint8_t *)),	\
>  		(len = rte_pktmbuf_data_len(mbuf)) : 0)
> 
> +#define BOTTOM_NIBBLE_OF_BYTE 0xf
> +#define TOP_NIBBLE_OF_BYTE 0xf0
> +
> +static void
> +process_zlib_deflate_chksum(struct rte_comp_op *op,
> +		z_stream *strm, enum rte_comp_checksum_type chksum)
> +{
> +	uint32_t dictionary_len = 0;
> +	op->status = RTE_COMP_OP_STATUS_SUCCESS;
> +
> +	switch (chksum) {
> +	case RTE_COMP_CHECKSUM_3GPP_PDCP_UDC:
> +		{
Fix indentation. This brace should be below case.

> +		uint8_t *dictionary = malloc(DEFLATE_MAX_WINDOW_SIZE);
Define variables at start of functions only.

> +
> +		if (!dictionary) {
> +			ZLIB_PMD_ERR("Unable to fetch dictionary");
> +			op->status = RTE_COMP_OP_STATUS_ERROR;
> +			return;
> +		}
> +
> +		if (deflateGetDictionary(strm, dictionary, &dictionary_len)) {
> +			ZLIB_PMD_ERR("Unable to fetch dictionary");
> +			op->status =
> RTE_COMP_OP_STATUS_CHECK_SUM_VALIDATION_FAILED;
> +			free(dictionary);
> +			return;
> +		}
> +


> +		uint32_t dictionary_start = (uint32_t)(*dictionary);
> +		uint32_t dictionary_end = (uint32_t)(*(dictionary +
> dictionary_len - 4));
> +		uint32_t sum = (dictionary_start & 0x0F0F0F0F)

Can you use a macro for 0x0F0F0F0F

> +			+ (dictionary_start & (0xF0F0F0F0 >> 4))
> +			+ (dictionary_end & 0x0F0F0F0F)
> +			+ (dictionary_end & (0xF0F0F0F0 >> 4));
> +		uint8_t *sum_bytes = (uint8_t *)&sum;

Same with above.

> +
> +		op->output_chksum = ~(sum_bytes[0] + sum_bytes[1] +
> sum_bytes[2] + sum_bytes[3])
> +			 & BOTTOM_NIBBLE_OF_BYTE;
> +
> +		free(dictionary);
> +
> +		break;
> +	}
> +	case RTE_COMP_CHECKSUM_NONE:
> +		break;
> +	case RTE_COMP_CHECKSUM_CRC32:
> +	case RTE_COMP_CHECKSUM_ADLER32:
> +	case RTE_COMP_CHECKSUM_CRC32_ADLER32:
> +	default:
> +		ZLIB_PMD_ERR("Checksum not supported");
> +		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> +		return;
> +	}
> +}
> +
> +static void
> +process_zlib_inflate_chksum(struct rte_comp_op *op,
> +		z_stream *strm,
> +		enum rte_comp_checksum_type chksum)
> +{
> +	uint32_t dictionary_len = 0;
> +	op->status = RTE_COMP_OP_STATUS_SUCCESS;
> +
> +	switch (chksum) {
> +	case RTE_COMP_CHECKSUM_3GPP_PDCP_UDC:
> +		{
> +		uint8_t *dictionary = malloc(DEFLATE_MAX_WINDOW_SIZE);
> +
> +		if (!dictionary) {
> +			ZLIB_PMD_ERR("Unable to fetch dictionary");
> +			op->status = RTE_COMP_OP_STATUS_ERROR;
> +			return;
> +		}
> +
> +		if (inflateGetDictionary(strm, dictionary, &dictionary_len)) {
> +			ZLIB_PMD_ERR("Unable to fetch dictionary");
> +			op->status =
> RTE_COMP_OP_STATUS_CHECK_SUM_VALIDATION_FAILED;
> +			free(dictionary);
> +			return;
> +		}
> +
> +		uint32_t dictionary_start = (uint32_t)(*dictionary);
> +		uint32_t dictionary_end = (uint32_t)(*(dictionary +
> dictionary_len - 4));
> +		uint32_t sum = (dictionary_start & 0x0F0F0F0F)
> +			+ (dictionary_start & (0xF0F0F0F0 >> 4))
> +			+ (dictionary_end & 0x0F0F0F0F)
> +			+ (dictionary_end & (0xF0F0F0F0 >> 4));
> +		uint8_t *sum_bytes = (uint8_t *)&sum;
> +
> +		op->output_chksum = ~(sum_bytes[0] + sum_bytes[1] +
> sum_bytes[2] + sum_bytes[3])
> +			 & BOTTOM_NIBBLE_OF_BYTE;
> +
> +		if (op->input_chksum != op->output_chksum) {
> +			ZLIB_PMD_ERR("Checksum does not match");
> +			op->status =
> RTE_COMP_OP_STATUS_CHECK_SUM_VALIDATION_FAILED;
> +			free(dictionary);
> +			return;
> +		}
> +		free(dictionary);
> +
> +		break;
> +		}
> +	case RTE_COMP_CHECKSUM_NONE:
> +		break;
> +	case RTE_COMP_CHECKSUM_CRC32:
> +	case RTE_COMP_CHECKSUM_ADLER32:
> +	case RTE_COMP_CHECKSUM_CRC32_ADLER32:
> +	default:
> +		ZLIB_PMD_ERR("Checksum not supported");
> +		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
> +		return;
> +	}
> +}
> +
>  static void
>  process_zlib_deflate(struct rte_comp_op *op, z_stream *strm)
>  {
> @@ -27,6 +142,9 @@ process_zlib_deflate(struct rte_comp_op *op, z_stream
> *strm)
>  	case RTE_COMP_FLUSH_FINAL:
>  		fin_flush = Z_FINISH;
>  		break;
> +	case RTE_COMP_FLUSH_SYNC:
> +		fin_flush = Z_SYNC_FLUSH;
> +		break;
>  	default:
>  		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
>  		ZLIB_PMD_ERR("Invalid flush value");
> @@ -49,6 +167,9 @@ process_zlib_deflate(struct rte_comp_op *op, z_stream
> *strm)
> 
>  	strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
> 
> +	uLong total_in_at_start = strm->total_in;
> +	uLong total_out_at_start = strm->total_out;
> +
>  	/* Set flush value to NO_FLUSH unless it is last mbuf */
>  	flush = Z_NO_FLUSH;
>  	/* Initialize status to SUCCESS */
> @@ -56,8 +177,8 @@ process_zlib_deflate(struct rte_comp_op *op, z_stream
> *strm)
> 
>  	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);
> +		if ((op->src.length - (strm->total_in - total_in_at_start)) <= strm-
> >avail_in) {
> +			strm->avail_in = (op->src.length - (strm->total_in -
> total_in_at_start));
>  			flush = fin_flush;
>  		}
>  		do {
> @@ -92,17 +213,18 @@ process_zlib_deflate(struct rte_comp_op *op, z_stream
> *strm)
>  	/* Update op stats */
>  	switch (op->status) {
>  	case RTE_COMP_OP_STATUS_SUCCESS:
> -		op->consumed += strm->total_in;
> +		op->consumed += strm->total_in - total_in_at_start;
>  	/* Fall-through */
>  	case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
> -		op->produced += strm->total_out;
> +		op->produced += strm->total_out - total_out_at_start;
>  		break;
>  	default:
>  		ZLIB_PMD_ERR("stats not updated for status:%d",
>  				op->status);
>  	}
> 
> -	deflateReset(strm);
> +	if (op->flush_flag != RTE_COMP_FLUSH_SYNC)
> +		deflateReset(strm);
>  }
> 
>  static void
> @@ -127,6 +249,9 @@ process_zlib_inflate(struct rte_comp_op *op, z_stream
> *strm)
> 
>  	strm->avail_out = rte_pktmbuf_data_len(mbuf_dst) - op->dst.offset;
> 
> +	uLong total_in_at_start = strm->total_in;
> +	uLong total_out_at_start = strm->total_out;
> +
>  	/** Ignoring flush value provided from application for decompression */
>  	flush = Z_NO_FLUSH;
>  	/* initialize status to SUCCESS */
> @@ -178,17 +303,18 @@ process_zlib_inflate(struct rte_comp_op *op,
> z_stream *strm)
>  	/* Update op stats */
>  	switch (op->status) {
>  	case RTE_COMP_OP_STATUS_SUCCESS:
> -		op->consumed += strm->total_in;
> +		op->consumed += strm->total_in - total_in_at_start;
>  	/* Fall-through */
>  	case RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED:
> -		op->produced += strm->total_out;
> +		op->produced += strm->total_out - total_out_at_start;
>  		break;
>  	default:
>  		ZLIB_PMD_ERR("stats not produced for status:%d",
>  				op->status);
>  	}
> 
> -	inflateReset(strm);
> +	if (op->flush_flag != RTE_COMP_FLUSH_SYNC)
> +		inflateReset(strm);
>  }
> 
>  /** Process comp operation for mbuf */
> @@ -203,10 +329,14 @@ process_zlib_op(struct zlib_qp *qp, struct
> rte_comp_op *op)
>  			(op->dst.offset > rte_pktmbuf_data_len(op->m_dst))) {
>  		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
>  		ZLIB_PMD_ERR("Invalid source or destination buffers or "
> -			     "invalid Operation requested");
> +				  "invalid Operation requested");
>  	} else {
>  		private_xform = (struct zlib_priv_xform *)op->private_xform;
>  		stream = &private_xform->stream;
> +		stream->chksum(op, &stream->strm, stream->chksum_type);
> +		if (op->status != RTE_COMP_OP_STATUS_SUCCESS)
> +			return -1;
> +
>  		stream->comp(op, &stream->strm);
>  	}
>  	/* whatever is out of op, put it into completion queue with
> @@ -232,6 +362,7 @@ zlib_set_stream_parameters(const struct
> rte_comp_xform *xform,
>  	case RTE_COMP_COMPRESS:
>  		stream->comp = process_zlib_deflate;
>  		stream->free = deflateEnd;
> +		stream->chksum = process_zlib_deflate_chksum;
>  		/** Compression window bits */
>  		switch (xform->compress.algo) {
>  		case RTE_COMP_ALGO_DEFLATE:
> @@ -281,17 +412,30 @@ zlib_set_stream_parameters(const struct
> rte_comp_xform *xform,
>  			ZLIB_PMD_ERR("Compression strategy not supported");
>  			return -1;
>  		}
> +
> +		/** Checksum used */
> +		stream->chksum_type = xform->compress.chksum;
> +
>  		if (deflateInit2(strm, level,
>  					Z_DEFLATED, wbits,
>  					DEF_MEM_LEVEL, strategy) != Z_OK) {
>  			ZLIB_PMD_ERR("Deflate init failed");
>  			return -1;
>  		}
> +
> +		if (xform->compress.deflate.dictionary) {
> +			if (deflateSetDictionary(strm, xform-
> >compress.deflate.dictionary,
> +					xform-
> >compress.deflate.dictionary_len)) {
> +				ZLIB_PMD_ERR("Deflate set dictionary failed");
> +				return -1;
> +			}
> +		}
>  		break;
> 
>  	case RTE_COMP_DECOMPRESS:
>  		stream->comp = process_zlib_inflate;
>  		stream->free = inflateEnd;
> +		stream->chksum = process_zlib_inflate_chksum;
>  		/** window bits */
>  		switch (xform->decompress.algo) {
>  		case RTE_COMP_ALGO_DEFLATE:
> @@ -302,10 +446,21 @@ zlib_set_stream_parameters(const struct
> rte_comp_xform *xform,
>  			return -1;
>  		}
> 
> +		/** Checksum used */
> +		stream->chksum_type = xform->decompress.chksum;
> +
>  		if (inflateInit2(strm, wbits) != Z_OK) {
>  			ZLIB_PMD_ERR("Inflate init failed");
>  			return -1;
>  		}
> +
> +		if (xform->decompress.inflate.dictionary) {
> +			if (inflateSetDictionary(strm, xform-
> >decompress.inflate.dictionary,
> +					xform-
> >decompress.inflate.dictionary_len)) {
> +				ZLIB_PMD_ERR("inflate set dictionary failed");
> +				return -1;
> +			}
> +		}
>  		break;
>  	default:
>  		return -1;
> diff --git a/drivers/compress/zlib/zlib_pmd_private.h
> b/drivers/compress/zlib/zlib_pmd_private.h
> index fd8c4c55a4..99bfdc9a76 100644
> --- a/drivers/compress/zlib/zlib_pmd_private.h
> +++ b/drivers/compress/zlib/zlib_pmd_private.h
> @@ -46,6 +46,10 @@ typedef void (*comp_func_t)(struct rte_comp_op *op,
> z_stream *strm);
> 
>  typedef int (*comp_free_t)(z_stream *strm);
> 
> +typedef void (*chksum_func_t)
> +		(struct rte_comp_op *op, z_stream *strm, enum
> rte_comp_checksum_type chksum);
> +
> +

Remove extra line.

>  /** ZLIB Stream structure */
>  struct __rte_cache_aligned zlib_stream {
>  	z_stream strm;
> @@ -54,6 +58,10 @@ struct __rte_cache_aligned zlib_stream {
>  	/**< Operation (compression/decompression) */
>  	comp_free_t free;
>  	/**< Free Operation (compression/decompression) */
> +	chksum_func_t chksum;
> +	/**< Checksum Operation (compression/decompression) */
> +	enum rte_comp_checksum_type chksum_type;
> +	/**< Type of checksum to generate on the uncompressed data */
>  };
> 
>  /** ZLIB private xform structure */
> --
> 2.31.1