Read buffer size in clientloop.c

[Darren Tucker]

On Tue, Jul 07, 2009 at 12:57:16PM +0200, Corinna Vinschen wrote:
> when trying to optimize socket transfer rates under Cygwin, it turned
> out that the underlying WinSock implementation is surprisingly sensitive
> to buffer sizes.  The latest Cygwin from CVS is now setting the socket
> receive/send buffers (SO_RCVBUF/SO_SNDBUF) to 64K, rather than keeping
> them at their default values of 8K which thwarts data transfers a lot.
> 
> While testing I still had the problem that for some reason the ssh read
> transfer rates were only a third up to a half of the write transfer
> rates.  It turned out that the culprit was ssh itself.  In clientloop.c,
> it defines read buffers of the size 8K.  Setting them to 64K under
> Cygwin raises the read transfer rates comparable to the write rates.
> 
> It occured to me that we talked about this already 3 years ago in a
> thread about HPN-SSH, just the performance numbers were different:
> http://marc.info/?l=openssh-unix-dev&m=114414372902485&w=2
> 
> So the question I have is again this:
> 
> Would it be ok to raise the buffers in client_process_net_input() and
> client_process_input() to 64K, maybe only on Cygwin?  Or to make the
> buffer sizes a configurable option?

If there's a measureable performance gain (which it sounds like there is)
then I personally have no objection to making it a compile time option.
Damien?  Maybe something like this?

Index: clientloop.c
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/clientloop.c,v
retrieving revision 1.201
diff -u -p -r1.201 clientloop.c
--- clientloop.c	5 Jul 2009 21:17:35 -0000	1.201
+++ clientloop.c	7 Jul 2009 11:38:33 -0000
@@ -636,7 +636,7 @@ static void
 client_process_net_input(fd_set *readset)
 {
 	int len, cont = 0;
-	char buf[8192];
+	char buf[SSH_IOBUFSZ];
 
 	/*
 	 * Read input from the server, and add any such data to the buffer of
@@ -1129,7 +1129,7 @@ static void
 client_process_input(fd_set *readset)
 {
 	int len;
-	char buf[8192];
+	char buf[SSH_IOBUFSZ];
 
 	/* Read input from stdin. */
 	if (FD_ISSET(fileno(stdin), readset)) {
Index: configure.ac
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/configure.ac,v
retrieving revision 1.420
diff -u -p -r1.420 configure.ac
--- configure.ac	16 Jun 2009 06:11:02 -0000	1.420
+++ configure.ac	7 Jul 2009 11:41:04 -0000
@@ -442,6 +442,7 @@ int main(void) { exit(0); }
 	AC_DEFINE(DISABLE_FD_PASSING, 1,
 		[Define if your platform needs to skip post auth
 		file descriptor passing])
+	AC_DEFINE(SSH_IOBUFSZ, 65536, [Windows is sensitive to read buffer size])
 	;;
 *-*-dgux*)
 	AC_DEFINE(IP_TOS_IS_BROKEN, 1,
Index: defines.h
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/defines.h,v
retrieving revision 1.155
diff -u -p -r1.155 defines.h
--- defines.h	16 Jun 2009 06:11:02 -0000	1.155
+++ defines.h	7 Jul 2009 11:39:15 -0000
@@ -749,4 +749,8 @@ struct winsize {
 #define INET6_ADDRSTRLEN 46
 #endif
 
+#ifndef SSH_IOBUFSZ
+# define SSH_IOBUFSZ 8192
+#endif
+
 #endif /* _DEFINES_H */

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.