1. Patchew
  2. QEMU
  3. [RFC PATCH v5 00/26] vDPA shadow virtqueue
  4. View patch

[RFC PATCH v5 02/26] vhost: Fix last queue index of devices with no cvq

Eugenio Pérez posted 26 patches 4 years, 3 months ago
There is a newer version of this series
[RFC PATCH v5 02/26] vhost: Fix last queue index of devices with no cvq
Posted by Eugenio Pérez 4 years, 3 months ago 
The -1 assumes that all devices with no cvq have an spare vq allocated
for them, but with no offer of VIRTIO_NET_F_CTRL_VQ. This may not be the
case, and the device may have a pair number of queues.

To fix this, just resort to the lower even number of queues.

Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the virtio device")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/net/vhost_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 0d888f29a6..edf56a597f 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -330,7 +330,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
     NetClientState *peer;
 
     if (!cvq) {
-        last_index -= 1;
+        last_index &= ~1ULL;
     }
 
     if (!k->set_guest_notifiers) {
-- 
2.27.0
Re: [RFC PATCH v5 02/26] vhost: Fix last queue index of devices with no cvq
Posted by Juan Quintela 4 years, 3 months ago 
Eugenio Pérez <eperezma@redhat.com> wrote:
> The -1 assumes that all devices with no cvq have an spare vq allocated
> for them, but with no offer of VIRTIO_NET_F_CTRL_VQ. This may not be the
> case, and the device may have a pair number of queues.
                                  ^^^^
even

I know, I know, I am Spanish myself O:-)

> To fix this, just resort to the lower even number of queues.

I don't understand what you try to achieve here.

> Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the
> virtio device")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/net/vhost_net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 0d888f29a6..edf56a597f 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -330,7 +330,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
>      NetClientState *peer;
>  
>      if (!cvq) {
> -        last_index -= 1;
> +        last_index &= ~1ULL;
>      }

As far as I can see, that is a nop. last_index is defined as an int.

$ cat kk.c
#include <stdio.h>

int main(void)
{
	int i = 7;
	i &= -1ULL;
	printf("%d\n", i);
	i = 8;
	i &= -1ULL;
	printf("%d\n", i);
	i = 0;
	i &= -1ULL;
	printf("%d\n", i);
	i = -2;
	i &= -1ULL;
	printf("%d\n", i);
	return 0;
}
$ ./kk
7
8
0
-2
Re: [RFC PATCH v5 02/26] vhost: Fix last queue index of devices with no cvq
Posted by Michael S. Tsirkin 4 years, 3 months ago 
On Tue, Nov 02, 2021 at 08:25:27AM +0100, Juan Quintela wrote:
> Eugenio Pérez <eperezma@redhat.com> wrote:
> > The -1 assumes that all devices with no cvq have an spare vq allocated
> > for them, but with no offer of VIRTIO_NET_F_CTRL_VQ. This may not be the
> > case, and the device may have a pair number of queues.
>                                   ^^^^
> even
> 
> I know, I know, I am Spanish myself O:-)

Nobody expects the Spanish ;)

> > To fix this, just resort to the lower even number of queues.
> 
> I don't understand what you try to achieve here.
> 
> > Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the
> > virtio device")
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  hw/net/vhost_net.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 0d888f29a6..edf56a597f 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -330,7 +330,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
> >      NetClientState *peer;
> >  
> >      if (!cvq) {
> > -        last_index -= 1;
> > +        last_index &= ~1ULL;
> >      }
> 
> As far as I can see, that is a nop. last_index is defined as an int.
> 
> $ cat kk.c
> #include <stdio.h>
> 
> int main(void)
> {
> 	int i = 7;
> 	i &= -1ULL;

Stefano's patch has ~1ULL , not -1ULL here.

> 	printf("%d\n", i);
> 	i = 8;
> 	i &= -1ULL;
> 	printf("%d\n", i);
> 	i = 0;
> 	i &= -1ULL;
> 	printf("%d\n", i);
> 	i = -2;
> 	i &= -1ULL;
> 	printf("%d\n", i);
> 	return 0;
> }
> $ ./kk
> 7
> 8
> 0
> -2
Re: [RFC PATCH v5 02/26] vhost: Fix last queue index of devices with no cvq
Posted by Juan Quintela 4 years, 3 months ago 
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Nov 02, 2021 at 08:25:27AM +0100, Juan Quintela wrote:
>> Eugenio Pérez <eperezma@redhat.com> wrote:
>> > The -1 assumes that all devices with no cvq have an spare vq allocated
>> > for them, but with no offer of VIRTIO_NET_F_CTRL_VQ. This may not be the
>> > case, and the device may have a pair number of queues.
>>                                   ^^^^
>> even
>> 
>> I know, I know, I am Spanish myself O:-)
>
> Nobody expects the Spanish ;)

O:-)

>> int main(void)
>> {
>> 	int i = 7;
>> 	i &= -1ULL;
>
> Stefano's patch has ~1ULL , not -1ULL here.
>

Stupid eyes.

Thanks, Juan.
Re: [RFC PATCH v5 02/26] vhost: Fix last queue index of devices with no cvq
Posted by Eugenio Perez Martin 4 years, 3 months ago 
On Tue, Nov 2, 2021 at 8:25 AM Juan Quintela <quintela@redhat.com> wrote:
>
> Eugenio Pérez <eperezma@redhat.com> wrote:
> > The -1 assumes that all devices with no cvq have an spare vq allocated
> > for them, but with no offer of VIRTIO_NET_F_CTRL_VQ. This may not be the
> > case, and the device may have a pair number of queues.
>                                   ^^^^
> even
>
> I know, I know, I am Spanish myself O:-)
>

Ouch! Good catch! :).

> > To fix this, just resort to the lower even number of queues.
>
> I don't understand what you try to achieve here.
>
> > Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the
> > virtio device")
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  hw/net/vhost_net.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 0d888f29a6..edf56a597f 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -330,7 +330,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
> >      NetClientState *peer;
> >
> >      if (!cvq) {
> > -        last_index -= 1;
> > +        last_index &= ~1ULL;
> >      }
>
> As far as I can see, that is a nop. last_index is defined as an int.
>
> $ cat kk.c
> #include <stdio.h>
>
> int main(void)
> {
>         int i = 7;
>         i &= -1ULL;
>         printf("%d\n", i);
>         i = 8;
>         i &= -1ULL;
>         printf("%d\n", i);
>         i = 0;
>         i &= -1ULL;
>         printf("%d\n", i);
>         i = -2;
>         i &= -1ULL;
>         printf("%d\n", i);
>         return 0;
> }
> $ ./kk
> 7
> 8
> 0
> -2
>

(Already answered by MST in another thread, but to be consistent here)
it's actually a ~1ULL :).

I will rewrite the patch message,

Thanks!
Re: [RFC PATCH v5 02/26] vhost: Fix last queue index of devices with no cvq
Posted by Juan Quintela 4 years, 3 months ago 
Eugenio Pérez <eperezma@redhat.com> wrote:
> The -1 assumes that all devices with no cvq have an spare vq allocated
> for them, but with no offer of VIRTIO_NET_F_CTRL_VQ. This may not be the
> case, and the device may have a pair number of queues.
>
> To fix this, just resort to the lower even number of queues.
>
> Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the
> virtio device")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.