On (Wed) 03 Aug 2011 [13:24:22], Jan Kiszka wrote: > From: Fabien Chouteau <chout...@adacore.com> > > 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 <chout...@adacore.com> > Signed-off-by: Jan Kiszka <jan.kis...@siemens.com> > --- > 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
