DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap
@ 2021-07-13 10:27 Anoob Joseph
  2021-07-13 10:27 ` [dpdk-dev] [PATCH 2/3] net/octeontx2: add locking for inline IPsec tbl updates Anoob Joseph
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Anoob Joseph @ 2021-07-13 10:27 UTC (permalink / raw)
  To: Akhil Goyal, Jerin Jacob
  Cc: Anoob Joseph, Ankur Dwivedi, Tejasree Kondoj, dev

The member 'dir' should not overlap with 'ip'. Usage of union for all
members would mean dir would get corrupt.

Fixes: e91b4f45ff54 ("net/octeontx2: support anti-replay for security session")
Cc: adwivedi@marvell.com

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
---
 drivers/crypto/octeontx2/otx2_security.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/octeontx2/otx2_security.h b/drivers/crypto/octeontx2/otx2_security.h
index 9f1ba71..29c8fc3 100644
--- a/drivers/crypto/octeontx2/otx2_security.h
+++ b/drivers/crypto/octeontx2/otx2_security.h
@@ -20,14 +20,16 @@
 #define OTX2_SEC_AES_GCM_ROUNDUP_BYTE_LEN	4
 #define OTX2_SEC_AES_CBC_ROUNDUP_BYTE_LEN	16
 
-union otx2_sec_session_ipsec {
-	struct otx2_sec_session_ipsec_ip ip;
-	struct otx2_sec_session_ipsec_lp lp;
+struct otx2_sec_session_ipsec {
+	union {
+		struct otx2_sec_session_ipsec_ip ip;
+		struct otx2_sec_session_ipsec_lp lp;
+	};
 	enum rte_security_ipsec_sa_direction dir;
 };
 
 struct otx2_sec_session {
-	union otx2_sec_session_ipsec ipsec;
+	struct otx2_sec_session_ipsec ipsec;
 	void *userdata;
 	/**< Userdata registered by the application */
 } __rte_cache_aligned;
-- 
2.7.4


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

* [dpdk-dev] [PATCH 2/3] net/octeontx2: add locking for inline IPsec tbl updates
  2021-07-13 10:27 [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap Anoob Joseph
@ 2021-07-13 10:27 ` Anoob Joseph
  2021-07-13 10:27 ` [dpdk-dev] [PATCH 3/3] net/octeontx2: clear SA valid during session destroy Anoob Joseph
  2021-07-18  8:33 ` [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap Akhil Goyal
  2 siblings, 0 replies; 4+ messages in thread
From: Anoob Joseph @ 2021-07-13 10:27 UTC (permalink / raw)
  To: Akhil Goyal, Jerin Jacob
  Cc: Anoob Joseph, Ankur Dwivedi, Tejasree Kondoj, dev

Add locking for IPsec table updates.

Fixed error handling to clear SA entry if the SA
population functions encounters any error.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
---
 drivers/net/octeontx2/otx2_ethdev.h     |  2 +
 drivers/net/octeontx2/otx2_ethdev_sec.c | 83 +++++++++++++++++++++++++--------
 2 files changed, 66 insertions(+), 19 deletions(-)

diff --git a/drivers/net/octeontx2/otx2_ethdev.h b/drivers/net/octeontx2/otx2_ethdev.h
index e95d933..7871e3d 100644
--- a/drivers/net/octeontx2/otx2_ethdev.h
+++ b/drivers/net/octeontx2/otx2_ethdev.h
@@ -14,6 +14,7 @@
 #include <rte_mbuf.h>
 #include <rte_mempool.h>
 #include <rte_security_driver.h>
+#include <rte_spinlock.h>
 #include <rte_string_fns.h>
 #include <rte_time.h>
 
@@ -356,6 +357,7 @@ struct otx2_eth_dev {
 	bool sdp_link; /* SDP flag */
 	/* Inline IPsec params */
 	uint16_t ipsec_in_max_spi;
+	rte_spinlock_t ipsec_tbl_lock;
 	uint8_t duplex;
 	uint32_t speed;
 } __rte_cache_aligned;
diff --git a/drivers/net/octeontx2/otx2_ethdev_sec.c b/drivers/net/octeontx2/otx2_ethdev_sec.c
index 1ee597f..72298cf 100644
--- a/drivers/net/octeontx2/otx2_ethdev_sec.c
+++ b/drivers/net/octeontx2/otx2_ethdev_sec.c
@@ -21,6 +21,8 @@
 #include "otx2_sec_idev.h"
 #include "otx2_security.h"
 
+#define ERR_STR_SZ 256
+
 struct eth_sec_tag_const {
 	RTE_STD_C11
 	union {
@@ -162,7 +164,8 @@ lookup_mem_sa_tbl_clear(struct rte_eth_dev *eth_dev)
 }
 
 static int
-lookup_mem_sa_index_update(struct rte_eth_dev *eth_dev, int spi, void *sa)
+lookup_mem_sa_index_update(struct rte_eth_dev *eth_dev, int spi, void *sa,
+			   char *err_str)
 {
 	static const char name[] = OTX2_NIX_FASTPATH_LOOKUP_MEM;
 	struct otx2_eth_dev *dev = otx2_eth_pmd_priv(eth_dev);
@@ -173,7 +176,8 @@ lookup_mem_sa_index_update(struct rte_eth_dev *eth_dev, int spi, void *sa)
 
 	mz = rte_memzone_lookup(name);
 	if (mz == NULL) {
-		otx2_err("Could not find fastpath lookup table");
+		snprintf(err_str, ERR_STR_SZ,
+			 "Could not find fastpath lookup table");
 		return -EINVAL;
 	}
 
@@ -472,23 +476,32 @@ eth_sec_ipsec_in_sess_create(struct rte_eth_dev *eth_dev,
 	struct otx2_ipsec_fp_sa_ctl *ctl;
 	struct otx2_ipsec_fp_in_sa *sa;
 	struct otx2_sec_session *priv;
+	char err_str[ERR_STR_SZ];
 	struct otx2_cpt_qp *qp;
 
+	memset(err_str, 0, ERR_STR_SZ);
+
 	if (ipsec->spi >= dev->ipsec_in_max_spi) {
 		otx2_err("SPI exceeds max supported");
 		return -EINVAL;
 	}
 
 	sa = in_sa_get(port, ipsec->spi);
+	if (sa == NULL)
+		return -ENOMEM;
+
 	ctl = &sa->ctl;
 
 	priv = get_sec_session_private_data(sec_sess);
 	priv->ipsec.dir = RTE_SECURITY_IPSEC_SA_DIR_INGRESS;
 	sess = &priv->ipsec.ip;
 
+	rte_spinlock_lock(&dev->ipsec_tbl_lock);
+
 	if (ctl->valid) {
-		otx2_err("SA already registered");
-		return -EINVAL;
+		snprintf(err_str, ERR_STR_SZ, "SA already registered");
+		ret = -EEXIST;
+		goto tbl_unlock;
 	}
 
 	memset(sa, 0, sizeof(struct otx2_ipsec_fp_in_sa));
@@ -512,10 +525,13 @@ eth_sec_ipsec_in_sess_create(struct rte_eth_dev *eth_dev,
 		auth_key_len = auth_xform->auth.key.length;
 	}
 
-	if (cipher_key_len != 0)
+	if (cipher_key_len != 0) {
 		memcpy(sa->cipher_key, cipher_key, cipher_key_len);
-	else
-		return -EINVAL;
+	} else {
+		snprintf(err_str, ERR_STR_SZ, "Invalid cipher key len");
+		ret = -EINVAL;
+		goto sa_clear;
+	}
 
 	sess->in_sa = sa;
 
@@ -523,33 +539,49 @@ eth_sec_ipsec_in_sess_create(struct rte_eth_dev *eth_dev,
 
 	sa->replay_win_sz = ipsec->replay_win_sz;
 
-	if (lookup_mem_sa_index_update(eth_dev, ipsec->spi, sa))
-		return -EINVAL;
+	if (lookup_mem_sa_index_update(eth_dev, ipsec->spi, sa, err_str)) {
+		ret = -EINVAL;
+		goto sa_clear;
+	}
 
 	ret = ipsec_fp_sa_ctl_set(ipsec, crypto_xform, ctl);
-	if (ret)
-		return ret;
+	if (ret) {
+		snprintf(err_str, ERR_STR_SZ,
+			"Could not set SA CTL word (err: %d)", ret);
+		goto sa_clear;
+	}
 
 	if (auth_key_len && auth_key) {
 		/* Get a queue pair for HMAC init */
 		ret = otx2_sec_idev_tx_cpt_qp_get(port, &qp);
-		if (ret)
-			return ret;
+		if (ret) {
+			snprintf(err_str, ERR_STR_SZ, "Could not get CPT QP");
+			goto sa_clear;
+		}
+
 		ret = hmac_init(ctl, qp, auth_key, auth_key_len, sa->hmac_key);
 		otx2_sec_idev_tx_cpt_qp_put(qp);
-		if (ret)
-			return ret;
+		if (ret) {
+			snprintf(err_str, ERR_STR_SZ, "Could not put CPT QP");
+			goto sa_clear;
+		}
 	}
 
 	if (sa->replay_win_sz) {
 		if (sa->replay_win_sz > OTX2_IPSEC_MAX_REPLAY_WIN_SZ) {
-			otx2_err("Replay window size is not supported");
-			return -ENOTSUP;
+			snprintf(err_str, ERR_STR_SZ,
+				 "Replay window size is not supported");
+			ret = -ENOTSUP;
+			goto sa_clear;
 		}
 		sa->replay = rte_zmalloc(NULL, sizeof(struct otx2_ipsec_replay),
 				0);
-		if (sa->replay == NULL)
-			return -ENOMEM;
+		if (sa->replay == NULL) {
+			snprintf(err_str, ERR_STR_SZ,
+				"Could not allocate memory");
+			ret = -ENOMEM;
+			goto sa_clear;
+		}
 
 		rte_spinlock_init(&sa->replay->lock);
 		/*
@@ -563,6 +595,17 @@ eth_sec_ipsec_in_sess_create(struct rte_eth_dev *eth_dev,
 		sa->esn_hi = 0;
 	}
 
+	rte_spinlock_unlock(&dev->ipsec_tbl_lock);
+	return 0;
+
+sa_clear:
+	memset(sa, 0, sizeof(struct otx2_ipsec_fp_in_sa));
+
+tbl_unlock:
+	rte_spinlock_unlock(&dev->ipsec_tbl_lock);
+
+	otx2_err("%s", err_str);
+
 	return ret;
 }
 
@@ -853,6 +896,8 @@ otx2_eth_sec_init(struct rte_eth_dev *eth_dev)
 		goto sec_fini;
 	}
 
+	rte_spinlock_init(&dev->ipsec_tbl_lock);
+
 	return 0;
 
 sec_fini:
-- 
2.7.4


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

* [dpdk-dev] [PATCH 3/3] net/octeontx2: clear SA valid during session destroy
  2021-07-13 10:27 [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap Anoob Joseph
  2021-07-13 10:27 ` [dpdk-dev] [PATCH 2/3] net/octeontx2: add locking for inline IPsec tbl updates Anoob Joseph
@ 2021-07-13 10:27 ` Anoob Joseph
  2021-07-18  8:33 ` [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap Akhil Goyal
  2 siblings, 0 replies; 4+ messages in thread
From: Anoob Joseph @ 2021-07-13 10:27 UTC (permalink / raw)
  To: Akhil Goyal, Jerin Jacob
  Cc: Anoob Joseph, Ankur Dwivedi, Tejasree Kondoj, dev

SA table entry would be reserved for inline inbound operations. Clear
valid bit of the SA so that CPT would treat SA entry as invalid. Also,
move setting of valid bit to the end in case of session_create() to
eliminate possibility of hardware seeing partial data.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
---
 drivers/crypto/octeontx2/otx2_ipsec_fp.h |  1 -
 drivers/net/octeontx2/otx2_ethdev_sec.c  | 28 ++++++++++++++++++++++++----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/octeontx2/otx2_ipsec_fp.h b/drivers/crypto/octeontx2/otx2_ipsec_fp.h
index a33041d..58b24a2 100644
--- a/drivers/crypto/octeontx2/otx2_ipsec_fp.h
+++ b/drivers/crypto/octeontx2/otx2_ipsec_fp.h
@@ -365,7 +365,6 @@ ipsec_fp_sa_ctl_set(struct rte_security_ipsec_xform *ipsec,
 		ctl->esn_en = 1;
 
 	ctl->spi = rte_cpu_to_be_32(ipsec->spi);
-	ctl->valid = 1;
 
 	return 0;
 }
diff --git a/drivers/net/octeontx2/otx2_ethdev_sec.c b/drivers/net/octeontx2/otx2_ethdev_sec.c
index 72298cf..c2a3688 100644
--- a/drivers/net/octeontx2/otx2_ethdev_sec.c
+++ b/drivers/net/octeontx2/otx2_ethdev_sec.c
@@ -455,6 +455,9 @@ eth_sec_ipsec_out_sess_create(struct rte_eth_dev *eth_dev,
 			goto cpt_put;
 	}
 
+	rte_io_wmb();
+	ctl->valid = 1;
+
 	return 0;
 cpt_put:
 	otx2_sec_idev_tx_cpt_qp_put(sess->qp);
@@ -595,6 +598,9 @@ eth_sec_ipsec_in_sess_create(struct rte_eth_dev *eth_dev,
 		sa->esn_hi = 0;
 	}
 
+	rte_io_wmb();
+	ctl->valid = 1;
+
 	rte_spinlock_unlock(&dev->ipsec_tbl_lock);
 	return 0;
 
@@ -682,10 +688,12 @@ otx2_eth_sec_free_anti_replay(struct otx2_ipsec_fp_in_sa *sa)
 }
 
 static int
-otx2_eth_sec_session_destroy(void *device __rte_unused,
+otx2_eth_sec_session_destroy(void *device,
 			     struct rte_security_session *sess)
 {
+	struct otx2_eth_dev *dev = otx2_eth_pmd_priv(device);
 	struct otx2_sec_session_ipsec_ip *sess_ip;
+	struct otx2_ipsec_fp_in_sa *sa;
 	struct otx2_sec_session *priv;
 	struct rte_mempool *sess_mp;
 	int ret;
@@ -696,9 +704,21 @@ otx2_eth_sec_session_destroy(void *device __rte_unused,
 
 	sess_ip = &priv->ipsec.ip;
 
-	/* Release the anti replay window */
-	if (priv->ipsec.dir == RTE_SECURITY_IPSEC_SA_DIR_INGRESS)
-		otx2_eth_sec_free_anti_replay(sess_ip->in_sa);
+	if (priv->ipsec.dir == RTE_SECURITY_IPSEC_SA_DIR_INGRESS) {
+		rte_spinlock_lock(&dev->ipsec_tbl_lock);
+		sa = sess_ip->in_sa;
+
+		/* Release the anti replay window */
+		otx2_eth_sec_free_anti_replay(sa);
+
+		/* Clear SA table entry */
+		if (sa != NULL) {
+			sa->ctl.valid = 0;
+			rte_io_wmb();
+		}
+
+		rte_spinlock_unlock(&dev->ipsec_tbl_lock);
+	}
 
 	/* Release CPT LF used for this session */
 	if (sess_ip->qp != NULL) {
-- 
2.7.4


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

* Re: [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap
  2021-07-13 10:27 [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap Anoob Joseph
  2021-07-13 10:27 ` [dpdk-dev] [PATCH 2/3] net/octeontx2: add locking for inline IPsec tbl updates Anoob Joseph
  2021-07-13 10:27 ` [dpdk-dev] [PATCH 3/3] net/octeontx2: clear SA valid during session destroy Anoob Joseph
@ 2021-07-18  8:33 ` Akhil Goyal
  2 siblings, 0 replies; 4+ messages in thread
From: Akhil Goyal @ 2021-07-18  8:33 UTC (permalink / raw)
  To: Anoob Joseph, Jerin Jacob Kollanukkaran
  Cc: Anoob Joseph, Ankur Dwivedi, Tejasree Kondoj, dev, stable


> The member 'dir' should not overlap with 'ip'. Usage of union for all
> members would mean dir would get corrupt.
> 
> Fixes: e91b4f45ff54 ("net/octeontx2: support anti-replay for security
> session")
> Cc: adwivedi@marvell.com
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Cc: stable@dpdk.org

Series
Acked-by: Akhil Goyal <gakhil@marvell.com>

Applied to dpdk-next-crypto

Thanks.

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

end of thread, other threads:[~2021-07-18  8:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 10:27 [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap Anoob Joseph
2021-07-13 10:27 ` [dpdk-dev] [PATCH 2/3] net/octeontx2: add locking for inline IPsec tbl updates Anoob Joseph
2021-07-13 10:27 ` [dpdk-dev] [PATCH 3/3] net/octeontx2: clear SA valid during session destroy Anoob Joseph
2021-07-18  8:33 ` [dpdk-dev] [PATCH 1/3] crypto/octeontx2: fix member overlap Akhil Goyal

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).