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>