[BusyBox] udhcp patches/ checksumming

[Rainer Weikusat]

The checksumming routine, as present in packet.c, has a couple of
hypothetical and not-so-hypothetical problems, namely

	- It uses an int32_t as sum accumulator, which may lead to a
          signed overflow, which happens to be undefined behaviour
          according to ANSI

	- It uses right shifts of signed quantities, which have
          implementation defined behaviour in case the sign bit is not
          zero. Specifically, implementations may either sign-extend
          or zero-extend the result, and the gcc one happens to
          sign-extend, leading to wrong results (this isn't very
          likely to happen with a DHCP packet)

	- It uses & 0xffff for conversion of 32 bit to 16 bit
          numbers. Depending on the architecture, better methods for a
          compiler to pick may be available if it has the option to
          (this is definitely the case for ARM).          

This one fixes relieves all of the above and (just for fun) is about 30%
faster on the ARM hardware I am using this code for.

Index: busybox/networking/udhcp/packet.c
===================================================================
RCS file: /data/repo/busybox/networking/udhcp/packet.c,v
retrieving revision 1.3
diff -u -r1.3 packet.c
--- busybox/networking/udhcp/packet.c	29 Jan 2005 03:02:52 -0000	1.3
+++ busybox/networking/udhcp/packet.c	29 Jan 2005 21:48:43 -0000
@@ -41,7 +41,6 @@
 	add_simple_option(packet->options, DHCP_MESSAGE_TYPE, type);
 }
 
-
 /* read a packet from socket fd, return -1 on read error, -2 on packet error */
 int get_packet(struct dhcpMessage *packet, int fd)
 {
@@ -91,49 +90,56 @@
 	return bytes;
 }
 
-
-uint16_t checksum(void *addr, int count)
+uint16_t checksum(uint8_t *p, unsigned n)
 {
-	/* Compute Internet Checksum for "count" bytes
-	 *         beginning at location "addr".
-	 */
-	register int32_t sum = 0;
-	uint16_t *source = (uint16_t *) addr;
-
-	while (count > 1)  {
-		/*  This is the inner loop */
-		sum += *source++;
-		count -= 2;
+	uint32_t a_tmp, a_sum;
+	uint8_t *end;
+	unsigned remain;
+
+	remain = n & 3;
+	a_sum = 0;
+	if (n -= remain) {
+		end = p + n;
+	
+		do {
+			a_tmp = *(uint32_t *)p;
+			a_sum += (uint16_t)a_tmp;
+			a_sum += a_tmp >> 16;
+		} while ((p += 4) < end);
 	}
 
-	/*  Add left-over byte, if any */
-	if (count > 0) {
-		/* Make sure that the left-over byte is added correctly both
-		 * with little and big endian hosts */
-		uint16_t tmp = 0;
-		*(uint8_t *) (&tmp) = * (uint8_t *) source;
-		sum += tmp;
+	switch (remain) {
+	case 0:
+		break;
+		
+	default:
+		a_sum += *(uint16_t *)p;
+		if (!(remain & 1)) break;
+		p += 2;
+
+	case 1:
+		a_sum += *p;
 	}
-	/*  Fold 32-bit sum to 16 bits */
-	while (sum >> 16)
-		sum = (sum & 0xffff) + (sum >> 16);
 
-	return ~sum;
+	while ((a_tmp = a_sum >> 16)) 
+		a_sum = (uint16_t)a_sum + a_tmp;
+
+	return ~a_sum;
 }
 
 static int do_sendto(int fd, void const *msg, ssize_t msg_len,
 		     struct sockaddr *dest, socklen_t d_len)
 {
-    int rc;
+	int rc;
 
-    rc = sendto(fd, msg, msg_len, 0, dest, d_len);
-    if (rc == -1) syslog(LOG_ERR, "sendto failed: %m");
-    return rc;
+	rc = sendto(fd, msg, msg_len, 0, dest, d_len);
+	if (rc == -1) syslog(LOG_ERR, "sendto failed: %m");
+	return rc;
 }
 
 /* Construct a ip/udp header for a packet, and specify the source and dest hardware address */
 int raw_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port,
-		   uint32_t dest_ip, int dest_port, uint8_t *dest_arp, int ifindex)
+	       uint32_t dest_ip, int dest_port, uint8_t *dest_arp, int ifindex)
 {
 	int fd;
 	int result;
@@ -155,13 +161,13 @@
 	packet.udp.len = htons(sizeof(packet.udp) + sizeof(*payload)); /* cheat on the psuedo-header */
 	packet.ip.tot_len = packet.udp.len;
 	memcpy(&(packet.data), payload, sizeof(*payload));
-	packet.udp.check = checksum(&packet, sizeof(packet));
+	packet.udp.check = checksum((uint8_t *)&packet, sizeof(packet));
 
 	packet.ip.tot_len = htons(sizeof(packet));
 	packet.ip.ihl = sizeof(packet.ip) >> 2;
 	packet.ip.version = IPVERSION;
 	packet.ip.ttl = IPDEFTTL;
-	packet.ip.check = checksum(&packet.ip, sizeof(packet.ip));
+	packet.ip.check = checksum((uint8_t *)&packet.ip, sizeof(packet.ip));
 
 	dest.sll_family = AF_PACKET;
 	dest.sll_protocol = htons(ETH_P_IP);
@@ -176,7 +182,7 @@
 
 /* Let the kernel do all the work for packet generation */
 int kernel_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port,
-		   uint32_t dest_ip, int dest_port)
+		  uint32_t dest_ip, int dest_port)
 {
 	int n = 1;
 	int fd, result;
Index: busybox/networking/udhcp/packet.h
===================================================================
RCS file: /data/repo/busybox/networking/udhcp/packet.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 packet.h
--- busybox/networking/udhcp/packet.h	30 Nov 2004 16:02:32 -0000	1.1.1.1
+++ busybox/networking/udhcp/packet.h	29 Jan 2005 21:48:43 -0000
@@ -31,7 +31,7 @@
 
 void init_header(struct dhcpMessage *packet, char type);
 int get_packet(struct dhcpMessage *packet, int fd);
-uint16_t checksum(void *addr, int count);
+uint16_t checksum(uint8_t *addr, unsigned count);
 int raw_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port,
 		   uint32_t dest_ip, int dest_port, uint8_t *dest_arp, int ifindex);
 int kernel_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port,