On Sat, Jul 22, 2023 at 12:59 AM Si-Wei Liu <si-wei....@oracle.com> wrote: > > > > On 7/20/2023 11:14 AM, Eugenio Pérez wrote: > > Some dynamic state of a virtio-net vDPA devices is restored from CVQ in > > the event of a live migration. However, dataplane needs to be disabled > > so the NIC does not receive buffers in the invalid ring. > > > > As a default method to achieve it, let's offer a shadow vring with 0 > > avail idx. As a fallback method, we will enable dataplane vqs later, as > > proposed previously. > Let's not jump to conclusion too early what will be the default v.s. > fallback [1] - as this is on a latency sensitive path, I'm not fully > convinced ring reset could perform better than or equally same as the > deferred dataplane enablement approach on hardware. At this stage I > think ring_reset has no adoption on vendors device, while it's > definitely easier with lower hardware overhead for vendor to implement > deferred dataplane enabling. If at some point vendor's device has to > support RING_RESET for other use cases (MTU change propagation for ex., > a prerequisite for GRO HW) than live migration, defaulting to RING_RESET > on this SVQ path has no real benefit but adds complications needlessly > to vendor's device. >
I agree with that. Let's say "*This series* uses RING_RESET as the default method, and late vq enablement as fallback". Michael, given the current HW support, would it work to start merging the late enable for vDPA after the feature freeze, and then add the use of RING_RESET on top later? Thanks! > [1] > https://lore.kernel.org/virtualization/bf2164a9-1dfd-14d9-be2a-8bb7620a0...@oracle.com/T/#m15caca6fbb00ca9c00e2b33391297a2d8282ff89 > > Thanks, > -Siwei > > > > > Signed-off-by: Eugenio Pérez <epere...@redhat.com> > > --- > > net/vhost-vdpa.c | 49 +++++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 44 insertions(+), 5 deletions(-) > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > > index af83de92f8..e14ae48f23 100644 > > --- a/net/vhost-vdpa.c > > +++ b/net/vhost-vdpa.c > > @@ -338,10 +338,25 @@ static int vhost_vdpa_net_data_start(NetClientState > > *nc) > > { > > VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc); > > struct vhost_vdpa *v = &s->vhost_vdpa; > > + bool has_cvq = v->dev->vq_index_end % 2; > > > > assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA); > > > > - if (s->always_svq || > > + if (has_cvq && (v->dev->features & VIRTIO_F_RING_RESET)) { > > + /* > > + * Offer a fake vring to the device while the state is restored > > + * through CVQ. That way, the guest will not see packets in > > unexpected > > + * queues. > > + * > > + * This will be undone after loading all state through CVQ, at > > + * vhost_vdpa_net_load. > > + * > > + * TODO: Future optimizations may skip some SVQ setup and teardown, > > + * like set the right kick and call fd or doorbell maps directly, > > and > > + * the iova tree. > > + */ > > + v->shadow_vqs_enabled = true; > > + } else if (s->always_svq || > > migration_is_setup_or_active(migrate_get_current()->state)) { > > v->shadow_vqs_enabled = true; > > v->shadow_data = true; > > @@ -738,10 +753,34 @@ static int vhost_vdpa_net_load(NetClientState *nc) > > return r; > > } > > > > - for (int i = 0; i < v->dev->vq_index; ++i) { > > - r = vhost_vdpa_set_vring_ready(v, i); > > - if (unlikely(r)) { > > - return r; > > + if (v->dev->features & VIRTIO_F_RING_RESET && !s->always_svq && > > + !migration_is_setup_or_active(migrate_get_current()->state)) { > > + NICState *nic = qemu_get_nic(s->nc.peer); > > + int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; > > + > > + for (int i = 0; i < queue_pairs; ++i) { > > + NetClientState *ncs = qemu_get_peer(nic->ncs, i); > > + VhostVDPAState *s_i = DO_UPCAST(VhostVDPAState, nc, ncs); > > + > > + for (int j = 0; j < 2; ++j) { > > + vhost_net_virtqueue_reset(v->dev->vdev, ncs->peer, j); > > + } > > + > > + s_i->vhost_vdpa.shadow_vqs_enabled = false; > > + > > + for (int j = 0; j < 2; ++j) { > > + r = vhost_net_virtqueue_restart(v->dev->vdev, ncs->peer, > > j); > > + if (unlikely(r < 0)) { > > + return r; > > + } > > + } > > + } > > + } else { > > + for (int i = 0; i < v->dev->vq_index; ++i) { > > + r = vhost_vdpa_set_vring_ready(v, i); > > + if (unlikely(r)) { > > + return r; > > + } > > } > > } > > >
