[v4,1/3] compress/isal: enable checksum support in driver

Message ID 1544781564-63598-1-git-send-email-lee.daly@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series [v4,1/3] compress/isal: enable checksum support in driver |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Daly, Lee Dec. 14, 2018, 9:59 a.m. UTC
  This patch adds checksum support in the ISA-L  PMD for both compression
and decompression.
CRC32 is supported as well as Adler32.

V2:
Documentation Changes
V3:
Added Release note
V4:
Removed dependency on offset unit test

Signed-off-by: Lee Daly <lee.daly@intel.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>
---
 drivers/compress/isal/isal_compress_pmd.c     | 80 ++++++++++++++++++++++-----
 drivers/compress/isal/isal_compress_pmd_ops.c |  4 +-
 2 files changed, 69 insertions(+), 15 deletions(-)
  

Comments

Varghese, Vipin Dec. 17, 2018, 8:19 a.m. UTC | #1
Hi Lee,

Apologies if the logic is already done for my query

snipped
> +		/* Set private xform checksum */
> +		switch (xform->compress.chksum) {
> +		/* Raw deflate by default */
Does the user have to choose RTE_COMP_CHECKSUM_NONE while creating compress/isal instance?

> +		case(RTE_COMP_CHECKSUM_NONE):
> +			priv_xform->compress.chksum = IGZIP_DEFLATE;
> +			break;
> +		case(RTE_COMP_CHECKSUM_CRC32):
> +			priv_xform->compress.chksum = IGZIP_GZIP_NO_HDR;
> +			break;
> +		case(RTE_COMP_CHECKSUM_ADLER32):
> +			priv_xform->compress.chksum = IGZIP_ZLIB_NO_HDR;
> +			break;
> +		case(RTE_COMP_CHECKSUM_CRC32_ADLER32):
> +			ISAL_PMD_LOG(ERR, "Combined CRC and ADLER
> checksum not"
> +					" supported\n");
> +			return -ENOTSUP;
> +		default:
> +			ISAL_PMD_LOG(ERR, "Checksum type not
> supported\n");
> +			return -ENOTSUP;
Do we not fall back to RTE_COMP_CHECKSUM_NONE if the configuration is wrong and report warning?

snipped
  
Daly, Lee Dec. 17, 2018, 9:20 a.m. UTC | #2
Hi Vipin, thanks for the question.

> -----Original Message-----
> From: Varghese, Vipin
> Sent: Monday, December 17, 2018 8:20 AM
> To: Daly, Lee <lee.daly@intel.com>; akhil.goyal@nxp.com
> Cc: dev@dpdk.org; Daly, Lee <lee.daly@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v4 1/3] compress/isal: enable checksum
> support in driver
> 
> Hi Lee,
> 
> Apologies if the logic is already done for my query
> 
> snipped
> > +		/* Set private xform checksum */
> > +		switch (xform->compress.chksum) {
> > +		/* Raw deflate by default */
> Does the user have to choose RTE_COMP_CHECKSUM_NONE while creating
> compress/isal instance?

At the moment, yes the application must fill out the xform/instance. 
> 
> > +		case(RTE_COMP_CHECKSUM_NONE):
> > +			priv_xform->compress.chksum = IGZIP_DEFLATE;
> > +			break;
> > +		case(RTE_COMP_CHECKSUM_CRC32):
> > +			priv_xform->compress.chksum =
> IGZIP_GZIP_NO_HDR;
> > +			break;
> > +		case(RTE_COMP_CHECKSUM_ADLER32):
> > +			priv_xform->compress.chksum =
> IGZIP_ZLIB_NO_HDR;
> > +			break;
> > +		case(RTE_COMP_CHECKSUM_CRC32_ADLER32):
> > +			ISAL_PMD_LOG(ERR, "Combined CRC and ADLER
> > checksum not"
> > +					" supported\n");
> > +			return -ENOTSUP;
> > +		default:
> > +			ISAL_PMD_LOG(ERR, "Checksum type not
> > supported\n");
> > +			return -ENOTSUP;
> Do we not fall back to RTE_COMP_CHECKSUM_NONE if the configuration is
> wrong and report warning?
> 

Right now, the xform must be filled out correctly before compression can be executed. 
Perhaps we could do as you say and fall back to a default config if the non-essential params are incorrect.
> snipped
  
Varghese, Vipin Dec. 17, 2018, 12:33 p.m. UTC | #3
Thanks for the update

> -----Original Message-----
> From: Daly, Lee
> Sent: Monday, December 17, 2018 2:50 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; akhil.goyal@nxp.com
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v4 1/3] compress/isal: enable checksum
> support in driver
> 
> Hi Vipin, thanks for the question.
> 
> > -----Original Message-----
> > From: Varghese, Vipin
> > Sent: Monday, December 17, 2018 8:20 AM
> > To: Daly, Lee <lee.daly@intel.com>; akhil.goyal@nxp.com
> > Cc: dev@dpdk.org; Daly, Lee <lee.daly@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v4 1/3] compress/isal: enable checksum
> > support in driver
> >
> > Hi Lee,
> >
> > Apologies if the logic is already done for my query
> >
> > snipped
> > > +		/* Set private xform checksum */
> > > +		switch (xform->compress.chksum) {
> > > +		/* Raw deflate by default */
> > Does the user have to choose RTE_COMP_CHECKSUM_NONE while creating
> > compress/isal instance?
> 
> At the moment, yes the application must fill out the xform/instance.
> >
> > > +		case(RTE_COMP_CHECKSUM_NONE):
> > > +			priv_xform->compress.chksum = IGZIP_DEFLATE;
> > > +			break;
> > > +		case(RTE_COMP_CHECKSUM_CRC32):
> > > +			priv_xform->compress.chksum =
> > IGZIP_GZIP_NO_HDR;
> > > +			break;
> > > +		case(RTE_COMP_CHECKSUM_ADLER32):
> > > +			priv_xform->compress.chksum =
> > IGZIP_ZLIB_NO_HDR;
> > > +			break;
> > > +		case(RTE_COMP_CHECKSUM_CRC32_ADLER32):
> > > +			ISAL_PMD_LOG(ERR, "Combined CRC and ADLER
> > > checksum not"
> > > +					" supported\n");
> > > +			return -ENOTSUP;
> > > +		default:
> > > +			ISAL_PMD_LOG(ERR, "Checksum type not
> > > supported\n");
> > > +			return -ENOTSUP;
> > Do we not fall back to RTE_COMP_CHECKSUM_NONE if the configuration is
> > wrong and report warning?
> >
> 
> Right now, the xform must be filled out correctly before compression can be
> executed.
> Perhaps we could do as you say and fall back to a default config if the non-
> essential params are incorrect.
> > snipped
  

Patch

diff --git a/drivers/compress/isal/isal_compress_pmd.c b/drivers/compress/isal/isal_compress_pmd.c
index 9f1e968..b087ed8 100644
--- a/drivers/compress/isal/isal_compress_pmd.c
+++ b/drivers/compress/isal/isal_compress_pmd.c
@@ -16,6 +16,8 @@ 
 #define RTE_COMP_ISAL_LEVEL_ONE 1
 #define RTE_COMP_ISAL_LEVEL_TWO 2
 #define RTE_COMP_ISAL_LEVEL_THREE 3 /* Optimised for AVX512 & AVX2 only */
+#define CHKSUM_SZ_CRC 8
+#define CHKSUM_SZ_ADLER 4
 
 int isal_logtype_driver;
 
@@ -43,12 +45,6 @@  isal_comp_set_priv_xform_parameters(struct isal_priv_xform *priv_xform,
 		}
 		priv_xform->compress.algo = RTE_COMP_ALGO_DEFLATE;
 
-		/* Set private xform checksum - raw deflate by default */
-		if (xform->compress.chksum != RTE_COMP_CHECKSUM_NONE) {
-			ISAL_PMD_LOG(ERR, "Checksum not supported\n");
-			return -ENOTSUP;
-		}
-
 		/* Set private xform window size, 32K supported */
 		if (xform->compress.window_size == RTE_COMP_ISAL_WINDOW_SIZE)
 			priv_xform->compress.window_size =
@@ -77,6 +73,27 @@  isal_comp_set_priv_xform_parameters(struct isal_priv_xform *priv_xform,
 			return -ENOTSUP;
 		}
 
+		/* Set private xform checksum */
+		switch (xform->compress.chksum) {
+		/* Raw deflate by default */
+		case(RTE_COMP_CHECKSUM_NONE):
+			priv_xform->compress.chksum = IGZIP_DEFLATE;
+			break;
+		case(RTE_COMP_CHECKSUM_CRC32):
+			priv_xform->compress.chksum = IGZIP_GZIP_NO_HDR;
+			break;
+		case(RTE_COMP_CHECKSUM_ADLER32):
+			priv_xform->compress.chksum = IGZIP_ZLIB_NO_HDR;
+			break;
+		case(RTE_COMP_CHECKSUM_CRC32_ADLER32):
+			ISAL_PMD_LOG(ERR, "Combined CRC and ADLER checksum not"
+					" supported\n");
+			return -ENOTSUP;
+		default:
+			ISAL_PMD_LOG(ERR, "Checksum type not supported\n");
+			return -ENOTSUP;
+		}
+
 		/* Set private xform level.
 		 * Checking compliance with compressdev API, -1 <= level => 9
 		 */
@@ -170,9 +187,24 @@  isal_comp_set_priv_xform_parameters(struct isal_priv_xform *priv_xform,
 		}
 		priv_xform->decompress.algo = RTE_COMP_ALGO_DEFLATE;
 
-		/* Set private xform checksum - raw deflate by default */
-		if (xform->compress.chksum != RTE_COMP_CHECKSUM_NONE) {
-			ISAL_PMD_LOG(ERR, "Checksum not supported\n");
+		/* Set private xform checksum */
+		switch (xform->decompress.chksum) {
+		/* Raw deflate by default */
+		case(RTE_COMP_CHECKSUM_NONE):
+			priv_xform->decompress.chksum = ISAL_DEFLATE;
+			break;
+		case(RTE_COMP_CHECKSUM_CRC32):
+			priv_xform->decompress.chksum = ISAL_GZIP_NO_HDR_VER;
+			break;
+		case(RTE_COMP_CHECKSUM_ADLER32):
+			priv_xform->decompress.chksum = ISAL_ZLIB_NO_HDR_VER;
+			break;
+		case(RTE_COMP_CHECKSUM_CRC32_ADLER32):
+			ISAL_PMD_LOG(ERR, "Combined CRC and ADLER checksum not"
+					" supported\n");
+			return -ENOTSUP;
+		default:
+			ISAL_PMD_LOG(ERR, "Checksum type not supported\n");
 			return -ENOTSUP;
 		}
 
@@ -376,6 +408,9 @@  process_isal_deflate(struct rte_comp_op *op, struct isal_comp_qp *qp,
 
 	qp->stream->level_buf = temp_level_buf;
 
+	/* Set Checksum flag */
+	qp->stream->gzip_flag = priv_xform->compress.chksum;
+
 	/* Stateless operation, input will be consumed in one go */
 	qp->stream->flush = NO_FLUSH;
 
@@ -459,8 +494,18 @@  process_isal_deflate(struct rte_comp_op *op, struct isal_comp_qp *qp,
 			return ret;
 		}
 	}
+
 	op->consumed = qp->stream->total_in;
-	op->produced = qp->stream->total_out;
+	if (qp->stream->gzip_flag == IGZIP_DEFLATE) {
+		op->produced = qp->stream->total_out;
+	} else if (qp->stream->gzip_flag == IGZIP_ZLIB_NO_HDR) {
+		op->produced = qp->stream->total_out - CHKSUM_SZ_ADLER;
+		op->output_chksum = qp->stream->internal_state.crc + 1;
+	} else {
+		op->produced = qp->stream->total_out - CHKSUM_SZ_CRC;
+		op->output_chksum = qp->stream->internal_state.crc;
+	}
+
 	isal_deflate_reset(qp->stream);
 
 	return ret;
@@ -468,7 +513,8 @@  process_isal_deflate(struct rte_comp_op *op, struct isal_comp_qp *qp,
 
 /* Stateless Decompression Function */
 static int
-process_isal_inflate(struct rte_comp_op *op, struct isal_comp_qp *qp)
+process_isal_inflate(struct rte_comp_op *op, struct isal_comp_qp *qp,
+		struct isal_priv_xform *priv_xform)
 {
 	int ret = 0;
 
@@ -477,6 +523,9 @@  process_isal_inflate(struct rte_comp_op *op, struct isal_comp_qp *qp)
 	/* Initialize decompression state */
 	isal_inflate_init(qp->state);
 
+	/* Set Checksum flag */
+	qp->state->crc_flag = priv_xform->decompress.chksum;
+
 	if (op->m_src->pkt_len < (op->src.length + op->src.offset)) {
 		ISAL_PMD_LOG(ERR, "Input mbuf(s) not big enough.\n");
 		op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
@@ -531,13 +580,16 @@  process_isal_inflate(struct rte_comp_op *op, struct isal_comp_qp *qp)
 			return -1;
 		}
 
-		if (ret != ISAL_DECOMP_OK) {
+		if (ret != ISAL_DECOMP_OK && ret != ISAL_END_INPUT) {
+			ISAL_PMD_LOG(ERR, "Decompression operation failed\n");
 			op->status = RTE_COMP_OP_STATUS_ERROR;
 			return ret;
 		}
 		op->consumed = op->src.length - qp->state->avail_in;
 	}
-		op->produced = qp->state->total_out;
+	op->produced = qp->state->total_out;
+	op->output_chksum = qp->state->crc;
+
 	isal_inflate_reset(qp->state);
 
 	return ret;
@@ -553,7 +605,7 @@  process_op(struct isal_comp_qp *qp, struct rte_comp_op *op,
 		process_isal_deflate(op, qp, priv_xform);
 		break;
 	case RTE_COMP_DECOMPRESS:
-		process_isal_inflate(op, qp);
+		process_isal_inflate(op, qp, priv_xform);
 		break;
 	default:
 		ISAL_PMD_LOG(ERR, "Operation Not Supported\n");
diff --git a/drivers/compress/isal/isal_compress_pmd_ops.c b/drivers/compress/isal/isal_compress_pmd_ops.c
index 41cade8..7b91849 100644
--- a/drivers/compress/isal/isal_compress_pmd_ops.c
+++ b/drivers/compress/isal/isal_compress_pmd_ops.c
@@ -17,7 +17,9 @@  static const struct rte_compressdev_capabilities isal_pmd_capabilities[] = {
 					RTE_COMP_FF_OOP_LB_IN_SGL_OUT |
 					RTE_COMP_FF_SHAREABLE_PRIV_XFORM |
 					RTE_COMP_FF_HUFFMAN_FIXED |
-					RTE_COMP_FF_HUFFMAN_DYNAMIC,
+					RTE_COMP_FF_HUFFMAN_DYNAMIC |
+					RTE_COMP_FF_CRC32_CHECKSUM |
+					RTE_COMP_FF_ADLER32_CHECKSUM,
 		.window_size = {
 			.min = 15,
 			.max = 15,