[PATCH v1 0/6] IXGBE IPsec fixes

[PATCH v1 0/6] IXGBE IPsec fixes

[Anatoly Burakov]

Small array of fixes and refactors for IXGBE IPsec implementation.

Anatoly Burakov (6):
  net/ixgbe: add key length check in IPsec
  net/ixgbe: remove unnecessary dev pointer
  net/ixgbe: harmonize return values in IPsec
  net/ixgbe: get rid of unnecessary malloc in IPsec
  net/ixgbe: fix missing HW write in IPsec
  net/ixgbe: refactor IPsec code

 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 355 ++++++++++++--------------
 drivers/net/intel/ixgbe/ixgbe_ipsec.h |   1 +
 2 files changed, 161 insertions(+), 195 deletions(-)

-- 
2.52.0

[PATCH v1 1/6] net/ixgbe: add key length check in IPsec

[Anatoly Burakov]

The cryptodev API does not check for user parameters being within the
bounds of what the driver supports. Currently, the only key size supported
by ixgbe is 16 bytes, so that is what we will check for. Add the check.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
index bdd34344e8..aaf657af5f 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
@@ -384,6 +384,12 @@ ixgbe_crypto_create_session(void *device,
 	}
 	aead_xform = &conf->crypto_xform->aead;
 
+	/* Only 16-byte keys are supported. */
+	if (aead_xform->key.length != 16) {
+		PMD_DRV_LOG(ERR, "Unsupported key length %u", aead_xform->key.length);
+		return -ENOTSUP;
+	}
+
 	if (conf->ipsec.direction == RTE_SECURITY_IPSEC_SA_DIR_INGRESS) {
 		if (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_SECURITY) {
 			ic_session->op = IXGBE_OP_AUTHENTICATED_DECRYPTION;
-- 
2.52.0

[PATCH v1 2/6] net/ixgbe: remove unnecessary dev pointer

[Anatoly Burakov]

The `ic_session` already contains a `dev_data` pointer, so there is no need
to pass the `dev` pointer to `ixgbe_crypto_remove_sa`.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
index aaf657af5f..e2ec402b52 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
@@ -268,12 +268,11 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 }
 
 static int
-ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
-		       struct ixgbe_crypto_session *ic_session)
+ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(ic_session->dev_data->dev_private);
 	struct ixgbe_ipsec *priv =
-			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
+			IXGBE_DEV_PRIVATE_TO_IPSEC(ic_session->dev_data->dev_private);
 	uint32_t reg_val;
 	int sa_index = -1;
 
@@ -441,7 +440,7 @@ ixgbe_crypto_remove_session(void *device,
 		return -ENODEV;
 	}
 
-	if (ixgbe_crypto_remove_sa(eth_dev, ic_session)) {
+	if (ixgbe_crypto_remove_sa(ic_session)) {
 		PMD_DRV_LOG(ERR, "Failed to remove session");
 		return -EFAULT;
 	}
-- 
2.52.0

[PATCH v1 3/6] net/ixgbe: harmonize return values in IPsec

[Anatoly Burakov]

Currently, some paths in add/remove SA code return negative errno values
while others just return -1. Harmonize all of them to return appropriate
negative errno values.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 30 +++++++++++----------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
index e2ec402b52..41fc7551ff 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
@@ -119,9 +119,8 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 
 		/* Fail if no match and no free entries*/
 		if (ip_index < 0) {
-			PMD_DRV_LOG(ERR,
-				    "No free entry left in the Rx IP table");
-			return -1;
+			PMD_DRV_LOG(ERR, "No free entry left in the Rx IP table");
+			return -ENOSPC;
 		}
 
 		/* Find a free entry in the SA table*/
@@ -133,9 +132,8 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 		}
 		/* Fail if no free entries*/
 		if (sa_index < 0) {
-			PMD_DRV_LOG(ERR,
-				    "No free entry left in the Rx SA table");
-			return -1;
+			PMD_DRV_LOG(ERR, "No free entry left in the Rx SA table");
+			return -ENOSPC;
 		}
 
 		priv->rx_ip_tbl[ip_index].ip.ipv6[0] =
@@ -231,9 +229,8 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 		}
 		/* Fail if no free entries*/
 		if (sa_index < 0) {
-			PMD_DRV_LOG(ERR,
-				    "No free entry left in the Tx SA table");
-			return -1;
+			PMD_DRV_LOG(ERR, "No free entry left in the Tx SA table");
+			return -ENOSPC;
 		}
 
 		priv->tx_sa_tbl[sa_index].spi =
@@ -289,9 +286,8 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 
 		/* Fail if no match*/
 		if (ip_index < 0) {
-			PMD_DRV_LOG(ERR,
-				    "Entry not found in the Rx IP table");
-			return -1;
+			PMD_DRV_LOG(ERR, "Entry not found in the Rx IP table");
+			return -ENOENT;
 		}
 
 		/* Find a free entry in the SA table*/
@@ -304,9 +300,8 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 		}
 		/* Fail if no match*/
 		if (sa_index < 0) {
-			PMD_DRV_LOG(ERR,
-				    "Entry not found in the Rx SA table");
-			return -1;
+			PMD_DRV_LOG(ERR, "Entry not found in the Rx SA table");
+			return -ENOENT;
 		}
 
 		/* Disable and clear Rx SPI and key table entries*/
@@ -347,9 +342,8 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 		}
 		/* Fail if no match entries*/
 		if (sa_index < 0) {
-			PMD_DRV_LOG(ERR,
-				    "Entry not found in the Tx SA table");
-			return -1;
+			PMD_DRV_LOG(ERR, "Entry not found in the Tx SA table");
+			return -ENOENT;
 		}
 		reg_val = IPSRXIDX_WRITE | (sa_index << 3);
 		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0), 0);
-- 
2.52.0

[PATCH v1 4/6] net/ixgbe: get rid of unnecessary malloc in IPsec

[Anatoly Burakov]

Currently, when writing SA key to HW, a temporary buffer is allocated, to
which the key is written. The allocation does not have to happen on the
heap, so change the allocation to be on the stack, as key size is known at
compile time and is pretty tiny.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
index 41fc7551ff..1d001b955b 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
@@ -94,10 +94,12 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 			dev_data->dev_private);
 	uint32_t reg_val;
 	int sa_index = -1;
+	uint8_t key[16] = {0};
+
+	memcpy(key, ic_session->key, ic_session->key_len);
 
 	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
 		int i, ip_index = -1;
-		uint8_t *key;
 
 		/* Find a match in the IP table*/
 		for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
@@ -191,13 +193,6 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 				priv->rx_sa_tbl[sa_index].ip_index);
 		IXGBE_WAIT_RWRITE;
 
-		/* write Key table entry*/
-		key = malloc(ic_session->key_len);
-		if (!key)
-			return -ENOMEM;
-
-		memcpy(key, ic_session->key, ic_session->key_len);
-
 		reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
 				IPSRXIDX_TABLE_KEY | (sa_index << 3);
 		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0),
@@ -214,10 +209,7 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 				priv->rx_sa_tbl[sa_index].mode);
 		IXGBE_WAIT_RWRITE;
 
-		free(key);
-
 	} else { /* sess->dir == RTE_CRYPTO_OUTBOUND */
-		uint8_t *key;
 		int i;
 
 		/* Find a free entry in the SA table*/
@@ -238,12 +230,6 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 		priv->tx_sa_tbl[i].used = 1;
 		ic_session->sa_index = sa_index;
 
-		key = malloc(ic_session->key_len);
-		if (!key)
-			return -ENOMEM;
-
-		memcpy(key, ic_session->key, ic_session->key_len);
-
 		/* write Key table entry*/
 		reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE | (sa_index << 3);
 		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0),
@@ -257,8 +243,6 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 		IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT,
 				rte_cpu_to_be_32(ic_session->salt));
 		IXGBE_WAIT_TWRITE;
-
-		free(key);
 	}
 
 	return 0;
-- 
2.52.0

[PATCH v1 5/6] net/ixgbe: fix missing HW write in IPsec

[Anatoly Burakov]

When removing an SA and finding ourselves with zero refcount on an IP table
entry, we zero out the IP table registers, but do not write them. Add the
missing write.

Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
Cc: radu.nicolau@intel.com
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
index 1d001b955b..2d1b3a43ca 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
@@ -312,6 +312,7 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), 0);
 			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), 0);
 			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3), 0);
+			IXGBE_WAIT_RWRITE;
 		}
 	} else { /* session->dir == RTE_CRYPTO_OUTBOUND */
 		int i;
-- 
2.52.0

[PATCH v1 6/6] net/ixgbe: refactor IPsec code

[Anatoly Burakov]

Currently, the IPsec code mixes SW bookkeeping with HW writes quite a lot,
which results in difficulty extending the code, lots of duplication, and
potential for bugs. Refactor the code to be more readable by separating
out the HW writing parts into helper functions.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 291 ++++++++++++--------------
 drivers/net/intel/ixgbe/ixgbe_ipsec.h |   1 +
 2 files changed, 137 insertions(+), 155 deletions(-)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
index 2d1b3a43ca..c024892e1c 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ipsec.c
@@ -36,6 +36,85 @@
 	(a).ipv6[2] == (b).ipv6[2] && \
 	(a).ipv6[3] == (b).ipv6[3])
 
+static inline void
+ixgbe_crypto_write_rx_ip(struct ixgbe_hw *hw, uint32_t idx,
+		const struct ipaddr *ip, bool enable)
+{
+	uint32_t reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_IP | (idx << 3);
+	uint32_t addr[4] = {0};
+
+	if (enable)
+		reg_val |= IPSRXIDX_RX_EN;
+
+	if (ip->type == IPv4)
+		/* only write last 4 bytes */
+		addr[3] = ip->ipv4;
+	else
+		memcpy(addr, ip->ipv6, sizeof(addr));
+
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0), addr[0]);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), addr[1]);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), addr[2]);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3), addr[3]);
+	IXGBE_WAIT_RWRITE;
+}
+
+static inline void
+ixgbe_crypto_write_rx_spi(struct ixgbe_hw *hw, uint32_t idx,
+		uint32_t spi, uint32_t ip_idx, bool enable)
+{
+	uint32_t reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_SPI | (idx << 3);
+
+	if (enable)
+		reg_val |= IPSRXIDX_RX_EN;
+
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, spi);
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, ip_idx);
+	IXGBE_WAIT_RWRITE;
+}
+
+static inline void
+ixgbe_crypto_write_rx_key(struct ixgbe_hw *hw, uint32_t idx,
+		const uint8_t *key, uint32_t salt, uint32_t mode, bool enable)
+{
+	uint32_t reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_KEY | (idx << 3);
+
+	if (enable)
+		reg_val |= IPSRXIDX_RX_EN;
+
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[12]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[8]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[4]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[0]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, rte_cpu_to_be_32(salt));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, mode);
+	IXGBE_WAIT_RWRITE;
+}
+
+static inline void
+ixgbe_crypto_write_tx_key(struct ixgbe_hw *hw, uint32_t idx,
+		const uint8_t *key, uint32_t salt, bool enable)
+{
+	uint32_t reg_val = IPSRXIDX_WRITE | (idx << 3);
+
+	if (enable)
+		reg_val |= IPSRXIDX_TX_EN;
+
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[12]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[8]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[4]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3),
+		rte_cpu_to_be_32(*(const uint32_t *)&key[0]));
+	IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, rte_cpu_to_be_32(salt));
+	IXGBE_WAIT_TWRITE;
+}
 
 static void
 ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
@@ -43,41 +122,19 @@ ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_ipsec *priv = IXGBE_DEV_PRIVATE_TO_IPSEC(
 				dev->data->dev_private);
+	const struct ipaddr ip = {0};
+	const uint8_t key[16] = {0};
 	int i = 0;
 
 	/* clear Rx IP table*/
-	for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
-		uint16_t index = i << 3;
-		uint32_t reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_IP | index;
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3), 0);
-		IXGBE_WAIT_RWRITE;
-	}
+	for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++)
+		ixgbe_crypto_write_rx_ip(hw, i, &ip, false);
 
 	/* clear Rx SPI and Rx/Tx SA tables*/
 	for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
-		uint32_t index = i << 3;
-		uint32_t reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_SPI | index;
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, 0);
-		IXGBE_WAIT_RWRITE;
-		reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_KEY | index;
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, 0);
-		IXGBE_WAIT_RWRITE;
-		reg_val = IPSRXIDX_WRITE | index;
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, 0);
-		IXGBE_WAIT_TWRITE;
+		ixgbe_crypto_write_rx_spi(hw, i, 0, 0, false);
+		ixgbe_crypto_write_rx_key(hw, i, key, 0, 0, false);
+		ixgbe_crypto_write_tx_key(hw, i, key, 0, false);
 	}
 
 	memset(priv->rx_ip_tbl, 0, sizeof(priv->rx_ip_tbl));
@@ -90,16 +147,16 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 {
 	struct rte_eth_dev_data *dev_data = ic_session->dev_data;
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev_data->dev_private);
-	struct ixgbe_ipsec *priv = IXGBE_DEV_PRIVATE_TO_IPSEC(
-			dev_data->dev_private);
-	uint32_t reg_val;
-	int sa_index = -1;
+	struct ixgbe_ipsec *priv = IXGBE_DEV_PRIVATE_TO_IPSEC(dev_data->dev_private);
+	int i, sa_index = -1;
 	uint8_t key[16] = {0};
 
 	memcpy(key, ic_session->key, ic_session->key_len);
 
 	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
-		int i, ip_index = -1;
+		struct ixgbe_crypto_rx_ip_table *rxip;
+		struct ixgbe_crypto_rx_sa_table *rxsa;
+		int ip_index = -1, free_index = -1;
 
 		/* Find a match in the IP table*/
 		for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
@@ -108,22 +165,19 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 				ip_index = i;
 				break;
 			}
+			if (free_index == -1 && priv->rx_ip_tbl[i].ref_count == 0)
+				free_index = i;
 		}
 		/* If no match, find a free entry in the IP table*/
-		if (ip_index < 0) {
-			for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
-				if (priv->rx_ip_tbl[i].ref_count == 0) {
-					ip_index = i;
-					break;
-				}
-			}
-		}
+		if (ip_index < 0)
+			ip_index = free_index;
 
 		/* Fail if no match and no free entries*/
 		if (ip_index < 0) {
 			PMD_DRV_LOG(ERR, "No free entry left in the Rx IP table");
 			return -ENOSPC;
 		}
+		rxip = &priv->rx_ip_tbl[ip_index];
 
 		/* Find a free entry in the SA table*/
 		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
@@ -137,80 +191,31 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 			PMD_DRV_LOG(ERR, "No free entry left in the Rx SA table");
 			return -ENOSPC;
 		}
+		rxsa = &priv->rx_sa_tbl[sa_index];
 
-		priv->rx_ip_tbl[ip_index].ip.ipv6[0] =
-				ic_session->dst_ip.ipv6[0];
-		priv->rx_ip_tbl[ip_index].ip.ipv6[1] =
-				ic_session->dst_ip.ipv6[1];
-		priv->rx_ip_tbl[ip_index].ip.ipv6[2] =
-				ic_session->dst_ip.ipv6[2];
-		priv->rx_ip_tbl[ip_index].ip.ipv6[3] =
-				ic_session->dst_ip.ipv6[3];
-		priv->rx_ip_tbl[ip_index].ref_count++;
+		rxip->ref_count++;
+		memcpy(&rxip->ip, &ic_session->dst_ip, sizeof(rxip->ip));
 
-		priv->rx_sa_tbl[sa_index].spi =
-			rte_cpu_to_be_32(ic_session->spi);
-		priv->rx_sa_tbl[sa_index].ip_index = ip_index;
-		priv->rx_sa_tbl[sa_index].mode = IPSRXMOD_VALID;
-		if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION)
-			priv->rx_sa_tbl[sa_index].mode |=
-					(IPSRXMOD_PROTO | IPSRXMOD_DECRYPT);
-		if (ic_session->dst_ip.type == IPv6) {
-			priv->rx_sa_tbl[sa_index].mode |= IPSRXMOD_IPV6;
-			priv->rx_ip_tbl[ip_index].ip.type = IPv6;
-		} else if (ic_session->dst_ip.type == IPv4)
-			priv->rx_ip_tbl[ip_index].ip.type = IPv4;
+		rxsa->spi = rte_cpu_to_be_32(ic_session->spi);
+		rxsa->ip_index = ip_index;
+		rxsa->mode = IPSRXMOD_VALID | IPSRXMOD_PROTO | IPSRXMOD_DECRYPT;
+		if (ic_session->dst_ip.type == IPv6)
+			rxsa->mode |= IPSRXMOD_IPV6;
 
-		priv->rx_sa_tbl[sa_index].used = 1;
+		rxsa->used = 1;
 
 		/* write IP table entry*/
-		reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
-				IPSRXIDX_TABLE_IP | (ip_index << 3);
-		if (priv->rx_ip_tbl[ip_index].ip.type == IPv4) {
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0), 0);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), 0);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), 0);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3),
-					priv->rx_ip_tbl[ip_index].ip.ipv4);
-		} else {
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0),
-					priv->rx_ip_tbl[ip_index].ip.ipv6[0]);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1),
-					priv->rx_ip_tbl[ip_index].ip.ipv6[1]);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2),
-					priv->rx_ip_tbl[ip_index].ip.ipv6[2]);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3),
-					priv->rx_ip_tbl[ip_index].ip.ipv6[3]);
-		}
-		IXGBE_WAIT_RWRITE;
+		ixgbe_crypto_write_rx_ip(hw, ip_index, &rxip->ip, true);
 
 		/* write SPI table entry*/
-		reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
-				IPSRXIDX_TABLE_SPI | (sa_index << 3);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI,
-				priv->rx_sa_tbl[sa_index].spi);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX,
-				priv->rx_sa_tbl[sa_index].ip_index);
-		IXGBE_WAIT_RWRITE;
+		ixgbe_crypto_write_rx_spi(hw, sa_index, rxsa->spi, ip_index, true);
 
-		reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
-				IPSRXIDX_TABLE_KEY | (sa_index << 3);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0),
-			rte_cpu_to_be_32(*(uint32_t *)&key[12]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1),
-			rte_cpu_to_be_32(*(uint32_t *)&key[8]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2),
-			rte_cpu_to_be_32(*(uint32_t *)&key[4]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3),
-			rte_cpu_to_be_32(*(uint32_t *)&key[0]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT,
-				rte_cpu_to_be_32(ic_session->salt));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD,
-				priv->rx_sa_tbl[sa_index].mode);
-		IXGBE_WAIT_RWRITE;
+		/* write Key table entry*/
+		ixgbe_crypto_write_rx_key(hw, sa_index, key,
+				ic_session->salt, rxsa->mode, true);
 
 	} else { /* sess->dir == RTE_CRYPTO_OUTBOUND */
-		int i;
+		struct ixgbe_crypto_tx_sa_table *txsa;
 
 		/* Find a free entry in the SA table*/
 		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
@@ -224,25 +229,14 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 			PMD_DRV_LOG(ERR, "No free entry left in the Tx SA table");
 			return -ENOSPC;
 		}
+		txsa = &priv->tx_sa_tbl[sa_index];
 
-		priv->tx_sa_tbl[sa_index].spi =
-			rte_cpu_to_be_32(ic_session->spi);
-		priv->tx_sa_tbl[i].used = 1;
+		txsa->spi = rte_cpu_to_be_32(ic_session->spi);
+		txsa->used = 1;
 		ic_session->sa_index = sa_index;
 
 		/* write Key table entry*/
-		reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE | (sa_index << 3);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0),
-			rte_cpu_to_be_32(*(uint32_t *)&key[12]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1),
-			rte_cpu_to_be_32(*(uint32_t *)&key[8]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2),
-			rte_cpu_to_be_32(*(uint32_t *)&key[4]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3),
-			rte_cpu_to_be_32(*(uint32_t *)&key[0]));
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT,
-				rte_cpu_to_be_32(ic_session->salt));
-		IXGBE_WAIT_TWRITE;
+		ixgbe_crypto_write_tx_key(hw, sa_index, key, ic_session->salt, true);
 	}
 
 	return 0;
@@ -254,11 +248,13 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(ic_session->dev_data->dev_private);
 	struct ixgbe_ipsec *priv =
 			IXGBE_DEV_PRIVATE_TO_IPSEC(ic_session->dev_data->dev_private);
-	uint32_t reg_val;
-	int sa_index = -1;
+	const uint8_t key[16] = {0};
+	int i, sa_index = -1;
 
 	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
-		int i, ip_index = -1;
+		struct ixgbe_crypto_rx_ip_table *rxip;
+		struct ixgbe_crypto_rx_sa_table *rxsa;
+		int ip_index = -1;
 
 		/* Find a match in the IP table*/
 		for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
@@ -273,6 +269,7 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 			PMD_DRV_LOG(ERR, "Entry not found in the Rx IP table");
 			return -ENOENT;
 		}
+		rxip = &priv->rx_ip_tbl[ip_index];
 
 		/* Find a free entry in the SA table*/
 		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
@@ -287,35 +284,24 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 			PMD_DRV_LOG(ERR, "Entry not found in the Rx SA table");
 			return -ENOENT;
 		}
+		rxsa = &priv->rx_sa_tbl[sa_index];
 
 		/* Disable and clear Rx SPI and key table entries*/
-		reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_SPI | (sa_index << 3);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, 0);
-		IXGBE_WAIT_RWRITE;
-		reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_KEY | (sa_index << 3);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, 0);
-		IXGBE_WAIT_RWRITE;
-		priv->rx_sa_tbl[sa_index].used = 0;
+		ixgbe_crypto_write_rx_spi(hw, sa_index, 0, 0, false);
+		ixgbe_crypto_write_rx_key(hw, sa_index, key, 0, 0, false);
+
+		/* Clear the SA table entry*/
+		*rxsa = (struct ixgbe_crypto_rx_sa_table){0};
 
 		/* If last used then clear the IP table entry*/
-		priv->rx_ip_tbl[ip_index].ref_count--;
-		if (priv->rx_ip_tbl[ip_index].ref_count == 0) {
-			reg_val = IPSRXIDX_WRITE | IPSRXIDX_TABLE_IP |
-					(ip_index << 3);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0), 0);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), 0);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), 0);
-			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3), 0);
-			IXGBE_WAIT_RWRITE;
+		rxip->ref_count--;
+		if (rxip->ref_count == 0) {
+			const struct ipaddr ip = {0};
+			ixgbe_crypto_write_rx_ip(hw, ip_index, &ip, false);
+			*rxip = (struct ixgbe_crypto_rx_ip_table){0};
 		}
 	} else { /* session->dir == RTE_CRYPTO_OUTBOUND */
-		int i;
+		struct ixgbe_crypto_tx_sa_table *txsa;
 
 		/* Find a match in the SA table*/
 		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
@@ -330,15 +316,10 @@ ixgbe_crypto_remove_sa(struct ixgbe_crypto_session *ic_session)
 			PMD_DRV_LOG(ERR, "Entry not found in the Tx SA table");
 			return -ENOENT;
 		}
-		reg_val = IPSRXIDX_WRITE | (sa_index << 3);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3), 0);
-		IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, 0);
-		IXGBE_WAIT_TWRITE;
+		txsa = &priv->tx_sa_tbl[sa_index];
 
-		priv->tx_sa_tbl[sa_index].used = 0;
+		ixgbe_crypto_write_tx_key(hw, sa_index, key, 0, false);
+		*txsa = (struct ixgbe_crypto_tx_sa_table){0};
 	}
 
 	return 0;
diff --git a/drivers/net/intel/ixgbe/ixgbe_ipsec.h b/drivers/net/intel/ixgbe/ixgbe_ipsec.h
index 356817c61b..1099b5f598 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ipsec.h
+++ b/drivers/net/intel/ixgbe/ixgbe_ipsec.h
@@ -12,6 +12,7 @@
 #include <rte_flow.h>
 
 #define IPSRXIDX_RX_EN                                    0x00000001
+#define IPSRXIDX_TX_EN                                    0x00000001
 #define IPSRXIDX_TABLE_IP                                 0x00000002
 #define IPSRXIDX_TABLE_SPI                                0x00000004
 #define IPSRXIDX_TABLE_KEY                                0x00000006
-- 
2.52.0

Re: [PATCH v1 0/6] IXGBE IPsec fixes

[Radu Nicolau]

On 18-Aug-26 2:19 PM, Anatoly Burakov wrote:
> Small array of fixes and refactors for IXGBE IPsec implementation.
>
> Anatoly Burakov (6):
>    net/ixgbe: add key length check in IPsec
>    net/ixgbe: remove unnecessary dev pointer
>    net/ixgbe: harmonize return values in IPsec
>    net/ixgbe: get rid of unnecessary malloc in IPsec
>    net/ixgbe: fix missing HW write in IPsec
>    net/ixgbe: refactor IPsec code
>
>   drivers/net/intel/ixgbe/ixgbe_ipsec.c | 355 ++++++++++++--------------
>   drivers/net/intel/ixgbe/ixgbe_ipsec.h |   1 +
>   2 files changed, 161 insertions(+), 195 deletions(-)
Series-acked-by: Radu Nicolau <radu.nicolau@intel.com>