[PATCH 0/2] fix IPSec endianness problem

[PATCH 0/2] fix IPSec endianness problem

[Chaoyong He]

This patch series aims to fix the IPSec problem related with SA
salt endianness.

Shihong Wang (2):
  examples/ipsec-secgw: fix SA salt endianness problem
  net/nfp: fix data endianness problem

 drivers/net/nfp/nfp_ipsec.c | 72 +++++++++++++++++++++++--------------
 drivers/net/nfp/nfp_ipsec.h |  9 ++---
 examples/ipsec-secgw/sa.c   | 10 ++++--
 3 files changed, 54 insertions(+), 37 deletions(-)

-- 
2.39.1

[PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem

[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

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

[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

Re: [PATCH 0/2] fix IPSec endianness problem

[Ferruh Yigit]

On 3/11/2024 2:49 AM, Chaoyong He wrote:
> This patch series aims to fix the IPSec problem related with SA
> salt endianness.
> 
> Shihong Wang (2):
>   examples/ipsec-secgw: fix SA salt endianness problem
>   net/nfp: fix data endianness problem
>

Hi Chaoyong,

Just to confirm, there is no dependency between above two patches, right?
So it will be OK for me and Akhil merge them to different trees, and
they will end up merged in a different order.

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

[Ferruh Yigit]

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)

RE: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem

[Akhil Goyal]

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

RE: [EXTERNAL] [PATCH 1/2] examples/ipsec-secgw: fix SA salt endianness problem

[Chaoyong He]

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

[PATCH v2] examples/ipsec-secgw: fix SA salt endianness problem

[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

RE: [EXTERNAL] [PATCH v2] examples/ipsec-secgw: fix SA salt endianness problem

[Akhil Goyal]

> 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

RE: [EXTERNAL] [PATCH v2] examples/ipsec-secgw: fix SA salt endianness problem

[Akhil Goyal]

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