| Message ID | 20230912030008.3599514-4-lulu@redhat.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Support reconnection in vduse | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> 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 <lulu@redhat.com> > --- > drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++ > include/uapi/linux/vduse.h | 6 ++++++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > index 2c69f4004a6e..680b23dbdde2 100644 > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > @@ -1221,6 +1221,8 @@ 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; > > ret = -EFAULT; > if (copy_from_user(&vq_info, argp, sizeof(vq_info))) > @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > > vq_info.ready = vq->ready; > > + 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) && > + (vq_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; > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h > index 11bd48c72c6c..d585425803fd 100644 > --- a/include/uapi/linux/vduse.h > +++ b/include/uapi/linux/vduse.h > @@ -350,4 +350,10 @@ struct vduse_dev_response { > }; > }; > > +struct vhost_reconnect_vring { > + __u16 reconnect_time; > + __u16 last_avail_idx; > + _Bool avail_wrap_counter; Please add a comment for each field. And I never saw _Bool is used in uapi before, maybe it's better to pack it with last_avail_idx into a __u32. Btw, do we need to track inflight descriptors as well? Thanks > +}; > + > #endif /* _UAPI_VDUSE_H_ */ > -- > 2.34.3 >
On Tue, Sep 12, 2023 at 3:39 PM Jason Wang <jasowang@redhat.com> wrote: > > On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> 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 <lulu@redhat.com> > > --- > > drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++ > > include/uapi/linux/vduse.h | 6 ++++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > > index 2c69f4004a6e..680b23dbdde2 100644 > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > > @@ -1221,6 +1221,8 @@ 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; > > > > ret = -EFAULT; > > if (copy_from_user(&vq_info, argp, sizeof(vq_info))) > > @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > > > > vq_info.ready = vq->ready; > > > > + 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) && > > + (vq_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; > > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h > > index 11bd48c72c6c..d585425803fd 100644 > > --- a/include/uapi/linux/vduse.h > > +++ b/include/uapi/linux/vduse.h > > @@ -350,4 +350,10 @@ struct vduse_dev_response { > > }; > > }; > > > > +struct vhost_reconnect_vring { > > + __u16 reconnect_time; > > + __u16 last_avail_idx; > > + _Bool avail_wrap_counter; > > Please add a comment for each field. > Sure will do > And I never saw _Bool is used in uapi before, maybe it's better to > pack it with last_avail_idx into a __u32. > Thanks will fix this > Btw, do we need to track inflight descriptors as well? > I will check this Thanks cindy > Thanks > > > +}; > > + > > #endif /* _UAPI_VDUSE_H_ */ > > -- > > 2.34.3 > > >
On 9/25/23 06:15, Cindy Lu wrote: > On Tue, Sep 12, 2023 at 3:39 PM Jason Wang <jasowang@redhat.com> wrote: >> >> On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> 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 <lulu@redhat.com> >>> --- >>> drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++ >>> include/uapi/linux/vduse.h | 6 ++++++ >>> 2 files changed, 19 insertions(+) >>> >>> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c >>> index 2c69f4004a6e..680b23dbdde2 100644 >>> --- a/drivers/vdpa/vdpa_user/vduse_dev.c >>> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c >>> @@ -1221,6 +1221,8 @@ 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; >>> >>> ret = -EFAULT; >>> if (copy_from_user(&vq_info, argp, sizeof(vq_info))) >>> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, >>> >>> vq_info.ready = vq->ready; >>> >>> + 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) && >>> + (vq_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; >>> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h >>> index 11bd48c72c6c..d585425803fd 100644 >>> --- a/include/uapi/linux/vduse.h >>> +++ b/include/uapi/linux/vduse.h >>> @@ -350,4 +350,10 @@ struct vduse_dev_response { >>> }; >>> }; >>> >>> +struct vhost_reconnect_vring { >>> + __u16 reconnect_time; >>> + __u16 last_avail_idx; >>> + _Bool avail_wrap_counter; >> >> Please add a comment for each field. >> > Sure will do > >> And I never saw _Bool is used in uapi before, maybe it's better to >> pack it with last_avail_idx into a __u32. >> > Thanks will fix this >> Btw, do we need to track inflight descriptors as well? >> > I will check this For existing networking implemenation, this is not necessary. But it should be for block devices. Maxime > Thanks > > cindy >> Thanks >> >>> +}; >>> + >>> #endif /* _UAPI_VDUSE_H_ */ >>> -- >>> 2.34.3 >>> >> >
On 9/12/23 09:39, Jason Wang wrote: > On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> 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 <lulu@redhat.com> >> --- >> drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++ >> include/uapi/linux/vduse.h | 6 ++++++ >> 2 files changed, 19 insertions(+) >> >> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c >> index 2c69f4004a6e..680b23dbdde2 100644 >> --- a/drivers/vdpa/vdpa_user/vduse_dev.c >> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c >> @@ -1221,6 +1221,8 @@ 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; >> >> ret = -EFAULT; >> if (copy_from_user(&vq_info, argp, sizeof(vq_info))) >> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, >> >> vq_info.ready = vq->ready; >> >> + 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) && >> + (vq_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; >> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h >> index 11bd48c72c6c..d585425803fd 100644 >> --- a/include/uapi/linux/vduse.h >> +++ b/include/uapi/linux/vduse.h >> @@ -350,4 +350,10 @@ struct vduse_dev_response { >> }; >> }; >> >> +struct vhost_reconnect_vring { >> + __u16 reconnect_time; >> + __u16 last_avail_idx; >> + _Bool avail_wrap_counter; > > Please add a comment for each field. > > And I never saw _Bool is used in uapi before, maybe it's better to > pack it with last_avail_idx into a __u32. Better as two distincts __u16 IMHO. Thanks, Maxime > > Btw, do we need to track inflight descriptors as well? > > Thanks > >> +}; >> + >> #endif /* _UAPI_VDUSE_H_ */ >> -- >> 2.34.3 >> >
On Fri, Sep 29, 2023 at 5:12 PM Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > > > > On 9/12/23 09:39, Jason Wang wrote: > > On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> 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 <lulu@redhat.com> > >> --- > >> drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++ > >> include/uapi/linux/vduse.h | 6 ++++++ > >> 2 files changed, 19 insertions(+) > >> > >> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > >> index 2c69f4004a6e..680b23dbdde2 100644 > >> --- a/drivers/vdpa/vdpa_user/vduse_dev.c > >> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > >> @@ -1221,6 +1221,8 @@ 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; > >> > >> ret = -EFAULT; > >> if (copy_from_user(&vq_info, argp, sizeof(vq_info))) > >> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > >> > >> vq_info.ready = vq->ready; > >> > >> + 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) && > >> + (vq_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; > >> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h > >> index 11bd48c72c6c..d585425803fd 100644 > >> --- a/include/uapi/linux/vduse.h > >> +++ b/include/uapi/linux/vduse.h > >> @@ -350,4 +350,10 @@ struct vduse_dev_response { > >> }; > >> }; > >> > >> +struct vhost_reconnect_vring { > >> + __u16 reconnect_time; > >> + __u16 last_avail_idx; > >> + _Bool avail_wrap_counter; > > > > Please add a comment for each field. > > > > And I never saw _Bool is used in uapi before, maybe it's better to > > pack it with last_avail_idx into a __u32. > > Better as two distincts __u16 IMHO. Fine with me. Thanks > > Thanks, > Maxime > > > > > Btw, do we need to track inflight descriptors as well? > > > > Thanks > > > >> +}; > >> + > >> #endif /* _UAPI_VDUSE_H_ */ > >> -- > >> 2.34.3 > >> > > >
On Sun, Oct 8, 2023 at 1:17 PM Jason Wang <jasowang@redhat.com> wrote: > > On Fri, Sep 29, 2023 at 5:12 PM Maxime Coquelin > <maxime.coquelin@redhat.com> wrote: > > > > > > > > On 9/12/23 09:39, Jason Wang wrote: > > > On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> 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 <lulu@redhat.com> > > >> --- > > >> drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++ > > >> include/uapi/linux/vduse.h | 6 ++++++ > > >> 2 files changed, 19 insertions(+) > > >> > > >> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > > >> index 2c69f4004a6e..680b23dbdde2 100644 > > >> --- a/drivers/vdpa/vdpa_user/vduse_dev.c > > >> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > > >> @@ -1221,6 +1221,8 @@ 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; > > >> > > >> ret = -EFAULT; > > >> if (copy_from_user(&vq_info, argp, sizeof(vq_info))) > > >> @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > > >> > > >> vq_info.ready = vq->ready; > > >> > > >> + 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) && > > >> + (vq_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; > > >> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h > > >> index 11bd48c72c6c..d585425803fd 100644 > > >> --- a/include/uapi/linux/vduse.h > > >> +++ b/include/uapi/linux/vduse.h > > >> @@ -350,4 +350,10 @@ struct vduse_dev_response { > > >> }; > > >> }; > > >> > > >> +struct vhost_reconnect_vring { > > >> + __u16 reconnect_time; > > >> + __u16 last_avail_idx; > > >> + _Bool avail_wrap_counter; > > > > > > Please add a comment for each field. > > > > > > And I never saw _Bool is used in uapi before, maybe it's better to > > > pack it with last_avail_idx into a __u32. > > > > Better as two distincts __u16 IMHO. > > Fine with me. > > Thanks > sure will fix this Thanks Cindy > > > > Thanks, > > Maxime > > > > > > > > Btw, do we need to track inflight descriptors as well? > > > > > > Thanks > > > > > >> +}; > > >> + > > >> #endif /* _UAPI_VDUSE_H_ */ > > >> -- > > >> 2.34.3 > > >> > > > > > >
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c index 2c69f4004a6e..680b23dbdde2 100644 --- a/drivers/vdpa/vdpa_user/vduse_dev.c +++ b/drivers/vdpa/vdpa_user/vduse_dev.c @@ -1221,6 +1221,8 @@ 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; ret = -EFAULT; if (copy_from_user(&vq_info, argp, sizeof(vq_info))) @@ -1252,6 +1254,17 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, vq_info.ready = vq->ready; + 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) && + (vq_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; diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h index 11bd48c72c6c..d585425803fd 100644 --- a/include/uapi/linux/vduse.h +++ b/include/uapi/linux/vduse.h @@ -350,4 +350,10 @@ struct vduse_dev_response { }; }; +struct vhost_reconnect_vring { + __u16 reconnect_time; + __u16 last_avail_idx; + _Bool avail_wrap_counter; +}; + #endif /* _UAPI_VDUSE_H_ */
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 <lulu@redhat.com> --- drivers/vdpa/vdpa_user/vduse_dev.c | 13 +++++++++++++ include/uapi/linux/vduse.h | 6 ++++++ 2 files changed, 19 insertions(+)