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(+)
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;
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html/#opt...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [RFC v2 1/4] vduse: Add function to get/free the pages for reconnection Link: https://lore.kernel.org/stable/20230912030008.3599514-2-lulu%40redhat.com
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
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
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)
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
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_ */
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
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
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
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
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
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_ */
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
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
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
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
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
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
linux-stable-mirror@lists.linaro.org
-
Cindy Lu -
Jason Wang -
kernel test robot -
Maxime Coquelin