[v3.3-rc1 regression] TCP: too many of orphaned sockets

There's a new TCP regression in latest -git that triggers in 
randconfig testing.

With the attached config i'm getting tons of these messages:

[   32.751209] TCP: too many of orphaned sockets                                                   
[   37.198307] TCP: too many of orphaned sockets                                                   
[   42.464404] TCP: too many of orphaned sockets                                                   
[   48.920392] TCP: too many of orphaned sockets                                                   
[   56.370026] TCP: too many of orphaned sockets                                                   
[   64.605937] TCP: too many of orphaned sockets                     

and it's not possible to ssh into the testbox, the TCP 
connection hangs.

Before i put more effort into debugging this, is this bug 
already known/fixed?

NOTE: the .config is randconfig generated so it might have a few 
weird combinations of config options. One such thing i noticed 
is !CONFIG_BUG, this produces at least one of the following 
build warnings in the networking code:

net/core/ethtool.c:211:1: warning: control reaches end of non-void function [-Wreturn-type]
net/core/dev.c:1892:33: warning: unused variable ‘null_features’ [-Wunused-variable]
net/packet/af_packet.c:1878:30: warning: ‘hdrlen’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:2456:17: warning: ‘xfrm_dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1351:7: warning: ‘dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1333:18: note: ‘dst_ops’ was declared here
net/xfrm/xfrm_policy.c:1351:7: warning: ‘dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1333:18: note: ‘dst_ops’ was declared here

Note#2, the message should probably be fixed as well:

 -   TCP: too many of orphaned sockets                     
 +   TCP: too many orphaned sockets                     

Thanks,

	Ingo

On 01/27/2012 04:46 PM, Ingo Molnar wrote:

There's a new TCP regression in latest -git that triggers in
randconfig testing.

With the attached config i'm getting tons of these messages:

[   32.751209] TCP: too many of orphaned sockets
[   37.198307] TCP: too many of orphaned sockets
[   42.464404] TCP: too many of orphaned sockets
[   48.920392] TCP: too many of orphaned sockets
[   56.370026] TCP: too many of orphaned sockets
[   64.605937] TCP: too many of orphaned sockets

and it's not possible to ssh into the testbox, the TCP
connection hangs.

Before i put more effort into debugging this, is this bug
already known/fixed?

NOTE: the .config is randconfig generated so it might have a few
weird combinations of config options. One such thing i noticed
is !CONFIG_BUG, this produces at least one of the following
build warnings in the networking code:

net/core/ethtool.c:211:1: warning: control reaches end of non-void function [-Wreturn-type]
net/core/dev.c:1892:33: warning: unused variable ‘null_features’ [-Wunused-variable]
net/packet/af_packet.c:1878:30: warning: ‘hdrlen’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:2456:17: warning: ‘xfrm_dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1351:7: warning: ‘dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1333:18: note: ‘dst_ops’ was declared here
net/xfrm/xfrm_policy.c:1351:7: warning: ‘dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1333:18: note: ‘dst_ops’ was declared here

Note#2, the message should probably be fixed as well:

  -   TCP: too many of orphaned sockets
  +   TCP: too many orphaned sockets

Thanks,

	Ingo

Ingo,
mind sharing your config?

I haven't seen this, but I intend to run this against my recent 
patchseries and at least see if this is the culprit.

Thanks.

* Glauber Costa [off-list ref] wrote:

On 01/27/2012 04:46 PM, Ingo Molnar wrote:

quoted

There's a new TCP regression in latest -git that triggers in
randconfig testing.

With the attached config i'm getting tons of these messages:

[   32.751209] TCP: too many of orphaned sockets
[   37.198307] TCP: too many of orphaned sockets
[   42.464404] TCP: too many of orphaned sockets
[   48.920392] TCP: too many of orphaned sockets
[   56.370026] TCP: too many of orphaned sockets
[   64.605937] TCP: too many of orphaned sockets

and it's not possible to ssh into the testbox, the TCP
connection hangs.

Before i put more effort into debugging this, is this bug
already known/fixed?

NOTE: the .config is randconfig generated so it might have a few
weird combinations of config options. One such thing i noticed
is !CONFIG_BUG, this produces at least one of the following
build warnings in the networking code:

net/core/ethtool.c:211:1: warning: control reaches end of non-void function [-Wreturn-type]
net/core/dev.c:1892:33: warning: unused variable ‘null_features’ [-Wunused-variable]
net/packet/af_packet.c:1878:30: warning: ‘hdrlen’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:2456:17: warning: ‘xfrm_dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1351:7: warning: ‘dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1333:18: note: ‘dst_ops’ was declared here
net/xfrm/xfrm_policy.c:1351:7: warning: ‘dst_ops’ may be used uninitialized in this function [-Wmaybe-uninitialized]
net/xfrm/xfrm_policy.c:1333:18: note: ‘dst_ops’ was declared here

Note#2, the message should probably be fixed as well:

 -   TCP: too many of orphaned sockets
 +   TCP: too many orphaned sockets

Thanks,

Ingo

Ingo,
mind sharing your config?

attached.

Thanks,

	Ingo

ok, i've bisected it, and the bad commit is:

3dc43e3e4d0b52197d3205214fe8f162f9e0c334 is the first bad commit
commit 3dc43e3e4d0b52197d3205214fe8f162f9e0c334
Author: Glauber Costa [off-list ref]
Date:   Sun Dec 11 21:47:05 2011 +0000

    per-netns ipv4 sysctl_tcp_mem
    
    This patch allows each namespace to independently set up
    its levels for tcp memory pressure thresholds. This patch
    alone does not buy much: we need to make this values
    per group of process somehow. This is achieved in the
    patches that follows in this patchset.
    
    Signed-off-by: Glauber Costa [off-list ref]
    Reviewed-by: KAMEZAWA Hiroyuki [off-list ref]
    CC: David S. Miller [off-list ref]
    CC: Eric W. Biederman [off-list ref]
    Signed-off-by: David S. Miller [off-list ref]

Thanks,

	Ingo

* Ingo Molnar [off-list ref] wrote:

ok, i've bisected it, and the bad commit is:

3dc43e3e4d0b52197d3205214fe8f162f9e0c334 is the first bad commit
commit 3dc43e3e4d0b52197d3205214fe8f162f9e0c334
Author: Glauber Costa [off-list ref]
Date:   Sun Dec 11 21:47:05 2011 +0000

    per-netns ipv4 sysctl_tcp_mem

Might be related to this detail in the .config:

 # CONFIG_PROC_SYSCTL is not set

So former tcp_init() code does not get run?

Thanks,

	Ingo

On 01/27/2012 06:22 PM, Ingo Molnar wrote:

* Ingo Molnar[off-list ref]  wrote:

quoted

ok, i've bisected it, and the bad commit is:

3dc43e3e4d0b52197d3205214fe8f162f9e0c334 is the first bad commit
commit 3dc43e3e4d0b52197d3205214fe8f162f9e0c334
Author: Glauber Costa[off-list ref]
Date:   Sun Dec 11 21:47:05 2011 +0000

     per-netns ipv4 sysctl_tcp_mem

Might be related to this detail in the .config:

  # CONFIG_PROC_SYSCTL is not set

So former tcp_init() code does not get run?

Thanks,

	Ingo

Yeah, the problem is that the sysctl variables are used internally
even though CONFIG_PROC_SYSCTL is not set. This patch makes the
initialization happen in the per-netns sysctl initializer, so it will 
have bogus values with your config.

Let me send you a test patch shortly.

On 01/27/2012 06:22 PM, Ingo Molnar wrote:

* Ingo Molnar[off-list ref]  wrote:

quoted

ok, i've bisected it, and the bad commit is:

3dc43e3e4d0b52197d3205214fe8f162f9e0c334 is the first bad commit
commit 3dc43e3e4d0b52197d3205214fe8f162f9e0c334
Author: Glauber Costa[off-list ref]
Date:   Sun Dec 11 21:47:05 2011 +0000

     per-netns ipv4 sysctl_tcp_mem

Might be related to this detail in the .config:

  # CONFIG_PROC_SYSCTL is not set

So former tcp_init() code does not get run?

Thanks,

	Ingo

Can you tell me if the following patch fixes your problem?

On 01/27/2012 06:35 PM, Glauber Costa wrote:

On 01/27/2012 06:22 PM, Ingo Molnar wrote:

quoted

* Ingo Molnar[off-list ref] wrote:

quoted

ok, i've bisected it, and the bad commit is:

3dc43e3e4d0b52197d3205214fe8f162f9e0c334 is the first bad commit
commit 3dc43e3e4d0b52197d3205214fe8f162f9e0c334
Author: Glauber Costa[off-list ref]
Date: Sun Dec 11 21:47:05 2011 +0000

per-netns ipv4 sysctl_tcp_mem

Might be related to this detail in the .config:

# CONFIG_PROC_SYSCTL is not set

So former tcp_init() code does not get run?

Thanks,

Ingo

Can you tell me if the following patch fixes your problem?

Update on this:

What really makes it break is CONFIG_SYSCTL.
CONFIG_PROC_SYSCTL selects that, so if you get the one, you
end up getting the other. (The config mingo provided lacks both)

Also, I believe there is no harm in initializing this unconditionally,
so instead of cluttering tcp_init() with #ifdef, I am proposing we just 
init it here, and then init it again in sysctl initialization. I don't
expect it to harm workload, since it is a one-shot.

Now, I am attaching my proposed final patch for this, but I can't really
generate a config without sysctl that boots okay for me.

Ingo, would you please confirm that this fixes the problem for you? If 
I'm mistaken, let me know and I'll get back to it ASAP.

Dave, once Ingo acks that it fixes the problem he says, I'll submit the 
patch formally.

Thanks.

From: Glauber Costa <redacted>
Date: Fri, 27 Jan 2012 20:27:06 +0400

+extern void init_tcp_mem(struct net *net);

Please name this "tcp_init_mem" or similar, keeping all TCP functions
globally exported with a "tcp_*" prefix.

On 01/28/2012 01:28 AM, David Miller wrote:

From: Glauber Costa<redacted>
Date: Fri, 27 Jan 2012 20:27:06 +0400

quoted

+extern void init_tcp_mem(struct net *net);

Please name this "tcp_init_mem" or similar, keeping all TCP functions
globally exported with a "tcp_*" prefix.

Ok, will do.

ok, the patch works fine - in the tested version i've renamed 
the function and cleaned up the changelog a bit, see the final 
patch below.

Thanks,

	Ingo

-------------------->

From c5b258533cb5f6f216522163a9aa7ff029dc390f Mon Sep 17 00:00:00 2001

From: Glauber Costa <redacted>
Date: Fri, 27 Jan 2012 20:27:06 +0400
Subject: [PATCH] net/tcp: Fix tcp memory limits initialization when !CONFIG_SYSCTL

sysctl_tcp_mem() initialization was moved to sysctl_tcp_ipv4.c 
in commit 3dc43e3e4d0b52197d3205214fe8f162f9e0c334, since it 
became a per-ns value.

That code, however, will never run when CONFIG_SYSCTL is 
disabled, leading to bogus values on those fields - causing hung 
TCP sockets.

This patch fixes it by keeping an initialization code in 
tcp_init(). It will be overwritten by the first net namespace 
init if CONFIG_SYSCTL is compiled in, and do the right thing if 
it is compiled out.

It is also named properly as tcp_init_mem(), to properly signal 
its non-sysctl side effect on TCP limits.

Reported-by: Ingo Molnar <redacted>
Signed-off-by: Glauber Costa <redacted>
Cc: David S. Miller <davem@davemloft.net>
Link: http://lkml.kernel.org/r/4F22D05A.8030604@parallels.com
[ renamed the function, tidied up the changelog a bit ]
Signed-off-by: Ingo Molnar <redacted>
---
 include/net/tcp.h          |    2 ++
 net/ipv4/sysctl_net_ipv4.c |    1 +
 net/ipv4/tcp.c             |   16 +++++++++++++---
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0118ea9..d49db01 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h

@@ -311,6 +311,8 @@ extern struct proto tcp_prot;
 #define TCP_ADD_STATS_USER(net, field, val) SNMP_ADD_STATS_USER((net)->mib.tcp_statistics, field, val)
 #define TCP_ADD_STATS(net, field, val)	SNMP_ADD_STATS((net)->mib.tcp_statistics, field, val)
 
+extern void tcp_init_mem(struct net *net);
+
 extern void tcp_v4_err(struct sk_buff *skb, u32);
 
 extern void tcp_shutdown (struct sock *sk, int how);

diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 4aa7e9d..4cb9cd2 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c

@@ -814,6 +814,7 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
 
 	net->ipv4.sysctl_rt_cache_rebuild_count = 4;
 
+	tcp_init_mem(net);
 	limit = nr_free_buffer_pages() / 8;
 	limit = max(limit, 128UL);
 	net->ipv4.sysctl_tcp_mem[0] = limit / 4 * 3;

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9bcdec3..06373b4 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c

@@ -3216,6 +3216,16 @@ static int __init set_thash_entries(char *str)
 }
 __setup("thash_entries=", set_thash_entries);
 
+void tcp_init_mem(struct net *net)
+{
+	/* Set per-socket limits to no more than 1/128 the pressure threshold */
+	unsigned long limit = nr_free_buffer_pages() / 8;
+	limit = max(limit, 128UL);
+	net->ipv4.sysctl_tcp_mem[0] = limit / 4 * 3;
+	net->ipv4.sysctl_tcp_mem[1] = limit;
+	net->ipv4.sysctl_tcp_mem[2] = net->ipv4.sysctl_tcp_mem[0] * 2;
+}
+
 void __init tcp_init(void)
 {
 	struct sk_buff *skb = NULL;

@@ -3276,9 +3286,9 @@ void __init tcp_init(void)
 	sysctl_tcp_max_orphans = cnt / 2;
 	sysctl_max_syn_backlog = max(128, cnt / 256);
 
-	/* Set per-socket limits to no more than 1/128 the pressure threshold */
-	limit = ((unsigned long)init_net.ipv4.sysctl_tcp_mem[1])
-		<< (PAGE_SHIFT - 7);
+	tcp_init_mem(&init_net);
+	limit = nr_free_buffer_pages() / 8;
+	limit = max(limit, 128UL);
 	max_share = min(4UL*1024*1024, limit);
 
 	sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;

On 01/28/2012 03:50 PM, Ingo Molnar wrote:

ok, the patch works fine - in the tested version i've renamed
the function and cleaned up the changelog a bit, see the final
patch below.

Thanks,

	Ingo

Thank you Ingo.

will submit this patch now, then.