Re: [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain

[Wei Xu <wexu@redhat.com>]

On Mon, Oct 15, 2018 at 03:45:46PM +0800, Jason Wang wrote:
> 
> 
> On 2018年10月11日 22:08, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >With the support of marking a descriptor used/unused in 'flags'
> >field for 1.1, the current way of filling a chained descriptors
> >does not work since driver side may get the wrong 'num_buffer'
> >information in case of the head descriptor has been filled in
> >while the subsequent ones are still in processing in device side.
> >
> >This patch fills the head one after done all the others one.
> >
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> >  hw/net/virtio-net.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >index 4bdd5b8..186c86cd2 100644
> >--- a/hw/net/virtio-net.c
> >+++ b/hw/net/virtio-net.c
> >@@ -1198,6 +1198,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> >      struct virtio_net_hdr_mrg_rxbuf mhdr;
> >      unsigned mhdr_cnt = 0;
> >      size_t offset, i, guest_offset;
> >+    VirtQueueElement head;
> >+    int head_len = 0;
> >      if (!virtio_net_can_receive(nc)) {
> >          return -1;
> >@@ -1275,7 +1277,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> >          }
> >          /* signal other side */
> >-        virtqueue_fill(q->rx_vq, elem, total, i++);
> >+        if (i == 0) {
> >+            head_len = total;
> >+            head = *elem;
> >+        } else {
> >+            virtqueue_fill(q->rx_vq, elem, len, i);
> >+        }
> >+        i++;
> >          g_free(elem);
> >      }
> >@@ -1286,6 +1294,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> >                       &mhdr.num_buffers, sizeof mhdr.num_buffers);
> >      }
> >+    virtqueue_fill(q->rx_vq, &head, head_len, 0);
> 
> It's not a good idea to fix API in device implementation. Let's introduce
> new API and fix it there.
> 
> E.g virtqueue_fill_n() and update the flag of first elem at the last step.

OK, I haven't considered about the other devices so far.

Wei

> 
> Thanks
> 
> >      virtqueue_flush(q->rx_vq, i);
> >      virtio_notify(vdev, q->rx_vq);
> 
>