1. Patchew
  2. QEMU
  3. View series

[PATCH 0/7] Enable vdpa net migration with features depending on CVQ

Eugenio Pérez posted 7 patches 9 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230728172028.2074052-1-eperezma@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
include/hw/virtio/vhost-vdpa.h |  9 +++++
hw/virtio/vhost-vdpa.c         | 33 ++++++++++++----
net/vhost-vdpa.c               | 69 ++++++++++++++++++++++++++--------
hw/virtio/trace-events         |  2 +-
4 files changed, 89 insertions(+), 24 deletions(-)
[PATCH 0/7] Enable vdpa net migration with features depending on CVQ
Posted by Eugenio Pérez 9 months, 1 week ago 
At this moment the migration of net features that depends on CVQ is not
possible, as there is no reliable way to restore the device state like mac
address, number of enabled queues, etc to the destination.  This is mainly
caused because the device must only read CVQ, and process all the commands
before resuming the dataplane.

This series lift that requirement, sending the VHOST_VDPA_SET_VRING_ENABLE
ioctl for dataplane vqs only after the device has processed all commands.
---
From FRC:
* Enable vqs early in case CVQ cannot be shadowed.

Eugenio Pérez (7):
  vdpa: export vhost_vdpa_set_vring_ready
  vdpa: add should_enable op
  vdpa: use virtio_ops->should_enable at vhost_vdpa_set_vrings_ready
  vdpa: add stub vhost_vdpa_should_enable
  vdpa: delay enable of data vqs
  vdpa: enable cvq svq if data vq are shadowed
  vdpa: remove net cvq migration blocker

 include/hw/virtio/vhost-vdpa.h |  9 +++++
 hw/virtio/vhost-vdpa.c         | 33 ++++++++++++----
 net/vhost-vdpa.c               | 69 ++++++++++++++++++++++++++--------
 hw/virtio/trace-events         |  2 +-
 4 files changed, 89 insertions(+), 24 deletions(-)

-- 
2.39.3
Re: [PATCH 0/7] Enable vdpa net migration with features depending on CVQ
Posted by Jason Wang 9 months ago 
On Sat, Jul 29, 2023 at 1:20 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> At this moment the migration of net features that depends on CVQ is not
> possible, as there is no reliable way to restore the device state like mac
> address, number of enabled queues, etc to the destination.  This is mainly
> caused because the device must only read CVQ, and process all the commands
> before resuming the dataplane.
>
> This series lift that requirement, sending the VHOST_VDPA_SET_VRING_ENABLE
> ioctl for dataplane vqs only after the device has processed all commands.

I think it's better to explain (that is what I don't understand) why
we can not simply reorder vhost_net_start_one() in vhost_net_start()?

    for (i = 0; i < nvhosts; i++) {
        if (i < data_queue_pairs) {
            peer = qemu_get_peer(ncs, i);
        } else {
            peer = qemu_get_peer(ncs, n->max_queue_pairs);
        }

        if (peer->vring_enable) {
            /* restore vring enable state */
            r = vhost_set_vring_enable(peer, peer->vring_enable);

            if (r < 0) {
                goto err_start;
            }
        }

=>      r = vhost_net_start_one(get_vhost_net(peer), dev);
        if (r < 0) {
            goto err_start;
        }
    }

Can we simply start cvq first here?

Thanks

> ---
> From FRC:
> * Enable vqs early in case CVQ cannot be shadowed.
>
> Eugenio Pérez (7):
>   vdpa: export vhost_vdpa_set_vring_ready
>   vdpa: add should_enable op
>   vdpa: use virtio_ops->should_enable at vhost_vdpa_set_vrings_ready
>   vdpa: add stub vhost_vdpa_should_enable
>   vdpa: delay enable of data vqs
>   vdpa: enable cvq svq if data vq are shadowed
>   vdpa: remove net cvq migration blocker
>
>  include/hw/virtio/vhost-vdpa.h |  9 +++++
>  hw/virtio/vhost-vdpa.c         | 33 ++++++++++++----
>  net/vhost-vdpa.c               | 69 ++++++++++++++++++++++++++--------
>  hw/virtio/trace-events         |  2 +-
>  4 files changed, 89 insertions(+), 24 deletions(-)
>
> --
> 2.39.3
>
>
Re: [PATCH 0/7] Enable vdpa net migration with features depending on CVQ
Posted by Eugenio Perez Martin 9 months ago 
On Mon, Jul 31, 2023 at 8:41 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Sat, Jul 29, 2023 at 1:20 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > At this moment the migration of net features that depends on CVQ is not
> > possible, as there is no reliable way to restore the device state like mac
> > address, number of enabled queues, etc to the destination.  This is mainly
> > caused because the device must only read CVQ, and process all the commands
> > before resuming the dataplane.
> >
> > This series lift that requirement, sending the VHOST_VDPA_SET_VRING_ENABLE
> > ioctl for dataplane vqs only after the device has processed all commands.
>
> I think it's better to explain (that is what I don't understand) why
> we can not simply reorder vhost_net_start_one() in vhost_net_start()?
>
>     for (i = 0; i < nvhosts; i++) {
>         if (i < data_queue_pairs) {
>             peer = qemu_get_peer(ncs, i);
>         } else {
>             peer = qemu_get_peer(ncs, n->max_queue_pairs);
>         }
>
>         if (peer->vring_enable) {
>             /* restore vring enable state */
>             r = vhost_set_vring_enable(peer, peer->vring_enable);
>
>             if (r < 0) {
>                 goto err_start;
>             }
>         }
>
> =>      r = vhost_net_start_one(get_vhost_net(peer), dev);
>         if (r < 0) {
>             goto err_start;
>         }
>     }
>
> Can we simply start cvq first here?
>

Well the current order is:
* set dev features (conditioned by
* Configure all vq addresses
* Configure all vq size
...
* Enable cvq
* DRIVER_OK
* Enable all the rest of the queues.

If we just start CVQ first, we need to modify vhost_vdpa_set_features
as minimum. A lot of code that depends on vdev->vq_index{,_end} may be
affected.

Also, I'm not sure if all the devices will support configure address,
vq size, etc after DRIVER_OK.

> Thanks
>
> > ---
> > From FRC:
> > * Enable vqs early in case CVQ cannot be shadowed.
> >
> > Eugenio Pérez (7):
> >   vdpa: export vhost_vdpa_set_vring_ready
> >   vdpa: add should_enable op
> >   vdpa: use virtio_ops->should_enable at vhost_vdpa_set_vrings_ready
> >   vdpa: add stub vhost_vdpa_should_enable
> >   vdpa: delay enable of data vqs
> >   vdpa: enable cvq svq if data vq are shadowed
> >   vdpa: remove net cvq migration blocker
> >
> >  include/hw/virtio/vhost-vdpa.h |  9 +++++
> >  hw/virtio/vhost-vdpa.c         | 33 ++++++++++++----
> >  net/vhost-vdpa.c               | 69 ++++++++++++++++++++++++++--------
> >  hw/virtio/trace-events         |  2 +-
> >  4 files changed, 89 insertions(+), 24 deletions(-)
> >
> > --
> > 2.39.3
> >
> >
>
Re: [PATCH 0/7] Enable vdpa net migration with features depending on CVQ
Posted by Jason Wang 9 months ago 
On Mon, Jul 31, 2023 at 6:15 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Mon, Jul 31, 2023 at 8:41 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Sat, Jul 29, 2023 at 1:20 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > >
> > > At this moment the migration of net features that depends on CVQ is not
> > > possible, as there is no reliable way to restore the device state like mac
> > > address, number of enabled queues, etc to the destination.  This is mainly
> > > caused because the device must only read CVQ, and process all the commands
> > > before resuming the dataplane.
> > >
> > > This series lift that requirement, sending the VHOST_VDPA_SET_VRING_ENABLE
> > > ioctl for dataplane vqs only after the device has processed all commands.
> >
> > I think it's better to explain (that is what I don't understand) why
> > we can not simply reorder vhost_net_start_one() in vhost_net_start()?
> >
> >     for (i = 0; i < nvhosts; i++) {
> >         if (i < data_queue_pairs) {
> >             peer = qemu_get_peer(ncs, i);
> >         } else {
> >             peer = qemu_get_peer(ncs, n->max_queue_pairs);
> >         }
> >
> >         if (peer->vring_enable) {
> >             /* restore vring enable state */
> >             r = vhost_set_vring_enable(peer, peer->vring_enable);
> >
> >             if (r < 0) {
> >                 goto err_start;
> >             }
> >         }
> >
> > =>      r = vhost_net_start_one(get_vhost_net(peer), dev);
> >         if (r < 0) {
> >             goto err_start;
> >         }
> >     }
> >
> > Can we simply start cvq first here?
> >
>
> Well the current order is:
> * set dev features (conditioned by
> * Configure all vq addresses
> * Configure all vq size
> ...
> * Enable cvq
> * DRIVER_OK
> * Enable all the rest of the queues.
>
> If we just start CVQ first, we need to modify vhost_vdpa_set_features
> as minimum. A lot of code that depends on vdev->vq_index{,_end} may be
> affected.
>
> Also, I'm not sure if all the devices will support configure address,
> vq size, etc after DRIVER_OK.

Ok, so basically what I meant is to seek a way to refactor
vhost_net_start() instead of introducing new ops (e.g introducing
virtio ops in vhost seems a layer violation anyhow).

Can we simply factor VRING_ENABLE out and then we can enable vring in
any order as we want in vhost_net_start()?

Thanks

>
> > Thanks
> >
> > > ---
> > > From FRC:
> > > * Enable vqs early in case CVQ cannot be shadowed.
> > >
> > > Eugenio Pérez (7):
> > >   vdpa: export vhost_vdpa_set_vring_ready
> > >   vdpa: add should_enable op
> > >   vdpa: use virtio_ops->should_enable at vhost_vdpa_set_vrings_ready
> > >   vdpa: add stub vhost_vdpa_should_enable
> > >   vdpa: delay enable of data vqs
> > >   vdpa: enable cvq svq if data vq are shadowed
> > >   vdpa: remove net cvq migration blocker
> > >
> > >  include/hw/virtio/vhost-vdpa.h |  9 +++++
> > >  hw/virtio/vhost-vdpa.c         | 33 ++++++++++++----
> > >  net/vhost-vdpa.c               | 69 ++++++++++++++++++++++++++--------
> > >  hw/virtio/trace-events         |  2 +-
> > >  4 files changed, 89 insertions(+), 24 deletions(-)
> > >
> > > --
> > > 2.39.3
> > >
> > >
> >
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.