From 43092b7d087dbbefed0bd481d320d1f8b1294cb3 Mon Sep 17 00:00:00 2001 From: Hiroki Sato Date: Thu, 27 Feb 2020 19:49:59 +0000 Subject: [PATCH] Fix poor performance of ftp(1) due to small SO_SNDBUF and SO_RCVBUF. ftp(1) from vendor/tnftp always tried the following for every TCP connection: 1. Get the current buffer length of SO_SNDBUF and SO_RCVBUF by getsockopt(2). 2. Invoke setsockopt(2) to set them to the same values after checking if they are in a range between 8 KiB to 8 MiB. This behavior broke dynamic buffer sizing enabled by default (net.inet.tcp.{recv,send}buf_auto sysctls) and led to a very poor transfer rate. The fetch(1) utility does not have this problem. This change prevents SO_SNDBUF and SO_RCVBUF from configuring when the buffer auto-sizing is enabled unless the buffer sizes are explicitly specified. PR: 240827 Spotted by: Yuichiro NAITO MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D23732 --- contrib/tnftp/src/cmds.c | 8 ++++++-- contrib/tnftp/src/ftp_var.h | 2 ++ contrib/tnftp/src/main.c | 10 ++++++++++ contrib/tnftp/src/util.c | 14 ++++++++++++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/contrib/tnftp/src/cmds.c b/contrib/tnftp/src/cmds.c index a92d8c5672b6..7d79b43695b5 100644 --- a/contrib/tnftp/src/cmds.c +++ b/contrib/tnftp/src/cmds.c @@ -2653,10 +2653,14 @@ setxferbuf(int argc, char *argv[]) goto usage; } - if (dir & RATE_PUT) + if (dir & RATE_PUT) { sndbuf_size = size; - if (dir & RATE_GET) + auto_sndbuf = 0; + } + if (dir & RATE_GET) { rcvbuf_size = size; + auto_rcvbuf = 0; + } fprintf(ttyout, "Socket buffer sizes: send %d, receive %d.\n", sndbuf_size, rcvbuf_size); code = 0; diff --git a/contrib/tnftp/src/ftp_var.h b/contrib/tnftp/src/ftp_var.h index 5282077d304f..29ba390db8d7 100644 --- a/contrib/tnftp/src/ftp_var.h +++ b/contrib/tnftp/src/ftp_var.h @@ -298,6 +298,8 @@ GLOBAL int options; /* used during socket creation */ GLOBAL int sndbuf_size; /* socket send buffer size */ GLOBAL int rcvbuf_size; /* socket receive buffer size */ +GLOBAL int auto_sndbuf; /* flag: if != 0 then use auto sndbuf size */ +GLOBAL int auto_rcvbuf; /* flag: if != 0 then use auto rcvbuf size */ GLOBAL int macnum; /* number of defined macros */ GLOBAL struct macel macros[16]; diff --git a/contrib/tnftp/src/main.c b/contrib/tnftp/src/main.c index b987324e3415..eaa6b0b0024b 100644 --- a/contrib/tnftp/src/main.c +++ b/contrib/tnftp/src/main.c @@ -127,6 +127,9 @@ __RCSID(" NetBSD: main.c,v 1.117 2009/07/13 19:05:41 roy Exp "); #include #endif /* tnftp */ +#ifdef __FreeBSD__ +#include +#endif #define GLOBAL /* force GLOBAL decls in ftp_var.h to be declared */ #include "ftp_var.h" @@ -510,6 +513,13 @@ main(int volatile argc, char **volatile argv) (void)xsignal(SIGUSR2, crankrate); (void)xsignal(SIGWINCH, setttywidth); + auto_rcvbuf = ((sysctlbyname("net.inet.tcp.recvbuf_auto", + &auto_rcvbuf, &(size_t []){[0] = sizeof(int)}[0], NULL, 0) == 0) && + auto_rcvbuf == 1); + auto_sndbuf = ((sysctlbyname("net.inet.tcp.sendbuf_auto", + &auto_sndbuf, &(size_t []){[0] = sizeof(int)}[0], NULL, 0) == 0) && + auto_sndbuf == 1); + if (argc > 0) { if (isupload) { rval = auto_put(argc, argv, upload_path); diff --git a/contrib/tnftp/src/util.c b/contrib/tnftp/src/util.c index bf46f026dee3..9f4f1c2a3f99 100644 --- a/contrib/tnftp/src/util.c +++ b/contrib/tnftp/src/util.c @@ -1087,13 +1087,27 @@ setupsockbufsize(int sock) sndbuf_size); } +#ifdef __FreeBSD__ + DPRINTF("auto_rcvbuf = %d\n", auto_rcvbuf); + if (auto_sndbuf == 0) { +#endif if (setsockopt(sock, SOL_SOCKET, SO_SNDBUF, (void *)&sndbuf_size, sizeof(sndbuf_size)) == -1) warn("Unable to set sndbuf size %d", sndbuf_size); +#ifdef __FreeBSD__ + } +#endif +#ifdef __FreeBSD__ + DPRINTF("auto_sndbuf = %d\n", auto_sndbuf); + if (auto_rcvbuf == 0) { +#endif if (setsockopt(sock, SOL_SOCKET, SO_RCVBUF, (void *)&rcvbuf_size, sizeof(rcvbuf_size)) == -1) warn("Unable to set rcvbuf size %d", rcvbuf_size); +#ifdef __FreeBSD__ + } +#endif } /*