[PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
De Lara Guarch, Pablo
pablo.de.lara.guarch at intel.com
Tue Mar 22 13:25:42 CET 2022
Hi Kevin/Luca,
> -----Original Message-----
> From: Kevin Traynor <ktraynor at redhat.com>
> Sent: Tuesday, March 22, 2022 10:40 AM
> To: Luca Boccassi <bluca at debian.org>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>
> Cc: stable at dpdk.org
> Subject: Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
>
> On 21/03/2022 22:12, Luca Boccassi wrote:
> > On Mon, 21 Mar 2022 at 19:57, De Lara Guarch, Pablo
> > <pablo.de.lara.guarch at intel.com> wrote:
> >>
> >> Hi Luca,
> >>
> >>> -----Original Message-----
> >>> From: Luca Boccassi <bluca at debian.org>
> >>> Sent: Monday, March 14, 2022 12:20 PM
> >>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>;
> >>> stable at dpdk.org
> >>> Subject: Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and
> >>> offset settings
> >>>
> >>> On Mon, 2022-03-14 at 11:05 +0000, Pablo de Lara wrote:
> >>>> [ upstream commit a501609ea6466ed8526c0dfadedee332a4d4a451 ]
> >>>>
> >>>> KASUMI, SNOW3G and ZUC require lengths and offsets to be set in
> >>>> bits or bytes depending on the algorithm.
> >>>> There were some algorithms that were mixing these two, so this
> >>>> commit is fixing this issue.
> >>>>
> >>>> Fixes: ae8e085c608d ("crypto/aesni_mb: support KASUMI F8/F9")
> >>>> Fixes: 6c42e0cf4d12 ("crypto/aesni_mb: support SNOW3G-UEA2/UIA2")
> >>>> Fixes: fd8df85487c4 ("crypto/aesni_mb: support ZUC-EEA3/EIA3")
> >>>>
> >>>> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch at intel.com>
> >>>> ---
> >>>> drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 121
> >>>> +++++++++++++++------
> >>>> 1 file changed, 86 insertions(+), 35 deletions(-)
> >>>>
> >>>> diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> >>>> b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> >>>> index f4ffb21e10..07f5caa76f 100644
> >>>> --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> >>>> +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> >>>> @@ -1057,7 +1057,9 @@ get_session(struct aesni_mb_qp *qp, struct
> >>>> rte_crypto_op *op)
> >>>>
> >>>>
> >>>> static inline uint64_t
> >>>> auth_start_offset(struct rte_crypto_op *op, struct
> >>>> aesni_mb_session
> >>> *session,
> >>>> - uint32_t oop)
> >>>> + uint32_t oop, const uint32_t auth_offset,
> >>>> + const uint32_t cipher_offset, const uint32_t auth_length,
> >>>> + const uint32_t cipher_length)
> >>>> {
> >>>> struct rte_mbuf *m_src, *m_dst;
> >>>> uint8_t *p_src, *p_dst;
> >>>> @@ -1066,7 +1068,7 @@ auth_start_offset(struct rte_crypto_op *op,
> >>>> struct aesni_mb_session *session,
> >>>>
> >>>>
> >>>> /* Only cipher then hash needs special calculation. */
> >>>> if (!oop || session->chain_order != CIPHER_HASH)
> >>>> - return op->sym->auth.data.offset;
> >>>> + return auth_offset;
> >>>>
> >>>>
> >>>> m_src = op->sym->m_src;
> >>>> m_dst = op->sym->m_dst;
> >>>> @@ -1074,24 +1076,23 @@ auth_start_offset(struct rte_crypto_op *op,
> >>> struct aesni_mb_session *session,
> >>>> p_src = rte_pktmbuf_mtod(m_src, uint8_t *);
> >>>> p_dst = rte_pktmbuf_mtod(m_dst, uint8_t *);
> >>>> u_src = (uintptr_t)p_src;
> >>>> - u_dst = (uintptr_t)p_dst + op->sym->auth.data.offset;
> >>>> + u_dst = (uintptr_t)p_dst + auth_offset;
> >>>>
> >>>>
> >>>> /**
> >>>> * Copy the content between cipher offset and auth offset for
> >>> generating
> >>>> * correct digest.
> >>>> */
> >>>> - if (op->sym->cipher.data.offset > op->sym->auth.data.offset)
> >>>> - memcpy(p_dst + op->sym->auth.data.offset,
> >>>> - p_src + op->sym->auth.data.offset,
> >>>> - op->sym->cipher.data.offset -
> >>>> - op->sym->auth.data.offset);
> >>>> -
> >>>> + if (cipher_offset > auth_offset)
> >>>> + memcpy(p_dst + auth_offset,
> >>>> + p_src + auth_offset,
> >>>> + cipher_offset -
> >>>> + auth_offset);
> >>>> /**
> >>>> * Copy the content between (cipher offset + length) and (auth offset +
> >>>> * length) for generating correct digest
> >>>> */
> >>>> - cipher_end = op->sym->cipher.data.offset + op->sym-
> >>>> cipher.data.length;
> >>>> - auth_end = op->sym->auth.data.offset + op->sym->auth.data.length;
> >>>> + cipher_end = cipher_offset + cipher_length;
> >>>> + auth_end = auth_offset + auth_length;
> >>>> if (cipher_end < auth_end)
> >>>> memcpy(p_dst + cipher_end, p_src + cipher_end,
> >>>> auth_end - cipher_end); @@ -1246,6
> >>>> +1247,10 @@ set_mb_job_params(JOB_AES_HMAC *job, struct
> >>> aesni_mb_qp *qp,
> >>>> struct rte_mbuf *m_src = op->sym->m_src, *m_dst;
> >>>> struct aesni_mb_session *session;
> >>>> uint32_t m_offset, oop;
> >>>> + uint32_t auth_off_in_bytes;
> >>>> + uint32_t ciph_off_in_bytes;
> >>>> + uint32_t auth_len_in_bytes;
> >>>> + uint32_t ciph_len_in_bytes;
> >>>>
> >>>>
> >>>> session = get_session(qp, op);
> >>>> if (session == NULL) {
> >>>> @@ -1362,6 +1367,7 @@ set_mb_job_params(JOB_AES_HMAC *job,
> struct
> >>> aesni_mb_qp *qp,
> >>>> if (job->cipher_mode == IMB_CIPHER_ZUC_EEA3) {
> >>>> job->aes_enc_key_expanded = session->cipher.zuc_cipher_key;
> >>>> job->aes_dec_key_expanded =
> >>>> session->cipher.zuc_cipher_key;
> >>>> + m_offset >>= 3;
> >>>> } else if (job->cipher_mode == IMB_CIPHER_SNOW3G_UEA2_BITLEN) {
> >>>> job->enc_keys = &session->cipher.pKeySched_snow3g_cipher;
> >>>> m_offset = 0;
> >>>> @@ -1418,9 +1424,6 @@ set_mb_job_params(JOB_AES_HMAC *job,
> struct
> >>>> aesni_mb_qp *qp,
> >>>>
> >>>>
> >>>> switch (job->hash_alg) {
> >>>> case AES_CCM:
> >>>> - job->cipher_start_src_offset_in_bytes =
> >>>> - op->sym->aead.data.offset;
> >>>> - job->msg_len_to_cipher_in_bytes = op->sym-
> >>>> aead.data.length;
> >>>> job->hash_start_src_offset_in_bytes = op->sym-
> >>>> aead.data.offset;
> >>>> job->msg_len_to_hash_in_bytes =
> >>>> op->sym->aead.data.length;
> >>>>
> >>>>
> >>>> @@ -1430,19 +1433,11 @@ set_mb_job_params(JOB_AES_HMAC *job,
> struct
> >>>> aesni_mb_qp *qp,
> >>>>
> >>>>
> >>>> case AES_GMAC:
> >>>> if (session->cipher.mode == GCM) {
> >>>> - job->cipher_start_src_offset_in_bytes =
> >>>> - op->sym->aead.data.offset;
> >>>> job->hash_start_src_offset_in_bytes =
> >>>> op->sym->aead.data.offset;
> >>>> - job->msg_len_to_cipher_in_bytes =
> >>>> - op->sym->aead.data.length;
> >>>> job->msg_len_to_hash_in_bytes =
> >>>> op->sym->aead.data.length;
> >>>> } else {
> >>>> - job->cipher_start_src_offset_in_bytes =
> >>>> - op->sym->auth.data.offset;
> >>>> - job->hash_start_src_offset_in_bytes =
> >>>> - op->sym->auth.data.offset;
> >>>> job->msg_len_to_cipher_in_bytes = 0;
> >>>> job->msg_len_to_hash_in_bytes = 0;
> >>>> }
> >>>> @@ -1453,10 +1448,7 @@ set_mb_job_params(JOB_AES_HMAC *job,
> struct
> >>>> aesni_mb_qp *qp,
> >>>>
> >>>>
> >>>> #if IMB_VERSION(0, 54, 3) <= IMB_VERSION_NUM
> >>>> case IMB_AUTH_CHACHA20_POLY1305:
> >>>> - job->cipher_start_src_offset_in_bytes = op->sym-
> >>>> aead.data.offset;
> >>>> job->hash_start_src_offset_in_bytes = op->sym-
> >>>> aead.data.offset;
> >>>> - job->msg_len_to_cipher_in_bytes =
> >>>> - op->sym->aead.data.length;
> >>>> job->msg_len_to_hash_in_bytes =
> >>>> op->sym->aead.data.length;
> >>>>
> >>>>
> >>>> @@ -1464,26 +1456,85 @@ set_mb_job_params(JOB_AES_HMAC *job,
> struct
> >>> aesni_mb_qp *qp,
> >>>> session->iv.offset);
> >>>> break;
> >>>> #endif
> >>>> - default:
> >>>> - /* For SNOW3G, length and offsets are already in bits */
> >>>> - job->cipher_start_src_offset_in_bytes =
> >>>> - op->sym->cipher.data.offset;
> >>>> - job->msg_len_to_cipher_in_bytes = op->sym-
> >>>> cipher.data.length;
> >>>> +#if IMB_VERSION(0, 53, 3) <= IMB_VERSION_NUM
> >>>> + /* ZUC and SNOW3G require length in bits and offset in bytes */
> >>>> + case IMB_AUTH_ZUC_EIA3_BITLEN:
> >>>> + case IMB_AUTH_SNOW3G_UIA2_BITLEN:
> >>>> + auth_off_in_bytes = op->sym->auth.data.offset >> 3;
> >>>> + ciph_off_in_bytes = op->sym->cipher.data.offset >> 3;
> >>>> + auth_len_in_bytes = op->sym->auth.data.length >> 3;
> >>>> + ciph_len_in_bytes = op->sym->cipher.data.length >> 3;
> >>>> +
> >>>> + job->hash_start_src_offset_in_bytes = auth_start_offset(op,
> >>>> + session, oop, auth_off_in_bytes,
> >>>> + ciph_off_in_bytes, auth_len_in_bytes,
> >>>> + ciph_len_in_bytes);
> >>>> + job->msg_len_to_hash_in_bits =
> >>>> + op->sym->auth.data.length;
> >>>> +
> >>>> + job->iv = rte_crypto_op_ctod_offset(op, uint8_t *,
> >>>> + session->iv.offset);
> >>>> + break;
> >>>> +
> >>>> + /* KASUMI requires lengths and offset in bytes */
> >>>> + case IMB_AUTH_KASUMI_UIA1:
> >>>> + auth_off_in_bytes = op->sym->auth.data.offset >> 3;
> >>>> + ciph_off_in_bytes = op->sym->cipher.data.offset >> 3;
> >>>> + auth_len_in_bytes = op->sym->auth.data.length >> 3;
> >>>> + ciph_len_in_bytes = op->sym->cipher.data.length >> 3;
> >>>>
> >>>>
> >>>> job->hash_start_src_offset_in_bytes = auth_start_offset(op,
> >>>> - session, oop);
> >>>> + session, oop, auth_off_in_bytes,
> >>>> + ciph_off_in_bytes, auth_len_in_bytes,
> >>>> + ciph_len_in_bytes);
> >>>> + job->msg_len_to_hash_in_bytes = auth_len_in_bytes;
> >>>> +
> >>>> + job->iv = rte_crypto_op_ctod_offset(op, uint8_t *,
> >>>> + session->iv.offset);
> >>>> + break;
> >>>> +#endif
> >>>> +
> >>>> + default:
> >>>> + job->hash_start_src_offset_in_bytes = auth_start_offset(op,
> >>>> + session, oop, op->sym->auth.data.offset,
> >>>> + op->sym->cipher.data.offset,
> >>>> + op->sym->auth.data.length,
> >>>> + op->sym->cipher.data.length);
> >>>> job->msg_len_to_hash_in_bytes =
> >>>> op->sym->auth.data.length;
> >>>>
> >>>>
> >>>> job->iv = rte_crypto_op_ctod_offset(op, uint8_t *,
> >>>> session->iv.offset);
> >>>> }
> >>>>
> >>>>
> >>>> + switch (job->cipher_mode) {
> >>>> #if IMB_VERSION(0, 53, 3) <= IMB_VERSION_NUM
> >>>> - if (job->cipher_mode == IMB_CIPHER_ZUC_EEA3)
> >>>> - job->msg_len_to_cipher_in_bytes >>= 3;
> >>>> - else if (job->hash_alg == IMB_AUTH_KASUMI_UIA1)
> >>>> - job->msg_len_to_hash_in_bytes >>= 3;
> >>>> + /* ZUC requires length and offset in bytes */
> >>>> + case IMB_CIPHER_ZUC_EEA3:
> >>>> + job->cipher_start_src_offset_in_bytes =
> >>>> + op->sym->cipher.data.offset >> 3;
> >>>> + job->msg_len_to_cipher_in_bytes =
> >>>> + op->sym->cipher.data.length >> 3;
> >>>> + break;
> >>>> + /* ZUC and SNOW3G require length and offset in bits */
> >>>> + case IMB_CIPHER_SNOW3G_UEA2_BITLEN:
> >>>> + case IMB_CIPHER_KASUMI_UEA1_BITLEN:
> >>>> + job->cipher_start_src_offset_in_bits =
> >>>> + op->sym->cipher.data.offset;
> >>>> + job->msg_len_to_cipher_in_bits =
> >>>> + op->sym->cipher.data.length;
> >>>> + break;
> >>>> #endif
> >>>> + case IMB_CIPHER_CCM:
> >>>> + case IMB_CIPHER_GCM:
> >>>> + case IMB_CIPHER_CHACHA20_POLY1305:
> >>>> + job->cipher_start_src_offset_in_bytes =
> >>>> + op->sym->aead.data.offset;
> >>>> + job->msg_len_to_cipher_in_bytes = op->sym-
> >>>> aead.data.length;
> >>>> + break;
> >>>> + default:
> >>>> + job->cipher_start_src_offset_in_bytes =
> >>>> + op->sym->cipher.data.offset;
> >>>> + job->msg_len_to_cipher_in_bytes = op->sym-
> >>>> cipher.data.length;
> >>>> + }
> >>>>
> >>>>
> >>>> /* Set user data to be crypto operation data struct */
> >>>> job->user_data = op;
> >>>
> >>> This breaks the build on Ubuntu 20.04:
> >>>
> >>> [ 1944s] ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c: In function
> >>> ‘set_mb_job_params’:
> >>> [ 1944s] ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c:1526:7: error:
> >>> ‘IMB_CIPHER_GCM’ undeclared (first use in this function) [ 1944s]
> >>> 1526 | case
> >>> IMB_CIPHER_GCM:
> >>> [ 1944s] | ^~~~~~~~~~~~~~
> >>> [ 1944s] ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c:1526:7: note:
> >>> each undeclared identifier is reported only once for each function
> >>> it appears in [ 1944s]
> ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c:1527:31: error:
> >>> ‘IMB_CIPHER_NULL’ undeclared (first use in this function)
> >>> [ 1944s] 1527 | if (session->cipher.mode == IMB_CIPHER_NULL) {
> >>> [ 1944s] | ^~~~~~~~~~~~~~~
> >>> [ 1944s] ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c:1538:7: error:
> >>> ‘IMB_CIPHER_CCM’ undeclared (first use in this function) [ 1944s]
> >>> 1538 | case
> >>> IMB_CIPHER_CCM:
> >>> [ 1944s] | ^~~~~~~~~~~~~~
> >>> [ 1944s] ../drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c:1539:7: error:
> >>> ‘IMB_CIPHER_CHACHA20_POLY1305’ undeclared (first use in this
> >>> function); did you mean ‘RTE_CRYPTO_AEAD_CHACHA20_POLY1305’?
> >>> [ 1944s] 1539 | case IMB_CIPHER_CHACHA20_POLY1305:
> >>> [ 1944s] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> [ 1944s] | RTE_CRYPTO_AEAD_CHACHA20_POLY1305
> >>>
> >>> https://build.opensuse.org/package/live_build_log/home:bluca:dpdk/dp
> >>> dk-
> >>> 20.11/Ubuntu_20.04/x86_64
> >>
> >> Apologies, I did not see this till today. I don't think this patch is actually
> breaking the build.
> >> To me, it looks like an environment issue (IMB_CIPHER_GCM for instance is
> already present in the code).
> >> I wonder if that system has the right version of IPSec Multi buffer library.
> >> Only versions from v1.0 onwards are supported.
> >>
> >> Thanks,
> >> Pablo
> >
> > In 20.11 ipsec-mb v0.52.0 and over are supported:
> >
> > https://git.dpdk.org/dpdk-stable/tree/drivers/crypto/aesni_mb/meson.bu
> > ild?h=20.11#n4
> >
> > If this patch requires a new version and breaks backward
> > compatibility, then it does not appear to be not suitable for
> > backporting.
> >
>
> +1
>
> Just to confirm for 21.11 there was already a dependency of 1.0.0, so patches
> would be suitable for it.
Got it! So I will submit fixed versions for 20.11 and second patch for 21.11.
Thanks,
Pablo
More information about the stable
mailing list