patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
@ 2022-03-22 13:39 Pablo de Lara
  2022-03-22 13:39 ` [PATCH 20.11 2/2] crypto/ipsec_mb: fix GMAC parameters setting Pablo de Lara
  0 siblings, 1 reply; 13+ messages in thread
From: Pablo de Lara @ 2022-03-22 13:39 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable, Pablo de Lara

[ 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@intel.com>
---
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 126 +++++++++++++++------
 1 file changed, 90 insertions(+), 36 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..ab9864739d 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,7 +1247,12 @@ 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;
-
+#if IMB_VERSION(0, 53, 3) <= IMB_VERSION_NUM
+	uint32_t auth_off_in_bytes;
+	uint32_t ciph_off_in_bytes;
+	uint32_t auth_len_in_bytes;
+	uint32_t ciph_len_in_bytes;
+#endif
 	session = get_session(qp, op);
 	if (session == NULL) {
 		op->status = RTE_CRYPTO_OP_STATUS_INVALID_SESSION;
@@ -1362,6 +1368,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 +1425,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 +1434,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 +1449,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 +1457,87 @@ 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);
+				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, 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 CCM:
+	case GCM:
+#if IMB_VERSION(0, 54, 3) <= IMB_VERSION_NUM
+	case IMB_CIPHER_CHACHA20_POLY1305:
 #endif
+		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;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 20.11 2/2] crypto/ipsec_mb: fix GMAC parameters setting
  2022-03-22 13:39 [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings Pablo de Lara
@ 2022-03-22 13:39 ` Pablo de Lara
  2022-04-04 13:35   ` Luca Boccassi
  0 siblings, 1 reply; 13+ messages in thread
From: Pablo de Lara @ 2022-03-22 13:39 UTC (permalink / raw)
  To: luca.boccassi; +Cc: stable, Pablo de Lara, Fan Zhang, Radu Nicolau

[ upstream commit 837269c2e5c5a8813adfcf59f23b80569048ddeb ]
AES-GMAC requires plaintext length to be 0 when using AES-GCM,
so only AAD data is used.

Fixes: a501609ea646 ("crypto/ipsec_mb: fix length and offset settings")
Cc: pablo.de.lara.guarch@intel.com
Cc: stable@dpdk.org

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
Tested-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index ab9864739d..94055d8177 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -1438,9 +1438,9 @@ set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
 					op->sym->aead.data.offset;
 			job->msg_len_to_hash_in_bytes =
 					op->sym->aead.data.length;
-		} else {
-			job->msg_len_to_cipher_in_bytes = 0;
+		} else { /* AES-GMAC only, only AAD used */
 			job->msg_len_to_hash_in_bytes = 0;
+			job->hash_start_src_offset_in_bytes = 0;
 		}
 
 		job->iv = rte_crypto_op_ctod_offset(op, uint8_t *,
@@ -1524,8 +1524,19 @@ set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
 					op->sym->cipher.data.length;
 		break;
 #endif
-	case CCM:
 	case GCM:
+		if (session->cipher.mode == NULL_CIPHER) {
+			/* AES-GMAC only (only AAD used) */
+			job->msg_len_to_cipher_in_bytes = 0;
+			job->cipher_start_src_offset_in_bytes = 0;
+		} else {
+			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;
+	case CCM:
 #if IMB_VERSION(0, 54, 3) <= IMB_VERSION_NUM
 	case IMB_CIPHER_CHACHA20_POLY1305:
 #endif
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 20.11 2/2] crypto/ipsec_mb: fix GMAC parameters setting
  2022-03-22 13:39 ` [PATCH 20.11 2/2] crypto/ipsec_mb: fix GMAC parameters setting Pablo de Lara
@ 2022-04-04 13:35   ` Luca Boccassi
  0 siblings, 0 replies; 13+ messages in thread
From: Luca Boccassi @ 2022-04-04 13:35 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: stable, Fan Zhang, Radu Nicolau

On Tue, 2022-03-22 at 13:39 +0000, Pablo de Lara wrote:
> [ upstream commit 837269c2e5c5a8813adfcf59f23b80569048ddeb ]
> AES-GMAC requires plaintext length to be 0 when using AES-GCM,
> so only AAD data is used.
> 
> Fixes: a501609ea646 ("crypto/ipsec_mb: fix length and offset settings")
> Cc: pablo.de.lara.guarch@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> Acked-by: Radu Nicolau <radu.nicolau@intel.com>
> Tested-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
>  drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)

Thanks, series queued for 20.11.6.

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-22 10:39       ` Kevin Traynor
@ 2022-03-22 12:25         ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 13+ messages in thread
From: De Lara Guarch, Pablo @ 2022-03-22 12:25 UTC (permalink / raw)
  To: Kevin Traynor, Luca Boccassi; +Cc: stable

Hi Kevin/Luca,

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Tuesday, March 22, 2022 10:40 AM
> To: Luca Boccassi <bluca@debian.org>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: stable@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@intel.com> wrote:
> >>
> >> Hi Luca,
> >>
> >>> -----Original Message-----
> >>> From: Luca Boccassi <bluca@debian.org>
> >>> Sent: Monday, March 14, 2022 12:20 PM
> >>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> >>> stable@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@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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-21 22:12     ` Luca Boccassi
@ 2022-03-22 10:39       ` Kevin Traynor
  2022-03-22 12:25         ` De Lara Guarch, Pablo
  0 siblings, 1 reply; 13+ messages in thread
From: Kevin Traynor @ 2022-03-22 10:39 UTC (permalink / raw)
  To: Luca Boccassi, De Lara Guarch, Pablo; +Cc: stable

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@intel.com> wrote:
>>
>> Hi Luca,
>>
>>> -----Original Message-----
>>> From: Luca Boccassi <bluca@debian.org>
>>> Sent: Monday, March 14, 2022 12:20 PM
>>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; stable@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@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/dpdk-
>>> 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.build?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.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-21 20:02     ` De Lara Guarch, Pablo
@ 2022-03-22 10:39       ` Kevin Traynor
  0 siblings, 0 replies; 13+ messages in thread
From: Kevin Traynor @ 2022-03-22 10:39 UTC (permalink / raw)
  To: De Lara Guarch, Pablo, luca.boccassi, stable

On 21/03/2022 20:02, De Lara Guarch, Pablo wrote:
> Hi Kevin,
> 
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Tuesday, March 15, 2022 11:19 AM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> luca.boccassi@gmail.com; stable@dpdk.org
>> Subject: Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
>>
>> On 15/03/2022 11:10, Kevin Traynor wrote:
>>> Hi Pablo,
>>>
>>> On 14/03/2022 11:05, 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@intel.com>
>>>> ---
>>>
>>> For 21.11. I backported and applied a501609ea6466 from dpdk main
>>> branch to 21.11 branch [0]. The version sent here directly to 20.11 is
>>> quite different as it adds the 'switch (job->cipher_mode)'. (It feels
>>> like some other patch adding 'switch (job->cipher_mode)' was missed
>>> for
>>> backported)
>>>
>>> As this switch is not in [0], the following GMAC patch [1] won't apply
>>> for 21.11.
>>>
>>> One option is that you send a patch to just add the missing parts, and
>>> then I apply the GMAC patch.
>>>
>>> But the simplest option is that I revert [0], apply updated 20.11
>>> version + GMAC patch.
>>>
>>> I'll go with the latter option (assuming they apply cleanly), but
>>> please let me know if that sounds ok.
>>>
>>
>> I just noticed Luca is reporting build failures. In that case, please send these 2
>> patches rebased for 21.11 branch as well. If the same patches work for
>> 20.11/21.11 you can indicate in the subject-prefix. I will revert [0] and apply
>> those.
>>
> 
> I'll wait for the build issue on 20.11 to be resolved (I think it's system environment related),
> but I think all we need is the second patch (crypto/ipsec_mb: fix GMAC parameters setting).
> I can send it as soon as this is done.
> 

Sure, i'll hold off the revert and wait for rework of GMAC patch. You 
can base rework off the 21.11 branch on dpdk-stable tree on dpdk.org as 
it is almost fully up to date.

thanks,
Kevin.

> Thanks,
> Pablo
> 
>>> thanks,
>>> Kevin
>>>
>>> [0]
>>> http://git.dpdk.org/dpdk-stable/commit/?h=21.11&id=daf06c45e8576d8a2a8
>>> 6455e33e2bc4574f8f8b4
>>>
>>> [1] 837269c2e5c5 ("crypto/ipsec_mb: fix GMAC parameters setting")
>>>
>>>
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-21 19:57   ` De Lara Guarch, Pablo
@ 2022-03-21 22:12     ` Luca Boccassi
  2022-03-22 10:39       ` Kevin Traynor
  0 siblings, 1 reply; 13+ messages in thread
From: Luca Boccassi @ 2022-03-21 22:12 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: stable

On Mon, 21 Mar 2022 at 19:57, De Lara Guarch, Pablo
<pablo.de.lara.guarch@intel.com> wrote:
>
> Hi Luca,
>
> > -----Original Message-----
> > From: Luca Boccassi <bluca@debian.org>
> > Sent: Monday, March 14, 2022 12:20 PM
> > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; stable@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@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/dpdk-
> > 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.build?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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-15 11:18   ` Kevin Traynor
@ 2022-03-21 20:02     ` De Lara Guarch, Pablo
  2022-03-22 10:39       ` Kevin Traynor
  0 siblings, 1 reply; 13+ messages in thread
From: De Lara Guarch, Pablo @ 2022-03-21 20:02 UTC (permalink / raw)
  To: Kevin Traynor, luca.boccassi, stable

Hi Kevin,

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Tuesday, March 15, 2022 11:19 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> luca.boccassi@gmail.com; stable@dpdk.org
> Subject: Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
> 
> On 15/03/2022 11:10, Kevin Traynor wrote:
> > Hi Pablo,
> >
> > On 14/03/2022 11:05, 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@intel.com>
> >> ---
> >
> > For 21.11. I backported and applied a501609ea6466 from dpdk main
> > branch to 21.11 branch [0]. The version sent here directly to 20.11 is
> > quite different as it adds the 'switch (job->cipher_mode)'. (It feels
> > like some other patch adding 'switch (job->cipher_mode)' was missed
> > for
> > backported)
> >
> > As this switch is not in [0], the following GMAC patch [1] won't apply
> > for 21.11.
> >
> > One option is that you send a patch to just add the missing parts, and
> > then I apply the GMAC patch.
> >
> > But the simplest option is that I revert [0], apply updated 20.11
> > version + GMAC patch.
> >
> > I'll go with the latter option (assuming they apply cleanly), but
> > please let me know if that sounds ok.
> >
> 
> I just noticed Luca is reporting build failures. In that case, please send these 2
> patches rebased for 21.11 branch as well. If the same patches work for
> 20.11/21.11 you can indicate in the subject-prefix. I will revert [0] and apply
> those.
> 

I'll wait for the build issue on 20.11 to be resolved (I think it's system environment related),
but I think all we need is the second patch (crypto/ipsec_mb: fix GMAC parameters setting).
I can send it as soon as this is done.

Thanks,
Pablo

> > thanks,
> > Kevin
> >
> > [0]
> > http://git.dpdk.org/dpdk-stable/commit/?h=21.11&id=daf06c45e8576d8a2a8
> > 6455e33e2bc4574f8f8b4
> >
> > [1] 837269c2e5c5 ("crypto/ipsec_mb: fix GMAC parameters setting")
> >
> >


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-14 12:20 ` Luca Boccassi
@ 2022-03-21 19:57   ` De Lara Guarch, Pablo
  2022-03-21 22:12     ` Luca Boccassi
  0 siblings, 1 reply; 13+ messages in thread
From: De Lara Guarch, Pablo @ 2022-03-21 19:57 UTC (permalink / raw)
  To: Luca Boccassi, stable

Hi Luca,

> -----Original Message-----
> From: Luca Boccassi <bluca@debian.org>
> Sent: Monday, March 14, 2022 12:20 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; stable@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@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/dpdk-
> 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

> 
> --
> Kind regards,
> Luca Boccassi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-15 11:10 ` Kevin Traynor
@ 2022-03-15 11:18   ` Kevin Traynor
  2022-03-21 20:02     ` De Lara Guarch, Pablo
  0 siblings, 1 reply; 13+ messages in thread
From: Kevin Traynor @ 2022-03-15 11:18 UTC (permalink / raw)
  To: Pablo de Lara, luca.boccassi, stable

On 15/03/2022 11:10, Kevin Traynor wrote:
> Hi Pablo,
> 
> On 14/03/2022 11:05, 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@intel.com>
>> ---
> 
> For 21.11. I backported and applied a501609ea6466 from dpdk main branch
> to 21.11 branch [0]. The version sent here directly to 20.11 is quite
> different as it adds the 'switch (job->cipher_mode)'. (It feels like
> some other patch adding 'switch (job->cipher_mode)' was missed for
> backported)
> 
> As this switch is not in [0], the following GMAC patch [1] won't apply
> for 21.11.
> 
> One option is that you send a patch to just add the missing parts, and
> then I apply the GMAC patch.
> 
> But the simplest option is that I revert [0], apply updated 20.11
> version + GMAC patch.
> 
> I'll go with the latter option (assuming they apply cleanly), but please
> let me know if that sounds ok.
> 

I just noticed Luca is reporting build failures. In that case, please 
send these 2 patches rebased for 21.11 branch as well. If the same 
patches work for 20.11/21.11 you can indicate in the subject-prefix. I 
will revert [0] and apply those.

> thanks,
> Kevin
> 
> [0]
> http://git.dpdk.org/dpdk-stable/commit/?h=21.11&id=daf06c45e8576d8a2a86455e33e2bc4574f8f8b4
> 
> [1] 837269c2e5c5 ("crypto/ipsec_mb: fix GMAC parameters setting")
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-14 11:05 [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings Pablo de Lara
  2022-03-14 12:20 ` Luca Boccassi
@ 2022-03-15 11:10 ` Kevin Traynor
  2022-03-15 11:18   ` Kevin Traynor
  1 sibling, 1 reply; 13+ messages in thread
From: Kevin Traynor @ 2022-03-15 11:10 UTC (permalink / raw)
  To: Pablo de Lara, luca.boccassi, stable

Hi Pablo,

On 14/03/2022 11:05, 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@intel.com>
> ---

For 21.11. I backported and applied a501609ea6466 from dpdk main branch 
to 21.11 branch [0]. The version sent here directly to 20.11 is quite 
different as it adds the 'switch (job->cipher_mode)'. (It feels like 
some other patch adding 'switch (job->cipher_mode)' was missed for 
backported)

As this switch is not in [0], the following GMAC patch [1] won't apply 
for 21.11.

One option is that you send a patch to just add the missing parts, and 
then I apply the GMAC patch.

But the simplest option is that I revert [0], apply updated 20.11 
version + GMAC patch.

I'll go with the latter option (assuming they apply cleanly), but please 
let me know if that sounds ok.

thanks,
Kevin

[0] 
http://git.dpdk.org/dpdk-stable/commit/?h=21.11&id=daf06c45e8576d8a2a86455e33e2bc4574f8f8b4

[1] 837269c2e5c5 ("crypto/ipsec_mb: fix GMAC parameters setting")



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
  2022-03-14 11:05 [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings Pablo de Lara
@ 2022-03-14 12:20 ` Luca Boccassi
  2022-03-21 19:57   ` De Lara Guarch, Pablo
  2022-03-15 11:10 ` Kevin Traynor
  1 sibling, 1 reply; 13+ messages in thread
From: Luca Boccassi @ 2022-03-14 12:20 UTC (permalink / raw)
  To: Pablo de Lara, stable

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@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/dpdk-20.11/Ubuntu_20.04/x86_64

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings
@ 2022-03-14 11:05 Pablo de Lara
  2022-03-14 12:20 ` Luca Boccassi
  2022-03-15 11:10 ` Kevin Traynor
  0 siblings, 2 replies; 13+ messages in thread
From: Pablo de Lara @ 2022-03-14 11:05 UTC (permalink / raw)
  To: luca.boccassi, stable; +Cc: Pablo de Lara

[ 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@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;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-04-04 13:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-22 13:39 [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings Pablo de Lara
2022-03-22 13:39 ` [PATCH 20.11 2/2] crypto/ipsec_mb: fix GMAC parameters setting Pablo de Lara
2022-04-04 13:35   ` Luca Boccassi
  -- strict thread matches above, loose matches on Subject: below --
2022-03-14 11:05 [PATCH 20.11 1/2] crypto/ipsec_mb: fix length and offset settings Pablo de Lara
2022-03-14 12:20 ` Luca Boccassi
2022-03-21 19:57   ` De Lara Guarch, Pablo
2022-03-21 22:12     ` Luca Boccassi
2022-03-22 10:39       ` Kevin Traynor
2022-03-22 12:25         ` De Lara Guarch, Pablo
2022-03-15 11:10 ` Kevin Traynor
2022-03-15 11:18   ` Kevin Traynor
2022-03-21 20:02     ` De Lara Guarch, Pablo
2022-03-22 10:39       ` Kevin Traynor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).