> > From: Power, Ciara <ciara.po...@intel.com>
> > Sent: Wednesday, September 30, 2020 21:04
> > To: dev@dpdk.org
> > Cc: Power, Ciara <ciara.po...@intel.com>; Zhao1, Wei <wei.zh...@intel.com>; 
> > Guo, Jia
> > <jia....@intel.com>; Wang, Haiyue <haiyue.w...@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.zh...@intel.com>
> > Cc: Jeff Guo <jia....@intel.com>
> >
> > Signed-off-by: Ciara Power <ciara.po...@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);

Could you probably explain a bit more why it is needed?

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

Reply via email to