[PATCH] netns: correctly use per-netns ipv4 sysctl_tcp_mem

[PATCH] netns: correctly use per-netns ipv4 sysctl_tcp_mem

[Huang Qiang]

From: Yang Zhenzhang <yangzhenzhang@huawei.com>

Now, kernel allows each net namespace to independently set up its levels
for tcp memory pressure thresholds.

But it seems there is a bug, as using the following steps:

[root@host socket]# lxc-start -n test -f config /bin/bash
[root@net-test socket]# ip route add default via 192.168.58.2
[root@net-test socket]# echo 0 0 0 > /proc/sys/net/ipv4/tcp_mem
[root@net-test socket]# scp root@192.168.58.174:/home/tcp_mem_test .

and it still can transport the "tcp_mem_test" file which we hope it
would not.

It's because inet_init() (net/ipv4/af_inet.c)initialize the
tcp_prot.sysctl_mem:
tcp_prot.sysctl_mem = init_net.ipv4.sysctl_tcp_mem;

So when the protocal is TCP, sk->sk_prot->sysctl_mem(following code)
always use the ipv4 sysctl_tcp_mem of init_net namespace rather than
it's own net namespace.
This patch simply set "prot" equal to net->ipv4.sysctl_tcp_mem when
the protocol type is TCP.

Signed-off-by: Yang Zhenzhang <yangzhenzhang@huawei.com>
---
 include/net/sock.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 4a45216..b62a8d9 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -59,6 +59,7 @@
 #include <linux/static_key.h>
 #include <linux/aio.h>
 #include <linux/sched.h>
+#include <linux/in.h>

 #include <linux/filter.h>
 #include <linux/rculist_nulls.h>
@@ -1062,7 +1063,12 @@ static inline void sk_enter_memory_pressure(struct sock *sk)

 static inline long sk_prot_mem_limits(const struct sock *sk, int index)
 {
+	struct net *net = sock_net(sk);
 	long *prot = sk->sk_prot->sysctl_mem;
+	
+	if (sk->protocol == IPPROTO_TCP)
+		prot = net->ipv4.sysctl_tcp_mem;
+
 	if (mem_cgroup_sockets_enabled && sk->sk_cgrp)
 		prot = sk->sk_cgrp->sysctl_mem;
 	return prot[index];
-- 
1.7.1

Re: [PATCH] netns: correctly use per-netns ipv4 sysctl_tcp_mem

[David Miller]

From: Huang Qiang <h.huangqiang@huawei.com>
Date: Mon, 9 Jul 2012 14:05:09 +0800

> From: Yang Zhenzhang <yangzhenzhang@huawei.com>
> 
> Now, kernel allows each net namespace to independently set up its levels
> for tcp memory pressure thresholds.
> 
> But it seems there is a bug, as using the following steps:
> 
> [root@host socket]# lxc-start -n test -f config /bin/bash
> [root@net-test socket]# ip route add default via 192.168.58.2
> [root@net-test socket]# echo 0 0 0 > /proc/sys/net/ipv4/tcp_mem
> [root@net-test socket]# scp root@192.168.58.174:/home/tcp_mem_test .
> 
> and it still can transport the "tcp_mem_test" file which we hope it
> would not.
> 
> It's because inet_init() (net/ipv4/af_inet.c)initialize the
> tcp_prot.sysctl_mem:
> tcp_prot.sysctl_mem = init_net.ipv4.sysctl_tcp_mem;
> 
> So when the protocal is TCP, sk->sk_prot->sysctl_mem(following code)
> always use the ipv4 sysctl_tcp_mem of init_net namespace rather than
> it's own net namespace.
> This patch simply set "prot" equal to net->ipv4.sysctl_tcp_mem when
> the protocol type is TCP.
> 
> Signed-off-by: Yang Zhenzhang <yangzhenzhang@huawei.com>

Another regression added by the socket memory cgroup code, BIG
SURPRISE.

But this patch is even worse than the bug.

>  	long *prot = sk->sk_prot->sysctl_mem;
> +	
   ^^^^^^^^

Trailing whitespace please remove.

Also, this patch causes build failures:

In file included from include/linux/tcp.h:227:0,
                 from include/linux/ipv6.h:221,
                 from include/net/ipv6.h:16,
                 from include/linux/sunrpc/clnt.h:26,
                 from include/linux/nfs_fs.h:57,
                 from init/do_mounts.c:30:
include/net/sock.h: In function ‘sk_prot_mem_limits’:
include/net/sock.h:1066:9: error: implicit declaration of function ‘sock_net’ [-Werror=implicit-function-declaration]
include/net/sock.h:1066:20: warning: initialization makes pointer from integer without a cast [enabled by default]
include/net/sock.h:1069:8: error: ‘const struct sock’ has no member named ‘protocol’
include/net/sock.h: At top level:
include/net/sock.h:2136:13: error: conflicting types for ‘sock_net’
include/net/sock.h:1066:20: note: previous implicit declaration of ‘sock_net’ was here
cc1: some warnings being treated as errors

It is basically impossible that you even compile tested this patch
because you're using an interface from the same header file before
it's even defined.

This is an incredibly poor patch submission, I hate to tell you.

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

Re: [PATCH] netns: correctly use per-netns ipv4 sysctl_tcp_mem

[Glauber Costa]

On 07/09/2012 07:21 PM, David Miller wrote:
> From: Huang Qiang <h.huangqiang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Date: Mon, 9 Jul 2012 14:05:09 +0800
> 
>> From: Yang Zhenzhang <yangzhenzhang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>
>> Now, kernel allows each net namespace to independently set up its levels
>> for tcp memory pressure thresholds.
>>
>> But it seems there is a bug, as using the following steps:
>>
>> [root@host socket]# lxc-start -n test -f config /bin/bash
>> [root@net-test socket]# ip route add default via 192.168.58.2
>> [root@net-test socket]# echo 0 0 0 > /proc/sys/net/ipv4/tcp_mem
>> [root@net-test socket]# scp root-Q0ErXNX1RuabR28l3DCWlg@public.gmane.org:/home/tcp_mem_test .
>>
>> and it still can transport the "tcp_mem_test" file which we hope it
>> would not.
>>
>> It's because inet_init() (net/ipv4/af_inet.c)initialize the
>> tcp_prot.sysctl_mem:
>> tcp_prot.sysctl_mem = init_net.ipv4.sysctl_tcp_mem;
>>
>> So when the protocal is TCP, sk->sk_prot->sysctl_mem(following code)
>> always use the ipv4 sysctl_tcp_mem of init_net namespace rather than
>> it's own net namespace.
>> This patch simply set "prot" equal to net->ipv4.sysctl_tcp_mem when
>> the protocol type is TCP.
>>
>> Signed-off-by: Yang Zhenzhang <yangzhenzhang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> 
> Another regression added by the socket memory cgroup code, BIG
> SURPRISE.
> 

Back from vacations: If I understand the submission correctly, this is
not a regression, since it seems to be only happening when those values
are set inside the network namespace - which was not possible before.

In any case, I believe from what I can see that the fix is already in
the way (haven't seen the whole backlog yet)