[tcp] Update received sequence number before delivering received data

iPXE currently updates the TCP sequence number after delivering the
data to the application via xfer_deliver_iob().  If the application
responds to the received data by transmitting more data, this would
result in a stale ACK number appearing in the transmitted packet,
which potentially causes retransmissions and also gives the
undesirable appearance of violating causality (by sending a response
to a message that we claim not to have yet received).

Reported-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 10 insertions, 8 deletions

diff --git a/src/net/tcp.c b/src/net/tcp.c
index decc9215..b01345fa 100644
--- a/src/net/tcp.c
+++ b/src/net/tcp.c
@@ -702,13 +702,13 @@ static int tcp_rx_syn ( struct tcp_connection *tcp, uint32_t seq,
 	if ( ( tcp->rcv_ack - seq ) > 0 )
 		return 0;
 
+	/* Acknowledge SYN */
+	tcp_rx_seq ( tcp, 1 );
+
 	/* Mark SYN as received and start sending ACKs with each packet */
 	tcp->tcp_state |= ( TCP_STATE_SENT ( TCP_ACK ) |
 			    TCP_STATE_RCVD ( TCP_SYN ) );
 
-	/* Acknowledge SYN */
-	tcp_rx_seq ( tcp, 1 );
-
 	return 0;
 }
 
@@ -809,6 +809,9 @@ static int tcp_rx_data ( struct tcp_connection *tcp, uint32_t seq,
 	iob_pull ( iobuf, already_rcvd );
 	len -= already_rcvd;
 
+	/* Acknowledge new data */
+	tcp_rx_seq ( tcp, len );
+
 	/* Deliver data to application */
 	if ( ( rc = xfer_deliver_iob ( &tcp->xfer, iobuf ) ) != 0 ) {
 		DBGC ( tcp, "TCP %p could not deliver %08x..%08x: %s\n",
@@ -816,9 +819,6 @@ static int tcp_rx_data ( struct tcp_connection *tcp, uint32_t seq,
 		return rc;
 	}
 
-	/* Acknowledge new data */
-	tcp_rx_seq ( tcp, len );
-
 	return 0;
 }
 
@@ -835,10 +835,12 @@ static int tcp_rx_fin ( struct tcp_connection *tcp, uint32_t seq ) {
 	if ( ( tcp->rcv_ack - seq ) > 0 )
 		return 0;
 
-	/* Mark FIN as received and acknowledge it */
-	tcp->tcp_state |= TCP_STATE_RCVD ( TCP_FIN );
+	/* Acknowledge FIN */
 	tcp_rx_seq ( tcp, 1 );
 
+	/* Mark FIN as received */
+	tcp->tcp_state |= TCP_STATE_RCVD ( TCP_FIN );
+
 	/* Close connection */
 	tcp_close ( tcp, 0 );