RE: [EXTERNAL] [PATCH v2 3/3] app/compress-perf: support for dictionaries and PDCP checksum

[Akhil Goyal <gakhil@marvell.com>]

> Adds support to read and apply dictionaries
> 
> Signed-off-by: Sameer Vaze <svaze@qti.qualcomm.com>
> ---
> v2:
> * Handles relocated dictionary params
> 
>  app/test-compress-perf/comp_perf_options.h    |  4 +
>  .../comp_perf_options_parse.c                 | 16 ++++
>  .../comp_perf_test_verify.c                   | 42 +++++++--
>  app/test-compress-perf/main.c                 | 90 +++++++++++++++++++
>  4 files changed, 147 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test-compress-perf/comp_perf_options.h b/app/test-compress-
> perf/comp_perf_options.h
> index 828a7309d8..4c34dcd8aa 100644
> --- a/app/test-compress-perf/comp_perf_options.h
> +++ b/app/test-compress-perf/comp_perf_options.h
> @@ -21,6 +21,7 @@ enum cleanup_st {
>  	ST_TEST_DATA,
>  	ST_COMPDEV,
>  	ST_INPUT_DATA,
> +	ST_DICTIONARY_DATA,
>  	ST_MEMORY_ALLOC,
>  	ST_DURING_TEST
>  };
> @@ -48,10 +49,13 @@ struct range_list {
>  struct comp_test_data {
>  	char driver_name[RTE_DEV_NAME_MAX_LEN];
>  	char input_file[PATH_MAX];
> +	char dictionary_file[PATH_MAX];
>  	enum cperf_test_type test;
> 
>  	uint8_t *input_data;
>  	size_t input_data_sz;
> +	uint8_t *dictionary_data;
> +	size_t dictionary_data_sz;
>  	uint16_t nb_qps;
>  	uint16_t seg_sz;
>  	uint16_t out_seg_sz;
> diff --git a/app/test-compress-perf/comp_perf_options_parse.c b/app/test-
> compress-perf/comp_perf_options_parse.c
> index 6d8c370fc2..37205ad806 100644
> --- a/app/test-compress-perf/comp_perf_options_parse.c
> +++ b/app/test-compress-perf/comp_perf_options_parse.c
> @@ -31,6 +31,7 @@
>  #define CPERF_LEVEL		("compress-level")
>  #define CPERF_WINDOW_SIZE	("window-sz")
>  #define CPERF_EXTERNAL_MBUFS	("external-mbufs")
> +#define CPERF_DICTIONARY	("dictionary")
> 
>  /* cyclecount-specific options */
>  #define CPERF_CYCLECOUNT_DELAY_US ("cc-delay-us")
> @@ -71,6 +72,7 @@ usage(char *progname)
>  		"		keeping the data directly in mbuf area\n"
>  		" --cc-delay-us N: delay between enqueue and dequeue
> operations in microseconds\n"
>  		"		valid only for cyclecount perf test (default: 500
> us)\n"
> +		" --dictionary NAME: file with dictionary\n"
>  		" -h: prints this help\n",
>  		progname);
>  }

Update documentation also for the newly added parameters. 
doc/guides/tools/comp_perf.rst


> @@ -609,6 +611,18 @@ parse_external_mbufs(struct comp_test_data
> *test_data,
>  	return 0;
>  }
> 
> +static int
> +parse_dictionary_file(struct comp_test_data *test_data, const char *arg)
> +{
> +	if (strlen(arg) > (sizeof(test_data->dictionary_file) - 1))
> +		return -1;
> +
> +	strlcpy(test_data->dictionary_file, arg, sizeof(test_data-
> >dictionary_file));
> +
> +	return 0;
> +}
> +
> +
Remove extra blank line.

>  static int
>  parse_cyclecount_delay_us(struct comp_test_data *test_data,
>  			const char *arg)
> @@ -647,6 +661,7 @@ static struct option lgopts[] = {
>  	{ CPERF_LEVEL, required_argument, 0, 0 },
>  	{ CPERF_WINDOW_SIZE, required_argument, 0, 0 },
>  	{ CPERF_EXTERNAL_MBUFS, 0, 0, 0 },
> +	{ CPERF_DICTIONARY, required_argument, 0, 0 },
>  	{ CPERF_CYCLECOUNT_DELAY_US, required_argument, 0, 0 },
>  	{ NULL, 0, 0, 0 }
>  };
> @@ -671,6 +686,7 @@ comp_perf_opts_parse_long(int opt_idx, struct
> comp_test_data *test_data)
>  		{ CPERF_LEVEL,		parse_level },
>  		{ CPERF_WINDOW_SIZE,	parse_window_sz },
>  		{ CPERF_EXTERNAL_MBUFS,	parse_external_mbufs },
> +		{ CPERF_DICTIONARY,	parse_dictionary_file },
>  		{ CPERF_CYCLECOUNT_DELAY_US,
> 	parse_cyclecount_delay_us },
>  	};
>  	unsigned int i;
> diff --git a/app/test-compress-perf/comp_perf_test_verify.c b/app/test-
> compress-perf/comp_perf_test_verify.c
> index 09d97c5cf7..3fc4b99b51 100644
> --- a/app/test-compress-perf/comp_perf_test_verify.c
> +++ b/app/test-compress-perf/comp_perf_test_verify.c
> @@ -64,6 +64,7 @@ main_loop(struct cperf_verify_ctx *ctx, enum
> rte_comp_xform_type type)
>  	int res = 0;
>  	int allocated = 0;
>  	uint32_t out_seg_sz;
> +	uint8_t dict[DEFLATE_MAX_WINDOW_SIZE] = {0};
> 
>  	if (test_data == NULL || !test_data->burst_sz) {
>  		RTE_LOG(ERR, USER1,
> @@ -71,6 +72,22 @@ main_loop(struct cperf_verify_ctx *ctx, enum
> rte_comp_xform_type type)
>  		return -1;
>  	}
> 
> +	uint16_t window_size = (1ULL << test_data->window_sz);

Do not define variables in middle of function. All variables shall be defined at beginning of the functions.

> +
> +	if (test_data->dictionary_data) {
> +		if (test_data->dictionary_data_sz >= window_size) {
> +			memcpy(dict,
> +				test_data->dictionary_data
> +				+ (test_data->dictionary_data_sz -
> window_size),
> +				window_size);
> +		} else if (test_data->dictionary_data_sz < window_size) {
> +			memcpy(dict + (window_size - test_data-
> >dictionary_data_sz),
> +				test_data->dictionary_data,
> +				test_data->dictionary_data_sz);
> +		}
> +	}
> +
> +
>  	ops = rte_zmalloc_socket(NULL,
>  		2 * mem->total_bufs * sizeof(struct rte_comp_op *),
>  		0, rte_socket_id());
> @@ -91,12 +108,14 @@ main_loop(struct cperf_verify_ctx *ctx, enum
> rte_comp_xform_type type)
>  				.level = test_data->level,
>  				.window_size = test_data->window_sz,
>  				.chksum = RTE_COMP_CHECKSUM_NONE,
> -				.hash_algo = RTE_COMP_HASH_ALGO_NONE
> +				.hash_algo = RTE_COMP_HASH_ALGO_NONE,
>  			}
>  		};
> -		if (test_data->test_algo == RTE_COMP_ALGO_DEFLATE)
> +		if (test_data->test_algo == RTE_COMP_ALGO_DEFLATE) {
>  			xform.compress.deflate.huffman = test_data-
> >huffman_enc;
> -		else if (test_data->test_algo == RTE_COMP_ALGO_LZ4)
> +			xform.compress.deflate.dictionary = dict;
> +			xform.compress.deflate.dictionary_len = window_size;
> +		} else if (test_data->test_algo == RTE_COMP_ALGO_LZ4)
>  			xform.compress.lz4.flags = test_data->lz4_flags;
>  		output_data_ptr = ctx->mem.compressed_data;
>  		output_data_sz = &ctx->comp_data_sz;
> @@ -113,7 +132,10 @@ main_loop(struct cperf_verify_ctx *ctx, enum
> rte_comp_xform_type type)
>  				.hash_algo = RTE_COMP_HASH_ALGO_NONE
>  			}
>  		};
> -		if (test_data->test_algo == RTE_COMP_ALGO_LZ4)
> +		if (test_data->test_algo == RTE_COMP_ALGO_DEFLATE) {
> +			xform.decompress.inflate.dictionary = dict;
> +			xform.decompress.inflate.dictionary_len = window_size;
> +		} else if (test_data->test_algo == RTE_COMP_ALGO_LZ4)
>  			xform.decompress.lz4.flags = test_data->lz4_flags;
>  		output_data_ptr = ctx->mem.decompressed_data;
>  		output_data_sz = &ctx->decomp_data_sz;
> @@ -194,7 +216,17 @@ main_loop(struct cperf_verify_ctx *ctx, enum
> rte_comp_xform_type type)
> 
> 	rte_pktmbuf_pkt_len(input_bufs[buf_id]);
>  				ops[op_id]->dst.offset = 0;
>  				ops[op_id]->flush_flag =
> RTE_COMP_FLUSH_FINAL;
> -				ops[op_id]->input_chksum = buf_id;
> +				if ((xform.type == RTE_COMP_DECOMPRESS)
> &&
> +					(xform.decompress.chksum
> +						==
> RTE_COMP_CHECKSUM_3GPP_PDCP_UDC)) {
> +					uint8_t *udc_header
> +						=
> rte_pktmbuf_mtod(ops[op_id]->m_src, uint8_t *);
> +					ops[op_id]->input_chksum =
> *udc_header & 0xf;
> +					ops[op_id]->src.offset = 1;
> +				} else {
> +					ops[op_id]->input_chksum = buf_id;
> +					ops[op_id]->src.offset = 0;
> +				}
>  				ops[op_id]->private_xform = priv_xform;
>  			}
> 
> diff --git a/app/test-compress-perf/main.c b/app/test-compress-perf/main.c
> index 70ce4316cc..693a7cc31b 100644
> --- a/app/test-compress-perf/main.c
> +++ b/app/test-compress-perf/main.c
> @@ -335,6 +335,86 @@ comp_perf_dump_input_data(struct comp_test_data
> *test_data)
>  	return ret;
>  }
> 
> +static int
> +comp_perf_dump_dictionary_data(struct comp_test_data *test_data)
> +{
> +	FILE *f = fopen(test_data->dictionary_file, "r");
> +	int ret = -1;
> +
> +	if (f == NULL) {
> +		RTE_LOG(ERR, USER1, "Dictionary file not specified\n");
> +		test_data->dictionary_data_sz = 0;
> +		test_data->dictionary_data = NULL;
> +		ret = 0;
> +		goto end;
> +	}
> +
> +	if (fseek(f, 0, SEEK_END) != 0) {
> +		RTE_LOG(ERR, USER1, "Size of input could not be calculated\n");
> +		goto end;
> +	}
> +	size_t actual_file_sz = ftell(f);
> +	/* If extended input data size has not been set,
> +	 * input data size = file size
> +	 */
> +
> +	if (test_data->dictionary_data_sz == 0)
> +		test_data->dictionary_data_sz = actual_file_sz;
> +
> +	if (test_data->dictionary_data_sz <= 0 || actual_file_sz <= 0 ||
> +			fseek(f, 0, SEEK_SET) != 0) {
> +		RTE_LOG(ERR, USER1, "Size of input could not be calculated\n");
> +		goto end;
> +	}
> +
> +	test_data->dictionary_data = rte_zmalloc_socket(NULL,
> +				test_data->dictionary_data_sz, 0,
> rte_socket_id());
> +
> +	if (test_data->dictionary_data == NULL) {
> +		RTE_LOG(ERR, USER1, "Memory to hold the data from the
> dictionary "
> +				"file could not be allocated\n");
> +		goto end;
> +	}
> +
> +	size_t remaining_data = test_data->dictionary_data_sz;
> +	uint8_t *data = test_data->dictionary_data;
> +
> +	while (remaining_data > 0) {
> +		size_t data_to_read = RTE_MIN(remaining_data, actual_file_sz);
> +
> +		if (fread(data, data_to_read, 1, f) != 1) {
> +			RTE_LOG(ERR, USER1, "Input file could not be read\n");
> +			goto end;
> +		}
> +		if (fseek(f, 0, SEEK_SET) != 0) {
> +			RTE_LOG(ERR, USER1,
> +				"Size of input could not be calculated\n");
> +			goto end;
> +		}
> +		remaining_data -= data_to_read;
> +		data += data_to_read;
> +	}
> +
> +	printf("\n");
> +	if (test_data->dictionary_data_sz > actual_file_sz)
> +		RTE_LOG(INFO, USER1,
> +		  "%zu bytes read from file %s, extending the file %.2f times\n",
> +			test_data->dictionary_data_sz, test_data-
> >dictionary_file,
> +			(double)test_data->dictionary_data_sz/actual_file_sz);
> +	else
> +		RTE_LOG(INFO, USER1,
> +			"%zu bytes read from file %s\n",
> +			test_data->dictionary_data_sz, test_data-
> >dictionary_file);
> +
> +	ret = 0;
> +
> +end:
> +	if (f)
> +		fclose(f);
> +
> +	return ret;
There is a memory leak here in case of error.
Dictionary data is not getting freed. 

> +}
> +
>  static void
>  comp_perf_cleanup_on_signal(int signalNumber __rte_unused)
>  {
> @@ -407,6 +487,13 @@ main(int argc, char **argv)
>  	}
> 
>  	test_data->cleanup = ST_INPUT_DATA;
> +	if (comp_perf_dump_dictionary_data(test_data) < 0) {
> +		ret = EXIT_FAILURE;
> +		goto end;
> +	}
> +
> +	test_data->cleanup = ST_DICTIONARY_DATA;
> +
> 
>  	if (test_data->level_lst.inc != 0)
>  		test_data->level = test_data->level_lst.min;
> @@ -496,6 +583,9 @@ main(int argc, char **argv)
>  			i++;
>  		}
>  		/* fallthrough */
> +	case ST_DICTIONARY_DATA:
> +		rte_free(test_data->dictionary_data);
> +		/* fallthrough */
>  	case ST_INPUT_DATA:
>  		rte_free(test_data->input_data);
>  		/* fallthrough */
> --
> 2.31.1