[dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset on VF

Lu, Wenzhuo wenzhuo.lu at intel.com
Wed Jun 8 09:24:22 CEST 2016


Hi Konstantin,

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, June 7, 2016 6:03 PM
> To: Tao, Zhe; dev at dpdk.org
> Cc: Lu, Wenzhuo; Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing;
> Zhang, Helin
> Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF
> 
> 
> 
> > -----Original Message-----
> > From: Tao, Zhe
> > Sent: Tuesday, June 07, 2016 7:53 AM
> > To: dev at dpdk.org
> > Cc: Lu, Wenzhuo; Tao, Zhe; Ananyev, Konstantin; Richardson, Bruce;
> > Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin
> > Subject: [PATCH v4 4/8] ixgbe: implement device reset on VF
> >
> > Implement the device reset function.
> > 1, Add the fake RX/TX functions.
> > 2, The reset function tries to stop RX/TX by replacing
> >    the RX/TX functions with the fake ones and getting the
> >    locks to make sure the regular RX/TX finished.
> > 3, After the RX/TX stopped, reset the VF port, and then
> >    release the locks and restore the RX/TX functions.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
> >
> >  static int
> > +ixgbevf_dev_reset(struct rte_eth_dev *dev) {
> > +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > +	struct ixgbe_adapter *adapter =
> > +		(struct ixgbe_adapter *)dev->data->dev_private;
> > +	int diag = 0;
> > +	uint32_t vteiam;
> > +	uint16_t i;
> > +	struct ixgbe_rx_queue *rxq;
> > +	struct ixgbe_tx_queue *txq;
> > +
> > +	/* Nothing needs to be done if the device is not started. */
> > +	if (!dev->data->dev_started)
> > +		return 0;
> > +
> > +	PMD_DRV_LOG(DEBUG, "Link up/down event detected.");
> > +
> > +	/**
> > +	 * Stop RX/TX by fake functions and locks.
> > +	 * Fake functions are used to make RX/TX lock easier.
> > +	 */
> > +	adapter->rx_backup = dev->rx_pkt_burst;
> > +	adapter->tx_backup = dev->tx_pkt_burst;
> > +	dev->rx_pkt_burst = ixgbevf_recv_pkts_fake;
> > +	dev->tx_pkt_burst = ixgbevf_xmit_pkts_fake;
> 
> If you have locking over each queue underneath, why do you still need fake
> functions?
The fake functions are used to help saving the time of waiting for the locks.
As you see, we want to lock every queue. If we don't use fake functions we have to wait for every queue.
But if the real functions are replaced by fake functions, ideally when we're waiting for the release of the first queue's lock,
the other queues will run into the fake functions. So we need not wait for them and get the locks directly.

> 
> > +
> > +	if (dev->data->rx_queues)
> > +		for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > +			rxq = dev->data->rx_queues[i];
> > +			rte_spinlock_lock(&rxq->rx_lock);
> > +		}
> > +
> > +	if (dev->data->tx_queues)
> > +		for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > +			txq = dev->data->tx_queues[i];
> > +			rte_spinlock_lock(&txq->tx_lock);
> > +		}
> 
> Probably worth to create a separate function for the lines above:
> lock_all_queues(), unlock_all_queues.
> But as I sadi in previous mail - I think that code better be in rte_ethdev.
We're discussing it in the previous thread :)

> >
> > @@ -5235,11 +5243,21 @@ ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev)
> >  			rxdctl = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(i));
> >  		} while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE));
> >  		if (!poll_ms)
> > +#ifndef RTE_NEXT_ABI
> > +			PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d",
> i); #else
> > +		{
> >  			PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d",
> i);
> > +			if (dev->data->dev_conf.rxmode.lock_mode)
> > +				return -1;
> > +		}
> > +#endif
> 
> 
> Why the code has to be different here?
As you see this rxtx_start may have chance to fail. I want to expose this failure, so the reset function can try again.

> Thanks
> Konstantin



More information about the dev mailing list