[dpdk-dev] [PATCH] qat: addition of optimized content descriptor for AES128-SHA1-HMAC
Griffin, John
john.griffin at intel.com
Tue Jun 21 16:05:01 CEST 2016
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, June 21, 2016 2:50 PM
> To: Griffin, John <john.griffin at intel.com>
> Cc: dev at dpdk.org; Jain, Deepak K <deepak.k.jain at intel.com>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch at intel.com>
> Subject: Re: [dpdk-dev] [PATCH] qat: addition of optimized content
> descriptor for AES128-SHA1-HMAC
>
> Hi,
>
> I'm not used to review crypto patches but I think this patch can be improved.
>
> 2016-06-21 13:55, John Griffin:
> > Adding an optimized content descriptor for AES128-SHA1-HMAC to
> improve
> > thoughput performance.
>
> Maybe you can explain how it improves the performance.
[JFG] Can add some text- it's though a reduction in the amount of the session information which
DMA'ed to the device - helps to reduce PCIe b/w. I'll add some text.
>
> > +/*
> > + * Function which will return if it's possible to use the
> > + * optimised content descriptor.
> > + */
> > +int qat_crypto_sym_use_optimized_alg(struct qat_session *session)
> [...]
> > +/*
> > + * Function to create an optimised content descriptor for AES128 SHA1.
> > + */
> > +int qat_crypto_create_optimzed_session(struct qat_session *session,
>
> These function are very specific with a generic name.
> Maybe that CBC AES128 SHA1 or something like that must be part of the
> function name.
> Otherwise you will come with yet another crypto refactoring patch in few
> weeks.
[JFG]
[JFG] Ok - I might not rename, but I'll make the implementation more generic to all it cope better with
the potential addition of other algorithms.
>
> > case ICP_QAT_FW_LA_CMD_CIPHER:
> > - session = qat_crypto_sym_configure_session_cipher(dev, xform,
> session);
> > + session = qat_crypto_sym_configure_session_cipher(dev,
> xform,
> > + session);
> > break;
> > case ICP_QAT_FW_LA_CMD_AUTH:
> > - session = qat_crypto_sym_configure_session_auth(dev, xform,
> session);
> > + session = qat_crypto_sym_configure_session_auth(dev,
> xform,
> > + session);
> > break;
> > case ICP_QAT_FW_LA_CMD_CIPHER_HASH:
> > - session = qat_crypto_sym_configure_session_cipher(dev, xform,
> session);
> > - session = qat_crypto_sym_configure_session_auth(dev, xform,
> session);
> > - break;
> > + session = qat_crypto_sym_configure_session_cipher(dev,
> xform,
> > + session);
> > + session = qat_crypto_sym_configure_session_auth(dev,
> xform,
> > + session);
> > + if (qat_crypto_sym_use_optimized_alg(session))
> > + qat_crypto_sym_configure_optimized_session(dev,
> xform,
> > + session);
> > + break;
> > case ICP_QAT_FW_LA_CMD_HASH_CIPHER:
> > - session = qat_crypto_sym_configure_session_auth(dev, xform,
> session);
> > - session = qat_crypto_sym_configure_session_cipher(dev, xform,
> session);
> > - break;
> > + session = qat_crypto_sym_configure_session_auth(dev,
> xform,
> > + session);
> > + session = qat_crypto_sym_configure_session_cipher(dev,
> xform,
> > + session);
> > + if (qat_crypto_sym_use_optimized_alg(session))
> > + qat_crypto_sym_configure_optimized_session(dev,
> xform,
> > + session);
> > + break;
>
> There is a lot of indent fixing mixed with the addition here.
> 2 patches would make things easier to understand.
[JFG]
[JFG] Ok let me remove from this patch.
>
> > @@ -551,11 +591,11 @@ qat_crypto_sym_configure_session_auth(struct
> rte_cryptodev *dev,
> > auth_xform->algo);
> > goto error_out;
> > }
> > - cipher_xform = qat_get_cipher_xform(xform);
> >
> > if ((session->qat_hash_alg ==
> ICP_QAT_HW_AUTH_ALGO_GALOIS_128) ||
> > (session->qat_hash_alg ==
> > ICP_QAT_HW_AUTH_ALGO_GALOIS_64)) {
> > + cipher_xform = qat_get_cipher_xform(xform);
> > if
> (qat_alg_aead_session_create_content_desc_auth(session,
>
> How this move is related to the patch?
[JFG] It's not tbh - I'll remove from this patch.
More information about the dev
mailing list