Fixed whitespaces; updated UDP code

src/include/gpxe/udp.h

@@ -18,7 +18,6 @@
  * UDP constants
  */
 
-#define UDP_HLEN    	8
 #define UDP_MAX_HLEN	72
 #define UDP_MAX_TXPKB	ETH_MAX_MTU
 #define UDP_MIN_TXPKB	ETH_ZLEN

src/net/udp.c

@@ -17,7 +17,40 @@
  * UDP protocol
  */
 
-void copy_sockaddr ( struct sockaddr *source, struct sockaddr *dest );
+static inline void copy_sockaddr ( struct sockaddr *source, struct sockaddr *dest ) {
+	memcpy ( dest, source, sizeof ( *dest ) );
+}
+
+static inline uint16_t dest_port ( struct sockaddr *sock, uint16_t *dest ) {
+	switch ( sock->sa_family ) {
+	case AF_INET:
+		dest = &sock->sin.sin_port;
+		break;
+	case AF_INET6:
+		dest = &sock->sin6.sin6_port;
+		break;
+	default:
+		DBG ( "Network family %d not supported\n", sock->sa_family );
+		return -EAFNOSUPPORT;
+	}
+	return 0;
+}
+
+/**
+ * Dump the UDP header
+ *
+ * @v udphdr	UDP header
+ */
+void udp_dump ( struct udp_header *udphdr ) {
+
+	/* Print UDP header for debugging */
+	DBG ( "UDP header at %#x + %d\n", udphdr, sizeof ( *udphdr ) );
+	DBG ( "\tSource Port = %d\n", ntohs ( udphdr->source_port ) );
+	DBG ( "\tDestination Port = %d\n", ntohs ( udphdr->dest_port ) );
+	DBG ( "\tLength = %d\n", ntohs ( udphdr->len ) );
+	DBG ( "\tChecksum = %d\n", ntohs ( udphdr->chksum ) );
+	DBG ( "\tChecksum located at %#x\n", &udphdr->chksum );
+}
 
 /**
  * Open a UDP connection
@@ -29,8 +62,9 @@ void copy_sockaddr ( struct sockaddr *source, struct sockaddr *dest );
  */
 void udp_connect ( struct udp_connection *conn, struct sockaddr *peer ) {
 	copy_sockaddr ( peer, &conn->sin );
-        /**
+
-         * Not sure if this should add the connection to udp_conns; If it does, uncomment the following code
+	/* Not sure if this should add the connection to udp_conns; If it does,
+	 * uncomment the following code
 	 */
 //	list_add ( &conn->list, &udp_conns );
 }
@@ -74,17 +108,19 @@ int udp_buf_alloc ( struct udp_connection *conn, size_t len ) {
  * @v data      Data to send
  * @v len       Length of data
  *
- * This function fills up the UDP headers and sends the data. Discover the network protocol to
+ * This function fills up the UDP headers and sends the data. Discover the
- * use through the sa_family field in the destination socket address.
+ * network protocol through the sa_family field in the destination socket
+ * address.
  */
 int udp_send ( struct udp_connection *conn, const void *data, size_t len ) {
        	struct udp_header *udphdr;		/* UDP header */
 	struct sockaddr *sock = &conn->sin;	/* Destination sockaddr */
+	uint16_t *dest;
 	int rc;
 
-        /* Copy data into packet buffer if necessary */
+	/* Copy data into packet buffer, if necessary */
 	if ( data != conn->tx_pkb->data ) {
-                /* Allocate a buffer */
+		/* Allocate space for data and lower layer headers */
 		if ( ( rc = udp_buf_alloc ( conn, len + UDP_MAX_HLEN ) ) != 0 ) {
 			DBG ( "Error allocating buffer" );
 			return rc;
@@ -95,24 +131,24 @@ int udp_send ( struct udp_connection *conn, const void *data, size_t len ) {
 		memcpy ( pkb_put ( conn->tx_pkb, len ), data, len );
 	}
 
-        /* Add the UDP header */
+	/*
-        udphdr = pkb_push ( conn->tx_pkb, UDP_HLEN );
+	 * Add the UDP header
-        if ( sock->sa_family == AF_INET )
+	 *
-                udphdr->dest_port = sock->sin.sin_port;
+	 * Covert all 16- and 32- bit integers into network btye order before
-        else
+	 * sending it over the network
-                udphdr->dest_port = sock->sin6.sin6_port;
+	 */
-        udphdr->source_port = conn->local_port;
+	udphdr = pkb_push ( conn->tx_pkb, sizeof ( *udphdr ) );
+	if ( (rc = dest_port ( sock, dest ) ) != 0 ) {
+		return rc;
+	}
+	udphdr->dest_port = htons ( *dest );
+	udphdr->source_port = htons ( conn->local_port );
 	udphdr->len = htons ( pkb_len ( conn->tx_pkb ) );
-        udphdr->chksum = calc_chksum ( udphdr, UDP_HLEN );
+	udphdr->chksum = htons ( calc_chksum ( udphdr, sizeof ( *udphdr ) ) );
 
-        /* Print UDP header for debugging */
+	udp_dump ( udphdr );
-        DBG ( "UDP header at %#x + %d\n", udphdr, UDP_HDR_LEN  );
-        DBG ( "\tSource Port = %d\n", udphdr->source_port );
-        DBG ( "\tDestination Port = %d\n", udphdr->dest_port );
-        DBG ( "\tLength = %d\n", udphdr->len );
-        DBG ( "\tChecksum = %d\n", udphdr->chksum );
-        DBG ( "\tChecksum located at %#x\n", &udphdr->chksum );
 
+	/* Send it to the next layer for processing */
 	return trans_tx ( conn->tx_pkb, IP_UDP, sock );
 }
 
@@ -154,15 +190,21 @@ void udp_close ( struct udp_connection *conn ) {
 int udp_open ( struct udp_connection *conn, uint16_t local_port ) {
 	struct udp_connection *connr;
 	uint16_t min_port = 0xffff;
+
+	/* Iterate through udp_conns to see if local_port is available */
 	list_for_each_entry ( connr, &udp_conns, list ) {
 		if ( connr->local_port == local_port ) {
-                        return -EISCONN; /* CHECK THE ERROR NUMBER */
+			return -EISCONN;
 		}
 		if ( min_port > connr->local_port ) {
 			min_port = connr->local_port;
 		}
 	}
-        conn->local_port = local_port == 0 ? min_port > 1024 ? 1024 : min_port + 1 : local_port; // FAULTY!!!
+	/* This code is buggy. I will update it soon :) */
+	conn->local_port = local_port == 0 ? min_port > 1024 ? 1024 :
+						min_port + 1 : local_port;
+
+	/* Add the connection to the list of listening connections */
 	list_add ( &conn->list, &udp_conns );
 	return 0;
 }
@@ -178,20 +220,37 @@ void udp_rx ( struct pk_buff *pkb, struct in_addr *src_net_addr __unused,
 			struct in_addr *dest_net_addr __unused ) {
 	struct udp_header *udphdr = pkb->data;
 	struct udp_connection *conn;
+	uint16_t ulen;
+	uint16_t chksum;
 
-        /* todo: Compute and verify checksum */
+	udp_dump ( udphdr );
 
-        /* Print UDP header for debugging */
+	/* Validate the packet and the UDP length */
-        DBG ( "UDP header at %#x + %d\n", udphdr, UDP_HDR_LEN  );
+	if ( pkb_len ( pkb ) < sizeof ( *udphdr ) ) {
-        DBG ( "\tSource Port = %d\n", udphdr->source_port );
+		DBG ( "UDP packet too short (%d bytes)\n",
-        DBG ( "\tDestination Port = %d\n", udphdr->dest_port );
+		      pkb_len ( pkb ) );
-        DBG ( "\tLength = %d\n", udphdr->len );
+		return;
-        DBG ( "\tChecksum = %d\n", udphdr->chksum );
+	}
-        DBG ( "\tChecksum located at %#x\n", &udphdr->chksum );
+
+	ulen = ntohs ( udphdr->len );
+	if ( ulen != pkb_len ( pkb ) ) {
+		DBG ( "Inconsistent UDP packet length (%d bytes)\n",
+		      pkb_len ( pkb ) );
+		return;
+	}
+
+	/* Verify the checksum */
+	chksum = calc_chksum ( pkb->data, pkb_len ( pkb ) );
+	if ( chksum != 0xffff ) {
+		DBG ( "Bad checksum %d\n", chksum );
+		return;
+	}
+
+	/* Todo: Check if it is a broadcast or multicast address */
 
 	/* Demux the connection */
 	list_for_each_entry ( conn, &udp_conns, list ) {
-                if ( conn->local_port == udphdr->dest_port ) {
+		if ( conn->local_port == ntohs ( udphdr->dest_port ) ) {
 			goto conn;
 		}
 	}
@@ -199,14 +258,14 @@ void udp_rx ( struct pk_buff *pkb, struct in_addr *src_net_addr __unused,
 
 	conn:
 	/** Strip off the UDP header */
-        pkb_pull ( pkb, UDP_HLEN );
+	pkb_pull ( pkb, sizeof ( *udphdr ) );
 
 	/** Allocate max possible buffer space to the tx buffer */
 	conn->tx_pkb = alloc_pkb ( UDP_MAX_TXPKB );
 	pkb_reserve ( conn->tx_pkb, UDP_MAX_HLEN );
 
 	/** Call the application's callback */
-        conn->udp_op->newdata ( conn, pkb->data, ntohs ( udphdr->len ) - UDP_HLEN  );
+	conn->udp_op->newdata ( conn, pkb->data, ulen - sizeof ( *udphdr ) );
 }
 
 struct trans_protocol udp_protocol  = {
@@ -216,10 +275,3 @@ struct trans_protocol udp_protocol  = {
 };
 
 TRANS_PROTOCOL ( udp_protocol );
-
-/**
- * Internal functions
- */
-void copy_sockaddr ( struct sockaddr *source, struct sockaddr *dest ) {
-        memcpy ( dest, source, sizeof ( struct sockaddr ) );
-}