* [RFC v2 0/4] Support reconnection in vduse
@ 2023-09-12 3:00 Cindy Lu
2023-09-12 3:00 ` [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection Cindy Lu
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Cindy Lu @ 2023-09-12 3:00 UTC (permalink / raw)
To: lulu, jasowang, mst, maxime.coquelin, xieyongji, kvm,
linux-kernel, virtualization, netdev
Cc: stable
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection
2023-09-12 3:00 [RFC v2 0/4] Support reconnection in vduse Cindy Lu
@ 2023-09-12 3:00 ` Cindy Lu
2023-09-18 8:41 ` Jason Wang
2023-09-12 3:00 ` [RFC v2 2/4] vduse: Add file operation for mmap Cindy Lu
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Cindy Lu @ 2023-09-12 3:00 UTC (permalink / raw)
To: lulu, jasowang, mst, maxime.coquelin, xieyongji, kvm,
linux-kernel, virtualization, netdev
Cc: stable
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC v2 2/4] vduse: Add file operation for mmap
2023-09-12 3:00 [RFC v2 0/4] Support reconnection in vduse Cindy Lu
2023-09-12 3:00 ` [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection Cindy Lu
@ 2023-09-12 3:00 ` Cindy Lu
2023-09-18 8:46 ` Jason Wang
2023-09-12 3:00 ` [RFC v2 3/4] vduse: update the vq_info in ioctl Cindy Lu
2023-09-12 3:00 ` [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO Cindy Lu
3 siblings, 1 reply; 20+ messages in thread
From: Cindy Lu @ 2023-09-12 3:00 UTC (permalink / raw)
To: lulu, jasowang, mst, maxime.coquelin, xieyongji, kvm,
linux-kernel, virtualization, netdev
Cc: stable
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC v2 3/4] vduse: update the vq_info in ioctl
2023-09-12 3:00 [RFC v2 0/4] Support reconnection in vduse Cindy Lu
2023-09-12 3:00 ` [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection Cindy Lu
2023-09-12 3:00 ` [RFC v2 2/4] vduse: Add file operation for mmap Cindy Lu
@ 2023-09-12 3:00 ` Cindy Lu
2023-09-12 7:39 ` Jason Wang
2023-09-12 3:00 ` [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO Cindy Lu
3 siblings, 1 reply; 20+ messages in thread
From: Cindy Lu @ 2023-09-12 3:00 UTC (permalink / raw)
To: lulu, jasowang, mst, maxime.coquelin, xieyongji, kvm,
linux-kernel, virtualization, netdev
Cc: stable
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
2023-09-12 3:00 [RFC v2 0/4] Support reconnection in vduse Cindy Lu
` (2 preceding siblings ...)
2023-09-12 3:00 ` [RFC v2 3/4] vduse: update the vq_info in ioctl Cindy Lu
@ 2023-09-12 3:00 ` Cindy Lu
2023-09-18 8:48 ` Jason Wang
3 siblings, 1 reply; 20+ messages in thread
From: Cindy Lu @ 2023-09-12 3:00 UTC (permalink / raw)
To: lulu, jasowang, mst, maxime.coquelin, xieyongji, kvm,
linux-kernel, virtualization, netdev
Cc: stable
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC v2 3/4] vduse: update the vq_info in ioctl
2023-09-12 3:00 ` [RFC v2 3/4] vduse: update the vq_info in ioctl Cindy Lu
@ 2023-09-12 7:39 ` Jason Wang
2023-09-25 4:15 ` Cindy Lu
2023-09-29 9:12 ` Maxime Coquelin
0 siblings, 2 replies; 20+ messages in thread
From: Jason Wang @ 2023-09-12 7:39 UTC (permalink / raw)
To: Cindy Lu
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection
2023-09-12 3:00 ` [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection Cindy Lu
@ 2023-09-18 8:41 ` Jason Wang
2023-09-21 14:06 ` Cindy Lu
0 siblings, 1 reply; 20+ messages in thread
From: Jason Wang @ 2023-09-18 8:41 UTC (permalink / raw)
To: Cindy Lu
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 2/4] vduse: Add file operation for mmap
2023-09-12 3:00 ` [RFC v2 2/4] vduse: Add file operation for mmap Cindy Lu
@ 2023-09-18 8:46 ` Jason Wang
0 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2023-09-18 8:46 UTC (permalink / raw)
To: Cindy Lu
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
2023-09-12 3:00 ` [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO Cindy Lu
@ 2023-09-18 8:48 ` Jason Wang
2023-09-21 14:01 ` Cindy Lu
0 siblings, 1 reply; 20+ messages in thread
From: Jason Wang @ 2023-09-18 8:48 UTC (permalink / raw)
To: Cindy Lu
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
2023-09-18 8:48 ` Jason Wang
@ 2023-09-21 14:01 ` Cindy Lu
2023-09-25 2:57 ` Jason Wang
0 siblings, 1 reply; 20+ messages in thread
From: Cindy Lu @ 2023-09-21 14:01 UTC (permalink / raw)
To: Jason Wang
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection
2023-09-18 8:41 ` Jason Wang
@ 2023-09-21 14:06 ` Cindy Lu
0 siblings, 0 replies; 20+ messages in thread
From: Cindy Lu @ 2023-09-21 14:06 UTC (permalink / raw)
To: Jason Wang
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
2023-09-21 14:01 ` Cindy Lu
@ 2023-09-25 2:57 ` Jason Wang
2023-09-25 3:19 ` Cindy Lu
2023-09-29 9:08 ` Maxime Coquelin
0 siblings, 2 replies; 20+ messages in thread
From: Jason Wang @ 2023-09-25 2:57 UTC (permalink / raw)
To: Cindy Lu
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
> > >
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
2023-09-25 2:57 ` Jason Wang
@ 2023-09-25 3:19 ` Cindy Lu
2023-09-29 9:08 ` Maxime Coquelin
1 sibling, 0 replies; 20+ messages in thread
From: Cindy Lu @ 2023-09-25 3:19 UTC (permalink / raw)
To: Jason Wang
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 3/4] vduse: update the vq_info in ioctl
2023-09-12 7:39 ` Jason Wang
@ 2023-09-25 4:15 ` Cindy Lu
2023-09-29 9:08 ` Maxime Coquelin
2023-09-29 9:12 ` Maxime Coquelin
1 sibling, 1 reply; 20+ messages in thread
From: Cindy Lu @ 2023-09-25 4:15 UTC (permalink / raw)
To: Jason Wang
Cc: mst, maxime.coquelin, xieyongji, kvm, linux-kernel,
virtualization, netdev, stable
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
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 3/4] vduse: update the vq_info in ioctl
2023-09-25 4:15 ` Cindy Lu
@ 2023-09-29 9:08 ` Maxime Coquelin
0 siblings, 0 replies; 20+ messages in thread
From: Maxime Coquelin @ 2023-09-29 9:08 UTC (permalink / raw)
To: Cindy Lu, Jason Wang
Cc: mst, xieyongji, kvm, linux-kernel, virtualization, netdev, stable
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
>>>
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
2023-09-25 2:57 ` Jason Wang
2023-09-25 3:19 ` Cindy Lu
@ 2023-09-29 9:08 ` Maxime Coquelin
2023-10-09 3:00 ` Cindy Lu
1 sibling, 1 reply; 20+ messages in thread
From: Maxime Coquelin @ 2023-09-29 9:08 UTC (permalink / raw)
To: Jason Wang, Cindy Lu
Cc: mst, xieyongji, kvm, linux-kernel, virtualization, netdev, stable
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
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 3/4] vduse: update the vq_info in ioctl
2023-09-12 7:39 ` Jason Wang
2023-09-25 4:15 ` Cindy Lu
@ 2023-09-29 9:12 ` Maxime Coquelin
2023-10-08 5:17 ` Jason Wang
1 sibling, 1 reply; 20+ messages in thread
From: Maxime Coquelin @ 2023-09-29 9:12 UTC (permalink / raw)
To: Jason Wang, Cindy Lu
Cc: mst, xieyongji, kvm, linux-kernel, virtualization, netdev, stable
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
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 3/4] vduse: update the vq_info in ioctl
2023-09-29 9:12 ` Maxime Coquelin
@ 2023-10-08 5:17 ` Jason Wang
2023-10-08 12:55 ` Cindy Lu
0 siblings, 1 reply; 20+ messages in thread
From: Jason Wang @ 2023-10-08 5:17 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Cindy Lu, mst, xieyongji, kvm, linux-kernel, virtualization,
netdev, stable
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
> >>
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 3/4] vduse: update the vq_info in ioctl
2023-10-08 5:17 ` Jason Wang
@ 2023-10-08 12:55 ` Cindy Lu
0 siblings, 0 replies; 20+ messages in thread
From: Cindy Lu @ 2023-10-08 12:55 UTC (permalink / raw)
To: Jason Wang
Cc: Maxime Coquelin, mst, xieyongji, kvm, linux-kernel,
virtualization, netdev
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
> > >>
> > >
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
2023-09-29 9:08 ` Maxime Coquelin
@ 2023-10-09 3:00 ` Cindy Lu
0 siblings, 0 replies; 20+ messages in thread
From: Cindy Lu @ 2023-10-09 3:00 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Jason Wang, mst, xieyongji, kvm, linux-kernel, virtualization,
netdev, stable
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
> >>>>
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-10-09 3:01 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12 3:00 [RFC v2 0/4] Support reconnection in vduse Cindy Lu
2023-09-12 3:00 ` [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection Cindy Lu
2023-09-18 8:41 ` Jason Wang
2023-09-21 14:06 ` Cindy Lu
2023-09-12 3:00 ` [RFC v2 2/4] vduse: Add file operation for mmap Cindy Lu
2023-09-18 8:46 ` Jason Wang
2023-09-12 3:00 ` [RFC v2 3/4] vduse: update the vq_info in ioctl Cindy Lu
2023-09-12 7:39 ` Jason Wang
2023-09-25 4:15 ` Cindy Lu
2023-09-29 9:08 ` Maxime Coquelin
2023-09-29 9:12 ` Maxime Coquelin
2023-10-08 5:17 ` Jason Wang
2023-10-08 12:55 ` Cindy Lu
2023-09-12 3:00 ` [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO Cindy Lu
2023-09-18 8:48 ` Jason Wang
2023-09-21 14:01 ` Cindy Lu
2023-09-25 2:57 ` Jason Wang
2023-09-25 3:19 ` Cindy Lu
2023-09-29 9:08 ` Maxime Coquelin
2023-10-09 3:00 ` Cindy Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).