All of lore.kernel.org
 help / color / mirror / Atom feed
From: Calvin Owens <calvinowens@fb.com>
To: David Miller <davem@davemloft.net>
Cc: <kuznet@ms2.inr.ac.ru>, <jmorris@namei.org>,
	<yoshfuji@linux-ipv6.org>, <kaber@trash.net>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<kernel-team@fb.com>, <sorin@returnze.ro>
Subject: [PATCH] net: Use SK_MEM_QUANTUM as minimum for tcp/udp rmem/wmem
Date: Tue, 11 Aug 2015 21:54:20 -0700	[thread overview]
Message-ID: <20150812045420.GA3908557@mail.thefacebook.com> (raw)
In-Reply-To: <20150810.204630.1903301700926701432.davem@davemloft.net>

Commit 8133534c760d4083 ("net: limit tcp/udp rmem/wmem to
SOCK_{RCV,SND}BUF_MIN") modified four sysctls to enforce that the values
written to them are not less than SOCK_MIN_{RCV,SND}BUF.

That change causes 4096 (or SK_MEM_QUANTUM) to no longer be accepted as
a valid value for 'min' in tcp_wmem and udp_wmem_min. 4096 has been the
default for both of those sysctls for a long time, and unfortunately
seems to be an extremely popular setting. This change breaks a large
number of sysctl configurations at FB.

That commit referred to b1cb59cf2efe7971 ("net: sysctl_net_core: check
SNDBUF and RCVBUF for min length"), which choose to use the SOCK_MIN
constants as the lower limits to avoid nasty bugs. But AFAICS, a limit
of SOCK_MIN_SNDBUF isn't necessary to do that: the BUG_ON cited in the
commit message seems to have happened because unix_stream_sendmsg()
expects a minimum of a full page (ie SK_MEM_QUANTUM) and the math broke,
not because it had less than SOCK_MIN_SNDBUF allocated.

Nothing seems to assume that it has at least SOCK_MIN_SNDBUF to play
with, so I think enforcing a minimum of SK_MEM_QUANTUM avoids the sort
of bugs 8133534c was trying to avoid, and it does so without breaking
anybody's sysctl configurations.

Fixes: 8133534c760d4083 ("net: limit tcp/udp rmem/wmem to SOCK_MIN...")
Signed-off-by: Calvin Owens <calvinowens@fb.com>
---
 net/ipv4/sysctl_net_ipv4.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 433231c..a214b6a 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -41,8 +41,7 @@ static int tcp_syn_retries_min = 1;
 static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
 static int ip_ping_group_range_min[] = { 0, 0 };
 static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
-static int min_sndbuf = SOCK_MIN_SNDBUF;
-static int min_rcvbuf = SOCK_MIN_RCVBUF;
+static int min_buf = SK_MEM_QUANTUM;
 
 /* Update system visible IP port range */
 static void set_local_port_range(struct net *net, int range[2])
@@ -530,7 +529,7 @@ static struct ctl_table ipv4_table[] = {
 		.maxlen		= sizeof(sysctl_tcp_wmem),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_sndbuf,
+		.extra1		= &min_buf,
 	},
 	{
 		.procname	= "tcp_notsent_lowat",
@@ -545,7 +544,7 @@ static struct ctl_table ipv4_table[] = {
 		.maxlen		= sizeof(sysctl_tcp_rmem),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_rcvbuf,
+		.extra1		= &min_buf,
 	},
 	{
 		.procname	= "tcp_app_win",
@@ -758,7 +757,7 @@ static struct ctl_table ipv4_table[] = {
 		.maxlen		= sizeof(sysctl_udp_rmem_min),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_rcvbuf,
+		.extra1		= &min_buf,
 	},
 	{
 		.procname	= "udp_wmem_min",
@@ -766,7 +765,7 @@ static struct ctl_table ipv4_table[] = {
 		.maxlen		= sizeof(sysctl_udp_wmem_min),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_sndbuf,
+		.extra1		= &min_buf,
 	},
 	{ }
 };
-- 
1.8.1


  reply	other threads:[~2015-08-12  4:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-05 20:26 [PATCH] net: Unbreak resetting default values for tcp_wmem/udp_wmem_min Calvin Owens
2015-08-10  5:41 ` David Miller
2015-08-11  3:34   ` Calvin Owens
2015-08-11  3:46     ` David Miller
2015-08-12  4:54       ` Calvin Owens [this message]
2015-08-12 14:21         ` [PATCH] net: Use SK_MEM_QUANTUM as minimum for tcp/udp rmem/wmem Eric Dumazet
2015-08-12 17:00           ` Sorin Dumitru
2015-08-12 17:46             ` Eric Dumazet
2015-08-13 21:07               ` Calvin Owens
2015-08-13 21:21         ` [PATCH] Revert "net: limit tcp/udp rmem/wmem to SOCK_{RCV,SND}BUF_MIN" Calvin Owens
2015-08-13 22:56           ` Eric Dumazet
2015-08-17 19:11           ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150812045420.GA3908557@mail.thefacebook.com \
    --to=calvinowens@fb.com \
    --cc=davem@davemloft.net \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=kernel-team@fb.com \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sorin@returnze.ro \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.