[Qemu-devel] TAP network breaks after debugger break-in

[Qemu-devel] TAP network breaks after debugger break-in

[Max Filippov]

Hello,

I'm using QEMU with TAP network and after the commit
0a2df857a703 "Merge remote-tracking branch
'remotes/stefanha/tags/net-pull-request' into staging"
I've noticed that activation of debugger connected to QEMU's
gdbstub during network I/O almost always breaks network
connection: network stops working completely after return
from the debugger.

Stefan, Fam, any hint on where to start debugging it?

-- 
Thanks.
-- Max

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Fam Zheng]

On Sat, 07/04 10:47, Max Filippov wrote:
> Hello,
> 
> I'm using QEMU with TAP network and after the commit
> 0a2df857a703 "Merge remote-tracking branch
> 'remotes/stefanha/tags/net-pull-request' into staging"
> I've noticed that activation of debugger connected to QEMU's
> gdbstub during network I/O almost always breaks network
> connection: network stops working completely after return
> from the debugger.
> 
> Stefan, Fam, any hint on where to start debugging it?
> 

Which NIC are you using?

Fam

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Max Filippov]

On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
> On Sat, 07/04 10:47, Max Filippov wrote:
>> Hello,
>>
>> I'm using QEMU with TAP network and after the commit
>> 0a2df857a703 "Merge remote-tracking branch
>> 'remotes/stefanha/tags/net-pull-request' into staging"
>> I've noticed that activation of debugger connected to QEMU's
>> gdbstub during network I/O almost always breaks network
>> connection: network stops working completely after return
>> from the debugger.
>>
>> Stefan, Fam, any hint on where to start debugging it?
>>
>
> Which NIC are you using?

opencores_eth.

-- 
Thanks.
-- Max

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Fam Zheng]

On Mon, 07/06 10:27, Max Filippov wrote:
> On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
> > On Sat, 07/04 10:47, Max Filippov wrote:
> >> Hello,
> >>
> >> I'm using QEMU with TAP network and after the commit
> >> 0a2df857a703 "Merge remote-tracking branch
> >> 'remotes/stefanha/tags/net-pull-request' into staging"
> >> I've noticed that activation of debugger connected to QEMU's
> >> gdbstub during network I/O almost always breaks network
> >> connection: network stops working completely after return
> >> from the debugger.
> >>
> >> Stefan, Fam, any hint on where to start debugging it?
> >>
> >
> > Which NIC are you using?
> 
> opencores_eth.
> 

Does reverting a90a7425cf592a3afeff3eaf32f543b83050ee5c work?

Fam

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Fam Zheng]

On Mon, 07/06 15:36, Fam Zheng wrote:
> On Mon, 07/06 10:27, Max Filippov wrote:
> > On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
> > > On Sat, 07/04 10:47, Max Filippov wrote:
> > >> Hello,
> > >>
> > >> I'm using QEMU with TAP network and after the commit
> > >> 0a2df857a703 "Merge remote-tracking branch
> > >> 'remotes/stefanha/tags/net-pull-request' into staging"
> > >> I've noticed that activation of debugger connected to QEMU's
> > >> gdbstub during network I/O almost always breaks network
> > >> connection: network stops working completely after return
> > >> from the debugger.
> > >>
> > >> Stefan, Fam, any hint on where to start debugging it?
> > >>
> > >
> > > Which NIC are you using?
> > 
> > opencores_eth.
> > 
> 
> Does reverting a90a7425cf592a3afeff3eaf32f543b83050ee5c work?
> 

Sorry, there are dependencies, I should say "bisect between a90a7425cf5 and
a90a7425cf5^" :)

Fam

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Max Filippov]

On Mon, Jul 6, 2015 at 10:36 AM, Fam Zheng <famz@redhat.com> wrote:
> On Mon, 07/06 10:27, Max Filippov wrote:
>> On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
>> > On Sat, 07/04 10:47, Max Filippov wrote:
>> >> Hello,
>> >>
>> >> I'm using QEMU with TAP network and after the commit
>> >> 0a2df857a703 "Merge remote-tracking branch
>> >> 'remotes/stefanha/tags/net-pull-request' into staging"
>> >> I've noticed that activation of debugger connected to QEMU's
>> >> gdbstub during network I/O almost always breaks network
>> >> connection: network stops working completely after return
>> >> from the debugger.
>> >>
>> >> Stefan, Fam, any hint on where to start debugging it?
>> >>
>> >
>> > Which NIC are you using?
>>
>> opencores_eth.
>>
>
> Does reverting a90a7425cf592a3afeff3eaf32f543b83050ee5c work?

Looks like it does, though the revert isn't clean.

-- 
Thanks.
-- Max

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Fam Zheng]

On Mon, 07/06 10:45, Max Filippov wrote:
> On Mon, Jul 6, 2015 at 10:36 AM, Fam Zheng <famz@redhat.com> wrote:
> > On Mon, 07/06 10:27, Max Filippov wrote:
> >> On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
> >> > On Sat, 07/04 10:47, Max Filippov wrote:
> >> >> Hello,
> >> >>
> >> >> I'm using QEMU with TAP network and after the commit
> >> >> 0a2df857a703 "Merge remote-tracking branch
> >> >> 'remotes/stefanha/tags/net-pull-request' into staging"
> >> >> I've noticed that activation of debugger connected to QEMU's
> >> >> gdbstub during network I/O almost always breaks network
> >> >> connection: network stops working completely after return
> >> >> from the debugger.
> >> >>
> >> >> Stefan, Fam, any hint on where to start debugging it?
> >> >>
> >> >
> >> > Which NIC are you using?
> >>
> >> opencores_eth.
> >>
> >
> > Does reverting a90a7425cf592a3afeff3eaf32f543b83050ee5c work?
> 
> Looks like it does, though the revert isn't clean.

I can't really tell what happened to opencores_eth because it has proper
open_eth_notify_can_receive() calls that should flush the queue as expected
since a90a7425cf592a3afeff3eaf32f543b83050ee5c (some other NICs are broken
because of missing such flushes).

FWIW, what is changed by that patch is that if NIC's .can_receive() callback
returns 0, it should later call qemu_flush_queued_packets() explicitly, when
conditions becomes ready again (i.e. .can_receive() would return true on next
invocation).

Fam

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Max Filippov]

On Mon, Jul 6, 2015 at 10:57 AM, Fam Zheng <famz@redhat.com> wrote:
> On Mon, 07/06 10:45, Max Filippov wrote:
>> On Mon, Jul 6, 2015 at 10:36 AM, Fam Zheng <famz@redhat.com> wrote:
>> > On Mon, 07/06 10:27, Max Filippov wrote:
>> >> On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
>> >> > On Sat, 07/04 10:47, Max Filippov wrote:
>> >> >> Hello,
>> >> >>
>> >> >> I'm using QEMU with TAP network and after the commit
>> >> >> 0a2df857a703 "Merge remote-tracking branch
>> >> >> 'remotes/stefanha/tags/net-pull-request' into staging"
>> >> >> I've noticed that activation of debugger connected to QEMU's
>> >> >> gdbstub during network I/O almost always breaks network
>> >> >> connection: network stops working completely after return
>> >> >> from the debugger.
>> >> >>
>> >> >> Stefan, Fam, any hint on where to start debugging it?
>> >> >>
>> >> >
>> >> > Which NIC are you using?
>> >>
>> >> opencores_eth.
>> >>
>> >
>> > Does reverting a90a7425cf592a3afeff3eaf32f543b83050ee5c work?
>>
>> Looks like it does, though the revert isn't clean.
>
> I can't really tell what happened to opencores_eth because it has proper
> open_eth_notify_can_receive() calls that should flush the queue as expected
> since a90a7425cf592a3afeff3eaf32f543b83050ee5c (some other NICs are broken
> because of missing such flushes).
>
> FWIW, what is changed by that patch is that if NIC's .can_receive() callback
> returns 0, it should later call qemu_flush_queued_packets() explicitly, when
> conditions becomes ready again (i.e. .can_receive() would return true on next
> invocation).

Ok, what I see is the following:

#0  tap_update_fd_handler (s=0x555555fbe290) at qemu/net/tap.c:69
#1  0x00005555556ec0d1 in tap_read_poll (s=0x555555fbe290,
enable=false) at qemu/net/tap.c:80
#2  0x00005555556ec68d in tap_send (opaque=0x555555fbe290) at qemu/net/tap.c:202

tap_read_poll is called with 'enable' set to false from tap_send, because size
returned from qemu_send_packet_async is 0, because of this piece of code
in the qemu_net_queue_send:

    if (queue->delivering || !qemu_can_send_packet(sender)) {
        qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
        return 0;
    }

Previously this function wasn't called after qemu_can_send_packet returns 0,
which was changed by the patch a90a7425cf592a.

tap_read_poll is never called with 'enable' set to true after that.

-- 
Thanks.
-- Max

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2841 bytes --]

On Mon, Jul 06, 2015 at 02:34:24PM +0300, Max Filippov wrote:
> On Mon, Jul 6, 2015 at 10:57 AM, Fam Zheng <famz@redhat.com> wrote:
> > On Mon, 07/06 10:45, Max Filippov wrote:
> >> On Mon, Jul 6, 2015 at 10:36 AM, Fam Zheng <famz@redhat.com> wrote:
> >> > On Mon, 07/06 10:27, Max Filippov wrote:
> >> >> On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
> >> >> > On Sat, 07/04 10:47, Max Filippov wrote:
> >> >> >> Hello,
> >> >> >>
> >> >> >> I'm using QEMU with TAP network and after the commit
> >> >> >> 0a2df857a703 "Merge remote-tracking branch
> >> >> >> 'remotes/stefanha/tags/net-pull-request' into staging"
> >> >> >> I've noticed that activation of debugger connected to QEMU's
> >> >> >> gdbstub during network I/O almost always breaks network
> >> >> >> connection: network stops working completely after return
> >> >> >> from the debugger.
> >> >> >>
> >> >> >> Stefan, Fam, any hint on where to start debugging it?
> >> >> >>
> >> >> >
> >> >> > Which NIC are you using?
> >> >>
> >> >> opencores_eth.
> >> >>
> >> >
> >> > Does reverting a90a7425cf592a3afeff3eaf32f543b83050ee5c work?
> >>
> >> Looks like it does, though the revert isn't clean.
> >
> > I can't really tell what happened to opencores_eth because it has proper
> > open_eth_notify_can_receive() calls that should flush the queue as expected
> > since a90a7425cf592a3afeff3eaf32f543b83050ee5c (some other NICs are broken
> > because of missing such flushes).
> >
> > FWIW, what is changed by that patch is that if NIC's .can_receive() callback
> > returns 0, it should later call qemu_flush_queued_packets() explicitly, when
> > conditions becomes ready again (i.e. .can_receive() would return true on next
> > invocation).
> 
> Ok, what I see is the following:
> 
> #0  tap_update_fd_handler (s=0x555555fbe290) at qemu/net/tap.c:69
> #1  0x00005555556ec0d1 in tap_read_poll (s=0x555555fbe290,
> enable=false) at qemu/net/tap.c:80
> #2  0x00005555556ec68d in tap_send (opaque=0x555555fbe290) at qemu/net/tap.c:202
> 
> tap_read_poll is called with 'enable' set to false from tap_send, because size
> returned from qemu_send_packet_async is 0, because of this piece of code
> in the qemu_net_queue_send:
> 
>     if (queue->delivering || !qemu_can_send_packet(sender)) {
>         qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
>         return 0;
>     }
> 
> Previously this function wasn't called after qemu_can_send_packet returns 0,
> which was changed by the patch a90a7425cf592a.
> 
> tap_read_poll is never called with 'enable' set to true after that.

The GDB stub starts/stops the VM.  Code for the runstate STOPPED ->
START transition was missing in the net subsystem.

Please try this patch:
http://patchwork.ozlabs.org/patch/489941/

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] TAP network breaks after debugger break-in

[Max Filippov]

On Mon, Jul 6, 2015 at 6:36 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Mon, Jul 06, 2015 at 02:34:24PM +0300, Max Filippov wrote:
>> On Mon, Jul 6, 2015 at 10:57 AM, Fam Zheng <famz@redhat.com> wrote:
>> > On Mon, 07/06 10:45, Max Filippov wrote:
>> >> On Mon, Jul 6, 2015 at 10:36 AM, Fam Zheng <famz@redhat.com> wrote:
>> >> > On Mon, 07/06 10:27, Max Filippov wrote:
>> >> >> On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <famz@redhat.com> wrote:
>> >> >> > On Sat, 07/04 10:47, Max Filippov wrote:
>> >> >> >> Hello,
>> >> >> >>
>> >> >> >> I'm using QEMU with TAP network and after the commit
>> >> >> >> 0a2df857a703 "Merge remote-tracking branch
>> >> >> >> 'remotes/stefanha/tags/net-pull-request' into staging"
>> >> >> >> I've noticed that activation of debugger connected to QEMU's
>> >> >> >> gdbstub during network I/O almost always breaks network
>> >> >> >> connection: network stops working completely after return
>> >> >> >> from the debugger.
>> >> >> >>
>> >> >> >> Stefan, Fam, any hint on where to start debugging it?
>> >> >> >>
>> >> >> >
>> >> >> > Which NIC are you using?
>> >> >>
>> >> >> opencores_eth.
>> >> >>
>> >> >
>> >> > Does reverting a90a7425cf592a3afeff3eaf32f543b83050ee5c work?
>> >>
>> >> Looks like it does, though the revert isn't clean.
>> >
>> > I can't really tell what happened to opencores_eth because it has proper
>> > open_eth_notify_can_receive() calls that should flush the queue as expected
>> > since a90a7425cf592a3afeff3eaf32f543b83050ee5c (some other NICs are broken
>> > because of missing such flushes).
>> >
>> > FWIW, what is changed by that patch is that if NIC's .can_receive() callback
>> > returns 0, it should later call qemu_flush_queued_packets() explicitly, when
>> > conditions becomes ready again (i.e. .can_receive() would return true on next
>> > invocation).
>>
>> Ok, what I see is the following:
>>
>> #0  tap_update_fd_handler (s=0x555555fbe290) at qemu/net/tap.c:69
>> #1  0x00005555556ec0d1 in tap_read_poll (s=0x555555fbe290,
>> enable=false) at qemu/net/tap.c:80
>> #2  0x00005555556ec68d in tap_send (opaque=0x555555fbe290) at qemu/net/tap.c:202
>>
>> tap_read_poll is called with 'enable' set to false from tap_send, because size
>> returned from qemu_send_packet_async is 0, because of this piece of code
>> in the qemu_net_queue_send:
>>
>>     if (queue->delivering || !qemu_can_send_packet(sender)) {
>>         qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
>>         return 0;
>>     }
>>
>> Previously this function wasn't called after qemu_can_send_packet returns 0,
>> which was changed by the patch a90a7425cf592a.
>>
>> tap_read_poll is never called with 'enable' set to true after that.
>
> The GDB stub starts/stops the VM.  Code for the runstate STOPPED ->
> START transition was missing in the net subsystem.
>
> Please try this patch:
> http://patchwork.ozlabs.org/patch/489941/

It helps, thank you!
Tested-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max