Re: [RFC v2 06/13] vhost: delay set_vring_ready after DRIVER_OK

[Jason Wang]

On Fri, Jan 13, 2023 at 1:25 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> To restore the device at the destination of a live migration we send the
> commands through control virtqueue. For a device to read CVQ it must
> have received the DRIVER_OK status bit.

This probably requires the support from the parent driver and requires
some changes or fixes in the parent driver.

Some drivers did:

parent_set_status():
if (DRIVER_OK)
    if (queue_enable)
        write queue_enable to the device

Examples are IFCVF or even vp_vdpa at least. MLX5 seems to be fine.

>
> However this opens a window where the device could start receiving
> packets in rx queue 0 before it receives the RSS configuration. To avoid
> that, we will not send vring_enable until all configuration is used by
> the device.
>
> As a first step, run vhost_set_vring_ready for all vhost_net backend after
> all of them are started (with DRIVER_OK). This code should not affect
> vdpa.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/net/vhost_net.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index c4eecc6f36..3900599465 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -399,6 +399,18 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> *ncs,
>          } else {
>              peer = qemu_get_peer(ncs, n->max_queue_pairs);
>          }
> +        r = vhost_net_start_one(get_vhost_net(peer), dev);
> +        if (r < 0) {
> +            goto err_start;
> +        }
> +    }
> +
> +    for (int j = 0; j < nvhosts; j++) {
> +        if (j < data_queue_pairs) {
> +            peer = qemu_get_peer(ncs, j);
> +        } else {
> +            peer = qemu_get_peer(ncs, n->max_queue_pairs);
> +        }

I fail to understand why we need to change the vhost_net layer? This
is vhost-vDPA specific, so I wonder if we can limit the changes to e.g
vhost_vdpa_dev_start()?

Thanks

>
>          if (peer->vring_enable) {
>              /* restore vring enable state */
> @@ -408,11 +420,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> *ncs,
>                  goto err_start;
>              }
>          }
> -
> -        r = vhost_net_start_one(get_vhost_net(peer), dev);
> -        if (r < 0) {
> -            goto err_start;
> -        }
>      }
>
>      return 0;
> --
> 2.31.1
>