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

[Max Filippov]

On Mon, Jul 6, 2015 at 6:36 PM, Stefan Hajnoczi <address@hidden> 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 <address@hidden> wrote:
>> > On Mon, 07/06 10:45, Max Filippov wrote:
>> >> On Mon, Jul 6, 2015 at 10:36 AM, Fam Zheng <address@hidden> wrote:
>> >> > On Mon, 07/06 10:27, Max Filippov wrote:
>> >> >> On Mon, Jul 6, 2015 at 4:55 AM, Fam Zheng <address@hidden> 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 <address@hidden>

-- 
Thanks.
-- Max