Re: [Qemu-devel] [PATCH] Fix scrambling of >32KB packets in slirp

["Ed Swierk" <eswierk@arastra.com>]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On 5/1/06, Fabrice Bellard <fabrice@bellard.org> wrote:
> Why not changing the definition itself to uint16_t and verifying each
> occurence of ip_off and ip_len ?

Indeed, why not. This is the solution adopted by Apple's OpenDarwin
(another BSD derivative). The attached patch changes the signed
definitions to unsigned.

I tried to verify by inspecting each occurrence of ip_off and ip_len;
in cases where the values are implicitly converted, the unsigned short
is converted to a signed int, which should be OK assuming 32-bit ints.
Does qemu support any platforms with 16-bit ints?

I also ran some tests (on i386 Linux) to ensure that slirp still works
as expected, and also handles packets > 32KB. Of course, more testing
on other platforms would be welcome.

--Ed

[-- Attachment #2: qemu-slirp-unsigned.patch --]
[-- Type: text/x-patch, Size: 1646 bytes --]

diff -BurN qemu.orig/slirp/ip.h qemu/slirp/ip.h
--- qemu.orig/slirp/ip.h	2004-04-22 00:10:47.000000000 +0000
+++ qemu/slirp/ip.h	2006-05-02 02:07:34.000000000 +0000
@@ -75,10 +75,6 @@
 
 /*
  * Structure of an internet header, naked of options.
- *
- * 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.
  */
 struct ip {
 #ifdef WORDS_BIGENDIAN
@@ -89,9 +85,9 @@
 		ip_v:4;			/* version */
 #endif
 	u_int8_t ip_tos;			/* type of service */
-	int16_t	ip_len;			/* total length */
+	u_int16_t	ip_len;			/* total length */
 	u_int16_t	ip_id;			/* identification */
-	int16_t	ip_off;			/* fragment offset field */
+	u_int16_t	ip_off;			/* fragment offset field */
 #define	IP_DF 0x4000			/* don't fragment flag */
 #define	IP_MF 0x2000			/* more fragments flag */
 #define	IP_OFFMASK 0x1fff		/* mask for fragmenting bits */
@@ -212,7 +208,7 @@
 	caddr32_t	ih_next, ih_prev;	/* for protocol sequence q's */
 	u_int8_t	ih_x1;			/* (unused) */
 	u_int8_t	ih_pr;			/* protocol */
-	int16_t	ih_len;			/* protocol length */
+	u_int16_t	ih_len;			/* protocol length */
 	struct	in_addr ih_src;		/* source internet address */
 	struct	in_addr ih_dst;		/* destination internet address */
 };
@@ -253,9 +249,9 @@
         u_int8_t ipf_mff;		/* XXX overlays ip_tos: use low bit
 					 * to avoid destroying tos (PPPDTRuu);
 					 * copied from (ip_off&IP_MF) */
-	int16_t	ip_len;
+	u_int16_t	ip_len;
 	u_int16_t	ip_id;
-	int16_t	ip_off;
+	u_int16_t	ip_off;
 	u_int8_t	ip_ttl;
 	u_int8_t	ip_p;
 	u_int16_t	ip_sum;