| Message ID | 1311956703-24788-2-git-send-email-chouteau@adacore.com |
|---|---|
| State | New |
| Headers | show |
On 2011-07-29 18:25, Fabien Chouteau wrote: > 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. > > Signed-off-by: Fabien Chouteau <chouteau@adacore.com> > --- > slirp/if.c | 28 ++++++++++++++++++++--- > slirp/main.h | 2 +- > slirp/mbuf.c | 2 + > slirp/mbuf.h | 2 + > slirp/slirp.c | 67 ++++++++++++++++++++++++++++++-------------------------- > slirp/slirp.h | 15 ++++++++++++ > 6 files changed, 80 insertions(+), 36 deletions(-) > > diff --git a/slirp/if.c b/slirp/if.c > index 0f04e13..80a5c4e 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(vm_clock) + 1000000000ULL; Slirp uses rt_clock for expiry, let's stick with that clock. > + > 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(vm_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 ba76757..b006620 100644 > --- a/slirp/slirp.c > +++ b/slirp/slirp.c > @@ -237,6 +237,8 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork, > > QTAILQ_INSERT_TAIL(&slirp_instances, slirp, entry); > > + slirp->delayed = NULL; > + > return slirp; > } > > @@ -693,54 +695,57 @@ 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) > +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; Even if the old cold lacked them as well: { } > > if (!arp_table_search(&slirp->arp_table, 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) { > + This newline is superfluous... > + /* 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); ...while it would improve readability here (and below). > + /* 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 { > + ifm->arp_requested = false; Why? If that is required, you would have to make expiry checks depend on arp_requested as well - or reset the expiry date here. Jan
On 29/07/2011 19:35, Jan Kiszka wrote: > On 2011-07-29 18:25, Fabien Chouteau wrote: >> 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. >> >> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> >> --- >> slirp/if.c | 28 ++++++++++++++++++++--- >> slirp/main.h | 2 +- >> slirp/mbuf.c | 2 + >> slirp/mbuf.h | 2 + >> slirp/slirp.c | 67 ++++++++++++++++++++++++++++++-------------------------- >> slirp/slirp.h | 15 ++++++++++++ >> 6 files changed, 80 insertions(+), 36 deletions(-) >> >> diff --git a/slirp/if.c b/slirp/if.c >> index 0f04e13..80a5c4e 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(vm_clock) + 1000000000ULL; > > Slirp uses rt_clock for expiry, let's stick with that clock. OK, fixed. >> @@ -693,54 +695,57 @@ 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) >> +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; > > Even if the old cold lacked them as well: { } I forgot to use checkpatch.pl... >> + 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 { >> + ifm->arp_requested = false; > > Why? If that is required, you would have to make expiry checks depend on > arp_requested as well - or reset the expiry date here. > That's right, the packet is sent so there's no need to reset this value. Thanks for your review.
diff --git a/slirp/if.c b/slirp/if.c index 0f04e13..80a5c4e 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(vm_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(vm_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 ba76757..b006620 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -237,6 +237,8 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork, QTAILQ_INSERT_TAIL(&slirp_instances, slirp, entry); + slirp->delayed = NULL; + return slirp; } @@ -693,54 +695,57 @@ 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) +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->arp_table, 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 { + ifm->arp_requested = false; 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; } } diff --git a/slirp/slirp.h b/slirp/slirp.h index 103b39d..07d7d29 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -216,6 +216,15 @@ bool arp_table_search(ArpTable *arptbl, int in_ip_addr, uint8_t out_ethaddr[ETH_ALEN]); +struct DelayedIpPacket; +typedef struct DelayedIpPacket DelayedIpPacket; + +struct DelayedIpPacket { + uint8_t *data; + int len; + DelayedIpPacket *next; +}; + struct Slirp { QTAILQ_ENTRY(Slirp) entry; @@ -271,6 +280,12 @@ struct Slirp { ArpTable arp_table; + /* List of delayed IP packets. + * These packets are delayed because the target hw address is unknown. Slirp + * will try to send them each time the arp table is updated. + */ + DelayedIpPacket *delayed; + void *opaque; };
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. Signed-off-by: Fabien Chouteau <chouteau@adacore.com> --- slirp/if.c | 28 ++++++++++++++++++++--- slirp/main.h | 2 +- slirp/mbuf.c | 2 + slirp/mbuf.h | 2 + slirp/slirp.c | 67 ++++++++++++++++++++++++++++++-------------------------- slirp/slirp.h | 15 ++++++++++++ 6 files changed, 80 insertions(+), 36 deletions(-)