[dpdk-dev] [PATCH] maintainers: claim to be reviewer of virtio/vhost component

[dpdk-dev] [PATCH] maintainers: claim to be reviewer of virtio/vhost component

[Yuanhan Liu]

Firstly, Chuangchun's email address's been invalid for a while.

Secondly, I'd like to take the responsibility to review patches
of virtio/vhost component.

Cc: Huawei Xie <huawei.xie@intel.com>
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c8be5d2..b05724a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -262,7 +262,7 @@ F: doc/guides/nics/mlx5.rst
 
 RedHat virtio
 M: Huawei Xie <huawei.xie@intel.com>
-M: Changchun Ouyang <changchun.ouyang@intel.com>
+M: Yuanhan Liu <yuanhan.liu@linux.intel.com>
 F: drivers/net/virtio/
 F: doc/guides/nics/virtio.rst
 F: lib/librte_vhost/
-- 
1.9.0

[dpdk-dev] [PATCH] vhost: reset device properly

[Yuanhan Liu]

Currently, we reset all fields of a device to zero when reset
happens, which is wrong, since for some fields like device_fh,
ifname, and virt_qp_nb, they should be same and be kept after
reset until the device is removed. And this is what's the new
helper function reset_device() for.

And use rte_zmalloc() instead of rte_malloc, so that we could
avoid init_device(), which basically dose zero reset only so far.
Hence, init_device() is dropped in this patch.

This patch also removes a hack of using the offset a specific
field (which is virtqueue now) inside of `virtio_net' structure
to do reset, which could be broken easily if someone changed the
field order without caution.

Cc: Tetsuya Mukawa <mukawa@igel.co.jp>
Cc: Xie Huawei <huawei.xie@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

---

This patch is based on:

http://dpdk.org/dev/patchwork/patch/8818/
---
 lib/librte_vhost/virtio-net.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index 39a6a5e..cc917da 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -204,6 +204,7 @@ cleanup_device(struct virtio_net *dev)
 		munmap((void *)(uintptr_t)dev->mem->mapped_address,
 			(size_t)dev->mem->mapped_size);
 		free(dev->mem);
+		dev->mem = NULL;
 	}
 
 	for (i = 0; i < dev->virt_qp_nb; i++) {
@@ -306,20 +307,18 @@ alloc_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx)
 }
 
 /*
- *  Initialise all variables in device structure.
+ * Reset some variables in device structure, while keeping few
+ * others untouched, such as device_fh, ifname, virt_qp_nb: they
+ * should be same unless the device is removed.
  */
 static void
-init_device(struct virtio_net *dev)
+reset_device(struct virtio_net *dev)
 {
-	int vq_offset;
 	uint32_t i;
 
-	/*
-	 * Virtqueues have already been malloced so
-	 * we don't want to set them to NULL.
-	 */
-	vq_offset = offsetof(struct virtio_net, virtqueue);
-	memset(dev, 0, vq_offset);
+	dev->features = 0;
+	dev->protocol_features = 0;
+	dev->flags = 0;
 
 	for (i = 0; i < dev->virt_qp_nb; i++)
 		init_vring_queue_pair(dev, i);
@@ -336,7 +335,7 @@ new_device(struct vhost_device_ctx ctx)
 	struct virtio_net_config_ll *new_ll_dev;
 
 	/* Setup device and virtqueues. */
-	new_ll_dev = rte_malloc(NULL, sizeof(struct virtio_net_config_ll), 0);
+	new_ll_dev = rte_zmalloc(NULL, sizeof(struct virtio_net_config_ll), 0);
 	if (new_ll_dev == NULL) {
 		RTE_LOG(ERR, VHOST_CONFIG,
 			"(%"PRIu64") Failed to allocate memory for dev.\n",
@@ -344,9 +343,6 @@ new_device(struct vhost_device_ctx ctx)
 		return -1;
 	}
 
-	/* Initialise device and virtqueues. */
-	init_device(&new_ll_dev->dev);
-
 	new_ll_dev->next = NULL;
 
 	/* Add entry to device configuration linked list. */
@@ -430,7 +426,6 @@ static int
 reset_owner(struct vhost_device_ctx ctx)
 {
 	struct virtio_net *dev;
-	uint64_t device_fh;
 
 	dev = get_device(ctx);
 	if (dev == NULL)
@@ -439,10 +434,8 @@ reset_owner(struct vhost_device_ctx ctx)
 	if (dev->flags & VIRTIO_DEV_RUNNING)
 		notify_ops->destroy_device(dev);
 
-	device_fh = dev->device_fh;
 	cleanup_device(dev);
-	init_device(dev);
-	dev->device_fh = device_fh;
+	reset_device(dev);
 	return 0;
 }
 
-- 
1.9.0

Re: [dpdk-dev] [PATCH] vhost: reset device properly

[Rich Lane]

On Wed, Nov 11, 2015 at 8:10 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com>
wrote:

> Currently, we reset all fields of a device to zero when reset
> happens, which is wrong, since for some fields like device_fh,
> ifname, and virt_qp_nb, they should be same and be kept after
> reset until the device is removed. And this is what's the new
> helper function reset_device() for.
>
> And use rte_zmalloc() instead of rte_malloc, so that we could
> avoid init_device(), which basically dose zero reset only so far.
> Hence, init_device() is dropped in this patch.
>
> This patch also removes a hack of using the offset a specific
> field (which is virtqueue now) inside of `virtio_net' structure
> to do reset, which could be broken easily if someone changed the
> field order without caution.
>
> Cc: Tetsuya Mukawa <mukawa@igel.co.jp>
> Cc: Xie Huawei <huawei.xie@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>

I had a patch that just saved the ifname but this is much better.

Acked-by: Rich Lane <rlane@bigswitch.com>

Re: [dpdk-dev] [PATCH] vhost: reset device properly

[Thomas Monjalon]

> > Currently, we reset all fields of a device to zero when reset
> > happens, which is wrong, since for some fields like device_fh,
> > ifname, and virt_qp_nb, they should be same and be kept after
> > reset until the device is removed. And this is what's the new
> > helper function reset_device() for.
> >
> > And use rte_zmalloc() instead of rte_malloc, so that we could
> > avoid init_device(), which basically dose zero reset only so far.
> > Hence, init_device() is dropped in this patch.
> >
> > This patch also removes a hack of using the offset a specific
> > field (which is virtqueue now) inside of `virtio_net' structure
> > to do reset, which could be broken easily if someone changed the
> > field order without caution.
> >
> > Cc: Tetsuya Mukawa <mukawa@igel.co.jp>
> > Cc: Xie Huawei <huawei.xie@intel.com>
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> 
> I had a patch that just saved the ifname but this is much better.
> 
> Acked-by: Rich Lane <rlane@bigswitch.com>

Applied, thanks

Re: [dpdk-dev] [PATCH] maintainers: claim to be reviewer of virtio/vhost component

[Thomas Monjalon]

2015-11-12 12:10, Yuanhan Liu:
> Firstly, Chuangchun's email address's been invalid for a while.
> 
> Secondly, I'd like to take the responsibility to review patches
> of virtio/vhost component.
[...]
>  RedHat virtio
>  M: Huawei Xie <huawei.xie@intel.com>
> -M: Changchun Ouyang <changchun.ouyang@intel.com>
> +M: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>  F: drivers/net/virtio/
>  F: doc/guides/nics/virtio.rst
>  F: lib/librte_vhost/

Again, thanks Yuanhan for the excellent contributions
and welcome new maintainer!

Changchun, you are still welcome with a new email address
if you have some time.