qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] Preparatory patches for live migration downtime improvement
@ 2024-02-14 11:27 Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 01/12] vdpa: add back vhost_vdpa_net_first_nc_vdpa Si-Wei Liu
                   ` (11 more replies)
  0 siblings, 12 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

This small series is a spin-off from [1], where the patches
already acked from that large patchset may get merged earlier
without having to wait for those that are still in review.

The last 3 patches (10 - 12) are bug fix to an issue where
cancellation of ongoing migration may lead to busted network.
These are the only outstanding patches in this patchset with
no acknowledgement received as yet. Please try to review
them at the earliest oppotunity. Thanks!

Regards,
-Siwei

[1] [PATCH 00/40] vdpa-net: improve migration downtime through descriptor ASID and persistent IOTLB
https://lore.kernel.org/qemu-devel/1701970793-6865-1-git-send-email-si-wei.liu@oracle.com/

---

Si-Wei Liu (12):
  vdpa: add back vhost_vdpa_net_first_nc_vdpa
  vdpa: no repeat setting shadow_data
  vdpa: factor out vhost_vdpa_last_dev
  vdpa: factor out vhost_vdpa_net_get_nc_vdpa
  vdpa: add vhost_vdpa_set_address_space_id trace
  vdpa: add vhost_vdpa_get_vring_base trace for svq mode
  vdpa: add vhost_vdpa_set_dev_vring_base trace for svq mode
  vdpa: add trace events for vhost_vdpa_net_load_cmd
  vdpa: add trace event for vhost_vdpa_net_load_mq
  vdpa: define SVQ transitioning state for mode switching
  vdpa: indicate transitional state for SVQ switching
  vdpa: fix network breakage after cancelling migration

 hw/virtio/trace-events         |  4 ++--
 hw/virtio/vhost-vdpa.c         | 27 ++++++++++++++++++++++-----
 include/hw/virtio/vhost-vdpa.h |  9 +++++++++
 net/trace-events               |  6 ++++++
 net/vhost-vdpa.c               | 33 +++++++++++++++++++++++++++++----
 5 files changed, 68 insertions(+), 11 deletions(-)

-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 01/12] vdpa: add back vhost_vdpa_net_first_nc_vdpa
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 02/12] vdpa: no repeat setting shadow_data Si-Wei Liu
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

Previous commits had it removed. Now adding it back because
this function will be needed by future patches.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/vhost-vdpa.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 46e350a..4479ffa 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -280,6 +280,16 @@ static ssize_t vhost_vdpa_receive(NetClientState *nc, const uint8_t *buf,
     return size;
 }
 
+
+/** From any vdpa net client, get the netclient of the first queue pair */
+static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
+{
+    NICState *nic = qemu_get_nic(s->nc.peer);
+    NetClientState *nc0 = qemu_get_peer(nic->ncs, 0);
+
+    return DO_UPCAST(VhostVDPAState, nc, nc0);
+}
+
 static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
 {
     struct vhost_vdpa *v = &s->vhost_vdpa;
@@ -492,7 +502,7 @@ dma_map_err:
 
 static int vhost_vdpa_net_cvq_start(NetClientState *nc)
 {
-    VhostVDPAState *s;
+    VhostVDPAState *s, *s0;
     struct vhost_vdpa *v;
     int64_t cvq_group;
     int r;
@@ -503,7 +513,8 @@ static int vhost_vdpa_net_cvq_start(NetClientState *nc)
     s = DO_UPCAST(VhostVDPAState, nc, nc);
     v = &s->vhost_vdpa;
 
-    v->shadow_vqs_enabled = v->shared->shadow_data;
+    s0 = vhost_vdpa_net_first_nc_vdpa(s);
+    v->shadow_vqs_enabled = s0->vhost_vdpa.shadow_vqs_enabled;
     s->vhost_vdpa.address_space_id = VHOST_VDPA_GUEST_PA_ASID;
 
     if (v->shared->shadow_data) {
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 02/12] vdpa: no repeat setting shadow_data
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 01/12] vdpa: add back vhost_vdpa_net_first_nc_vdpa Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 03/12] vdpa: factor out vhost_vdpa_last_dev Si-Wei Liu
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

Since shadow_data is now shared in the parent data struct, it
just needs to be set only once by the first vq. This change
will make shadow_data independent of svq enabled state, which
can be optionally turned off when SVQ descritors and device
driver areas are all isolated to a separate address space.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/vhost-vdpa.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 4479ffa..06c83b4 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -354,13 +354,12 @@ static int vhost_vdpa_net_data_start(NetClientState *nc)
     if (s->always_svq ||
         migration_is_setup_or_active(migrate_get_current()->state)) {
         v->shadow_vqs_enabled = true;
-        v->shared->shadow_data = true;
     } else {
         v->shadow_vqs_enabled = false;
-        v->shared->shadow_data = false;
     }
 
     if (v->index == 0) {
+        v->shared->shadow_data = v->shadow_vqs_enabled;
         vhost_vdpa_net_data_start_first(s);
         return 0;
     }
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 03/12] vdpa: factor out vhost_vdpa_last_dev
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 01/12] vdpa: add back vhost_vdpa_net_first_nc_vdpa Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 02/12] vdpa: no repeat setting shadow_data Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa Si-Wei Liu
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

Generalize duplicated condition check for the last vq of vdpa
device to a common function.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 hw/virtio/vhost-vdpa.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index f7162da..1d3154a 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -551,6 +551,11 @@ static bool vhost_vdpa_first_dev(struct vhost_dev *dev)
     return v->index == 0;
 }
 
+static bool vhost_vdpa_last_dev(struct vhost_dev *dev)
+{
+    return dev->vq_index + dev->nvqs == dev->vq_index_end;
+}
+
 static int vhost_vdpa_get_dev_features(struct vhost_dev *dev,
                                        uint64_t *features)
 {
@@ -1317,7 +1322,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
         vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
     }
 
-    if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
+    if (!vhost_vdpa_last_dev(dev)) {
         return 0;
     }
 
@@ -1347,7 +1352,7 @@ static void vhost_vdpa_reset_status(struct vhost_dev *dev)
 {
     struct vhost_vdpa *v = dev->opaque;
 
-    if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
+    if (!vhost_vdpa_last_dev(dev)) {
         return;
     }
 
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (2 preceding siblings ...)
  2024-02-14 11:27 ` [PATCH 03/12] vdpa: factor out vhost_vdpa_last_dev Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 18:54   ` Eugenio Perez Martin
  2024-02-14 11:27 ` [PATCH 05/12] vdpa: add vhost_vdpa_set_address_space_id trace Si-Wei Liu
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

Introduce new API. No functional change on existing API.

Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/vhost-vdpa.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 06c83b4..4168cad 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -281,13 +281,18 @@ static ssize_t vhost_vdpa_receive(NetClientState *nc, const uint8_t *buf,
 }
 
 
-/** From any vdpa net client, get the netclient of the first queue pair */
-static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
+/** From any vdpa net client, get the netclient of the i-th queue pair */
+static VhostVDPAState *vhost_vdpa_net_get_nc_vdpa(VhostVDPAState *s, int i)
 {
     NICState *nic = qemu_get_nic(s->nc.peer);
-    NetClientState *nc0 = qemu_get_peer(nic->ncs, 0);
+    NetClientState *nc_i = qemu_get_peer(nic->ncs, i);
+
+    return DO_UPCAST(VhostVDPAState, nc, nc_i);
+}
 
-    return DO_UPCAST(VhostVDPAState, nc, nc0);
+static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
+{
+    return vhost_vdpa_net_get_nc_vdpa(s, 0);
 }
 
 static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 05/12] vdpa: add vhost_vdpa_set_address_space_id trace
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (3 preceding siblings ...)
  2024-02-14 11:27 ` [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 06/12] vdpa: add vhost_vdpa_get_vring_base trace for svq mode Si-Wei Liu
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

For better debuggability and observability.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/trace-events | 3 +++
 net/vhost-vdpa.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/net/trace-events b/net/trace-events
index 823a071..aab666a 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -23,3 +23,6 @@ colo_compare_tcp_info(const char *pkt, uint32_t seq, uint32_t ack, int hdlen, in
 # filter-rewriter.c
 colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=0x%x"
 colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u"
+
+# vhost-vdpa.c
+vhost_vdpa_set_address_space_id(void *v, unsigned vq_group, unsigned asid_num) "vhost_vdpa: %p vq_group: %u asid: %u"
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 4168cad..48a5608 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -29,6 +29,7 @@
 #include "migration/migration.h"
 #include "migration/misc.h"
 #include "hw/virtio/vhost.h"
+#include "trace.h"
 
 /* Todo:need to add the multiqueue support here */
 typedef struct VhostVDPAState {
@@ -440,6 +441,8 @@ static int vhost_vdpa_set_address_space_id(struct vhost_vdpa *v,
     };
     int r;
 
+    trace_vhost_vdpa_set_address_space_id(v, vq_group, asid_num);
+
     r = ioctl(v->shared->device_fd, VHOST_VDPA_SET_GROUP_ASID, &asid);
     if (unlikely(r < 0)) {
         error_report("Can't set vq group %u asid %u, errno=%d (%s)",
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 06/12] vdpa: add vhost_vdpa_get_vring_base trace for svq mode
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (4 preceding siblings ...)
  2024-02-14 11:27 ` [PATCH 05/12] vdpa: add vhost_vdpa_set_address_space_id trace Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 07/12] vdpa: add vhost_vdpa_set_dev_vring_base " Si-Wei Liu
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

For better debuggability and observability.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 hw/virtio/trace-events | 2 +-
 hw/virtio/vhost-vdpa.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 77905d1..28d6d78 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -58,7 +58,7 @@ vhost_vdpa_set_log_base(void *dev, uint64_t base, unsigned long long size, int r
 vhost_vdpa_set_vring_addr(void *dev, unsigned int index, unsigned int flags, uint64_t desc_user_addr, uint64_t used_user_addr, uint64_t avail_user_addr, uint64_t log_guest_addr) "dev: %p index: %u flags: 0x%x desc_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" log_guest_addr: 0x%"PRIx64
 vhost_vdpa_set_vring_num(void *dev, unsigned int index, unsigned int num) "dev: %p index: %u num: %u"
 vhost_vdpa_set_vring_base(void *dev, unsigned int index, unsigned int num) "dev: %p index: %u num: %u"
-vhost_vdpa_get_vring_base(void *dev, unsigned int index, unsigned int num) "dev: %p index: %u num: %u"
+vhost_vdpa_get_vring_base(void *dev, unsigned int index, unsigned int num, bool svq) "dev: %p index: %u num: %u svq: %d"
 vhost_vdpa_set_vring_kick(void *dev, unsigned int index, int fd) "dev: %p index: %u fd: %d"
 vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: %u fd: %d"
 vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 1d3154a..0de7bdf 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1424,6 +1424,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev *dev,
 
     if (v->shadow_vqs_enabled) {
         ring->num = virtio_queue_get_last_avail_idx(dev->vdev, ring->index);
+        trace_vhost_vdpa_get_vring_base(dev, ring->index, ring->num, true);
         return 0;
     }
 
@@ -1436,7 +1437,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev *dev,
     }
 
     ret = vhost_vdpa_call(dev, VHOST_GET_VRING_BASE, ring);
-    trace_vhost_vdpa_get_vring_base(dev, ring->index, ring->num);
+    trace_vhost_vdpa_get_vring_base(dev, ring->index, ring->num, false);
     return ret;
 }
 
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 07/12] vdpa: add vhost_vdpa_set_dev_vring_base trace for svq mode
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (5 preceding siblings ...)
  2024-02-14 11:27 ` [PATCH 06/12] vdpa: add vhost_vdpa_get_vring_base trace for svq mode Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 08/12] vdpa: add trace events for vhost_vdpa_net_load_cmd Si-Wei Liu
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

For better debuggability and observability.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 hw/virtio/trace-events | 2 +-
 hw/virtio/vhost-vdpa.c | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 28d6d78..20577aa 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -57,7 +57,7 @@ vhost_vdpa_dev_start(void *dev, bool started) "dev: %p started: %d"
 vhost_vdpa_set_log_base(void *dev, uint64_t base, unsigned long long size, int refcnt, int fd, void *log) "dev: %p base: 0x%"PRIx64" size: %llu refcnt: %d fd: %d log: %p"
 vhost_vdpa_set_vring_addr(void *dev, unsigned int index, unsigned int flags, uint64_t desc_user_addr, uint64_t used_user_addr, uint64_t avail_user_addr, uint64_t log_guest_addr) "dev: %p index: %u flags: 0x%x desc_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" log_guest_addr: 0x%"PRIx64
 vhost_vdpa_set_vring_num(void *dev, unsigned int index, unsigned int num) "dev: %p index: %u num: %u"
-vhost_vdpa_set_vring_base(void *dev, unsigned int index, unsigned int num) "dev: %p index: %u num: %u"
+vhost_vdpa_set_dev_vring_base(void *dev, unsigned int index, unsigned int num, bool svq) "dev: %p index: %u num: %u svq: %d"
 vhost_vdpa_get_vring_base(void *dev, unsigned int index, unsigned int num, bool svq) "dev: %p index: %u num: %u svq: %d"
 vhost_vdpa_set_vring_kick(void *dev, unsigned int index, int fd) "dev: %p index: %u fd: %d"
 vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index: %u fd: %d"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 0de7bdf..004110f 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -972,7 +972,10 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
 static int vhost_vdpa_set_dev_vring_base(struct vhost_dev *dev,
                                          struct vhost_vring_state *ring)
 {
-    trace_vhost_vdpa_set_vring_base(dev, ring->index, ring->num);
+    struct vhost_vdpa *v = dev->opaque;
+
+    trace_vhost_vdpa_set_dev_vring_base(dev, ring->index, ring->num,
+                                        v->shadow_vqs_enabled);
     return vhost_vdpa_call(dev, VHOST_SET_VRING_BASE, ring);
 }
 
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 08/12] vdpa: add trace events for vhost_vdpa_net_load_cmd
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (6 preceding siblings ...)
  2024-02-14 11:27 ` [PATCH 07/12] vdpa: add vhost_vdpa_set_dev_vring_base " Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:27 ` [PATCH 09/12] vdpa: add trace event for vhost_vdpa_net_load_mq Si-Wei Liu
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

For better debuggability and observability.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/trace-events | 2 ++
 net/vhost-vdpa.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/trace-events b/net/trace-events
index aab666a..88f56f2 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -26,3 +26,5 @@ colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u"
 
 # vhost-vdpa.c
 vhost_vdpa_set_address_space_id(void *v, unsigned vq_group, unsigned asid_num) "vhost_vdpa: %p vq_group: %u asid: %u"
+vhost_vdpa_net_load_cmd(void *s, uint8_t class, uint8_t cmd, int data_num, int data_size) "vdpa state: %p class: %u cmd: %u sg_num: %d size: %d"
+vhost_vdpa_net_load_cmd_retval(void *s, uint8_t class, uint8_t cmd, int r) "vdpa state: %p class: %u cmd: %u retval: %d"
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 48a5608..6ee438f 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -677,6 +677,7 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
 
     assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl));
     cmd_size = sizeof(ctrl) + data_size;
+    trace_vhost_vdpa_net_load_cmd(s, class, cmd, data_num, data_size);
     if (vhost_svq_available_slots(svq) < 2 ||
         iov_size(out_cursor, 1) < cmd_size) {
         /*
@@ -708,6 +709,7 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
 
     r = vhost_vdpa_net_cvq_add(s, &out, 1, &in, 1);
     if (unlikely(r < 0)) {
+        trace_vhost_vdpa_net_load_cmd_retval(s, class, cmd, r);
         return r;
     }
 
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 09/12] vdpa: add trace event for vhost_vdpa_net_load_mq
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (7 preceding siblings ...)
  2024-02-14 11:27 ` [PATCH 08/12] vdpa: add trace events for vhost_vdpa_net_load_cmd Si-Wei Liu
@ 2024-02-14 11:27 ` Si-Wei Liu
  2024-02-14 11:28 ` [PATCH 10/12] vdpa: define SVQ transitioning state for mode switching Si-Wei Liu
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:27 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

For better debuggability and observability.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/trace-events | 1 +
 net/vhost-vdpa.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/net/trace-events b/net/trace-events
index 88f56f2..cda960f 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -28,3 +28,4 @@ colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u"
 vhost_vdpa_set_address_space_id(void *v, unsigned vq_group, unsigned asid_num) "vhost_vdpa: %p vq_group: %u asid: %u"
 vhost_vdpa_net_load_cmd(void *s, uint8_t class, uint8_t cmd, int data_num, int data_size) "vdpa state: %p class: %u cmd: %u sg_num: %d size: %d"
 vhost_vdpa_net_load_cmd_retval(void *s, uint8_t class, uint8_t cmd, int r) "vdpa state: %p class: %u cmd: %u retval: %d"
+vhost_vdpa_net_load_mq(void *s, int ncurqps) "vdpa state: %p current_qpairs: %d"
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 6ee438f..9f25221 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -901,6 +901,8 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
         return 0;
     }
 
+    trace_vhost_vdpa_net_load_mq(s, n->curr_queue_pairs);
+
     mq.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs);
     const struct iovec data = {
         .iov_base = &mq,
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 10/12] vdpa: define SVQ transitioning state for mode switching
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (8 preceding siblings ...)
  2024-02-14 11:27 ` [PATCH 09/12] vdpa: add trace event for vhost_vdpa_net_load_mq Si-Wei Liu
@ 2024-02-14 11:28 ` Si-Wei Liu
  2024-02-14 11:28 ` [PATCH 11/12] vdpa: indicate transitional state for SVQ switching Si-Wei Liu
  2024-02-14 11:28 ` [PATCH 12/12] vdpa: fix network breakage after cancelling migration Si-Wei Liu
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:28 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

Will be used in following patches.

DISABLING(-1) means SVQ is being switched off to passthrough
mode.

ENABLING(1) means passthrough VQs are being switched to SVQ.

DONE(0) means SVQ switching is completed.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 include/hw/virtio/vhost-vdpa.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index ad754eb..449bf5c 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -30,6 +30,12 @@ typedef struct VhostVDPAHostNotifier {
     void *addr;
 } VhostVDPAHostNotifier;
 
+typedef enum SVQTransitionState {
+    SVQ_TSTATE_DISABLING = -1,
+    SVQ_TSTATE_DONE,
+    SVQ_TSTATE_ENABLING
+} SVQTransitionState;
+
 /* Info shared by all vhost_vdpa device models */
 typedef struct vhost_vdpa_shared {
     int device_fd;
@@ -67,6 +73,9 @@ typedef struct vhost_vdpa_shared {
 
     /* Vdpa must send shadow addresses as IOTLB key for data queues, not GPA */
     bool shadow_data;
+
+    /* SVQ switching is in progress, or already completed? */
+    SVQTransitionState svq_switching;
 } VhostVDPAShared;
 
 typedef struct vhost_vdpa {
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 11/12] vdpa: indicate transitional state for SVQ switching
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (9 preceding siblings ...)
  2024-02-14 11:28 ` [PATCH 10/12] vdpa: define SVQ transitioning state for mode switching Si-Wei Liu
@ 2024-02-14 11:28 ` Si-Wei Liu
  2024-02-14 11:28 ` [PATCH 12/12] vdpa: fix network breakage after cancelling migration Si-Wei Liu
  11 siblings, 0 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:28 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

svq_switching indicates the transitional state whether
or not SVQ mode switching is in progress, and towards
which direction. Add the neccessary state around where
the switching would take place.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/vhost-vdpa.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 9f25221..96d95b9 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -317,6 +317,8 @@ static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
     data_queue_pairs = n->multiqueue ? n->max_queue_pairs : 1;
     cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ?
                                   n->max_ncs - n->max_queue_pairs : 0;
+    v->shared->svq_switching = enable ?
+        SVQ_TSTATE_ENABLING : SVQ_TSTATE_DISABLING;
     /*
      * TODO: vhost_net_stop does suspend, get_base and reset. We can be smarter
      * in the future and resume the device if read-only operations between
@@ -329,6 +331,7 @@ static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
     if (unlikely(r < 0)) {
         error_report("unable to start vhost net: %s(%d)", g_strerror(-r), -r);
     }
+    v->shared->svq_switching = SVQ_TSTATE_DONE;
 }
 
 static void vdpa_net_migration_state_notifier(Notifier *notifier, void *data)
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 12/12] vdpa: fix network breakage after cancelling migration
  2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
                   ` (10 preceding siblings ...)
  2024-02-14 11:28 ` [PATCH 11/12] vdpa: indicate transitional state for SVQ switching Si-Wei Liu
@ 2024-02-14 11:28 ` Si-Wei Liu
  2024-02-15 16:15   ` Eugenio Perez Martin
  2024-03-13 18:12   ` Michael Tokarev
  11 siblings, 2 replies; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 11:28 UTC (permalink / raw)
  To: eperezma, jasowang, mst, dtatulea, leiyang, yin31149; +Cc: qemu-devel

Fix an issue where cancellation of ongoing migration ends up
with no network connectivity.

When canceling migration, SVQ will be switched back to the
passthrough mode, but the right call fd is not programed to
the device and the svq's own call fd is still used. At the
point of this transitioning period, the shadow_vqs_enabled
hadn't been set back to false yet, causing the installation
of call fd inadvertently bypassed.

Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding capabilities")
Cc: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 hw/virtio/vhost-vdpa.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 004110f..dfeca8b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
 
     /* Remember last call fd because we can switch to SVQ anytime. */
     vhost_svq_set_svq_call_fd(svq, file->fd);
-    if (v->shadow_vqs_enabled) {
+    /*
+     * When SVQ is transitioning to off, shadow_vqs_enabled has
+     * not been set back to false yet, but the underlying call fd
+     * will have to switch back to the guest notifier to signal the
+     * passthrough virtqueues. In other situations, SVQ's own call
+     * fd shall be used to signal the device model.
+     */
+    if (v->shadow_vqs_enabled &&
+        v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
         return 0;
     }
 
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa
  2024-02-14 11:27 ` [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa Si-Wei Liu
@ 2024-02-14 18:54   ` Eugenio Perez Martin
  2024-02-14 20:58     ` Si-Wei Liu
  0 siblings, 1 reply; 22+ messages in thread
From: Eugenio Perez Martin @ 2024-02-14 18:54 UTC (permalink / raw)
  To: Si-Wei Liu; +Cc: jasowang, mst, dtatulea, leiyang, yin31149, qemu-devel

On Wed, Feb 14, 2024 at 1:39 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> Introduce new API. No functional change on existing API.
>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>

I'm ok with the new function, but doesn't the compiler complain
because adding a static function is not used?

> ---
>  net/vhost-vdpa.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 06c83b4..4168cad 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -281,13 +281,18 @@ static ssize_t vhost_vdpa_receive(NetClientState *nc, const uint8_t *buf,
>  }
>
>
> -/** From any vdpa net client, get the netclient of the first queue pair */
> -static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
> +/** From any vdpa net client, get the netclient of the i-th queue pair */
> +static VhostVDPAState *vhost_vdpa_net_get_nc_vdpa(VhostVDPAState *s, int i)
>  {
>      NICState *nic = qemu_get_nic(s->nc.peer);
> -    NetClientState *nc0 = qemu_get_peer(nic->ncs, 0);
> +    NetClientState *nc_i = qemu_get_peer(nic->ncs, i);
> +
> +    return DO_UPCAST(VhostVDPAState, nc, nc_i);
> +}
>
> -    return DO_UPCAST(VhostVDPAState, nc, nc0);
> +static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
> +{
> +    return vhost_vdpa_net_get_nc_vdpa(s, 0);
>  }
>
>  static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
> --
> 1.8.3.1
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa
  2024-02-14 18:54   ` Eugenio Perez Martin
@ 2024-02-14 20:58     ` Si-Wei Liu
  2024-02-15  6:33       ` Eugenio Perez Martin
  0 siblings, 1 reply; 22+ messages in thread
From: Si-Wei Liu @ 2024-02-14 20:58 UTC (permalink / raw)
  To: Eugenio Perez Martin
  Cc: jasowang, mst, dtatulea, leiyang, yin31149, qemu-devel



On 2/14/2024 10:54 AM, Eugenio Perez Martin wrote:
> On Wed, Feb 14, 2024 at 1:39 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>> Introduce new API. No functional change on existing API.
>>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> I'm ok with the new function, but doesn't the compiler complain
> because adding a static function is not used?
Hmmm, which one? vhost_vdpa_net_get_nc_vdpa is used by 
vhost_vdpa_net_first_nc_vdpa internally, and 
vhost_vdpa_net_first_nc_vdpa is used by vhost_vdpa_net_cvq_start (Patch 
01). I think we should be fine?

-Siwei
>
>> ---
>>   net/vhost-vdpa.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index 06c83b4..4168cad 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -281,13 +281,18 @@ static ssize_t vhost_vdpa_receive(NetClientState *nc, const uint8_t *buf,
>>   }
>>
>>
>> -/** From any vdpa net client, get the netclient of the first queue pair */
>> -static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
>> +/** From any vdpa net client, get the netclient of the i-th queue pair */
>> +static VhostVDPAState *vhost_vdpa_net_get_nc_vdpa(VhostVDPAState *s, int i)
>>   {
>>       NICState *nic = qemu_get_nic(s->nc.peer);
>> -    NetClientState *nc0 = qemu_get_peer(nic->ncs, 0);
>> +    NetClientState *nc_i = qemu_get_peer(nic->ncs, i);
>> +
>> +    return DO_UPCAST(VhostVDPAState, nc, nc_i);
>> +}
>>
>> -    return DO_UPCAST(VhostVDPAState, nc, nc0);
>> +static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
>> +{
>> +    return vhost_vdpa_net_get_nc_vdpa(s, 0);
>>   }
>>
>>   static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
>> --
>> 1.8.3.1
>>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa
  2024-02-14 20:58     ` Si-Wei Liu
@ 2024-02-15  6:33       ` Eugenio Perez Martin
  0 siblings, 0 replies; 22+ messages in thread
From: Eugenio Perez Martin @ 2024-02-15  6:33 UTC (permalink / raw)
  To: Si-Wei Liu; +Cc: jasowang, mst, dtatulea, leiyang, yin31149, qemu-devel

On Wed, Feb 14, 2024 at 9:59 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
>
>
> On 2/14/2024 10:54 AM, Eugenio Perez Martin wrote:
> > On Wed, Feb 14, 2024 at 1:39 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
> >> Introduce new API. No functional change on existing API.
> >>
> >> Acked-by: Jason Wang <jasowang@redhat.com>
> >> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> > I'm ok with the new function, but doesn't the compiler complain
> > because adding a static function is not used?
> Hmmm, which one? vhost_vdpa_net_get_nc_vdpa is used by
> vhost_vdpa_net_first_nc_vdpa internally, and
> vhost_vdpa_net_first_nc_vdpa is used by vhost_vdpa_net_cvq_start (Patch
> 01). I think we should be fine?
>

Ouch, you're totally right.

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>

Thanks!

> -Siwei
> >
> >> ---
> >>   net/vhost-vdpa.c | 13 +++++++++----
> >>   1 file changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> >> index 06c83b4..4168cad 100644
> >> --- a/net/vhost-vdpa.c
> >> +++ b/net/vhost-vdpa.c
> >> @@ -281,13 +281,18 @@ static ssize_t vhost_vdpa_receive(NetClientState *nc, const uint8_t *buf,
> >>   }
> >>
> >>
> >> -/** From any vdpa net client, get the netclient of the first queue pair */
> >> -static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
> >> +/** From any vdpa net client, get the netclient of the i-th queue pair */
> >> +static VhostVDPAState *vhost_vdpa_net_get_nc_vdpa(VhostVDPAState *s, int i)
> >>   {
> >>       NICState *nic = qemu_get_nic(s->nc.peer);
> >> -    NetClientState *nc0 = qemu_get_peer(nic->ncs, 0);
> >> +    NetClientState *nc_i = qemu_get_peer(nic->ncs, i);
> >> +
> >> +    return DO_UPCAST(VhostVDPAState, nc, nc_i);
> >> +}
> >>
> >> -    return DO_UPCAST(VhostVDPAState, nc, nc0);
> >> +static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
> >> +{
> >> +    return vhost_vdpa_net_get_nc_vdpa(s, 0);
> >>   }
> >>
> >>   static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
> >> --
> >> 1.8.3.1
> >>
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
  2024-02-14 11:28 ` [PATCH 12/12] vdpa: fix network breakage after cancelling migration Si-Wei Liu
@ 2024-02-15 16:15   ` Eugenio Perez Martin
  2024-02-15 17:10     ` Eugenio Perez Martin
  2024-03-13 18:12   ` Michael Tokarev
  1 sibling, 1 reply; 22+ messages in thread
From: Eugenio Perez Martin @ 2024-02-15 16:15 UTC (permalink / raw)
  To: Si-Wei Liu; +Cc: jasowang, mst, dtatulea, leiyang, yin31149, qemu-devel

On Wed, Feb 14, 2024 at 1:39 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> Fix an issue where cancellation of ongoing migration ends up
> with no network connectivity.
>
> When canceling migration, SVQ will be switched back to the
> passthrough mode, but the right call fd is not programed to
> the device and the svq's own call fd is still used. At the
> point of this transitioning period, the shadow_vqs_enabled
> hadn't been set back to false yet, causing the installation
> of call fd inadvertently bypassed.
>
> Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding capabilities")
> Cc: Eugenio Pérez <eperezma@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  hw/virtio/vhost-vdpa.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 004110f..dfeca8b 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>
>      /* Remember last call fd because we can switch to SVQ anytime. */
>      vhost_svq_set_svq_call_fd(svq, file->fd);
> -    if (v->shadow_vqs_enabled) {
> +    /*
> +     * When SVQ is transitioning to off, shadow_vqs_enabled has
> +     * not been set back to false yet, but the underlying call fd
> +     * will have to switch back to the guest notifier to signal the
> +     * passthrough virtqueues. In other situations, SVQ's own call
> +     * fd shall be used to signal the device model.
> +     */
> +    if (v->shadow_vqs_enabled &&
> +        v->shared->svq_switching != SVQ_TSTATE_DISABLING) {

I think it would be great to not need to add more status variables to
vhost_vdpa (or any struct).

What if we recover the call file descriptor at vhost_vdpa_svqs_stop?
This way everything is more symmetrical as kick and call are set by
vhost_vdpa_svqs_start.

Thanks!

>          return 0;
>      }
>
> --
> 1.8.3.1
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
  2024-02-15 16:15   ` Eugenio Perez Martin
@ 2024-02-15 17:10     ` Eugenio Perez Martin
  0 siblings, 0 replies; 22+ messages in thread
From: Eugenio Perez Martin @ 2024-02-15 17:10 UTC (permalink / raw)
  To: Si-Wei Liu; +Cc: jasowang, mst, dtatulea, leiyang, yin31149, qemu-devel

On Thu, Feb 15, 2024 at 5:15 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Wed, Feb 14, 2024 at 1:39 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
> >
> > Fix an issue where cancellation of ongoing migration ends up
> > with no network connectivity.
> >
> > When canceling migration, SVQ will be switched back to the
> > passthrough mode, but the right call fd is not programed to
> > the device and the svq's own call fd is still used. At the
> > point of this transitioning period, the shadow_vqs_enabled
> > hadn't been set back to false yet, causing the installation
> > of call fd inadvertently bypassed.
> >
> > Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding capabilities")
> > Cc: Eugenio Pérez <eperezma@redhat.com>
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> > ---
> >  hw/virtio/vhost-vdpa.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 004110f..dfeca8b 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> >
> >      /* Remember last call fd because we can switch to SVQ anytime. */
> >      vhost_svq_set_svq_call_fd(svq, file->fd);
> > -    if (v->shadow_vqs_enabled) {
> > +    /*
> > +     * When SVQ is transitioning to off, shadow_vqs_enabled has
> > +     * not been set back to false yet, but the underlying call fd
> > +     * will have to switch back to the guest notifier to signal the
> > +     * passthrough virtqueues. In other situations, SVQ's own call
> > +     * fd shall be used to signal the device model.
> > +     */
> > +    if (v->shadow_vqs_enabled &&
> > +        v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
>
> I think it would be great to not need to add more status variables to
> vhost_vdpa (or any struct).
>
> What if we recover the call file descriptor at vhost_vdpa_svqs_stop?
> This way everything is more symmetrical as kick and call are set by
> vhost_vdpa_svqs_start.
>

Also, we could check for dev->started.

> Thanks!
>
> >          return 0;
> >      }
> >
> > --
> > 1.8.3.1
> >



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
  2024-02-14 11:28 ` [PATCH 12/12] vdpa: fix network breakage after cancelling migration Si-Wei Liu
  2024-02-15 16:15   ` Eugenio Perez Martin
@ 2024-03-13 18:12   ` Michael Tokarev
  2024-03-13 18:52     ` Michael Tokarev
  2024-03-13 19:10     ` Si-Wei Liu
  1 sibling, 2 replies; 22+ messages in thread
From: Michael Tokarev @ 2024-03-13 18:12 UTC (permalink / raw)
  To: Si-Wei Liu, jasowang, mst, dtatulea, leiyang, yin31149,
	Eugenio Pérez
  Cc: qemu-devel

14.02.2024 14:28, Si-Wei Liu wrote:
> Fix an issue where cancellation of ongoing migration ends up
> with no network connectivity.
> 
> When canceling migration, SVQ will be switched back to the
> passthrough mode, but the right call fd is not programed to
> the device and the svq's own call fd is still used. At the
> point of this transitioning period, the shadow_vqs_enabled
> hadn't been set back to false yet, causing the installation
> of call fd inadvertently bypassed.
> 
> Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding capabilities")
> Cc: Eugenio Pérez <eperezma@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>   hw/virtio/vhost-vdpa.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)

Is this a -stable material?

If yes, is it also applicable for stable-7.2 (mentioned commit is in 7.2.0),
which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set",
or should this one also be picked up?

Thanks,

/mjt

> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 004110f..dfeca8b 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>   
>       /* Remember last call fd because we can switch to SVQ anytime. */
>       vhost_svq_set_svq_call_fd(svq, file->fd);
> -    if (v->shadow_vqs_enabled) {
> +    /*
> +     * When SVQ is transitioning to off, shadow_vqs_enabled has
> +     * not been set back to false yet, but the underlying call fd
> +     * will have to switch back to the guest notifier to signal the
> +     * passthrough virtqueues. In other situations, SVQ's own call
> +     * fd shall be used to signal the device model.
> +     */
> +    if (v->shadow_vqs_enabled &&
> +        v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
>           return 0;
>       }
>   



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
  2024-03-13 18:12   ` Michael Tokarev
@ 2024-03-13 18:52     ` Michael Tokarev
  2024-03-13 19:10     ` Si-Wei Liu
  1 sibling, 0 replies; 22+ messages in thread
From: Michael Tokarev @ 2024-03-13 18:52 UTC (permalink / raw)
  To: Si-Wei Liu, jasowang, mst, dtatulea, leiyang, yin31149,
	Eugenio Pérez
  Cc: qemu-devel

13.03.2024 21:12, Michael Tokarev пишет:
> 14.02.2024 14:28, Si-Wei Liu wrote:
>> Fix an issue where cancellation of ongoing migration ends up
>> with no network connectivity.
>>
>> When canceling migration, SVQ will be switched back to the
>> passthrough mode, but the right call fd is not programed to
>> the device and the svq's own call fd is still used. At the
>> point of this transitioning period, the shadow_vqs_enabled
>> hadn't been set back to false yet, causing the installation
>> of call fd inadvertently bypassed.
>>
>> Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding capabilities")
>> Cc: Eugenio Pérez <eperezma@redhat.com>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
>> ---
>>   hw/virtio/vhost-vdpa.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> Is this a -stable material?
> 
> If yes, is it also applicable for stable-7.2 (mentioned commit is in 7.2.0),
> which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set",
> or should this one also be picked up?

Aha, this does not actually work without v8.2.0-171-gf6fe3e333f
"vdpa: move memory listener to vhost_vdpa_shared".

/mjt

>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> index 004110f..dfeca8b 100644
>> --- a/hw/virtio/vhost-vdpa.c
>> +++ b/hw/virtio/vhost-vdpa.c
>> @@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>>       /* Remember last call fd because we can switch to SVQ anytime. */
>>       vhost_svq_set_svq_call_fd(svq, file->fd);
>> -    if (v->shadow_vqs_enabled) {
>> +    /*
>> +     * When SVQ is transitioning to off, shadow_vqs_enabled has
>> +     * not been set back to false yet, but the underlying call fd
>> +     * will have to switch back to the guest notifier to signal the
>> +     * passthrough virtqueues. In other situations, SVQ's own call
>> +     * fd shall be used to signal the device model.
>> +     */
>> +    if (v->shadow_vqs_enabled &&
>> +        v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
>>           return 0;
>>       }
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
  2024-03-13 18:12   ` Michael Tokarev
  2024-03-13 18:52     ` Michael Tokarev
@ 2024-03-13 19:10     ` Si-Wei Liu
  2024-03-13 20:41       ` Michael Tokarev
  1 sibling, 1 reply; 22+ messages in thread
From: Si-Wei Liu @ 2024-03-13 19:10 UTC (permalink / raw)
  To: Michael Tokarev, jasowang, mst, dtatulea, leiyang, yin31149,
	Eugenio Pérez
  Cc: qemu-devel



On 3/13/2024 11:12 AM, Michael Tokarev wrote:
> 14.02.2024 14:28, Si-Wei Liu wrote:
>> Fix an issue where cancellation of ongoing migration ends up
>> with no network connectivity.
>>
>> When canceling migration, SVQ will be switched back to the
>> passthrough mode, but the right call fd is not programed to
>> the device and the svq's own call fd is still used. At the
>> point of this transitioning period, the shadow_vqs_enabled
>> hadn't been set back to false yet, causing the installation
>> of call fd inadvertently bypassed.
>>
>> Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding 
>> capabilities")
>> Cc: Eugenio Pérez <eperezma@redhat.com>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
>> ---
>>   hw/virtio/vhost-vdpa.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> Is this a -stable material?
Probably yes, the pre-requisites of this patch are PATCH #10 and #11 
from this series (where SVQ_TSTATE_DISABLING gets defined and set).

>
> If yes, is it also applicable for stable-7.2 (mentioned commit is in 
> 7.2.0),
> which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set",
> or should this one also be picked up?
Eugenio can judge, but seems to me the relevant code path cannot be 
effectively called as the dynamic SVQ feature (switching over to SVQ 
dynamically when migration is started) is not supported from 7.2. Maybe 
not worth it to cherry-pick this one to 7.2. Cherry-pick to stable-8.0 
and above should be applicable though (it needs some tweaks on patch #10 
to move svq_switching from @struct VhostVDPAShared to @struct vhost_vdpa).

Regards,
-Siwei

>
> Thanks,
>
> /mjt
>
>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> index 004110f..dfeca8b 100644
>> --- a/hw/virtio/vhost-vdpa.c
>> +++ b/hw/virtio/vhost-vdpa.c
>> @@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct 
>> vhost_dev *dev,
>>         /* Remember last call fd because we can switch to SVQ 
>> anytime. */
>>       vhost_svq_set_svq_call_fd(svq, file->fd);
>> -    if (v->shadow_vqs_enabled) {
>> +    /*
>> +     * When SVQ is transitioning to off, shadow_vqs_enabled has
>> +     * not been set back to false yet, but the underlying call fd
>> +     * will have to switch back to the guest notifier to signal the
>> +     * passthrough virtqueues. In other situations, SVQ's own call
>> +     * fd shall be used to signal the device model.
>> +     */
>> +    if (v->shadow_vqs_enabled &&
>> +        v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
>>           return 0;
>>       }
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
  2024-03-13 19:10     ` Si-Wei Liu
@ 2024-03-13 20:41       ` Michael Tokarev
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Tokarev @ 2024-03-13 20:41 UTC (permalink / raw)
  To: Si-Wei Liu, jasowang, mst, dtatulea, leiyang, yin31149,
	Eugenio Pérez
  Cc: qemu-devel

13.03.2024 22:10, Si-Wei Liu wrote:
> On 3/13/2024 11:12 AM, Michael Tokarev wrote:
..
>> Is this a -stable material?
> Probably yes, the pre-requisites of this patch are PATCH #10 and #11 from this series (where SVQ_TSTATE_DISABLING gets defined and set).
> 
>>
>> If yes, is it also applicable for stable-7.2 (mentioned commit is in 7.2.0),
>> which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set",
>> or should this one also be picked up?
> Eugenio can judge, but seems to me the relevant code path cannot be effectively called as the dynamic SVQ feature (switching over to SVQ dynamically 
> when migration is started) is not supported from 7.2. Maybe not worth it to cherry-pick this one to 7.2. Cherry-pick to stable-8.0 and above should be 
> applicable though (it needs some tweaks on patch #10 to move svq_switching from @struct VhostVDPAShared to @struct vhost_vdpa).

Uhh.  That's a bit larger than I thought.  With this amount of prepreqs
and manual tweaking, and with the fact this whole thing introduces Yet
Another State value, - let's omit this one.  Or at the very least I'd
love to have actual real-life testcase and a clean backport to 8.2 of
all the required changes.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2024-03-13 20:41 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-14 11:27 [PATCH 00/12] Preparatory patches for live migration downtime improvement Si-Wei Liu
2024-02-14 11:27 ` [PATCH 01/12] vdpa: add back vhost_vdpa_net_first_nc_vdpa Si-Wei Liu
2024-02-14 11:27 ` [PATCH 02/12] vdpa: no repeat setting shadow_data Si-Wei Liu
2024-02-14 11:27 ` [PATCH 03/12] vdpa: factor out vhost_vdpa_last_dev Si-Wei Liu
2024-02-14 11:27 ` [PATCH 04/12] vdpa: factor out vhost_vdpa_net_get_nc_vdpa Si-Wei Liu
2024-02-14 18:54   ` Eugenio Perez Martin
2024-02-14 20:58     ` Si-Wei Liu
2024-02-15  6:33       ` Eugenio Perez Martin
2024-02-14 11:27 ` [PATCH 05/12] vdpa: add vhost_vdpa_set_address_space_id trace Si-Wei Liu
2024-02-14 11:27 ` [PATCH 06/12] vdpa: add vhost_vdpa_get_vring_base trace for svq mode Si-Wei Liu
2024-02-14 11:27 ` [PATCH 07/12] vdpa: add vhost_vdpa_set_dev_vring_base " Si-Wei Liu
2024-02-14 11:27 ` [PATCH 08/12] vdpa: add trace events for vhost_vdpa_net_load_cmd Si-Wei Liu
2024-02-14 11:27 ` [PATCH 09/12] vdpa: add trace event for vhost_vdpa_net_load_mq Si-Wei Liu
2024-02-14 11:28 ` [PATCH 10/12] vdpa: define SVQ transitioning state for mode switching Si-Wei Liu
2024-02-14 11:28 ` [PATCH 11/12] vdpa: indicate transitional state for SVQ switching Si-Wei Liu
2024-02-14 11:28 ` [PATCH 12/12] vdpa: fix network breakage after cancelling migration Si-Wei Liu
2024-02-15 16:15   ` Eugenio Perez Martin
2024-02-15 17:10     ` Eugenio Perez Martin
2024-03-13 18:12   ` Michael Tokarev
2024-03-13 18:52     ` Michael Tokarev
2024-03-13 19:10     ` Si-Wei Liu
2024-03-13 20:41       ` Michael Tokarev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).