1. Patchew
  2. QEMU
  3. View series

[PATCH] virtio-net: fix qemu set used ring flag even vhost started

Yajun Wu posted 1 patch 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240402045109.97729-1-yajunw@nvidia.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
hw/net/virtio-net.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] virtio-net: fix qemu set used ring flag even vhost started
Posted by Yajun Wu 4 weeks ago 
When vhost-user or vhost-kernel is handling virtio net datapath, qemu
should not touch used ring.

But with vhost-user socket reconnect scenario, in a very rare case (has
pending kick event). VRING_USED_F_NO_NOTIFY is set by qemu in
following code path:

	#0  virtio_queue_split_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:511
	#1  0x0000559d6dbf033b in virtio_queue_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:576
	#2  0x0000559d6dbbbdbc in virtio_net_handle_tx_bh (vdev=0x559d703a6aa0, vq=0x7ff5f4c920a8) at ../hw/net/virtio-net.c:2801
	#3  0x0000559d6dbf4791 in virtio_queue_notify_vq (vq=0x7ff5f4c920a8) at ../hw/virtio/virtio.c:2248
	#4  0x0000559d6dbf79da in virtio_queue_host_notifier_read (n=0x7ff5f4c9211c) at ../hw/virtio/virtio.c:3525
	#5  0x0000559d6d9a5814 in virtio_bus_cleanup_host_notifier (bus=0x559d703a6a20, n=1) at ../hw/virtio/virtio-bus.c:321
	#6  0x0000559d6dbf83c9 in virtio_device_stop_ioeventfd_impl (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3774
	#7  0x0000559d6d9a55c8 in virtio_bus_stop_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:259
	#8  0x0000559d6d9a53e8 in virtio_bus_grab_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:199
	#9  0x0000559d6dbf841c in virtio_device_grab_ioeventfd (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3783
	#10 0x0000559d6d9bde18 in vhost_dev_enable_notifiers (hdev=0x559d707edd70, vdev=0x559d703a6aa0) at ../hw/virtio/vhost.c:1592
	#11 0x0000559d6d89a0b8 in vhost_net_start_one (net=0x559d707edd70, dev=0x559d703a6aa0) at ../hw/net/vhost_net.c:266
	#12 0x0000559d6d89a6df in vhost_net_start (dev=0x559d703a6aa0, ncs=0x559d7048d890, data_queue_pairs=31, cvq=0) at ../hw/net/vhost_net.c:412
	#13 0x0000559d6dbb5b89 in virtio_net_vhost_status (n=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:311
	#14 0x0000559d6dbb5e34 in virtio_net_set_status (vdev=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:392
	#15 0x0000559d6dbb60d8 in virtio_net_set_link_status (nc=0x559d7048d890) at ../hw/net/virtio-net.c:455
	#16 0x0000559d6da64863 in qmp_set_link (name=0x559d6f0b83d0 "hostnet1", up=true, errp=0x7ffdd76569f0) at ../net/net.c:1459
	#17 0x0000559d6da7226e in net_vhost_user_event (opaque=0x559d6f0b83d0, event=CHR_EVENT_OPENED) at ../net/vhost-user.c:301
	#18 0x0000559d6ddc7f63 in chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:62
	#19 0x0000559d6ddc7fdc in qemu_chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:82

This issue causes guest kernel stop kicking device and traffic stop.

Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong
VRING_USED_F_NO_NOTIFY set.

Signed-off-by: Yajun Wu <yajunw@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
---
 hw/net/virtio-net.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a6ff000cd9..8035e01fdf 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2865,6 +2865,10 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
     VirtIONet *n = VIRTIO_NET(vdev);
     VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
 
+    if (n->vhost_started) {
+        return;
+    }
+
     if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) {
         virtio_net_drop_tx_queue_data(vdev, vq);
         return;
-- 
2.27.0
Re: [PATCH] virtio-net: fix qemu set used ring flag even vhost started
Posted by Michael Tokarev 3 weeks, 6 days ago 
02.04.2024 07:51, Yajun Wu:
> When vhost-user or vhost-kernel is handling virtio net datapath, qemu
> should not touch used ring.
> 
> But with vhost-user socket reconnect scenario, in a very rare case (has
> pending kick event). VRING_USED_F_NO_NOTIFY is set by qemu in
> following code path:
...
> This issue causes guest kernel stop kicking device and traffic stop.
> 
> Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong
> VRING_USED_F_NO_NOTIFY set.

This seems to be qemu-stable material, even if the case when the issue
happens is "very rare", -- is it not?

Thanks,

/mjt

> Signed-off-by: Yajun Wu <yajunw@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> ---
>   hw/net/virtio-net.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a6ff000cd9..8035e01fdf 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -2865,6 +2865,10 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>       VirtIONet *n = VIRTIO_NET(vdev);
>       VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>   
> +    if (n->vhost_started) {
> +        return;
> +    }
> +
>       if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) {
>           virtio_net_drop_tx_queue_data(vdev, vq);
>           return;
Re: [PATCH] virtio-net: fix qemu set used ring flag even vhost started
Posted by Yajun Wu 3 weeks, 6 days ago 
On 4/3/2024 1:10 PM, Michael Tokarev wrote:
> External email: Use caution opening links or attachments
>
>
> 02.04.2024 07:51, Yajun Wu:
>> When vhost-user or vhost-kernel is handling virtio net datapath, qemu
>> should not touch used ring.
>>
>> But with vhost-user socket reconnect scenario, in a very rare case (has
>> pending kick event). VRING_USED_F_NO_NOTIFY is set by qemu in
>> following code path:
> ...
>> This issue causes guest kernel stop kicking device and traffic stop.
>>
>> Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong
>> VRING_USED_F_NO_NOTIFY set.
> This seems to be qemu-stable material, even if the case when the issue
> happens is "very rare", -- is it not?
If you mean this fix needs back port to previous version? Yes.
> Thanks,
>
> /mjt
>
>> Signed-off-by: Yajun Wu <yajunw@nvidia.com>
>> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>>    hw/net/virtio-net.c | 4 ++++
>>    1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index a6ff000cd9..8035e01fdf 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -2865,6 +2865,10 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>        VirtIONet *n = VIRTIO_NET(vdev);
>>        VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>>
>> +    if (n->vhost_started) {
>> +        return;
>> +    }
>> +
>>        if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) {
>>            virtio_net_drop_tx_queue_data(vdev, vq);
>>            return;
Re: [PATCH] virtio-net: fix qemu set used ring flag even vhost started
Posted by Philippe Mathieu-Daudé 4 weeks ago 
Hi Yajun,

On 2/4/24 06:51, Yajun Wu wrote:
> When vhost-user or vhost-kernel is handling virtio net datapath, qemu

"QEMU"

> should not touch used ring.
> 
> But with vhost-user socket reconnect scenario, in a very rare case (has
> pending kick event). VRING_USED_F_NO_NOTIFY is set by qemu in

"QEMU"

> following code path:
> 
> 	#0  virtio_queue_split_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:511
> 	#1  0x0000559d6dbf033b in virtio_queue_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:576
> 	#2  0x0000559d6dbbbdbc in virtio_net_handle_tx_bh (vdev=0x559d703a6aa0, vq=0x7ff5f4c920a8) at ../hw/net/virtio-net.c:2801
> 	#3  0x0000559d6dbf4791 in virtio_queue_notify_vq (vq=0x7ff5f4c920a8) at ../hw/virtio/virtio.c:2248
> 	#4  0x0000559d6dbf79da in virtio_queue_host_notifier_read (n=0x7ff5f4c9211c) at ../hw/virtio/virtio.c:3525
> 	#5  0x0000559d6d9a5814 in virtio_bus_cleanup_host_notifier (bus=0x559d703a6a20, n=1) at ../hw/virtio/virtio-bus.c:321
> 	#6  0x0000559d6dbf83c9 in virtio_device_stop_ioeventfd_impl (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3774
> 	#7  0x0000559d6d9a55c8 in virtio_bus_stop_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:259
> 	#8  0x0000559d6d9a53e8 in virtio_bus_grab_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:199
> 	#9  0x0000559d6dbf841c in virtio_device_grab_ioeventfd (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3783
> 	#10 0x0000559d6d9bde18 in vhost_dev_enable_notifiers (hdev=0x559d707edd70, vdev=0x559d703a6aa0) at ../hw/virtio/vhost.c:1592
> 	#11 0x0000559d6d89a0b8 in vhost_net_start_one (net=0x559d707edd70, dev=0x559d703a6aa0) at ../hw/net/vhost_net.c:266
> 	#12 0x0000559d6d89a6df in vhost_net_start (dev=0x559d703a6aa0, ncs=0x559d7048d890, data_queue_pairs=31, cvq=0) at ../hw/net/vhost_net.c:412
> 	#13 0x0000559d6dbb5b89 in virtio_net_vhost_status (n=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:311
> 	#14 0x0000559d6dbb5e34 in virtio_net_set_status (vdev=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:392
> 	#15 0x0000559d6dbb60d8 in virtio_net_set_link_status (nc=0x559d7048d890) at ../hw/net/virtio-net.c:455
> 	#16 0x0000559d6da64863 in qmp_set_link (name=0x559d6f0b83d0 "hostnet1", up=true, errp=0x7ffdd76569f0) at ../net/net.c:1459
> 	#17 0x0000559d6da7226e in net_vhost_user_event (opaque=0x559d6f0b83d0, event=CHR_EVENT_OPENED) at ../net/vhost-user.c:301
> 	#18 0x0000559d6ddc7f63 in chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:62
> 	#19 0x0000559d6ddc7fdc in qemu_chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:82
> 
> This issue causes guest kernel stop kicking device and traffic stop.
> 
> Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong
> VRING_USED_F_NO_NOTIFY set.
> 
> Signed-off-by: Yajun Wu <yajunw@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> ---
>   hw/net/virtio-net.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a6ff000cd9..8035e01fdf 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -2865,6 +2865,10 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>       VirtIONet *n = VIRTIO_NET(vdev);
>       VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>   
> +    if (n->vhost_started) {

Since you mentioned "in a very rare case", maybe use unlikely()?

> +        return;
> +    }
> +
>       if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) {
>           virtio_net_drop_tx_queue_data(vdev, vq);
>           return;
Re: [PATCH] virtio-net: fix qemu set used ring flag even vhost started
Posted by Yajun Wu 4 weeks ago 
On 4/2/2024 8:11 PM, Philippe Mathieu-Daudé wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Yajun,
>
> On 2/4/24 06:51, Yajun Wu wrote:
>> When vhost-user or vhost-kernel is handling virtio net datapath, qemu
> "QEMU"
Ack.
>> should not touch used ring.
>>
>> But with vhost-user socket reconnect scenario, in a very rare case (has
>> pending kick event). VRING_USED_F_NO_NOTIFY is set by qemu in
> "QEMU"
Ack.
>> following code path:
>>
>>        #0  virtio_queue_split_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:511
>>        #1  0x0000559d6dbf033b in virtio_queue_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:576
>>        #2  0x0000559d6dbbbdbc in virtio_net_handle_tx_bh (vdev=0x559d703a6aa0, vq=0x7ff5f4c920a8) at ../hw/net/virtio-net.c:2801
>>        #3  0x0000559d6dbf4791 in virtio_queue_notify_vq (vq=0x7ff5f4c920a8) at ../hw/virtio/virtio.c:2248
>>        #4  0x0000559d6dbf79da in virtio_queue_host_notifier_read (n=0x7ff5f4c9211c) at ../hw/virtio/virtio.c:3525
>>        #5  0x0000559d6d9a5814 in virtio_bus_cleanup_host_notifier (bus=0x559d703a6a20, n=1) at ../hw/virtio/virtio-bus.c:321
>>        #6  0x0000559d6dbf83c9 in virtio_device_stop_ioeventfd_impl (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3774
>>        #7  0x0000559d6d9a55c8 in virtio_bus_stop_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:259
>>        #8  0x0000559d6d9a53e8 in virtio_bus_grab_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:199
>>        #9  0x0000559d6dbf841c in virtio_device_grab_ioeventfd (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3783
>>        #10 0x0000559d6d9bde18 in vhost_dev_enable_notifiers (hdev=0x559d707edd70, vdev=0x559d703a6aa0) at ../hw/virtio/vhost.c:1592
>>        #11 0x0000559d6d89a0b8 in vhost_net_start_one (net=0x559d707edd70, dev=0x559d703a6aa0) at ../hw/net/vhost_net.c:266
>>        #12 0x0000559d6d89a6df in vhost_net_start (dev=0x559d703a6aa0, ncs=0x559d7048d890, data_queue_pairs=31, cvq=0) at ../hw/net/vhost_net.c:412
>>        #13 0x0000559d6dbb5b89 in virtio_net_vhost_status (n=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:311
>>        #14 0x0000559d6dbb5e34 in virtio_net_set_status (vdev=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:392
>>        #15 0x0000559d6dbb60d8 in virtio_net_set_link_status (nc=0x559d7048d890) at ../hw/net/virtio-net.c:455
>>        #16 0x0000559d6da64863 in qmp_set_link (name=0x559d6f0b83d0 "hostnet1", up=true, errp=0x7ffdd76569f0) at ../net/net.c:1459
>>        #17 0x0000559d6da7226e in net_vhost_user_event (opaque=0x559d6f0b83d0, event=CHR_EVENT_OPENED) at ../net/vhost-user.c:301
>>        #18 0x0000559d6ddc7f63 in chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:62
>>        #19 0x0000559d6ddc7fdc in qemu_chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:82
>>
>> This issue causes guest kernel stop kicking device and traffic stop.
>>
>> Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong
>> VRING_USED_F_NO_NOTIFY set.
>>
>> Signed-off-by: Yajun Wu <yajunw@nvidia.com>
>> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>>    hw/net/virtio-net.c | 4 ++++
>>    1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index a6ff000cd9..8035e01fdf 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -2865,6 +2865,10 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>        VirtIONet *n = VIRTIO_NET(vdev);
>>        VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>>
>> +    if (n->vhost_started) {
> Since you mentioned "in a very rare case", maybe use unlikely()?
Ack.
>
>> +        return;
>> +    }
>> +
>>        if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) {
>>            virtio_net_drop_tx_queue_data(vdev, vq);
>>            return;
Re: [PATCH] virtio-net: fix qemu set used ring flag even vhost started
Posted by Philippe Mathieu-Daudé 4 weeks ago 
On 2/4/24 14:41, Yajun Wu wrote:
> 
> On 4/2/2024 8:11 PM, Philippe Mathieu-Daudé wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Yajun,
>>
>> On 2/4/24 06:51, Yajun Wu wrote:
>>> When vhost-user or vhost-kernel is handling virtio net datapath, qemu
>> "QEMU"
> Ack.
>>> should not touch used ring.
>>>
>>> But with vhost-user socket reconnect scenario, in a very rare case (has
>>> pending kick event). VRING_USED_F_NO_NOTIFY is set by qemu in
>> "QEMU"
> Ack.
>>> following code path:
>>>
>>>        #0  virtio_queue_split_set_notification (vq=0x7ff5f4c920a8, 
>>> enable=0) at ../hw/virtio/virtio.c:511
>>>        #1  0x0000559d6dbf033b in virtio_queue_set_notification 
>>> (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:576
>>>        #2  0x0000559d6dbbbdbc in virtio_net_handle_tx_bh 
>>> (vdev=0x559d703a6aa0, vq=0x7ff5f4c920a8) at ../hw/net/virtio-net.c:2801
>>>        #3  0x0000559d6dbf4791 in virtio_queue_notify_vq 
>>> (vq=0x7ff5f4c920a8) at ../hw/virtio/virtio.c:2248
>>>        #4  0x0000559d6dbf79da in virtio_queue_host_notifier_read 
>>> (n=0x7ff5f4c9211c) at ../hw/virtio/virtio.c:3525
>>>        #5  0x0000559d6d9a5814 in virtio_bus_cleanup_host_notifier 
>>> (bus=0x559d703a6a20, n=1) at ../hw/virtio/virtio-bus.c:321
>>>        #6  0x0000559d6dbf83c9 in virtio_device_stop_ioeventfd_impl 
>>> (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3774
>>>        #7  0x0000559d6d9a55c8 in virtio_bus_stop_ioeventfd 
>>> (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:259
>>>        #8  0x0000559d6d9a53e8 in virtio_bus_grab_ioeventfd 
>>> (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:199
>>>        #9  0x0000559d6dbf841c in virtio_device_grab_ioeventfd 
>>> (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3783
>>>        #10 0x0000559d6d9bde18 in vhost_dev_enable_notifiers 
>>> (hdev=0x559d707edd70, vdev=0x559d703a6aa0) at ../hw/virtio/vhost.c:1592
>>>        #11 0x0000559d6d89a0b8 in vhost_net_start_one 
>>> (net=0x559d707edd70, dev=0x559d703a6aa0) at ../hw/net/vhost_net.c:266
>>>        #12 0x0000559d6d89a6df in vhost_net_start (dev=0x559d703a6aa0, 
>>> ncs=0x559d7048d890, data_queue_pairs=31, cvq=0) at 
>>> ../hw/net/vhost_net.c:412
>>>        #13 0x0000559d6dbb5b89 in virtio_net_vhost_status 
>>> (n=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:311
>>>        #14 0x0000559d6dbb5e34 in virtio_net_set_status 
>>> (vdev=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:392
>>>        #15 0x0000559d6dbb60d8 in virtio_net_set_link_status 
>>> (nc=0x559d7048d890) at ../hw/net/virtio-net.c:455
>>>        #16 0x0000559d6da64863 in qmp_set_link (name=0x559d6f0b83d0 
>>> "hostnet1", up=true, errp=0x7ffdd76569f0) at ../net/net.c:1459
>>>        #17 0x0000559d6da7226e in net_vhost_user_event 
>>> (opaque=0x559d6f0b83d0, event=CHR_EVENT_OPENED) at 
>>> ../net/vhost-user.c:301
>>>        #18 0x0000559d6ddc7f63 in chr_be_event (s=0x559d6f2ffea0, 
>>> event=CHR_EVENT_OPENED) at ../chardev/char.c:62
>>>        #19 0x0000559d6ddc7fdc in qemu_chr_be_event (s=0x559d6f2ffea0, 
>>> event=CHR_EVENT_OPENED) at ../chardev/char.c:82
>>>
>>> This issue causes guest kernel stop kicking device and traffic stop.
>>>
>>> Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong
>>> VRING_USED_F_NO_NOTIFY set.
>>>
>>> Signed-off-by: Yajun Wu <yajunw@nvidia.com>
>>> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>>> ---
>>>    hw/net/virtio-net.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index a6ff000cd9..8035e01fdf 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -2865,6 +2865,10 @@ static void 
>>> virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>>        VirtIONet *n = VIRTIO_NET(vdev);
>>>        VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>>>
>>> +    if (n->vhost_started) {
>> Since you mentioned "in a very rare case", maybe use unlikely()?
> Ack.

Thanks, queued squashing:

-    if (n->vhost_started) {
+    if (unlikely(n->vhost_started)) {
          return;
      }

>>
>>> +        return;
>>> +    }
>>> +
>>>        if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) {
>>>            virtio_net_drop_tx_queue_data(vdev, vq);
>>>            return;
Re: [PATCH] virtio-net: fix qemu set used ring flag even vhost started
Posted by Michael S. Tsirkin 4 weeks ago 
On Tue, Apr 02, 2024 at 12:51:09PM +0800, Yajun Wu wrote:
> When vhost-user or vhost-kernel is handling virtio net datapath, qemu
> should not touch used ring.
> 
> But with vhost-user socket reconnect scenario, in a very rare case (has
> pending kick event). VRING_USED_F_NO_NOTIFY is set by qemu in
> following code path:
> 
> 	#0  virtio_queue_split_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:511
> 	#1  0x0000559d6dbf033b in virtio_queue_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:576
> 	#2  0x0000559d6dbbbdbc in virtio_net_handle_tx_bh (vdev=0x559d703a6aa0, vq=0x7ff5f4c920a8) at ../hw/net/virtio-net.c:2801
> 	#3  0x0000559d6dbf4791 in virtio_queue_notify_vq (vq=0x7ff5f4c920a8) at ../hw/virtio/virtio.c:2248
> 	#4  0x0000559d6dbf79da in virtio_queue_host_notifier_read (n=0x7ff5f4c9211c) at ../hw/virtio/virtio.c:3525
> 	#5  0x0000559d6d9a5814 in virtio_bus_cleanup_host_notifier (bus=0x559d703a6a20, n=1) at ../hw/virtio/virtio-bus.c:321
> 	#6  0x0000559d6dbf83c9 in virtio_device_stop_ioeventfd_impl (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3774
> 	#7  0x0000559d6d9a55c8 in virtio_bus_stop_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:259
> 	#8  0x0000559d6d9a53e8 in virtio_bus_grab_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:199
> 	#9  0x0000559d6dbf841c in virtio_device_grab_ioeventfd (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3783
> 	#10 0x0000559d6d9bde18 in vhost_dev_enable_notifiers (hdev=0x559d707edd70, vdev=0x559d703a6aa0) at ../hw/virtio/vhost.c:1592
> 	#11 0x0000559d6d89a0b8 in vhost_net_start_one (net=0x559d707edd70, dev=0x559d703a6aa0) at ../hw/net/vhost_net.c:266
> 	#12 0x0000559d6d89a6df in vhost_net_start (dev=0x559d703a6aa0, ncs=0x559d7048d890, data_queue_pairs=31, cvq=0) at ../hw/net/vhost_net.c:412
> 	#13 0x0000559d6dbb5b89 in virtio_net_vhost_status (n=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:311
> 	#14 0x0000559d6dbb5e34 in virtio_net_set_status (vdev=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:392
> 	#15 0x0000559d6dbb60d8 in virtio_net_set_link_status (nc=0x559d7048d890) at ../hw/net/virtio-net.c:455
> 	#16 0x0000559d6da64863 in qmp_set_link (name=0x559d6f0b83d0 "hostnet1", up=true, errp=0x7ffdd76569f0) at ../net/net.c:1459
> 	#17 0x0000559d6da7226e in net_vhost_user_event (opaque=0x559d6f0b83d0, event=CHR_EVENT_OPENED) at ../net/vhost-user.c:301
> 	#18 0x0000559d6ddc7f63 in chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:62
> 	#19 0x0000559d6ddc7fdc in qemu_chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:82
> 
> This issue causes guest kernel stop kicking device and traffic stop.
> 
> Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong
> VRING_USED_F_NO_NOTIFY set.
> 
> Signed-off-by: Yajun Wu <yajunw@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>


Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/net/virtio-net.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a6ff000cd9..8035e01fdf 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -2865,6 +2865,10 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>      VirtIONet *n = VIRTIO_NET(vdev);
>      VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>  
> +    if (n->vhost_started) {
> +        return;
> +    }
> +
>      if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) {
>          virtio_net_drop_tx_queue_data(vdev, vq);
>          return;
> -- 
> 2.27.0

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.