From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id BA0571B835; Thu, 1 Feb 2018 04:14:17 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Jan 2018 19:14:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,443,1511856000"; d="scan'208";a="26377936" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga004.fm.intel.com with ESMTP; 31 Jan 2018 19:14:16 -0800 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 31 Jan 2018 19:14:16 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 31 Jan 2018 19:14:16 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.124]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.116]) with mapi id 14.03.0319.002; Thu, 1 Feb 2018 11:14:15 +0800 From: "Yao, Lei A" To: Olivier Matz , "dev@dpdk.org" , "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" , Thomas Monjalon CC: "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 06/10] net/virtio: fix queue setup consistency Thread-Index: AQHTJ9MCGbgx8MzJlEGL/cq+7ZGYFKOPwDXA Date: Thu, 1 Feb 2018 03:14:15 +0000 Message-ID: <2DBBFF226F7CF64BAFCA79B681719D953A354152@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> In-Reply-To: <20170907121347.16208-7-olivier.matz@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOGRhODQ0ZTktMDRjMy00YzJlLWI2ZDYtZDE2ZjVlYTUzYzRjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6InhTTWlZZEZrZW1IWGJjQTZvVFh0dkRKQ3NoenpHeStlN21oc0xxZFc1dEU9In0= 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: Thu, 01 Feb 2018 03:14:19 -0000 Hi, Olivier This is Lei from DPDK validation team in Intel. During our DPDK 18.02-rc1 t= est, I find the following patch will cause one serious issue with virtio vector = path:=20 the traffic can't resume after stop/start the virtio device.=20 The step like following: 1. Launch vhost-user port using testpmd at Host=20 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=20 virtio_recv_pkts_vec =20 4. Send traffic to virtio device from vhost side, then stop the virtio devi= ce 5. Start the virtio device again After step 5, the traffic can't resume.=20 Could you help check this and give a fix? This issue will impact the virtio= pmd user=20 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 consiste= ncy >=20 > 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 configuration: >=20 > - 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 >=20 > Fix this by postponing a part of Rx/Tx queue initialization in > dev_start(), as it was the case in the initial implementation. >=20 > Fixes: 48cec290a3d2 ("net/virtio: move queue configure code to proper > place") > Cc: stable@dpdk.org >=20 > 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(-) >=20 > 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; > + } >=20 > /* 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); >=20 > +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); >=20 > +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); >=20 > 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; >=20 > PMD_INIT_FUNC_TRACE(); >=20 > @@ -440,10 +437,24 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev > *dev, > } > dev->data->rx_queues[queue_idx] =3D rxvq; >=20 > + 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(); >=20 > /* Allocate blank mbufs for the each rx descriptor */ > nbufs =3D 0; > - error =3D ENOSPC; >=20 > 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; >=20 > PMD_INIT_FUNC_TRACE(); >=20 > @@ -563,9 +573,24 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev > *dev, >=20 > vq->vq_free_thresh =3D tx_free_thresh; >=20 > - 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; >=20 > + 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, >=20 > VIRTQUEUE_DUMP(vq); >=20 > - dev->data->tx_queues[queue_idx] =3D txvq; > return 0; > } >=20 > -- > 2.11.0