[dpdk-dev] [PATCH v2 0/3] virtio: size bug fixes

[dpdk-dev] [PATCH v2 0/3] virtio: size bug fixes

[Stephen Hemminger]

Fix for a fix for a fix...
This set includes Changchun's patch for vq_size but adds
follow on fix to make it work with GCE and other environments.

Ouyang Changchun (1):
  virtio: fix the vq size issue

Stephen Hemminger (2):
  virtio: allow nb_desc < vq_size
  virtio: small cleanups

 drivers/net/virtio/virtio_ethdev.c | 24 +++++++-----------------
 drivers/net/virtio/virtio_ethdev.h |  2 +-
 drivers/net/virtio/virtio_rxtx.c   |  2 +-
 3 files changed, 9 insertions(+), 19 deletions(-)

-- 
2.1.4

[dpdk-dev] [PATCH v2 1/3] virtio: fix the vq size issue

[Stephen Hemminger]

From: Ouyang Changchun <changchun.ouyang@intel.com>

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
packet 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 <changchun.ouyang@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 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 9ca9bb2..94b7a81 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -276,8 +276,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 	 */
 	vq_size = 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 == 0)
-		nb_desc = vq_size;
 	if (vq_size == 0) {
 		PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", __func__);
 		return -EINVAL;
@@ -288,15 +286,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 = nb_desc;
-	}
+	if (nb_desc != 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);
 
 	if (queue_type == VTNET_RQ) {
 		snprintf(vq_name, sizeof(vq_name), "port%d_rvq%d",
-- 
2.1.4

[dpdk-dev] [PATCH v2 2/3] virtio: allow nb_desc < vq_size

[Stephen Hemminger]

When running on GCE vq_size is 16K but number of Rx descriptors
desired maybe less than that. Handle the situtaiton by initializing
full ring but only filling the smaller number.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/virtio/virtio_ethdev.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 94b7a81..d460d89 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -286,10 +286,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
-	if (nb_desc != 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);
-
 	if (queue_type == VTNET_RQ) {
 		snprintf(vq_name, sizeof(vq_name), "port%d_rvq%d",
 			dev->data->port_id, queue_idx);
@@ -317,7 +313,10 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 	vq->queue_id = queue_idx;
 	vq->vq_queue_index = vtpci_queue_idx;
 	vq->vq_nentries = vq_size;
-	vq->vq_free_cnt = vq_size;
+
+	if (nb_desc == 0 || nb_desc > vq_size)
+		nb_desc = vq_size;
+	vq->vq_free_cnt = nb_desc;
 
 	/*
 	 * Reserve a memzone for vring elements
-- 
2.1.4

[dpdk-dev] [PATCH v2 3/3] virtio: small cleanups

[Stephen Hemminger]

Some minor cleanups.
  * pass constant to virtio_dev_queue_setup
  * fix message on rx_queue_setup
  * get rid of extra double spaces

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/virtio/virtio_ethdev.c | 7 +++----
 drivers/net/virtio/virtio_ethdev.h | 2 +-
 drivers/net/virtio/virtio_rxtx.c   | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d460d89..465d3cd 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -254,7 +254,7 @@ virtio_dev_queue_release(struct virtqueue *vq) {
 int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 			int queue_type,
 			uint16_t queue_idx,
-			uint16_t  vtpci_queue_idx,
+			uint16_t vtpci_queue_idx,
 			uint16_t nb_desc,
 			unsigned int socket_id,
 			struct virtqueue **pvq)
@@ -264,7 +264,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 	uint16_t vq_size;
 	int size;
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct virtqueue  *vq = NULL;
+	struct virtqueue *vq = NULL;
 
 	/* Write the virtqueue index to the Queue Select Field */
 	VIRTIO_WRITE_REG_2(hw, VIRTIO_PCI_QUEUE_SEL, vtpci_queue_idx);
@@ -413,13 +413,12 @@ virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx,
 		uint32_t socket_id)
 {
 	struct virtqueue *vq;
-	uint16_t nb_desc = 0;
 	int ret;
 	struct virtio_hw *hw = dev->data->dev_private;
 
 	PMD_INIT_FUNC_TRACE();
 	ret = virtio_dev_queue_setup(dev, VTNET_CQ, VTNET_SQ_CQ_QUEUE_IDX,
-			vtpci_queue_idx, nb_desc, socket_id, &vq);
+			vtpci_queue_idx, 0, socket_id, &vq);
 	if (ret < 0) {
 		PMD_INIT_LOG(ERR, "control vq initialization failed");
 		return ret;
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 3858b00..9026d42 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -79,7 +79,7 @@ void virtio_dev_rxtx_start(struct rte_eth_dev *dev);
 int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 			int queue_type,
 			uint16_t queue_idx,
-			uint16_t  vtpci_queue_idx,
+			uint16_t vtpci_queue_idx,
 			uint16_t nb_desc,
 			unsigned int socket_id,
 			struct virtqueue **pvq);
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 5388caa..c5b53bb 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -390,7 +390,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	ret = virtio_dev_queue_setup(dev, VTNET_RQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
 	if (ret < 0) {
-		PMD_INIT_LOG(ERR, "tvq initialization failed");
+		PMD_INIT_LOG(ERR, "rvq initialization failed");
 		return ret;
 	}
 
-- 
2.1.4

Re: [dpdk-dev] [PATCH v2 0/3] virtio: size bug fixes

[Thomas Monjalon]

2015-07-20 10:25, Stephen Hemminger:
> Fix for a fix for a fix...
> This set includes Changchun's patch for vq_size but adds
> follow on fix to make it work with GCE and other environments.
> 
> Ouyang Changchun (1):
>   virtio: fix the vq size issue
> 
> Stephen Hemminger (2):
>   virtio: allow nb_desc < vq_size
>   virtio: small cleanups

Patch 2 partially reverts what patch 1 introduced.
Merging them will make things clearer.

Re: [dpdk-dev] [PATCH v2 0/3] virtio: size bug fixes

[Stephen Hemminger]

On Mon, 20 Jul 2015 19:43:20 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2015-07-20 10:25, Stephen Hemminger:
> > Fix for a fix for a fix...
> > This set includes Changchun's patch for vq_size but adds
> > follow on fix to make it work with GCE and other environments.
> > 
> > Ouyang Changchun (1):
> >   virtio: fix the vq size issue
> > 
> > Stephen Hemminger (2):
> >   virtio: allow nb_desc < vq_size
> >   virtio: small cleanups
> 
> Patch 2 partially reverts what patch 1 introduced.
> Merging them will make things clearer.

Ok, but it was easier to review that way.