From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 611041B796; Wed, 7 Feb 2018 09:31:12 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Feb 2018 00:31:11 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,471,1511856000"; d="scan'208";a="15806654" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga007.jf.intel.com with ESMTP; 07 Feb 2018 00:31:10 -0800 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 7 Feb 2018 00:31:10 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 7 Feb 2018 00:31:09 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.124]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.130]) with mapi id 14.03.0319.002; Wed, 7 Feb 2018 16:31:08 +0800 From: "Xu, Qian Q" To: Olivier Matz , "Yao, Lei A" CC: "dev@dpdk.org" , "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" , Thomas Monjalon , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 06/10] net/virtio: fix queue setup consistency Thread-Index: AQHTJ9MMPpsqO4CB002zuwkUTclI86OPP7iAgABXi4CACe/cgA== Date: Wed, 7 Feb 2018 08:31:07 +0000 Message-ID: <82F45D86ADE5454A95A89742C8D1410E3B902F26@shsmsx102.ccr.corp.intel.com> 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> <20180201082735.w4nyppsg4vpgenei@platinum> In-Reply-To: <20180201082735.w4nyppsg4vpgenei@platinum> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTNkNGQxMDEtZjQxMC00NjlmLTkzYjEtNTFmYzRmYzJjZDUyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImFBMHVWRlFoUmFYZHNrdjN3SERXSmRVaGlmcGpqR0d0MGMxUDJST0kwUzA9In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Wed, 07 Feb 2018 08:31:14 -0000 Any update, Olivier?=20 We are near to release, and the bug-fix is important for the virtio vector = path usage. Thanks.=20 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz > Sent: Thursday, February 1, 2018 4:28 PM > To: Yao, Lei A > Cc: dev@dpdk.org; yliu@fridaylinux.org; maxime.coquelin@redhat.com; > Thomas Monjalon ; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 06/10] net/virtio: fix queue setup cons= istency >=20 > Hi Lei, >=20 > It's on my todo list, I'll check this as soon as possible. >=20 > Olivier >=20 >=20 > 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 versi= on. > 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 =3D=3D 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 configurati= on: > > > > > > - dev_configure is called: use_simple_rxtx is initialized to 0 > > > - rx queue setup is called: queues are initialized without simple pat= h > > > 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 =3D dev->data->dev_private; > > > + int ret; > > > + > > > + /* Finish the initialization of the queues */ > > > + for (i =3D 0; i < dev->data->nb_rx_queues; i++) { > > > + ret =3D virtio_dev_rx_queue_setup_finish(dev, i); > > > + if (ret < 0) > > > + return ret; > > > + } > > > + for (i =3D 0; i < dev->data->nb_tx_queues; i++) { > > > + ret =3D 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 =3D dev->data->dev_private; > > > struct virtqueue *vq =3D 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] =3D rxvq; > > > > > > + return 0; > > > +} > > > + > > > +int > > > +virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t > > > queue_idx) > > > +{ > > > + uint16_t vtpci_queue_idx =3D 2 * queue_idx + > > > VTNET_SQ_RQ_QUEUE_IDX; > > > + struct virtio_hw *hw =3D dev->data->dev_private; > > > + struct virtqueue *vq =3D hw->vqs[vtpci_queue_idx]; > > > + struct virtnet_rx *rxvq =3D &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 =3D 0; > > > - error =3D ENOSPC; > > > > > > if (hw->use_simple_rxtx) { > > > for (desc_idx =3D 0; desc_idx < vq->vq_nentries; @@ -534,7 > +545,6 > > > @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev, > > > struct virtqueue *vq =3D 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 =3D tx_free_thresh; > > > > > > - if (hw->use_simple_rxtx) { > > > - uint16_t mid_idx =3D vq->vq_nentries >> 1; > > > + dev->data->tx_queues[queue_idx] =3D txvq; > > > + return 0; > > > +} > > > + > > > +int > > > +virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev, > > > + uint16_t queue_idx) > > > +{ > > > + uint8_t vtpci_queue_idx =3D 2 * queue_idx + > > > VTNET_SQ_TQ_QUEUE_IDX; > > > + struct virtio_hw *hw =3D dev->data->dev_private; > > > + struct virtqueue *vq =3D hw->vqs[vtpci_queue_idx]; > > > + uint16_t mid_idx =3D vq->vq_nentries >> 1; > > > + struct virtnet_tx *txvq =3D &vq->txq; > > > + uint16_t desc_idx; > > > > > > + PMD_INIT_FUNC_TRACE(); > > > + > > > + if (hw->use_simple_rxtx) { > > > for (desc_idx =3D 0; desc_idx < mid_idx; desc_idx++) { > > > vq->vq_ring.avail->ring[desc_idx] =3D > > > 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] =3D txvq; > > > return 0; > > > } > > > > > > -- > > > 2.11.0 > >