| Message ID | 1466422360-25151-1-git-send-email-nikunj@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
On 20.06.2016 13:32, Nikunj A Dadhania wrote: > A bug crept in while doing the virtio 1.0 enablement in > commit 6e4d62c2 (virtio-net: enable virtio 1.0) > > + idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.used->idx); > > [...] > > - vq_rx.avail->ring[vq_rx.avail->idx % vq_rx.size] = id - 1; > + vq_rx.avail->ring[idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); > sync(); > - vq_rx.avail->idx += 1; > + vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, idx + 1); > > Should be using avail->idx in place of used->idx. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > lib/libvirtio/virtio-net.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/libvirtio/virtio-net.c b/lib/libvirtio/virtio-net.c > index fc620a2..2573031 100644 > --- a/lib/libvirtio/virtio-net.c > +++ b/lib/libvirtio/virtio-net.c > @@ -266,6 +266,7 @@ static int virtionet_receive(char *buf, int maxlen) > { > uint32_t len = 0; > uint32_t id, idx; > + uint16_t avail_idx; > > idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.used->idx); > > @@ -304,9 +305,10 @@ static int virtionet_receive(char *buf, int maxlen) > /* Move indices to next entries */ > last_rx_idx = last_rx_idx + 1; > > - vq_rx.avail->ring[idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); > + avail_idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.avail->idx); > + vq_rx.avail->ring[avail_idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); > sync(); > - vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, idx + 1); > + vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, avail_idx + 1); > > /* Tell HV that RX queue entry is ready */ > virtio_queue_notify(&virtiodev, VQ_RX); > You're right, that looks better this way! Reviewed-by: Thomas Huth <thuth@redhat.com>
On 22/06/16 05:41, Thomas Huth wrote: > On 20.06.2016 13:32, Nikunj A Dadhania wrote: >> A bug crept in while doing the virtio 1.0 enablement in >> commit 6e4d62c2 (virtio-net: enable virtio 1.0) >> >> + idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.used->idx); >> >> [...] >> >> - vq_rx.avail->ring[vq_rx.avail->idx % vq_rx.size] = id - 1; >> + vq_rx.avail->ring[idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); >> sync(); >> - vq_rx.avail->idx += 1; >> + vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, idx + 1); >> >> Should be using avail->idx in place of used->idx. Thanks, applied. >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> --- >> lib/libvirtio/virtio-net.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/lib/libvirtio/virtio-net.c b/lib/libvirtio/virtio-net.c >> index fc620a2..2573031 100644 >> --- a/lib/libvirtio/virtio-net.c >> +++ b/lib/libvirtio/virtio-net.c >> @@ -266,6 +266,7 @@ static int virtionet_receive(char *buf, int maxlen) >> { >> uint32_t len = 0; >> uint32_t id, idx; >> + uint16_t avail_idx; >> >> idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.used->idx); >> >> @@ -304,9 +305,10 @@ static int virtionet_receive(char *buf, int maxlen) >> /* Move indices to next entries */ >> last_rx_idx = last_rx_idx + 1; >> >> - vq_rx.avail->ring[idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); >> + avail_idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.avail->idx); >> + vq_rx.avail->ring[avail_idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); >> sync(); >> - vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, idx + 1); >> + vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, avail_idx + 1); >> >> /* Tell HV that RX queue entry is ready */ >> virtio_queue_notify(&virtiodev, VQ_RX); >> > > You're right, that looks better this way! > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > _______________________________________________ > SLOF mailing list > SLOF@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/slof >
diff --git a/lib/libvirtio/virtio-net.c b/lib/libvirtio/virtio-net.c index fc620a2..2573031 100644 --- a/lib/libvirtio/virtio-net.c +++ b/lib/libvirtio/virtio-net.c @@ -266,6 +266,7 @@ static int virtionet_receive(char *buf, int maxlen) { uint32_t len = 0; uint32_t id, idx; + uint16_t avail_idx; idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.used->idx); @@ -304,9 +305,10 @@ static int virtionet_receive(char *buf, int maxlen) /* Move indices to next entries */ last_rx_idx = last_rx_idx + 1; - vq_rx.avail->ring[idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); + avail_idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.avail->idx); + vq_rx.avail->ring[avail_idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); sync(); - vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, idx + 1); + vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, avail_idx + 1); /* Tell HV that RX queue entry is ready */ virtio_queue_notify(&virtiodev, VQ_RX);
A bug crept in while doing the virtio 1.0 enablement in commit 6e4d62c2 (virtio-net: enable virtio 1.0) + idx = virtio_modern16_to_cpu(&virtiodev, vq_rx.used->idx); [...] - vq_rx.avail->ring[vq_rx.avail->idx % vq_rx.size] = id - 1; + vq_rx.avail->ring[idx % vq_rx.size] = virtio_cpu_to_modern16(&virtiodev, id - 1); sync(); - vq_rx.avail->idx += 1; + vq_rx.avail->idx = virtio_cpu_to_modern16(&virtiodev, idx + 1); Should be using avail->idx in place of used->idx. Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- lib/libvirtio/virtio-net.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)