Redefine TCP state to include "flags that have been sent" rather than

"flags that are currently being sent".  This allows at least one special
case (checking that we haven't already sent a FIN in tcp_rx_fin()) to be
collapsed.

src/include/gpxe/tcp.h

@@ -42,17 +42,16 @@ struct tcp_header {
 /**
 * @defgroup tcpstates TCP states
 *
-* The TCP state is defined by a combination of the flags that are
-* currently being sent in outgoing packets, the flags that have been
-* sent and acknowledged by the peer, and the flags that have been
-* received from the peer.
+* The TCP state is defined by a combination of the flags that have
+* been sent to the peer, the flags that have been acknowledged by the
+* peer, and the flags that have been received from the peer.
 *
 * @{
 */
 
-/** TCP flags that are currently being sent in outgoing packets */
-#define TCP_STATE_SENDING(flags) ( (flags) << 0 )
-#define TCP_FLAGS_SENDING(state) ( ( (state) >> 0 ) & 0xff )
+/** TCP flags that have been sent in outgoing packets */
+#define TCP_STATE_SENT(flags) ( (flags) << 0 )
+#define TCP_FLAGS_SENT(state) ( ( (state) >> 0 ) & 0xff )
 
 /** TCP flags that have been acknowledged by the peer
  *
@@ -69,6 +68,10 @@ struct tcp_header {
 #define TCP_STATE_RCVD(flags) ( (flags) << 12 )
 #define TCP_FLAGS_RCVD(state) ( ( (state) >> 12 ) & 0x03 )
 
+/** TCP flags that are currently being sent in outgoing packets */
+#define TCP_FLAGS_SENDING(state) \
+	( TCP_FLAGS_SENT ( state ) & ~TCP_FLAGS_ACKED ( state ) )
+
 /** CLOSED
  *
  * The connection has not yet been used for anything.
@@ -86,21 +89,21 @@ struct tcp_header {
  *
  * SYN has been sent, nothing has yet been received or acknowledged.
  */
-#define TCP_SYN_SENT	( TCP_STATE_SENDING ( TCP_SYN ) )
+#define TCP_SYN_SENT	( TCP_STATE_SENT ( TCP_SYN ) )
 
 /** SYN_RCVD
  *
  * SYN has been sent but not acknowledged, SYN has been received.
  */
-#define TCP_SYN_RCVD	( TCP_STATE_SENDING ( TCP_SYN | TCP_ACK ) |	\
+#define TCP_SYN_RCVD	( TCP_STATE_SENT ( TCP_SYN | TCP_ACK ) |	    \
 			  TCP_STATE_RCVD ( TCP_SYN ) )
 
 /** ESTABLISHED
  *
  * SYN has been sent and acknowledged, SYN has been received.
  */
-#define TCP_ESTABLISHED	( TCP_STATE_SENDING ( TCP_ACK ) |		\
-			  TCP_STATE_ACKED ( TCP_SYN ) |			\
+#define TCP_ESTABLISHED	( TCP_STATE_SENT ( TCP_SYN | TCP_ACK ) |	    \
+			  TCP_STATE_ACKED ( TCP_SYN ) |			    \
 			  TCP_STATE_RCVD ( TCP_SYN ) )
 
 /** FIN_WAIT_1
@@ -117,8 +120,8 @@ struct tcp_header {
  * to FIN_WAIT_1, we have to remember to set TCP_STATE_ACKED(TCP_SYN)
  * and increment our sequence number.
  */
-#define TCP_FIN_WAIT_1	( TCP_STATE_SENDING ( TCP_ACK | TCP_FIN ) |	\
-			  TCP_STATE_ACKED ( TCP_SYN ) |			\
+#define TCP_FIN_WAIT_1	( TCP_STATE_SENT ( TCP_SYN | TCP_ACK | TCP_FIN ) |  \
+			  TCP_STATE_ACKED ( TCP_SYN ) |			    \
 			  TCP_STATE_RCVD ( TCP_SYN ) )
 
 /** FIN_WAIT_2
@@ -126,8 +129,8 @@ struct tcp_header {
  * SYN has been sent and acknowledged, SYN has been received, FIN has
  * been sent and acknowledged, FIN ha not been received.
  */
-#define TCP_FIN_WAIT_2	( TCP_STATE_SENDING ( TCP_ACK ) |		\
-			  TCP_STATE_ACKED ( TCP_SYN | TCP_FIN ) |	\
+#define TCP_FIN_WAIT_2	( TCP_STATE_SENT ( TCP_SYN | TCP_ACK | TCP_FIN ) |  \
+			  TCP_STATE_ACKED ( TCP_SYN | TCP_FIN ) |	    \
 			  TCP_STATE_RCVD ( TCP_SYN ) )
 
 /** CLOSING / LAST_ACK
@@ -139,9 +142,9 @@ struct tcp_header {
  * identical with the definition of state that we use.  I don't
  * *believe* that they need to be distinguished.
  */
-#define TCP_CLOSING_OR_LAST_ACK					\
-			( TCP_STATE_SENDING ( TCP_ACK | TCP_FIN ) |	\
-			  TCP_STATE_ACKED ( TCP_SYN ) |			\
+#define TCP_CLOSING_OR_LAST_ACK						    \
+			( TCP_STATE_SENT ( TCP_SYN | TCP_ACK | TCP_FIN ) |  \
+			  TCP_STATE_ACKED ( TCP_SYN ) |			    \
 			  TCP_STATE_RCVD ( TCP_SYN | TCP_FIN ) )
 
 /** TIME_WAIT
@@ -149,8 +152,8 @@ struct tcp_header {
  * SYN has been sent and acknowledged, SYN has been received, FIN has
  * been sent and acknowledged, FIN has been received.
  */
-#define TCP_TIME_WAIT	( TCP_STATE_SENDING ( TCP_ACK ) |		\
-			  TCP_STATE_ACKED ( TCP_SYN | TCP_FIN ) |	\
+#define TCP_TIME_WAIT	( TCP_STATE_SENT ( TCP_SYN | TCP_ACK | TCP_FIN ) |  \
+			  TCP_STATE_ACKED ( TCP_SYN | TCP_FIN ) |	    \
 			  TCP_STATE_RCVD ( TCP_SYN | TCP_FIN ) )
 
 /** CLOSE_WAIT
@@ -158,8 +161,8 @@ struct tcp_header {
  * SYN has been sent and acknowledged, SYN has been received, FIN has
  * been received.
  */
-#define TCP_CLOSE_WAIT	( TCP_STATE_SENDING ( TCP_ACK ) |		\
-			  TCP_STATE_ACKED ( TCP_SYN ) |			\
+#define TCP_CLOSE_WAIT	( TCP_STATE_SENT ( TCP_SYN | TCP_ACK ) |	    \
+			  TCP_STATE_ACKED ( TCP_SYN ) |			    \
 			  TCP_STATE_RCVD ( TCP_SYN | TCP_FIN ) )
 
 /** Can send data in current state
@@ -167,9 +170,9 @@ struct tcp_header {
  * We can send data if and only if we have had our SYN acked and we
  * have not yet sent our FIN.
  */
-#define TCP_CAN_SEND_DATA(state)					\
-	( ( (state) & ( TCP_STATE_ACKED ( TCP_SYN | TCP_FIN ) |		\
-		      TCP_STATE_SENDING ( TCP_FIN ) ) )			\
+#define TCP_CAN_SEND_DATA(state)					    \
+	( ( (state) & ( TCP_STATE_ACKED ( TCP_SYN ) |			    \
+			TCP_STATE_SENT ( TCP_FIN ) ) )			    \
 	  == TCP_STATE_ACKED ( TCP_SYN ) )
 
 /** Have closed gracefully
@@ -177,9 +180,9 @@ struct tcp_header {
  * We have closed gracefully if we have both received a FIN and had
  * our own FIN acked.
  */
-#define TCP_CLOSED_GRACEFULLY(state)					\
-	( ( (state) & ( TCP_STATE_ACKED ( TCP_FIN ) |			\
-			TCP_STATE_RCVD ( TCP_FIN ) ) )			\
+#define TCP_CLOSED_GRACEFULLY(state)					    \
+	( ( (state) & ( TCP_STATE_ACKED ( TCP_FIN ) |			    \
+			TCP_STATE_RCVD ( TCP_FIN ) ) )			    \
 	  == ( TCP_STATE_ACKED ( TCP_FIN ) | TCP_STATE_RCVD ( TCP_FIN ) ) )
 
 /** @} */

src/net/tcp.c

@@ -229,6 +229,7 @@ static int tcp_senddata_conn ( struct tcp_connection *conn, int force_send ) {
 	struct tcp_application *app = conn->app;
 	struct pk_buff *pkb;
 	struct tcp_header *tcphdr;
+	unsigned int flags;
 	size_t len;
 	size_t seq_len;
 
@@ -264,9 +265,9 @@ static int tcp_senddata_conn ( struct tcp_connection *conn, int force_send ) {
 	 */
 	len = pkb_len ( pkb );
 	seq_len = len;
-	assert ( ! ( ( conn->tcp_state & TCP_STATE_SENDING ( TCP_SYN ) ) &&
-		     ( conn->tcp_state & TCP_STATE_SENDING ( TCP_FIN ) ) ) );
-	if ( conn->tcp_state & TCP_STATE_SENDING ( TCP_SYN | TCP_FIN ) )
+	flags = TCP_FLAGS_SENDING ( conn->tcp_state );
+	assert ( ! ( ( flags & TCP_SYN ) && ( flags & TCP_FIN ) ) );
+	if ( flags & ( TCP_SYN | TCP_FIN ) )
 		seq_len++;
 	conn->snd_sent = seq_len;
 
@@ -291,7 +292,7 @@ static int tcp_senddata_conn ( struct tcp_connection *conn, int force_send ) {
 	tcphdr->seq = htonl ( conn->snd_seq );
 	tcphdr->ack = htonl ( conn->rcv_ack );
 	tcphdr->hlen = ( ( sizeof ( *tcphdr ) / 4 ) << 4 );
-	tcphdr->flags = TCP_FLAGS_SENDING ( conn->tcp_state );
+	tcphdr->flags = flags;
 	tcphdr->win = htons ( TCP_WINDOW_SIZE );
 	tcphdr->csum = tcpip_chksum ( pkb->data, pkb_len ( pkb ) );
 
@@ -458,7 +459,7 @@ static int tcp_rx_syn ( struct tcp_connection *conn, uint32_t seq ) {
 		return 0;
 
 	/* Mark SYN as received and start sending ACKs with each packet */
-	conn->tcp_state |= ( TCP_STATE_SENDING ( TCP_ACK ) |
+	conn->tcp_state |= ( TCP_STATE_SENT ( TCP_ACK ) |
 			     TCP_STATE_RCVD ( TCP_SYN ) );
 
 	/* Acknowledge SYN */
@@ -517,10 +518,8 @@ static int tcp_rx_ack ( struct tcp_connection *conn, uint32_t ack,
 		app->tcp_op->acked ( app, len );
 
 	/* Mark SYN/FIN as acknowledged if applicable. */
-	if ( acked_flags ) {
-		conn->tcp_state &= ~TCP_STATE_SENDING ( TCP_SYN | TCP_FIN );
+	if ( acked_flags )
 		conn->tcp_state |= TCP_STATE_ACKED ( acked_flags );
-	}
 
 	/* Notify application of established connection, if applicable */
 	if ( ( acked_flags & TCP_SYN ) && app && app->tcp_op->connected )
@@ -573,14 +572,11 @@ static int tcp_rx_fin ( struct tcp_connection *conn, uint32_t seq ) {
 	if ( ( conn->rcv_ack - seq ) > 0 )
 		return 0;
 
-	/* Mark FIN as received and acknowledge it */
-	conn->tcp_state |= TCP_STATE_RCVD ( TCP_FIN );
+	/* Mark FIN as received, acknowledge it, and send our own FIN */
+	conn->tcp_state |= ( TCP_STATE_RCVD ( TCP_FIN ) |
+			     TCP_STATE_SENT ( TCP_FIN ) );
 	conn->rcv_ack++;
 
-	/* If we haven't already sent our FIN, send a FIN */
-	if ( ! ( conn->tcp_state & TCP_STATE_ACKED ( TCP_FIN ) ) )
-		conn->tcp_state |= TCP_STATE_SENDING ( TCP_FIN );
-
 	/* Break association with application */
 	tcp_disassociate ( conn );
 
@@ -832,15 +828,15 @@ void tcp_close ( struct tcp_application *app ) {
 		return;
 	}
 
-	/* If we have sent a SYN but not had it acknowledged (i.e. we
-	 * are in SYN_RCVD), pretend that it has been acknowledged so
-	 * that we can send a FIN without breaking things.
+	/* If we have not had our SYN acknowledged (i.e. we are in
+	 * SYN_RCVD), pretend that it has been acknowledged so that we
+	 * can send a FIN without breaking things.
 	 */
-	if ( conn->tcp_state & TCP_STATE_SENDING ( TCP_SYN ) )
+	if ( ! ( conn->tcp_state & TCP_STATE_ACKED ( TCP_SYN ) ) )
 		tcp_rx_ack ( conn, ( conn->snd_seq + 1 ), 0 );
 
 	/* Send a FIN to initiate the close */
-	conn->tcp_state |= TCP_STATE_SENDING ( TCP_FIN );
+	conn->tcp_state |= TCP_STATE_SENT ( TCP_FIN );
 	tcp_dump_state ( conn );
 	tcp_senddata_conn ( conn, 0 );
 }