Query: Is it possible to lose interrupts between vhost and virtio_net during migration?

[The test scenario]:

Doing migration between two Hosts roundly(A->B, B->A) ,after about 20 times, network of the VM is unreachable. There are other 20 VMs in each Host, and they send ipv4 or ipv6 and multicast packets to each other. Sometimes the CPU idle of the Host maybe 0;

[Problem description]:

I wonder if it was interrupts missing that cause the network unreachable.
In the migration process of kvm, source end should suspend, which include steps as follows:
1. do_vm_stop->pause_all_vcpus
2. vm_state_notify-> vhost_net_stop->set_guest_notifiers->kvm_virtio_pci_vq_vector_release
3. vm_state_notify-> vhost_net_stop-> vhost_net_stop_one->OST_NET_SET_BACKEND-> vhost_net_flush_vq-> vhost_work_flush
This may cause interrupts missing. Supose the scene that, virtqueue_notify() is called in virtio_net,
then the VM is paused. And, just before the portiowrite being handled, eventfd of kvm is released.
Then, vhost could not sense the notify, and the tx notify is lost.
On the other side, if eventfd of kvm is released just after vhost_notify(), and before eventfd_signal(), then rx signal by vhost is lost.

Quoted 8 lines · Zhangjie (HZ)

[The test scenario]:

Doing migration between two Hosts roundly(A->B, B->A) ,after about 20 times, network of the VM is unreachable. There are other 20 VMs in each Host, and they send ipv4 or ipv6 and multicast packets to each other. Sometimes the CPU idle of the Host maybe 0;

[Problem description]:

Quoted 9 lines

I wonder if it was interrupts missing that cause the network unreachable.
In the migration process of kvm, source end should suspend, which include steps as follows:
1. do_vm_stop->pause_all_vcpus
2. vm_state_notify-> vhost_net_stop->set_guest_notifiers->kvm_virtio_pci_vq_vector_release
3. vm_state_notify-> vhost_net_stop-> vhost_net_stop_one->OST_NET_SET_BACKEND-> vhost_net_flush_vq-> vhost_work_flush
This may cause interrupts missing. Supose the scene that, virtqueue_notify() is called in virtio_net,
then the VM is paused. And, just before the portiowrite being handled, eventfd of kvm is released.
Then, vhost could not sense the notify, and the tx notify is lost.
On the other side, if eventfd of kvm is released just after vhost_notify(), and before eventfd_signal(), then rx signal by vhost is lost.

Could be a bug in userspace: should should cleanups notifiers after it stops vhost.

Could you please send this to appropriate mailing lists? I have a policy against off-list discussions.

Signature · 1 line

MST

Quoted 19 lines · Michael S. Tsirkin

Quoted 8 lines · Zhangjie (HZ)

[The test scenario]:

Doing migration between two Hosts roundly(A->B, B->A) ,after about 20 times, network of the VM is unreachable. There are other 20 VMs in each Host, and they send ipv4 or ipv6 and multicast packets to each other. Sometimes the CPU idle of the Host maybe 0;

[Problem description]:

Quoted 9 lines

I wonder if it was interrupts missing that cause the network unreachable.
In the migration process of kvm, source end should suspend, which include steps as follows:
1. do_vm_stop->pause_all_vcpus
2. vm_state_notify-> vhost_net_stop->set_guest_notifiers->kvm_virtio_pci_vq_vector_release
3. vm_state_notify-> vhost_net_stop-> vhost_net_stop_one->OST_NET_SET_BACKEND-> vhost_net_flush_vq-> vhost_work_flush
This may cause interrupts missing. Supose the scene that, virtqueue_notify() is called in virtio_net,
then the VM is paused. And, just before the portiowrite being handled, eventfd of kvm is released.
Then, vhost could not sense the notify, and the tx notify is lost.
On the other side, if eventfd of kvm is released just after vhost_notify(), and before eventfd_signal(), then rx signal by vhost is lost.

Quoted 5 lines

Could be a bug in userspace: should should cleanups notifiers after it stops vhost.

Could you please send this to appropriate mailing lists? I have a policy against off-list discussions.

Also, Jason, could you take a look please?
Looks like your patch a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 changed the order of stopping the device.
Previously vhost_dev_stop would disable backend and only afterwards, unset guest notifiers. You now unset guest notifiers while vhost is still active. Looks like this can lose events?

Quoted 2 lines

Signature · 1 line

MST

.

Quoted 22 lines · Michael S. Tsirkin

Quoted 22 lines · Zhangjie (HZ)

Quoted 8 lines

Quoted 8 lines

[The test scenario]:

Doing migration between two Hosts roundly(A->B, B->A) ,after about 20 times, network of the VM is unreachable. There are other 20 VMs in each Host, and they send ipv4 or ipv6 and multicast packets to each other. Sometimes the CPU idle of the Host maybe 0;

[Problem description]:

Quoted 9 lines

Quoted 9 lines

I wonder if it was interrupts missing that cause the network unreachable.
In the migration process of kvm, source end should suspend, which include steps as follows:
1. do_vm_stop->pause_all_vcpus
2. vm_state_notify-> vhost_net_stop->set_guest_notifiers->kvm_virtio_pci_vq_vector_release
3. vm_state_notify-> vhost_net_stop-> vhost_net_stop_one->OST_NET_SET_BACKEND-> vhost_net_flush_vq-> vhost_work_flush
This may cause interrupts missing. Supose the scene that, virtqueue_notify() is called in virtio_net,
then the VM is paused. And, just before the portiowrite being handled, eventfd of kvm is released.
Then, vhost could not sense the notify, and the tx notify is lost.
On the other side, if eventfd of kvm is released just after vhost_notify(), and before eventfd_signal(), then rx signal by vhost is lost.

Could be a bug in userspace: should should cleanups notifiers after it stops vhost.

Quoted 2 lines

Quoted 2 lines

Could you please send this to appropriate mailing lists? I have a policy against off-list discussions.

Also, Jason, could you take a look please?
Looks like your patch a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 changed the order of stopping the device.
Previously vhost_dev_stop would disable backend and only afterwards, unset guest notifiers. You now unset guest notifiers while vhost is still active. Looks like this can lose events?

Quoted 23 lines · Michael S. Tsirkin

Quoted 23 lines · Michael S. Tsirkin

Quoted 22 lines

Quoted 22 lines

Quoted 22 lines

Quoted 22 lines · Zhangjie (HZ)

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

[The test scenario]:

Doing migration between two Hosts roundly(A->B, B->A) ,after about 20 times, network of the VM is unreachable. There are other 20 VMs in each Host, and they send ipv4 or ipv6 and multicast packets to each other. Sometimes the CPU idle of the Host maybe 0;

[Problem description]:

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

I wonder if it was interrupts missing that cause the network unreachable.
In the migration process of kvm, source end should suspend, which include steps as follows:
1. do_vm_stop->pause_all_vcpus
2. vm_state_notify-> vhost_net_stop->set_guest_notifiers->kvm_virtio_pci_vq_vector_release
3. vm_state_notify-> vhost_net_stop-> vhost_net_stop_one->OST_NET_SET_BACKEND-> vhost_net_flush_vq-> vhost_work_flush
This may cause interrupts missing. Supose the scene that, virtqueue_notify() is called in virtio_net,
then the VM is paused. And, just before the portiowrite being handled, eventfd of kvm is released.
Then, vhost could not sense the notify, and the tx notify is lost.
On the other side, if eventfd of kvm is released just after vhost_notify(), and before eventfd_signal(), then rx signal by vhost is lost.

Could be a bug in userspace: should should cleanups notifiers after it stops vhost.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Could you please send this to appropriate mailing lists? I have a policy against off-list discussions.

Also, Jason, could you take a look please?
Looks like your patch a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 changed the order of stopping the device.
Previously vhost_dev_stop would disable backend and only afterwards, unset guest notifiers. You now unset guest notifiers while vhost is still active. Looks like this can lose events?

Not sure it will really cause the issue. Since during guest notifier deassign in virtio_queue_set_guest_notifier_fd_handler() it will test the notifier and trigger callback if set. Looks like this can guarantee the interrupt was not lost.

Quoted 32 lines · Jason Wang

Quoted 23 lines · Michael S. Tsirkin

Quoted 23 lines · Michael S. Tsirkin

Quoted 22 lines

Quoted 22 lines

Quoted 22 lines

Quoted 22 lines · Zhangjie (HZ)

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

[The test scenario]:

Doing migration between two Hosts roundly(A->B, B->A) ,after about 20 times, network of the VM is unreachable. There are other 20 VMs in each Host, and they send ipv4 or ipv6 and multicast packets to each other. Sometimes the CPU idle of the Host maybe 0;

[Problem description]:

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

I wonder if it was interrupts missing that cause the network unreachable.
In the migration process of kvm, source end should suspend, which include steps as follows:
1. do_vm_stop->pause_all_vcpus
2. vm_state_notify-> vhost_net_stop->set_guest_notifiers->kvm_virtio_pci_vq_vector_release
3. vm_state_notify-> vhost_net_stop-> vhost_net_stop_one->OST_NET_SET_BACKEND-> vhost_net_flush_vq-> vhost_work_flush
This may cause interrupts missing. Supose the scene that, virtqueue_notify() is called in virtio_net,
then the VM is paused. And, just before the portiowrite being handled, eventfd of kvm is released.
Then, vhost could not sense the notify, and the tx notify is lost.
On the other side, if eventfd of kvm is released just after vhost_notify(), and before eventfd_signal(), then rx signal by vhost is lost.

Could be a bug in userspace: should should cleanups notifiers after it stops vhost.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Could you please send this to appropriate mailing lists? I have a policy against off-list discussions.

Also, Jason, could you take a look please?
Looks like your patch a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 changed the order of stopping the device.
Previously vhost_dev_stop would disable backend and only afterwards, unset guest notifiers. You now unset guest notifiers while vhost is still active. Looks like this can lose events?

Quoted 4 lines

Not sure it will really cause the issue. Since during guest notifier deassign in virtio_queue_set_guest_notifier_fd_handler() it will test the notifier and trigger callback if set. Looks like this can guarantee the interrupt was not lost.

More thought on this, looks like it was still a window between guest notifiers disabling and vhost_net stopping.

Please Zhang Jie test the patch of changing its order and if it works, sends a formal patch to qemu-devel.

btw, vhost_scsi may need the fix as well since it may meet the same issue.

Thanks
.

Jason is right, the new order is not the cause of network unreachable. Changing order seems not work. After about 40 times, the problem occurs again. Maybe there is other hidden reasons for that.

Quoted 3 lines · Zhangjie (HZ)

Jason is right, the new order is not the cause of network unreachable. Changing order seems not work. After about 40 times, the problem occurs again. Maybe there is other hidden reasons for that.

To make sure, you tested the patch that I posted to list:
"vhost_net: stop guest notifiers after backend"?

Please confirm.

Quoted 4 lines · Michael S. Tsirkin

Quoted 3 lines · Zhangjie (HZ)

Jason is right, the new order is not the cause of network unreachable. Changing order seems not work. After about 40 times, the problem occurs again. Maybe there is other hidden reasons for that.

I modified the code to change the order myself yesterday. This result is about my code.

Quoted 1 line

Quoted 5 lines

To make sure, you tested the patch that I posted to list:
"vhost_net: stop guest notifiers after backend"?

Please confirm.

OK, I will test with your patch "vhost_net: stop guest notifiers after backend".

(quoted diff) +6 -0

@@ -308,6 +308,12 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
        goto err;
    }
 
+    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
+    if (r < 0) {
+        error_report("Error binding guest notifier: %d", -r);
+        goto err;
+    }
+
    for (i = 0; i < total_queues; i++) {
        r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev, i * 2);
 

Quoted 5 lines · Zhangjie (HZ)

Quoted 4 lines · Michael S. Tsirkin

Quoted 3 lines · Zhangjie (HZ)

Jason is right, the new order is not the cause of network unreachable. Changing order seems not work. After about 40 times, the problem occurs again. Maybe there is other hidden reasons for that.

Quoted 9 lines

I modified the code to change the order myself yesterday. This result is about my code.

Quoted 5 lines

To make sure, you tested the patch that I posted to list:
"vhost_net: stop guest notifiers after backend"?

Please confirm.

OK, I will test with your patch "vhost_net: stop guest notifiers after backend".

Unfortunately, after using the patch "vhost_net: stop guest notifiers after backend", Linux VMs stopt themselves a few minutes after they were started.

Quoted 17 lines

(quoted diff) +6 -0

@@ -308,6 +308,12 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
        goto err;
    }
 
+    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
+    if (r < 0) {
+        error_report("Error binding guest notifier: %d", -r);
+        goto err;
+    }
+
    for (i = 0; i < total_queues; i++) {
        r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev, i * 2);
 

@@ -316,12 +322,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
        }
    }
 

Review int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, · line 325

Quoted 7 lines

(quoted diff) +0 -6

-    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
-    if (r < 0) {
-        error_report("Error binding guest notifier: %d", -r);
-        goto err;
-    }
-
    return 0;

I wonder if k->set_guest_notifiers should be called after "hdev->started = true;" in vhost_dev_start.

Quoted 34 lines · Zhangjie (HZ)

Quoted 5 lines · Zhangjie (HZ)

Quoted 4 lines · Michael S. Tsirkin

Quoted 3 lines · Zhangjie (HZ)

Jason is right, the new order is not the cause of network unreachable. Changing order seems not work. After about 40 times, the problem occurs again. Maybe there is other hidden reasons for that.

Quoted 9 lines

I modified the code to change the order myself yesterday. This result is about my code.

Quoted 5 lines

To make sure, you tested the patch that I posted to list:
"vhost_net: stop guest notifiers after backend"?

Please confirm.

OK, I will test with your patch "vhost_net: stop guest notifiers after backend".

Unfortunately, after using the patch "vhost_net: stop guest notifiers after backend", Linux VMs stopt themselves a few minutes after they were started.

Quoted 17 lines

(quoted diff) +6 -0

@@ -308,6 +308,12 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
        goto err;
    }
 
+    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
+    if (r < 0) {
+        error_report("Error binding guest notifier: %d", -r);
+        goto err;
+    }
+
    for (i = 0; i < total_queues; i++) {
        r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev, i * 2);
 

@@ -316,12 +322,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
        }
    }
 

Quoted 8 lines

Review int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, · line 325

Quoted 7 lines

(quoted diff) +0 -6

-    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
-    if (r < 0) {
-        error_report("Error binding guest notifier: %d", -r);
-        goto err;
-    }
-
    return 0;

I wonder if k->set_guest_notifiers should be called after "hdev->started = true;" in vhost_dev_start.

Michael, can we just remove those assertions? Since you may want to set guest notifiers before starting the backend.

Another question for virtio_pci_vector_poll(): why not using msix_notify() instead of msix_set_pending().

If so, there's no need to
change the vhost_net_start() ?

Zhang Jie, is this a regression? If yes, could you please do a bisection to find the first bad commit.

Thanks

Quoted 43 lines · Jason Wang

Quoted 42 lines · Zhangjie (HZ)

Quoted 5 lines · Zhangjie (HZ)

Quoted 4 lines · Michael S. Tsirkin

Quoted 3 lines · Zhangjie (HZ)

Jason is right, the new order is not the cause of network unreachable. Changing order seems not work. After about 40 times, the problem occurs again. Maybe there is other hidden reasons for that.

Quoted 9 lines

I modified the code to change the order myself yesterday. This result is about my code.

Quoted 5 lines

To make sure, you tested the patch that I posted to list:
"vhost_net: stop guest notifiers after backend"?

Please confirm.

OK, I will test with your patch "vhost_net: stop guest notifiers after backend".

Unfortunately, after using the patch "vhost_net: stop guest notifiers after backend", Linux VMs stopt themselves a few minutes after they were started.

Quoted 14 lines

(quoted diff) +6 -0

@@ -308,6 +308,12 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
        goto err;
    }
 
+    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
+    if (r < 0) {
+        error_report("Error binding guest notifier: %d", -r);
+        goto err;
+    }
+
    for (i = 0; i < total_queues; i++) {
        r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev, i * 2);
 

Review int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, · line 325

Quoted 10 lines

(quoted diff) +0 -6

@@ -316,12 +322,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
        }
    }
 
-    r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
-    if (r < 0) {
-        error_report("Error binding guest notifier: %d", -r);
-        goto err;
-    }
-
    return 0;

I wonder if k->set_guest_notifiers should be called after "hdev->started = true;" in vhost_dev_start.

Quoted 2 lines

Michael, can we just remove those assertions? Since you may want to set guest notifiers before starting the backend.

Which assertions?

Quoted 2 lines

Another question for virtio_pci_vector_poll(): why not using msix_notify() instead of msix_set_pending().

We can do that but the effect will be same since we know vector is masked.

Quoted 3 lines

If so, there's no need to
change the vhost_net_start() ?

Confused, don't see the connection.

Quoted 4 lines

Zhang Jie, is this a regression? If yes, could you please do a bisection to find the first bad commit.

Quoted 1 line

Thanks

Pretty sure it's the mq patch: a9f98bb5ebe6fb1869321dcc58e72041ae626ad8

Since we may have many vhost/net devices for a virtio-net device. The setting of guest notifiers were moved out of the starting/stopping of a specific vhost thread. The vhost_net_{start|stop}() were renamed to vhost_net_{start|stop}_one(), and a new vhost_net_{start|stop}() were introduced to configure the guest notifiers and start/stop all vhost/vhost_net devices.

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

I wonder if k->set_guest_notifiers should be called after "hdev->started = true;" in vhost_dev_start.

Michael, can we just remove those assertions? Since you may want to set guest notifiers before starting the backend.

Which assertions?

I mean assert(hdev->started) in vhost.c. Your patch may hit them.

Quoted 5 lines · Jason Wang

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

I wonder if k->set_guest_notifiers should be called after "hdev->started = true;" in vhost_dev_start.

Michael, can we just remove those assertions? Since you may want to set guest notifiers before starting the backend.

Which assertions?

I mean assert(hdev->started) in vhost.c. Your patch may hit them.

I don't follow, but since my patch doesn't help anyway, pls go ahead and post your idea in form of a patch, will be clearer and can be tested.