diff mbox

[dpdk-dev] net/ixgbe: fix IPsec Rx with SSE on

Message ID 1509110679-23926-1-git-send-email-radu.nicolau@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Radu Nicolau Oct. 27, 2017, 1:24 p.m. UTC 
  Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
And small performance improvement

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 79 +++++++++++++---------------------
 1 file changed, 30 insertions(+), 49 deletions(-)

Comments

Ananyev, Konstantin Oct. 31, 2017, 3:47 p.m. UTC | #1 
> -----Original Message-----
> From: Nicolau, Radu
> Sent: Friday, October 27, 2017 2:25 PM
> To: dev@dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>
> Subject: [PATCH] net/ixgbe: fix IPsec Rx with SSE on
> 
> Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
> And small performance improvement
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 79 +++++++++++++---------------------
>  1 file changed, 30 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> index 158ccd9..6d7ed5d 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> @@ -126,54 +126,35 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
>  static inline void
>  desc_to_olflags_v_ipsec(__m128i descs[4], struct rte_mbuf **rx_pkts)
>  {
> -	__m128i sterr0, sterr1, sterr2, sterr3;
> -	__m128i tmp1, tmp2, tmp3, tmp4;
> -	__m128i rearm0, rearm1, rearm2, rearm3;
> -
> -	const __m128i ipsec_sterr_msk = _mm_set_epi32(
> -		0, IXGBE_RXDADV_IPSEC_STATUS_SECP |
> -			IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED,
> -		0, 0);
> -	const __m128i ipsec_proc_msk  = _mm_set_epi32(
> -		0, IXGBE_RXDADV_IPSEC_STATUS_SECP, 0, 0);
> -	const __m128i ipsec_err_flag  = _mm_set_epi32(
> -		0, PKT_RX_SEC_OFFLOAD_FAILED | PKT_RX_SEC_OFFLOAD,
> -		0, 0);
> -	const __m128i ipsec_proc_flag = _mm_set_epi32(
> -		0, PKT_RX_SEC_OFFLOAD, 0, 0);
> -
> -	rearm0 = _mm_load_si128((__m128i *)&rx_pkts[0]->rearm_data);
> -	rearm1 = _mm_load_si128((__m128i *)&rx_pkts[1]->rearm_data);
> -	rearm2 = _mm_load_si128((__m128i *)&rx_pkts[2]->rearm_data);
> -	rearm3 = _mm_load_si128((__m128i *)&rx_pkts[3]->rearm_data);
> -	sterr0 = _mm_and_si128(descs[0], ipsec_sterr_msk);
> -	sterr1 = _mm_and_si128(descs[1], ipsec_sterr_msk);
> -	sterr2 = _mm_and_si128(descs[2], ipsec_sterr_msk);
> -	sterr3 = _mm_and_si128(descs[3], ipsec_sterr_msk);
> -	tmp1 = _mm_cmpeq_epi32(sterr0, ipsec_sterr_msk);
> -	tmp2 = _mm_cmpeq_epi32(sterr0, ipsec_proc_msk);
> -	tmp3 = _mm_cmpeq_epi32(sterr1, ipsec_sterr_msk);
> -	tmp4 = _mm_cmpeq_epi32(sterr1, ipsec_proc_msk);
> -	sterr0 = _mm_or_si128(_mm_and_si128(tmp1, ipsec_err_flag),
> -				_mm_and_si128(tmp2, ipsec_proc_flag));
> -	sterr1 = _mm_or_si128(_mm_and_si128(tmp3, ipsec_err_flag),
> -				_mm_and_si128(tmp4, ipsec_proc_flag));
> -	tmp1 = _mm_cmpeq_epi32(sterr2, ipsec_sterr_msk);
> -	tmp2 = _mm_cmpeq_epi32(sterr2, ipsec_proc_msk);
> -	tmp3 = _mm_cmpeq_epi32(sterr3, ipsec_sterr_msk);
> -	tmp4 = _mm_cmpeq_epi32(sterr3, ipsec_proc_msk);
> -	sterr2 = _mm_or_si128(_mm_and_si128(tmp1, ipsec_err_flag),
> -				_mm_and_si128(tmp2, ipsec_proc_flag));
> -	sterr3 = _mm_or_si128(_mm_and_si128(tmp3, ipsec_err_flag),
> -				_mm_and_si128(tmp4, ipsec_proc_flag));
> -	rearm0 = _mm_or_si128(rearm0, sterr0);
> -	rearm1 = _mm_or_si128(rearm1, sterr1);
> -	rearm2 = _mm_or_si128(rearm2, sterr2);
> -	rearm3 = _mm_or_si128(rearm3, sterr3);
> -	_mm_store_si128((__m128i *)&rx_pkts[0]->rearm_data, rearm0);
> -	_mm_store_si128((__m128i *)&rx_pkts[1]->rearm_data, rearm1);
> -	_mm_store_si128((__m128i *)&rx_pkts[2]->rearm_data, rearm2);
> -	_mm_store_si128((__m128i *)&rx_pkts[3]->rearm_data, rearm3);
> +	__m128i sterr, rearm, tmp_e, tmp_p;
> +	const __m128i ipsec_sterr_msk =
> +			_mm_set1_epi32(IXGBE_RXDADV_IPSEC_STATUS_SECP |
> +				       IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED);
> +	const __m128i ipsec_proc_msk  =
> +			_mm_set1_epi32(IXGBE_RXDADV_IPSEC_STATUS_SECP);
> +	const __m128i ipsec_err_flag  =
> +			_mm_set1_epi32(PKT_RX_SEC_OFFLOAD_FAILED |
> +				       PKT_RX_SEC_OFFLOAD);
> +	const __m128i ipsec_proc_flag = _mm_set1_epi32(PKT_RX_SEC_OFFLOAD);
> +
> +	rearm = _mm_set_epi32(((uint32_t *)rx_pkts[0]->rearm_data)[2],
> +			      ((uint32_t *)rx_pkts[1]->rearm_data)[2],
> +			      ((uint32_t *)rx_pkts[2]->rearm_data)[2],
> +			      ((uint32_t *)rx_pkts[3]->rearm_data)[2]);
> +	sterr = _mm_set_epi32(_mm_extract_epi32(descs[0], 2),
> +			      _mm_extract_epi32(descs[1], 2),
> +			      _mm_extract_epi32(descs[2], 2),
> +			      _mm_extract_epi32(descs[3], 2));
> +	sterr = _mm_and_si128(sterr, ipsec_sterr_msk);
> +	tmp_e = _mm_cmpeq_epi32(sterr, ipsec_sterr_msk);
> +	tmp_p = _mm_cmpeq_epi32(sterr, ipsec_proc_msk);
> +	sterr = _mm_or_si128(_mm_and_si128(tmp_e, ipsec_err_flag),
> +				_mm_and_si128(tmp_p, ipsec_proc_flag));
> +	rearm = _mm_or_si128(rearm, sterr);
> +	((uint32_t *)rx_pkts[0]->rearm_data)[2] = _mm_extract_epi32(rearm, 3);
> +	((uint32_t *)rx_pkts[1]->rearm_data)[2] = _mm_extract_epi32(rearm, 2);
> +	((uint32_t *)rx_pkts[2]->rearm_data)[2] = _mm_extract_epi32(rearm, 1);
> +	((uint32_t *)rx_pkts[3]->rearm_data)[2] = _mm_extract_epi32(rearm, 0);
>  }
>  #endif
> 
> @@ -533,7 +514,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
> 
>  #ifdef RTE_LIBRTE_SECURITY
>  		if (unlikely(use_ipsec))
> -			desc_to_olflags_v_ipsec(descs, rx_pkts);
> +			desc_to_olflags_v_ipsec(descs, &rx_pkts[pos]);
>  #endif
> 
>  		/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.7.5
Ferruh Yigit Oct. 31, 2017, 5:15 p.m. UTC | #2 
On 10/31/2017 8:47 AM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Nicolau, Radu
>> Sent: Friday, October 27, 2017 2:25 PM
>> To: dev@dpdk.org
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>
>> Subject: [PATCH] net/ixgbe: fix IPsec Rx with SSE on
>>
>> Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
>> And small performance improvement
>>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied to dpdk-next-net/master, thanks.
Shahaf Shuler Oct. 31, 2017, 6:51 p.m. UTC | #3 
Tuesday, October 31, 2017 7:16 PM, Ferruh Yigit:
> On 10/31/2017 8:47 AM, Ananyev, Konstantin wrote:

> >

> >

> >> -----Original Message-----

> >> From: Nicolau, Radu

> >> Sent: Friday, October 27, 2017 2:25 PM

> >> To: dev@dpdk.org

> >> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Nicolau, Radu

> >> <radu.nicolau@intel.com>

> >> Subject: [PATCH] net/ixgbe: fix IPsec Rx with SSE on

> >>

> >> Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec") And small

> >> performance improvement

> >>

> >> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

> 

> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 

> Applied to dpdk-next-net/master, thanks.


It looks this commit breaks the compilation with GCC on redhat 7.3.

Server details:
4.13.0-rc4+ #1 SMP Wed Aug 9 09:18:48 IDT 2017 x86_64 x86_64 x86_64 GNU/Linux
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-11)


Compilation log:

Configuration done using x86_64-native-linuxapp-gcc
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c: In function ‘desc_to_olflags_v_ipsec’:
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:140:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
rearm = _mm_set_epi32(((uint32_t *)rx_pkts[0]->rearm_data)[2],
^
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:141:10: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
((uint32_t *)rx_pkts[1]->rearm_data)[2],
^
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:142:10: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
((uint32_t *)rx_pkts[2]->rearm_data)[2],
^
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:143:10: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
((uint32_t *)rx_pkts[3]->rearm_data)[2]);
^
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:154:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
((uint32_t *)rx_pkts[0]->rearm_data)[2] = _mm_extract_epi32(rearm, 3);
^
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:155:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
((uint32_t *)rx_pkts[1]->rearm_data)[2] = _mm_extract_epi32(rearm, 2);
^
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:156:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
((uint32_t *)rx_pkts[2]->rearm_data)[2] = _mm_extract_epi32(rearm, 1);
^
/work/dpdk.org/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c:157:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
((uint32_t *)rx_pkts[3]->rearm_data)[2] = _mm_extract_epi32(rearm, 0);
^
cc1: all warnings being treated as errors
make[4]: *** [ixgbe_rxtx_vec_sse.o] Error 1
make[3]: *** [ixgbe] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [net] Error 2
make[1]: *** [drivers] Error 2
make: *** [all] Error 2
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
index 158ccd9..6d7ed5d 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
@@ -126,54 +126,35 @@  ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
 static inline void
 desc_to_olflags_v_ipsec(__m128i descs[4], struct rte_mbuf **rx_pkts)
 {
-	__m128i sterr0, sterr1, sterr2, sterr3;
-	__m128i tmp1, tmp2, tmp3, tmp4;
-	__m128i rearm0, rearm1, rearm2, rearm3;
-
-	const __m128i ipsec_sterr_msk = _mm_set_epi32(
-		0, IXGBE_RXDADV_IPSEC_STATUS_SECP |
-			IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED,
-		0, 0);
-	const __m128i ipsec_proc_msk  = _mm_set_epi32(
-		0, IXGBE_RXDADV_IPSEC_STATUS_SECP, 0, 0);
-	const __m128i ipsec_err_flag  = _mm_set_epi32(
-		0, PKT_RX_SEC_OFFLOAD_FAILED | PKT_RX_SEC_OFFLOAD,
-		0, 0);
-	const __m128i ipsec_proc_flag = _mm_set_epi32(
-		0, PKT_RX_SEC_OFFLOAD, 0, 0);
-
-	rearm0 = _mm_load_si128((__m128i *)&rx_pkts[0]->rearm_data);
-	rearm1 = _mm_load_si128((__m128i *)&rx_pkts[1]->rearm_data);
-	rearm2 = _mm_load_si128((__m128i *)&rx_pkts[2]->rearm_data);
-	rearm3 = _mm_load_si128((__m128i *)&rx_pkts[3]->rearm_data);
-	sterr0 = _mm_and_si128(descs[0], ipsec_sterr_msk);
-	sterr1 = _mm_and_si128(descs[1], ipsec_sterr_msk);
-	sterr2 = _mm_and_si128(descs[2], ipsec_sterr_msk);
-	sterr3 = _mm_and_si128(descs[3], ipsec_sterr_msk);
-	tmp1 = _mm_cmpeq_epi32(sterr0, ipsec_sterr_msk);
-	tmp2 = _mm_cmpeq_epi32(sterr0, ipsec_proc_msk);
-	tmp3 = _mm_cmpeq_epi32(sterr1, ipsec_sterr_msk);
-	tmp4 = _mm_cmpeq_epi32(sterr1, ipsec_proc_msk);
-	sterr0 = _mm_or_si128(_mm_and_si128(tmp1, ipsec_err_flag),
-				_mm_and_si128(tmp2, ipsec_proc_flag));
-	sterr1 = _mm_or_si128(_mm_and_si128(tmp3, ipsec_err_flag),
-				_mm_and_si128(tmp4, ipsec_proc_flag));
-	tmp1 = _mm_cmpeq_epi32(sterr2, ipsec_sterr_msk);
-	tmp2 = _mm_cmpeq_epi32(sterr2, ipsec_proc_msk);
-	tmp3 = _mm_cmpeq_epi32(sterr3, ipsec_sterr_msk);
-	tmp4 = _mm_cmpeq_epi32(sterr3, ipsec_proc_msk);
-	sterr2 = _mm_or_si128(_mm_and_si128(tmp1, ipsec_err_flag),
-				_mm_and_si128(tmp2, ipsec_proc_flag));
-	sterr3 = _mm_or_si128(_mm_and_si128(tmp3, ipsec_err_flag),
-				_mm_and_si128(tmp4, ipsec_proc_flag));
-	rearm0 = _mm_or_si128(rearm0, sterr0);
-	rearm1 = _mm_or_si128(rearm1, sterr1);
-	rearm2 = _mm_or_si128(rearm2, sterr2);
-	rearm3 = _mm_or_si128(rearm3, sterr3);
-	_mm_store_si128((__m128i *)&rx_pkts[0]->rearm_data, rearm0);
-	_mm_store_si128((__m128i *)&rx_pkts[1]->rearm_data, rearm1);
-	_mm_store_si128((__m128i *)&rx_pkts[2]->rearm_data, rearm2);
-	_mm_store_si128((__m128i *)&rx_pkts[3]->rearm_data, rearm3);
+	__m128i sterr, rearm, tmp_e, tmp_p;
+	const __m128i ipsec_sterr_msk =
+			_mm_set1_epi32(IXGBE_RXDADV_IPSEC_STATUS_SECP |
+				       IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED);
+	const __m128i ipsec_proc_msk  =
+			_mm_set1_epi32(IXGBE_RXDADV_IPSEC_STATUS_SECP);
+	const __m128i ipsec_err_flag  =
+			_mm_set1_epi32(PKT_RX_SEC_OFFLOAD_FAILED |
+				       PKT_RX_SEC_OFFLOAD);
+	const __m128i ipsec_proc_flag = _mm_set1_epi32(PKT_RX_SEC_OFFLOAD);
+
+	rearm = _mm_set_epi32(((uint32_t *)rx_pkts[0]->rearm_data)[2],
+			      ((uint32_t *)rx_pkts[1]->rearm_data)[2],
+			      ((uint32_t *)rx_pkts[2]->rearm_data)[2],
+			      ((uint32_t *)rx_pkts[3]->rearm_data)[2]);
+	sterr = _mm_set_epi32(_mm_extract_epi32(descs[0], 2),
+			      _mm_extract_epi32(descs[1], 2),
+			      _mm_extract_epi32(descs[2], 2),
+			      _mm_extract_epi32(descs[3], 2));
+	sterr = _mm_and_si128(sterr, ipsec_sterr_msk);
+	tmp_e = _mm_cmpeq_epi32(sterr, ipsec_sterr_msk);
+	tmp_p = _mm_cmpeq_epi32(sterr, ipsec_proc_msk);
+	sterr = _mm_or_si128(_mm_and_si128(tmp_e, ipsec_err_flag),
+				_mm_and_si128(tmp_p, ipsec_proc_flag));
+	rearm = _mm_or_si128(rearm, sterr);
+	((uint32_t *)rx_pkts[0]->rearm_data)[2] = _mm_extract_epi32(rearm, 3);
+	((uint32_t *)rx_pkts[1]->rearm_data)[2] = _mm_extract_epi32(rearm, 2);
+	((uint32_t *)rx_pkts[2]->rearm_data)[2] = _mm_extract_epi32(rearm, 1);
+	((uint32_t *)rx_pkts[3]->rearm_data)[2] = _mm_extract_epi32(rearm, 0);
 }
 #endif
 
@@ -533,7 +514,7 @@  _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 
 #ifdef RTE_LIBRTE_SECURITY
 		if (unlikely(use_ipsec))
-			desc_to_olflags_v_ipsec(descs, rx_pkts);
+			desc_to_olflags_v_ipsec(descs, &rx_pkts[pos]);
 #endif
 
 		/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */