Jason Wang
2023-Sep-25 02:57 UTC
[RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
On Thu, Sep 21, 2023 at 10:07?PM Cindy Lu <lulu at redhat.com> wrote:> > On Mon, Sep 18, 2023 at 4:49?PM Jason Wang <jasowang at redhat.com> wrote: > > > > On Tue, Sep 12, 2023 at 11:01?AM Cindy Lu <lulu at 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 at 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 > > > > > >
Maxime Coquelin
2023-Sep-29 09:08 UTC
[RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
On 9/25/23 04:57, Jason Wang wrote:> On Thu, Sep 21, 2023 at 10:07?PM Cindy Lu <lulu at redhat.com> wrote: >> >> On Mon, Sep 18, 2023 at 4:49?PM Jason Wang <jasowang at redhat.com> wrote: >>> >>> On Tue, Sep 12, 2023 at 11:01?AM Cindy Lu <lulu at 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 at 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 >>>> >>> >> >