[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] Delayed IP packets
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] Delayed IP packets |
Date: |
Thu, 29 Sep 2011 21:36:46 +0530 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On (Wed) 03 Aug 2011 [13:24:22], Jan Kiszka wrote:
> From: Fabien Chouteau <address@hidden>
>
> In the current implementation, if Slirp tries to send an IP packet to a client
> with an unknown hardware address, the packet is simply dropped and an ARP
> request is sent (if_encap in slirp/slirp.c).
>
> With this patch, Slirp will send the ARP request, re-queue the packet and try
> to send it later. The packet is dropped after one second if the ARP reply is
> not received.
This patch causes a segfault when guests wake up from hibernate.
Recipe:
1. Start guest with -net user -net nic,model=virtio
2. (guest) ping 10.0.2.2
3. (guest) echo "disk" > /sys/power/state
4. Re-start guest with same command line
5. Ping has stopped receiving replies.
6. Kill that ping process and start a new one. qemu segfaults.
This needs the not-upstream-yet virtio S4 handling patches, found at
http://thread.gmane.org/gmane.linux.kernel/1197141
The backtrace is:
(gdb) bt
#0 0x00007ffff7e421f7 in slirp_insque (a=0x0, b=0x7ffff8f95d50) at
/home/amit/src/qemu/slirp/misc.c:27
#1 0x00007ffff7e40738 in if_start (slirp=0x7ffff8a9cdf0) at
/home/amit/src/qemu/slirp/if.c:194
#2 0x00007ffff7e44828 in slirp_select_poll (readfds=0x7fffffffd930,
writefds=0x7fffffffd9b0, xfds=0x7fffffffda30, select_error=0)
at /home/amit/src/qemu/slirp/slirp.c:588
#3 0x00007ffff7e110f1 in main_loop_wait (nonblocking=<optimized out>)
at /home/amit/src/qemu/vl.c:1549
#4 0x00007ffff7d7dc47 in main_loop () at
/home/amit/src/qemu/vl.c:1579
#5 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized
out>) at /home/amit/src/qemu/vl.c:3574
Reverting the patch keeps the ping going on after resume.
Leaving the patch in context:
>
> Signed-off-by: Fabien Chouteau <address@hidden>
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
> slirp/if.c | 28 +++++++++++++++++++---
> slirp/main.h | 2 +-
> slirp/mbuf.c | 2 +
> slirp/mbuf.h | 2 +
> slirp/slirp.c | 72 +++++++++++++++++++++++++++++++-------------------------
> 5 files changed, 69 insertions(+), 37 deletions(-)
>
> diff --git a/slirp/if.c b/slirp/if.c
> index 0f04e13..2d79e45 100644
> --- a/slirp/if.c
> +++ b/slirp/if.c
> @@ -6,6 +6,7 @@
> */
>
> #include <slirp.h>
> +#include "qemu-timer.h"
>
> #define ifs_init(ifm) ((ifm)->ifs_next = (ifm)->ifs_prev = (ifm))
>
> @@ -105,6 +106,9 @@ if_output(struct socket *so, struct mbuf *ifm)
> ifs_init(ifm);
> insque(ifm, ifq);
>
> + /* Expiration date = Now + 1 second */
> + ifm->expiration_date = qemu_get_clock_ns(rt_clock) + 1000000000ULL;
> +
> diddit:
> slirp->if_queued++;
>
> @@ -153,6 +157,9 @@ diddit:
> void
> if_start(Slirp *slirp)
> {
> + int requeued = 0;
> + uint64_t now;
> +
> struct mbuf *ifm, *ifqt;
>
> DEBUG_CALL("if_start");
> @@ -165,6 +172,8 @@ if_start(Slirp *slirp)
> if (!slirp_can_output(slirp->opaque))
> return;
>
> + now = qemu_get_clock_ns(rt_clock);
> +
> /*
> * See which queue to get next packet from
> * If there's something in the fastq, select it immediately
> @@ -199,11 +208,22 @@ if_start(Slirp *slirp)
> ifm->ifq_so->so_nqueued = 0;
> }
>
> - /* Encapsulate the packet for sending */
> - if_encap(slirp, (uint8_t *)ifm->m_data, ifm->m_len);
> -
> - m_free(ifm);
> + if (ifm->expiration_date < now) {
> + /* Expired */
> + m_free(ifm);
> + } else {
> + /* Encapsulate the packet for sending */
> + if (if_encap(slirp, ifm)) {
> + m_free(ifm);
> + } else {
> + /* re-queue */
> + insque(ifm, ifqt);
> + requeued++;
> + }
> + }
>
> if (slirp->if_queued)
> goto again;
> +
> + slirp->if_queued = requeued;
> }
> diff --git a/slirp/main.h b/slirp/main.h
> index 0dd8d81..028df4b 100644
> --- a/slirp/main.h
> +++ b/slirp/main.h
> @@ -42,5 +42,5 @@ extern int tcp_keepintvl;
> #define PROTO_PPP 0x2
> #endif
>
> -void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len);
> +int if_encap(Slirp *slirp, struct mbuf *ifm);
> ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int
> flags);
> diff --git a/slirp/mbuf.c b/slirp/mbuf.c
> index ce2eb84..c699c75 100644
> --- a/slirp/mbuf.c
> +++ b/slirp/mbuf.c
> @@ -70,6 +70,8 @@ m_get(Slirp *slirp)
> m->m_len = 0;
> m->m_nextpkt = NULL;
> m->m_prevpkt = NULL;
> + m->arp_requested = false;
> + m->expiration_date = (uint64_t)-1;
> end_error:
> DEBUG_ARG("m = %lx", (long )m);
> return m;
> diff --git a/slirp/mbuf.h b/slirp/mbuf.h
> index b74544b..55170e5 100644
> --- a/slirp/mbuf.h
> +++ b/slirp/mbuf.h
> @@ -86,6 +86,8 @@ struct mbuf {
> char m_dat_[1]; /* ANSI don't like 0 sized arrays */
> char *m_ext_;
> } M_dat;
> + bool arp_requested;
> + uint64_t expiration_date;
> };
>
> #define m_next m_hdr.mh_next
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 4a9a4d5..a86cc6e 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -692,55 +692,63 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int
> pkt_len)
> }
> }
>
> -/* output the IP packet to the ethernet device */
> -void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len)
> +/* Output the IP packet to the ethernet device. Returns 0 if the packet must
> be
> + * re-queued.
> + */
> +int if_encap(Slirp *slirp, struct mbuf *ifm)
> {
> uint8_t buf[1600];
> struct ethhdr *eh = (struct ethhdr *)buf;
> uint8_t ethaddr[ETH_ALEN];
> - const struct ip *iph = (const struct ip *)ip_data;
> + const struct ip *iph = (const struct ip *)ifm->m_data;
>
> - if (ip_data_len + ETH_HLEN > sizeof(buf))
> - return;
> + if (ifm->m_len + ETH_HLEN > sizeof(buf)) {
> + return 1;
> + }
>
> if (!arp_table_search(slirp, iph->ip_dst.s_addr, ethaddr)) {
> uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)];
> struct ethhdr *reh = (struct ethhdr *)arp_req;
> struct arphdr *rah = (struct arphdr *)(arp_req + ETH_HLEN);
>
> - /* If the client addr is not known, there is no point in
> - sending the packet to it. Normally the sender should have
> - done an ARP request to get its MAC address. Here we do it
> - in place of sending the packet and we hope that the sender
> - will retry sending its packet. */
> - memset(reh->h_dest, 0xff, ETH_ALEN);
> - memcpy(reh->h_source, special_ethaddr, ETH_ALEN - 4);
> - memcpy(&reh->h_source[2], &slirp->vhost_addr, 4);
> - reh->h_proto = htons(ETH_P_ARP);
> - rah->ar_hrd = htons(1);
> - rah->ar_pro = htons(ETH_P_IP);
> - rah->ar_hln = ETH_ALEN;
> - rah->ar_pln = 4;
> - rah->ar_op = htons(ARPOP_REQUEST);
> - /* source hw addr */
> - memcpy(rah->ar_sha, special_ethaddr, ETH_ALEN - 4);
> - memcpy(&rah->ar_sha[2], &slirp->vhost_addr, 4);
> - /* source IP */
> - rah->ar_sip = slirp->vhost_addr.s_addr;
> - /* target hw addr (none) */
> - memset(rah->ar_tha, 0, ETH_ALEN);
> - /* target IP */
> - rah->ar_tip = iph->ip_dst.s_addr;
> - slirp->client_ipaddr = iph->ip_dst;
> - slirp_output(slirp->opaque, arp_req, sizeof(arp_req));
> + if (!ifm->arp_requested) {
> + /* If the client addr is not known, send an ARP request */
> + memset(reh->h_dest, 0xff, ETH_ALEN);
> + memcpy(reh->h_source, special_ethaddr, ETH_ALEN - 4);
> + memcpy(&reh->h_source[2], &slirp->vhost_addr, 4);
> + reh->h_proto = htons(ETH_P_ARP);
> + rah->ar_hrd = htons(1);
> + rah->ar_pro = htons(ETH_P_IP);
> + rah->ar_hln = ETH_ALEN;
> + rah->ar_pln = 4;
> + rah->ar_op = htons(ARPOP_REQUEST);
> +
> + /* source hw addr */
> + memcpy(rah->ar_sha, special_ethaddr, ETH_ALEN - 4);
> + memcpy(&rah->ar_sha[2], &slirp->vhost_addr, 4);
> +
> + /* source IP */
> + rah->ar_sip = slirp->vhost_addr.s_addr;
> +
> + /* target hw addr (none) */
> + memset(rah->ar_tha, 0, ETH_ALEN);
> +
> + /* target IP */
> + rah->ar_tip = iph->ip_dst.s_addr;
> + slirp->client_ipaddr = iph->ip_dst;
> + slirp_output(slirp->opaque, arp_req, sizeof(arp_req));
> + ifm->arp_requested = true;
> + }
> + return 0;
> } else {
> memcpy(eh->h_dest, ethaddr, ETH_ALEN);
> memcpy(eh->h_source, special_ethaddr, ETH_ALEN - 4);
> /* XXX: not correct */
> memcpy(&eh->h_source[2], &slirp->vhost_addr, 4);
> eh->h_proto = htons(ETH_P_IP);
> - memcpy(buf + sizeof(struct ethhdr), ip_data, ip_data_len);
> - slirp_output(slirp->opaque, buf, ip_data_len + ETH_HLEN);
> + memcpy(buf + sizeof(struct ethhdr), ifm->m_data, ifm->m_len);
> + slirp_output(slirp->opaque, buf, ifm->m_len + ETH_HLEN);
> + return 1;
> }
> }
>
> --
> 1.7.3.4
>
>
Amit
- Re: [Qemu-devel] [PATCH 3/3] Delayed IP packets,
Amit Shah <=