Re: [Qemu-devel] [PATCH v2 0/4] slirp: Fix for requeuing crash, cleanups

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-03-02 22:05, Stefan Weil wrote:
> Am 02.03.2012 19:57, schrieb Jan Kiszka:
>> Well, this requeuing bug seems to have a long breath. Previous attempts
>> to fix it (mine included) neglected the fact that we need to walk the
>> queue of pending packets, not just restart from the beginning after a
>> requeue. This version should get it Right(TM).
>>
>> This also comes with a fix for resource cleanups on slirp shutdown. At
>> least valgrind is happy now.
>>
>> Changes in v2:
>> - fixed corner case of session list walk that Stefan Weil reported
>>
>> CC: Fabien Chouteau <chouteau@adacore.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Stefan Weil <sw@weilnetz.de>
>> CC: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>
>> Jan Kiszka (4):
>> slirp: Keep next_m always valid
>> slirp: Fix queue walking in if_start
>> slirp: Remove unneeded if_queued
>> slirp: Cleanup resources on instance removal
>>
>> slirp/if.c | 64 +++++++++++++++++++++++++++++------------------------
>> slirp/ip_icmp.c | 7 ++++++
>> slirp/ip_icmp.h | 1 +
>> slirp/ip_input.c | 7 ++++++
>> slirp/mbuf.c | 21 +++++++++++++++++
>> slirp/mbuf.h | 1 +
>> slirp/slirp.c | 10 +++-----
>> slirp/slirp.h | 3 +-
>> slirp/tcp_subr.c | 7 ++++++
>> slirp/udp.c | 8 ++++++
>> slirp/udp.h | 1 +
>> 11 files changed, 94 insertions(+), 36 deletions(-)
> 
> Hi Jan,
> 
> this is what I get with your new patch series.
> 
> Regards,
> Stefan
> 
> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fffe9bf0700 (LWP 5863)]
> 0x00005555557781bf in slirp_remque (a=0x5555569916b0) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/misc.c:39
> 39        ((struct quehead *)(element->qh_rlink))->qh_link = 
> element->qh_link;
> (gdb) i s
> #0  0x00005555557781bf in slirp_remque (a=0x5555569916b0) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/misc.c:39
> #1  0x0000555555777b00 in m_get (slirp=0x5555562bdb80) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/mbuf.c:81
> #2  0x000055555577abdf in slirp_input (slirp=0x5555562bdb80, 
> pkt=0x555556305d58 "RU\n", pkt_len=54) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/slirp.c:673
> #3  0x0000555555730f8b in net_slirp_receive (nc=0x5555562bd950, 
> buf=0x555556305d58 "RU\n", size=54) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net/slirp.c:116
> #4  0x000055555572dc11 in qemu_vlan_deliver_packet 
> (sender=0x5555563074c0, flags=0, buf=0x555556305d58 "RU\n", size=54, 
> opaque=0x5555562bd8b0)
>      at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:451
> #5  0x0000555555730938 in qemu_net_queue_deliver (queue=0x5555562bd8f0, 
> sender=0x5555563074c0, flags=0, data=0x555556305d58 "RU\n", size=54)
>      at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net/queue.c:154
> #6  0x0000555555730a78 in qemu_net_queue_send (queue=0x5555562bd8f0, 
> sender=0x5555563074c0, flags=0, data=0x555556305d58 "RU\n", size=54, 
> sent_cb=0)
>      at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net/queue.c:188
> #7  0x000055555572de30 in qemu_send_packet_async_with_flags 
> (sender=0x5555563074c0, flags=0, buf=0x555556305d58 "RU\n", size=54, 
> sent_cb=0)
>      at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:519
> #8  0x000055555572de8b in qemu_send_packet_async (sender=0x5555563074c0, 
> buf=0x555556305d58 "RU\n", size=54, sent_cb=0) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:526
> #9  0x000055555572dedb in qemu_send_packet (vc=0x5555563074c0, 
> buf=0x555556305d58 "RU\n", size=54) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:532
> #10 0x00005555556e9daa in pcnet_transmit (s=0x555556305af8) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet.c:1258
> #11 0x00005555556ea0fd in pcnet_poll_timer (opaque=0x555556305af8) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet.c:1321
> #12 0x00005555556ea8e9 in pcnet_ioport_writew (opaque=0x555556305af8, 
> addr=18, val=0) at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet.c:1571
> #13 0x00005555556e62b3 in pcnet_ioport_write (opaque=0x555556305af8, 
> addr=18, data=0, size=2) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet-pci.c:120
> #14 0x0000555555801c8b in memory_region_write_accessor 
> (opaque=0x555556306d80, addr=18, value=0x7fffe9bef690, size=2, shift=0, 
> mask=65535)
>      at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:329
> #15 0x0000555555801d6d in access_with_adjusted_size (addr=18, 
> value=0x7fffe9bef690, size=2, access_size_min=1, access_size_max=4,
>      access=0x555555801c13 <memory_region_write_accessor>, 
> opaque=0x555556306d80) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:359
> #16 0x000055555580217d in memory_region_iorange_write 
> (iorange=0x555556306dc0, offset=18, width=2, data=0) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:428
> #17 0x00005555557fb41c in ioport_writew_thunk (opaque=0x555556306dc0, 
> addr=4146, data=0) at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/ioport.c:218
> #18 0x00005555557facb5 in ioport_write (index=1, address=4146, data=0) 
> at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/ioport.c:82
> #19 0x00005555557fb8a3 in cpu_outw (addr=4146, val=0) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/ioport.c:281
> #20 0x00005555556c7ae4 in isa_mmio_writew (opaque=0x0, addr=4146, val=0) 
> at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/isa_mmio.c:38
> #21 0x000055555580477f in memory_region_dispatch_write 
> (mr=0x5555562ffc38, addr=4146, data=0, size=2) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:913
> #22 0x0000555555807184 in io_mem_write (io_index=38, addr=4146, val=0, 
> size=2) at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:1502
> #23 0x000055555581d4e3 in io_writew (physaddr=4146, val=0, 
> addr=3087011890, retaddr=0x4034685f) at 
> /home/stefan/src/qemu/repo.or.cz/qemu/ar7/softmmu_template.h:225
> #24 0x000055555581d5cc in __stw_mmu (addr=3087011890, val=0, mmu_idx=0) 
> at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/softmmu_template.h:257
> #25 0x0000000040346860 in ?? ()
> #26 0x0000000000000000 in ?? ()
> (gdb) p ((struct quehead *)(element->qh_rlink))
> $1 = (struct quehead *) 0x0
> 

Grmbl. Was very hard to reproduce here (triggered once every few hours
with lots of interaction beforehand), but now I think I got the point
(recursion of if_start due to if_encap). Will rework the code to address
this.

Thanks for testing so far, will likely need your services again soon.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux