Subject: RFC on two possible ftpd changes
To: None <tech-userlevel@NetBSD.org>
From: Brian Ginsbach <ginsbach@NetBSD.org>
List: tech-userlevel
Date: 08/16/2005 18:17:27
--9amGYk9869ThD9tj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline


I've got a couple changes to ftpd that I'm wondering if I should
commit.  I've discovered a performance issue, at least on some
systems when using tnftpd, with puts to the server.  The performance
is poor because the buffer used by the server is too small.

I've coded a patch to make receive_data() work very similar to
send_data_with_read().  This patch makes the readsize configuration
parameter apply to reads from the disk (sending data to a client)
and reads from the network (receiving data from a client).  If the
readsize isn't set, again similar to send_data_with_read(), the
filesystem block size is used.  I kept BUFSIZ as a failsafe incase
the fstat(2) failed.

Also for some additional performance tuning ability I added an
additional configuration parameter: recvbufsize.  This is very
similar to the current sendbufsize option.

I'd like to hear thoughts/comments on these changes.  Patch attached.

--
Brian Ginsbach

--9amGYk9869ThD9tj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ftpd.patch"

Index: conf.c
===================================================================
RCS file: /cvsroot/src/libexec/ftpd/conf.c,v
retrieving revision 1.54
diff -u -r1.54 conf.c
--- conf.c	3 Mar 2005 22:19:47 -0000	1.54
+++ conf.c	16 Aug 2005 17:45:53 -0000
@@ -389,6 +389,10 @@
 			curclass.writesize = 0;
 			CONF_LL(writesize, arg, 0, LLTMAX);
 
+		} else if (strcasecmp(word, "recvbufsize") == 0) {
+			curclass.recvbufsize = 0;
+			CONF_LL(recvbufsize, arg, 0, LLTMAX);
+
 		} else if (strcasecmp(word, "sendbufsize") == 0) {
 			curclass.sendbufsize = 0;
 			CONF_LL(sendbufsize, arg, 0, LLTMAX);
Index: extern.h
===================================================================
RCS file: /cvsroot/src/libexec/ftpd/extern.h,v
retrieving revision 1.53
diff -u -r1.53 extern.h
--- extern.h	23 Jun 2005 04:20:41 -0000	1.53
+++ extern.h	16 Aug 2005 17:45:53 -0000
@@ -298,6 +298,7 @@
 	LLT		 mmapsize;	/* mmap window size */
 	LLT		 readsize;	/* data read size */
 	LLT		 writesize;	/* data write size */
+	LLT		 recvbufsize;	/* SO_RCVBUF size */
 	LLT		 sendbufsize;	/* SO_SNDBUF size */
 	LLT		 sendlowat;	/* SO_SNDLOWAT size */
 };
Index: ftpd.c
===================================================================
RCS file: /cvsroot/src/libexec/ftpd/ftpd.c,v
retrieving revision 1.167
diff -u -r1.167 ftpd.c
--- ftpd.c	4 Aug 2005 17:41:35 -0000	1.167
+++ ftpd.c	16 Aug 2005 17:45:54 -0000
@@ -2283,8 +2283,10 @@
 {
 	int	c, bare_lfs, netfd, filefd, rval;
 	off_t	byteswritten;
-	char	buf[BUFSIZ];
+	char	*buf;
+	size_t	readsize;
 	struct sigaction sa, sa_saved;
+	struct stat st;
 #ifdef __GNUC__
 	(void) &bare_lfs;
 #endif
@@ -2300,6 +2302,7 @@
 	transflag = 1;
 	rval = -1;
 	byteswritten = 0;
+	buf = NULL;
 
 #define FILESIZECHECK(x) \
 			do { \
@@ -2317,6 +2320,16 @@
 		netfd = fileno(instr);
 		filefd = fileno(outstr);
 		(void) alarm(curclass.timeout);
+		if (curclass.readsize)
+			readsize = curclass.readsize;
+		else if (fstat(filefd, &st))
+			readsize = (size_t)st.st_blksize;
+		else
+			readsize = BUFSIZ;
+		if ((buf = malloc(readsize)) == NULL) {
+			perror_reply(451, "Local resource failure: malloc");
+			goto cleanup_recv_data;
+		}
 		if (curclass.rateput) {
 			while (1) {
 				int d;
@@ -2327,7 +2340,7 @@
 				errno = c = d = 0;
 				for (bufrem = curclass.rateput; bufrem > 0; ) {
 					if ((c = read(netfd, buf,
-					    MIN(sizeof(buf), bufrem))) <= 0)
+					    MIN(readsize, bufrem))) <= 0)
 						goto recvdone;
 					if (urgflag && handleoobcmd())
 						goto cleanup_recv_data;
@@ -2348,7 +2361,7 @@
 					usleep(1000000 - td.tv_usec);
 			}
 		} else {
-			while ((c = read(netfd, buf, sizeof(buf))) > 0) {
+			while ((c = read(netfd, buf, readsize)) > 0) {
 				if (urgflag && handleoobcmd())
 					goto cleanup_recv_data;
 				FILESIZECHECK(byte_count + c);
@@ -2444,6 +2457,8 @@
  cleanup_recv_data:
 	(void) alarm(0);
 	(void) sigaction(SIGALRM, &sa_saved, NULL);
+	if (buf)
+		free(buf);
 	transflag = 0;
 	urgflag = 0;
 	total_files_in++;
@@ -2691,6 +2706,11 @@
 			reply(0, "Write size: " LLF, (LLT)curclass.writesize);
 		else
 			reply(0, "Write size: default");
+		if (curclass.recvbufsize)
+			reply(0, "Receive buffer size: " LLF,
+			    (LLT)curclass.recvbufsize);
+		else
+			reply(0, "Receive buffer size: default");
 		if (curclass.sendbufsize)
 			reply(0, "Send buffer size: " LLF,
 			    (LLT)curclass.sendbufsize);
@@ -2910,7 +2930,7 @@
 void
 passive(void)
 {
-	int len;
+	int len, recvbufsize;
 	char *p, *a;
 
 	if (pdata >= 0)
@@ -2928,6 +2948,13 @@
 	if (getsockname(pdata, (struct sockaddr *) &pasv_addr.si_su, &len) < 0)
 		goto pasv_error;
 	pasv_addr.su_len = len;
+	if (curclass.recvbufsize) {
+		recvbufsize = curclass.recvbufsize;
+		if (setsockopt(pdata, SOL_SOCKET, SO_RCVBUF, &recvbufsize,
+			       sizeof(int)) == -1)
+			syslog(LOG_WARNING, "setsockopt(SO_RCVBUF, %d): %m",
+			       recvbufsize);
+	}
 	if (listen(pdata, 1) < 0)
 		goto pasv_error;
 	if (curclass.advertise.su_len != 0)
Index: ftpd.conf.5
===================================================================
RCS file: /cvsroot/src/libexec/ftpd/ftpd.conf.5,v
retrieving revision 1.29
diff -u -r1.29 ftpd.conf.5
--- ftpd.conf.5	3 Mar 2005 22:19:47 -0000	1.29
+++ ftpd.conf.5	16 Aug 2005 17:45:55 -0000
@@ -578,6 +578,19 @@
 or
 .Ar size
 is not specified, use the default.
+.It Sy recvbufsize Ar class Op Ar size
+Set the size of the socket recveive buffer.
+An optional suffix may be provided as per
+.Sy rateget .
+The default is zero and the system default value will be used.
+This option affects only passive transfers.
+If
+.Ar class
+is
+.Dq none
+or
+.Ar size
+is not specified, use the default.
 .It Sy sanenames Ar class Op Sy off
 If
 .Ar class

--9amGYk9869ThD9tj--