From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47137) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2eQV-0008Ss-W6 for qemu-devel@nongnu.org; Wed, 29 Feb 2012 02:58:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S2eQQ-0001GF-Mb for qemu-devel@nongnu.org; Wed, 29 Feb 2012 02:58:19 -0500 Received: from fmmailgate05.web.de ([217.72.192.243]:41960) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2eQQ-0001FZ-Cp for qemu-devel@nongnu.org; Wed, 29 Feb 2012 02:58:14 -0500 Received: from moweb001.kundenserver.de (moweb001.kundenserver.de [172.19.20.114]) by fmmailgate05.web.de (Postfix) with ESMTP id 6A3146A99DF8 for ; Wed, 29 Feb 2012 08:58:12 +0100 (CET) Message-ID: <4F4DDA8B.60105@web.de> Date: Wed, 29 Feb 2012 08:58:03 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <0715077165dcc37924cd9113ea7ba23ddb043a22.1329493546.git.jan.kiszka@siemens.com> <4F4D52C2.7050304@weilnetz.de> <4F4D5AA1.5050309@web.de> In-Reply-To: <4F4D5AA1.5050309@web.de> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig5F432087B071FC119C00F848" Subject: Re: [Qemu-devel] [PATCH 2/3] slirp: Fix requeuing of batchq packets in if_start List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: Zhi Yong Wu , qemu-devel@nongnu.org, Fabien Chouteau This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig5F432087B071FC119C00F848 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2012-02-28 23:52, Jan Kiszka wrote: > On 2012-02-28 23:18, Stefan Weil wrote: >> Am 17.02.2012 16:45, schrieb Jan Kiszka: >>> In case we requeued a packet that was the head of a longer session >>> queue, we failed to restore this ordering. Also, we did not properly >>> deal with changes to Slirp::next_m. >>> >>> Instead of a cumbersome roll back, this fix simply avoids any changes= >>> until we know if the packet was actually sent. Both fixes crashes due= >>> to inconsistent queues and simplifies the logic. >>> >>> Thanks to Zhi Yong Wu who found the reason for these crashes. >>> >>> CC: Zhi Yong Wu >>> CC: Fabien Chouteau >>> Signed-off-by: Jan Kiszka >>> --- >>> slirp/if.c | 35 +++++++++++++++++++---------------- >>> 1 files changed, 19 insertions(+), 16 deletions(-) >> >> Latest QEMU crashed here 4 times with MIPS Malta >> when I tried 'apt-get update' in the guest. See gdb output >> below for details. >> >> I only got the crash with big endian MIPS, not with little >> endian which is strange. >> >> After I reverted the above patch, MIPS Malta worked >> again as before. >> >> So maybe we changed one crash against a new one. >=20 > Embarrassing. >=20 > Does this help? Specifically expired packet handling is broken. >=20 > diff --git a/slirp/if.c b/slirp/if.c > index 33f08e1..954ef1e 100644 > --- a/slirp/if.c > +++ b/slirp/if.c > @@ -153,7 +153,7 @@ void if_start(Slirp *slirp) > { > uint64_t now =3D qemu_get_clock_ns(rt_clock); > int requeued =3D 0; > - bool from_batchq =3D false; > + bool from_batchq; > struct mbuf *ifm, *ifqt; > =20 > DEBUG_CALL("if_start"); > @@ -169,6 +169,7 @@ void if_start(Slirp *slirp) > */ > if (slirp->if_fastq.ifq_next !=3D &slirp->if_fastq) { > ifm =3D slirp->if_fastq.ifq_next; > + from_batchq =3D false; > } else { > /* Nothing on fastq, see if next_m is valid */ > if (slirp->next_m !=3D &slirp->if_batchq) { > @@ -176,14 +177,17 @@ void if_start(Slirp *slirp) > } else { > ifm =3D slirp->if_batchq.ifq_next; > } > - > from_batchq =3D true; > } > =20 > slirp->if_queued--; > =20 > /* Try to send packet unless it already expired */ > - if (ifm->expiration_date >=3D now && !if_encap(slirp, ifm)) { > + if (ifm->expiration_date < now) { > + m_free(ifm); > + continue; > + } > + if (!if_encap(slirp, ifm)) { > /* Packet is delayed due to pending ARP resolution */ > requeued++; > continue; >=20 > Jan Nonsense. We need to walk the queues properly and carefully. Will come up with a better version. Jan --------------enig5F432087B071FC119C00F848 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk9N2o4ACgkQitSsb3rl5xSPYQCg7LXe8U5IwJOJOHCIULXnUbu9 JBIAoJIT4MNufWej0e7/YKii2GcY2s6e =3XWD -----END PGP SIGNATURE----- --------------enig5F432087B071FC119C00F848--