qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers
@ 2018-09-12  8:01 Claudio
  2018-09-12  8:22 ` Jason Wang
  2018-09-12 12:37 ` Michael S. Tsirkin
  0 siblings, 2 replies; 7+ messages in thread
From: Claudio @ 2018-09-12  8:01 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang; +Cc: qemu-devel

Hello Michael, Jason and all,

I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.

QNX 7 is not free software anymore unfortunately, with the
the last open source versions in the 6.x range.

I am using the official virtio-net guest driver from QNX 7.

During initialization I sporadically get this error message:

$ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 2048 -display none -nodefconfig -nodefaults -chardev stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon chardev=char0,mode=readline -netdev user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 qnx.img

virtio-net ctrl missing headers

and following that my host->guest UDP port forwarding does not work, that is,
the qemu process shows up as listening on the interface, but no packets appear in the guest.

This error during initialization does not appear every time I launch QEMU.
It appears to be more or less random.

Whenever the error does not appear, the interface works as expected, and port forwarding works.

Latest commit is 

19b599f7664b ("Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2'")

Thanks a lot for any advice!

Ciao,

Claudio

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers
  2018-09-12  8:01 [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers Claudio
@ 2018-09-12  8:22 ` Jason Wang
  2018-09-12 12:37 ` Michael S. Tsirkin
  1 sibling, 0 replies; 7+ messages in thread
From: Jason Wang @ 2018-09-12  8:22 UTC (permalink / raw)
  To: Claudio, Michael S. Tsirkin; +Cc: qemu-devel



On 2018年09月12日 16:01, Claudio wrote:
> Hello Michael, Jason and all,
>
> I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.
>
> QNX 7 is not free software anymore unfortunately, with the
> the last open source versions in the 6.x range.
>
> I am using the official virtio-net guest driver from QNX 7.
>
> During initialization I sporadically get this error message:
>
> $ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 2048 -display none -nodefconfig -nodefaults -chardev stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon chardev=char0,mode=readline -netdev user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 qnx.img
>
> virtio-net ctrl missing headers
>
> and following that my host->guest UDP port forwarding does not work, that is,
> the qemu process shows up as listening on the interface, but no packets appear in the guest.
>
> This error during initialization does not appear every time I launch QEMU.
> It appears to be more or less random.
>
> Whenever the error does not appear, the interface works as expected, and port forwarding works.
>
> Latest commit is
>
> 19b599f7664b ("Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2'")
>
> Thanks a lot for any advice!
>
> Ciao,
>
> Claudio
>

Hi:

Please try some early qemu version to make sure it's not a regression of 
qemu. And if 6.X works, it could be bug of guest driver.

Thanks

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers
  2018-09-12  8:01 [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers Claudio
  2018-09-12  8:22 ` Jason Wang
@ 2018-09-12 12:37 ` Michael S. Tsirkin
  2018-09-12 15:16   ` Claudio
  1 sibling, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2018-09-12 12:37 UTC (permalink / raw)
  To: Claudio; +Cc: Jason Wang, qemu-devel

On Wed, Sep 12, 2018 at 10:01:34AM +0200, Claudio wrote:
> Hello Michael, Jason and all,
> 
> I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.
> 
> QNX 7 is not free software anymore unfortunately, with the
> the last open source versions in the 6.x range.
> 
> I am using the official virtio-net guest driver from QNX 7.
> 
> During initialization I sporadically get this error message:
> 
> $ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 2048 -display none -nodefconfig -nodefaults -chardev stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon chardev=char0,mode=readline -netdev user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 qnx.img
> 
> virtio-net ctrl missing headers

This means a control buffer is sent either without the input element
or without an output element, or with a single byte output element.

> and following that my host->guest UDP port forwarding does not work, that is,
> the qemu process shows up as listening on the interface, but no packets appear in the guest.
> 
> This error during initialization does not appear every time I launch QEMU.
> It appears to be more or less random.
> 
> Whenever the error does not appear, the interface works as expected, and port forwarding works.
> 
> Latest commit is 
> 
> 19b599f7664b ("Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2'")
> 
> Thanks a lot for any advice!
> 
> Ciao,
> 
> Claudio

If it's easy to reproduce, you can try printing out all commands.
E.g.:

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f154756e85..34251273a9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -987,6 +987,10 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (!elem) {
             break;
         }
+        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
+                elem->in_num, iov_size(elem->in_sg, elem->in_num),
+                elem->out_num, iov_size(elem->out_sg, elem->out_num));
+
         if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
             iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
             virtio_error(vdev, "virtio-net ctrl missing headers");
@@ -1014,6 +1018,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
             status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
         }
+        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
+                ctrl.class, ctrl.cmd, iov_cnt, status);
+
 
         s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
         assert(s == sizeof(status));


You can also try replacing virtio_error with fprintf - that will
avoid wedging the device on an error and let you proceed
with debugging.

-- 
MST

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers
  2018-09-12 12:37 ` Michael S. Tsirkin
@ 2018-09-12 15:16   ` Claudio
  2018-09-12 15:31     ` Michael S. Tsirkin
  0 siblings, 1 reply; 7+ messages in thread
From: Claudio @ 2018-09-12 15:16 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Jason Wang, qemu-devel, paolo bonzini

Thank you both for your responses,

and ciao Paolo,

On 09/12/2018 02:37 PM, Michael S. Tsirkin wrote:
> On Wed, Sep 12, 2018 at 10:01:34AM +0200, Claudio wrote:
>> Hello Michael, Jason and all,
>>
>> I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.
>>
>> QNX 7 is not free software anymore unfortunately, with the
>> the last open source versions in the 6.x range.
>>
>> I am using the official virtio-net guest driver from QNX 7.
>>
>> During initialization I sporadically get this error message:
>>
>> $ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 2048 -display none -nodefconfig -nodefaults -chardev stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon chardev=char0,mode=readline -netdev user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 qnx.img
>>
>> virtio-net ctrl missing headers
> 
> This means a control buffer is sent either without the input element
> or without an output element, or with a single byte output element.
> 
>> and following that my host->guest UDP port forwarding does not work, that is,
>> the qemu process shows up as listening on the interface, but no packets appear in the guest.
>>
>> This error during initialization does not appear every time I launch QEMU.
>> It appears to be more or less random.
>>
>> Whenever the error does not appear, the interface works as expected, and port forwarding works.
>>
>> Latest commit is 
>>
>> 19b599f7664b ("Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2'")
>>
>> Thanks a lot for any advice!
>>
>> Ciao,
>>
>> Claudio
> 
> If it's easy to reproduce, you can try printing out all commands.
> E.g.:
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index f154756e85..34251273a9 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -987,6 +987,10 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>          if (!elem) {
>              break;
>          }
> +        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
> +                elem->in_num, iov_size(elem->in_sg, elem->in_num),
> +                elem->out_num, iov_size(elem->out_sg, elem->out_num));
> +
>          if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
>              iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
>              virtio_error(vdev, "virtio-net ctrl missing headers");
> @@ -1014,6 +1018,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>          } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
>              status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
>          }
> +        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
> +                ctrl.class, ctrl.cmd, iov_cnt, status);
> +
>  
>          s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
>          assert(s == sizeof(status));
> 
> 
> You can also try replacing virtio_error with fprintf - that will
> avoid wedging the device on an error and let you proceed
> with debugging.
> 

I managed to automate the detection of the error, and it does appear to be a regression from my point of view.

I did a git bisect, and you can see the result and log as follows:

>From 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3 Mon Sep 17 00:00:00 2001                   
From: Paolo Bonzini <pbonzini@redhat.com>                                                
Date: Wed, 11 Jan 2017 09:38:15 +0100                                                    
Subject: [PATCH] virtio-net: enable ioeventfd even if vhost=off                          
                                                                                         
virtio-net-pci does not enable ioeventfd for historical reasons (and                     
nobody ever checked whether it should be revisited).  Note that other                    
backends do enable ioeventfd for virtio-net.                                             
                                                                                         
However, it has a major effect on performance.  On Windows, throughput is                
_multiplied_ by 2 or 3 on TCP_STREAM (on small packets it is "only" a 30%                
improvement) and a little less so on TCP_MAERTS albeit still very much                   
statistically significant.  Latency also has a single digit improvement.                 
                                                                                         
This is not visible when using vhost, which forces ioeventfd=on, but it                  
is substantial without vhost.  In addition, also on Windows and with the                 
RHEL 7.3 kernel, APICv seems to slow down virtio-net performance a bit,                  
but the penalty with this patch goes from -25% to -7%.                                   
                                                                                         
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>                                       
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>                                         
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>                                       
---
 hw/virtio/virtio-pci.c | 2 +-                                                           
 1 file changed, 1 insertion(+), 1 deletion(-)                                           
                                                                                         
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c                             
index 854b8f2..8baaf2b 100644                                                            
--- a/hw/virtio/virtio-pci.c                                                             
+++ b/hw/virtio/virtio-pci.c                                                             
@@ -2278,7 +2278,7 @@ static const TypeInfo virtio_serial_pci_info = {
                                                                                         
 static Property virtio_net_properties[] = {                                             
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,                                 
-                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),                          
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),                           
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),                         
     DEFINE_PROP_END_OF_LIST(),                                                          
 };                                                                                      
--                                                                                       
2.7.4



git bisect start
# good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
# good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
# good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
# bad: [19b599f7664b2ebfd0f405fb79c14dd241557452] Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2' into staging
git bisect bad 19b599f7664b2ebfd0f405fb79c14dd241557452
# good: [c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7] block: m25p80: 4byte address mode
git bisect good c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7
# bad: [67b9c5d4f37ea373ebf9aad251883886e34bf2e1] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
git bisect bad 67b9c5d4f37ea373ebf9aad251883886e34bf2e1
# good: [6bbcb76301a72dc80c8d29af13d40bb9a759c9c6] MAINTAINERS: Remove obsolete stable branches
git bisect good 6bbcb76301a72dc80c8d29af13d40bb9a759c9c6
# bad: [ff79d5e939c38677a575e3493eb9b4d36eb21865] Merge remote-tracking branch 'remotes/xtensa/tags/20170306-xtensa' into staging
git bisect bad ff79d5e939c38677a575e3493eb9b4d36eb21865
# bad: [a0def594286d9110a6035e02eef558cf3cf5d847] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
git bisect bad a0def594286d9110a6035e02eef558cf3cf5d847
# good: [b6c08970bc989bfddcf830684ea7a96b7a4d62a7] Merge remote-tracking branch 'remotes/bkoppelmann/tags/pull-tricore-2017-01-11-2' into staging
git bisect good b6c08970bc989bfddcf830684ea7a96b7a4d62a7
# good: [c5fc89b36c0a167548ae7af40dc085707a7756d2] hw/intc/arm_gicv3: Implement gicv3_cpuif_virt_update()
git bisect good c5fc89b36c0a167548ae7af40dc085707a7756d2
# bad: [009fad7f4ccbbf3ea115e9196edde88c0a17c1c2] migration: Change name of live migration thread
git bisect bad 009fad7f4ccbbf3ea115e9196edde88c0a17c1c2
# bad: [ebe9383caefd56d519e965a5d87bca29f0aeffe3] target-hppa: Implement floating-point insns
git bisect bad ebe9383caefd56d519e965a5d87bca29f0aeffe3
# bad: [db655a9653af05aaa90f41e55433d41e398d4b75] Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
git bisect bad db655a9653af05aaa90f41e55433d41e398d4b75
# bad: [2943b53f682f54548e7ddcf2ebb6c6d12d8dc821] virtio: force VIRTIO_F_IOMMU_PLATFORM
git bisect bad 2943b53f682f54548e7ddcf2ebb6c6d12d8dc821
# bad: [a5b3ebfd23bc70fa68461dff1d7145ff65e07150] fw-cfg: bump "x-file-slots" to 0x20 for 2.9+ machine types
git bisect bad a5b3ebfd23bc70fa68461dff1d7145ff65e07150
# bad: [c471ad0e9bd46ca5f5c9c796e727230e043a091d] vhost_net: device IOTLB support
git bisect bad c471ad0e9bd46ca5f5c9c796e727230e043a091d
# bad: [332fa82d0963409fa14997a02639289afa226596] Revert "virtio: turn vq->notification into a nested counter"
git bisect bad 332fa82d0963409fa14997a02639289afa226596
# bad: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off
git bisect bad 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3
# first bad commit: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off


I reverted the commit on master, and this solves the problem for me.

Ciao,

Claudio

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers
  2018-09-12 15:16   ` Claudio
@ 2018-09-12 15:31     ` Michael S. Tsirkin
  2018-09-12 17:12       ` Claudio
  0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2018-09-12 15:31 UTC (permalink / raw)
  To: Claudio; +Cc: Jason Wang, qemu-devel, paolo bonzini

On Wed, Sep 12, 2018 at 05:16:38PM +0200, Claudio wrote:
> Thank you both for your responses,
> 
> and ciao Paolo,
> 
> On 09/12/2018 02:37 PM, Michael S. Tsirkin wrote:
> > On Wed, Sep 12, 2018 at 10:01:34AM +0200, Claudio wrote:
> >> Hello Michael, Jason and all,
> >>
> >> I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.
> >>
> >> QNX 7 is not free software anymore unfortunately, with the
> >> the last open source versions in the 6.x range.
> >>
> >> I am using the official virtio-net guest driver from QNX 7.
> >>
> >> During initialization I sporadically get this error message:
> >>
> >> $ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 2048 -display none -nodefconfig -nodefaults -chardev stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon chardev=char0,mode=readline -netdev user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 qnx.img
> >>
> >> virtio-net ctrl missing headers
> > 
> > This means a control buffer is sent either without the input element
> > or without an output element, or with a single byte output element.
> > 
> >> and following that my host->guest UDP port forwarding does not work, that is,
> >> the qemu process shows up as listening on the interface, but no packets appear in the guest.
> >>
> >> This error during initialization does not appear every time I launch QEMU.
> >> It appears to be more or less random.
> >>
> >> Whenever the error does not appear, the interface works as expected, and port forwarding works.
> >>
> >> Latest commit is 
> >>
> >> 19b599f7664b ("Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2'")
> >>
> >> Thanks a lot for any advice!
> >>
> >> Ciao,
> >>
> >> Claudio
> > 
> > If it's easy to reproduce, you can try printing out all commands.
> > E.g.:
> > 
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index f154756e85..34251273a9 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -987,6 +987,10 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >          if (!elem) {
> >              break;
> >          }
> > +        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
> > +                elem->in_num, iov_size(elem->in_sg, elem->in_num),
> > +                elem->out_num, iov_size(elem->out_sg, elem->out_num));
> > +
> >          if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
> >              iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
> >              virtio_error(vdev, "virtio-net ctrl missing headers");
> > @@ -1014,6 +1018,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >          } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> >              status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> >          }
> > +        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
> > +                ctrl.class, ctrl.cmd, iov_cnt, status);
> > +
> >  
> >          s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
> >          assert(s == sizeof(status));
> > 
> > 
> > You can also try replacing virtio_error with fprintf - that will
> > avoid wedging the device on an error and let you proceed
> > with debugging.
> > 
> 
> I managed to automate the detection of the error, and it does appear to be a regression from my point of view.
> 
> I did a git bisect, and you can see the result and log as follows:
> 
> >From 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3 Mon Sep 17 00:00:00 2001                   
> From: Paolo Bonzini <pbonzini@redhat.com>                                                
> Date: Wed, 11 Jan 2017 09:38:15 +0100                                                    
> Subject: [PATCH] virtio-net: enable ioeventfd even if vhost=off                          
>                                                                                          
> virtio-net-pci does not enable ioeventfd for historical reasons (and                     
> nobody ever checked whether it should be revisited).  Note that other                    
> backends do enable ioeventfd for virtio-net.                                             
>                                                                                          
> However, it has a major effect on performance.  On Windows, throughput is                
> _multiplied_ by 2 or 3 on TCP_STREAM (on small packets it is "only" a 30%                
> improvement) and a little less so on TCP_MAERTS albeit still very much                   
> statistically significant.  Latency also has a single digit improvement.                 
>                                                                                          
> This is not visible when using vhost, which forces ioeventfd=on, but it                  
> is substantial without vhost.  In addition, also on Windows and with the                 
> RHEL 7.3 kernel, APICv seems to slow down virtio-net performance a bit,                  
> but the penalty with this patch goes from -25% to -7%.                                   
>                                                                                          
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>                                       
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>                                         
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>                                       
> ---
>  hw/virtio/virtio-pci.c | 2 +-                                                           
>  1 file changed, 1 insertion(+), 1 deletion(-)                                           
>                                                                                          
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c                             
> index 854b8f2..8baaf2b 100644                                                            
> --- a/hw/virtio/virtio-pci.c                                                             
> +++ b/hw/virtio/virtio-pci.c                                                             
> @@ -2278,7 +2278,7 @@ static const TypeInfo virtio_serial_pci_info = {
>                                                                                          
>  static Property virtio_net_properties[] = {                                             
>      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,                                 
> -                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),                          
> +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),                           
>      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),                         
>      DEFINE_PROP_END_OF_LIST(),                                                          
>  };                                                                                      
> --                                                                                       
> 2.7.4
> 
> 
> 
> git bisect start
> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
> # bad: [19b599f7664b2ebfd0f405fb79c14dd241557452] Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2' into staging
> git bisect bad 19b599f7664b2ebfd0f405fb79c14dd241557452
> # good: [c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7] block: m25p80: 4byte address mode
> git bisect good c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7
> # bad: [67b9c5d4f37ea373ebf9aad251883886e34bf2e1] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
> git bisect bad 67b9c5d4f37ea373ebf9aad251883886e34bf2e1
> # good: [6bbcb76301a72dc80c8d29af13d40bb9a759c9c6] MAINTAINERS: Remove obsolete stable branches
> git bisect good 6bbcb76301a72dc80c8d29af13d40bb9a759c9c6
> # bad: [ff79d5e939c38677a575e3493eb9b4d36eb21865] Merge remote-tracking branch 'remotes/xtensa/tags/20170306-xtensa' into staging
> git bisect bad ff79d5e939c38677a575e3493eb9b4d36eb21865
> # bad: [a0def594286d9110a6035e02eef558cf3cf5d847] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
> git bisect bad a0def594286d9110a6035e02eef558cf3cf5d847
> # good: [b6c08970bc989bfddcf830684ea7a96b7a4d62a7] Merge remote-tracking branch 'remotes/bkoppelmann/tags/pull-tricore-2017-01-11-2' into staging
> git bisect good b6c08970bc989bfddcf830684ea7a96b7a4d62a7
> # good: [c5fc89b36c0a167548ae7af40dc085707a7756d2] hw/intc/arm_gicv3: Implement gicv3_cpuif_virt_update()
> git bisect good c5fc89b36c0a167548ae7af40dc085707a7756d2
> # bad: [009fad7f4ccbbf3ea115e9196edde88c0a17c1c2] migration: Change name of live migration thread
> git bisect bad 009fad7f4ccbbf3ea115e9196edde88c0a17c1c2
> # bad: [ebe9383caefd56d519e965a5d87bca29f0aeffe3] target-hppa: Implement floating-point insns
> git bisect bad ebe9383caefd56d519e965a5d87bca29f0aeffe3
> # bad: [db655a9653af05aaa90f41e55433d41e398d4b75] Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
> git bisect bad db655a9653af05aaa90f41e55433d41e398d4b75
> # bad: [2943b53f682f54548e7ddcf2ebb6c6d12d8dc821] virtio: force VIRTIO_F_IOMMU_PLATFORM
> git bisect bad 2943b53f682f54548e7ddcf2ebb6c6d12d8dc821
> # bad: [a5b3ebfd23bc70fa68461dff1d7145ff65e07150] fw-cfg: bump "x-file-slots" to 0x20 for 2.9+ machine types
> git bisect bad a5b3ebfd23bc70fa68461dff1d7145ff65e07150
> # bad: [c471ad0e9bd46ca5f5c9c796e727230e043a091d] vhost_net: device IOTLB support
> git bisect bad c471ad0e9bd46ca5f5c9c796e727230e043a091d
> # bad: [332fa82d0963409fa14997a02639289afa226596] Revert "virtio: turn vq->notification into a nested counter"
> git bisect bad 332fa82d0963409fa14997a02639289afa226596
> # bad: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off
> git bisect bad 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3
> # first bad commit: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off
> 
> 
> I reverted the commit on master, and this solves the problem for me.
> 
> Ciao,
> 
> Claudio

Well this seems to tell us this is some kind of race.
Does it change anything if we just retry?

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f154756e85..8d4e0cce61 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -987,12 +987,22 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (!elem) {
             break;
         }
+        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
+                elem->in_num, iov_size(elem->in_sg, elem->in_num),
+                elem->out_num, iov_size(elem->out_sg, elem->out_num));
+
         if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
             iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
+            virtqueue_unpop(vq, elem, 0);
+            g_free(elem);
+            continue;
+#if 0
+
             virtio_error(vdev, "virtio-net ctrl missing headers");
             virtqueue_detach_element(vq, elem, 0);
             g_free(elem);
             break;
+#endif
         }
 
         iov_cnt = elem->out_num;
@@ -1014,6 +1024,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
             status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
         }
+        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
+                ctrl.class, ctrl.cmd, iov_cnt, status);
+
 
         s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
         assert(s == sizeof(status));

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers
  2018-09-12 15:31     ` Michael S. Tsirkin
@ 2018-09-12 17:12       ` Claudio
  2018-09-12 17:26         ` Michael S. Tsirkin
  0 siblings, 1 reply; 7+ messages in thread
From: Claudio @ 2018-09-12 17:12 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Jason Wang, qemu-devel, paolo bonzini

Hi Michael,

On 09/12/2018 05:31 PM, Michael S. Tsirkin wrote:
> On Wed, Sep 12, 2018 at 05:16:38PM +0200, Claudio wrote:
>> Thank you both for your responses,
>>
>> and ciao Paolo,
>>
>> On 09/12/2018 02:37 PM, Michael S. Tsirkin wrote:
>>> On Wed, Sep 12, 2018 at 10:01:34AM +0200, Claudio wrote:
>>>> Hello Michael, Jason and all,
>>>>
>>>> I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.
>>>>
>>>> QNX 7 is not free software anymore unfortunately, with the
>>>> the last open source versions in the 6.x range.
>>>>
>>>> I am using the official virtio-net guest driver from QNX 7.
>>>>
>>>> During initialization I sporadically get this error message:
>>>>
>>>> $ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 2048 -display none -nodefconfig -nodefaults -chardev stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon chardev=char0,mode=readline -netdev user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 qnx.img
>>>>
>>>> virtio-net ctrl missing headers
>>>
>>> This means a control buffer is sent either without the input element
>>> or without an output element, or with a single byte output element.
>>>
>>>> and following that my host->guest UDP port forwarding does not work, that is,
>>>> the qemu process shows up as listening on the interface, but no packets appear in the guest.
>>>>
>>>> This error during initialization does not appear every time I launch QEMU.
>>>> It appears to be more or less random.
>>>>
>>>> Whenever the error does not appear, the interface works as expected, and port forwarding works.
>>>>
>>>> Latest commit is 
>>>>
>>>> 19b599f7664b ("Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2'")
>>>>
>>>> Thanks a lot for any advice!
>>>>
>>>> Ciao,
>>>>
>>>> Claudio
>>>
>>> If it's easy to reproduce, you can try printing out all commands.
>>> E.g.:
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index f154756e85..34251273a9 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -987,6 +987,10 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>>          if (!elem) {
>>>              break;
>>>          }
>>> +        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
>>> +                elem->in_num, iov_size(elem->in_sg, elem->in_num),
>>> +                elem->out_num, iov_size(elem->out_sg, elem->out_num));
>>> +
>>>          if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
>>>              iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
>>>              virtio_error(vdev, "virtio-net ctrl missing headers");
>>> @@ -1014,6 +1018,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>>          } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
>>>              status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
>>>          }
>>> +        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
>>> +                ctrl.class, ctrl.cmd, iov_cnt, status);
>>> +
>>>  
>>>          s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
>>>          assert(s == sizeof(status));
>>>
>>>
>>> You can also try replacing virtio_error with fprintf - that will
>>> avoid wedging the device on an error and let you proceed
>>> with debugging.
>>>
>>
>> I managed to automate the detection of the error, and it does appear to be a regression from my point of view.
>>
>> I did a git bisect, and you can see the result and log as follows:
>>
>> >From 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3 Mon Sep 17 00:00:00 2001                   
>> From: Paolo Bonzini <pbonzini@redhat.com>                                                
>> Date: Wed, 11 Jan 2017 09:38:15 +0100                                                    
>> Subject: [PATCH] virtio-net: enable ioeventfd even if vhost=off                          
>>                                                                                          
>> virtio-net-pci does not enable ioeventfd for historical reasons (and                     
>> nobody ever checked whether it should be revisited).  Note that other                    
>> backends do enable ioeventfd for virtio-net.                                             
>>                                                                                          
>> However, it has a major effect on performance.  On Windows, throughput is                
>> _multiplied_ by 2 or 3 on TCP_STREAM (on small packets it is "only" a 30%                
>> improvement) and a little less so on TCP_MAERTS albeit still very much                   
>> statistically significant.  Latency also has a single digit improvement.                 
>>                                                                                          
>> This is not visible when using vhost, which forces ioeventfd=on, but it                  
>> is substantial without vhost.  In addition, also on Windows and with the                 
>> RHEL 7.3 kernel, APICv seems to slow down virtio-net performance a bit,                  
>> but the penalty with this patch goes from -25% to -7%.                                   
>>                                                                                          
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>                                       
>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>                                         
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>                                       
>> ---
>>  hw/virtio/virtio-pci.c | 2 +-                                                           
>>  1 file changed, 1 insertion(+), 1 deletion(-)                                           
>>                                                                                          
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c                             
>> index 854b8f2..8baaf2b 100644                                                            
>> --- a/hw/virtio/virtio-pci.c                                                             
>> +++ b/hw/virtio/virtio-pci.c                                                             
>> @@ -2278,7 +2278,7 @@ static const TypeInfo virtio_serial_pci_info = {
>>                                                                                          
>>  static Property virtio_net_properties[] = {                                             
>>      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,                                 
>> -                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),                          
>> +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),                           
>>      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),                         
>>      DEFINE_PROP_END_OF_LIST(),                                                          
>>  };                                                                                      
>> --                                                                                       
>> 2.7.4
>>
>>
>>
>> git bisect start
>> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
>> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
>> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
>> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
>> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
>> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
>> # bad: [19b599f7664b2ebfd0f405fb79c14dd241557452] Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2' into staging
>> git bisect bad 19b599f7664b2ebfd0f405fb79c14dd241557452
>> # good: [c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7] block: m25p80: 4byte address mode
>> git bisect good c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7
>> # bad: [67b9c5d4f37ea373ebf9aad251883886e34bf2e1] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
>> git bisect bad 67b9c5d4f37ea373ebf9aad251883886e34bf2e1
>> # good: [6bbcb76301a72dc80c8d29af13d40bb9a759c9c6] MAINTAINERS: Remove obsolete stable branches
>> git bisect good 6bbcb76301a72dc80c8d29af13d40bb9a759c9c6
>> # bad: [ff79d5e939c38677a575e3493eb9b4d36eb21865] Merge remote-tracking branch 'remotes/xtensa/tags/20170306-xtensa' into staging
>> git bisect bad ff79d5e939c38677a575e3493eb9b4d36eb21865
>> # bad: [a0def594286d9110a6035e02eef558cf3cf5d847] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
>> git bisect bad a0def594286d9110a6035e02eef558cf3cf5d847
>> # good: [b6c08970bc989bfddcf830684ea7a96b7a4d62a7] Merge remote-tracking branch 'remotes/bkoppelmann/tags/pull-tricore-2017-01-11-2' into staging
>> git bisect good b6c08970bc989bfddcf830684ea7a96b7a4d62a7
>> # good: [c5fc89b36c0a167548ae7af40dc085707a7756d2] hw/intc/arm_gicv3: Implement gicv3_cpuif_virt_update()
>> git bisect good c5fc89b36c0a167548ae7af40dc085707a7756d2
>> # bad: [009fad7f4ccbbf3ea115e9196edde88c0a17c1c2] migration: Change name of live migration thread
>> git bisect bad 009fad7f4ccbbf3ea115e9196edde88c0a17c1c2
>> # bad: [ebe9383caefd56d519e965a5d87bca29f0aeffe3] target-hppa: Implement floating-point insns
>> git bisect bad ebe9383caefd56d519e965a5d87bca29f0aeffe3
>> # bad: [db655a9653af05aaa90f41e55433d41e398d4b75] Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
>> git bisect bad db655a9653af05aaa90f41e55433d41e398d4b75
>> # bad: [2943b53f682f54548e7ddcf2ebb6c6d12d8dc821] virtio: force VIRTIO_F_IOMMU_PLATFORM
>> git bisect bad 2943b53f682f54548e7ddcf2ebb6c6d12d8dc821
>> # bad: [a5b3ebfd23bc70fa68461dff1d7145ff65e07150] fw-cfg: bump "x-file-slots" to 0x20 for 2.9+ machine types
>> git bisect bad a5b3ebfd23bc70fa68461dff1d7145ff65e07150
>> # bad: [c471ad0e9bd46ca5f5c9c796e727230e043a091d] vhost_net: device IOTLB support
>> git bisect bad c471ad0e9bd46ca5f5c9c796e727230e043a091d
>> # bad: [332fa82d0963409fa14997a02639289afa226596] Revert "virtio: turn vq->notification into a nested counter"
>> git bisect bad 332fa82d0963409fa14997a02639289afa226596
>> # bad: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off
>> git bisect bad 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3
>> # first bad commit: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off
>>
>>
>> I reverted the commit on master, and this solves the problem for me.
>>
>> Ciao,
>>
>> Claudio
> 
> Well this seems to tell us this is some kind of race.
> Does it change anything if we just retry?
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index f154756e85..8d4e0cce61 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -987,12 +987,22 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>          if (!elem) {
>              break;
>          }
> +        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
> +                elem->in_num, iov_size(elem->in_sg, elem->in_num),
> +                elem->out_num, iov_size(elem->out_sg, elem->out_num));
> +
>          if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
>              iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
> +            virtqueue_unpop(vq, elem, 0);
> +            g_free(elem);
> +            continue;
> +#if 0
> +
>              virtio_error(vdev, "virtio-net ctrl missing headers");
>              virtqueue_detach_element(vq, elem, 0);
>              g_free(elem);
>              break;
> +#endif
>          }
>  
>          iov_cnt = elem->out_num;
> @@ -1014,6 +1024,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>          } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
>              status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
>          }
> +        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
> +                ctrl.class, ctrl.cmd, iov_cnt, status);
> +
>  
>          s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
>          assert(s == sizeof(status));
> 

I applied a slight variation of the above to make gcc happy:

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f154756..74bfc39 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -987,12 +987,22 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         if (!elem) {
             break;
         }
+        fprintf(stderr, "%s (%lu): in %u/%lu, out %u/%lu\n",
+                __func__, time(NULL),
+                elem->in_num, iov_size(elem->in_sg, elem->in_num),
+                elem->out_num, iov_size(elem->out_sg, elem->out_num));
+
         if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
             iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
+            virtqueue_unpop(vq, elem, 0);
+            g_free(elem);
+            continue;
+#if 0
             virtio_error(vdev, "virtio-net ctrl missing headers");
             virtqueue_detach_element(vq, elem, 0);
             g_free(elem);
             break;
+#endif
         }
 
         iov_cnt = elem->out_num;
@@ -1014,6 +1024,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
             status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
         }
+        fprintf(stderr, "%s (%lu): class 0x%x cmd 0x%x cnt %d status 0x%x\n",
+                __func__, time(NULL),
+                ctrl.class, ctrl.cmd, iov_cnt, status);
 
         s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
         assert(s == sizeof(status));
--

The result is actually that it loops forever. I left it running for some minutes.

If there is a race condition, maybe it already happened before we reach here?
The "out" parameters are 0/0 on unsuccessful runs, while they are 2/400 on success.

Here is a successful run (it sometimes happens). 

virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400
virtio_net_handle_ctrl (1536771438): class 0x0 cmd 0x0 cnt 1 status 0x0
virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400
virtio_net_handle_ctrl (1536771438): class 0x0 cmd 0x0 cnt 1 status 0x0
virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400
virtio_net_handle_ctrl (1536771438): class 0x1 cmd 0x0 cnt 1 status 0x1
virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400
virtio_net_handle_ctrl (1536771438): class 0x0 cmd 0x0 cnt 1 status 0x0

Here is an unsuccessful run - when it happens it then seems to loop forever though - retrying does not help:

virtio_net_handle_ctrl (1536771600): in 1/1, out 0/0
virtio_net_handle_ctrl (1536771600): in 1/1, out 0/0
virtio_net_handle_ctrl (1536771601): in 1/1, out 0/0
...
(repeats forever)

Thank you,

Ciao,

Claudio

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers
  2018-09-12 17:12       ` Claudio
@ 2018-09-12 17:26         ` Michael S. Tsirkin
  0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2018-09-12 17:26 UTC (permalink / raw)
  To: Claudio; +Cc: Jason Wang, qemu-devel, paolo bonzini

On Wed, Sep 12, 2018 at 07:12:58PM +0200, Claudio wrote:
> Hi Michael,
> 
> On 09/12/2018 05:31 PM, Michael S. Tsirkin wrote:
> > On Wed, Sep 12, 2018 at 05:16:38PM +0200, Claudio wrote:
> >> Thank you both for your responses,
> >>
> >> and ciao Paolo,
> >>
> >> On 09/12/2018 02:37 PM, Michael S. Tsirkin wrote:
> >>> On Wed, Sep 12, 2018 at 10:01:34AM +0200, Claudio wrote:
> >>>> Hello Michael, Jason and all,
> >>>>
> >>>> I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.
> >>>>
> >>>> QNX 7 is not free software anymore unfortunately, with the
> >>>> the last open source versions in the 6.x range.
> >>>>
> >>>> I am using the official virtio-net guest driver from QNX 7.
> >>>>
> >>>> During initialization I sporadically get this error message:
> >>>>
> >>>> $ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 2048 -display none -nodefconfig -nodefaults -chardev stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon chardev=char0,mode=readline -netdev user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 qnx.img
> >>>>
> >>>> virtio-net ctrl missing headers
> >>>
> >>> This means a control buffer is sent either without the input element
> >>> or without an output element, or with a single byte output element.
> >>>
> >>>> and following that my host->guest UDP port forwarding does not work, that is,
> >>>> the qemu process shows up as listening on the interface, but no packets appear in the guest.
> >>>>
> >>>> This error during initialization does not appear every time I launch QEMU.
> >>>> It appears to be more or less random.
> >>>>
> >>>> Whenever the error does not appear, the interface works as expected, and port forwarding works.
> >>>>
> >>>> Latest commit is 
> >>>>
> >>>> 19b599f7664b ("Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2'")
> >>>>
> >>>> Thanks a lot for any advice!
> >>>>
> >>>> Ciao,
> >>>>
> >>>> Claudio
> >>>
> >>> If it's easy to reproduce, you can try printing out all commands.
> >>> E.g.:
> >>>
> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>> index f154756e85..34251273a9 100644
> >>> --- a/hw/net/virtio-net.c
> >>> +++ b/hw/net/virtio-net.c
> >>> @@ -987,6 +987,10 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >>>          if (!elem) {
> >>>              break;
> >>>          }
> >>> +        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
> >>> +                elem->in_num, iov_size(elem->in_sg, elem->in_num),
> >>> +                elem->out_num, iov_size(elem->out_sg, elem->out_num));
> >>> +
> >>>          if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
> >>>              iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
> >>>              virtio_error(vdev, "virtio-net ctrl missing headers");
> >>> @@ -1014,6 +1018,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >>>          } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> >>>              status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> >>>          }
> >>> +        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
> >>> +                ctrl.class, ctrl.cmd, iov_cnt, status);
> >>> +
> >>>  
> >>>          s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
> >>>          assert(s == sizeof(status));
> >>>
> >>>
> >>> You can also try replacing virtio_error with fprintf - that will
> >>> avoid wedging the device on an error and let you proceed
> >>> with debugging.
> >>>
> >>
> >> I managed to automate the detection of the error, and it does appear to be a regression from my point of view.
> >>
> >> I did a git bisect, and you can see the result and log as follows:
> >>
> >> >From 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3 Mon Sep 17 00:00:00 2001                   
> >> From: Paolo Bonzini <pbonzini@redhat.com>                                                
> >> Date: Wed, 11 Jan 2017 09:38:15 +0100                                                    
> >> Subject: [PATCH] virtio-net: enable ioeventfd even if vhost=off                          
> >>                                                                                          
> >> virtio-net-pci does not enable ioeventfd for historical reasons (and                     
> >> nobody ever checked whether it should be revisited).  Note that other                    
> >> backends do enable ioeventfd for virtio-net.                                             
> >>                                                                                          
> >> However, it has a major effect on performance.  On Windows, throughput is                
> >> _multiplied_ by 2 or 3 on TCP_STREAM (on small packets it is "only" a 30%                
> >> improvement) and a little less so on TCP_MAERTS albeit still very much                   
> >> statistically significant.  Latency also has a single digit improvement.                 
> >>                                                                                          
> >> This is not visible when using vhost, which forces ioeventfd=on, but it                  
> >> is substantial without vhost.  In addition, also on Windows and with the                 
> >> RHEL 7.3 kernel, APICv seems to slow down virtio-net performance a bit,                  
> >> but the penalty with this patch goes from -25% to -7%.                                   
> >>                                                                                          
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>                                       
> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>                                         
> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>                                       
> >> ---
> >>  hw/virtio/virtio-pci.c | 2 +-                                                           
> >>  1 file changed, 1 insertion(+), 1 deletion(-)                                           
> >>                                                                                          
> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c                             
> >> index 854b8f2..8baaf2b 100644                                                            
> >> --- a/hw/virtio/virtio-pci.c                                                             
> >> +++ b/hw/virtio/virtio-pci.c                                                             
> >> @@ -2278,7 +2278,7 @@ static const TypeInfo virtio_serial_pci_info = {
> >>                                                                                          
> >>  static Property virtio_net_properties[] = {                                             
> >>      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,                                 
> >> -                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),                          
> >> +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),                           
> >>      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),                         
> >>      DEFINE_PROP_END_OF_LIST(),                                                          
> >>  };                                                                                      
> >> --                                                                                       
> >> 2.7.4
> >>
> >>
> >>
> >> git bisect start
> >> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
> >> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
> >> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
> >> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
> >> # good: [ec57db1630f9cdcd13c8c55acbc8daf5237aabf1] virtio-net: unbreak the minix guest
> >> git bisect good ec57db1630f9cdcd13c8c55acbc8daf5237aabf1
> >> # bad: [19b599f7664b2ebfd0f405fb79c14dd241557452] Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2' into staging
> >> git bisect bad 19b599f7664b2ebfd0f405fb79c14dd241557452
> >> # good: [c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7] block: m25p80: 4byte address mode
> >> git bisect good c0f3f6754a120abc3451ebf7a7c703fbaf7b29e7
> >> # bad: [67b9c5d4f37ea373ebf9aad251883886e34bf2e1] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
> >> git bisect bad 67b9c5d4f37ea373ebf9aad251883886e34bf2e1
> >> # good: [6bbcb76301a72dc80c8d29af13d40bb9a759c9c6] MAINTAINERS: Remove obsolete stable branches
> >> git bisect good 6bbcb76301a72dc80c8d29af13d40bb9a759c9c6
> >> # bad: [ff79d5e939c38677a575e3493eb9b4d36eb21865] Merge remote-tracking branch 'remotes/xtensa/tags/20170306-xtensa' into staging
> >> git bisect bad ff79d5e939c38677a575e3493eb9b4d36eb21865
> >> # bad: [a0def594286d9110a6035e02eef558cf3cf5d847] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
> >> git bisect bad a0def594286d9110a6035e02eef558cf3cf5d847
> >> # good: [b6c08970bc989bfddcf830684ea7a96b7a4d62a7] Merge remote-tracking branch 'remotes/bkoppelmann/tags/pull-tricore-2017-01-11-2' into staging
> >> git bisect good b6c08970bc989bfddcf830684ea7a96b7a4d62a7
> >> # good: [c5fc89b36c0a167548ae7af40dc085707a7756d2] hw/intc/arm_gicv3: Implement gicv3_cpuif_virt_update()
> >> git bisect good c5fc89b36c0a167548ae7af40dc085707a7756d2
> >> # bad: [009fad7f4ccbbf3ea115e9196edde88c0a17c1c2] migration: Change name of live migration thread
> >> git bisect bad 009fad7f4ccbbf3ea115e9196edde88c0a17c1c2
> >> # bad: [ebe9383caefd56d519e965a5d87bca29f0aeffe3] target-hppa: Implement floating-point insns
> >> git bisect bad ebe9383caefd56d519e965a5d87bca29f0aeffe3
> >> # bad: [db655a9653af05aaa90f41e55433d41e398d4b75] Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
> >> git bisect bad db655a9653af05aaa90f41e55433d41e398d4b75
> >> # bad: [2943b53f682f54548e7ddcf2ebb6c6d12d8dc821] virtio: force VIRTIO_F_IOMMU_PLATFORM
> >> git bisect bad 2943b53f682f54548e7ddcf2ebb6c6d12d8dc821
> >> # bad: [a5b3ebfd23bc70fa68461dff1d7145ff65e07150] fw-cfg: bump "x-file-slots" to 0x20 for 2.9+ machine types
> >> git bisect bad a5b3ebfd23bc70fa68461dff1d7145ff65e07150
> >> # bad: [c471ad0e9bd46ca5f5c9c796e727230e043a091d] vhost_net: device IOTLB support
> >> git bisect bad c471ad0e9bd46ca5f5c9c796e727230e043a091d
> >> # bad: [332fa82d0963409fa14997a02639289afa226596] Revert "virtio: turn vq->notification into a nested counter"
> >> git bisect bad 332fa82d0963409fa14997a02639289afa226596
> >> # bad: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off
> >> git bisect bad 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3
> >> # first bad commit: [4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3] virtio-net: enable ioeventfd even if vhost=off
> >>
> >>
> >> I reverted the commit on master, and this solves the problem for me.
> >>
> >> Ciao,
> >>
> >> Claudio
> > 
> > Well this seems to tell us this is some kind of race.
> > Does it change anything if we just retry?
> > 
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index f154756e85..8d4e0cce61 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -987,12 +987,22 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >          if (!elem) {
> >              break;
> >          }
> > +        fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
> > +                elem->in_num, iov_size(elem->in_sg, elem->in_num),
> > +                elem->out_num, iov_size(elem->out_sg, elem->out_num));
> > +
> >          if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
> >              iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
> > +            virtqueue_unpop(vq, elem, 0);
> > +            g_free(elem);
> > +            continue;
> > +#if 0
> > +
> >              virtio_error(vdev, "virtio-net ctrl missing headers");
> >              virtqueue_detach_element(vq, elem, 0);
> >              g_free(elem);
> >              break;
> > +#endif
> >          }
> >  
> >          iov_cnt = elem->out_num;
> > @@ -1014,6 +1024,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> >          } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> >              status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> >          }
> > +        fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
> > +                ctrl.class, ctrl.cmd, iov_cnt, status);
> > +
> >  
> >          s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
> >          assert(s == sizeof(status));
> > 
> 
> I applied a slight variation of the above to make gcc happy:
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index f154756..74bfc39 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -987,12 +987,22 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>          if (!elem) {
>              break;
>          }
> +        fprintf(stderr, "%s (%lu): in %u/%lu, out %u/%lu\n",
> +                __func__, time(NULL),
> +                elem->in_num, iov_size(elem->in_sg, elem->in_num),
> +                elem->out_num, iov_size(elem->out_sg, elem->out_num));
> +
>          if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
>              iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
> +            virtqueue_unpop(vq, elem, 0);
> +            g_free(elem);
> +            continue;
> +#if 0
>              virtio_error(vdev, "virtio-net ctrl missing headers");
>              virtqueue_detach_element(vq, elem, 0);
>              g_free(elem);
>              break;
> +#endif
>          }
>  
>          iov_cnt = elem->out_num;
> @@ -1014,6 +1024,9 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>          } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
>              status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
>          }
> +        fprintf(stderr, "%s (%lu): class 0x%x cmd 0x%x cnt %d status 0x%x\n",
> +                __func__, time(NULL),
> +                ctrl.class, ctrl.cmd, iov_cnt, status);
>  
>          s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
>          assert(s == sizeof(status));
> --
> 
> The result is actually that it loops forever. I left it running for some minutes.
> 
> If there is a race condition, maybe it already happened before we reach here?
> The "out" parameters are 0/0 on unsuccessful runs, while they are 2/400 on success.
> 
> Here is a successful run (it sometimes happens). 
> 
> virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400
> virtio_net_handle_ctrl (1536771438): class 0x0 cmd 0x0 cnt 1 status 0x0
> virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400
> virtio_net_handle_ctrl (1536771438): class 0x0 cmd 0x0 cnt 1 status 0x0
> virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400
> virtio_net_handle_ctrl (1536771438): class 0x1 cmd 0x0 cnt 1 status 0x1
> virtio_net_handle_ctrl (1536771438): in 1/1, out 2/400

This does not look good either. Try to step into
virtio_net_handle_mac
and see why does it fail?

> virtio_net_handle_ctrl (1536771438): class 0x0 cmd 0x0 cnt 1 status 0x0
> 
> Here is an unsuccessful run - when it happens it then seems to loop forever though - retrying does not help:
> 
> virtio_net_handle_ctrl (1536771600): in 1/1, out 0/0
> virtio_net_handle_ctrl (1536771600): in 1/1, out 0/0
> virtio_net_handle_ctrl (1536771601): in 1/1, out 0/0

Weird. It's of course possible that ioeventfd triggers
some race in the driver that does not trigger otherwise,
but it does seem suspicious.

Try to dump the descriptors as they are read?

> ...
> (repeats forever)
> 
> Thank you,
> 
> Ciao,
> 
> Claudio


diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f6a588ab57..6b5f30f9d4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -893,6 +893,8 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 
     desc_cache = &caches->desc;
     vring_desc_read(vdev, &desc, desc_cache, i);
+    fprintf(stderr, "flags 0x%x addr 0x%llx len 0x%x\n",
+                desc.flags,(unsigned long long)desc.addr, desc.len);
     if (desc.flags & VRING_DESC_F_INDIRECT) {
         if (desc.len % sizeof(VRingDesc)) {
             virtio_error(vdev, "Invalid size for indirect buffer table");

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-09-12 17:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-12  8:01 [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers Claudio
2018-09-12  8:22 ` Jason Wang
2018-09-12 12:37 ` Michael S. Tsirkin
2018-09-12 15:16   ` Claudio
2018-09-12 15:31     ` Michael S. Tsirkin
2018-09-12 17:12       ` Claudio
2018-09-12 17:26         ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).