[2/2] net/nfp: fix data endianness problem

Message ID 20240311024939.2523778-3-chaoyong.he@corigine.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series fix IPSec endianness problem |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Chaoyong He March 11, 2024, 2:49 a.m. UTC
  From: Shihong Wang <shihong.wang@corigine.com>

The algorithm key of the security framework is stored in the u8
array according to big-endian, and the driver algorithm key is
CPU-endian of u32, so it maybe need to convert the endianness order
to ensure that the value assigned to the driver is CPU-endian.

This patch removes the operation of converting IPsec Tx metadata
to big-endian to ensure that IPsec Tx metadata is CPU-endian.

Fixes: 547137405be7 ("net/nfp: initialize IPsec related content")
Fixes: 3d21da66c06b ("net/nfp: create security session")
Fixes: 310a1780581e ("net/nfp: support IPsec Rx and Tx offload")
Cc: stable@dpdk.org

Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 drivers/net/nfp/nfp_ipsec.c | 72 +++++++++++++++++++++++--------------
 drivers/net/nfp/nfp_ipsec.h |  9 ++---
 2 files changed, 47 insertions(+), 34 deletions(-)
  

Comments

Ferruh Yigit March 13, 2024, 3:39 p.m. UTC | #1
On 3/11/2024 2:49 AM, Chaoyong He wrote:
> From: Shihong Wang <shihong.wang@corigine.com>
> 
> The algorithm key of the security framework is stored in the u8
> array according to big-endian, and the driver algorithm key is
> CPU-endian of u32, so it maybe need to convert the endianness order
> to ensure that the value assigned to the driver is CPU-endian.
> 
> This patch removes the operation of converting IPsec Tx metadata
> to big-endian to ensure that IPsec Tx metadata is CPU-endian.
> 
> Fixes: 547137405be7 ("net/nfp: initialize IPsec related content")
> Fixes: 3d21da66c06b ("net/nfp: create security session")
> Fixes: 310a1780581e ("net/nfp: support IPsec Rx and Tx offload")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>

Applied to dpdk-next-net/main, thanks.
(not the set, just this patch)
  

Patch

diff --git a/drivers/net/nfp/nfp_ipsec.c b/drivers/net/nfp/nfp_ipsec.c
index 0b815fa983..8824533656 100644
--- a/drivers/net/nfp/nfp_ipsec.c
+++ b/drivers/net/nfp/nfp_ipsec.c
@@ -20,6 +20,7 @@ 
 #include "nfp_rxtx.h"
 
 #define NFP_UDP_ESP_PORT            4500
+#define NFP_ESP_IV_LENGTH           8
 
 static const struct rte_cryptodev_capabilities nfp_crypto_caps[] = {
 	{
@@ -523,7 +524,8 @@  nfp_aesgcm_iv_update(struct ipsec_add_sa *cfg,
 	char *save;
 	char *iv_b;
 	char *iv_str;
-	uint8_t *cfg_iv;
+	const rte_be32_t *iv_value;
+	uint8_t cfg_iv[NFP_ESP_IV_LENGTH];
 
 	iv_str = strdup(iv_string);
 	if (iv_str == NULL) {
@@ -531,8 +533,6 @@  nfp_aesgcm_iv_update(struct ipsec_add_sa *cfg,
 		return;
 	}
 
-	cfg_iv = (uint8_t *)cfg->aesgcm_fields.iv;
-
 	for (i = 0; i < iv_len; i++) {
 		iv_b = strtok_r(i ? NULL : iv_str, ",", &save);
 		if (iv_b == NULL)
@@ -541,8 +541,9 @@  nfp_aesgcm_iv_update(struct ipsec_add_sa *cfg,
 		cfg_iv[i] = strtoul(iv_b, NULL, 0);
 	}
 
-	*(uint32_t *)cfg_iv = rte_be_to_cpu_32(*(uint32_t *)cfg_iv);
-	*(uint32_t *)&cfg_iv[4] = rte_be_to_cpu_32(*(uint32_t *)&cfg_iv[4]);
+	iv_value = (const rte_be32_t *)(cfg_iv);
+	cfg->aesgcm_fields.iv[0] = rte_be_to_cpu_32(iv_value[0]);
+	cfg->aesgcm_fields.iv[1] = rte_be_to_cpu_32(iv_value[1]);
 
 	free(iv_str);
 }
@@ -583,7 +584,7 @@  nfp_aead_map(struct rte_eth_dev *eth_dev,
 	uint32_t offset;
 	uint32_t device_id;
 	const char *iv_str;
-	const uint32_t *key;
+	const rte_be32_t *key;
 	struct nfp_net_hw *net_hw;
 
 	net_hw = eth_dev->data->dev_private;
@@ -633,7 +634,7 @@  nfp_aead_map(struct rte_eth_dev *eth_dev,
 		return -EINVAL;
 	}
 
-	key = (const uint32_t *)(aead->key.data);
+	key = (const rte_be32_t *)(aead->key.data);
 
 	/*
 	 * The CHACHA20's key order needs to be adjusted based on hardware design.
@@ -645,16 +646,22 @@  nfp_aead_map(struct rte_eth_dev *eth_dev,
 
 	for (i = 0; i < key_length / sizeof(cfg->cipher_key[0]); i++) {
 		index = (i + offset) % (key_length / sizeof(cfg->cipher_key[0]));
-		cfg->cipher_key[index] = rte_cpu_to_be_32(*key++);
+		cfg->cipher_key[index] = rte_be_to_cpu_32(key[i]);
 	}
 
 	/*
-	 * The iv of the FW is equal to ESN by default. Reading the
-	 * iv of the configuration information is not supported.
+	 * The iv of the FW is equal to ESN by default. Only the
+	 * aead algorithm can offload the iv of configuration and
+	 * the length of iv cannot be greater than NFP_ESP_IV_LENGTH.
 	 */
 	iv_str = getenv("ETH_SEC_IV_OVR");
 	if (iv_str != NULL) {
 		iv_len = aead->iv.length;
+		if (iv_len > NFP_ESP_IV_LENGTH) {
+			PMD_DRV_LOG(ERR, "Unsupported length of iv data");
+			return -EINVAL;
+		}
+
 		nfp_aesgcm_iv_update(cfg, iv_len, iv_str);
 	}
 
@@ -671,7 +678,7 @@  nfp_cipher_map(struct rte_eth_dev *eth_dev,
 	int ret;
 	uint32_t i;
 	uint32_t device_id;
-	const uint32_t *key;
+	const rte_be32_t *key;
 	struct nfp_net_hw *net_hw;
 
 	net_hw = eth_dev->data->dev_private;
@@ -705,14 +712,14 @@  nfp_cipher_map(struct rte_eth_dev *eth_dev,
 		return -EINVAL;
 	}
 
-	key = (const uint32_t  *)(cipher->key.data);
+	key = (const rte_be32_t *)(cipher->key.data);
 	if (key_length > sizeof(cfg->cipher_key)) {
 		PMD_DRV_LOG(ERR, "Insufficient space for offloaded key");
 		return -EINVAL;
 	}
 
 	for (i = 0; i < key_length / sizeof(cfg->cipher_key[0]); i++)
-		cfg->cipher_key[i] = rte_cpu_to_be_32(*key++);
+		cfg->cipher_key[i] = rte_be_to_cpu_32(key[i]);
 
 	return 0;
 }
@@ -807,7 +814,7 @@  nfp_auth_map(struct rte_eth_dev *eth_dev,
 	uint32_t i;
 	uint8_t key_length;
 	uint32_t device_id;
-	const uint32_t *key;
+	const rte_be32_t *key;
 	struct nfp_net_hw *net_hw;
 
 	if (digest_length == 0) {
@@ -854,7 +861,7 @@  nfp_auth_map(struct rte_eth_dev *eth_dev,
 		return -EINVAL;
 	}
 
-	key = (const uint32_t *)(auth->key.data);
+	key = (const rte_be32_t *)(auth->key.data);
 	key_length = auth->key.length;
 	if (key_length > sizeof(cfg->auth_key)) {
 		PMD_DRV_LOG(ERR, "Insufficient space for offloaded auth key!");
@@ -862,7 +869,7 @@  nfp_auth_map(struct rte_eth_dev *eth_dev,
 	}
 
 	for (i = 0; i < key_length / sizeof(cfg->auth_key[0]); i++)
-		cfg->auth_key[i] = rte_cpu_to_be_32(*key++);
+		cfg->auth_key[i] = rte_be_to_cpu_32(key[i]);
 
 	return 0;
 }
@@ -902,7 +909,7 @@  nfp_crypto_msg_build(struct rte_eth_dev *eth_dev,
 			return ret;
 		}
 
-		cfg->aesgcm_fields.salt = rte_cpu_to_be_32(conf->ipsec.salt);
+		cfg->aesgcm_fields.salt = conf->ipsec.salt;
 		break;
 	case RTE_CRYPTO_SYM_XFORM_AUTH:
 		/* Only support Auth + Cipher for inbound */
@@ -967,7 +974,10 @@  nfp_ipsec_msg_build(struct rte_eth_dev *eth_dev,
 		struct rte_security_session_conf *conf,
 		struct nfp_ipsec_msg *msg)
 {
+	int i;
 	int ret;
+	rte_be32_t *src_ip;
+	rte_be32_t *dst_ip;
 	struct ipsec_add_sa *cfg;
 	enum rte_security_ipsec_tunnel_type type;
 
@@ -1025,12 +1035,18 @@  nfp_ipsec_msg_build(struct rte_eth_dev *eth_dev,
 		type = conf->ipsec.tunnel.type;
 		cfg->ctrl_word.mode = NFP_IPSEC_MODE_TUNNEL;
 		if (type == RTE_SECURITY_IPSEC_TUNNEL_IPV4) {
-			cfg->src_ip.v4 = conf->ipsec.tunnel.ipv4.src_ip;
-			cfg->dst_ip.v4 = conf->ipsec.tunnel.ipv4.dst_ip;
+			src_ip = (rte_be32_t *)&conf->ipsec.tunnel.ipv4.src_ip.s_addr;
+			dst_ip = (rte_be32_t *)&conf->ipsec.tunnel.ipv4.dst_ip.s_addr;
+			cfg->src_ip[0] = rte_be_to_cpu_32(src_ip[0]);
+			cfg->dst_ip[0] = rte_be_to_cpu_32(dst_ip[0]);
 			cfg->ipv6 = 0;
 		} else if (type == RTE_SECURITY_IPSEC_TUNNEL_IPV6) {
-			cfg->src_ip.v6 = conf->ipsec.tunnel.ipv6.src_addr;
-			cfg->dst_ip.v6 = conf->ipsec.tunnel.ipv6.dst_addr;
+			src_ip = (rte_be32_t *)conf->ipsec.tunnel.ipv6.src_addr.s6_addr;
+			dst_ip = (rte_be32_t *)conf->ipsec.tunnel.ipv6.dst_addr.s6_addr;
+			for (i = 0; i < 4; i++) {
+				cfg->src_ip[i] = rte_be_to_cpu_32(src_ip[i]);
+				cfg->dst_ip[i] = rte_be_to_cpu_32(dst_ip[i]);
+			}
 			cfg->ipv6 = 1;
 		} else {
 			PMD_DRV_LOG(ERR, "Unsupported address family!");
@@ -1043,9 +1059,11 @@  nfp_ipsec_msg_build(struct rte_eth_dev *eth_dev,
 		cfg->ctrl_word.mode = NFP_IPSEC_MODE_TRANSPORT;
 		if (type == RTE_SECURITY_IPSEC_TUNNEL_IPV4) {
 			memset(&cfg->src_ip, 0, sizeof(cfg->src_ip));
+			memset(&cfg->dst_ip, 0, sizeof(cfg->dst_ip));
 			cfg->ipv6 = 0;
 		} else if (type == RTE_SECURITY_IPSEC_TUNNEL_IPV6) {
 			memset(&cfg->src_ip, 0, sizeof(cfg->src_ip));
+			memset(&cfg->dst_ip, 0, sizeof(cfg->dst_ip));
 			cfg->ipv6 = 1;
 		} else {
 			PMD_DRV_LOG(ERR, "Unsupported address family!");
@@ -1179,18 +1197,18 @@  nfp_security_set_pkt_metadata(void *device,
 		desc_md = RTE_MBUF_DYNFIELD(m, offset, struct nfp_tx_ipsec_desc_msg *);
 
 		if (priv_session->msg.ctrl_word.ext_seq != 0 && sqn != NULL) {
-			desc_md->esn.low = rte_cpu_to_be_32(*sqn);
-			desc_md->esn.hi = rte_cpu_to_be_32(*sqn >> 32);
+			desc_md->esn.low = (uint32_t)*sqn;
+			desc_md->esn.hi = (uint32_t)(*sqn >> 32);
 		} else if (priv_session->msg.ctrl_word.ext_seq != 0) {
-			desc_md->esn.low = rte_cpu_to_be_32(priv_session->ipsec.esn.low);
-			desc_md->esn.hi = rte_cpu_to_be_32(priv_session->ipsec.esn.hi);
+			desc_md->esn.low = priv_session->ipsec.esn.low;
+			desc_md->esn.hi = priv_session->ipsec.esn.hi;
 		} else {
-			desc_md->esn.low = rte_cpu_to_be_32(priv_session->ipsec.esn.value);
+			desc_md->esn.low = priv_session->ipsec.esn.low;
 			desc_md->esn.hi = 0;
 		}
 
 		desc_md->enc = 1;
-		desc_md->sa_idx = rte_cpu_to_be_32(priv_session->sa_index);
+		desc_md->sa_idx = priv_session->sa_index;
 	}
 
 	return 0;
diff --git a/drivers/net/nfp/nfp_ipsec.h b/drivers/net/nfp/nfp_ipsec.h
index d7a729398a..f7c4f3f225 100644
--- a/drivers/net/nfp/nfp_ipsec.h
+++ b/drivers/net/nfp/nfp_ipsec.h
@@ -36,11 +36,6 @@  struct sa_ctrl_word {
 	uint32_t spare2 :1;      /**< Must be set to 0 */
 };
 
-union nfp_ip_addr {
-	struct in6_addr v6;
-	struct in_addr v4;
-};
-
 struct ipsec_add_sa {
 	uint32_t cipher_key[8];           /**< Cipher Key */
 	union {
@@ -60,8 +55,8 @@  struct ipsec_add_sa {
 	uint8_t spare1;
 	uint32_t soft_byte_cnt;           /**< Soft lifetime byte count */
 	uint32_t hard_byte_cnt;           /**< Hard lifetime byte count */
-	union nfp_ip_addr src_ip;         /**< Src IP addr */
-	union nfp_ip_addr dst_ip;         /**< Dst IP addr */
+	uint32_t src_ip[4];               /**< Src IP addr */
+	uint32_t dst_ip[4];               /**< Dst IP addr */
 	uint16_t natt_dst_port;           /**< NAT-T UDP Header dst port */
 	uint16_t natt_src_port;           /**< NAT-T UDP Header src port */
 	uint32_t soft_lifetime_limit;     /**< Soft lifetime time limit */