Re: [PATCH v3 1/5] test/compress: add initial unit tests

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

Hi Lee,

> -----Original Message-----
> From: Daly, Lee
> Sent: Wednesday, May 2, 2018 2:44 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; shally.verma@cavium.com;
> ahmed.mansour@nxp.com; Ashish.Gupta@cavium.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>; Shally Verma
> <shally.verma@caviumnetworks.com>
> Subject: RE: [dpdk-dev] [PATCH v3 1/5] test/compress: add initial unit tests
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pablo de Lara
> > Sent: Friday, April 27, 2018 3:15 PM
> > To: dev@dpdk.org
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>; shally.verma@cavium.com;
> > ahmed.mansour@nxp.com; Ashish.Gupta@cavium.com; De Lara Guarch,
> Pablo
> > <pablo.de.lara.guarch@intel.com>; Ashish Gupta
> > <ashish.gupta@caviumnetworks.com>; Shally Verma
> > <shally.verma@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH v3 1/5] test/compress: add initial unit
> > tests
> >
> > This commit introduces the initial tests for compressdev, performing
> > basic compression and decompression operations of sample test buffers,
> > using the Zlib library in one direction and compressdev in another
> > direction, to make sure that the library is compatible with Zlib.
> >
> > Due to the use of Zlib API, the test is disabled by default, to avoid
> > adding a new dependency on DPDK.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> > Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> 
> 
> <...>
> 
> > +static int
> > +testsuite_setup(void)
> > +{
> > +	struct comp_testsuite_params *ts_params = &testsuite_params;
> > +	unsigned int i;
> > +
> > +	if (rte_compressdev_count() == 0) {
> > +		RTE_LOG(ERR, USER1, "Need at least one compress
> > device\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	uint32_t max_buf_size = 0;
> > +	for (i = 0; i < RTE_DIM(compress_test_bufs); i++)
> > +		max_buf_size = RTE_MAX(max_buf_size,
> > +				strlen(compress_test_bufs[i]) + 1);
> > +
> > +	max_buf_size *= COMPRESS_BUF_SIZE_RATIO;
> > +	/*
> > +	 * Buffers to be used in compression and decompression.
> > +	 * Since decompressed data might be larger than
> > +	 * compressed data (due to block header),
> > +	 * buffers should be big enough for both cases.
> > +	 */
> > +	ts_params->mbuf_pool = rte_pktmbuf_pool_create("mbuf_pool",
> > +			NUM_MBUFS,
> > +			CACHE_SIZE, 0,
> > +			max_buf_size + RTE_PKTMBUF_HEADROOM,
> > +			rte_socket_id());
> > +	if (ts_params->mbuf_pool == NULL) {
> > +		RTE_LOG(ERR, USER1, "Large mbuf pool could not be
> > created\n");
> > +		goto exit;
> 
> [Lee]At this point there is nothing to be freed yet, therefore TEST_FAILED can be
> returned here without testsuite_teardown();

Right, will change.

> 
> > +	}
> > +
> > +	ts_params->op_pool = rte_comp_op_pool_create("op_pool",
> > NUM_OPS,
> > +						0, 0, rte_socket_id());
> > +	if (ts_params->op_pool == NULL) {
> > +		RTE_LOG(ERR, USER1, "Operation pool could not be
> > created\n");
> > +		goto exit;
> 
> [Lee]Similar point here, wasted cycles on freeing memory which has not be
> allocated yet.
> May not be a major issue since this is only done on setup.

Well, at this point mbuf_pool has been created, so we need to free that.

> 
> > +	}
> > +
> > +	/* Initializes default values for compress/decompress xforms */
> > +	ts_params->def_comp_xform.type = RTE_COMP_COMPRESS;
> > +	ts_params->def_comp_xform.compress.algo =
> > RTE_COMP_ALGO_DEFLATE,
> > +	ts_params->def_comp_xform.compress.deflate.huffman =
> > +
> > 	RTE_COMP_HUFFMAN_DEFAULT;
> > +	ts_params->def_comp_xform.compress.level =
> > RTE_COMP_LEVEL_PMD_DEFAULT;
> > +	ts_params->def_comp_xform.compress.chksum =
> > RTE_COMP_CHECKSUM_NONE;
> > +	ts_params->def_comp_xform.compress.window_size =
> > DEFAULT_WINDOW_SIZE;
> > +
> > +	ts_params->def_decomp_xform.type = RTE_COMP_DECOMPRESS;
> > +	ts_params->def_decomp_xform.decompress.algo =
> > RTE_COMP_ALGO_DEFLATE,
> > +	ts_params->def_decomp_xform.decompress.chksum =
> > RTE_COMP_CHECKSUM_NONE;
> > +	ts_params->def_decomp_xform.decompress.window_size =
> > +DEFAULT_WINDOW_SIZE;
> > +
> > +	return TEST_SUCCESS;
> > +
> > +exit:
> > +	testsuite_teardown();
> > +
> > +	return TEST_FAILED;
> > +}
> > +
> <...>