Hi Rami,
> -----Original Message----- > From: Rami Rosen [mailto:roszenr...@gmail.com] > Sent: Monday, December 3, 2018 11:24 PM > To: Lu, Wenzhuo <wenzhuo...@intel.com> > Cc: dev@dpdk.org; Yang, Qiming <qiming.y...@intel.com>; Li, Xiaoyun > <xiaoyun...@intel.com>; Wu, Jingjing <jingjing...@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
