[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6c4cd924-0d44-582e-13a4-791f38d10fe8@redhat.com>
Date: Fri, 29 Sep 2023 11:08:08 +0200
From: Maxime Coquelin <maxime.coquelin@...hat.com>
To: Jason Wang <jasowang@...hat.com>, Cindy Lu <lulu@...hat.com>
Cc: mst@...hat.com, xieyongji@...edance.com, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org,
virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [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@...hat.com> wrote:
>>
>> On Mon, Sep 18, 2023 at 4:49 PM Jason Wang <jasowang@...hat.com> wrote:
>>>
>>> On Tue, Sep 12, 2023 at 11:01 AM Cindy Lu <lulu@...hat.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@...hat.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
>>>>
>>>
>>
>
Powered by blists - more mailing lists