[RFC v2 0/4] Support reconnection in vduse

[RFC v2 0/4] Support reconnection in vduse

[Cindy Lu]

These patches add the reconnect support in vduse, The steps
is map the pages from kernel to userspace, userspace    
app will sync the reconnection status and vq_info in the pages
Also, add the new IOCTL VDUSE_GET_RECONNECT_INFO
userspace app will use this information to mmap the memory

Will send the patch for DPDK later

Tested in vduse + dpdk test-pmd
 
Signed-off-by: Cindy Lu <lulu@redhat.com>

Cindy Lu (4):
  vduse: Add function to get/free the pages for reconnection
  vduse: Add file operation for mmap
  vduse: update the vq_info in ioctl
  vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

 drivers/vdpa/vdpa_user/vduse_dev.c | 177 +++++++++++++++++++++++++++++
 include/uapi/linux/vduse.h         |  21 ++++
 2 files changed, 198 insertions(+)

-- 
2.34.3

[RFC v2 1/4] vduse: Add function to get/free the pages for reconnection

[Cindy Lu]

Add the function vduse_alloc_reconnnect_info_mem
and vduse_alloc_reconnnect_info_mem
In this 2 function, vduse will get/free (vq_num + 1)*page  
Page 0 will be used to save the reconnection information, The
Userspace App will maintain this. Page 1 ~ vq_num + 1 will save
the reconnection information for vqs.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 86 ++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 26b7e29cb900..4c256fa31fc4 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -30,6 +30,10 @@
 #include <uapi/linux/virtio_blk.h>
 #include <linux/mod_devicetable.h>
 
+#ifdef CONFIG_X86
+#include <asm/set_memory.h>
+#endif
+
 #include "iova_domain.h"
 
 #define DRV_AUTHOR   "Yongji Xie <xieyongji@bytedance.com>"
@@ -41,6 +45,23 @@
 #define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
 #define VDUSE_MSG_DEFAULT_TIMEOUT 30
 
+/* struct vdpa_reconnect_info save the page information for reconnection
+ * kernel will init these information while alloc the pages
+ * and use these information to free the pages
+ */
+struct vdpa_reconnect_info {
+	/* Offset (within vm_file) in PAGE_SIZE,
+	 * this just for check, not using
+	 */
+	u32 index;
+	/* physical address for this page*/
+	phys_addr_t addr;
+	/* virtual address for this page*/
+	unsigned long vaddr;
+	/* memory size, here always page_size*/
+	phys_addr_t size;
+};
+
 struct vduse_virtqueue {
 	u16 index;
 	u16 num_max;
@@ -57,6 +78,7 @@ struct vduse_virtqueue {
 	struct vdpa_callback cb;
 	struct work_struct inject;
 	struct work_struct kick;
+	struct vdpa_reconnect_info reconnect_info;
 };
 
 struct vduse_dev;
@@ -106,6 +128,7 @@ struct vduse_dev {
 	u32 vq_align;
 	struct vduse_umem *umem;
 	struct mutex mem_lock;
+	struct vdpa_reconnect_info reconnect_status;
 };
 
 struct vduse_dev_msg {
@@ -1030,6 +1053,65 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
 	return ret;
 }
 
+int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
+{
+	struct vdpa_reconnect_info *info;
+	struct vduse_virtqueue *vq;
+	void *addr;
+
+	/*page 0 is use to save status,dpdk will use this to save the information
+	 *needed in reconnection,kernel don't need to maintain this
+	 */
+	info = &dev->reconnect_status;
+	addr = (void *)get_zeroed_page(GFP_KERNEL);
+	if (!addr)
+		return -1;
+
+	info->addr = virt_to_phys(addr);
+	info->vaddr = (unsigned long)(addr);
+	info->size = PAGE_SIZE;
+	/* index is vm Offset in PAGE_SIZE */
+	info->index = 0;
+
+	/*page 1~ vq_num + 1 save the reconnect info for vqs*/
+	for (int i = 0; i < dev->vq_num + 1; i++) {
+		vq = &dev->vqs[i];
+		info = &vq->reconnect_info;
+		addr = (void *)get_zeroed_page(GFP_KERNEL);
+		if (!addr)
+			return -1;
+
+		info->addr = virt_to_phys(addr);
+		info->vaddr = (unsigned long)(addr);
+		info->size = PAGE_SIZE;
+		info->index = i + 1;
+	}
+
+	return 0;
+}
+
+int vduse_free_reconnnect_info_mem(struct vduse_dev *dev)
+{
+	struct vdpa_reconnect_info *info;
+	struct vduse_virtqueue *vq;
+
+	info = &dev->reconnect_status;
+	free_page(info->vaddr);
+	info->size = 0;
+	info->addr = 0;
+	info->vaddr = 0;
+	for (int i = 0; i < dev->vq_num + 1; i++) {
+		vq = &dev->vqs[i];
+		info = &vq->reconnect_info;
+		free_page(info->vaddr);
+		info->size = 0;
+		info->addr = 0;
+		info->vaddr = 0;
+	}
+
+	return 0;
+}
+
 static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg)
 {
@@ -1390,6 +1472,8 @@ static int vduse_destroy_dev(char *name)
 		mutex_unlock(&dev->lock);
 		return -EBUSY;
 	}
+	vduse_free_reconnnect_info_mem(dev);
+
 	dev->connected = true;
 	mutex_unlock(&dev->lock);
 
@@ -1542,6 +1626,8 @@ static int vduse_create_dev(struct vduse_dev_config *config,
 		ret = PTR_ERR(dev->dev);
 		goto err_dev;
 	}
+
+	vduse_alloc_reconnnect_info_mem(dev);
 	__module_get(THIS_MODULE);
 
 	return 0;
-- 
2.34.3

Re: [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection

[Jason Wang]

On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
>
> Add the function vduse_alloc_reconnnect_info_mem
> and vduse_alloc_reconnnect_info_mem
> In this 2 function, vduse will get/free (vq_num + 1)*page
> Page 0 will be used to save the reconnection information, The
> Userspace App will maintain this. Page 1 ~ vq_num + 1 will save
> the reconnection information for vqs.

Please explain why this is needed instead of only describing how it is
implemented. (Code can explain itself).

>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 86 ++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 26b7e29cb900..4c256fa31fc4 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -30,6 +30,10 @@
>  #include <uapi/linux/virtio_blk.h>
>  #include <linux/mod_devicetable.h>
>
> +#ifdef CONFIG_X86
> +#include <asm/set_memory.h>
> +#endif
> +
>  #include "iova_domain.h"
>
>  #define DRV_AUTHOR   "Yongji Xie <xieyongji@bytedance.com>"
> @@ -41,6 +45,23 @@
>  #define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
>  #define VDUSE_MSG_DEFAULT_TIMEOUT 30
>
> +/* struct vdpa_reconnect_info save the page information for reconnection
> + * kernel will init these information while alloc the pages
> + * and use these information to free the pages
> + */
> +struct vdpa_reconnect_info {
> +       /* Offset (within vm_file) in PAGE_SIZE,
> +        * this just for check, not using
> +        */
> +       u32 index;
> +       /* physical address for this page*/
> +       phys_addr_t addr;
> +       /* virtual address for this page*/
> +       unsigned long vaddr;

If it could be switched by virt_to_phys() why duplicate those fields?

> +       /* memory size, here always page_size*/
> +       phys_addr_t size;

If it's always PAGE_SIZE why would we have this?

> +};
> +
>  struct vduse_virtqueue {
>         u16 index;
>         u16 num_max;
> @@ -57,6 +78,7 @@ struct vduse_virtqueue {
>         struct vdpa_callback cb;
>         struct work_struct inject;
>         struct work_struct kick;
> +       struct vdpa_reconnect_info reconnect_info;
>  };
>
>  struct vduse_dev;
> @@ -106,6 +128,7 @@ struct vduse_dev {
>         u32 vq_align;
>         struct vduse_umem *umem;
>         struct mutex mem_lock;
> +       struct vdpa_reconnect_info reconnect_status;
>  };
>
>  struct vduse_dev_msg {
> @@ -1030,6 +1053,65 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
>         return ret;
>  }
>
> +int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
> +{
> +       struct vdpa_reconnect_info *info;
> +       struct vduse_virtqueue *vq;
> +       void *addr;
> +
> +       /*page 0 is use to save status,dpdk will use this to save the information
> +        *needed in reconnection,kernel don't need to maintain this
> +        */
> +       info = &dev->reconnect_status;
> +       addr = (void *)get_zeroed_page(GFP_KERNEL);
> +       if (!addr)
> +               return -1;

-ENOMEM?

Thanks

Re: [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection

[Cindy Lu]

On Mon, Sep 18, 2023 at 4:41 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > Add the function vduse_alloc_reconnnect_info_mem
> > and vduse_alloc_reconnnect_info_mem
> > In this 2 function, vduse will get/free (vq_num + 1)*page
> > Page 0 will be used to save the reconnection information, The
> > Userspace App will maintain this. Page 1 ~ vq_num + 1 will save
> > the reconnection information for vqs.
>
> Please explain why this is needed instead of only describing how it is
> implemented. (Code can explain itself).
>
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 86 ++++++++++++++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 26b7e29cb900..4c256fa31fc4 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -30,6 +30,10 @@
> >  #include <uapi/linux/virtio_blk.h>
> >  #include <linux/mod_devicetable.h>
> >
> > +#ifdef CONFIG_X86
> > +#include <asm/set_memory.h>
> > +#endif
> > +
> >  #include "iova_domain.h"
> >
> >  #define DRV_AUTHOR   "Yongji Xie <xieyongji@bytedance.com>"
> > @@ -41,6 +45,23 @@
> >  #define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
> >  #define VDUSE_MSG_DEFAULT_TIMEOUT 30
> >
> > +/* struct vdpa_reconnect_info save the page information for reconnection
> > + * kernel will init these information while alloc the pages
> > + * and use these information to free the pages
> > + */
> > +struct vdpa_reconnect_info {
> > +       /* Offset (within vm_file) in PAGE_SIZE,
> > +        * this just for check, not using
> > +        */
> > +       u32 index;
> > +       /* physical address for this page*/
> > +       phys_addr_t addr;
> > +       /* virtual address for this page*/
> > +       unsigned long vaddr;
>
> If it could be switched by virt_to_phys() why duplicate those fields?
>
yes will remove this part
Thanks
Cindy
> > +       /* memory size, here always page_size*/
> > +       phys_addr_t size;
>
> If it's always PAGE_SIZE why would we have this?
will remove this
Thanks
Cindy
>
> > +};
> > +
> >  struct vduse_virtqueue {
> >         u16 index;
> >         u16 num_max;
> > @@ -57,6 +78,7 @@ struct vduse_virtqueue {
> >         struct vdpa_callback cb;
> >         struct work_struct inject;
> >         struct work_struct kick;
> > +       struct vdpa_reconnect_info reconnect_info;
> >  };
> >
> >  struct vduse_dev;
> > @@ -106,6 +128,7 @@ struct vduse_dev {
> >         u32 vq_align;
> >         struct vduse_umem *umem;
> >         struct mutex mem_lock;
> > +       struct vdpa_reconnect_info reconnect_status;
> >  };
> >
> >  struct vduse_dev_msg {
> > @@ -1030,6 +1053,65 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
> >         return ret;
> >  }
> >
> > +int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
> > +{
> > +       struct vdpa_reconnect_info *info;
> > +       struct vduse_virtqueue *vq;
> > +       void *addr;
> > +
> > +       /*page 0 is use to save status,dpdk will use this to save the information
> > +        *needed in reconnection,kernel don't need to maintain this
> > +        */
> > +       info = &dev->reconnect_status;
> > +       addr = (void *)get_zeroed_page(GFP_KERNEL);
> > +       if (!addr)
> > +               return -1;
>
> -ENOMEM?
>
sure will change this
Thanks
Cidny
> Thanks
>

[RFC v2 2/4] vduse: Add file operation for mmap

[Cindy Lu]

Add the operation for mmap, The user space APP will
use this function to map the pages to userspace

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 63 ++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 4c256fa31fc4..2c69f4004a6e 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1388,6 +1388,67 @@ static struct vduse_dev *vduse_dev_get_from_minor(int minor)
 	return dev;
 }
 
+static vm_fault_t vduse_vm_fault(struct vm_fault *vmf)
+{
+	struct vduse_dev *dev = vmf->vma->vm_file->private_data;
+	struct vm_area_struct *vma = vmf->vma;
+	u16 index = vma->vm_pgoff;
+	struct vduse_virtqueue *vq;
+	struct vdpa_reconnect_info *info;
+
+	if (index == 0) {
+		info = &dev->reconnect_status;
+	} else {
+		vq = &dev->vqs[index - 1];
+		info = &vq->reconnect_info;
+	}
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	if (remap_pfn_range(vma, vmf->address & PAGE_MASK, PFN_DOWN(info->addr),
+			    PAGE_SIZE, vma->vm_page_prot))
+		return VM_FAULT_SIGBUS;
+	return VM_FAULT_NOPAGE;
+}
+
+static const struct vm_operations_struct vduse_vm_ops = {
+	.fault = vduse_vm_fault,
+};
+
+static int vduse_dev_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct vduse_dev *dev = file->private_data;
+	struct vdpa_reconnect_info *info;
+	unsigned long index = vma->vm_pgoff;
+	struct vduse_virtqueue *vq;
+
+	if (vma->vm_end - vma->vm_start != PAGE_SIZE)
+		return -EINVAL;
+	if ((vma->vm_flags & VM_SHARED) == 0)
+		return -EINVAL;
+
+	if (index > 65535)
+		return -EINVAL;
+
+	if (index == 0) {
+		info = &dev->reconnect_status;
+	} else {
+		vq = &dev->vqs[index - 1];
+		info = &vq->reconnect_info;
+	}
+
+	if (info->index != index)
+		return -EINVAL;
+
+	if (info->addr & (PAGE_SIZE - 1))
+		return -EINVAL;
+	if (vma->vm_end - vma->vm_start != info->size)
+		return -EOPNOTSUPP;
+
+	vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTDUMP);
+	vma->vm_ops = &vduse_vm_ops;
+
+	return 0;
+}
+
 static int vduse_dev_open(struct inode *inode, struct file *file)
 {
 	int ret;
@@ -1420,6 +1481,8 @@ static const struct file_operations vduse_dev_fops = {
 	.unlocked_ioctl	= vduse_dev_ioctl,
 	.compat_ioctl	= compat_ptr_ioctl,
 	.llseek		= noop_llseek,
+	.mmap		= vduse_dev_mmap,
+
 };
 
 static struct vduse_dev *vduse_dev_create(void)
-- 
2.34.3

Re: [RFC v2 2/4] vduse: Add file operation for mmap

[Jason Wang]

On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
>
> Add the operation for mmap, The user space APP will
> use this function to map the pages to userspace
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 63 ++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 4c256fa31fc4..2c69f4004a6e 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1388,6 +1388,67 @@ static struct vduse_dev *vduse_dev_get_from_minor(int minor)
>         return dev;
>  }
>
> +static vm_fault_t vduse_vm_fault(struct vm_fault *vmf)
> +{
> +       struct vduse_dev *dev = vmf->vma->vm_file->private_data;
> +       struct vm_area_struct *vma = vmf->vma;
> +       u16 index = vma->vm_pgoff;
> +       struct vduse_virtqueue *vq;
> +       struct vdpa_reconnect_info *info;
> +
> +       if (index == 0) {
> +               info = &dev->reconnect_status;
> +       } else {
> +               vq = &dev->vqs[index - 1];
> +               info = &vq->reconnect_info;
> +       }
> +       vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +       if (remap_pfn_range(vma, vmf->address & PAGE_MASK, PFN_DOWN(info->addr),
> +                           PAGE_SIZE, vma->vm_page_prot))
> +               return VM_FAULT_SIGBUS;
> +       return VM_FAULT_NOPAGE;
> +}
> +
> +static const struct vm_operations_struct vduse_vm_ops = {
> +       .fault = vduse_vm_fault,
> +};
> +
> +static int vduse_dev_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +       struct vduse_dev *dev = file->private_data;
> +       struct vdpa_reconnect_info *info;
> +       unsigned long index = vma->vm_pgoff;
> +       struct vduse_virtqueue *vq;
> +
> +       if (vma->vm_end - vma->vm_start != PAGE_SIZE)
> +               return -EINVAL;
> +       if ((vma->vm_flags & VM_SHARED) == 0)
> +               return -EINVAL;
> +
> +       if (index > 65535)
> +               return -EINVAL;
> +
> +       if (index == 0) {
> +               info = &dev->reconnect_status;
> +       } else {
> +               vq = &dev->vqs[index - 1];
> +               info = &vq->reconnect_info;
> +       }
> +
> +       if (info->index != index)
> +               return -EINVAL;

Under which case could we meet this?

> +
> +       if (info->addr & (PAGE_SIZE - 1))
> +               return -EINVAL;

And this?

> +       if (vma->vm_end - vma->vm_start != info->size)
> +               return -EOPNOTSUPP;
> +
> +       vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTDUMP);

Why do you use VM_IO, VM_PFNMAP and VM_DONTDUMP?

Thanks

> +       vma->vm_ops = &vduse_vm_ops;
> +
> +       return 0;
> +}
> +
>  static int vduse_dev_open(struct inode *inode, struct file *file)
>  {
>         int ret;
> @@ -1420,6 +1481,8 @@ static const struct file_operations vduse_dev_fops = {
>         .unlocked_ioctl = vduse_dev_ioctl,
>         .compat_ioctl   = compat_ptr_ioctl,
>         .llseek         = noop_llseek,
> +       .mmap           = vduse_dev_mmap,
> +
>  };
>
>  static struct vduse_dev *vduse_dev_create(void)
> --
> 2.34.3
>

[RFC v2 3/4] vduse: update the vq_info in ioctl

[Cindy Lu]

In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
with reconnect info, After mapping the reconnect pages to userspace
The userspace App will update the reconnect_time in
struct vhost_reconnect_vring, If this is not 0 then it means this
vq is reconnected and will update the last_avail_idx

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++
 include/uapi/linux/vduse.h         |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 2c69f4004a6e..680b23dbdde2 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1221,6 +1221,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		struct vduse_vq_info vq_info;
 		struct vduse_virtqueue *vq;
 		u32 index;
+		struct vdpa_reconnect_info *area;
+		struct vhost_reconnect_vring *vq_reconnect;
 
 		ret = -EFAULT;
 		if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
@@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 
 		vq_info.ready = vq->ready;
 
+		area = &vq->reconnect_info;
+
+		vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
+		/*check if the vq is reconnect, if yes then update the last_avail_idx*/
+		if ((vq_reconnect->last_avail_idx !=
+		     vq_info.split.avail_index) &&
+		    (vq_reconnect->reconnect_time != 0)) {
+			vq_info.split.avail_index =
+				vq_reconnect->last_avail_idx;
+		}
+
 		ret = -EFAULT;
 		if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
 			break;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index 11bd48c72c6c..d585425803fd 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -350,4 +350,10 @@ struct vduse_dev_response {
 	};
 };
 
+struct vhost_reconnect_vring {
+	__u16 reconnect_time;
+	__u16 last_avail_idx;
+	_Bool avail_wrap_counter;
+};
+
 #endif /* _UAPI_VDUSE_H_ */
-- 
2.34.3

Re: [RFC v2 3/4] vduse: update the vq_info in ioctl

[Jason Wang]

On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
>
> In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
> with reconnect info, After mapping the reconnect pages to userspace
> The userspace App will update the reconnect_time in
> struct vhost_reconnect_vring, If this is not 0 then it means this
> vq is reconnected and will update the last_avail_idx
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++
>  include/uapi/linux/vduse.h         |  6 ++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 2c69f4004a6e..680b23dbdde2 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1221,6 +1221,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 struct vduse_vq_info vq_info;
>                 struct vduse_virtqueue *vq;
>                 u32 index;
> +               struct vdpa_reconnect_info *area;
> +               struct vhost_reconnect_vring *vq_reconnect;
>
>                 ret = -EFAULT;
>                 if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>
>                 vq_info.ready = vq->ready;
>
> +               area = &vq->reconnect_info;
> +
> +               vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
> +               /*check if the vq is reconnect, if yes then update the last_avail_idx*/
> +               if ((vq_reconnect->last_avail_idx !=
> +                    vq_info.split.avail_index) &&
> +                   (vq_reconnect->reconnect_time != 0)) {
> +                       vq_info.split.avail_index =
> +                               vq_reconnect->last_avail_idx;
> +               }
> +
>                 ret = -EFAULT;
>                 if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
>                         break;
> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> index 11bd48c72c6c..d585425803fd 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -350,4 +350,10 @@ struct vduse_dev_response {
>         };
>  };
>
> +struct vhost_reconnect_vring {
> +       __u16 reconnect_time;
> +       __u16 last_avail_idx;
> +       _Bool avail_wrap_counter;

Please add a comment for each field.

And I never saw _Bool is used in uapi before, maybe it's better to
pack it with last_avail_idx into a __u32.

Btw, do we need to track inflight descriptors as well?

Thanks

> +};
> +
>  #endif /* _UAPI_VDUSE_H_ */
> --
> 2.34.3
>

Re: [RFC v2 3/4] vduse: update the vq_info in ioctl

[Cindy Lu]

On Tue, Sep 12, 2023 at 3:39 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
> > with reconnect info, After mapping the reconnect pages to userspace
> > The userspace App will update the reconnect_time in
> > struct vhost_reconnect_vring, If this is not 0 then it means this
> > vq is reconnected and will update the last_avail_idx
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++
> >  include/uapi/linux/vduse.h         |  6 ++++++
> >  2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 2c69f4004a6e..680b23dbdde2 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1221,6 +1221,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >                 struct vduse_vq_info vq_info;
> >                 struct vduse_virtqueue *vq;
> >                 u32 index;
> > +               struct vdpa_reconnect_info *area;
> > +               struct vhost_reconnect_vring *vq_reconnect;
> >
> >                 ret = -EFAULT;
> >                 if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
> > @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >
> >                 vq_info.ready = vq->ready;
> >
> > +               area = &vq->reconnect_info;
> > +
> > +               vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
> > +               /*check if the vq is reconnect, if yes then update the last_avail_idx*/
> > +               if ((vq_reconnect->last_avail_idx !=
> > +                    vq_info.split.avail_index) &&
> > +                   (vq_reconnect->reconnect_time != 0)) {
> > +                       vq_info.split.avail_index =
> > +                               vq_reconnect->last_avail_idx;
> > +               }
> > +
> >                 ret = -EFAULT;
> >                 if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
> >                         break;
> > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > index 11bd48c72c6c..d585425803fd 100644
> > --- a/include/uapi/linux/vduse.h
> > +++ b/include/uapi/linux/vduse.h
> > @@ -350,4 +350,10 @@ struct vduse_dev_response {
> >         };
> >  };
> >
> > +struct vhost_reconnect_vring {
> > +       __u16 reconnect_time;
> > +       __u16 last_avail_idx;
> > +       _Bool avail_wrap_counter;
>
> Please add a comment for each field.
>
Sure will do

> And I never saw _Bool is used in uapi before, maybe it's better to
> pack it with last_avail_idx into a __u32.
>
Thanks will fix this
> Btw, do we need to track inflight descriptors as well?
>
I will check this
Thanks

cindy
> Thanks
>
> > +};
> > +
> >  #endif /* _UAPI_VDUSE_H_ */
> > --
> > 2.34.3
> >
>

Re: [RFC v2 3/4] vduse: update the vq_info in ioctl

[Maxime Coquelin]

On 9/25/23 06:15, Cindy Lu wrote:
> On Tue, Sep 12, 2023 at 3:39 PM Jason Wang <jasowang@redhat.com> wrote:
>>
>> On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
>>>
>>> In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
>>> with reconnect info, After mapping the reconnect pages to userspace
>>> The userspace App will update the reconnect_time in
>>> struct vhost_reconnect_vring, If this is not 0 then it means this
>>> vq is reconnected and will update the last_avail_idx
>>>
>>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>>> ---
>>>   drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++
>>>   include/uapi/linux/vduse.h         |  6 ++++++
>>>   2 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
>>> index 2c69f4004a6e..680b23dbdde2 100644
>>> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
>>> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
>>> @@ -1221,6 +1221,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>>>                  struct vduse_vq_info vq_info;
>>>                  struct vduse_virtqueue *vq;
>>>                  u32 index;
>>> +               struct vdpa_reconnect_info *area;
>>> +               struct vhost_reconnect_vring *vq_reconnect;
>>>
>>>                  ret = -EFAULT;
>>>                  if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
>>> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>>>
>>>                  vq_info.ready = vq->ready;
>>>
>>> +               area = &vq->reconnect_info;
>>> +
>>> +               vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
>>> +               /*check if the vq is reconnect, if yes then update the last_avail_idx*/
>>> +               if ((vq_reconnect->last_avail_idx !=
>>> +                    vq_info.split.avail_index) &&
>>> +                   (vq_reconnect->reconnect_time != 0)) {
>>> +                       vq_info.split.avail_index =
>>> +                               vq_reconnect->last_avail_idx;
>>> +               }
>>> +
>>>                  ret = -EFAULT;
>>>                  if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
>>>                          break;
>>> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
>>> index 11bd48c72c6c..d585425803fd 100644
>>> --- a/include/uapi/linux/vduse.h
>>> +++ b/include/uapi/linux/vduse.h
>>> @@ -350,4 +350,10 @@ struct vduse_dev_response {
>>>          };
>>>   };
>>>
>>> +struct vhost_reconnect_vring {
>>> +       __u16 reconnect_time;
>>> +       __u16 last_avail_idx;
>>> +       _Bool avail_wrap_counter;
>>
>> Please add a comment for each field.
>>
> Sure will do
> 
>> And I never saw _Bool is used in uapi before, maybe it's better to
>> pack it with last_avail_idx into a __u32.
>>
> Thanks will fix this
>> Btw, do we need to track inflight descriptors as well?
>>
> I will check this

For existing networking implemenation, this is not necessary.
But it should be for block devices.

Maxime

> Thanks
> 
> cindy
>> Thanks
>>
>>> +};
>>> +
>>>   #endif /* _UAPI_VDUSE_H_ */
>>> --
>>> 2.34.3
>>>
>>
> 

Re: [RFC v2 3/4] vduse: update the vq_info in ioctl

[Maxime Coquelin]

On 9/12/23 09:39, Jason Wang wrote:
> On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
>>
>> In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
>> with reconnect info, After mapping the reconnect pages to userspace
>> The userspace App will update the reconnect_time in
>> struct vhost_reconnect_vring, If this is not 0 then it means this
>> vq is reconnected and will update the last_avail_idx
>>
>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>> ---
>>   drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++
>>   include/uapi/linux/vduse.h         |  6 ++++++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
>> index 2c69f4004a6e..680b23dbdde2 100644
>> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
>> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
>> @@ -1221,6 +1221,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>>                  struct vduse_vq_info vq_info;
>>                  struct vduse_virtqueue *vq;
>>                  u32 index;
>> +               struct vdpa_reconnect_info *area;
>> +               struct vhost_reconnect_vring *vq_reconnect;
>>
>>                  ret = -EFAULT;
>>                  if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
>> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>>
>>                  vq_info.ready = vq->ready;
>>
>> +               area = &vq->reconnect_info;
>> +
>> +               vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
>> +               /*check if the vq is reconnect, if yes then update the last_avail_idx*/
>> +               if ((vq_reconnect->last_avail_idx !=
>> +                    vq_info.split.avail_index) &&
>> +                   (vq_reconnect->reconnect_time != 0)) {
>> +                       vq_info.split.avail_index =
>> +                               vq_reconnect->last_avail_idx;
>> +               }
>> +
>>                  ret = -EFAULT;
>>                  if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
>>                          break;
>> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
>> index 11bd48c72c6c..d585425803fd 100644
>> --- a/include/uapi/linux/vduse.h
>> +++ b/include/uapi/linux/vduse.h
>> @@ -350,4 +350,10 @@ struct vduse_dev_response {
>>          };
>>   };
>>
>> +struct vhost_reconnect_vring {
>> +       __u16 reconnect_time;
>> +       __u16 last_avail_idx;
>> +       _Bool avail_wrap_counter;
> 
> Please add a comment for each field.
> 
> And I never saw _Bool is used in uapi before, maybe it's better to
> pack it with last_avail_idx into a __u32.

Better as two distincts __u16 IMHO.

Thanks,
Maxime

> 
> Btw, do we need to track inflight descriptors as well?
> 
> Thanks
> 
>> +};
>> +
>>   #endif /* _UAPI_VDUSE_H_ */
>> --
>> 2.34.3
>>
> 

Re: [RFC v2 3/4] vduse: update the vq_info in ioctl

[Jason Wang]

On Fri, Sep 29, 2023 at 5:12 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
>
> On 9/12/23 09:39, Jason Wang wrote:
> > On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
> >>
> >> In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
> >> with reconnect info, After mapping the reconnect pages to userspace
> >> The userspace App will update the reconnect_time in
> >> struct vhost_reconnect_vring, If this is not 0 then it means this
> >> vq is reconnected and will update the last_avail_idx
> >>
> >> Signed-off-by: Cindy Lu <lulu@redhat.com>
> >> ---
> >>   drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++
> >>   include/uapi/linux/vduse.h         |  6 ++++++
> >>   2 files changed, 19 insertions(+)
> >>
> >> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> >> index 2c69f4004a6e..680b23dbdde2 100644
> >> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> >> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> >> @@ -1221,6 +1221,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >>                  struct vduse_vq_info vq_info;
> >>                  struct vduse_virtqueue *vq;
> >>                  u32 index;
> >> +               struct vdpa_reconnect_info *area;
> >> +               struct vhost_reconnect_vring *vq_reconnect;
> >>
> >>                  ret = -EFAULT;
> >>                  if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
> >> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >>
> >>                  vq_info.ready = vq->ready;
> >>
> >> +               area = &vq->reconnect_info;
> >> +
> >> +               vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
> >> +               /*check if the vq is reconnect, if yes then update the last_avail_idx*/
> >> +               if ((vq_reconnect->last_avail_idx !=
> >> +                    vq_info.split.avail_index) &&
> >> +                   (vq_reconnect->reconnect_time != 0)) {
> >> +                       vq_info.split.avail_index =
> >> +                               vq_reconnect->last_avail_idx;
> >> +               }
> >> +
> >>                  ret = -EFAULT;
> >>                  if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
> >>                          break;
> >> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> >> index 11bd48c72c6c..d585425803fd 100644
> >> --- a/include/uapi/linux/vduse.h
> >> +++ b/include/uapi/linux/vduse.h
> >> @@ -350,4 +350,10 @@ struct vduse_dev_response {
> >>          };
> >>   };
> >>
> >> +struct vhost_reconnect_vring {
> >> +       __u16 reconnect_time;
> >> +       __u16 last_avail_idx;
> >> +       _Bool avail_wrap_counter;
> >
> > Please add a comment for each field.
> >
> > And I never saw _Bool is used in uapi before, maybe it's better to
> > pack it with last_avail_idx into a __u32.
>
> Better as two distincts __u16 IMHO.

Fine with me.

Thanks

>
> Thanks,
> Maxime
>
> >
> > Btw, do we need to track inflight descriptors as well?
> >
> > Thanks
> >
> >> +};
> >> +
> >>   #endif /* _UAPI_VDUSE_H_ */
> >> --
> >> 2.34.3
> >>
> >
>

Re: [RFC v2 3/4] vduse: update the vq_info in ioctl

[Cindy Lu]

On Sun, Oct 8, 2023 at 1:17 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Sep 29, 2023 at 5:12 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
> >
> >
> >
> > On 9/12/23 09:39, Jason Wang wrote:
> > > On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote:
> > >>
> > >> In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
> > >> with reconnect info, After mapping the reconnect pages to userspace
> > >> The userspace App will update the reconnect_time in
> > >> struct vhost_reconnect_vring, If this is not 0 then it means this
> > >> vq is reconnected and will update the last_avail_idx
> > >>
> > >> Signed-off-by: Cindy Lu <lulu@redhat.com>
> > >> ---
> > >>   drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++
> > >>   include/uapi/linux/vduse.h         |  6 ++++++
> > >>   2 files changed, 19 insertions(+)
> > >>
> > >> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > >> index 2c69f4004a6e..680b23dbdde2 100644
> > >> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > >> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > >> @@ -1221,6 +1221,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> > >>                  struct vduse_vq_info vq_info;
> > >>                  struct vduse_virtqueue *vq;
> > >>                  u32 index;
> > >> +               struct vdpa_reconnect_info *area;
> > >> +               struct vhost_reconnect_vring *vq_reconnect;
> > >>
> > >>                  ret = -EFAULT;
> > >>                  if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
> > >> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> > >>
> > >>                  vq_info.ready = vq->ready;
> > >>
> > >> +               area = &vq->reconnect_info;
> > >> +
> > >> +               vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
> > >> +               /*check if the vq is reconnect, if yes then update the last_avail_idx*/
> > >> +               if ((vq_reconnect->last_avail_idx !=
> > >> +                    vq_info.split.avail_index) &&
> > >> +                   (vq_reconnect->reconnect_time != 0)) {
> > >> +                       vq_info.split.avail_index =
> > >> +                               vq_reconnect->last_avail_idx;
> > >> +               }
> > >> +
> > >>                  ret = -EFAULT;
> > >>                  if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
> > >>                          break;
> > >> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > >> index 11bd48c72c6c..d585425803fd 100644
> > >> --- a/include/uapi/linux/vduse.h
> > >> +++ b/include/uapi/linux/vduse.h
> > >> @@ -350,4 +350,10 @@ struct vduse_dev_response {
> > >>          };
> > >>   };
> > >>
> > >> +struct vhost_reconnect_vring {
> > >> +       __u16 reconnect_time;
> > >> +       __u16 last_avail_idx;
> > >> +       _Bool avail_wrap_counter;
> > >
> > > Please add a comment for each field.
> > >
> > > And I never saw _Bool is used in uapi before, maybe it's better to
> > > pack it with last_avail_idx into a __u32.
> >
> > Better as two distincts __u16 IMHO.
>
> Fine with me.
>
> Thanks
>

sure will fix  this
Thanks
Cindy
> >
> > Thanks,
> > Maxime
> >
> > >
> > > Btw, do we need to track inflight descriptors as well?
> > >
> > > Thanks
> > >
> > >> +};
> > >> +
> > >>   #endif /* _UAPI_VDUSE_H_ */
> > >> --
> > >> 2.34.3
> > >>
> > >
> >
>

[RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Cindy Lu]

In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
and The number of mapping memory pages from the kernel. The userspace
App can use this information to map the pages.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++++++
 include/uapi/linux/vduse.h         | 15 +++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 680b23dbdde2..c99f99892b5c 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1368,6 +1368,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		ret = 0;
 		break;
 	}
+	case VDUSE_GET_RECONNECT_INFO: {
+		struct vduse_reconnect_mmap_info info;
+
+		ret = -EFAULT;
+		if (copy_from_user(&info, argp, sizeof(info)))
+			break;
+
+		info.size = PAGE_SIZE;
+		info.max_index = dev->vq_num + 1;
+
+		if (copy_to_user(argp, &info, sizeof(info)))
+			break;
+		ret = 0;
+		break;
+	}
 	default:
 		ret = -ENOIOCTLCMD;
 		break;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index d585425803fd..ce55e34f63d7 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -356,4 +356,19 @@ struct vhost_reconnect_vring {
 	_Bool avail_wrap_counter;
 };
 
+/**
+ * struct vduse_reconnect_mmap_info
+ * @size: mapping memory size, always page_size here
+ * @max_index: the number of pages allocated in kernel,just
+ * use for check
+ */
+
+struct vduse_reconnect_mmap_info {
+	__u32 size;
+	__u32 max_index;
+};
+
+#define VDUSE_GET_RECONNECT_INFO \
+	_IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
+
 #endif /* _UAPI_VDUSE_H_ */
-- 
2.34.3

Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Jason Wang]

On Tue, Sep 12, 2023 at 11:01 AM Cindy Lu <lulu@redhat.com> wrote:
>
> In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> and The number of mapping memory pages from the kernel. The userspace
> App can use this information to map the pages.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++++++
>  include/uapi/linux/vduse.h         | 15 +++++++++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 680b23dbdde2..c99f99892b5c 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1368,6 +1368,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 ret = 0;
>                 break;
>         }
> +       case VDUSE_GET_RECONNECT_INFO: {
> +               struct vduse_reconnect_mmap_info info;
> +
> +               ret = -EFAULT;
> +               if (copy_from_user(&info, argp, sizeof(info)))
> +                       break;
> +
> +               info.size = PAGE_SIZE;
> +               info.max_index = dev->vq_num + 1;
> +
> +               if (copy_to_user(argp, &info, sizeof(info)))
> +                       break;
> +               ret = 0;
> +               break;
> +       }
>         default:
>                 ret = -ENOIOCTLCMD;
>                 break;
> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> index d585425803fd..ce55e34f63d7 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -356,4 +356,19 @@ struct vhost_reconnect_vring {
>         _Bool avail_wrap_counter;
>  };
>
> +/**
> + * struct vduse_reconnect_mmap_info
> + * @size: mapping memory size, always page_size here
> + * @max_index: the number of pages allocated in kernel,just
> + * use for check
> + */
> +
> +struct vduse_reconnect_mmap_info {
> +       __u32 size;
> +       __u32 max_index;
> +};

One thing I didn't understand is that, aren't the things we used to
store connection info belong to uAPI? If not, how can we make sure the
connections work across different vendors/implementations. If yes,
where?

Thanks

> +
> +#define VDUSE_GET_RECONNECT_INFO \
> +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> +
>  #endif /* _UAPI_VDUSE_H_ */
> --
> 2.34.3
>

Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Cindy Lu]

On Mon, Sep 18, 2023 at 4:49 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Sep 12, 2023 at 11:01 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> > and The number of mapping memory pages from the kernel. The userspace
> > App can use this information to map the pages.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++++++
> >  include/uapi/linux/vduse.h         | 15 +++++++++++++++
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 680b23dbdde2..c99f99892b5c 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1368,6 +1368,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >                 ret = 0;
> >                 break;
> >         }
> > +       case VDUSE_GET_RECONNECT_INFO: {
> > +               struct vduse_reconnect_mmap_info info;
> > +
> > +               ret = -EFAULT;
> > +               if (copy_from_user(&info, argp, sizeof(info)))
> > +                       break;
> > +
> > +               info.size = PAGE_SIZE;
> > +               info.max_index = dev->vq_num + 1;
> > +
> > +               if (copy_to_user(argp, &info, sizeof(info)))
> > +                       break;
> > +               ret = 0;
> > +               break;
> > +       }
> >         default:
> >                 ret = -ENOIOCTLCMD;
> >                 break;
> > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > index d585425803fd..ce55e34f63d7 100644
> > --- a/include/uapi/linux/vduse.h
> > +++ b/include/uapi/linux/vduse.h
> > @@ -356,4 +356,19 @@ struct vhost_reconnect_vring {
> >         _Bool avail_wrap_counter;
> >  };
> >
> > +/**
> > + * struct vduse_reconnect_mmap_info
> > + * @size: mapping memory size, always page_size here
> > + * @max_index: the number of pages allocated in kernel,just
> > + * use for check
> > + */
> > +
> > +struct vduse_reconnect_mmap_info {
> > +       __u32 size;
> > +       __u32 max_index;
> > +};
>
> One thing I didn't understand is that, aren't the things we used to
> store connection info belong to uAPI? If not, how can we make sure the
> connections work across different vendors/implementations. If yes,
> where?
>
> Thanks
>
The process for this reconnecttion  is
A.The first-time connection
1> The userland app checks if the device exists
2>  use the ioctl to create the vduse device
3> Mapping the kernel page to userland and save the
App-version/features/other information to this page
4>  if the Userland app needs to exit, then the Userland app will only
unmap the page and then exit

B, the re-connection
1> the userland app finds the device is existing
2> Mapping the kernel page to userland
3> check if the information in shared memory is satisfied to
reconnect,if ok then continue to reconnect
4> continue working

 For now these information are all from userland,So here the page will
be maintained by the userland App
in the previous code we only saved the api-version by uAPI .  if  we
need to support reconnection maybe we need to add 2 new uAPI for this,
one of the uAPI is to save the reconnect  information and another is
to get the information

maybe something like

struct vhost_reconnect_data {
uint32_t version;
uint64_t features;
uint8_t status;
struct virtio_net_config config;
uint32_t nr_vrings;
};

#define VDUSE_GET_RECONNECT_INFO _IOR (VDUSE_BASE, 0x1c, struct
vhost_reconnect_data)

#define VDUSE_SET_RECONNECT_INFO  _IOWR(VDUSE_BASE, 0x1d, struct
vhost_reconnect_data)

Thanks
Cindy




> > +
> > +#define VDUSE_GET_RECONNECT_INFO \
> > +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> > +
> >  #endif /* _UAPI_VDUSE_H_ */
> > --
> > 2.34.3
> >
>

Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Jason Wang]

On Thu, Sep 21, 2023 at 10:07 PM Cindy Lu <lulu@redhat.com> wrote:
>
> On Mon, Sep 18, 2023 at 4:49 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Sep 12, 2023 at 11:01 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> > > and The number of mapping memory pages from the kernel. The userspace
> > > App can use this information to map the pages.
> > >
> > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > ---
> > >  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++++++
> > >  include/uapi/linux/vduse.h         | 15 +++++++++++++++
> > >  2 files changed, 30 insertions(+)
> > >
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index 680b23dbdde2..c99f99892b5c 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -1368,6 +1368,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> > >                 ret = 0;
> > >                 break;
> > >         }
> > > +       case VDUSE_GET_RECONNECT_INFO: {
> > > +               struct vduse_reconnect_mmap_info info;
> > > +
> > > +               ret = -EFAULT;
> > > +               if (copy_from_user(&info, argp, sizeof(info)))
> > > +                       break;
> > > +
> > > +               info.size = PAGE_SIZE;
> > > +               info.max_index = dev->vq_num + 1;
> > > +
> > > +               if (copy_to_user(argp, &info, sizeof(info)))
> > > +                       break;
> > > +               ret = 0;
> > > +               break;
> > > +       }
> > >         default:
> > >                 ret = -ENOIOCTLCMD;
> > >                 break;
> > > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > > index d585425803fd..ce55e34f63d7 100644
> > > --- a/include/uapi/linux/vduse.h
> > > +++ b/include/uapi/linux/vduse.h
> > > @@ -356,4 +356,19 @@ struct vhost_reconnect_vring {
> > >         _Bool avail_wrap_counter;
> > >  };
> > >
> > > +/**
> > > + * struct vduse_reconnect_mmap_info
> > > + * @size: mapping memory size, always page_size here
> > > + * @max_index: the number of pages allocated in kernel,just
> > > + * use for check
> > > + */
> > > +
> > > +struct vduse_reconnect_mmap_info {
> > > +       __u32 size;
> > > +       __u32 max_index;
> > > +};
> >
> > One thing I didn't understand is that, aren't the things we used to
> > store connection info belong to uAPI? If not, how can we make sure the
> > connections work across different vendors/implementations. If yes,
> > where?
> >
> > Thanks
> >
> The process for this reconnecttion  is
> A.The first-time connection
> 1> The userland app checks if the device exists
> 2>  use the ioctl to create the vduse device
> 3> Mapping the kernel page to userland and save the
> App-version/features/other information to this page
> 4>  if the Userland app needs to exit, then the Userland app will only
> unmap the page and then exit
>
> B, the re-connection
> 1> the userland app finds the device is existing
> 2> Mapping the kernel page to userland
> 3> check if the information in shared memory is satisfied to
> reconnect,if ok then continue to reconnect
> 4> continue working
>
>  For now these information are all from userland,So here the page will
> be maintained by the userland App
> in the previous code we only saved the api-version by uAPI .  if  we
> need to support reconnection maybe we need to add 2 new uAPI for this,
> one of the uAPI is to save the reconnect  information and another is
> to get the information
>
> maybe something like
>
> struct vhost_reconnect_data {
> uint32_t version;
> uint64_t features;
> uint8_t status;
> struct virtio_net_config config;
> uint32_t nr_vrings;
> };

Probably, then we can make sure the re-connection works across
different vduse-daemon implementations.

>
> #define VDUSE_GET_RECONNECT_INFO _IOR (VDUSE_BASE, 0x1c, struct
> vhost_reconnect_data)
>
> #define VDUSE_SET_RECONNECT_INFO  _IOWR(VDUSE_BASE, 0x1d, struct
> vhost_reconnect_data)

Not sure I get this, but the idea is to map those pages to user space,
any reason we need this uAPI?

Thanks

>
> Thanks
> Cindy
>
>
>
>
> > > +
> > > +#define VDUSE_GET_RECONNECT_INFO \
> > > +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> > > +
> > >  #endif /* _UAPI_VDUSE_H_ */
> > > --
> > > 2.34.3
> > >
> >
>

Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Cindy Lu]

On Mon, Sep 25, 2023 at 10:58 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Sep 21, 2023 at 10:07 PM Cindy Lu <lulu@redhat.com> wrote:
> >
> > On Mon, Sep 18, 2023 at 4:49 PM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Tue, Sep 12, 2023 at 11:01 AM Cindy Lu <lulu@redhat.com> wrote:
> > > >
> > > > In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> > > > and The number of mapping memory pages from the kernel. The userspace
> > > > App can use this information to map the pages.
> > > >
> > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > > ---
> > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++++++
> > > >  include/uapi/linux/vduse.h         | 15 +++++++++++++++
> > > >  2 files changed, 30 insertions(+)
> > > >
> > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > index 680b23dbdde2..c99f99892b5c 100644
> > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > @@ -1368,6 +1368,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> > > >                 ret = 0;
> > > >                 break;
> > > >         }
> > > > +       case VDUSE_GET_RECONNECT_INFO: {
> > > > +               struct vduse_reconnect_mmap_info info;
> > > > +
> > > > +               ret = -EFAULT;
> > > > +               if (copy_from_user(&info, argp, sizeof(info)))
> > > > +                       break;
> > > > +
> > > > +               info.size = PAGE_SIZE;
> > > > +               info.max_index = dev->vq_num + 1;
> > > > +
> > > > +               if (copy_to_user(argp, &info, sizeof(info)))
> > > > +                       break;
> > > > +               ret = 0;
> > > > +               break;
> > > > +       }
> > > >         default:
> > > >                 ret = -ENOIOCTLCMD;
> > > >                 break;
> > > > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > > > index d585425803fd..ce55e34f63d7 100644
> > > > --- a/include/uapi/linux/vduse.h
> > > > +++ b/include/uapi/linux/vduse.h
> > > > @@ -356,4 +356,19 @@ struct vhost_reconnect_vring {
> > > >         _Bool avail_wrap_counter;
> > > >  };
> > > >
> > > > +/**
> > > > + * struct vduse_reconnect_mmap_info
> > > > + * @size: mapping memory size, always page_size here
> > > > + * @max_index: the number of pages allocated in kernel,just
> > > > + * use for check
> > > > + */
> > > > +
> > > > +struct vduse_reconnect_mmap_info {
> > > > +       __u32 size;
> > > > +       __u32 max_index;
> > > > +};
> > >
> > > One thing I didn't understand is that, aren't the things we used to
> > > store connection info belong to uAPI? If not, how can we make sure the
> > > connections work across different vendors/implementations. If yes,
> > > where?
> > >
> > > Thanks
> > >
> > The process for this reconnecttion  is
> > A.The first-time connection
> > 1> The userland app checks if the device exists
> > 2>  use the ioctl to create the vduse device
> > 3> Mapping the kernel page to userland and save the
> > App-version/features/other information to this page
> > 4>  if the Userland app needs to exit, then the Userland app will only
> > unmap the page and then exit
> >
> > B, the re-connection
> > 1> the userland app finds the device is existing
> > 2> Mapping the kernel page to userland
> > 3> check if the information in shared memory is satisfied to
> > reconnect,if ok then continue to reconnect
> > 4> continue working
> >
> >  For now these information are all from userland,So here the page will
> > be maintained by the userland App
> > in the previous code we only saved the api-version by uAPI .  if  we
> > need to support reconnection maybe we need to add 2 new uAPI for this,
> > one of the uAPI is to save the reconnect  information and another is
> > to get the information
> >
> > maybe something like
> >
> > struct vhost_reconnect_data {
> > uint32_t version;
> > uint64_t features;
> > uint8_t status;
> > struct virtio_net_config config;
> > uint32_t nr_vrings;
> > };
>
> Probably, then we can make sure the re-connection works across
> different vduse-daemon implementations.
>
> >
> > #define VDUSE_GET_RECONNECT_INFO _IOR (VDUSE_BASE, 0x1c, struct
> > vhost_reconnect_data)
> >
> > #define VDUSE_SET_RECONNECT_INFO  _IOWR(VDUSE_BASE, 0x1d, struct
> > vhost_reconnect_data)
>
> Not sure I get this, but the idea is to map those pages to user space,
> any reason we need this uAPI?
>
> Thanks
>
Sorry， I didn't write it clearly, I mean if I we don't want to use the
mmap to sync/check the vduse status, we need to add these 2 new uAPIs,
 these 2 methods are all ok for me.
For the vq related information still need to use the mmap to sync
Thanks
cindy
> >
> > Thanks
> > Cindy
> >
> >
> >
> >
> > > > +
> > > > +#define VDUSE_GET_RECONNECT_INFO \
> > > > +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> > > > +
> > > >  #endif /* _UAPI_VDUSE_H_ */
> > > > --
> > > > 2.34.3
> > > >
> > >
> >
>

Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Maxime Coquelin]

On 9/25/23 04:57, Jason Wang wrote:
> On Thu, Sep 21, 2023 at 10:07 PM Cindy Lu <lulu@redhat.com> wrote:
>>
>> On Mon, Sep 18, 2023 at 4:49 PM Jason Wang <jasowang@redhat.com> wrote:
>>>
>>> On Tue, Sep 12, 2023 at 11:01 AM Cindy Lu <lulu@redhat.com> wrote:
>>>>
>>>> In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
>>>> and The number of mapping memory pages from the kernel. The userspace
>>>> App can use this information to map the pages.
>>>>
>>>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>>>> ---
>>>>   drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++++++
>>>>   include/uapi/linux/vduse.h         | 15 +++++++++++++++
>>>>   2 files changed, 30 insertions(+)
>>>>
>>>> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
>>>> index 680b23dbdde2..c99f99892b5c 100644
>>>> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
>>>> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
>>>> @@ -1368,6 +1368,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>>>>                  ret = 0;
>>>>                  break;
>>>>          }
>>>> +       case VDUSE_GET_RECONNECT_INFO: {
>>>> +               struct vduse_reconnect_mmap_info info;
>>>> +
>>>> +               ret = -EFAULT;
>>>> +               if (copy_from_user(&info, argp, sizeof(info)))
>>>> +                       break;
>>>> +
>>>> +               info.size = PAGE_SIZE;
>>>> +               info.max_index = dev->vq_num + 1;
>>>> +
>>>> +               if (copy_to_user(argp, &info, sizeof(info)))
>>>> +                       break;
>>>> +               ret = 0;
>>>> +               break;
>>>> +       }
>>>>          default:
>>>>                  ret = -ENOIOCTLCMD;
>>>>                  break;
>>>> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
>>>> index d585425803fd..ce55e34f63d7 100644
>>>> --- a/include/uapi/linux/vduse.h
>>>> +++ b/include/uapi/linux/vduse.h
>>>> @@ -356,4 +356,19 @@ struct vhost_reconnect_vring {
>>>>          _Bool avail_wrap_counter;
>>>>   };
>>>>
>>>> +/**
>>>> + * struct vduse_reconnect_mmap_info
>>>> + * @size: mapping memory size, always page_size here
>>>> + * @max_index: the number of pages allocated in kernel,just
>>>> + * use for check
>>>> + */
>>>> +
>>>> +struct vduse_reconnect_mmap_info {
>>>> +       __u32 size;
>>>> +       __u32 max_index;
>>>> +};
>>>
>>> One thing I didn't understand is that, aren't the things we used to
>>> store connection info belong to uAPI? If not, how can we make sure the
>>> connections work across different vendors/implementations. If yes,
>>> where?
>>>
>>> Thanks
>>>
>> The process for this reconnecttion  is
>> A.The first-time connection
>> 1> The userland app checks if the device exists
>> 2>  use the ioctl to create the vduse device
>> 3> Mapping the kernel page to userland and save the
>> App-version/features/other information to this page
>> 4>  if the Userland app needs to exit, then the Userland app will only
>> unmap the page and then exit
>>
>> B, the re-connection
>> 1> the userland app finds the device is existing
>> 2> Mapping the kernel page to userland
>> 3> check if the information in shared memory is satisfied to
>> reconnect,if ok then continue to reconnect
>> 4> continue working
>>
>>   For now these information are all from userland,So here the page will
>> be maintained by the userland App
>> in the previous code we only saved the api-version by uAPI .  if  we
>> need to support reconnection maybe we need to add 2 new uAPI for this,
>> one of the uAPI is to save the reconnect  information and another is
>> to get the information
>>
>> maybe something like
>>
>> struct vhost_reconnect_data {
>> uint32_t version;
>> uint64_t features;
>> uint8_t status;
>> struct virtio_net_config config;
>> uint32_t nr_vrings;
>> };
> 
> Probably, then we can make sure the re-connection works across
> different vduse-daemon implementations.

+1, we need to have this defined in the uAPI to support interoperability
across different VDUSE userspace implementations.

> 
>>
>> #define VDUSE_GET_RECONNECT_INFO _IOR (VDUSE_BASE, 0x1c, struct
>> vhost_reconnect_data)
>>
>> #define VDUSE_SET_RECONNECT_INFO  _IOWR(VDUSE_BASE, 0x1d, struct
>> vhost_reconnect_data)
> 
> Not sure I get this, but the idea is to map those pages to user space,
> any reason we need this uAPI?

It should not be necessary if the mmapped layout is properly defined.

Thanks,
Maxime

> Thanks
> 
>>
>> Thanks
>> Cindy
>>
>>
>>
>>
>>>> +
>>>> +#define VDUSE_GET_RECONNECT_INFO \
>>>> +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
>>>> +
>>>>   #endif /* _UAPI_VDUSE_H_ */
>>>> --
>>>> 2.34.3
>>>>
>>>
>>
> 

Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Cindy Lu]

On Fri, Sep 29, 2023 at 5:08 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
>
> On 9/25/23 04:57, Jason Wang wrote:
> > On Thu, Sep 21, 2023 at 10:07 PM Cindy Lu <lulu@redhat.com> wrote:
> >>
> >> On Mon, Sep 18, 2023 at 4:49 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>
> >>> On Tue, Sep 12, 2023 at 11:01 AM Cindy Lu <lulu@redhat.com> wrote:
> >>>>
> >>>> In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> >>>> and The number of mapping memory pages from the kernel. The userspace
> >>>> App can use this information to map the pages.
> >>>>
> >>>> Signed-off-by: Cindy Lu <lulu@redhat.com>
> >>>> ---
> >>>>   drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++++++
> >>>>   include/uapi/linux/vduse.h         | 15 +++++++++++++++
> >>>>   2 files changed, 30 insertions(+)
> >>>>
> >>>> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> >>>> index 680b23dbdde2..c99f99892b5c 100644
> >>>> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> >>>> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> >>>> @@ -1368,6 +1368,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >>>>                  ret = 0;
> >>>>                  break;
> >>>>          }
> >>>> +       case VDUSE_GET_RECONNECT_INFO: {
> >>>> +               struct vduse_reconnect_mmap_info info;
> >>>> +
> >>>> +               ret = -EFAULT;
> >>>> +               if (copy_from_user(&info, argp, sizeof(info)))
> >>>> +                       break;
> >>>> +
> >>>> +               info.size = PAGE_SIZE;
> >>>> +               info.max_index = dev->vq_num + 1;
> >>>> +
> >>>> +               if (copy_to_user(argp, &info, sizeof(info)))
> >>>> +                       break;
> >>>> +               ret = 0;
> >>>> +               break;
> >>>> +       }
> >>>>          default:
> >>>>                  ret = -ENOIOCTLCMD;
> >>>>                  break;
> >>>> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> >>>> index d585425803fd..ce55e34f63d7 100644
> >>>> --- a/include/uapi/linux/vduse.h
> >>>> +++ b/include/uapi/linux/vduse.h
> >>>> @@ -356,4 +356,19 @@ struct vhost_reconnect_vring {
> >>>>          _Bool avail_wrap_counter;
> >>>>   };
> >>>>
> >>>> +/**
> >>>> + * struct vduse_reconnect_mmap_info
> >>>> + * @size: mapping memory size, always page_size here
> >>>> + * @max_index: the number of pages allocated in kernel,just
> >>>> + * use for check
> >>>> + */
> >>>> +
> >>>> +struct vduse_reconnect_mmap_info {
> >>>> +       __u32 size;
> >>>> +       __u32 max_index;
> >>>> +};
> >>>
> >>> One thing I didn't understand is that, aren't the things we used to
> >>> store connection info belong to uAPI? If not, how can we make sure the
> >>> connections work across different vendors/implementations. If yes,
> >>> where?
> >>>
> >>> Thanks
> >>>
> >> The process for this reconnecttion  is
> >> A.The first-time connection
> >> 1> The userland app checks if the device exists
> >> 2>  use the ioctl to create the vduse device
> >> 3> Mapping the kernel page to userland and save the
> >> App-version/features/other information to this page
> >> 4>  if the Userland app needs to exit, then the Userland app will only
> >> unmap the page and then exit
> >>
> >> B, the re-connection
> >> 1> the userland app finds the device is existing
> >> 2> Mapping the kernel page to userland
> >> 3> check if the information in shared memory is satisfied to
> >> reconnect,if ok then continue to reconnect
> >> 4> continue working
> >>
> >>   For now these information are all from userland,So here the page will
> >> be maintained by the userland App
> >> in the previous code we only saved the api-version by uAPI .  if  we
> >> need to support reconnection maybe we need to add 2 new uAPI for this,
> >> one of the uAPI is to save the reconnect  information and another is
> >> to get the information
> >>
> >> maybe something like
> >>
> >> struct vhost_reconnect_data {
> >> uint32_t version;
> >> uint64_t features;
> >> uint8_t status;
> >> struct virtio_net_config config;
> >> uint32_t nr_vrings;
> >> };
> >
> > Probably, then we can make sure the re-connection works across
> > different vduse-daemon implementations.
>
> +1, we need to have this defined in the uAPI to support interoperability
> across different VDUSE userspace implementations.
>
> >
> >>
> >> #define VDUSE_GET_RECONNECT_INFO _IOR (VDUSE_BASE, 0x1c, struct
> >> vhost_reconnect_data)
> >>
> >> #define VDUSE_SET_RECONNECT_INFO  _IOWR(VDUSE_BASE, 0x1d, struct
> >> vhost_reconnect_data)
> >
> > Not sure I get this, but the idea is to map those pages to user space,
> > any reason we need this uAPI?
>
> It should not be necessary if the mmapped layout is properly defined.
>
> Thanks,
> Maxime
>
Sure , I will use mmap to sync the reconnect status
Thanks
cindy
> > Thanks
> >
> >>
> >> Thanks
> >> Cindy
> >>
> >>
> >>
> >>
> >>>> +
> >>>> +#define VDUSE_GET_RECONNECT_INFO \
> >>>> +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> >>>> +
> >>>>   #endif /* _UAPI_VDUSE_H_ */
> >>>> --
> >>>> 2.34.3
> >>>>
> >>>
> >>
> >
>