From 94fc68895d8aed4ec4eeb7c7f1e62fb6312afc6d Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Wed, 3 Jan 2007 21:13:11 +0000 Subject: [PATCH] Improve consistency between TCP and UDP RX datapaths --- src/net/tcp.c | 29 ++++++++------ src/net/udp.c | 104 +++++++++++++++++++++++++++----------------------- 2 files changed, 74 insertions(+), 59 deletions(-) diff --git a/src/net/tcp.c b/src/net/tcp.c index d9d427c4..8b56604a 100644 --- a/src/net/tcp.c +++ b/src/net/tcp.c @@ -600,7 +600,7 @@ static int tcp_rx ( struct pk_buff *pkb, struct sockaddr_tcpip *st_src __unused, struct sockaddr_tcpip *st_dest __unused, uint16_t pshdr_csum ) { - struct tcp_header *tcphdr; + struct tcp_header *tcphdr = pkb->data; struct tcp_connection *conn; unsigned int hlen; uint16_t csum; @@ -611,34 +611,34 @@ static int tcp_rx ( struct pk_buff *pkb, unsigned int flags; void *data; size_t len; - int rc = 0; + int rc; /* Sanity check packet */ if ( pkb_len ( pkb ) < sizeof ( *tcphdr ) ) { DBG ( "TCP packet too short at %d bytes (min %d bytes)\n", pkb_len ( pkb ), sizeof ( *tcphdr ) ); rc = -EINVAL; - goto err; + goto done; } - tcphdr = pkb->data; hlen = ( ( tcphdr->hlen & TCP_MASK_HLEN ) / 16 ) * 4; if ( hlen < sizeof ( *tcphdr ) ) { DBG ( "TCP header too short at %d bytes (min %d bytes)\n", hlen, sizeof ( *tcphdr ) ); rc = -EINVAL; - goto err; + goto done; } if ( hlen > pkb_len ( pkb ) ) { DBG ( "TCP header too long at %d bytes (max %d bytes)\n", hlen, pkb_len ( pkb ) ); rc = -EINVAL; - goto err; + goto done; } csum = tcpip_continue_chksum ( pshdr_csum, pkb->data, pkb_len ( pkb )); if ( csum != 0 ) { DBG ( "TCP checksum incorrect (is %04x including checksum " "field, should be 0000)\n", csum ); - goto err; + rc = -EINVAL; + goto done; } /* Parse parameters from header and strip header */ @@ -663,13 +663,17 @@ static int tcp_rx ( struct pk_buff *pkb, * sending RST */ #warning "Handle non-matched connections" - if ( ! conn ) - goto err; + if ( ! conn ) { + rc = -ENOTCONN; + goto done; + } /* Handle RST, if present */ #warning "Handle RST" - if ( flags & TCP_RST ) - goto err; + if ( flags & TCP_RST ) { + rc = -ECONNRESET; + goto done; + } /* Handle ACK, if present */ if ( flags & TCP_ACK ) @@ -707,7 +711,8 @@ static int tcp_rx ( struct pk_buff *pkb, start_timer ( &conn->timer ); } - err: + rc = 0; + done: /* Free received packet */ free_pkb ( pkb ); return rc; diff --git a/src/net/udp.c b/src/net/udp.c index e55cfa2e..d927e08b 100644 --- a/src/net/udp.c +++ b/src/net/udp.c @@ -69,7 +69,8 @@ int udp_open ( struct udp_connection *conn, uint16_t local_port ) { /* Add to UDP connection list */ list_add ( &conn->list, &udp_conns ); - DBG ( "UDP opened %p on port %d\n", conn, ntohs ( local_port ) ); + DBGC ( conn, "UDP %p opened on port %d\n", conn, + ntohs ( local_port ) ); return 0; } @@ -81,7 +82,7 @@ int udp_open ( struct udp_connection *conn, uint16_t local_port ) { */ void udp_close ( struct udp_connection *conn ) { list_del ( &conn->list ); - DBG ( "UDP closed %p\n", conn ); + DBGC ( conn, "UDP %p closed\n", conn ); } /** @@ -97,8 +98,8 @@ int udp_senddata ( struct udp_connection *conn ) { conn->tx_pkb = alloc_pkb ( UDP_MAX_TXPKB ); if ( conn->tx_pkb == NULL ) { - DBG ( "UDP %p cannot allocate packet buffer of length %d\n", - conn, UDP_MAX_TXPKB ); + DBGC ( conn, "UDP %p cannot allocate buffer of length %d\n", + conn, UDP_MAX_TXPKB ); return -ENOMEM; } pkb_reserve ( conn->tx_pkb, UDP_MAX_HLEN ); @@ -156,11 +157,9 @@ int udp_sendto ( struct udp_connection *conn, struct sockaddr_tcpip *peer, udphdr->chksum = tcpip_chksum ( udphdr, sizeof ( *udphdr ) + len ); /* Dump debugging information */ - DBG ( "UDP %p transmitting %p+%#zx len %#x src %d dest %d " - "chksum %#04x\n", conn, pkb->data, - pkb_len ( pkb ), ntohs ( udphdr->len ), - ntohs ( udphdr->source_port ), ntohs ( udphdr->dest_port ), - ntohs ( udphdr->chksum ) ); + DBGC ( conn, "UDP %p TX %d->%d len %zd\n", conn, + ntohs ( udphdr->source_port ), ntohs ( udphdr->dest_port ), + ntohs ( udphdr->len ) ); /* Send it to the next layer for processing */ return tcpip_tx ( pkb, &udp_protocol, peer, &udphdr->chksum ); @@ -184,6 +183,24 @@ int udp_send ( struct udp_connection *conn, const void *data, size_t len ) { return udp_sendto ( conn, &conn->peer, data, len ); } +/** + * Identify UDP connection by local port number + * + * @v local_port Local port (in network-endian order) + * @ret conn TCP connection, or NULL + */ +static struct udp_connection * udp_demux ( uint16_t local_port ) { + struct udp_connection *conn; + + list_for_each_entry ( conn, &udp_conns, list ) { + if ( ( conn->local_port == local_port ) || + ( conn->local_port == 0 ) ) { + return conn; + } + } + return NULL; +} + /** * Process a received packet * @@ -197,36 +214,32 @@ static int udp_rx ( struct pk_buff *pkb, struct sockaddr_tcpip *st_src, struct sockaddr_tcpip *st_dest, uint16_t pshdr_csum ) { struct udp_header *udphdr = pkb->data; struct udp_connection *conn; - unsigned int ulen; + size_t ulen; uint16_t csum; int rc; - /* Sanity check */ + /* Sanity check packet */ if ( pkb_len ( pkb ) < sizeof ( *udphdr ) ) { - DBG ( "UDP received underlength packet %p+%#zx\n", - pkb->data, pkb_len ( pkb ) ); + DBG ( "UDP packet too short at %d bytes (min %d bytes)\n", + pkb_len ( pkb ), sizeof ( *udphdr ) ); + rc = -EINVAL; goto done; } - - /* Dump debugging information */ - DBG ( "UDP received %p+%#zx len %#x src %d dest %d chksum %#04x\n", - pkb->data, pkb_len ( pkb ), ntohs ( udphdr->len ), - ntohs ( udphdr->source_port ), ntohs ( udphdr->dest_port ), - ntohs ( udphdr->chksum ) ); - - /* Check length and trim any excess */ ulen = ntohs ( udphdr->len ); - if ( ulen > pkb_len ( pkb ) ) { - DBG ( "UDP received truncated packet %p+%#zx\n", - pkb->data, pkb_len ( pkb ) ); + if ( ulen < sizeof ( *udphdr ) ) { + DBG ( "UDP length too short at %d bytes " + "(header is %d bytes)\n", ulen, sizeof ( *udphdr ) ); rc = -EINVAL; goto done; } - pkb_unput ( pkb, ( pkb_len ( pkb ) - ulen ) ); - - /* Verify the checksum */ - csum = tcpip_continue_chksum ( pshdr_csum, pkb->data, pkb_len ( pkb )); + if ( ulen > pkb_len ( pkb ) ) { + DBG ( "UDP length too long at %d bytes (packet is %d bytes)\n", + ulen, pkb_len ( pkb ) ); + rc = -EINVAL; + goto done; + } + csum = tcpip_continue_chksum ( pshdr_csum, pkb->data, ulen ); if ( csum != 0 ) { DBG ( "UDP checksum incorrect (is %04x including checksum " "field, should be 0000)\n", csum ); @@ -234,32 +247,29 @@ static int udp_rx ( struct pk_buff *pkb, struct sockaddr_tcpip *st_src, goto done; } - /* Complete the socket addresses */ + /* Parse parameters from header and strip header */ st_src->st_port = udphdr->source_port; st_dest->st_port = udphdr->dest_port; + conn = udp_demux ( udphdr->dest_port ); + pkb_unput ( pkb, ( pkb_len ( pkb ) - ulen ) ); + pkb_pull ( pkb, sizeof ( *udphdr ) ); - /* Demux the connection */ - list_for_each_entry ( conn, &udp_conns, list ) { - if ( conn->local_port && - ( conn->local_port != udphdr->dest_port ) ) { - /* Bound to local port and local port doesn't match */ - continue; - } - - /* Strip off the UDP header */ - pkb_pull ( pkb, sizeof ( *udphdr ) ); + /* Dump debugging information */ + DBGC ( conn, "UDP %p RX %d<-%d len %zd\n", conn, + ntohs ( udphdr->dest_port ), ntohs ( udphdr->source_port ), + ulen ); - DBG ( "UDP delivering to %p\n", conn ); - - /* Call the application's callback */ - rc = conn->udp_op->newdata ( conn, pkb->data, pkb_len( pkb ), - st_src, st_dest ); + /* Ignore if no matching connection found */ + if ( ! conn ) { + DBG ( "No UDP connection listening on port %d\n", + ntohs ( udphdr->dest_port ) ); + rc = -ENOTCONN; goto done; } - DBG ( "No UDP connection listening on port %d\n", - ntohs ( udphdr->dest_port ) ); - rc = 0; + /* Pass data to application */ + rc = conn->udp_op->newdata ( conn, pkb->data, pkb_len ( pkb ), + st_src, st_dest ); done: free_pkb ( pkb );