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

Hi Fan,

-----Original Message-----
From: Zhang, Roy Fan <redacted>
Sent: Thursday, June 25, 2020 2:32 PM
To: dev@dpdk.org
Cc: Trahe, Fiona <redacted>; akhil.goyal@nxp.com; Zhang, Roy Fan
[off-list ref]
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.

[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.