[PATCH v2 3/5] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

Cindy Lu posted 5 patches 2 years ago
There is a newer version of this series
[PATCH v2 3/5] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
Posted by Cindy Lu 2 years ago
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 <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++
 include/uapi/linux/vduse.h         | 50 ++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index ccb30e98bab5..d0fe9a7e86ab 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1343,6 +1343,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..c0b7133aebfd 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -3,6 +3,7 @@
 #define _UAPI_VDUSE_H_
 
 #include <linux/types.h>
+#include <linux/virtio_net.h>
 
 #define VDUSE_BASE	0x81
 
@@ -350,4 +351,53 @@ struct vduse_dev_response {
 	};
 };
 
+/**
+ * struct vhost_reconnect_data - saved the reconnect info for device
+ * @version; version for userspace APP
+ * @reconnected: indetify if this is reconnected. userspace APP needs set this bit to 1
+ *               while reconnecting.kernel will use this bit to indetify if this is
+ *               reconnect
+ * @features; Device features negotiated in the last connect.
+ * @status; Device status in last reconnect
+ * @nr_vrings; number of active vqs in last connect
+ * @struct virtio_net_config config; the config in last connect
+ */
+
+struct vhost_reconnect_data {
+	__u32 version;
+	__u32 reconnected;
+	__u64 features;
+	__u8 status;
+	__u32 nr_vrings;
+	struct virtio_net_config config;
+};
+
+/**
+ * struct vhost_reconnect_vring -saved the reconnect info for vqs
+ * when use split mode will only use last_avail_idx
+ * when use packed mode will use both last_avail_idx and avail_wrap_counter
+ * userspace App
+ * @last_avail_idx: device last 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 sanity check
+ */
+
+struct vduse_reconnect_mmap_info {
+	__u32 size;
+	__u32 max_index;
+};
+
+#define VDUSE_GET_RECONNECT_INFO \
+	_IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
+
 #endif /* _UAPI_VDUSE_H_ */
-- 
2.34.3
Re: [PATCH v2 3/5] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
Posted by Jason Wang 2 years ago
On Tue, Nov 21, 2023 at 3:31 PM 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.
>
> Add struct vhost_reconnect_data/vhost_reconnect_vring for sync the
> information in reconnection
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++
>  include/uapi/linux/vduse.h         | 50 ++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index ccb30e98bab5..d0fe9a7e86ab 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1343,6 +1343,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;

It looks to be both PAGE_SIZE and vq_num is the well knowledge that
doesn't require a query?

> +
> +               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..c0b7133aebfd 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -3,6 +3,7 @@
>  #define _UAPI_VDUSE_H_
>
>  #include <linux/types.h>
> +#include <linux/virtio_net.h>
>
>  #define VDUSE_BASE     0x81
>
> @@ -350,4 +351,53 @@ struct vduse_dev_response {
>         };
>  };
>
> +/**
> + * struct vhost_reconnect_data - saved the reconnect info for device
> + * @version; version for userspace APP
> + * @reconnected: indetify if this is reconnected. userspace APP needs set this bit to 1
> + *               while reconnecting.kernel will use this bit to indetify if this is
> + *               reconnect

Typos.

> + * @features; Device features negotiated in the last connect.
> + * @status; Device status in last reconnect
> + * @nr_vrings; number of active vqs in last connect

What's the meaning of "active vqs"? Is it the #active_queue_pairs? If
yes, couldn't we get it from the virtio_net_config?

> + * @struct virtio_net_config config; the config in last connect
> + */
> +
> +struct vhost_reconnect_data {
> +       __u32 version;
> +       __u32 reconnected;
> +       __u64 features;
> +       __u8 status;
> +       __u32 nr_vrings;
> +       struct virtio_net_config config;

This seems like a layer violation. The fields here needs to be type
agnostic or we should introduce a new device specific area with a
union.

Or can we simply invent VDUSE_DEV_GET_CONFIG() to do this?

> +};
> +
> +/**
> + * struct vhost_reconnect_vring -saved the reconnect info for vqs
> + * when use split mode will only use last_avail_idx
> + * when use packed mode will use both last_avail_idx and avail_wrap_counter

How about last_used_idx and last_used_wrap_counter.

Btw, vDPA or vhost-vDPA has a good uAPI for this, can we reuse that?

Thanks

> + * userspace App
> + * @last_avail_idx: device last 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 sanity check
> + */
> +
> +struct vduse_reconnect_mmap_info {
> +       __u32 size;
> +       __u32 max_index;
> +};
> +
> +#define VDUSE_GET_RECONNECT_INFO \
> +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> +
>  #endif /* _UAPI_VDUSE_H_ */
> --
> 2.34.3
>
Re: [PATCH v2 3/5] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
Posted by Cindy Lu 2 years ago
On Wed, Nov 22, 2023 at 2:29 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Nov 21, 2023 at 3:31 PM 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.
> >
> > Add struct vhost_reconnect_data/vhost_reconnect_vring for sync the
> > information in reconnection
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++
> >  include/uapi/linux/vduse.h         | 50 ++++++++++++++++++++++++++++++
> >  2 files changed, 65 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index ccb30e98bab5..d0fe9a7e86ab 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1343,6 +1343,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;
>
> It looks to be both PAGE_SIZE and vq_num is the well knowledge that
> doesn't require a query?
sure we can remove this IOCTL
>
> > +
> > +               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..c0b7133aebfd 100644
> > --- a/include/uapi/linux/vduse.h
> > +++ b/include/uapi/linux/vduse.h
> > @@ -3,6 +3,7 @@
> >  #define _UAPI_VDUSE_H_
> >
> >  #include <linux/types.h>
> > +#include <linux/virtio_net.h>
> >
> >  #define VDUSE_BASE     0x81
> >
> > @@ -350,4 +351,53 @@ struct vduse_dev_response {
> >         };
> >  };
> >
> > +/**
> > + * struct vhost_reconnect_data - saved the reconnect info for device
> > + * @version; version for userspace APP
> > + * @reconnected: indetify if this is reconnected. userspace APP needs set this bit to 1
> > + *               while reconnecting.kernel will use this bit to indetify if this is
> > + *               reconnect
>
> Typos.
>
will fix this
> > + * @features; Device features negotiated in the last connect.
> > + * @status; Device status in last reconnect
> > + * @nr_vrings; number of active vqs in last connect
>
> What's the meaning of "active vqs"? Is it the #active_queue_pairs? If
> yes, couldn't we get it from the virtio_net_config?
>

> > + * @struct virtio_net_config config; the config in last connect
> > + */
> > +
> > +struct vhost_reconnect_data {
> > +       __u32 version;
> > +       __u32 reconnected;
> > +       __u64 features;
> > +       __u8 status;
> > +       __u32 nr_vrings;
> > +       struct virtio_net_config config;
>
> This seems like a layer violation. The fields here needs to be type
> agnostic or we should introduce a new device specific area with a
> union.
>
> Or can we simply invent VDUSE_DEV_GET_CONFIG() to do this?
>
will remove virtio_net_config here
> her
> > +
> > +/**
> > + * struct vhost_reconnect_vring -saved the reconnect info for vqs
> > + * when use split mode will only use last_avail_idx
> > + * when use packed mode will use both last_avail_idx and avail_wrap_counter
>
> How about last_used_idx and last_used_wrap_counter.
>
> Btw, vDPA or vhost-vDPA has a good uAPI for this, can we reuse that?
>
I didn't get here, Do you mean set_vq_state/get_vq_state ?
Thanks
Cindy
> Thanks
>
> > + * userspace App
> > + * @last_avail_idx: device last 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 sanity 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
> >
>