[PATCH 21.11] Revert "crypto/ipsec_mb: fix length and offset settings"

[PATCH 21.11] Revert "crypto/ipsec_mb: fix length and offset settings"

[Kevin Traynor]

This reverts commit daf06c45e8576d8a2a86455e33e2bc4574f8f8b4.

As noted [0] reverting this commit because it is reported to
introduce a regression [1] and there seems to be some issues [2]
with the fix.

[0] http://inbox.dpdk.org/stable/3a9ab8af-0a11-248d-24e3-37c7bde7b850@redhat.com/
[1] http://inbox.dpdk.org/stable/DM8PR11MB559167EBA31F8DFEEDE9B51584099@DM8PR11MB5591.namprd11.prod.outlook.com/
[2] http://inbox.dpdk.org/stable/f9030407a47a8554b2c09426c5bc40049647db2d.camel@debian.org/

Fixes: daf06c45e857 ("crypto/ipsec_mb: fix length and offset settings")

Cc: pablo.de.lara.guarch@intel.com
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 drivers/crypto/ipsec_mb/pmd_aesni_mb.c | 93 ++++++++++----------------
 1 file changed, 36 insertions(+), 57 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
index 748382320b..a308d42ffa 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
@@ -919,7 +919,5 @@ aesni_mb_set_docsis_sec_session_parameters(
 static inline uint64_t
 auth_start_offset(struct rte_crypto_op *op, struct aesni_mb_session *session,
-		uint32_t oop, const uint32_t auth_offset,
-		const uint32_t cipher_offset, const uint32_t auth_length,
-		const uint32_t cipher_length)
+		uint32_t oop)
 {
 	struct rte_mbuf *m_src, *m_dst;
@@ -930,5 +928,5 @@ 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 != IMB_ORDER_CIPHER_HASH)
-		return auth_offset;
+		return op->sym->auth.data.offset;
 
 	m_src = op->sym->m_src;
@@ -938,5 +936,5 @@ auth_start_offset(struct rte_crypto_op *op, struct aesni_mb_session *session,
 	p_dst = rte_pktmbuf_mtod(m_dst, uint8_t *);
 	u_src = (uintptr_t)p_src;
-	u_dst = (uintptr_t)p_dst + auth_offset;
+	u_dst = (uintptr_t)p_dst + op->sym->auth.data.offset;
 
 	/**
@@ -944,9 +942,9 @@ auth_start_offset(struct rte_crypto_op *op, struct aesni_mb_session *session,
 	 * correct digest.
 	 */
-	if (cipher_offset > auth_offset)
-		memcpy(p_dst + auth_offset,
-				p_src + auth_offset,
-				cipher_offset -
-				auth_offset);
+	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);
 
 	/**
@@ -954,6 +952,6 @@ auth_start_offset(struct rte_crypto_op *op, struct aesni_mb_session *session,
 	 * length) for generating correct digest
 	 */
-	cipher_end = cipher_offset + cipher_length;
-	auth_end = auth_offset + auth_length;
+	cipher_end = op->sym->cipher.data.offset + op->sym->cipher.data.length;
+	auth_end = op->sym->auth.data.offset + op->sym->auth.data.length;
 	if (cipher_end < auth_end)
 		memcpy(p_dst + cipher_end, p_src + cipher_end,
@@ -1102,8 +1100,4 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 	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 = ipsec_mb_get_session_private(qp, op);
@@ -1214,5 +1208,4 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 		job->enc_keys = session->cipher.zuc_cipher_key;
 		job->dec_keys = 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;
@@ -1272,4 +1265,7 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 	switch (job->hash_alg) {
 	case IMB_AUTH_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;
@@ -1281,9 +1277,17 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 	case IMB_AUTH_AES_GMAC:
 		if (session->cipher.mode == IMB_CIPHER_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;
@@ -1295,6 +1299,10 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 
 	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;
@@ -1303,46 +1311,12 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 				session->iv.offset);
 		break;
-	/* ZUC and SNOW3G require length in bits and offset in bytes */
-	case IMB_AUTH_ZUC_EIA3_BITLEN:
-	case IMB_AUTH_ZUC256_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, 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;
-
 	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;
+
 		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);
+				session, oop);
 		job->msg_len_to_hash_in_bytes = op->sym->auth.data.length;
 
@@ -1351,4 +1325,9 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 	}
 
+	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;
+
 	/* Set user data to be crypto operation data struct */
 	job->user_data = op;
-- 
2.34.1

Re: [PATCH 21.11] Revert "crypto/ipsec_mb: fix length and offset settings"

[Luca Boccassi]

On Mon, 21 Mar 2022 at 15:35, Kevin Traynor <ktraynor@redhat.com> wrote:
>
> This reverts commit daf06c45e8576d8a2a86455e33e2bc4574f8f8b4.
>
> As noted [0] reverting this commit because it is reported to
> introduce a regression [1] and there seems to be some issues [2]
> with the fix.
>
> [0] http://inbox.dpdk.org/stable/3a9ab8af-0a11-248d-24e3-37c7bde7b850@redhat.com/
> [1] http://inbox.dpdk.org/stable/DM8PR11MB559167EBA31F8DFEEDE9B51584099@DM8PR11MB5591.namprd11.prod.outlook.com/
> [2] http://inbox.dpdk.org/stable/f9030407a47a8554b2c09426c5bc40049647db2d.camel@debian.org/
>
> Fixes: daf06c45e857 ("crypto/ipsec_mb: fix length and offset settings")
>
> Cc: pablo.de.lara.guarch@intel.com
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
>  drivers/crypto/ipsec_mb/pmd_aesni_mb.c | 93 ++++++++++----------------
>  1 file changed, 36 insertions(+), 57 deletions(-)

Acked-by: Luca Boccassi <bluca@debian.org>

RE: [PATCH 21.11] Revert "crypto/ipsec_mb: fix length and offset settings"

[De Lara Guarch, Pablo]

Hi Kevin,

> -----Original Message-----
> From: Luca Boccassi <bluca@debian.org>
> Sent: Monday, March 21, 2022 7:13 PM
> To: Kevin Traynor <ktraynor@redhat.com>
> Cc: dpdk stable <stable@dpdk.org>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: Re: [PATCH 21.11] Revert "crypto/ipsec_mb: fix length and offset
> settings"
> 
> On Mon, 21 Mar 2022 at 15:35, Kevin Traynor <ktraynor@redhat.com> wrote:
> >
> > This reverts commit daf06c45e8576d8a2a86455e33e2bc4574f8f8b4.
> >
> > As noted [0] reverting this commit because it is reported to introduce
> > a regression [1] and there seems to be some issues [2] with the fix.
> >
> > [0]
> > http://inbox.dpdk.org/stable/3a9ab8af-0a11-248d-24e3-37c7bde7b850@redh
> > at.com/ [1]
> >
> http://inbox.dpdk.org/stable/DM8PR11MB559167EBA31F8DFEEDE9B51584099
> @DM
> > 8PR11MB5591.namprd11.prod.outlook.com/
> > [2]
> >
> http://inbox.dpdk.org/stable/f9030407a47a8554b2c09426c5bc40049647db2d.
> > camel@debian.org/
> >
> > Fixes: daf06c45e857 ("crypto/ipsec_mb: fix length and offset
> > settings")
> >
> > Cc: pablo.de.lara.guarch@intel.com
> > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> > ---
> >  drivers/crypto/ipsec_mb/pmd_aesni_mb.c | 93
> > ++++++++++----------------
> >  1 file changed, 36 insertions(+), 57 deletions(-)
> 
> Acked-by: Luca Boccassi <bluca@debian.org>

Could you hold this revert for the moment? I think the patch is OK and it's a matter of fixing
the system environment.

Thanks,
Pablo