Re: [dpdk-dev] [dpdk-dev v2 2/3] test/crypto: add unit-test for QAT direct APIs

["Trahe, Fiona" <fiona.trahe@intel.com>]

Hi Fan,

> -----Original Message-----
> From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Sent: Thursday, June 25, 2020 2:32 PM
> To: dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: [dpdk-dev v2 2/3] test/crypto: add unit-test for QAT direct APIs
> 
> This patch adds the test to use QAT symmetric crypto direct APIs.
> The test will be enabled only when QAT Sym PMD is built.
> 


> @@ -2451,7 +2615,11 @@ test_snow3g_authentication(const struct snow3g_hash_test_data *tdata)
>  	if (retval < 0)
>  		return retval;
> 
> -	ut_params->op = process_crypto_request(ts_params->valid_devs[0],
> +	if (qat_api_test)
> +		process_qat_api_op(ts_params->valid_devs[0], 0, ut_params->op,
> +				0, 1, 1);
[Fiona] it would be helpful wherever this is called to either use local params with useful names, enums or comments for last 3 params, e.g.:
process_qat_api_op(ts_params->valid_devs[0], 0, ut_params->op,
				 0,   /*is_cipher    */
				 1,   /* is_auth      */
				 1); /* len_in_bits*/

It would be better to generalise the tests too in light of Jerin's comments.
By passing the xform to process_crypto_request(), you should be able to avoid all the changes
to individual tests, e.g.;
in process_crypto_request(dev, op, xform )
if (direct_pmd_datapath)
	switch (pmd) {
	qat: process_qat_api_op();
	other PMD:/*future*/
	default: break;
	}
else
	rest of process_crypto_op fn


Unfortunately the xform is usually not available where process_crypto_request is called, it would have to be returned from the session create function  - but though the 3 params are enough for the moment, it's likely other session params will be needed for other tests. Or by other PMDs.


> +static int
> +test_qat_sym_direct_api(void /*argv __rte_unused, int argc __rte_unused*/)
> +{
> +	int ret;
> +
> +	gbl_driver_id =	rte_cryptodev_driver_id_get(
> +			RTE_STR(CRYPTODEV_NAME_QAT_SYM_PMD));
> +
> +	if (gbl_driver_id == -1) {
> +		RTE_LOG(ERR, USER1, "QAT PMD must be loaded. Check that both "
> +		"CONFIG_RTE_LIBRTE_PMD_QAT and CONFIG_RTE_LIBRTE_PMD_QAT_SYM "
> +		"are enabled in config file to run this testsuite.\n");
> +		return TEST_SKIPPED;
> +	}
> +
> +	qat_api_test = 1;
[Fiona] I'd suggest renaming this direct_pmd_datapath as above
This would be a good place to check that the pmd has the direct_pmd_datapath capability.
Checks for a finer granularity capability may be needed in process_qat_api_op() 

> +	ret = unit_test_suite_runner(&qat_direct_api_testsuite);
> +	qat_api_test = 0;
> +
> +	return ret;
> +}
> +

[Fiona] It would be good to add tests that validate the frame concept - enqueueing
and dequeueing only 1 descriptor at a time doesn't cover it.