[PATCH v3] vritio-net: Notify the guest with the latest available idx

thomas posted 1 patch 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240624081901.38956-1-east.moutain.yang@gmail.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
hw/net/virtio-net.c | 1 +
1 file changed, 1 insertion(+)
[PATCH v3] vritio-net: Notify the guest with the latest available idx
Posted by thomas 5 months ago
Patch 06b12970174 ("virtio-net: fix network stall under load")
added double-check to test whether the available buffer size
can satisfy the request or not, in case the guest has added
some buffers to the avail ring simultaneously after the first
check.

It will be lucky if the available buffer size becomes okay
after the double-check, then the host can send the packet to
the guest. If the buffer size still can't satisfy the request,
even if the guest has added some buffers, notify the guest
with the latest available idx seen by the host, similiar to
the action taken by the host after the first check.

Fixes: 06b12970174 ("virtio-net: fix network stall under load")
Signed-off-by: wencheng Yang <east.moutain.yang@gmail.com>
---
 hw/net/virtio-net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9c7e85caea..23c6c8c898 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1654,6 +1654,7 @@ static int virtio_net_has_buffers(VirtIONetQueue *q, int bufsize)
         if (virtio_queue_empty(q->rx_vq) ||
             (n->mergeable_rx_bufs &&
              !virtqueue_avail_bytes(q->rx_vq, bufsize, 0))) {
+            virtio_queue_set_notification(q->rx_vq, 1);
             return 0;
         }
     }
-- 
2.39.0
Re: [PATCH v3] vritio-net: Notify the guest with the latest available idx
Posted by Michael S. Tsirkin 5 months ago
vritio -> virtio?

When I see a typo straight in the subject I get suspicious about
the patch itself.


On Mon, Jun 24, 2024 at 04:19:01PM +0800, thomas wrote:
> Patch 06b12970174 ("virtio-net: fix network stall under load")
> added double-check to test whether the available buffer size
> can satisfy the request or not, in case the guest has added
> some buffers to the avail ring simultaneously after the first
> check.
> 
> It will be lucky if the available buffer size becomes okay
> after the double-check, then the host can send the packet to
> the guest. If the buffer size still can't satisfy the request,
> even if the guest has added some buffers, notify the guest
> with the latest available idx seen by the host, similiar to
> the action taken by the host after the first check.

The action taken there is to enable host notifications.
And this is also what your code does.

Is this some kind of optimization? Bugfix? Can't tell from
commit log.


> Fixes: 06b12970174 ("virtio-net: fix network stall under load")
> Signed-off-by: wencheng Yang <east.moutain.yang@gmail.com>

Is this spelling with lowercase w in wencheng, intentional?


> ---

Add changelog here.


>  hw/net/virtio-net.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9c7e85caea..23c6c8c898 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1654,6 +1654,7 @@ static int virtio_net_has_buffers(VirtIONetQueue *q, int bufsize)
>          if (virtio_queue_empty(q->rx_vq) ||
>              (n->mergeable_rx_bufs &&
>               !virtqueue_avail_bytes(q->rx_vq, bufsize, 0))) {
> +            virtio_queue_set_notification(q->rx_vq, 1);



I don't really understand what is going on here.
Why is the previous virtio_queue_set_notification(q->rx_vq, 1)
insufficient?




>              return 0;
>          }
>      }
> -- 
> 2.39.0