| Message ID | 1539266915-15216-10-git-send-email-wexu@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | packed ring virtio-net userspace backend support | expand |
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. Thanks > virtqueue_flush(q->rx_vq, i); > virtio_notify(vdev, q->rx_vq); >
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); > >
On 2018年10月15日 16:03, Wei Xu wrote: > 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. It will be an issue if they use virtqueue_fill() directly. Thanks > > Wei > >> Thanks >> >>> virtqueue_flush(q->rx_vq, i); >>> virtio_notify(vdev, q->rx_vq); >>
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); virtqueue_flush(q->rx_vq, i); virtio_notify(vdev, q->rx_vq);