patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem
       [not found] <20240311024939.2523778-1-chaoyong.he@corigine.com>
@ 2024-03-11  2:49 ` Chaoyong He
  2024-03-13 18:33   ` [EXTERNAL] " Akhil Goyal
  2024-03-14  2:00   ` [PATCH v2] " Chaoyong He
  2024-03-11  2:49 ` [PATCH 2/2] net/nfp: fix data " Chaoyong He
  1 sibling, 2 replies; 8+ messages in thread
From: Chaoyong He @ 2024-03-11  2:49 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Shihong Wang, stable, Chaoyong He

From: Shihong Wang <shihong.wang@corigine.com>

The SA salt of struct ipsec_sa is a CPU-endian u32 variable, but it’s
value is stored in an array of encryption or authentication keys
according to big-endian. So it maybe need to convert the endianness
order to ensure that the value assigned to the SA salt is CPU-endian.

Fixes: 50d75cae2a2c ("examples/ipsec-secgw: initialize SA salt")
Fixes: 9413c3901f31 ("examples/ipsec-secgw: support additional algorithms")
Fixes: 501e9c226adf ("examples/ipsec-secgw: add AEAD parameters")
Cc: stable@dpdk.org

Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 examples/ipsec-secgw/sa.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index c4bac17cd7..4018b0558a 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -374,6 +374,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 	uint32_t ti; /*token index*/
 	uint32_t *ri /*rule index*/;
 	struct ipsec_sa_cnt *sa_cnt;
+	rte_be32_t salt; /*big-endian salt*/
 	uint32_t cipher_algo_p = 0;
 	uint32_t auth_algo_p = 0;
 	uint32_t aead_algo_p = 0;
@@ -508,8 +509,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 			if (algo->algo == RTE_CRYPTO_CIPHER_AES_CTR) {
 				key_len -= 4;
 				rule->cipher_key_len = key_len;
-				memcpy(&rule->salt,
+				memcpy(&salt,
 					&rule->cipher_key[key_len], 4);
+				rule->salt = rte_be_to_cpu_32(salt);
 			}
 
 			cipher_algo_p = 1;
@@ -573,8 +575,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 				key_len -= 4;
 				rule->auth_key_len = key_len;
 				rule->iv_len = algo->iv_len;
-				memcpy(&rule->salt,
+				memcpy(&salt,
 					&rule->auth_key[key_len], 4);
+				rule->salt = rte_be_to_cpu_32(salt);
 			}
 
 			auth_algo_p = 1;
@@ -632,8 +635,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 
 			key_len -= 4;
 			rule->cipher_key_len = key_len;
-			memcpy(&rule->salt,
+			memcpy(&salt,
 				&rule->cipher_key[key_len], 4);
+			rule->salt = rte_be_to_cpu_32(salt);
 
 			aead_algo_p = 1;
 			continue;
-- 
2.39.1


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

* [PATCH 2/2] net/nfp: fix data endianness problem
       [not found] <20240311024939.2523778-1-chaoyong.he@corigine.com>
  2024-03-11  2:49 ` [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem Chaoyong He
@ 2024-03-11  2:49 ` Chaoyong He
  2024-03-13 15:39   ` Ferruh Yigit
  1 sibling, 1 reply; 8+ messages in thread
From: Chaoyong He @ 2024-03-11  2:49 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Shihong Wang, stable, Chaoyong He

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

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 */
-- 
2.39.1


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

* Re: [PATCH 2/2] net/nfp: fix data endianness problem
  2024-03-11  2:49 ` [PATCH 2/2] net/nfp: fix data " Chaoyong He
@ 2024-03-13 15:39   ` Ferruh Yigit
  0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2024-03-13 15:39 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers, Shihong Wang, stable

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)

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

* RE: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem
  2024-03-11  2:49 ` [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem Chaoyong He
@ 2024-03-13 18:33   ` Akhil Goyal
  2024-03-14  1:41     ` Chaoyong He
  2024-03-14  2:00   ` [PATCH v2] " Chaoyong He
  1 sibling, 1 reply; 8+ messages in thread
From: Akhil Goyal @ 2024-03-13 18:33 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers, Shihong Wang, stable

> Subject: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness
> problem
> From: Shihong Wang <shihong.wang@corigine.com>
> 
> The SA salt of struct ipsec_sa is a CPU-endian u32 variable, but it’s
> value is stored in an array of encryption or authentication keys
> according to big-endian. So it maybe need to convert the endianness
> order to ensure that the value assigned to the SA salt is CPU-endian.
> 
> Fixes: 50d75cae2a2c ("examples/ipsec-secgw: initialize SA salt")
> Fixes: 9413c3901f31 ("examples/ipsec-secgw: support additional algorithms")
> Fixes: 501e9c226adf ("examples/ipsec-secgw: add AEAD parameters")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> ---
>  examples/ipsec-secgw/sa.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index c4bac17cd7..4018b0558a 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -374,6 +374,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  	uint32_t ti; /*token index*/
>  	uint32_t *ri /*rule index*/;
>  	struct ipsec_sa_cnt *sa_cnt;
> +	rte_be32_t salt; /*big-endian salt*/
>  	uint32_t cipher_algo_p = 0;
>  	uint32_t auth_algo_p = 0;
>  	uint32_t aead_algo_p = 0;
> @@ -508,8 +509,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  			if (algo->algo == RTE_CRYPTO_CIPHER_AES_CTR) {
>  				key_len -= 4;
>  				rule->cipher_key_len = key_len;
> -				memcpy(&rule->salt,
> +				memcpy(&salt,
>  					&rule->cipher_key[key_len], 4);
> +				rule->salt = rte_be_to_cpu_32(salt);
>  			}
> 
>  			cipher_algo_p = 1;
> @@ -573,8 +575,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  				key_len -= 4;
>  				rule->auth_key_len = key_len;
>  				rule->iv_len = algo->iv_len;
> -				memcpy(&rule->salt,
> +				memcpy(&salt,
>  					&rule->auth_key[key_len], 4);
> +				rule->salt = rte_be_to_cpu_32(salt);
>  			}
> 
>  			auth_algo_p = 1;
> @@ -632,8 +635,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> 
>  			key_len -= 4;
>  			rule->cipher_key_len = key_len;
> -			memcpy(&rule->salt,
> +			memcpy(&salt,
>  				&rule->cipher_key[key_len], 4);
Can you put the memcpy call in a single line?

> +			rule->salt = rte_be_to_cpu_32(salt);


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

* RE: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem
  2024-03-13 18:33   ` [EXTERNAL] " Akhil Goyal
@ 2024-03-14  1:41     ` Chaoyong He
  0 siblings, 0 replies; 8+ messages in thread
From: Chaoyong He @ 2024-03-14  1:41 UTC (permalink / raw)
  To: Akhil Goyal, dev; +Cc: oss-drivers, Shihong Wang, stable

> > Subject: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt
> > endianness problem
> > From: Shihong Wang <shihong.wang@corigine.com>
> >
> > The SA salt of struct ipsec_sa is a CPU-endian u32 variable, but it’s
> > value is stored in an array of encryption or authentication keys
> > according to big-endian. So it maybe need to convert the endianness
> > order to ensure that the value assigned to the SA salt is CPU-endian.
> >
> > Fixes: 50d75cae2a2c ("examples/ipsec-secgw: initialize SA salt")
> > Fixes: 9413c3901f31 ("examples/ipsec-secgw: support additional
> > algorithms")
> > Fixes: 501e9c226adf ("examples/ipsec-secgw: add AEAD parameters")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > ---
> >  examples/ipsec-secgw/sa.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> > index c4bac17cd7..4018b0558a 100644
> > --- a/examples/ipsec-secgw/sa.c
> > +++ b/examples/ipsec-secgw/sa.c
> > @@ -374,6 +374,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> >  	uint32_t ti; /*token index*/
> >  	uint32_t *ri /*rule index*/;
> >  	struct ipsec_sa_cnt *sa_cnt;
> > +	rte_be32_t salt; /*big-endian salt*/
> >  	uint32_t cipher_algo_p = 0;
> >  	uint32_t auth_algo_p = 0;
> >  	uint32_t aead_algo_p = 0;
> > @@ -508,8 +509,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> >  			if (algo->algo == RTE_CRYPTO_CIPHER_AES_CTR) {
> >  				key_len -= 4;
> >  				rule->cipher_key_len = key_len;
> > -				memcpy(&rule->salt,
> > +				memcpy(&salt,
> >  					&rule->cipher_key[key_len], 4);
> > +				rule->salt = rte_be_to_cpu_32(salt);
> >  			}
> >
> >  			cipher_algo_p = 1;
> > @@ -573,8 +575,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> >  				key_len -= 4;
> >  				rule->auth_key_len = key_len;
> >  				rule->iv_len = algo->iv_len;
> > -				memcpy(&rule->salt,
> > +				memcpy(&salt,
> >  					&rule->auth_key[key_len], 4);
> > +				rule->salt = rte_be_to_cpu_32(salt);
> >  			}
> >
> >  			auth_algo_p = 1;
> > @@ -632,8 +635,9 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> >
> >  			key_len -= 4;
> >  			rule->cipher_key_len = key_len;
> > -			memcpy(&rule->salt,
> > +			memcpy(&salt,
> >  				&rule->cipher_key[key_len], 4);
> Can you put the memcpy call in a single line?

Okay, no problem.
I will send a v2 patch to fix that, thanks.

> 
> > +			rule->salt = rte_be_to_cpu_32(salt);


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

* [PATCH v2] examples/ipsec-secgw: fix SA salt endianness problem
  2024-03-11  2:49 ` [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem Chaoyong He
  2024-03-13 18:33   ` [EXTERNAL] " Akhil Goyal
@ 2024-03-14  2:00   ` Chaoyong He
  2024-03-14 18:17     ` [EXTERNAL] " Akhil Goyal
  1 sibling, 1 reply; 8+ messages in thread
From: Chaoyong He @ 2024-03-14  2:00 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Shihong Wang, stable, Chaoyong He

From: Shihong Wang <shihong.wang@corigine.com>

The SA salt of struct ipsec_sa is a CPU-endian u32 variable, but it’s
value is stored in an array of encryption or authentication keys
according to big-endian. So it maybe need to convert the endianness
order to ensure that the value assigned to the SA salt is CPU-endian.

Fixes: 50d75cae2a2c ("examples/ipsec-secgw: initialize SA salt")
Fixes: 9413c3901f31 ("examples/ipsec-secgw: support additional algorithms")
Fixes: 501e9c226adf ("examples/ipsec-secgw: add AEAD parameters")
Cc: stable@dpdk.org

Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>

---
v2:
* Put the 'memcpy()' call in a singal line as the review comment.
---
 examples/ipsec-secgw/sa.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index c4bac17cd7..8aa9aca739 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -374,6 +374,7 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 	uint32_t ti; /*token index*/
 	uint32_t *ri /*rule index*/;
 	struct ipsec_sa_cnt *sa_cnt;
+	rte_be32_t salt; /*big-endian salt*/
 	uint32_t cipher_algo_p = 0;
 	uint32_t auth_algo_p = 0;
 	uint32_t aead_algo_p = 0;
@@ -508,8 +509,8 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 			if (algo->algo == RTE_CRYPTO_CIPHER_AES_CTR) {
 				key_len -= 4;
 				rule->cipher_key_len = key_len;
-				memcpy(&rule->salt,
-					&rule->cipher_key[key_len], 4);
+				memcpy(&salt, &rule->cipher_key[key_len], 4);
+				rule->salt = rte_be_to_cpu_32(salt);
 			}
 
 			cipher_algo_p = 1;
@@ -573,8 +574,8 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 				key_len -= 4;
 				rule->auth_key_len = key_len;
 				rule->iv_len = algo->iv_len;
-				memcpy(&rule->salt,
-					&rule->auth_key[key_len], 4);
+				memcpy(&salt, &rule->auth_key[key_len], 4);
+				rule->salt = rte_be_to_cpu_32(salt);
 			}
 
 			auth_algo_p = 1;
@@ -632,8 +633,8 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 
 			key_len -= 4;
 			rule->cipher_key_len = key_len;
-			memcpy(&rule->salt,
-				&rule->cipher_key[key_len], 4);
+			memcpy(&salt, &rule->cipher_key[key_len], 4);
+			rule->salt = rte_be_to_cpu_32(salt);
 
 			aead_algo_p = 1;
 			continue;
-- 
2.39.1


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

* RE: [EXTERNAL] [PATCH v2] examples/ipsec-secgw: fix SA salt endianness problem
  2024-03-14  2:00   ` [PATCH v2] " Chaoyong He
@ 2024-03-14 18:17     ` Akhil Goyal
  2024-03-14 19:11       ` Akhil Goyal
  0 siblings, 1 reply; 8+ messages in thread
From: Akhil Goyal @ 2024-03-14 18:17 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers, Shihong Wang, stable

> From: Shihong Wang <shihong.wang@corigine.com>
> 
> The SA salt of struct ipsec_sa is a CPU-endian u32 variable, but it’s
> value is stored in an array of encryption or authentication keys
> according to big-endian. So it maybe need to convert the endianness
> order to ensure that the value assigned to the SA salt is CPU-endian.
> 
> Fixes: 50d75cae2a2c ("examples/ipsec-secgw: initialize SA salt")
> Fixes: 9413c3901f31 ("examples/ipsec-secgw: support additional algorithms")
> Fixes: 501e9c226adf ("examples/ipsec-secgw: add AEAD parameters")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> 
Acked-by: Akhil Goyal <gakhil@marvell.com>

Applied to dpdk-next-crypto
Thanks

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

* RE: [EXTERNAL] [PATCH v2] examples/ipsec-secgw: fix SA salt endianness problem
  2024-03-14 18:17     ` [EXTERNAL] " Akhil Goyal
@ 2024-03-14 19:11       ` Akhil Goyal
  0 siblings, 0 replies; 8+ messages in thread
From: Akhil Goyal @ 2024-03-14 19:11 UTC (permalink / raw)
  To: Akhil Goyal, Chaoyong He, dev; +Cc: oss-drivers, Shihong Wang, stable

> Subject: RE: [EXTERNAL] [PATCH v2] examples/ipsec-secgw: fix SA salt
> endianness problem
> 
> > From: Shihong Wang <shihong.wang@corigine.com>
> >
> > The SA salt of struct ipsec_sa is a CPU-endian u32 variable, but it’s
> > value is stored in an array of encryption or authentication keys
> > according to big-endian. So it maybe need to convert the endianness
> > order to ensure that the value assigned to the SA salt is CPU-endian.
> >
> > Fixes: 50d75cae2a2c ("examples/ipsec-secgw: initialize SA salt")
> > Fixes: 9413c3901f31 ("examples/ipsec-secgw: support additional algorithms")
> > Fixes: 501e9c226adf ("examples/ipsec-secgw: add AEAD parameters")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Shihong Wang <shihong.wang@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> >
> Acked-by: Akhil Goyal <gakhil@marvell.com>
> 
> Applied to dpdk-next-crypto

The patch is pulled back from dpdk-next-crypto.
This change may cause all the PMDs to fail these cases.
Would need acks from PMDs.


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

end of thread, other threads:[~2024-03-14 19:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20240311024939.2523778-1-chaoyong.he@corigine.com>
2024-03-11  2:49 ` [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem Chaoyong He
2024-03-13 18:33   ` [EXTERNAL] " Akhil Goyal
2024-03-14  1:41     ` Chaoyong He
2024-03-14  2:00   ` [PATCH v2] " Chaoyong He
2024-03-14 18:17     ` [EXTERNAL] " Akhil Goyal
2024-03-14 19:11       ` Akhil Goyal
2024-03-11  2:49 ` [PATCH 2/2] net/nfp: fix data " Chaoyong He
2024-03-13 15:39   ` Ferruh Yigit

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