diff mbox series

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

Message ID 20240624081901.38956-1-east.moutain.yang@gmail.com
State New
Headers show
Series [v3] vritio-net: Notify the guest with the latest available idx | expand

Commit Message

Wencheng Yang June 24, 2024, 8:19 a.m. UTC
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(+)

Comments

Michael S. Tsirkin June 24, 2024, 10:22 a.m. UTC | #1
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
diff mbox series

Patch

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;
         }
     }