From d6536b2c971f7323e58dfbe1e6f3b7c7c0c4edf3 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 29 Feb 2012 09:27:33 +0100 Subject: slirp: Keep next_m always valid Make sure that next_m always points to a packet if batchq is non-empty. This will simplify walking the queues in if_start. CC: Fabien Chouteau CC: Zhi Yong Wu CC: Stefan Weil Signed-off-by: Jan Kiszka --- slirp/if.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/slirp/if.c b/slirp/if.c index 33f08e1..14fdef1 100644 --- a/slirp/if.c +++ b/slirp/if.c @@ -96,8 +96,13 @@ if_output(struct socket *so, struct mbuf *ifm) ifs_insque(ifm, ifq->ifs_prev); goto diddit; } - } else + } else { ifq = slirp->if_batchq.ifq_prev; + /* Set next_m if the queue was empty so far */ + if (slirp->next_m == &slirp->if_batchq) { + slirp->next_m = ifm; + } + } /* Create a new doubly linked list for this session */ ifm->ifq_so = so; @@ -170,13 +175,8 @@ void if_start(Slirp *slirp) if (slirp->if_fastq.ifq_next != &slirp->if_fastq) { ifm = slirp->if_fastq.ifq_next; } else { - /* Nothing on fastq, see if next_m is valid */ - if (slirp->next_m != &slirp->if_batchq) { - ifm = slirp->next_m; - } else { - ifm = slirp->if_batchq.ifq_next; - } - + /* Nothing on fastq, pick up from batchq via next_m */ + ifm = slirp->next_m; from_batchq = true; } @@ -202,6 +202,12 @@ void if_start(Slirp *slirp) if (ifm->ifs_next != ifm) { insque(ifm->ifs_next, ifqt); ifs_remque(ifm); + /* Set next_m if the session packet is now the only one on + * batchq */ + if (ifqt == &slirp->if_batchq && + slirp->next_m == &slirp->if_batchq) { + slirp->next_m = ifm->ifs_next; + } } /* Update so_queued */ -- cgit v1.1 From 953e7f54e679cd40fff28e29189ed9e24bfb0758 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Mon, 5 Mar 2012 19:50:39 +0100 Subject: slirp: Prevent recursion of if_start if_start can be called recursively via if_encap. Avoid this as our scheme of dequeuing packets is not compatible with this. CC: Fabien Chouteau CC: Zhi Yong Wu CC: Stefan Weil Signed-off-by: Jan Kiszka --- slirp/if.c | 11 ++++++++++- slirp/slirp.h | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/slirp/if.c b/slirp/if.c index 14fdef1..f7aebe9 100644 --- a/slirp/if.c +++ b/slirp/if.c @@ -163,10 +163,17 @@ void if_start(Slirp *slirp) DEBUG_CALL("if_start"); + if (slirp->if_start_busy) { + return; + } + slirp->if_start_busy = true; + while (slirp->if_queued) { /* check if we can really output */ - if (!slirp_can_output(slirp->opaque)) + if (!slirp_can_output(slirp->opaque)) { + slirp->if_start_busy = false; return; + } /* * See which queue to get next packet from @@ -221,4 +228,6 @@ void if_start(Slirp *slirp) } slirp->if_queued = requeued; + + slirp->if_start_busy = false; } diff --git a/slirp/slirp.h b/slirp/slirp.h index 28a5c037..416d44a 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -239,6 +239,7 @@ struct Slirp { struct mbuf if_fastq; /* fast queue (for interactive data) */ struct mbuf if_batchq; /* queue for non-interactive data */ struct mbuf *next_m; /* pointer to next mbuf to output */ + bool if_start_busy; /* avoid if_start recursion */ /* ip states */ struct ipq ipq; /* ip reass. queue */ -- cgit v1.1 From e3078bf40a33b59fa11d077b1d0bb8796470982e Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 6 Mar 2012 00:00:07 +0100 Subject: slirp: Fix queue walking in if_start Another attempt to get this right: We need to carefully walk both the fastq and the batchq in if_start while trying to send packets to possibly not yet resolved hosts on the virtual network. So far we just requeued a delayed packet where it was and then started walking the queues from the top again - that couldn't work. Now we pre- calculate the next packet in the queue so that the current one can safely be removed if it was sent successfully. We also need to take into account that the next packet can be from the same session if the current one was sent and there are no other sessions. CC: Fabien Chouteau CC: Zhi Yong Wu CC: Stefan Weil Tested-by: Stefan Weil Signed-off-by: Jan Kiszka --- slirp/if.c | 60 +++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/slirp/if.c b/slirp/if.c index f7aebe9..f6e848a 100644 --- a/slirp/if.c +++ b/slirp/if.c @@ -158,8 +158,8 @@ void if_start(Slirp *slirp) { uint64_t now = qemu_get_clock_ns(rt_clock); int requeued = 0; - bool from_batchq = false; - struct mbuf *ifm, *ifqt; + bool from_batchq, next_from_batchq; + struct mbuf *ifm, *ifm_next, *ifqt; DEBUG_CALL("if_start"); @@ -168,23 +168,36 @@ void if_start(Slirp *slirp) } slirp->if_start_busy = true; - while (slirp->if_queued) { + if (slirp->if_fastq.ifq_next != &slirp->if_fastq) { + ifm_next = slirp->if_fastq.ifq_next; + next_from_batchq = false; + } else if (slirp->next_m != &slirp->if_batchq) { + /* Nothing on fastq, pick up from batchq via next_m */ + ifm_next = slirp->next_m; + next_from_batchq = true; + } else { + ifm_next = NULL; + } + + while (ifm_next) { /* check if we can really output */ if (!slirp_can_output(slirp->opaque)) { slirp->if_start_busy = false; return; } - /* - * See which queue to get next packet from - * If there's something in the fastq, select it immediately - */ - if (slirp->if_fastq.ifq_next != &slirp->if_fastq) { - ifm = slirp->if_fastq.ifq_next; - } else { - /* Nothing on fastq, pick up from batchq via next_m */ - ifm = slirp->next_m; - from_batchq = true; + ifm = ifm_next; + from_batchq = next_from_batchq; + + ifm_next = ifm->ifq_next; + if (ifm_next == &slirp->if_fastq) { + /* No more packets in fastq, switch to batchq */ + ifm_next = slirp->next_m; + next_from_batchq = true; + } + if (ifm_next == &slirp->if_batchq) { + /* end of batchq */ + ifm_next = NULL; } slirp->if_queued--; @@ -196,7 +209,7 @@ void if_start(Slirp *slirp) continue; } - if (from_batchq) { + if (ifm == slirp->next_m) { /* Set which packet to send on next iteration */ slirp->next_m = ifm->ifq_next; } @@ -207,13 +220,19 @@ void if_start(Slirp *slirp) /* If there are more packets for this session, re-queue them */ if (ifm->ifs_next != ifm) { - insque(ifm->ifs_next, ifqt); + struct mbuf *next = ifm->ifs_next; + + insque(next, ifqt); ifs_remque(ifm); - /* Set next_m if the session packet is now the only one on - * batchq */ - if (ifqt == &slirp->if_batchq && - slirp->next_m == &slirp->if_batchq) { - slirp->next_m = ifm->ifs_next; + + if (!from_batchq) { + /* Next packet in fastq is from the same session */ + ifm_next = next; + next_from_batchq = false; + } else if (slirp->next_m == &slirp->if_batchq) { + /* Set next_m and ifm_next if the session packet is now the + * only one on batchq */ + slirp->next_m = ifm_next = next; } } @@ -224,7 +243,6 @@ void if_start(Slirp *slirp) } m_free(ifm); - } slirp->if_queued = requeued; -- cgit v1.1 From f37343197708d90f119007ce5ecc2503be9c04c1 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 6 Mar 2012 00:02:23 +0100 Subject: slirp: Remove unneeded if_queued There is now a trivial check on entry of if_start for pending packets, so we can drop the additional tracking via if_queued. Signed-off-by: Jan Kiszka --- slirp/if.c | 11 +---------- slirp/slirp.c | 7 +------ slirp/slirp.h | 1 - 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/slirp/if.c b/slirp/if.c index f6e848a..096cf6f 100644 --- a/slirp/if.c +++ b/slirp/if.c @@ -110,8 +110,6 @@ if_output(struct socket *so, struct mbuf *ifm) insque(ifm, ifq); diddit: - slirp->if_queued++; - if (so) { /* Update *_queued */ so->so_queued++; @@ -157,7 +155,6 @@ diddit: void if_start(Slirp *slirp) { uint64_t now = qemu_get_clock_ns(rt_clock); - int requeued = 0; bool from_batchq, next_from_batchq; struct mbuf *ifm, *ifm_next, *ifqt; @@ -182,8 +179,7 @@ void if_start(Slirp *slirp) while (ifm_next) { /* check if we can really output */ if (!slirp_can_output(slirp->opaque)) { - slirp->if_start_busy = false; - return; + break; } ifm = ifm_next; @@ -200,12 +196,9 @@ void if_start(Slirp *slirp) ifm_next = NULL; } - slirp->if_queued--; - /* Try to send packet unless it already expired */ if (ifm->expiration_date >= now && !if_encap(slirp, ifm)) { /* Packet is delayed due to pending ARP resolution */ - requeued++; continue; } @@ -245,7 +238,5 @@ void if_start(Slirp *slirp) m_free(ifm); } - slirp->if_queued = requeued; - slirp->if_start_busy = false; } diff --git a/slirp/slirp.c b/slirp/slirp.c index 19d69eb..bcffc34 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -581,12 +581,7 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, } } - /* - * See if we can start outputting - */ - if (slirp->if_queued) { - if_start(slirp); - } + if_start(slirp); } /* clear global file descriptor sets. diff --git a/slirp/slirp.h b/slirp/slirp.h index 416d44a..cbe8a3c 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -235,7 +235,6 @@ struct Slirp { int mbuf_alloced; /* if states */ - int if_queued; /* number of packets queued so far */ struct mbuf if_fastq; /* fast queue (for interactive data) */ struct mbuf if_batchq; /* queue for non-interactive data */ struct mbuf *next_m; /* pointer to next mbuf to output */ -- cgit v1.1 From a68adc220603baffc355ecea8865b3ea9707ab00 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 29 Feb 2012 19:14:23 +0100 Subject: slirp: Cleanup resources on instance removal Close & free sockets when shutting down a slirp instance, also release all buffers. CC: Michael S. Tsirkin Signed-off-by: Jan Kiszka --- slirp/ip_icmp.c | 7 +++++++ slirp/ip_icmp.h | 1 + slirp/ip_input.c | 7 +++++++ slirp/mbuf.c | 21 +++++++++++++++++++++ slirp/mbuf.h | 1 + slirp/slirp.c | 3 +++ slirp/slirp.h | 2 ++ slirp/tcp_subr.c | 7 +++++++ slirp/udp.c | 8 ++++++++ slirp/udp.h | 1 + 10 files changed, 58 insertions(+) diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c index 5dbf21d..d571fd0 100644 --- a/slirp/ip_icmp.c +++ b/slirp/ip_icmp.c @@ -66,6 +66,13 @@ void icmp_init(Slirp *slirp) slirp->icmp_last_so = &slirp->icmp; } +void icmp_cleanup(Slirp *slirp) +{ + while (slirp->icmp.so_next != &slirp->icmp) { + icmp_detach(slirp->icmp.so_next); + } +} + static int icmp_send(struct socket *so, struct mbuf *m, int hlen) { struct ip *ip = mtod(m, struct ip *); diff --git a/slirp/ip_icmp.h b/slirp/ip_icmp.h index b3da1f2..1a1af91 100644 --- a/slirp/ip_icmp.h +++ b/slirp/ip_icmp.h @@ -154,6 +154,7 @@ struct icmp { (type) == ICMP_MASKREQ || (type) == ICMP_MASKREPLY) void icmp_init(Slirp *slirp); +void icmp_cleanup(Slirp *slirp); void icmp_input(struct mbuf *, int); void icmp_error(struct mbuf *msrc, u_char type, u_char code, int minsize, const char *message); diff --git a/slirp/ip_input.c b/slirp/ip_input.c index c7b3eb4..ce24faf 100644 --- a/slirp/ip_input.c +++ b/slirp/ip_input.c @@ -61,6 +61,13 @@ ip_init(Slirp *slirp) icmp_init(slirp); } +void ip_cleanup(Slirp *slirp) +{ + udp_cleanup(slirp); + tcp_cleanup(slirp); + icmp_cleanup(slirp); +} + /* * Ip input routine. Checksum and byte swap header. If fragmented * try to reassemble. Process options. Pass to next level. diff --git a/slirp/mbuf.c b/slirp/mbuf.c index c699c75..4fefb04 100644 --- a/slirp/mbuf.c +++ b/slirp/mbuf.c @@ -32,6 +32,27 @@ m_init(Slirp *slirp) slirp->m_usedlist.m_next = slirp->m_usedlist.m_prev = &slirp->m_usedlist; } +void m_cleanup(Slirp *slirp) +{ + struct mbuf *m, *next; + + m = slirp->m_usedlist.m_next; + while (m != &slirp->m_usedlist) { + next = m->m_next; + if (m->m_flags & M_EXT) { + free(m->m_ext); + } + free(m); + m = next; + } + m = slirp->m_freelist.m_next; + while (m != &slirp->m_freelist) { + next = m->m_next; + free(m); + m = next; + } +} + /* * Get an mbuf from the free list, if there are none * malloc one diff --git a/slirp/mbuf.h b/slirp/mbuf.h index 8d7951f..3f3ab09 100644 --- a/slirp/mbuf.h +++ b/slirp/mbuf.h @@ -116,6 +116,7 @@ struct mbuf { * it rather than putting it on the free list */ void m_init(Slirp *); +void m_cleanup(Slirp *slirp); struct mbuf * m_get(Slirp *); void m_free(struct mbuf *); void m_cat(register struct mbuf *, register struct mbuf *); diff --git a/slirp/slirp.c b/slirp/slirp.c index bcffc34..1502830 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -246,6 +246,9 @@ void slirp_cleanup(Slirp *slirp) unregister_savevm(NULL, "slirp", slirp); + ip_cleanup(slirp); + m_cleanup(slirp); + g_free(slirp->tftp_prefix); g_free(slirp->bootp_filename); g_free(slirp); diff --git a/slirp/slirp.h b/slirp/slirp.h index cbe8a3c..5033ee3 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -315,6 +315,7 @@ void if_output(struct socket *, struct mbuf *); /* ip_input.c */ void ip_init(Slirp *); +void ip_cleanup(Slirp *); void ip_input(struct mbuf *); void ip_slowtimo(Slirp *); void ip_stripoptions(register struct mbuf *, struct mbuf *); @@ -332,6 +333,7 @@ void tcp_setpersist(register struct tcpcb *); /* tcp_subr.c */ void tcp_init(Slirp *); +void tcp_cleanup(Slirp *); void tcp_template(struct tcpcb *); void tcp_respond(struct tcpcb *, register struct tcpiphdr *, register struct mbuf *, tcp_seq, tcp_seq, int); struct tcpcb * tcp_newtcpcb(struct socket *); diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c index 143a238..6f6585a 100644 --- a/slirp/tcp_subr.c +++ b/slirp/tcp_subr.c @@ -55,6 +55,13 @@ tcp_init(Slirp *slirp) slirp->tcp_last_so = &slirp->tcb; } +void tcp_cleanup(Slirp *slirp) +{ + while (slirp->tcb.so_next != &slirp->tcb) { + tcp_close(sototcpcb(slirp->tcb.so_next)); + } +} + /* * Create template to be used to send tcp packets on a connection. * Call after host entry created, fills diff --git a/slirp/udp.c b/slirp/udp.c index 5b060f3..ced5096 100644 --- a/slirp/udp.c +++ b/slirp/udp.c @@ -49,6 +49,14 @@ udp_init(Slirp *slirp) slirp->udb.so_next = slirp->udb.so_prev = &slirp->udb; slirp->udp_last_so = &slirp->udb; } + +void udp_cleanup(Slirp *slirp) +{ + while (slirp->udb.so_next != &slirp->udb) { + udp_detach(slirp->udb.so_next); + } +} + /* m->m_data points at ip packet header * m->m_len length ip packet * ip->ip_len length data (IPDU) diff --git a/slirp/udp.h b/slirp/udp.h index 9b5c3cf..9bf31fe 100644 --- a/slirp/udp.h +++ b/slirp/udp.h @@ -74,6 +74,7 @@ struct udpiphdr { struct mbuf; void udp_init(Slirp *); +void udp_cleanup(Slirp *); void udp_input(register struct mbuf *, int); int udp_output(struct socket *, struct mbuf *, struct sockaddr_in *); int udp_attach(struct socket *); -- cgit v1.1 From 2d26512b45b5236fa521c4492608fe9fb5bedf46 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 10 Mar 2012 21:20:53 +0100 Subject: slirp: Fix compiler warning for w64 Casting a pointer to an integer value must use uintptr_t or intptr_t (not long) for portable code. MinGW-w64 requires this because sizeof(long) != sizeof(void *) for w64 hosts, so casting to long raises a compiler warning. I use uintptr_t instead of intptr_t because changing the sign does not matter here and casting pointers to unsigned values seems more reasonable (the unsigned value is a non negative offset. Cc: Jan Kiszka Signed-off-by: Stefan Weil Signed-off-by: Jan Kiszka --- slirp/cksum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slirp/cksum.c b/slirp/cksum.c index e43867d..6328660 100644 --- a/slirp/cksum.c +++ b/slirp/cksum.c @@ -75,7 +75,7 @@ int cksum(struct mbuf *m, int len) /* * Force to even boundary. */ - if ((1 & (long) w) && (mlen > 0)) { + if ((1 & (uintptr_t)w) && (mlen > 0)) { REDUCE; sum <<= 8; s_util.c[0] = *(uint8_t *)w; -- cgit v1.1