Browse Source

[tcp] Use a dedicated timer for the TIME_WAIT state

iPXE currently repurposes the retransmission timer to hold the TCP
connection in the TIME_WAIT state (i.e. waiting for up to 2*MSL in
case we are required to re-ACK our peer's FIN due to a lost ACK).
However, the fact that this timer is running will prevent such an ACK
from ever being sent, since the logic in tcp_xmit() assumes that a
running timer indicates that we ourselves are waiting for an ACK and
so blocks the transmission.  (We always wait for an ACK before sending
our next packet, to keep our transmit data path as simple as
possible.)

Fix by using an entirely separate timer for the TIME_WAIT state, so
that packets can still be sent.

Reported-by: Piotr Jaroszyński <p.jaroszynski@gmail.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
tags/v1.20.1
Michael Brown 14 years ago
parent
commit
c57e26381c
1 changed files with 32 additions and 9 deletions
  1. 32
    9
      src/net/tcp.c

+ 32
- 9
src/net/tcp.c View File

84
 	struct list_head queue;
84
 	struct list_head queue;
85
 	/** Retransmission timer */
85
 	/** Retransmission timer */
86
 	struct retry_timer timer;
86
 	struct retry_timer timer;
87
+	/** Shutdown (TIME_WAIT) timer */
88
+	struct retry_timer wait;
87
 };
89
 };
88
 
90
 
89
 /**
91
 /**
94
 /* Forward declarations */
96
 /* Forward declarations */
95
 static struct interface_descriptor tcp_xfer_desc;
97
 static struct interface_descriptor tcp_xfer_desc;
96
 static void tcp_expired ( struct retry_timer *timer, int over );
98
 static void tcp_expired ( struct retry_timer *timer, int over );
99
+static void tcp_wait_expired ( struct retry_timer *timer, int over );
97
 static int tcp_rx_ack ( struct tcp_connection *tcp, uint32_t ack,
100
 static int tcp_rx_ack ( struct tcp_connection *tcp, uint32_t ack,
98
 			uint32_t win );
101
 			uint32_t win );
99
 
102
 
229
 	ref_init ( &tcp->refcnt, NULL );
232
 	ref_init ( &tcp->refcnt, NULL );
230
 	intf_init ( &tcp->xfer, &tcp_xfer_desc, &tcp->refcnt );
233
 	intf_init ( &tcp->xfer, &tcp_xfer_desc, &tcp->refcnt );
231
 	timer_init ( &tcp->timer, tcp_expired );
234
 	timer_init ( &tcp->timer, tcp_expired );
235
+	timer_init ( &tcp->wait, tcp_wait_expired );
232
 	tcp->prev_tcp_state = TCP_CLOSED;
236
 	tcp->prev_tcp_state = TCP_CLOSED;
233
 	tcp->tcp_state = TCP_STATE_SENT ( TCP_SYN );
237
 	tcp->tcp_state = TCP_STATE_SENT ( TCP_SYN );
234
 	tcp_dump_state ( tcp );
238
 	tcp_dump_state ( tcp );
514
 /**
518
 /**
515
  * Retransmission timer expired
519
  * Retransmission timer expired
516
  *
520
  *
517
- * @v timer	Retry timer
518
- * @v over	Failure indicator
521
+ * @v timer		Retransmission timer
522
+ * @v over		Failure indicator
519
  */
523
  */
520
 static void tcp_expired ( struct retry_timer *timer, int over ) {
524
 static void tcp_expired ( struct retry_timer *timer, int over ) {
521
 	struct tcp_connection *tcp =
525
 	struct tcp_connection *tcp =
522
 		container_of ( timer, struct tcp_connection, timer );
526
 		container_of ( timer, struct tcp_connection, timer );
523
-	int graceful_close = TCP_CLOSED_GRACEFULLY ( tcp->tcp_state );
524
 
527
 
525
 	DBGC ( tcp, "TCP %p timer %s in %s for %08x..%08x %08x\n", tcp,
528
 	DBGC ( tcp, "TCP %p timer %s in %s for %08x..%08x %08x\n", tcp,
526
 	       ( over ? "expired" : "fired" ), tcp_state ( tcp->tcp_state ),
529
 	       ( over ? "expired" : "fired" ), tcp_state ( tcp->tcp_state ),
530
 		 ( tcp->tcp_state == TCP_SYN_RCVD ) ||
533
 		 ( tcp->tcp_state == TCP_SYN_RCVD ) ||
531
 		 ( tcp->tcp_state == TCP_ESTABLISHED ) ||
534
 		 ( tcp->tcp_state == TCP_ESTABLISHED ) ||
532
 		 ( tcp->tcp_state == TCP_FIN_WAIT_1 ) ||
535
 		 ( tcp->tcp_state == TCP_FIN_WAIT_1 ) ||
533
-		 ( tcp->tcp_state == TCP_TIME_WAIT ) ||
534
 		 ( tcp->tcp_state == TCP_CLOSE_WAIT ) ||
536
 		 ( tcp->tcp_state == TCP_CLOSE_WAIT ) ||
535
 		 ( tcp->tcp_state == TCP_CLOSING_OR_LAST_ACK ) );
537
 		 ( tcp->tcp_state == TCP_CLOSING_OR_LAST_ACK ) );
536
 
538
 
537
-	if ( over || graceful_close ) {
538
-		/* If we have finally timed out and given up, or if
539
-		 * this is the result of a graceful close, terminate
540
-		 * the connection
539
+	if ( over ) {
540
+		/* If we have finally timed out and given up,
541
+		 * terminate the connection
541
 		 */
542
 		 */
542
 		tcp->tcp_state = TCP_CLOSED;
543
 		tcp->tcp_state = TCP_CLOSED;
543
 		tcp_dump_state ( tcp );
544
 		tcp_dump_state ( tcp );
548
 	}
549
 	}
549
 }
550
 }
550
 
551
 
552
+/**
553
+ * Shutdown timer expired
554
+ *
555
+ * @v timer		Shutdown timer
556
+ * @v over		Failure indicator
557
+ */
558
+static void tcp_wait_expired ( struct retry_timer *timer, int over __unused ) {
559
+	struct tcp_connection *tcp =
560
+		container_of ( timer, struct tcp_connection, wait );
561
+
562
+	assert ( tcp->tcp_state == TCP_TIME_WAIT );
563
+
564
+	DBGC ( tcp, "TCP %p wait complete in %s for %08x..%08x %08x\n", tcp,
565
+	       tcp_state ( tcp->tcp_state ), tcp->snd_seq,
566
+	       ( tcp->snd_seq + tcp->snd_sent ), tcp->rcv_ack );
567
+
568
+	tcp->tcp_state = TCP_CLOSED;
569
+	tcp_dump_state ( tcp );
570
+	tcp_close ( tcp, 0 );
571
+}
572
+
551
 /**
573
 /**
552
  * Send RST response to incoming packet
574
  * Send RST response to incoming packet
553
  *
575
  *
1020
 	 * timer to expire and cause the connection to be freed.
1042
 	 * timer to expire and cause the connection to be freed.
1021
 	 */
1043
 	 */
1022
 	if ( TCP_CLOSED_GRACEFULLY ( tcp->tcp_state ) ) {
1044
 	if ( TCP_CLOSED_GRACEFULLY ( tcp->tcp_state ) ) {
1023
-		start_timer_fixed ( &tcp->timer, ( 2 * TCP_MSL ) );
1045
+		stop_timer ( &tcp->wait );
1046
+		start_timer_fixed ( &tcp->wait, ( 2 * TCP_MSL ) );
1024
 	}
1047
 	}
1025
 
1048
 
1026
 	return 0;
1049
 	return 0;

Loading…
Cancel
Save