Here is the reconnect support in vduse,
The kernel will allocate pages for reconnection
userspace needs to use ioctl VDUSE_GET_RECONNECT_INFO to
get the mmap related infomation and then map these pages
to userspace.
The kernel and userspace will use these pages to sync
the reconnect information
Tested in vduse + dpdk test-pmd
Cindy Lu (4):
vduse: Add function to get/free the pages for reconnection
vduse: Add file operation for mmap
vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
vduse: update the vq_info in ioctl
drivers/vdpa/vdpa_user/vduse_dev.c | 175 +++++++++++++++++++++++++++++
include/uapi/linux/vduse.h | 43 +++++++
2 files changed, 218 insertions(+)
--
2.34.3
Add the operation for mmap, The user space APP will
use this function to map the pages to userspace
Signed-off-by: Cindy Lu <[email protected]>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 66 ++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 565126a9ab01..05e72d752fb6 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1380,6 +1380,70 @@ 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;
+
+ /* index 0 page reserved for vduse status*/
+ if (index == 0) {
+ info = &dev->reconnect_status;
+ } else {
+ /* index 1+vq_number page reserved for vduse vqs*/
+ vq = &dev->vqs[index - 1];
+ info = &vq->reconnect_info;
+ }
+ if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
+ PFN_DOWN(virt_to_phys((void *)info->vaddr)),
+ 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 > dev->vq_num + 1)
+ return -EINVAL;
+
+ /* index 0 page reserved for vduse status*/
+ if (index == 0) {
+ info = &dev->reconnect_status;
+ } else {
+ /* index 1+vq_number page reserved for vduse vqs*/
+ vq = &dev->vqs[index - 1];
+ info = &vq->reconnect_info;
+ }
+ if (info->vaddr == 0)
+ return -ENOTSUPP;
+
+ if (virt_to_phys((void *)info->vaddr) & (PAGE_SIZE - 1))
+ return -EINVAL;
+ if (vma->vm_end - vma->vm_start != info->size)
+ return -EOPNOTSUPP;
+
+ vm_flags_set(vma, VM_IO | VM_DONTDUMP);
+ vma->vm_ops = &vduse_vm_ops;
+
+ return 0;
+}
+
static int vduse_dev_open(struct inode *inode, struct file *file)
{
int ret;
@@ -1412,6 +1476,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_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.
Add struct vhost_reconnect_data/vhost_reconnect_vring for sync the
information in reconnection
Signed-off-by: Cindy Lu <[email protected]>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++
include/uapi/linux/vduse.h | 43 ++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 05e72d752fb6..0f15e7ac716b 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1347,6 +1347,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 11bd48c72c6c..5ccac535fba6 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -350,4 +350,47 @@ struct vduse_dev_response {
};
};
+/**
+ * struct vhost_reconnect_data - saved the reconnect info for device
+ * @reconnect_time: reconnect time for this device. userspace APP needs to do ++
+ * while reconnecting
+ * @version; version for userspace APP
+ * @features; Device features negotiated in the last reconnect.
+ * @status; Device status in last reconnect
+ * @nr_vrings; number of vqs
+ */
+
+struct vhost_reconnect_data {
+ __u32 reconnect_time;
+ __u32 version;
+ __u64 features;
+ __u8 status;
+ __u32 nr_vrings;
+};
+
+/**
+ * struct vhost_reconnect_vring -saved the reconnect info for vqs
+ * @last_avail_idx: device available index
+ * @avail_wrap_counter: Driver ring wrap counter
+ */
+struct vhost_reconnect_vring {
+ __u16 last_avail_idx;
+ __u16 avail_wrap_counter;
+};
+
+/**
+ * struct vduse_reconnect_mmap_info
+ * @size: mapping memory size, here we use page_size
+ * @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
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 <[email protected]>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0f15e7ac716b..42e7a90ab74c 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1213,6 +1213,9 @@ 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;
+ struct vhost_reconnect_data *dev_reconnect;
ret = -EFAULT;
if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
@@ -1244,6 +1247,19 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
vq_info.ready = vq->ready;
+ area = &dev->reconnect_status;
+ dev_reconnect = (struct vhost_reconnect_data *)area->vaddr;
+
+ 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) &&
+ (dev_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;
--
2.34.3
On Wed, Oct 11, 2023 at 2:42 PM Cindy Lu <[email protected]> wrote:
>
> Here is the reconnect support in vduse,
>
> The kernel will allocate pages for reconnection
> userspace needs to use ioctl VDUSE_GET_RECONNECT_INFO to
> get the mmap related infomation and then map these pages
> to userspace.
> The kernel and userspace will use these pages to sync
> the reconnect information
>
> Tested in vduse + dpdk test-pmd
I would like to see the DPDK part as a reference. Would you mind
giving me a pointer?
Thanks
>
> Cindy Lu (4):
> vduse: Add function to get/free the pages for reconnection
> vduse: Add file operation for mmap
> vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
> vduse: update the vq_info in ioctl
>
> drivers/vdpa/vdpa_user/vduse_dev.c | 175 +++++++++++++++++++++++++++++
> include/uapi/linux/vduse.h | 43 +++++++
> 2 files changed, 218 insertions(+)
>
> --
> 2.34.3
>
On Tue, Oct 17, 2023 at 10:44 AM Jason Wang <[email protected]> wrote:
>
> On Wed, Oct 11, 2023 at 2:42 PM Cindy Lu <[email protected]> wrote:
> >
> > Here is the reconnect support in vduse,
> >
> > The kernel will allocate pages for reconnection
> > userspace needs to use ioctl VDUSE_GET_RECONNECT_INFO to
> > get the mmap related infomation and then map these pages
> > to userspace.
> > The kernel and userspace will use these pages to sync
> > the reconnect information
> >
> > Tested in vduse + dpdk test-pmd
>
> I would like to see the DPDK part as a reference. Would you mind
> giving me a pointer?
>
> Thanks
>
sure, Will send the dpdk patch soon
thanks
cindy
> >
> > Cindy Lu (4):
> > vduse: Add function to get/free the pages for reconnection
> > vduse: Add file operation for mmap
> > vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
> > vduse: update the vq_info in ioctl
> >
> > drivers/vdpa/vdpa_user/vduse_dev.c | 175 +++++++++++++++++++++++++++++
> > include/uapi/linux/vduse.h | 43 +++++++
> > 2 files changed, 218 insertions(+)
> >
> > --
> > 2.34.3
> >
>
On Wed, Oct 11, 2023 at 2:42 PM Cindy Lu <[email protected]> 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 <[email protected]>
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 66 ++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 565126a9ab01..05e72d752fb6 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1380,6 +1380,70 @@ 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;
> +
> + /* index 0 page reserved for vduse status*/
> + if (index == 0) {
> + info = &dev->reconnect_status;
> + } else {
> + /* index 1+vq_number page reserved for vduse vqs*/
> + vq = &dev->vqs[index - 1];
> + info = &vq->reconnect_info;
> + }
> + if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
> + PFN_DOWN(virt_to_phys((void *)info->vaddr)),
> + 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 > dev->vq_num + 1)
> + return -EINVAL;
> +
> + /* index 0 page reserved for vduse status*/
> + if (index == 0) {
> + info = &dev->reconnect_status;
> + } else {
> + /* index 1+vq_number page reserved for vduse vqs*/
> + vq = &dev->vqs[index - 1];
> + info = &vq->reconnect_info;
> + }
> + if (info->vaddr == 0)
> + return -ENOTSUPP;
Under which condition could we meet this?
> +
> + if (virt_to_phys((void *)info->vaddr) & (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_DONTDUMP);
I still don't understand the flags here. Or at least it requires some
comment to explain why they are needed here.
Please refer to the kernel sources which explains their use cases.
Thanks
> + vma->vm_ops = &vduse_vm_ops;
> +
> + return 0;
> +}
> +
> static int vduse_dev_open(struct inode *inode, struct file *file)
> {
> int ret;
> @@ -1412,6 +1476,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
>
On Wed, Oct 11, 2023 at 2:42 PM Cindy Lu <[email protected]> wrote:
>
> In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
I'm not sure why this is needed, the structure that mmaped to
userspace should belong to uAPI then userspace can do versions there?
> and The number of mapping memory pages from the kernel.
Userspace knows how many vq at most, why is this still needed?
> The userspace
> App can use this information to map the pages.
>
> Add struct vhost_reconnect_data/vhost_reconnect_vring for sync the
> information in reconnection
>
> Signed-off-by: Cindy Lu <[email protected]>
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++++
> include/uapi/linux/vduse.h | 43 ++++++++++++++++++++++++++++++
> 2 files changed, 58 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 05e72d752fb6..0f15e7ac716b 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1347,6 +1347,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 11bd48c72c6c..5ccac535fba6 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -350,4 +350,47 @@ struct vduse_dev_response {
> };
> };
>
> +/**
> + * struct vhost_reconnect_data - saved the reconnect info for device
> + * @reconnect_time: reconnect time for this device. userspace APP needs to do ++
> + * while reconnecting
This commnet needs tweaking as I don't think "++" is good English. And
you need to explain why we need this.
> + * @version; version for userspace APP
It's the version of uAPI not the userspace APP. And in order to be
able for a correct bootstrap, this needs to be the first field instead
of the second.
> + * @features; Device features negotiated in the last reconnect.
> + * @status; Device status in last reconnect
I wonder if we need just more than this, for example the number of
active queues, or this is what does @nr_vrings mean?
> + * @nr_vrings; number of vqs
> + */
> +
> +struct vhost_reconnect_data {
> + __u32 reconnect_time;
> + __u32 version;
> + __u64 features;
> + __u8 status;
> + __u32 nr_vrings;
> +};
> +
> +/**
> + * struct vhost_reconnect_vring -saved the reconnect info for vqs
> + * @last_avail_idx: device available index
> + * @avail_wrap_counter: Driver ring wrap counter
> + */
> +struct vhost_reconnect_vring {
> + __u16 last_avail_idx;
> + __u16 avail_wrap_counter;
> +};
Do we need the last_used_idx and last_used_wrap_counter? If not,
please document why.
> +
> +/**
> + * struct vduse_reconnect_mmap_info
> + * @size: mapping memory size, here we use page_size
> + * @max_index: the number of pages allocated in kernel,just
> + * use for check
Did you mean "sanity check"?
Thanks
> + */
> +
> +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
>
On Wed, Oct 18, 2023 at 11:23 AM Jason Wang <[email protected]> wrote:
>
> On Wed, Oct 11, 2023 at 2:42 PM Cindy Lu <[email protected]> wrote:
> >
> > In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
>
> I'm not sure why this is needed, the structure that mmaped to
> userspace should belong to uAPI then userspace can do versions there?
Ok, so I think this makes sense, it can help to avoid duplicating
versions in each mmaped pages.
Thanks
On Wed, Oct 11, 2023 at 2:42 PM Cindy Lu <[email protected]> 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 <[email protected]>
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 0f15e7ac716b..42e7a90ab74c 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1213,6 +1213,9 @@ 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;
> + struct vhost_reconnect_data *dev_reconnect;
>
> ret = -EFAULT;
> if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
> @@ -1244,6 +1247,19 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>
> vq_info.ready = vq->ready;
>
> + area = &dev->reconnect_status;
> + dev_reconnect = (struct vhost_reconnect_data *)area->vaddr;
> +
> + 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) &&
> + (dev_reconnect->reconnect_time != 0)) {
> + vq_info.split.avail_index =
> + vq_reconnect->last_avail_idx;
> + }
This may work only for split, how about packed? And I wonder when such
synchronization is not done at set_vq_state() but get_vq_info()?
Thanks
> +
> ret = -EFAULT;
> if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
> break;
> --
> 2.34.3
>
On Tue, Oct 17, 2023 at 10:52 AM Cindy Lu <[email protected]> wrote:
>
> On Tue, Oct 17, 2023 at 10:44 AM Jason Wang <[email protected]> wrote:
> >
> > On Wed, Oct 11, 2023 at 2:42 PM Cindy Lu <[email protected]> wrote:
> > >
> > > Here is the reconnect support in vduse,
> > >
> > > The kernel will allocate pages for reconnection
> > > userspace needs to use ioctl VDUSE_GET_RECONNECT_INFO to
> > > get the mmap related infomation and then map these pages
> > > to userspace.
> > > The kernel and userspace will use these pages to sync
> > > the reconnect information
> > >
> > > Tested in vduse + dpdk test-pmd
> >
> > I would like to see the DPDK part as a reference. Would you mind
> > giving me a pointer?
> >
> > Thanks
> >
> sure, Will send the dpdk patch soon
> thanks
> cindy
Btw, I would expect to add the protocol into the VDUSE doc to describe
the exact step for doing reconnection.
Thanks
> > >
> > > Cindy Lu (4):
> > > vduse: Add function to get/free the pages for reconnection
> > > vduse: Add file operation for mmap
> > > vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
> > > vduse: update the vq_info in ioctl
> > >
> > > drivers/vdpa/vdpa_user/vduse_dev.c | 175 +++++++++++++++++++++++++++++
> > > include/uapi/linux/vduse.h | 43 +++++++
> > > 2 files changed, 218 insertions(+)
> > >
> > > --
> > > 2.34.3
> > >
> >
>