From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MJU25-0007v1-5Q for qemu-devel@nongnu.org; Wed, 24 Jun 2009 11:05:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MJU1z-0007sD-B9 for qemu-devel@nongnu.org; Wed, 24 Jun 2009 11:05:03 -0400 Received: from [199.232.76.173] (port=52092 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MJU1y-0007s0-Pa for qemu-devel@nongnu.org; Wed, 24 Jun 2009 11:04:59 -0400 Received: from mail-ew0-f211.google.com ([209.85.219.211]:38947) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MJU1y-0008Tq-7Z for qemu-devel@nongnu.org; Wed, 24 Jun 2009 11:04:58 -0400 Received: by ewy7 with SMTP id 7so1176534ewy.34 for ; Wed, 24 Jun 2009 08:04:57 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4A423CA9.10109@siemens.com> References: <20090624124227.29402.750.stgit@mchn012c.ww002.siemens.net> <20090624124230.29402.75964.stgit@mchn012c.ww002.siemens.net> <5b31733c0906240734t3ae09c99u792e074489500f59@mail.gmail.com> <4A423CA9.10109@siemens.com> Date: Wed, 24 Jun 2009 17:04:56 +0200 Message-ID: <5b31733c0906240804s33560981leb557c5c4e2211db@mail.gmail.com> Subject: Re: [Qemu-devel] [PATCH 25/41] slirp: Make IP packet ID consistent From: Filip Navara Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Mark McLoughlin , Anthony Liguori , qemu-devel@nongnu.org On Wed, Jun 24, 2009 at 4:48 PM, Jan Kiszka wrote: > Filip Navara wrote: >> On Wed, Jun 24, 2009 at 2:42 PM, Jan Kiszka wrot= e: >>> Currently, ip_id is always initialized to 0 on slirp startup (despite >>> the broken attempt to derive it from the clock). This is good for >>> reproducibility. But it is not preserved across save/restore. This patc= h >>> therefore drops the dead initialization code from ip_init and introduce= s >>> ip_id to the persistent slirp state. >>> >>> Signed-off-by: Jan Kiszka >>> --- >>> >>> =A0slirp/ip_input.c | =A0 =A01 - >>> =A0slirp/slirp.c =A0 =A0| =A0 =A08 +++++++- >>> =A02 files changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/slirp/ip_input.c b/slirp/ip_input.c >>> index 0356eb5..b07d3d5 100644 >>> --- a/slirp/ip_input.c >>> +++ b/slirp/ip_input.c >>> @@ -59,7 +59,6 @@ void >>> =A0ip_init(void) >>> =A0{ >>> =A0 =A0 =A0 =A0ipq.ip_link.next =3D ipq.ip_link.prev =3D &ipq.ip_link; >>> - =A0 =A0 =A0 ip_id =3D tt.tv_sec & 0xffff; >> >> You removed the ip_id initialization and now it's never initialized in >> the code. That sounds wrong. > > Thanks for having a look! All this slirp code is really hairy and no fun > to dig through. Totally agreed. At one point I was considering to replace it with lwip, but I don't have the time necessary for that and it would be hard to keep all the existing services (TFTP, DHCP, SMB) working. > But now to your remark: That removal is in fact not changing the > behavior. ip_id is also initialized to 0 afterwards. To understand this > one has to track the messy use of 'tt' across slirp. It is in fact > carrying the time at some point (when select_fill is invoked), but not > yet on ip_init. Ah, I didn't see that, makes sense. It actually explains what I saw last week in the packet dump. > I was thinking about fixing the above initialization to actually take > the current time and derive ip_id, but then I thought it might be better > to keep this for network traffic reproducibility across qemu starts. Agreed. Best regards, Filip Navara