[dpdk-dev] [PATCH 03/19] net/ice: support device and queue ops

Lu, Wenzhuo wenzhuo.lu at intel.com
Thu Dec 6 03:53:57 CET 2018


Hi Rami,


> -----Original Message-----
> From: Rami Rosen [mailto:roszenrami at gmail.com]
> Sent: Monday, December 3, 2018 11:24 PM
> To: Lu, Wenzhuo <wenzhuo.lu at intel.com>
> Cc: dev at dpdk.org; Yang, Qiming <qiming.yang at intel.com>; Li, Xiaoyun
> <xiaoyun.li at intel.com>; Wu, Jingjing <jingjing.wu at intel.com>
> Subject: Re: [dpdk-dev] [PATCH 03/19] net/ice: support device and queue
> ops
> 
> Hi, Wenzhuo,
> 
> > +static int
> > +ice_dev_start(struct rte_eth_dev *dev) {
> > +       struct rte_eth_dev_data *data = dev->data;
> > +       struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > +       struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > +       uint16_t nb_rxq = 0;
> > +       uint16_t nb_txq, i;
> > +       int ret;
> > +
> > +       if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> > +               return -E_RTE_SECONDARY;
> > +
> [Rami Rosen] Suppose start of a TX queue failes in the loop below. You go to
> **tx_err** label, where you stop all **RX** queues (which actually were not
> started at all, since they are started only later in this method; and then you
> return -EIO and the ice_dev_start() method is terminated, without actually
> stopping any TX queues which were already started;
> So maybe it is better to call   ice_tx_queue_stop() in tx_err and
> ice_rx_queue_stop() in rx_err.
> Apart from it, there is a typo:  "Tx queues' contex" should be =>Tx queues'
> context"
Thanks for the comments. The logic is not good. Will make it better in the next version.

> 
> > +       /* program Tx queues' contex in hardware */
> > +       for (nb_txq = 0; nb_txq < data->nb_tx_queues; nb_txq++) {
> > +               ret = ice_tx_queue_start(dev, nb_txq);
> > +               if (ret) {
> > +                       PMD_DRV_LOG(ERR, "fail to start Tx queue %u", nb_txq);
> > +                       goto tx_err;
> > +               }
> > +       }
> > +
> 
> > +       /* program Rx queues' context in hardware*/
> > +       for (nb_rxq = 0; nb_rxq < data->nb_rx_queues; nb_rxq++) {
> > +               ret = ice_rx_queue_start(dev, nb_rxq);
> > +               if (ret) {
> > +                       PMD_DRV_LOG(ERR, "fail to start Rx queue %u", nb_rxq);
> > +                       goto rx_err;
> > +               }
> > +       }
> ,,,
> 
> > +       /* stop the started queues if failed to start all queues */
> > +rx_err:
> > +       for (i = 0; i < nb_txq; i++)
> > +               ice_tx_queue_stop(dev, i);
> > +tx_err:
> > +       for (i = 0; i < nb_rxq; i++)
> > +               ice_rx_queue_stop(dev, i);
> > +
> > +       return -EIO;
> > +}
> > +
> 
> Regards,
> Rami Rosen


More information about the dev mailing list