[gPXE-devel] [PATCHv2] [tcp core] Wait for TCP to safely close

cooldavid at cooldavid.org cooldavid at cooldavid.org
Thu Jul 15 01:18:05 EDT 2010


From: Guo-Fu Tseng <cooldavid at cooldavid.org>

Difference between previous patch:
	Added activity_wait() to main().
	Centralize ACTIVITY_TIMEOUT.

After some discussion with Piotr and Michael, I think it might
be an acceptable solution for the issue that gPXE leaves remote
peer TCP in undesired state.

The reason I don't mark activity_start at somewhere like tcp_open
or ESTABLISHED is that sanboot do not close the TCP connection
before hand of the control to OS.

Signed-off-by: Guo-Fu Tseng <cooldavid at cooldavid.org>
---
 src/core/activity.c         |   64 +++++++++++++++++++++++++++++++++++++++++++
 src/core/image.c            |    8 +++++
 src/core/main.c             |    2 +
 src/include/gpxe/activity.h |   28 +++++++++++++++++++
 src/include/gpxe/tcp.h      |   13 +++++++++
 src/net/tcp.c               |   16 +++++++++++
 src/usr/autoboot.c          |    6 ++++
 7 files changed, 137 insertions(+), 0 deletions(-)
 create mode 100644 src/core/activity.c
 create mode 100644 src/include/gpxe/activity.h

diff --git a/src/core/activity.c b/src/core/activity.c
new file mode 100644
index 0000000..099d85a
--- /dev/null
+++ b/src/core/activity.c
@@ -0,0 +1,64 @@
+/*
+ * The original idea is from: Michael Brown <mbrown at fensystems.co.uk>.
+ * Implemented into gPXE by: Guo-Fu Tseng <cooldavid at cooldavid.org>.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+FILE_LICENCE ( GPL2_OR_LATER );
+
+#include <gpxe/timer.h>
+#include <gpxe/process.h>
+#include <gpxe/activity.h>
+
+/** @file
+ *
+ * Activities
+ *
+ * We implement a facility to keep the processes running while waiting
+ * for some critical events to complete.
+ */
+
+
+/** Currently waiting activities */
+static unsigned int num_activities = 0;
+
+/**
+ * Start an activity
+ */
+void activity_start ( void ) {
+	num_activities++;
+}
+
+/**
+ * Stop an activity
+ */
+void activity_stop ( void ) {
+	num_activities--;
+}
+
+/**
+ * Wait for activities to complete
+ *
+ * @v max_timeout	The max waiting time in tenths of a second.
+ * @ret boolean		If all the activities are cleared.
+ */
+int activity_wait ( unsigned long max_timeout ) {
+	max_timeout = currticks() + ( max_timeout * TICKS_PER_SEC ) / 10;
+	while ( num_activities && currticks() < max_timeout ) {
+		step();
+	}
+	return !num_activities;
+}
diff --git a/src/core/image.c b/src/core/image.c
index d7fed72..369a709 100644
--- a/src/core/image.c
+++ b/src/core/image.c
@@ -28,6 +28,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <gpxe/list.h>
 #include <gpxe/umalloc.h>
 #include <gpxe/uri.h>
+#include <gpxe/activity.h>
 #include <gpxe/image.h>
 
 /** @file
@@ -262,6 +263,13 @@ int image_exec ( struct image *image ) {
 	 */
 	image_get ( image );
 
+	/**
+	 * Wait for possible pending activities before possible leaves gPXE
+	 * at later image execution.
+	 * Currently only TCP close event implemented it.
+	 */
+	activity_wait( ACTIVITY_TIMEOUT );
+
 	/* Try executing the image */
 	if ( ( rc = image->type->exec ( image ) ) != 0 ) {
 		DBGC ( image, "IMAGE %p could not execute: %s\n",
diff --git a/src/core/main.c b/src/core/main.c
index 36e6b02..0d1852e 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -20,6 +20,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <gpxe/shell.h>
 #include <gpxe/shell_banner.h>
 #include <gpxe/image.h>
+#include <gpxe/activity.h>
 #include <usr/autoboot.h>
 #include <config/general.h>
 
@@ -85,6 +86,7 @@ __asmcall int main ( void ) {
 			shell();
 	}
 
+	activity_wait ( ACTIVITY_TIMEOUT );
 	shutdown ( SHUTDOWN_EXIT | shutdown_exit_flags );
 
 	return 0;
diff --git a/src/include/gpxe/activity.h b/src/include/gpxe/activity.h
new file mode 100644
index 0000000..a77a679
--- /dev/null
+++ b/src/include/gpxe/activity.h
@@ -0,0 +1,28 @@
+#ifndef	_GPXE_ACTIVITY_H
+#define _GPXE_ACTIVITY_H
+
+/** @file
+ *
+ * gPXE activity API
+ *
+ * The activity API provides facility to keep the processes running
+ * while waiting for some critical events to complete.
+ *
+ * It is suggested that we don't call activity_wait() within any
+ * process's scope.
+ */
+
+FILE_LICENCE ( GPL2_OR_LATER );
+
+extern void activity_start ( void );
+extern void activity_stop ( void );
+
+/* Default timeout for 1 second */
+#define ACTIVITY_TIMEOUT 10
+
+/**
+ * max_timeout is in tenths of a second
+ */
+extern int activity_wait ( unsigned long max_timeout );
+
+#endif
diff --git a/src/include/gpxe/tcp.h b/src/include/gpxe/tcp.h
index aabd4d6..94091b8 100644
--- a/src/include/gpxe/tcp.h
+++ b/src/include/gpxe/tcp.h
@@ -298,6 +298,19 @@ struct tcp_options {
 			TCP_STATE_RCVD ( TCP_FIN ) ) )			    \
 	  == ( TCP_STATE_ACKED ( TCP_FIN ) | TCP_STATE_RCVD ( TCP_FIN ) ) )
 
+/** TCP close activities in progress
+ *
+ * TCP is in a state that connection is trying to close, and should
+ * wait for the state to TIME_WAIT or CLOSED.
+ * This is used for preventing remote peer dangling TCP state.
+ */
+#define TCP_CLOSE_INPROGRESS(state)					    \
+	( (state) == TCP_CLOSE_WAIT ||					    \
+	  (state) == TCP_LAST_ACK   ||					    \
+	  (state) == TCP_CLOSING    ||					    \
+	  (state) == TCP_FIN_WAIT_1 ||					    \
+	  (state) == TCP_FIN_WAIT_2 )
+
 /** @} */
 
 /** Mask for TCP header length field */
diff --git a/src/net/tcp.c b/src/net/tcp.c
index 41fae41..f25a83e 100644
--- a/src/net/tcp.c
+++ b/src/net/tcp.c
@@ -7,6 +7,7 @@
 #include <gpxe/timer.h>
 #include <gpxe/iobuf.h>
 #include <gpxe/malloc.h>
+#include <gpxe/activity.h>
 #include <gpxe/retry.h>
 #include <gpxe/refcnt.h>
 #include <gpxe/xfer.h>
@@ -95,6 +96,9 @@ struct tcp_connection {
 	struct list_head queue;
 	/** Retransmission timer */
 	struct retry_timer timer;
+
+	/** Activity started **/
+	int act_started;
 };
 
 /**
@@ -145,6 +149,18 @@ tcp_dump_state ( struct tcp_connection *tcp ) {
 		DBGC ( tcp, "TCP %p transitioned from %s to %s\n", tcp,
 		       tcp_state ( tcp->prev_tcp_state ),
 		       tcp_state ( tcp->tcp_state ) );
+
+		if ( TCP_CLOSE_INPROGRESS ( tcp->tcp_state ) &&
+		     ! tcp->act_started ) {
+			activity_start();
+			tcp->act_started = 1;
+		}
+
+		if ( TCP_CLOSED_GRACEFULLY( tcp->tcp_state ) &&
+		     tcp->act_started ) {
+			activity_stop();
+			tcp->act_started = 0;
+		}
 	}
 	tcp->prev_tcp_state = tcp->tcp_state;
 }
diff --git a/src/usr/autoboot.c b/src/usr/autoboot.c
index d76751b..0299b12 100644
--- a/src/usr/autoboot.c
+++ b/src/usr/autoboot.c
@@ -27,6 +27,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <gpxe/image.h>
 #include <gpxe/sanboot.h>
 #include <gpxe/uri.h>
+#include <gpxe/activity.h>
 #include <usr/ifmgmt.h>
 #include <usr/route.h>
 #include <usr/dhcpmgmt.h>
@@ -232,6 +233,11 @@ void autoboot ( void ) {
 	for_each_netdev ( netdev ) {
 		if ( netdev == boot_netdev )
 			continue;
+		/**
+		 * Wait for possible pending activities.
+		 * Currently only TCP close event implemented it.
+		 */
+		activity_wait( ACTIVITY_TIMEOUT );
 		close_all_netdevs();
 		netboot ( netdev );
 	}
-- 
1.7.1



More information about the gPXE-devel mailing list