[Qemu-devel] Patch for sending large (>4k) packets through qemu/slirp

["Kenneth Duda" <kduda@arastra.com>]

[-- 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;
 	}