bin/132798: ggated/ggatec

Yoshihiro Ota ota at j.email.ne.jp
Wed Mar 18 22:20:02 PDT 2009


>Number:         132798
>Category:       bin
>Synopsis:       ggated/ggatec
>Confidential:   no
>Severity:       critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Mar 19 05:20:00 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Yoshihiro Ota
>Release:        Since 6.1-RELEASE to 8-CURRENT
>Organization:
>Environment:
%uname -a
FreeBSD xxx 7.1-RELEASE-p3 FreeBSD 7.1-RELEASE-p3 #463 r188634M: Thu Mar 12 12:27:06 EDT 2009     xxx:/usr/obj/usr/src/sys/GENERIC  i386

>Description:
Connection between ggate server and client slows down significantly while coying a  file, about more than 20MB file size can cause the problem.  During slowdown, both server and client wants for each other and its transmission rate is about 1 packet per second.  As a result, the system really hangs up on any responses as FS does not response in timely manner.  All the commands then takes a couple of minutes easily to response to the user.

Copying 20MB file taking more than 20 minuets is critically wrong.
>How-To-Repeat:
server# echo "192.168.0.0/8 RW /dev/ad1" > /etc/gg.exports
server# ggated

client# ggatec create -orw server /dev/ad1
client# mount /dev/ad1 /mnt/backup
client# rsync -a --delete /home /mnt/backup

This only happens when server CPU is 10 to 20 times slower than client and client writes to the device; as far as NIC, they are the same 100BASE-TX on these hosts.    It does not happen with all files, neither.
>Fix:
The problem is ggated and ggatec create 2 TCP uni-directional connections.  That is screwing up the TCP layer.  Using 1 TCP connection for bi-directional messaging fixes the problem.

Patch attached with submission follows:

diff -r 2fccee0290c5 -r bc62f8d4a1f0 ggatec/ggatec.c
--- a/ggatec/ggatec.c	Tue Mar 17 14:17:39 2009 -0400
+++ b/ggatec/ggatec.c	Wed Mar 18 14:46:02 2009 -0400
@@ -71,7 +71,7 @@
 static off_t mediasize;
 static unsigned sectorsize = 0;
 static unsigned timeout = G_GATE_TIMEOUT;
-static int sendfd, recvfd;
+static int fd = -1;
 static uint32_t token;
 static pthread_t sendtd, recvtd;
 static int reconnect;
@@ -183,7 +183,7 @@
 		hdr.gh_error = 0;
 		g_gate_swap2n_hdr(&hdr);
 
-		data = g_gate_send(sendfd, &hdr, sizeof(hdr), MSG_NOSIGNAL);
+		data = g_gate_send(fd, &hdr, sizeof(hdr), MSG_NOSIGNAL);
 		g_gate_swap2h_hdr(&hdr);
 		g_gate_log(LOG_DEBUG, "Sent hdr packet (seq=%llu).", hdr.gh_seq);
 		send_seq = hdr.gh_seq;
@@ -198,7 +198,7 @@
 		}
 
 		if (hdr.gh_cmd == GGATE_CMD_WRITE) {
-			data = g_gate_send(sendfd, ggio.gctl_data,
+			data = g_gate_send(fd, ggio.gctl_data,
 			    ggio.gctl_length, MSG_NOSIGNAL);
 			if (reconnect)
 				break;
@@ -231,7 +231,7 @@
 	ggio.gctl_data = buf;
 
 	for (;;) {
-		data = g_gate_recv(recvfd, &hdr, sizeof(hdr), MSG_WAITALL);
+		data = g_gate_recv(fd, &hdr, sizeof(hdr), MSG_WAITALL);
 		if (reconnect)
 			break;
 		g_gate_swap2h_hdr(&hdr);
@@ -254,7 +254,7 @@
 
 		if (ggio.gctl_error == 0 && ggio.gctl_cmd == GGATE_CMD_READ) {
 g_gate_log(LOG_DEBUG, "Received READ packet.");
-			data = g_gate_recv(recvfd, ggio.gctl_data,
+			data = g_gate_recv(fd, ggio.gctl_data,
 			    ggio.gctl_length, MSG_WAITALL);
 			if (reconnect)
 				break;
@@ -277,7 +277,7 @@
 }
 
 static int
-handshake(int dir)
+handshake(void)
 {
 	struct g_gate_version ver;
 	struct g_gate_cinit cinit;
@@ -367,7 +367,7 @@
 		close(sfd);
 		return (-1);
 	}
-	cinit.gc_flags = flags | dir;
+	cinit.gc_flags = flags;
 	cinit.gc_token = token;
 	cinit.gc_nconn = 2;
 	g_gate_swap2n_cinit(&cinit);
@@ -426,15 +426,8 @@
 	 * Our receive descriptor is connected to the send descriptor on the
 	 * server side.
 	 */
-	recvfd = handshake(GGATE_FLAG_SEND);
-	if (recvfd == -1)
-		return (0);
-	/*
-	 * Our send descriptor is connected to the receive descriptor on the
-	 * server side.
-	 */
-	sendfd = handshake(GGATE_FLAG_RECV);
-	if (sendfd == -1)
+	fd = handshake();
+	if (fd == -1)
 		return (0);
 	return (1);
 }
@@ -454,8 +447,7 @@
 	sendtd = pthread_self();
 	send_thread(NULL);
 	/* Disconnected. */
-	close(sendfd);
-	close(recvfd);
+	close(fd);
 }
 
 static void
diff -r 2fccee0290c5 -r bc62f8d4a1f0 ggated/ggated.c
--- a/ggated/ggated.c	Tue Mar 17 14:17:39 2009 -0400
+++ b/ggated/ggated.c	Wed Mar 18 14:46:02 2009 -0400
@@ -65,8 +65,7 @@
 	unsigned	 c_sectorsize;
 	unsigned	 c_flags;	/* flags (RO/RW) */
 	int		 c_diskfd;
-	int		 c_sendfd;
-	int		 c_recvfd;
+	int		 c_netfd;
 	time_t		 c_birthtime;
 	char		*c_path;
 	uint64_t	 c_token;
@@ -138,10 +137,12 @@
 	/* Use snprintf(3) so that we don't reenter stdio(3). */
 	LIST_FOREACH_SAFE(conn, &connections, c_next, tconn) {
 		snprintf(buf, sizeof(buf),
+			"token %llu\n"
 			"path %s\n"
 			"send_seq %llu\n"
 			"recv_seq %llu\n"
 			"",
+			conn->c_token,
 			conn->c_path,
 			conn->c_send_seq, conn->c_recv_seq);
 		write(STDERR_FILENO, buf, strlen(buf));
@@ -527,24 +528,11 @@
 			    ip2str((struct sockaddr*)&conn->c_srcaddr),
 			    conn->c_path);
 			close(conn->c_diskfd);
-			close(conn->c_sendfd);
-			close(conn->c_recvfd);
+			close(conn->c_netfd);
 			free(conn->c_path);
 			free(conn);
 		}
 	}
-}
-
-static struct ggd_connection *
-connection_find(struct g_gate_cinit *cinit)
-{
-	struct ggd_connection *conn;
-
-	LIST_FOREACH(conn, &connections, c_next) {
-		if (conn->c_token == cinit->gc_token)
-			break;
-	}
-	return (conn);
 }
 
 static struct ggd_connection *
@@ -570,11 +558,8 @@
 	}
 	conn->c_token = cinit->gc_token;
 	memcpy(&conn->c_srcaddr, s, s->sa_len);
-	conn->c_sendfd = conn->c_recvfd = -1;
-	if ((cinit->gc_flags & GGATE_FLAG_SEND) != 0)
-		conn->c_sendfd = sfd;
-	else
-		conn->c_recvfd = sfd;
+	conn->c_diskfd = -1;
+	conn->c_netfd = sfd;
 	conn->c_mediasize = 0;
 	conn->c_sectorsize = 0;
 	time(&conn->c_birthtime);
@@ -583,32 +568,6 @@
 	g_gate_log(LOG_DEBUG, "Connection created [%s, %s].", ip2str(s),
 	    conn->c_path);
 	return (conn);
-}
-
-static int
-connection_add(struct ggd_connection *conn, struct g_gate_cinit *cinit,
-    struct sockaddr *s, int sfd)
-{
-	if ((cinit->gc_flags & GGATE_FLAG_SEND) != 0) {
-		if (conn->c_sendfd != -1) {
-			g_gate_log(LOG_WARNING,
-			    "Send socket already exists [%s, %s].", ip2str(s),
-			    conn->c_path);
-			return (EEXIST);
-		}
-		conn->c_sendfd = sfd;
-	} else {
-		if (conn->c_recvfd != -1) {
-			g_gate_log(LOG_WARNING,
-			    "Receive socket already exists [%s, %s].",
-			    ip2str(s), conn->c_path);
-			return (EEXIST);
-		}
-		conn->c_recvfd = sfd;
-	}
-	g_gate_log(LOG_DEBUG, "Connection added [%s, %s].", ip2str(s),
-	    conn->c_path);
-	return (0);
 }
 
 /*
@@ -622,20 +581,11 @@
 	LIST_REMOVE(conn, c_next);
 	g_gate_log(LOG_DEBUG, "Connection removed [%s %s].",
 	    ip2str((struct sockaddr*)&conn->c_srcaddr), conn->c_path);
-	if (conn->c_sendfd != -1)
-		close(conn->c_sendfd);
-	if (conn->c_recvfd != -1)
-		close(conn->c_recvfd);
-	close(conn->c_diskfd);
+	close(conn->c_netfd);
+	if(conn->c_diskfd == -1)
+		close(conn->c_diskfd);
 	free(conn->c_path);
 	free(conn);
-}
-
-static int
-connection_ready(struct ggd_connection *conn)
-{
-
-	return (conn->c_sendfd != -1 && conn->c_recvfd != -1);
 }
 
 static void
@@ -742,7 +692,7 @@
 
 	conn = arg;
 	g_gate_log(LOG_NOTICE, "%s: started [%s]!", __func__, conn->c_path);
-	fd = conn->c_recvfd;
+	fd = conn->c_netfd;
 	for (;;) {
 		/*
 		 * Get header packet.
@@ -886,7 +836,7 @@
 
 	conn = arg;
 	g_gate_log(LOG_NOTICE, "%s: started [%s]!", __func__, conn->c_path);
-	fd = conn->c_sendfd;
+	fd = conn->c_netfd;
 	for (;;) {
 		/*
 		 * Get a request from the outgoing queue.
@@ -937,6 +887,9 @@
 	g_gate_log(LOG_INFO, "Connection from: %s.", ip2str(from));
 }
 
+/*
+ * handshake return 1 if sfd is closed; otherwise 0 so that caller has to close.
+ */
 static int
 handshake(struct sockaddr *from, int sfd)
 {
@@ -989,36 +942,23 @@
 		return (0);
 	}
 	g_gate_log(LOG_DEBUG, "Initial packet received.");
-	conn = connection_find(&cinit);
-	if (conn != NULL) {
-		/*
-		 * Connection should already exists.
-		 */
-		g_gate_log(LOG_DEBUG, "Found existing connection (token=%lu).",
-		    (unsigned long)conn->c_token);
-		if (connection_add(conn, &cinit, from, sfd) == -1) {
-			connection_remove(conn);
-			return (0);
-		}
-	} else {
-		/*
-		 * New connection, allocate space.
-		 */
-		conn = connection_new(&cinit, from, sfd);
-		if (conn == NULL) {
-			sendfail(sfd, ENOMEM,
-			    "Cannot allocate new connection.");
-			return (0);
-		}
-		g_gate_log(LOG_DEBUG, "New connection created (token=%lu).",
-		    (unsigned long)conn->c_token);
+
+	/*
+	 * New connection, allocate space.
+	 */
+	conn = connection_new(&cinit, from, sfd);
+	if (conn == NULL) {
+		sendfail(sfd, ENOMEM,
+		    "Cannot allocate new connection.");
+		return (0);
 	}
+	g_gate_log(LOG_DEBUG, "New connection created (token=%lu).",
+	    (unsigned long)conn->c_token);
 
 	ex = exports_find(from, &cinit, conn);
 	if (ex == NULL) {
-		connection_remove(conn);
 		sendfail(sfd, errno, NULL);
-		return (0);
+		goto out;
 	}
 	if (conn->c_mediasize == 0) {
 		conn->c_mediasize = g_gate_mediasize(conn->c_diskfd);
@@ -1033,16 +973,13 @@
 	g_gate_swap2n_sinit(&sinit);
 	data = g_gate_send(sfd, &sinit, sizeof(sinit), 0);
 	g_gate_swap2h_sinit(&sinit);
-	if (data == -1) {
+	if (data == -1)
 		sendfail(sfd, errno, "Error while sending initial packet: %s.",
 		    strerror(errno));
-		return (0);
-	}
-
-	if (connection_ready(conn)) {
+	else
 		connection_launch(conn);
-		connection_remove(conn);
-	}
+out:
+	connection_remove(conn);
 	return (1);
 }
 
diff -r 2fccee0290c5 -r bc62f8d4a1f0 shared/ggate.h
--- a/shared/ggate.h	Tue Mar 17 14:17:39 2009 -0400
+++ b/shared/ggate.h	Wed Mar 18 14:46:02 2009 -0400
@@ -41,18 +41,10 @@
 #define	G_GATE_TIMEOUT		0
 
 #define	GGATE_MAGIC		"GEOM_GATE       "
-#define	GGATE_VERSION		0
+#define	GGATE_VERSION		1
 
 #define	GGATE_FLAG_RDONLY	0x0001
 #define	GGATE_FLAG_WRONLY	0x0002
-/*
- * If GGATE_FLAG_SEND not GGATE_FLAG_RECV flag is set, this is initial
- * connection.
- * If GGATE_FLAG_SEND flag is set - this is socket to send data.
- * If GGATE_FLAG_RECV flag is set - this is socket to receive data.
- */
-#define	GGATE_FLAG_SEND		0x0004
-#define	GGATE_FLAG_RECV		0x0008
 
 #define	GGATE_CMD_READ		0
 #define	GGATE_CMD_WRITE		1


>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list