net/ice: cleanup for vec path check

Message ID 20200116070948.108027-1-xiaolong.ye@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: xiaolong ye
Headers
Series net/ice: cleanup for vec path check |

Checks

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

Commit Message

Xiaolong Ye Jan. 16, 2020, 7:09 a.m. UTC
  Move the conditional compilation block to the inner check helper, so we
can reduce the number of multiple ifdef check pairs used.

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/ice/ice_rxtx.c            | 9 ---------
 drivers/net/ice/ice_rxtx_vec_common.h | 8 ++++++++
 2 files changed, 8 insertions(+), 9 deletions(-)
  

Comments

Ferruh Yigit Jan. 16, 2020, 10:19 a.m. UTC | #1
On 1/16/2020 7:09 AM, Xiaolong Ye wrote:
> Move the conditional compilation block to the inner check helper, so we
> can reduce the number of multiple ifdef check pairs used.
> 
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>

<...>
	
> @@ -2794,10 +2792,8 @@ ice_tx_done_cleanup(void *txq, uint32_t free_cnt)
>  	struct ice_adapter *ad =
>  		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>  
> -#ifdef RTE_ARCH_X86
>  	if (ad->tx_vec_allowed)
>  		return ice_tx_done_cleanup_vec(q, free_cnt);
> -#endif
>  	if (ad->tx_simple_allowed)
>  		return ice_tx_done_cleanup_simple(q, free_cnt);
>  	else
> @@ -2953,7 +2949,6 @@ ice_set_rx_function(struct rte_eth_dev *dev)
>  	PMD_INIT_FUNC_TRACE();
>  	struct ice_adapter *ad =
>  		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> -#ifdef RTE_ARCH_X86
>  	struct ice_rx_queue *rxq;
>  	int i;
>  	bool use_avx2 = false;


The build is still failing for arm, some defines like 'RTE_CPUFLAG_AVX2' &
'RTE_CPUFLAG_AVX512F' or functions 'ice_rxq_vec_setup', 'ice_recv_pkts_vec',
'ice_recv_scattered_pkts_vec' etc only defined for x86

It looks like more work is required, to created dummy versions of these failin
functions also moving 'ice_rx_vec_dev_check()' form 'ice_rxtx_vec_sse.c' to
'ice_rxtx.c', otherwise we are having chicken-egg problem. So needs something
similar to done in i40e.

If it is too much work for rc1, we can with existing #ifdef for now, up to you.
  
Qiming Yang Jan. 17, 2020, 2:21 a.m. UTC | #2
> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Thursday, January 16, 2020 3:10 PM
> To: Yang, Qiming <qiming.yang@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Di, ChenxuX
> <chenxux.di@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: [PATCH] net/ice: cleanup for vec path check
> 
> Move the conditional compilation block to the inner check helper, so we can
> reduce the number of multiple ifdef check pairs used.
> 
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>

Acked-by: Qiming Yang <qiming.yang@intel.com>

> ---
>  drivers/net/ice/ice_rxtx.c            | 9 ---------
>  drivers/net/ice/ice_rxtx_vec_common.h | 8 ++++++++
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> 71adba809..8feeeb828 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -2753,14 +2753,12 @@ ice_tx_done_cleanup_full(struct ice_tx_queue *txq,
>  	return (int)pkt_cnt;
>  }
> 
> -#ifdef RTE_ARCH_X86
>  static int
>  ice_tx_done_cleanup_vec(struct ice_tx_queue *txq __rte_unused,
>  			uint32_t free_cnt __rte_unused)
>  {
>  	return -ENOTSUP;
>  }
> -#endif
> 
>  static int
>  ice_tx_done_cleanup_simple(struct ice_tx_queue *txq, @@ -2794,10 +2792,8
> @@ ice_tx_done_cleanup(void *txq, uint32_t free_cnt)
>  	struct ice_adapter *ad =
>  		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> 
> -#ifdef RTE_ARCH_X86
>  	if (ad->tx_vec_allowed)
>  		return ice_tx_done_cleanup_vec(q, free_cnt); -#endif
>  	if (ad->tx_simple_allowed)
>  		return ice_tx_done_cleanup_simple(q, free_cnt);
>  	else
> @@ -2953,7 +2949,6 @@ ice_set_rx_function(struct rte_eth_dev *dev)
>  	PMD_INIT_FUNC_TRACE();
>  	struct ice_adapter *ad =
>  		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> -#ifdef RTE_ARCH_X86
>  	struct ice_rx_queue *rxq;
>  	int i;
>  	bool use_avx2 = false;
> @@ -2998,8 +2993,6 @@ ice_set_rx_function(struct rte_eth_dev *dev)
>  		return;
>  	}
> 
> -#endif
> -
>  	if (dev->data->scattered_rx) {
>  		/* Set the non-LRO scattered function */
>  		PMD_INIT_LOG(DEBUG,
> @@ -3131,7 +3124,6 @@ ice_set_tx_function(struct rte_eth_dev *dev)  {
>  	struct ice_adapter *ad =
>  		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> -#ifdef RTE_ARCH_X86
>  	struct ice_tx_queue *txq;
>  	int i;
>  	bool use_avx2 = false;
> @@ -3167,7 +3159,6 @@ ice_set_tx_function(struct rte_eth_dev *dev)
> 
>  		return;
>  	}
> -#endif
> 
>  	if (ad->tx_simple_allowed) {
>  		PMD_INIT_LOG(DEBUG, "Simple tx finally be used."); diff --git
> a/drivers/net/ice/ice_rxtx_vec_common.h
> b/drivers/net/ice/ice_rxtx_vec_common.h
> index 6b57ff2ae..223aac878 100644
> --- a/drivers/net/ice/ice_rxtx_vec_common.h
> +++ b/drivers/net/ice/ice_rxtx_vec_common.h
> @@ -267,6 +267,7 @@ ice_tx_vec_queue_default(struct ice_tx_queue *txq)
> static inline int  ice_rx_vec_dev_check_default(struct rte_eth_dev *dev)  {
> +#ifdef RTE_ARCH_X86
>  	int i;
>  	struct ice_rx_queue *rxq;
>  	struct ice_adapter *ad =
> @@ -283,11 +284,15 @@ ice_rx_vec_dev_check_default(struct rte_eth_dev
> *dev)
>  	}
> 
>  	return 0;
> +#else
> +	return -1;
> +#endif
>  }
> 
>  static inline int
>  ice_tx_vec_dev_check_default(struct rte_eth_dev *dev)  {
> +#ifdef RTE_ARCH_X86
>  	int i;
>  	struct ice_tx_queue *txq;
> 
> @@ -298,6 +303,9 @@ ice_tx_vec_dev_check_default(struct rte_eth_dev *dev)
>  	}
> 
>  	return 0;
> +#else
> +	return -1;
> +#endif
>  }
> 
>  #endif
> --
> 2.17.1
  

Patch

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 71adba809..8feeeb828 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2753,14 +2753,12 @@  ice_tx_done_cleanup_full(struct ice_tx_queue *txq,
 	return (int)pkt_cnt;
 }
 
-#ifdef RTE_ARCH_X86
 static int
 ice_tx_done_cleanup_vec(struct ice_tx_queue *txq __rte_unused,
 			uint32_t free_cnt __rte_unused)
 {
 	return -ENOTSUP;
 }
-#endif
 
 static int
 ice_tx_done_cleanup_simple(struct ice_tx_queue *txq,
@@ -2794,10 +2792,8 @@  ice_tx_done_cleanup(void *txq, uint32_t free_cnt)
 	struct ice_adapter *ad =
 		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 
-#ifdef RTE_ARCH_X86
 	if (ad->tx_vec_allowed)
 		return ice_tx_done_cleanup_vec(q, free_cnt);
-#endif
 	if (ad->tx_simple_allowed)
 		return ice_tx_done_cleanup_simple(q, free_cnt);
 	else
@@ -2953,7 +2949,6 @@  ice_set_rx_function(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 	struct ice_adapter *ad =
 		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-#ifdef RTE_ARCH_X86
 	struct ice_rx_queue *rxq;
 	int i;
 	bool use_avx2 = false;
@@ -2998,8 +2993,6 @@  ice_set_rx_function(struct rte_eth_dev *dev)
 		return;
 	}
 
-#endif
-
 	if (dev->data->scattered_rx) {
 		/* Set the non-LRO scattered function */
 		PMD_INIT_LOG(DEBUG,
@@ -3131,7 +3124,6 @@  ice_set_tx_function(struct rte_eth_dev *dev)
 {
 	struct ice_adapter *ad =
 		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-#ifdef RTE_ARCH_X86
 	struct ice_tx_queue *txq;
 	int i;
 	bool use_avx2 = false;
@@ -3167,7 +3159,6 @@  ice_set_tx_function(struct rte_eth_dev *dev)
 
 		return;
 	}
-#endif
 
 	if (ad->tx_simple_allowed) {
 		PMD_INIT_LOG(DEBUG, "Simple tx finally be used.");
diff --git a/drivers/net/ice/ice_rxtx_vec_common.h b/drivers/net/ice/ice_rxtx_vec_common.h
index 6b57ff2ae..223aac878 100644
--- a/drivers/net/ice/ice_rxtx_vec_common.h
+++ b/drivers/net/ice/ice_rxtx_vec_common.h
@@ -267,6 +267,7 @@  ice_tx_vec_queue_default(struct ice_tx_queue *txq)
 static inline int
 ice_rx_vec_dev_check_default(struct rte_eth_dev *dev)
 {
+#ifdef RTE_ARCH_X86
 	int i;
 	struct ice_rx_queue *rxq;
 	struct ice_adapter *ad =
@@ -283,11 +284,15 @@  ice_rx_vec_dev_check_default(struct rte_eth_dev *dev)
 	}
 
 	return 0;
+#else
+	return -1;
+#endif
 }
 
 static inline int
 ice_tx_vec_dev_check_default(struct rte_eth_dev *dev)
 {
+#ifdef RTE_ARCH_X86
 	int i;
 	struct ice_tx_queue *txq;
 
@@ -298,6 +303,9 @@  ice_tx_vec_dev_check_default(struct rte_eth_dev *dev)
 	}
 
 	return 0;
+#else
+	return -1;
+#endif
 }
 
 #endif