[dpdk-dev] [PATCH v3 11/18] net/ixgbe: add checks for max SIMD bitwidth

[Wang, Haiyue]

Hi Ciara,

> -----Original Message-----
> From: Power, Ciara <ciara.power at intel.com>
> Sent: Wednesday, September 30, 2020 21:04
> To: dev at dpdk.org
> Cc: Power, Ciara <ciara.power at intel.com>; Zhao1, Wei <wei.zhao1 at intel.com>; Guo, Jia
> <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>
> Subject: [PATCH v3 11/18] net/ixgbe: add checks for max SIMD bitwidth
> 
> When choosing a vector path to take, an extra condition must be
> satisfied to ensure the max SIMD bitwidth allows for the CPU enabled
> path.
> 
> Cc: Wei Zhao <wei.zhao1 at intel.com>
> Cc: Jeff Guo <jia.guo at intel.com>
> 
> Signed-off-by: Ciara Power <ciara.power at intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 977ecf5137..eadc7183f2 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2503,7 +2503,9 @@ ixgbe_set_tx_function(struct rte_eth_dev *dev, struct ixgbe_tx_queue *txq)
>  		dev->tx_pkt_prepare = NULL;
>  		if (txq->tx_rs_thresh <= RTE_IXGBE_TX_MAX_FREE_BUF_SZ &&
>  				(rte_eal_process_type() != RTE_PROC_PRIMARY ||
> -					ixgbe_txq_vec_setup(txq) == 0)) {
> +					ixgbe_txq_vec_setup(txq) == 0) &&
> +				rte_get_max_simd_bitwidth()

As Konstantin mentioned: " I think it is a bit safer to do all checks first before
 doing txq_vec_setup()."

Fox x86 & arm platforms, the setup is always 0, since 'sw_ring_v' is union with
'sw_ring' which is initialize at 'ixgbe_dev_tx_queue_setup'.

	union {
		struct ixgbe_tx_entry *sw_ring; /**< address of SW ring for scalar PMD. */
		struct ixgbe_tx_entry_v *sw_ring_v; /**< address of SW ring for vector PMD */
	};

static inline int
ixgbe_txq_vec_setup_default(struct ixgbe_tx_queue *txq,
			    const struct ixgbe_txq_ops *txq_ops)
{
	if (txq->sw_ring_v == NULL)
		return -1;

	/* leave the first one for overflow */
	txq->sw_ring_v = txq->sw_ring_v + 1;
	txq->ops = txq_ops;

	return 0;
}

So we need check the SIMD bitwidth firstly to avoid changing the sw_ring* pointer address.


Also, looks like we need to add check on:

int
ixgbe_dev_tx_done_cleanup(void *tx_queue, uint32_t free_cnt)
{
	struct ixgbe_tx_queue *txq = (struct ixgbe_tx_queue *)tx_queue;
	if (txq->offloads == 0 &&
#ifdef RTE_LIBRTE_SECURITY
			!(txq->using_ipsec) &&
#endif
			txq->tx_rs_thresh >= RTE_PMD_IXGBE_TX_MAX_BURST) {
		if (txq->tx_rs_thresh <= RTE_IXGBE_TX_MAX_FREE_BUF_SZ &&
                                                     <------------------- Add the same check
				(rte_eal_process_type() != RTE_PROC_PRIMARY ||
					txq->sw_ring_v != NULL)) {
			return ixgbe_tx_done_cleanup_vec(txq, free_cnt);
		} else {
			return ixgbe_tx_done_cleanup_simple(txq, free_cnt);
		}
	}

> +				>= RTE_MAX_128_SIMD) {
>  			PMD_INIT_LOG(DEBUG, "Vector tx enabled.");
>  			dev->tx_pkt_burst = ixgbe_xmit_pkts_vec;
>  		} else
> @@ -4743,7 +4745,8 @@ ixgbe_set_rx_function(struct rte_eth_dev *dev)
>  	 * conditions to be met and Rx Bulk Allocation should be allowed.
>  	 */
>  	if (ixgbe_rx_vec_dev_conf_condition_check(dev) ||
> -	    !adapter->rx_bulk_alloc_allowed) {
> +	    !adapter->rx_bulk_alloc_allowed ||
> +			rte_get_max_simd_bitwidth() < RTE_MAX_128_SIMD) {
>  		PMD_INIT_LOG(DEBUG, "Port[%d] doesn't meet Vector Rx "
>  				    "preconditions",
>  			     dev->data->port_id);
> --
> 2.17.1