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(-)
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
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 > >
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 > > > > >
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 > > > > > > > > >
© 2016 - 2024 Red Hat, Inc.