From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 805A91B849; Thu, 1 Feb 2018 09:27:46 +0100 (CET) Received: from lfbn-lil-1-110-231.w90-45.abo.wanadoo.fr ([90.45.197.231] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1ehADl-0008Cz-1F; Thu, 01 Feb 2018 09:27:50 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Thu, 01 Feb 2018 09:27:35 +0100 Date: Thu, 1 Feb 2018 09:27:35 +0100 From: Olivier Matz To: "Yao, Lei A" Cc: "dev@dpdk.org" , "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" , Thomas Monjalon , "stable@dpdk.org" Message-ID: <20180201082735.w4nyppsg4vpgenei@platinum> References: <20170831134015.1383-1-olivier.matz@6wind.com> <20170907121347.16208-1-olivier.matz@6wind.com> <20170907121347.16208-7-olivier.matz@6wind.com> <2DBBFF226F7CF64BAFCA79B681719D953A354152@shsmsx102.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2DBBFF226F7CF64BAFCA79B681719D953A354152@shsmsx102.ccr.corp.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2 06/10] net/virtio: fix queue setup consistency X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Feb 2018 08:27:46 -0000 Hi Lei, It's on my todo list, I'll check this as soon as possible. Olivier On Thu, Feb 01, 2018 at 03:14:15AM +0000, Yao, Lei A wrote: > Hi, Olivier > > This is Lei from DPDK validation team in Intel. During our DPDK 18.02-rc1 test, > I find the following patch will cause one serious issue with virtio vector path: > the traffic can't resume after stop/start the virtio device. > > The step like following: > 1. Launch vhost-user port using testpmd at Host > 2. Launch VM with virtio device, mergeable is off > 3. Bind the virtio device to pmd driver, launch testpmd, let the tx/rx use vector path > virtio_xmit_pkts_simple > virtio_recv_pkts_vec > 4. Send traffic to virtio device from vhost side, then stop the virtio device > 5. Start the virtio device again > After step 5, the traffic can't resume. > > Could you help check this and give a fix? This issue will impact the virtio pmd user > experience heavily. By the way, this patch is already included into V17.11. Looks like > we need give a patch to this LTS version. Thanks a lot! > > BRs > Lei > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz > > Sent: Thursday, September 7, 2017 8:14 PM > > To: dev@dpdk.org; yliu@fridaylinux.org; maxime.coquelin@redhat.com > > Cc: stephen@networkplumber.org; stable@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 06/10] net/virtio: fix queue setup consistency > > > > In rx/tx queue setup functions, some code is executed only if > > use_simple_rxtx == 1. The value of this variable can change depending on > > the offload flags or sse support. If Rx queue setup is called before Tx > > queue setup, it can result in an invalid configuration: > > > > - dev_configure is called: use_simple_rxtx is initialized to 0 > > - rx queue setup is called: queues are initialized without simple path > > support > > - tx queue setup is called: use_simple_rxtx switch to 1, and simple > > Rx/Tx handlers are selected > > > > Fix this by postponing a part of Rx/Tx queue initialization in > > dev_start(), as it was the case in the initial implementation. > > > > Fixes: 48cec290a3d2 ("net/virtio: move queue configure code to proper > > place") > > Cc: stable@dpdk.org > > > > Signed-off-by: Olivier Matz > > --- > > drivers/net/virtio/virtio_ethdev.c | 13 +++++++++++++ > > drivers/net/virtio/virtio_ethdev.h | 6 ++++++ > > drivers/net/virtio/virtio_rxtx.c | 40 ++++++++++++++++++++++++++++++- > > ------- > > 3 files changed, 51 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/virtio/virtio_ethdev.c > > b/drivers/net/virtio/virtio_ethdev.c > > index 8eee3ff80..c7888f103 100644 > > --- a/drivers/net/virtio/virtio_ethdev.c > > +++ b/drivers/net/virtio/virtio_ethdev.c > > @@ -1737,6 +1737,19 @@ virtio_dev_start(struct rte_eth_dev *dev) > > struct virtnet_rx *rxvq; > > struct virtnet_tx *txvq __rte_unused; > > struct virtio_hw *hw = dev->data->dev_private; > > + int ret; > > + > > + /* Finish the initialization of the queues */ > > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > > + ret = virtio_dev_rx_queue_setup_finish(dev, i); > > + if (ret < 0) > > + return ret; > > + } > > + for (i = 0; i < dev->data->nb_tx_queues; i++) { > > + ret = virtio_dev_tx_queue_setup_finish(dev, i); > > + if (ret < 0) > > + return ret; > > + } > > > > /* check if lsc interrupt feature is enabled */ > > if (dev->data->dev_conf.intr_conf.lsc) { > > diff --git a/drivers/net/virtio/virtio_ethdev.h > > b/drivers/net/virtio/virtio_ethdev.h > > index c3413c6d9..2039bc547 100644 > > --- a/drivers/net/virtio/virtio_ethdev.h > > +++ b/drivers/net/virtio/virtio_ethdev.h > > @@ -92,10 +92,16 @@ int virtio_dev_rx_queue_setup(struct rte_eth_dev > > *dev, uint16_t rx_queue_id, > > const struct rte_eth_rxconf *rx_conf, > > struct rte_mempool *mb_pool); > > > > +int virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, > > + uint16_t rx_queue_id); > > + > > int virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t > > tx_queue_id, > > uint16_t nb_tx_desc, unsigned int socket_id, > > const struct rte_eth_txconf *tx_conf); > > > > +int virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev, > > + uint16_t tx_queue_id); > > + > > uint16_t virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, > > uint16_t nb_pkts); > > > > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c > > index e30377c51..a32e3229f 100644 > > --- a/drivers/net/virtio/virtio_rxtx.c > > +++ b/drivers/net/virtio/virtio_rxtx.c > > @@ -421,9 +421,6 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev, > > struct virtio_hw *hw = dev->data->dev_private; > > struct virtqueue *vq = hw->vqs[vtpci_queue_idx]; > > struct virtnet_rx *rxvq; > > - int error, nbufs; > > - struct rte_mbuf *m; > > - uint16_t desc_idx; > > > > PMD_INIT_FUNC_TRACE(); > > > > @@ -440,10 +437,24 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev > > *dev, > > } > > dev->data->rx_queues[queue_idx] = rxvq; > > > > + return 0; > > +} > > + > > +int > > +virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t > > queue_idx) > > +{ > > + uint16_t vtpci_queue_idx = 2 * queue_idx + > > VTNET_SQ_RQ_QUEUE_IDX; > > + struct virtio_hw *hw = dev->data->dev_private; > > + struct virtqueue *vq = hw->vqs[vtpci_queue_idx]; > > + struct virtnet_rx *rxvq = &vq->rxq; > > + struct rte_mbuf *m; > > + uint16_t desc_idx; > > + int error, nbufs; > > + > > + PMD_INIT_FUNC_TRACE(); > > > > /* Allocate blank mbufs for the each rx descriptor */ > > nbufs = 0; > > - error = ENOSPC; > > > > if (hw->use_simple_rxtx) { > > for (desc_idx = 0; desc_idx < vq->vq_nentries; > > @@ -534,7 +545,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > struct virtqueue *vq = hw->vqs[vtpci_queue_idx]; > > struct virtnet_tx *txvq; > > uint16_t tx_free_thresh; > > - uint16_t desc_idx; > > > > PMD_INIT_FUNC_TRACE(); > > > > @@ -563,9 +573,24 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev > > *dev, > > > > vq->vq_free_thresh = tx_free_thresh; > > > > - if (hw->use_simple_rxtx) { > > - uint16_t mid_idx = vq->vq_nentries >> 1; > > + dev->data->tx_queues[queue_idx] = txvq; > > + return 0; > > +} > > + > > +int > > +virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev, > > + uint16_t queue_idx) > > +{ > > + uint8_t vtpci_queue_idx = 2 * queue_idx + > > VTNET_SQ_TQ_QUEUE_IDX; > > + struct virtio_hw *hw = dev->data->dev_private; > > + struct virtqueue *vq = hw->vqs[vtpci_queue_idx]; > > + uint16_t mid_idx = vq->vq_nentries >> 1; > > + struct virtnet_tx *txvq = &vq->txq; > > + uint16_t desc_idx; > > > > + PMD_INIT_FUNC_TRACE(); > > + > > + if (hw->use_simple_rxtx) { > > for (desc_idx = 0; desc_idx < mid_idx; desc_idx++) { > > vq->vq_ring.avail->ring[desc_idx] = > > desc_idx + mid_idx; > > @@ -587,7 +612,6 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > > > VIRTQUEUE_DUMP(vq); > > > > - dev->data->tx_queues[queue_idx] = txvq; > > return 0; > > } > > > > -- > > 2.11.0 >