Re: [dpdk-dev] [PATCH] test/crypto: add operation status checks

["Dybkowski, AdamX" <adamx.dybkowski@intel.com>]

Hi Fiona.
Answers inline below.

> -----Original Message-----
> From: Trahe, Fiona
> Sent: Friday, 20 December, 2019 13:39
> To: Dybkowski, AdamX <adamx.dybkowski@intel.com>; dev@dpdk.org;
> akhil.goyal@nxp.com; Doherty, Declan <declan.doherty@intel.com>
> Cc: Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [PATCH] test/crypto: add operation status checks
> 
> Hi Adam,
> 
> 
> > -----Original Message-----
> > From: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> > Sent: Friday, December 20, 2019 11:50 AM
> > To: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>;
> > akhil.goyal@nxp.com; Doherty, Declan <declan.doherty@intel.com>
> > Cc: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> > Subject: [PATCH] test/crypto: add operation status checks
> >
> > This patch adds checking of the symmetric crypto operation status that
> > was silently skipped before. It fixes the wireless algorithms session
> > creation (SNOW3G, KASUMI, ZUC) and passing of the digest data for the
> > verification by PMD.
> >
> [Fiona] This should be marked as a fix for backporting

[Adam] OK, will do in v2.

> 
> > Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
> > ---
> >  app/test/test_cryptodev.c | 96
> > +++++++++++++++++++++------------------
> >  1 file changed, 52 insertions(+), 44 deletions(-)
> >
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > index 1b561456d..241a1f97a 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -143,7 +143,7 @@ static struct rte_crypto_op *
> > process_crypto_request(uint8_t dev_id, struct rte_crypto_op *op)  {
> > if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
> > -printf("Error sending packet for encryption");
> > +printf("Error sending packet for encryption\n");
> [Fiona] Can you replace this with RTE_LOG while you're modifying it please

[Adam] OK, will do in v2.

> >  return NULL;
> >  }
> >
> > @@ -152,6 +152,11 @@ process_crypto_request(uint8_t dev_id, struct
> > rte_crypto_op *op)  while (rte_cryptodev_dequeue_burst(dev_id, 0, &op,
> > 1) == 0)  rte_pause();
> >
> > +if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) { RTE_LOG(DEBUG,
> > +USER1, "Operation status %d\n", op->status); return NULL; }
> > +
> [Fiona] are there any negative tests - e.g. that expect to see an auth verify
> failure or invalid session - that would be affected by this? If so should the
> actual status be returned?

[Adam] Few negative tests had to be updated because now in the case of any
op error, the process request function returns NULL. The negative tests now check
for the NULL instead of checking for nonzero op status. This was done in
test_authenticated_decryption_fail_when_corruption, test_authentication_verify_GMAC_fail_when_corruption and test_authentication_verify_fail_when_data_corruption.