Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 3 Sep 2011 13:00:49 -0600 (MDT)
From:      Ian Lepore <freebsd@damnhippie.dyndns.org>
To:        FreeBSD-gnats-submit@FreeBSD.org
Subject:   kern/160433: [patch] syslogd receiver buffer sizes set incorrectly
Message-ID:  <201109031900.p83J0n36099006@revolution.hippie.lan>
Resent-Message-ID: <201109031910.p83JA7As075186@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         160433
>Category:       kern
>Synopsis:       [patch] syslogd receiver buffer sizes set incorrectly
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Sep 03 19:10:07 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Ian Lepore <freebsd@damnhippie.dyndns.org>
>Release:        FreeBSD 8.2-RC3 arm
>Organization:
none
>Environment:
FreeBSD dvb 8.2-RC3 FreeBSD 8.2-RC3 #49: Tue Feb 15 22:52:14 UTC 2011     root@revolution.hippie.lan:/usr/obj/arm/usr/src/sys/DVB  arm

>Description:
This change fixes 3 problems in syslogd related to sizing receive buffers.

 - A call was misplaced at the wrong level of nested if blocks, so that the
   buffers for unix domain sockets (/dev/log, /dev/klog) were never increased
   at all; they remained at a way-too-small default size of 4096.

- The function that was supposed to double the size of the buffer sometimes 
  did nothing, and sometimes installed a wildly-wrong buffer size (either too 
  large or too small) due to an unitialized arglen variable passed to 
  getsockopt().  Most often it doubled the UDP buffers from 40k to 80k because 
  accidentally there would be harmless stack garbage in the unitialized 
  variables.  

- The whole concept of blindly doubling a socket's buffer size without 
  knowing what size it started at seems like a design flaw.  If the 
  double_rbuf() function had worked at all (I.E., if the other two bugs didn't 
  exist) this would lead to UDP sockets having an 80k buffer while unix dgram 
  sockets get an 8k buffer.  There's nothing about the problem being solved 
  that requires larger buffers for UDP than for unix dgram sockets -- the 
  buffering requirements are the same regardless of socket type.  

This change renames the double_rbuf() function to increase_rbuf() and 
increases the buffer size on all types of sockets to 80k.  80k was chosen 
only because it appears to be the size the original change was shooting for, 
and it certainly seems to be reasonably large (I might have picked 64k in 
the absence of any historical guidance).  

This change also fixes a problem where syslogd is too quick to close sockets
on errors.  Add a couple more error codes to ignore from sendto(), so that 
transient errors (such as would occur when restarting a network interface,
for example) get recovered.


>How-To-Repeat:
Log lots of messages from an app running on a heavily-loaded underpowered
system such as a 200mhz ARM; without the larger buffers for the unix-domain
sockets many messages get dropped in such an environment.

>Fix:
This problem was discovered in the 8.2 environment, but this diff/patch
is to -current.


--- diff.tmp begins here ---
Index: syslogd.c
===================================================================
RCS file: /local/base/FreeBSD-CVS/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.169
diff -u -p -r1.169 syslogd.c
--- syslogd.c	14 Jul 2011 07:33:53 -0000	1.169
+++ syslogd.c	3 Sep 2011 18:41:55 -0000
@@ -74,6 +74,7 @@ __FBSDID("$FreeBSD: src/usr.sbin/syslogd
 #define	DEFSPRI		(LOG_KERN|LOG_CRIT)
 #define	TIMERINTVL	30		/* interval for checking flush, mark */
 #define	TTYMSGTIME	1		/* timeout passed to ttymsg */
+#define	RCVBUF_MINSIZE	(80 * 1024)	/* minimum size of dgram rcv buffer */
 
 #include <sys/param.h>
 #include <sys/ioctl.h>
@@ -336,7 +337,7 @@ static void	unmapped(struct sockaddr *);
 static void	wallmsg(struct filed *, struct iovec *, const int iovlen);
 static int	waitdaemon(int, int, int);
 static void	timedout(int);
-static void	double_rbuf(int);
+static void	increase_rcvbuf(int);
 
 int
 main(int argc, char *argv[])
@@ -547,8 +548,8 @@ main(int argc, char *argv[])
 				STAILQ_REMOVE(&funixes, fx, funix, next);
 				continue;
 			}
-			double_rbuf(fx->s);
 		}
+		increase_rcvbuf(fx->s);
 	}
 	if (SecureMode <= 1)
 		finet = socksetup(family, bindhostname);
@@ -1241,8 +1242,10 @@ fprintlog(struct filed *f, int flags, co
 				switch (errno) {
 				case ENOBUFS:
 				case ENETDOWN:
+				case ENETUNREACH:
 				case EHOSTUNREACH:
 				case EHOSTDOWN:
+				case EADDRNOTAVAIL:
 					break;
 				/* case EBADF: */
 				/* case EACCES: */
@@ -1253,7 +1256,7 @@ fprintlog(struct filed *f, int flags, co
 				/* case ENOBUFS: */
 				/* case ECONNREFUSED: */
 				default:
-					dprintf("removing entry\n");
+					dprintf("removing entry: errno=%d\n", e);
 					f->f_type = F_UNUSED;
 					break;
 				}
@@ -2697,6 +2700,9 @@ socksetup(int af, char *bindhostname)
 		 * If the system administrator choose not to obey
 		 * this, we can skip the bind() step so that the
 		 * system will choose a port for us.
+		 *
+		 * In SecureMode we will shutdown() the receive side of
+		 * the socket, so no need for a bigger receiver buffer.
 		 */
 		if (!NoBind) {
 			if (bind(*s, r->ai_addr, r->ai_addrlen) < 0) {
@@ -2704,9 +2710,8 @@ socksetup(int af, char *bindhostname)
 				logerror("bind");
 				continue;
 			}
-
 			if (!SecureMode)
-				double_rbuf(*s);
+				increase_rcvbuf(*s);
 		}
 
 		(*socks)++;
@@ -2727,12 +2732,15 @@ socksetup(int af, char *bindhostname)
 }
 
 static void
-double_rbuf(int fd)
+increase_rcvbuf(int fd)
 {
-	socklen_t slen, len;
+	socklen_t len;
+	socklen_t slen = sizeof(len);
 
 	if (getsockopt(fd, SOL_SOCKET, SO_RCVBUF, &len, &slen) == 0) {
-		len *= 2;
-		setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &len, slen);
+		if (len < RCVBUF_MINSIZE) {
+			len = RCVBUF_MINSIZE;
+			setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &len, sizeof(len));
+		}
 	}
 }
--- diff.tmp ends here ---

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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201109031900.p83J0n36099006>