[dpdk-dev] [EXT] Re: [PATCH v4 2/5] test/crypto: add combined mode tests

[Anoob Joseph]

Hi Hemant,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Hemant Agrawal <hemant.agrawal at oss.nxp.com>
> Sent: Friday, September 24, 2021 12:54 PM
> To: Anoob Joseph <anoobj at marvell.com>; Akhil Goyal <gakhil at marvell.com>;
> Declan Doherty <declan.doherty at intel.com>; Fan Zhang
> <roy.fan.zhang at intel.com>; Konstantin Ananyev
> <konstantin.ananyev at intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj at marvell.com>; Archana Muniganti
> <marchana at marvell.com>; Tejasree Kondoj <ktejasree at marvell.com>; Hemant
> Agrawal <hemant.agrawal at nxp.com>; Radu Nicolau
> <radu.nicolau at intel.com>; Ciara Power <ciara.power at intel.com>; Gagandeep
> Singh <g.singh at nxp.com>; dev at dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH v4 2/5] test/crypto: add combined mode
> tests
> 
> External Email
> 
> ----------------------------------------------------------------------
> HI Anoob,
> 
> On 9/17/2021 6:45 PM, Anoob Joseph wrote:
> > Add framework to test IPsec features with all supported combinations
> > of ciphers.
> >
> > Signed-off-by: Anoob Joseph <anoobj at marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejasree at marvell.com>
> >
> > ---
> >   app/test/test_cryptodev.c                |  73 +++++++++++++++++++--
> >   app/test/test_cryptodev_security_ipsec.c | 107
> +++++++++++++++++++++++++++++--
> >   app/test/test_cryptodev_security_ipsec.h |  52 ++++++++++++++-
> >   3 files changed, 223 insertions(+), 9 deletions(-)
> >
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > index 9c7875c..7fd246e 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -8904,7 +8904,8 @@ static int
> >   test_ipsec_proto_process(const struct ipsec_test_data td[],
> >   			 struct ipsec_test_data res_d[],
> >   			 int nb_td,
> > -			 bool silent)
> > +			 bool silent,
> > +			 const struct ipsec_test_flags *flags)
> >   {
> >   	struct crypto_testsuite_params *ts_params = &testsuite_params;
> >   	struct crypto_unittest_params *ut_params = &unittest_params; @@
> > -9021,7 +9022,7 @@ test_ipsec_proto_process(const struct ipsec_test_data
> td[],
> >   		/* Process crypto operation */
> >   		process_crypto_request(dev_id, ut_params->op);
> >
> > -		ret = test_ipsec_status_check(ut_params->op, dir);
> > +		ret = test_ipsec_status_check(ut_params->op, flags, dir);
> >   		if (ret != TEST_SUCCESS)
> >   			goto crypto_op_free;
> >
> > @@ -9029,7 +9030,7 @@ test_ipsec_proto_process(const struct
> ipsec_test_data td[],
> >   			res_d_tmp = &res_d[i];
> >
> >   		ret = test_ipsec_post_process(ut_params->ibuf, &td[i],
> > -					      res_d_tmp, silent);
> > +					      res_d_tmp, silent, flags);
> >   		if (ret != TEST_SUCCESS)
> >   			goto crypto_op_free;
> >
> > @@ -9057,11 +9058,71 @@ test_ipsec_proto_process(const struct
> ipsec_test_data td[],
> >   static int
> >   test_ipsec_proto_known_vec_inb(const void *td_outb)
> >   {
> > +	struct ipsec_test_flags flags;
> >   	struct ipsec_test_data td_inb;
> >
> > +	memset(&flags, 0, sizeof(flags));
> > +
> >   	test_ipsec_td_in_from_out(td_outb, &td_inb);
> >
> > -	return test_ipsec_proto_process(&td_inb, NULL, 1, false);
> > +	return test_ipsec_proto_process(&td_inb, NULL, 1, false, &flags);
> 
> In most places, you have hardcoded this silent flag - this is for debug prints.
> 
> Will you please driver it from some macro or variable or log-level, where it can
> be changed easily.
> 

[Anoob] Currently, for known vector tests, we have "silent" set as false to have more verbose output by default. For combined mode, since we attempt multiple ciphers, there would be lot of prints. Hence we kept "silent" flag as true to disable prints for such cases. The idea is, if a developer wants to quickly analyze one combined mode test case, he can do so by changing silent flag for just that. Enabling it universally could dump more info than desired. 

Making it a global flag would mean, we would have same kind of prints with known vector & combined mode (which attempts combinations). Do you think a global setting is better? If yes, what do you recommend? A custom flag or RTE_LOG?