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 <wu...@linux.vnet.ibm.com>
>>> CC: Fabien Chouteau <chout...@adacore.com>
>>> Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
>>> ---
>>> 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.
>
> Embarrassing.
>
> Does this help? Specifically expired packet handling is broken.
>
> 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 = qemu_get_clock_ns(rt_clock);
> int requeued = 0;
> - bool from_batchq = false;
> + bool from_batchq;
> struct mbuf *ifm, *ifqt;
>
> DEBUG_CALL("if_start");
> @@ -169,6 +169,7 @@ void if_start(Slirp *slirp)
> */
> if (slirp->if_fastq.ifq_next != &slirp->if_fastq) {
> ifm = slirp->if_fastq.ifq_next;
> + from_batchq = false;
> } else {
> /* Nothing on fastq, see if next_m is valid */
> if (slirp->next_m != &slirp->if_batchq) {
> @@ -176,14 +177,17 @@ void if_start(Slirp *slirp)
> } else {
> ifm = slirp->if_batchq.ifq_next;
> }
> -
> from_batchq = true;
> }
>
> slirp->if_queued--;
>
> /* Try to send packet unless it already expired */
> - if (ifm->expiration_date >= 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;
>
> JanNonsense. We need to walk the queues properly and carefully. Will come up with a better version. Jan
signature.asc
Description: OpenPGP digital signature
