[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 25/41] slirp: Make IP packet ID consistent
From: |
Filip Navara |
Subject: |
Re: [Qemu-devel] [PATCH 25/41] slirp: Make IP packet ID consistent |
Date: |
Wed, 24 Jun 2009 17:04:56 +0200 |
On Wed, Jun 24, 2009 at 4:48 PM, Jan Kiszka<address@hidden> wrote:
> Filip Navara wrote:
>> On Wed, Jun 24, 2009 at 2:42 PM, Jan Kiszka<address@hidden> wrote:
>>> 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 patch
>>> therefore drops the dead initialization code from ip_init and introduces
>>> ip_id to the persistent slirp state.
>>>
>>> Signed-off-by: Jan Kiszka <address@hidden>
>>> ---
>>>
>>> slirp/ip_input.c | 1 -
>>> slirp/slirp.c | 8 +++++++-
>>> 2 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
>>> ip_init(void)
>>> {
>>> ipq.ip_link.next = ipq.ip_link.prev = &ipq.ip_link;
>>> - ip_id = 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
- [Qemu-devel] [PATCH 23/41] slirp: tftp: Rework filename handling, (continued)
- [Qemu-devel] [PATCH 23/41] slirp: tftp: Rework filename handling, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 24/41] slirp: Factor out one-time initialization, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 19/41] slirp: Drop unused icmp_var.h, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 20/41] slirp: tftp: Cleanup tftp_prefix check, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 21/41] slirp: tftp: Clean up tftp_send_error, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 29/41] slirp: Clean up timeout handling around slirp_select_fill/poll, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 26/41] slirp: Clean up updtime, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 25/41] slirp: Make IP packet ID consistent, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 27/41] slirp: Kill slirp_is_inited, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 28/41] slirp: Drop redundant checks from slirp_output, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 36/41] slirp: Use shell to erase smb directory, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 35/41] slirp: Save/restore bootp client states, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 32/41] slirp: Use internal state in interface, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 33/41] slirp: Allocate/free stack instance dynamically, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 31/41] slirp: Factor out internal state structure, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 34/41] slirp: Enable multiple instances, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 41/41] slirp: Basic VLAN client info_str, Jan Kiszka, 2009/06/24
- [Qemu-devel] [PATCH 39/41] net: Provide VLAN client lookup helper, Jan Kiszka, 2009/06/24