* [Qemu-devel] Patch for sending large (>4k) packets through qemu/slirp [not found] <6fe044190604060145k53bdde3br925d884d7db08efa@mail.gmail.com> @ 2006-04-06 8:53 ` Kenneth Duda 2006-04-07 18:05 ` Kenneth Duda 0 siblings, 1 reply; 2+ messages in thread From: Kenneth Duda @ 2006-04-06 8:53 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 780 bytes --] In qemu-0.8.0.20060327, there are three problems with sending large packets from guest to host: (1) the code in slirp's ip_reass() reads a next pointer out an mbuf after freeing it via m_cat(). (2) the code in slirp's m_inc() calls realloc() on a large mbuf, but fails to adjust m_data to point to the new allocation (see http://lists.gnu.org/archive/html/qemu-devel/2005-05/msg00228.html). (3) there are many places within ip_input(), ip_reass(), udp_input(), etc., that treat ip_len and ip_off as though they were declared unsigned, when in fact they have been declared signed. Patches fixing these problems are attached. I hope they can be applied. Please let me know what I can do to make the patches more likely to be accepted. Thanks, -Ken [-- Attachment #2: qemu-slirp-reassembly-bug.patch --] [-- Type: text/plain, Size: 464 bytes --] diff -BurN qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c qemu-snapshot-2006-03-27_23/slirp/ip_input.c --- qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c 2004-04-22 00:10:47.000000000 +0000 +++ qemu-snapshot-2006-03-27_23/slirp/ip_input.c 2006-04-06 06:02:52.000000000 +0000 @@ -344,8 +344,8 @@ while (q != (struct ipasfrag *)fp) { struct mbuf *t; t = dtom(q); - m_cat(m, t); q = (struct ipasfrag *) q->ipf_next; + m_cat(m, t); } /* [-- Attachment #3: qemu-slirp-mbuf-bug.patch --] [-- Type: text/plain, Size: 885 bytes --] diff -BurN qemu-snapshot-2006-03-27_23.orig/slirp/mbuf.c qemu-snapshot-2006-03-27_23/slirp/mbuf.c --- qemu-snapshot-2006-03-27_23.orig/slirp/mbuf.c 2004-04-22 00:10:47.000000000 +0000 +++ qemu-snapshot-2006-03-27_23/slirp/mbuf.c 2006-04-05 13:03:03.000000000 +0000 @@ -146,18 +146,19 @@ struct mbuf *m; int size; { + int datasize; + /* some compiles throw up on gotos. This one we can fake. */ if(m->m_size>size) return; if (m->m_flags & M_EXT) { - /* datasize = m->m_data - m->m_ext; */ + datasize = m->m_data - m->m_ext; m->m_ext = (char *)realloc(m->m_ext,size); /* if (m->m_ext == NULL) * return (struct mbuf *)NULL; */ - /* m->m_data = m->m_ext + datasize; */ + m->m_data = m->m_ext + datasize; } else { - int datasize; char *dat; datasize = m->m_data - m->m_dat; dat = (char *)malloc(size); [-- Attachment #4: qemu-slirp-32k-packets.patch --] [-- Type: text/plain, Size: 4431 bytes --] diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/ip.h qemu-snapshot-2006-03-27_23/slirp/ip.h --- qemu-snapshot-2006-03-27_23.orig/slirp/ip.h 2004-04-21 17:10:47.000000000 -0700 +++ qemu-snapshot-2006-03-27_23/slirp/ip.h 2006-04-06 00:28:49.000000000 -0700 @@ -79,6 +79,11 @@ * We declare ip_len and ip_off to be short, rather than u_short * pragmatically since otherwise unsigned comparisons can result * against negative integers quite easily, and fail in subtle ways. + * + * The only problem with the above theory is that these quantities + * are in fact unsigned, and sorting fragments by a signed version + * of ip_off doesn't work very well, nor does length checks on + * ip packets with a signed version of their length! */ struct ip { #ifdef WORDS_BIGENDIAN @@ -101,6 +106,9 @@ struct in_addr ip_src,ip_dst; /* source and dest address */ }; +#define IP_OFF(ip) (*(u_int16_t *)&((ip)->ip_off)) +#define IP_LEN(ip) (*(u_int16_t *)&((ip)->ip_len)) + #define IP_MAXPACKET 65535 /* maximum packet size */ /* diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c qemu-snapshot-2006-03-27_23/slirp/ip_input.c --- qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c 2004-04-21 17:10:47.000000000 -0700 +++ qemu-snapshot-2006-03-27_23/slirp/ip_input.c 2006-04-06 00:32:19.000000000 -0700 @@ -111,7 +111,7 @@ * Convert fields to host representation. */ NTOHS(ip->ip_len); - if (ip->ip_len < hlen) { + if (IP_LEN(ip) < hlen) { ipstat.ips_badlen++; goto bad; } @@ -124,13 +124,13 @@ * Trim mbufs if longer than we expect. * Drop packet if shorter than we expect. */ - if (m->m_len < ip->ip_len) { + if (m->m_len < IP_LEN(ip)) { ipstat.ips_tooshort++; goto bad; } /* Should drop packet if mbuf too long? hmmm... */ - if (m->m_len > ip->ip_len) - m_adj(m, ip->ip_len - m->m_len); + if (m->m_len > IP_LEN(ip)) + m_adj(m, IP_LEN(ip) - m->m_len); /* check ip_ttl for a correct ICMP reply */ if(ip->ip_ttl==0 || ip->ip_ttl==1) { @@ -191,7 +191,7 @@ * or if this is not the first fragment, * attempt reassembly; if it succeeds, proceed. */ - if (((struct ipasfrag *)ip)->ipf_mff & 1 || ip->ip_off) { + if (((struct ipasfrag *)ip)->ipf_mff & 1 || IP_OFF(ip)) { ipstat.ips_fragments++; ip = ip_reass((struct ipasfrag *)ip, fp); if (ip == 0) @@ -281,7 +281,7 @@ */ for (q = (struct ipasfrag *)fp->ipq_next; q != (struct ipasfrag *)fp; q = (struct ipasfrag *)q->ipf_next) - if (q->ip_off > ip->ip_off) + if (IP_OFF(q) > IP_OFF(ip)) break; /* @@ -290,10 +290,10 @@ * segment. If it provides all of our data, drop us. */ if (q->ipf_prev != (ipasfragp_32)fp) { - i = ((struct ipasfrag *)(q->ipf_prev))->ip_off + - ((struct ipasfrag *)(q->ipf_prev))->ip_len - ip->ip_off; + i = IP_OFF((struct ipasfrag *)(q->ipf_prev)) + + IP_LEN((struct ipasfrag *)(q->ipf_prev)) - IP_OFF(ip); if (i > 0) { - if (i >= ip->ip_len) + if (i >= IP_LEN(ip)) goto dropfrag; m_adj(dtom(ip), i); ip->ip_off += i; @@ -305,9 +305,9 @@ * While we overlap succeeding segments trim them or, * if they are completely covered, dequeue them. */ - while (q != (struct ipasfrag *)fp && ip->ip_off + ip->ip_len > q->ip_off) { - i = (ip->ip_off + ip->ip_len) - q->ip_off; - if (i < q->ip_len) { + while (q != (struct ipasfrag *)fp && IP_OFF(ip) + IP_LEN(ip) > IP_OFF(q)) { + i = (IP_OFF(ip) + IP_LEN(ip)) - IP_OFF(q); + if (i < IP_LEN(q)) { q->ip_len -= i; q->ip_off += i; m_adj(dtom(q), i); @@ -327,9 +327,9 @@ next = 0; for (q = (struct ipasfrag *) fp->ipq_next; q != (struct ipasfrag *)fp; q = (struct ipasfrag *) q->ipf_next) { - if (q->ip_off != next) + if (IP_OFF(q) != next) return (0); - next += q->ip_len; + next += IP_LEN(q); } if (((struct ipasfrag *)(q->ipf_prev))->ipf_mff & 1) return (0); diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/udp.c qemu-snapshot-2006-03-27_23/slirp/udp.c --- qemu-snapshot-2006-03-27_23.orig/slirp/udp.c 2006-04-06 00:24:30.000000000 -0700 +++ qemu-snapshot-2006-03-27_23/slirp/udp.c 2006-04-06 00:32:55.000000000 -0700 @@ -111,12 +111,12 @@ */ len = ntohs((u_int16_t)uh->uh_ulen); - if (ip->ip_len != len) { - if (len > ip->ip_len) { + if (IP_LEN(ip) != len) { + if (len > IP_LEN(ip)) { udpstat.udps_badlen++; goto bad; } - m_adj(m, len - ip->ip_len); + m_adj(m, len - IP_LEN(ip)); ip->ip_len = len; } ^ permalink raw reply [flat|nested] 2+ messages in thread
* [Qemu-devel] Patch for sending large (>4k) packets through qemu/slirp 2006-04-06 8:53 ` [Qemu-devel] Patch for sending large (>4k) packets through qemu/slirp Kenneth Duda @ 2006-04-07 18:05 ` Kenneth Duda 0 siblings, 0 replies; 2+ messages in thread From: Kenneth Duda @ 2006-04-07 18:05 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 846 bytes --] Hi everyone, I have patches for a few bugs in qemu, and am new to the list --- if anyone could clue me in on the best way to get patches applied to the qemu mainline, that would be great. This patch fixes three problems (actually all in slirp) with sending large packets from guest to host in qemu-0.8.0.20060327: (1) the code in slirp's ip_reass() reads a next pointer out an mbuf after freeing it via m_cat(). (2) the code in slirp's m_inc() calls realloc() on a large mbuf, but fails to adjust m_data to point to the new allocation (see http://lists.gnu.org/archive/html/qemu-devel/2005-05/msg00228.html). (3) there are many places within ip_input(), ip_reass(), udp_input(), etc., that treat ip_len and ip_off as though they were declared unsigned, when in fact they have been declared signed. Thanks, -Ken [-- Attachment #2: qemu-slirp-reassembly-bug.patch --] [-- Type: text/plain, Size: 466 bytes --] diff -BurN qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c qemu-snapshot-2006-03-27_23/slirp/ip_input.c --- qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c 2004-04-22 00:10:47.000000000 +0000 +++ qemu-snapshot-2006-03-27_23/slirp/ip_input.c 2006-04-06 06:02:52.000000000 +0000 @@ -344,8 +344,8 @@ while (q != (struct ipasfrag *)fp) { struct mbuf *t; t = dtom(q); - m_cat(m, t); q = (struct ipasfrag *) q->ipf_next; + m_cat(m, t); } /* [-- Attachment #3: qemu-slirp-mbuf-bug.patch --] [-- Type: text/plain, Size: 887 bytes --] diff -BurN qemu-snapshot-2006-03-27_23.orig/slirp/mbuf.c qemu-snapshot-2006-03-27_23/slirp/mbuf.c --- qemu-snapshot-2006-03-27_23.orig/slirp/mbuf.c 2004-04-22 00:10:47.000000000 +0000 +++ qemu-snapshot-2006-03-27_23/slirp/mbuf.c 2006-04-05 13:03:03.000000000 +0000 @@ -146,18 +146,19 @@ struct mbuf *m; int size; { + int datasize; + /* some compiles throw up on gotos. This one we can fake. */ if(m->m_size>size) return; if (m->m_flags & M_EXT) { - /* datasize = m->m_data - m->m_ext; */ + datasize = m->m_data - m->m_ext; m->m_ext = (char *)realloc(m->m_ext,size); /* if (m->m_ext == NULL) * return (struct mbuf *)NULL; */ - /* m->m_data = m->m_ext + datasize; */ + m->m_data = m->m_ext + datasize; } else { - int datasize; char *dat; datasize = m->m_data - m->m_dat; dat = (char *)malloc(size); [-- Attachment #4: qemu-slirp-32k-packets.patch --] [-- Type: text/plain, Size: 4433 bytes --] diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/ip.h qemu-snapshot-2006-03-27_23/slirp/ip.h --- qemu-snapshot-2006-03-27_23.orig/slirp/ip.h 2004-04-21 17:10:47.000000000 -0700 +++ qemu-snapshot-2006-03-27_23/slirp/ip.h 2006-04-06 00:28:49.000000000 -0700 @@ -79,6 +79,11 @@ * We declare ip_len and ip_off to be short, rather than u_short * pragmatically since otherwise unsigned comparisons can result * against negative integers quite easily, and fail in subtle ways. + * + * The only problem with the above theory is that these quantities + * are in fact unsigned, and sorting fragments by a signed version + * of ip_off doesn't work very well, nor does length checks on + * ip packets with a signed version of their length! */ struct ip { #ifdef WORDS_BIGENDIAN @@ -101,6 +106,9 @@ struct in_addr ip_src,ip_dst; /* source and dest address */ }; +#define IP_OFF(ip) (*(u_int16_t *)&((ip)->ip_off)) +#define IP_LEN(ip) (*(u_int16_t *)&((ip)->ip_len)) + #define IP_MAXPACKET 65535 /* maximum packet size */ /* diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c qemu-snapshot-2006-03-27_23/slirp/ip_input.c --- qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c 2004-04-21 17:10:47.000000000 -0700 +++ qemu-snapshot-2006-03-27_23/slirp/ip_input.c 2006-04-06 00:32:19.000000000 -0700 @@ -111,7 +111,7 @@ * Convert fields to host representation. */ NTOHS(ip->ip_len); - if (ip->ip_len < hlen) { + if (IP_LEN(ip) < hlen) { ipstat.ips_badlen++; goto bad; } @@ -124,13 +124,13 @@ * Trim mbufs if longer than we expect. * Drop packet if shorter than we expect. */ - if (m->m_len < ip->ip_len) { + if (m->m_len < IP_LEN(ip)) { ipstat.ips_tooshort++; goto bad; } /* Should drop packet if mbuf too long? hmmm... */ - if (m->m_len > ip->ip_len) - m_adj(m, ip->ip_len - m->m_len); + if (m->m_len > IP_LEN(ip)) + m_adj(m, IP_LEN(ip) - m->m_len); /* check ip_ttl for a correct ICMP reply */ if(ip->ip_ttl==0 || ip->ip_ttl==1) { @@ -191,7 +191,7 @@ * or if this is not the first fragment, * attempt reassembly; if it succeeds, proceed. */ - if (((struct ipasfrag *)ip)->ipf_mff & 1 || ip->ip_off) { + if (((struct ipasfrag *)ip)->ipf_mff & 1 || IP_OFF(ip)) { ipstat.ips_fragments++; ip = ip_reass((struct ipasfrag *)ip, fp); if (ip == 0) @@ -281,7 +281,7 @@ */ for (q = (struct ipasfrag *)fp->ipq_next; q != (struct ipasfrag *)fp; q = (struct ipasfrag *)q->ipf_next) - if (q->ip_off > ip->ip_off) + if (IP_OFF(q) > IP_OFF(ip)) break; /* @@ -290,10 +290,10 @@ * segment. If it provides all of our data, drop us. */ if (q->ipf_prev != (ipasfragp_32)fp) { - i = ((struct ipasfrag *)(q->ipf_prev))->ip_off + - ((struct ipasfrag *)(q->ipf_prev))->ip_len - ip->ip_off; + i = IP_OFF((struct ipasfrag *)(q->ipf_prev)) + + IP_LEN((struct ipasfrag *)(q->ipf_prev)) - IP_OFF(ip); if (i > 0) { - if (i >= ip->ip_len) + if (i >= IP_LEN(ip)) goto dropfrag; m_adj(dtom(ip), i); ip->ip_off += i; @@ -305,9 +305,9 @@ * While we overlap succeeding segments trim them or, * if they are completely covered, dequeue them. */ - while (q != (struct ipasfrag *)fp && ip->ip_off + ip->ip_len > q->ip_off) { - i = (ip->ip_off + ip->ip_len) - q->ip_off; - if (i < q->ip_len) { + while (q != (struct ipasfrag *)fp && IP_OFF(ip) + IP_LEN(ip) > IP_OFF(q)) { + i = (IP_OFF(ip) + IP_LEN(ip)) - IP_OFF(q); + if (i < IP_LEN(q)) { q->ip_len -= i; q->ip_off += i; m_adj(dtom(q), i); @@ -327,9 +327,9 @@ next = 0; for (q = (struct ipasfrag *) fp->ipq_next; q != (struct ipasfrag *)fp; q = (struct ipasfrag *) q->ipf_next) { - if (q->ip_off != next) + if (IP_OFF(q) != next) return (0); - next += q->ip_len; + next += IP_LEN(q); } if (((struct ipasfrag *)(q->ipf_prev))->ipf_mff & 1) return (0); diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/udp.c qemu-snapshot-2006-03-27_23/slirp/udp.c --- qemu-snapshot-2006-03-27_23.orig/slirp/udp.c 2006-04-06 00:24:30.000000000 -0700 +++ qemu-snapshot-2006-03-27_23/slirp/udp.c 2006-04-06 00:32:55.000000000 -0700 @@ -111,12 +111,12 @@ */ len = ntohs((u_int16_t)uh->uh_ulen); - if (ip->ip_len != len) { - if (len > ip->ip_len) { + if (IP_LEN(ip) != len) { + if (len > IP_LEN(ip)) { udpstat.udps_badlen++; goto bad; } - m_adj(m, len - ip->ip_len); + m_adj(m, len - IP_LEN(ip)); ip->ip_len = len; } ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-04-07 18:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <6fe044190604060145k53bdde3br925d884d7db08efa@mail.gmail.com>
2006-04-06 8:53 ` [Qemu-devel] Patch for sending large (>4k) packets through qemu/slirp Kenneth Duda
2006-04-07 18:05 ` Kenneth Duda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).