hi,

subject should list the affected component, and be shorter.

ok, I will rewrite the subject:
"update the latest available idx seen by the host to event idx"

Fixes: 06b12970174 ("virtio-net: fix network stall under load")

this should come at the end.

I have submitted v2, it's at the end now.

and what exactly does this refer to?
>
Commit 06b12970174 double-checks whether the guest has made some
buffers available after the first check,  it will be lucky if the available
buffer
size can satisfy the request.

did this commit cause a regression of some sort?
>
No regression. If the buffer size still can't satisfy the request even if
the
guest has made some buffers.  this commit doesn't notify the latest
shadow_avail_idx seen by the host to the guest. Similar to the first
check, if the available buffer is not enough, notify the guest with the
updated shadow_avail_idx.

what does "double check" refer to?
>
it refers to the second nested if condition judgment in
virtio_net_has_buffers().

which makes sense why?  And which changes the correct behavious of what
> to a new behaviour of what which is better why?
>
Similar to the first check, if the buffer size still can't satisfy the
request, notify the
guest with the updated shadow_avail_idx, it's better than the old one.

On Mon, Jun 17, 2024 at 6:58 PM Michael S. Tsirkin <m...@redhat.com> wrote:

>
> Thanks for the patch!
> Yet something to improve:
>
>
>
> subject should list the affected component, and be shorter.
>
> On Thu, Jun 13, 2024 at 10:21:47AM +0800, thomas wrote:
> > Fixes: 06b12970174 ("virtio-net: fix network stall under load")
>
> this should come at the end. and what exactly does this
> refer to? did this commit cause a regression of some sort?
>
> > If guest has made some buffers available during double check,
>
> what does "double check" refer to?
>
> > but the total buffer size available is lower than @bufsize,
> > notify the guest with the latest available idx(event idx)
> > seen by the host.
>
> which makes sense why?  And which changes the correct behavious of what
> to a new behaviour of what which is better why?
>
> Pls review docs/devel/submitting-a-patch.rst and follow the
> process there.
>
>
>
> > ---
> >  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
>
>

Reply via email to