test/crypto: skip plain text compare for null cipher OOP

  NULL cipher is used for validating auth only cases. With out of place
processing, validating plain text should not be done as the PMD is only
expected to update auth data.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
---
 app/test/test_cryptodev.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
  

Hi Anoob,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Anoob Joseph
> Sent: Monday, November 8, 2021 3:20 PM
> To: Akhil Goyal <gakhil@marvell.com>; Doherty, Declan
> <declan.doherty@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; Jerin Jacob <jerinj@marvell.com>;
> Archana Muniganti <marchana@marvell.com>; Tejasree Kondoj
> <ktejasree@marvell.com>; Hemant Agrawal <hemant.agrawal@nxp.com>;
> Nicolau, Radu <radu.nicolau@intel.com>; Power, Ciara
> <ciara.power@intel.com>; Gagandeep Singh <g.singh@nxp.com>;
> dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] test/crypto: skip plain text compare for null cipher
> OOP
> 
> NULL cipher is used for validating auth only cases. With out of place processing,
> validating plain text should not be done as the PMD is only expected to update
> auth data.
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> ---
>  app/test/test_cryptodev.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index
> e54a1a9..964f44f 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -7490,6 +7490,22 @@ test_mixed_auth_cipher(const struct
> mixed_cipher_auth_test_data *tdata,
>  				tdata->digest_enc.len);
>  	}
> 
> +	/*
> +	 * NULL cipher is used for auth only cases where only authentication
> +	 * is done. With verify operation, MAC would be validated by the PMD.
> +	 * With generate operation, verify MAC generated by the PMD.
> +	 */
> +	if (op_mode == OUT_OF_PLACE &&
> +	    tdata->cipher_algo == RTE_CRYPTO_CIPHER_NULL) {

Why only checking for OUT_OF_PLACE? As far as cipher algorithm is NULL,
only digest should be checked.
Also, looking at the code, there is this same check, but with hardcoded tag length.
Could you rearrange the code to have less lines and generic?

Something like:

<pseudo-code>
if (!verify)
	check_digest

if (cipher_algo != NULL)
	check_ciphertext/plaintext

check op status
</pseudo-code>

Also, I think this change is applicable in test_mixed_auth_cipher_sgl.

Thanks,
Pablo
  

Hi Pablo,

Thanks for the review. Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Sent: Wednesday, November 10, 2021 4:54 PM
> To: Anoob Joseph <anoobj@marvell.com>; Akhil Goyal
> <gakhil@marvell.com>; Doherty, Declan <declan.doherty@intel.com>;
> Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Archana Muniganti
> <marchana@marvell.com>; Tejasree Kondoj <ktejasree@marvell.com>;
> Hemant Agrawal <hemant.agrawal@nxp.com>; Nicolau, Radu
> <radu.nicolau@intel.com>; Power, Ciara <ciara.power@intel.com>;
> Gagandeep Singh <g.singh@nxp.com>; dev@dpdk.org
> Subject: [EXT] RE: [dpdk-dev] [PATCH] test/crypto: skip plain text compare
> for null cipher OOP
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Anoob,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Anoob Joseph
> > Sent: Monday, November 8, 2021 3:20 PM
> > To: Akhil Goyal <gakhil@marvell.com>; Doherty, Declan
> > <declan.doherty@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Cc: Anoob Joseph <anoobj@marvell.com>; Jerin Jacob
> > <jerinj@marvell.com>; Archana Muniganti <marchana@marvell.com>;
> > Tejasree Kondoj <ktejasree@marvell.com>; Hemant Agrawal
> > <hemant.agrawal@nxp.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> > Power, Ciara <ciara.power@intel.com>; Gagandeep Singh
> > <g.singh@nxp.com>; dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] test/crypto: skip plain text compare for
> > null cipher OOP
> >
> > NULL cipher is used for validating auth only cases. With out of place
> > processing, validating plain text should not be done as the PMD is
> > only expected to update auth data.
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > ---
> >  app/test/test_cryptodev.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > index e54a1a9..964f44f 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -7490,6 +7490,22 @@ test_mixed_auth_cipher(const struct
> > mixed_cipher_auth_test_data *tdata,
> >  				tdata->digest_enc.len);
> >  	}
> >
> > +	/*
> > +	 * NULL cipher is used for auth only cases where only authentication
> > +	 * is done. With verify operation, MAC would be validated by the
> PMD.
> > +	 * With generate operation, verify MAC generated by the PMD.
> > +	 */
> > +	if (op_mode == OUT_OF_PLACE &&
> > +	    tdata->cipher_algo == RTE_CRYPTO_CIPHER_NULL) {
> 
> Why only checking for OUT_OF_PLACE? As far as cipher algorithm is NULL,
> only digest should be checked.

[Anoob] Agreed. I will make this change in v2.
 
> Also, looking at the code, there is this same check, but with hardcoded tag
> length.
> Could you rearrange the code to have less lines and generic?

[Anoob] Yes. This looks better and the code would be self-explanatory as well. Will make this change in v2.

> 
> Something like:
> 
> <pseudo-code>
> if (!verify)
> 	check_digest
> 
> if (cipher_algo != NULL)
> 	check_ciphertext/plaintext
> 
> check op status
> </pseudo-code>
> 
> Also, I think this change is applicable in test_mixed_auth_cipher_sgl.

[Anoob] Yes. I'll make that change as well.

> 
> Thanks,
> Pablo