From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id EE14D5A9B for ; Sat, 18 Jul 2015 14:11:19 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 18 Jul 2015 05:11:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,499,1432623600"; d="scan'208";a="608592917" Received: from kmsmsx154.gar.corp.intel.com ([172.21.73.14]) by orsmga003.jf.intel.com with ESMTP; 18 Jul 2015 05:11:17 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by KMSMSX154.gar.corp.intel.com (172.21.73.14) with Microsoft SMTP Server (TLS) id 14.3.224.2; Sat, 18 Jul 2015 20:11:16 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.165]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.146]) with mapi id 14.03.0224.002; Sat, 18 Jul 2015 20:11:11 +0800 From: "Ouyang, Changchun" To: Stephen Hemminger Thread-Topic: [dpdk-dev] [PATCH] virtio: fix the vq size issue Thread-Index: AQHQs9JlQEOaxBUIeEyxKL7zRjwCJJ3fbVEAgAHIeGA= Date: Sat, 18 Jul 2015 12:11:11 +0000 Message-ID: References: <1435736930-26737-1-git-send-email-changchun.ouyang@intel.com> <20150717092756.16d7265e@urahara> In-Reply-To: <20150717092756.16d7265e@urahara> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 Jul 2015 12:11:20 -0000 Hi Stephen, > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Saturday, July 18, 2015 12:28 AM > To: Ouyang, Changchun > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue >=20 > On Wed, 1 Jul 2015 15:48:50 +0800 > Ouyang Changchun wrote: >=20 > > This commit breaks virtio basic packets rx functionality: > > d78deadae4dca240e85054bf2d604a801676becc > > > > The QEMU use 256 as default vring size, also use this default value to > > calculate the virtio avail ring base address and used ring base > > address, and vhost in the backend use the ring base address to do packe= t > IO. > > > > Virtio spec also says the queue size in PCI configuration is > > read-only, so virtio front end can't change it. just need use the > > read-only value to allocate space for vring and calculate the avail > > and used ring base address. Otherwise, the avail and used ring base > address will be different between host and guest, accordingly, packet IO > can't work normally. > > > > Signed-off-by: Changchun Ouyang > > --- > > drivers/net/virtio/virtio_ethdev.c | 14 +++----------- > > 1 file changed, 3 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/net/virtio/virtio_ethdev.c > > b/drivers/net/virtio/virtio_ethdev.c > > index fe5f9a1..d84de13 100644 > > --- a/drivers/net/virtio/virtio_ethdev.c > > +++ b/drivers/net/virtio/virtio_ethdev.c > > @@ -263,8 +263,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev > *dev, > > */ > > vq_size =3D VIRTIO_READ_REG_2(hw, VIRTIO_PCI_QUEUE_NUM); > > PMD_INIT_LOG(DEBUG, "vq_size: %d nb_desc:%d", vq_size, > nb_desc); > > - if (nb_desc =3D=3D 0) > > - nb_desc =3D vq_size; >=20 > command queue is setup with nb_desc =3D 0 nb_desc is not used in the rest of the function, then why we need such an a= ssignment here? Why command queues is setup whit nb_desc =3D 0? Even if it is the case, what the code change break?=20 >=20 > > if (vq_size =3D=3D 0) { > > PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", > __func__); > > return -EINVAL; > > @@ -275,15 +273,9 @@ int virtio_dev_queue_setup(struct rte_eth_dev > *dev, > > return -EINVAL; > > } > > > > - if (nb_desc < vq_size) { > > - if (!rte_is_power_of_2(nb_desc)) { > > - PMD_INIT_LOG(ERR, > > - "nb_desc(%u) size is not powerof 2", > > - nb_desc); > > - return -EINVAL; > > - } > > - vq_size =3D nb_desc; > > - } > > + if (nb_desc !=3D vq_size) > > + PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to > vq size (%d), fall to vq size", > > + nb_desc, vq_size); >=20 > Nack. This breaks onn Google Compute Engine the vring size is 16K. As I mentioned before, the commit d78deadae4dca240e85054bf2d604a801676becc = break the basic functionality of virtio pmd, I don't think keeping it broken is good way for us. We have to revert it firstly to recover its functionality on qemu! Why we need break current functionality to just meet a new thing's requirem= ent? >=20 > An application that wants to work on both QEMU and GCE will want to pass = a > reasonable size and have the negotiation resolve to best value. Do you have already a patch to revert the mistaken and support both qemu an= d gce? If you have, then pls send out it, and let's review. >=20 > For example, vRouter passes 512 as Rx ring size. > On QEMU this gets rounded down to 256 and on GCE only 512 elements are > used. >=20 > This is what the Linux kernel virtio does.