From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZADVr-0004Bp-Rz for qemu-devel@nongnu.org; Wed, 01 Jul 2015 04:37:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZADVn-00011e-HZ for qemu-devel@nongnu.org; Wed, 01 Jul 2015 04:36:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55140) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZADVn-00010x-Ag for qemu-devel@nongnu.org; Wed, 01 Jul 2015 04:36:55 -0400 Date: Wed, 1 Jul 2015 09:36:53 +0100 From: Stefan Hajnoczi Message-ID: <20150701083653.GA13991@stefanha-thinkpad.redhat.com> References: <1435161381-31521-1-git-send-email-thuth@redhat.com> <1435161381-31521-5-git-send-email-thuth@redhat.com> <20150626094459.GG15457@stefanha-thinkpad.redhat.com> <20150630123746.5e8626df@thh440s> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="LZvS9be/3tNcYl/X" Content-Disposition: inline In-Reply-To: <20150630123746.5e8626df@thh440s> Subject: Re: [Qemu-devel] [PATCH 4/5] net/dump: Add dump option for netdev devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: Stefan Hajnoczi , Jason Wang , qemu-devel@nongnu.org, Markus Armbruster --LZvS9be/3tNcYl/X Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 30, 2015 at 12:37:46PM +0200, Thomas Huth wrote: > On Fri, 26 Jun 2015 10:44:59 +0100 > Stefan Hajnoczi wrote: >=20 > > On Wed, Jun 24, 2015 at 05:56:20PM +0200, Thomas Huth wrote: > > > diff --git a/net/net.c b/net/net.c > > > index cc36c7b..8871b77 100644 > > > --- a/net/net.c > > > +++ b/net/net.c > > > @@ -568,6 +568,12 @@ ssize_t qemu_deliver_packet(NetClientState *send= er, > > > return 0; > > > } > > > =20 > > > + if (nc->netdev_dump_enabled) { > > > + net_dump_receive(nc, data, size); > > > + } else if (sender->netdev_dump_enabled) { > > > + net_dump_receive(sender, data, size); > > > + } > ... > > Perhaps dumping should happen after ->receive() has returned size. If > > ->receive() returns -1 the packet is discarded, and if it returns 0 the > > packet is queued (not delivered yet). > >=20 > > If you dump unconditionally before ->receive() you will see queued > > packets dumped multiple times (each time the queue gets flushed). >=20 > I've now tried this, but then I suddenly get the packets in the wrong > order in the dump file (when using slirp networking), e.g.: >=20 > 1 0.000000 10.0.2.2 -> 255.255.255.255 DHCP 590 DHCP Offer > 2 0.000012 0.0.0.0 -> 255.255.255.255 DHCP 342 DHCP Discover >=20 > instead of: >=20 > 1 0.000000 0.0.0.0 -> 255.255.255.255 DHCP 342 DHCP Discover > 2 0.000035 10.0.2.2 -> 255.255.255.255 DHCP 590 DHCP Offer >=20 > Looks like with slirp, the answer is already sent before the initial > receive() function returns? >=20 > Any idea how to avoid that issue? If not, I think I'll simply keep the > dump hooks before calling the receive functions. I see. So either way the packet capture will be misleading. Please leave add a comment to the code mentioning that either queued packets are duplicated or slirp traffic is out-of-order. That way we won't forget why the code is the way it is. Stefan --LZvS9be/3tNcYl/X Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVk6alAAoJEJykq7OBq3PIw2IH/iua5/+XvSNNUr2NsKQewuS8 7AKeFvkzaCAkk8K1Vq4026Tz97LCz/B5B2IpAL4YCj+8hbz0SVV3/wYb9P0gMY3+ aK9uvMLHuTclXA1ZzFlLedmRZuB197AScpTx8erT53fQND2QD5Cs4J2D4QwWtS24 ohkcpdmm5T8qlESSm+NghUYW7gHFQzJtL6ItOEenpp2Xzcc6D6wUhoAOoEFklYds W0ymgnQTFn5/hCKZEcflgc9KhJ1oxNnj6X2Ulcok/+SW2OvSU3EvNAAVuFU2ve5G NbZGaZafnoHIhw72TXYxx6LfFDw/ho18kH+AjRSgCOMd5F90/ysd5V2g/JJ7yS8= =KJQ1 -----END PGP SIGNATURE----- --LZvS9be/3tNcYl/X--