Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
MIME-Version: 1.0
References: <20230912030008.3599514-1-lulu@redhat.com> <20230912030008.3599514-5-lulu@redhat.com>
 <CACGkMEtCYG8-Pt+V-OOwUV7fYFp_cnxU68Moisfxju9veJ-=qw@mail.gmail.com>
 <CACLfguW3NS_4+YhqTtGqvQb70mVazGVfheryHx4aCBn+=Skf9w@mail.gmail.com>
 <CACGkMEt-m9bOh9YnqLw0So5wqbZ69D0XRVBbfG73Oh7Q8qTJsQ@mail.gmail.com> <6c4cd924-0d44-582e-13a4-791f38d10fe8@redhat.com>
In-Reply-To: <6c4cd924-0d44-582e-13a4-791f38d10fe8@redhat.com>
From:   Cindy Lu <lulu@redhat.com>
Date:   Mon, 9 Oct 2023 11:00:30 +0800
Message-ID: <CACLfguVTxZR2U-CFhkFWYFcgvB-6TLcQjUaEvtm+oka2XstVqw@mail.gmail.com>
Subject: Re: [RFC v2 4/4] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
To:     Maxime Coquelin <maxime.coquelin@redhat.com>
Cc:     Jason Wang <jasowang@redhat.com>, mst@redhat.com,
        xieyongji@bytedance.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
        stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Sep 29, 2023 at 5:08=E2=80=AFPM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
>
> On 9/25/23 04:57, Jason Wang wrote:
> > On Thu, Sep 21, 2023 at 10:07=E2=80=AFPM Cindy Lu <lulu@redhat.com> wro=
te:
> >>
> >> On Mon, Sep 18, 2023 at 4:49=E2=80=AFPM Jason Wang <jasowang@redhat.co=
m> wrote:
> >>>
> >>> On Tue, Sep 12, 2023 at 11:01=E2=80=AFAM Cindy Lu <lulu@redhat.com> w=
rote:
> >>>>
> >>>> In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> >>>> and The number of mapping memory pages from the kernel. The userspac=
e
> >>>> 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 =3D 0;
> >>>>                  break;
> >>>>          }
> >>>> +       case VDUSE_GET_RECONNECT_INFO: {
> >>>> +               struct vduse_reconnect_mmap_info info;
> >>>> +
> >>>> +               ret =3D -EFAULT;
> >>>> +               if (copy_from_user(&info, argp, sizeof(info)))
> >>>> +                       break;
> >>>> +
> >>>> +               info.size =3D PAGE_SIZE;
> >>>> +               info.max_index =3D dev->vq_num + 1;
> >>>> +
> >>>> +               if (copy_to_user(argp, &info, sizeof(info)))
> >>>> +                       break;
> >>>> +               ret =3D 0;
> >>>> +               break;
> >>>> +       }
> >>>>          default:
> >>>>                  ret =3D -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 th=
e
> >>> 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 wil=
l
> >> 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
> >>>>
> >>>
> >>
> >
>