Skip to content

Commit e903aa1

Browse files
committed
Testing the removal of the HPNBufferSize and TcpRcvBuf options as
a) they don't seem to have any sort of impact anymore. This was likely due to a change in the code sometime in the past 16 years that I overlooked. b) the only reason to reimplement them is to create a bandwidth limiting service which can be implemented in other, more straighforward ways The upside is that it decreases the codes complexity a bit along with making it less likely to generate user confusion. Follow up is to see what else can be simplified and possibly even removing options.hpn_buffer_size entirely. It's still there because some code paths use it but I don't know if it makes sense in those situations (like X11 forwarding).
1 parent 8b3859a commit e903aa1

File tree

7 files changed

+6
-124
lines changed

7 files changed

+6
-124
lines changed

channels.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ static int rdynamic_connect_finish(struct ssh *, Channel *);
229229
/* Setup helper */
230230
static void channel_handler_init(struct ssh_channels *sc);
231231

232+
/* default values to enable hpn and the initial buffer size */
232233
static int hpn_disabled = 0;
233234
static int hpn_buffer_size = 2 * 1024 * 1024;
234235

readconf.c

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ typedef enum {
170170
oHashKnownHosts,
171171
oTunnel, oTunnelDevice,
172172
oLocalCommand, oPermitLocalCommand, oRemoteCommand,
173-
oTcpRcvBufPoll, oTcpRcvBuf, oHPNDisabled, oHPNBufferSize,
173+
oTcpRcvBufPoll, oHPNDisabled,
174174
oNoneEnabled, oNoneMacEnabled, oNoneSwitch, oHPNBufferLimit,
175175
oMetrics, oMetricsPath, oMetricsInterval, oFallback, oFallbackPort,
176176
oVisualHostKey,
@@ -339,9 +339,7 @@ static struct {
339339
{ "securitykeyprovider", oSecurityKeyProvider },
340340
{ "knownhostscommand", oKnownHostsCommand },
341341
{ "tcprcvbufpoll", oTcpRcvBufPoll },
342-
{ "tcprcvbuf", oTcpRcvBuf },
343342
{ "hpndisabled", oHPNDisabled },
344-
{ "hpnbuffersize", oHPNBufferSize },
345343
{ "requiredrsasize", oRequiredRSASize },
346344
{ "enableescapecommandline", oEnableEscapeCommandline },
347345
{ "obscurekeystroketiming", oObscureKeystrokeTiming },
@@ -1238,10 +1236,6 @@ process_config_line_depth(Options *options, struct passwd *pw, const char *host,
12381236
intptr = &options->hpn_disabled;
12391237
goto parse_flag;
12401238

1241-
case oHPNBufferSize:
1242-
intptr = &options->hpn_buffer_size;
1243-
goto parse_int;
1244-
12451239
case oTcpRcvBufPoll:
12461240
intptr = &options->tcp_rcv_buf_poll;
12471241
goto parse_flag;
@@ -1553,10 +1547,6 @@ process_config_line_depth(Options *options, struct passwd *pw, const char *host,
15531547
*intptr = value;
15541548
break;
15551549

1556-
case oTcpRcvBuf:
1557-
intptr = &options->tcp_rcv_buf;
1558-
goto parse_int;
1559-
15601550
case oCiphers:
15611551
arg = argv_next(&ac, &av);
15621552
if (!arg || *arg == '\0') {
@@ -2644,12 +2634,10 @@ initialize_options(Options * options)
26442634
options->metrics_path = NULL;
26452635
options->metrics_interval = -1;
26462636
options->hpn_disabled = -1;
2647-
options->hpn_buffer_size = -1;
26482637
options->hpn_buffer_limit = -1;
26492638
options->fallback = -1;
26502639
options->fallback_port = -1;
26512640
options->tcp_rcv_buf_poll = -1;
2652-
options->tcp_rcv_buf = -1;
26532641
options->session_type = -1;
26542642
options->stdin_null = -1;
26552643
options->fork_after_authentication = -1;
@@ -2821,24 +2809,8 @@ fill_default_options(Options * options)
28212809
options->server_alive_count_max = 3;
28222810
if (options->hpn_disabled == -1)
28232811
options->hpn_disabled = 0;
2824-
if (options->hpn_buffer_size > -1) {
2825-
/* if a user tries to set the size to 0 set it to 1KB */
2826-
if (options->hpn_buffer_size == 0)
2827-
options->hpn_buffer_size = 1;
2828-
/* limit the buffer to SSHBUF_SIZE_MAX (currently 256MB) */
2829-
if (options->hpn_buffer_size > (SSHBUF_SIZE_MAX / 1024)) {
2830-
options->hpn_buffer_size = SSHBUF_SIZE_MAX;
2831-
debug("User requested buffer larger than 256MB. Request reverted to 256MB");
2832-
} else
2833-
options->hpn_buffer_size *= 1024;
2834-
debug("hpn_buffer_size set to %d", options->hpn_buffer_size);
2835-
}
28362812
if (options->hpn_buffer_limit == -1)
28372813
options->hpn_buffer_limit = 0;
2838-
if (options->tcp_rcv_buf == 0)
2839-
options->tcp_rcv_buf = 1;
2840-
if (options->tcp_rcv_buf > -1)
2841-
options->tcp_rcv_buf *=1024;
28422814
if (options->tcp_rcv_buf_poll == -1)
28432815
options->tcp_rcv_buf_poll = 1;
28442816
if (options->none_switch == -1)

readconf.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,9 @@ typedef struct {
5050
int strict_host_key_checking; /* Strict host key checking. */
5151
int compression; /* Compress packets in both directions. */
5252
int tcp_keep_alive; /* Set SO_KEEPALIVE. */
53-
int tcp_rcv_buf; /* user switch to set tcp recv buffer */
5453
int tcp_rcv_buf_poll; /* Option to poll recv buf every window transfer */
5554
int hpn_disabled; /* Switch to disable HPN buffer management */
56-
int hpn_buffer_size; /* User definable size for HPN buffer window */
55+
int hpn_buffer_size; /* Size of HPN buffer window */
5756
int hpn_buffer_limit; /* limit local_window_max to 1/2 receive buffer */
5857
int ip_qos_interactive; /* IP ToS/DSCP/class for interactive */
5958
int ip_qos_bulk; /* IP ToS/DSCP/class for bulk traffic */

servconf.c

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ initialize_server_options(ServerOptions *options)
190190
options->authorized_principals_command_user = NULL;
191191
options->tcp_rcv_buf_poll = -1;
192192
options->hpn_disabled = -1;
193-
options->hpn_buffer_size = -1;
194193
options->none_enabled = -1;
195194
options->nonemac_enabled = -1;
196195
options->hpn_buffer_limit = -1;
@@ -448,39 +447,6 @@ fill_default_server_options(ServerOptions *options)
448447
options->hpn_disabled = 0;
449448
if (options->hpn_buffer_limit == -1)
450449
options->hpn_buffer_limit = 0;
451-
452-
if (options->hpn_buffer_size == -1) {
453-
/* option not explicitly set. Now we have to figure out */
454-
/* what value to use */
455-
if (options->hpn_disabled == 1) {
456-
options->hpn_buffer_size = CHAN_SES_WINDOW_DEFAULT;
457-
} else {
458-
/* get the current RCV size and set it to that */
459-
/*create a socket but don't connect it */
460-
/* we use that the get the rcv socket size */
461-
sock = socket(AF_INET, SOCK_STREAM, 0);
462-
getsockopt(sock, SOL_SOCKET, SO_RCVBUF,
463-
&socksize, &socksizelen);
464-
close(sock);
465-
options->hpn_buffer_size = socksize;
466-
debug("HPN Buffer Size: %d", options->hpn_buffer_size);
467-
}
468-
} else {
469-
/* we have to do this in case the user sets both values in a contradictory */
470-
/* manner. hpn_disabled overrrides hpn_buffer_size*/
471-
if (options->hpn_disabled <= 0) {
472-
if (options->hpn_buffer_size == 0)
473-
options->hpn_buffer_size = 1;
474-
/* limit the maximum buffer to SSHBUF_SIZE_MAX (currently 256MB) */
475-
if (options->hpn_buffer_size > (SSHBUF_SIZE_MAX / 1024)) {
476-
options->hpn_buffer_size = SSHBUF_SIZE_MAX;
477-
} else {
478-
options->hpn_buffer_size *= 1024;
479-
}
480-
} else
481-
options->hpn_buffer_size = CHAN_TCP_WINDOW_DEFAULT;
482-
}
483-
484450
if (options->ip_qos_interactive == -1)
485451
options->ip_qos_interactive = IPTOS_DSCP_AF21;
486452
if (options->ip_qos_bulk == -1)
@@ -563,7 +529,7 @@ typedef enum {
563529
sKbdInteractiveAuthentication, sListenAddress, sAddressFamily,
564530
sPrintMotd, sPrintLastLog, sIgnoreRhosts,
565531
sNoneEnabled, sNoneMacEnabled, sHPNBufferLimit,
566-
sTcpRcvBufPoll, sHPNDisabled, sHPNBufferSize,
532+
sTcpRcvBufPoll, sHPNDisabled,
567533
sX11Forwarding, sX11DisplayOffset, sX11UseLocalhost,
568534
sPermitTTY, sStrictModes, sEmptyPasswd, sTCPKeepAlive,
569535
sPermitUserEnvironment, sAllowTcpForwarding, sCompression,
@@ -731,7 +697,6 @@ static struct {
731697
{ "trustedusercakeys", sTrustedUserCAKeys, SSHCFG_ALL },
732698
{ "authorizedprincipalsfile", sAuthorizedPrincipalsFile, SSHCFG_ALL },
733699
{ "hpndisabled", sHPNDisabled, SSHCFG_ALL },
734-
{ "hpnbuffersize", sHPNBufferSize, SSHCFG_ALL },
735700
{ "tcprcvbufpoll", sTcpRcvBufPoll, SSHCFG_ALL },
736701
{ "noneenabled", sNoneEnabled, SSHCFG_ALL },
737702
{ "nonemacenabled", sNoneMacEnabled, SSHCFG_ALL },
@@ -1632,10 +1597,6 @@ process_server_config_line_depth(ServerOptions *options, char *line,
16321597
intptr = &options->hpn_disabled;
16331598
goto parse_flag;
16341599

1635-
case sHPNBufferSize:
1636-
intptr = &options->hpn_buffer_size;
1637-
goto parse_int;
1638-
16391600
case sIgnoreUserKnownHosts:
16401601
intptr = &options->ignore_user_known_hosts;
16411602
parse_flag:

servconf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ typedef struct {
200200
int use_pam; /* Enable auth via PAM */
201201
int tcp_rcv_buf_poll; /* poll tcp rcv window in autotuning kernels*/
202202
int hpn_disabled; /* disable hpn functionality. false by default */
203-
int hpn_buffer_size; /* set the hpn buffer size - default 3MB */
203+
int hpn_buffer_size; /* hpn buffer size - default 2MB */
204204
int none_enabled; /* Enable NONE cipher switch */
205205
int nonemac_enabled; /* Enable NONE MAC switch */
206206
int hpn_buffer_limit; /* limit local_window_max to 1/2 receive buffer */

ssh.c

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2157,7 +2157,7 @@ hpn_options_init(struct ssh *ssh)
21572157
if (tty_flag)
21582158
options.hpn_buffer_size = CHAN_SES_WINDOW_DEFAULT;
21592159
else
2160-
options.hpn_buffer_size = 2 * 1024 * 1024;
2160+
options.hpn_buffer_size = (2 * 1024 * 1024);
21612161

21622162
if (ssh->compat & SSH_BUG_LARGEWINDOW) {
21632163
debug("HPN to Non-HPN connection");
@@ -2177,30 +2177,6 @@ hpn_options_init(struct ssh *ssh)
21772177
debug("socksize %d", socksize);
21782178
options.hpn_buffer_size = socksize;
21792179
debug("HPNBufferSize set to TCP RWIN: %d", options.hpn_buffer_size);
2180-
} else {
2181-
if (options.tcp_rcv_buf > 0) {
2182-
/*
2183-
* Create a socket but don't connect it:
2184-
* we use that the get the rcv socket size
2185-
*/
2186-
sock = socket(AF_INET, SOCK_STREAM, 0);
2187-
/*
2188-
* If they are using the tcp_rcv_buf option,
2189-
* attempt to set the buffer size to that.
2190-
*/
2191-
if (options.tcp_rcv_buf) {
2192-
socksizelen = sizeof(options.tcp_rcv_buf);
2193-
setsockopt(sock, SOL_SOCKET, SO_RCVBUF,
2194-
&options.tcp_rcv_buf, socksizelen);
2195-
}
2196-
socksizelen = sizeof(socksize);
2197-
getsockopt(sock, SOL_SOCKET, SO_RCVBUF,
2198-
&socksize, &socksizelen);
2199-
close(sock);
2200-
debug("socksize %d", socksize);
2201-
options.hpn_buffer_size = socksize;
2202-
debug("HPNBufferSize set to user TCPRcvBuf: %d", options.hpn_buffer_size);
2203-
}
22042180
}
22052181
}
22062182

sshconnect.c

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -343,30 +343,6 @@ check_ifaddrs(const char *ifname, int af, const struct ifaddrs *ifaddrs,
343343
}
344344
#endif
345345

346-
/*
347-
* Set TCP receive buffer if requested.
348-
* Note: tuning needs to happen after the socket is
349-
* created but before the connection happens
350-
* so winscale is negotiated properly -cjr
351-
*/
352-
static void
353-
ssh_set_socket_recvbuf(int sock)
354-
{
355-
void *buf = (void *)&options.tcp_rcv_buf;
356-
int sz = sizeof(options.tcp_rcv_buf);
357-
int socksize;
358-
int socksizelen = sizeof(int);
359-
360-
debug("setsockopt Attempting to set SO_RCVBUF to %d", options.tcp_rcv_buf);
361-
if (setsockopt(sock, SOL_SOCKET, SO_RCVBUF, buf, sz) >= 0) {
362-
getsockopt(sock, SOL_SOCKET, SO_RCVBUF, &socksize, &socksizelen);
363-
debug("setsockopt SO_RCVBUF: %.100s %d", strerror(errno), socksize);
364-
}
365-
else
366-
error("Couldn't set socket receive buffer to %d: %.100s",
367-
options.tcp_rcv_buf, strerror(errno));
368-
}
369-
370346
/*
371347
* Creates a socket for use as the ssh connection.
372348
*/
@@ -389,9 +365,6 @@ ssh_create_socket(struct addrinfo *ai)
389365
}
390366
(void)fcntl(sock, F_SETFD, FD_CLOEXEC);
391367

392-
if (options.tcp_rcv_buf > 0)
393-
ssh_set_socket_recvbuf(sock);
394-
395368
/* Use interactive QOS (if specified) until authentication completed */
396369
if (options.ip_qos_interactive != INT_MAX)
397370
set_sock_tos(sock, options.ip_qos_interactive);

0 commit comments

Comments
 (0)