Patchwork IPv4/IPv6: update sysctl files

login
register
mail settings
Submitter Shen Feng
Date April 8, 2009, 2:17 a.m.
Message ID <49DC091C.6070708@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/25699/
State Rejected
Delegated to: David Miller
Headers show

Comments

Shen Feng - April 8, 2009, 2:17 a.m.
Now the following sysctl files in /proc/sys/net/ipv4 are used by
both IPv4 and IPv6.
tcp_mem  tcp_rmem  tcp_wmem
udp_mem  udp_rmem_min  udp_wmem_min
Putting them in /proc/sys/net/ipv4 is not a good choice.

So move tcp_mem  tcp_rmem  tcp_wmem to /proc/sys/net/tcp and
move udp_mem  udp_rmem_min  udp_wmem_min to /poc/sys/net/udp.

Signed-off-by: Shen Feng <shen@cn.fujitsu.com>
---
 net/ipv4/sysctl_net_ipv4.c |   95 ++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 29 deletions(-)
Eric W. Biederman - April 8, 2009, 2:41 a.m.
Shen Feng <shen@cn.fujitsu.com> writes:

> Now the following sysctl files in /proc/sys/net/ipv4 are used by
> both IPv4 and IPv6.
> tcp_mem  tcp_rmem  tcp_wmem
> udp_mem  udp_rmem_min  udp_wmem_min
> Putting them in /proc/sys/net/ipv4 is not a good choice.
>
> So move tcp_mem  tcp_rmem  tcp_wmem to /proc/sys/net/tcp and
> move udp_mem  udp_rmem_min  udp_wmem_min to /poc/sys/net/udp.

How badly does this blow up when you enable sysctl_check?
You just enabled new binary sysctl mappings, which is big no-no.

Further how many user space scripts did you break that touch those
files?  This is an ABI change.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - April 8, 2009, 2:47 a.m.
On Wed, 2009-04-08 at 02:39 +0000, Shen Feng wrote:
> Now the following sysctl files in /proc/sys/net/ipv4 are used by
> both IPv4 and IPv6.
> tcp_mem  tcp_rmem  tcp_wmem
> udp_mem  udp_rmem_min  udp_wmem_min
> Putting them in /proc/sys/net/ipv4 is not a good choice.
[...]

But this is part of the ABI to userland.  You cannot remove sysctl files
without long advance notice documented in feature-removal-schedule.txt
(if at all).

If it is possible to add the paths
/proc/sys/net/{tcp,udp} while retaining aliases under /proc/sys/net/ipv4
then that might be a workable solution.

Ben.
Shen Feng - April 8, 2009, 3:28 a.m.
on 04/08/2009 10:47 AM, Ben Hutchings wrote:
> On Wed, 2009-04-08 at 02:39 +0000, Shen Feng wrote:
>> Now the following sysctl files in /proc/sys/net/ipv4 are used by
>> both IPv4 and IPv6.
>> tcp_mem  tcp_rmem  tcp_wmem
>> udp_mem  udp_rmem_min  udp_wmem_min
>> Putting them in /proc/sys/net/ipv4 is not a good choice.
> [...]
> 
> But this is part of the ABI to userland.  You cannot remove sysctl files
> without long advance notice documented in feature-removal-schedule.txt
> (if at all).
> 
> If it is possible to add the paths
> /proc/sys/net/{tcp,udp} while retaining aliases under /proc/sys/net/ipv4
> then that might be a workable solution.

Thanks. That's a good solution.

But I'm still confused.
Why not create another tcp_mem in /proc/sys/net/ipv6?

Shen

> 
> Ben.
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 8, 2009, 7:18 a.m.
From: Shen Feng <shen@cn.fujitsu.com>
Date: Wed, 08 Apr 2009 11:28:21 +0800

> 
> 
> on 04/08/2009 10:47 AM, Ben Hutchings wrote:
>> On Wed, 2009-04-08 at 02:39 +0000, Shen Feng wrote:
>>> Now the following sysctl files in /proc/sys/net/ipv4 are used by
>>> both IPv4 and IPv6.
>>> tcp_mem  tcp_rmem  tcp_wmem
>>> udp_mem  udp_rmem_min  udp_wmem_min
>>> Putting them in /proc/sys/net/ipv4 is not a good choice.
>> [...]
>> 
>> But this is part of the ABI to userland.  You cannot remove sysctl files
>> without long advance notice documented in feature-removal-schedule.txt
>> (if at all).
>> 
>> If it is possible to add the paths
>> /proc/sys/net/{tcp,udp} while retaining aliases under /proc/sys/net/ipv4
>> then that might be a workable solution.
> 
> Thanks. That's a good solution.
> 
> But I'm still confused.
> Why not create another tcp_mem in /proc/sys/net/ipv6?

People just need to understand that ipv4 is always going to be
there and that's where all the tcp controls are.

I really am not going to entertain changes that try to move generic
inet sysctl things out of the ipv4 directory.  There is really no
point at all.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shen Feng - April 8, 2009, 8:50 a.m.
on 04/08/2009 03:18 PM, David Miller wrote:
> From: Shen Feng <shen@cn.fujitsu.com>
> Date: Wed, 08 Apr 2009 11:28:21 +0800
> 
>>
>> on 04/08/2009 10:47 AM, Ben Hutchings wrote:
>>> On Wed, 2009-04-08 at 02:39 +0000, Shen Feng wrote:
>>>> Now the following sysctl files in /proc/sys/net/ipv4 are used by
>>>> both IPv4 and IPv6.
>>>> tcp_mem  tcp_rmem  tcp_wmem
>>>> udp_mem  udp_rmem_min  udp_wmem_min
>>>> Putting them in /proc/sys/net/ipv4 is not a good choice.
>>> [...]
>>>
>>> But this is part of the ABI to userland.  You cannot remove sysctl files
>>> without long advance notice documented in feature-removal-schedule.txt
>>> (if at all).
>>>
>>> If it is possible to add the paths
>>> /proc/sys/net/{tcp,udp} while retaining aliases under /proc/sys/net/ipv4
>>> then that might be a workable solution.
>> Thanks. That's a good solution.
>>
>> But I'm still confused.
>> Why not create another tcp_mem in /proc/sys/net/ipv6?
> 
> People just need to understand that ipv4 is always going to be
> there and that's where all the tcp controls are.
> 
> I really am not going to entertain changes that try to move generic
> inet sysctl things out of the ipv4 directory.  There is really no
> point at all.
> 

/proc/sys/net/ipv4/tcp_mem is a inet sysctl, but it also controls the tcp v6.
So it's also a inet6 sysctl. Is it intentional?
This may confuse users. We may have a /proc/sys/net/ipv6/tcp6_mem.

> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 8, 2009, 9:09 a.m.
From: Shen Feng <shen@cn.fujitsu.com>
Date: Wed, 08 Apr 2009 16:50:49 +0800

> /proc/sys/net/ipv4/tcp_mem is a inet sysctl, but it also controls the tcp v6.
> So it's also a inet6 sysctl. Is it intentional?

Yes, please just drop this.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 4710d21..a520011 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -503,30 +503,6 @@  static struct ctl_table ipv4_table[] = {
 		.proc_handler	= proc_dointvec
 	},
 	{
-		.ctl_name	= NET_TCP_MEM,
-		.procname	= "tcp_mem",
-		.data		= &sysctl_tcp_mem,
-		.maxlen		= sizeof(sysctl_tcp_mem),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec
-	},
-	{
-		.ctl_name	= NET_TCP_WMEM,
-		.procname	= "tcp_wmem",
-		.data		= &sysctl_tcp_wmem,
-		.maxlen		= sizeof(sysctl_tcp_wmem),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec
-	},
-	{
-		.ctl_name	= NET_TCP_RMEM,
-		.procname	= "tcp_rmem",
-		.data		= &sysctl_tcp_rmem,
-		.maxlen		= sizeof(sysctl_tcp_rmem),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec
-	},
-	{
 		.ctl_name	= NET_TCP_APP_WIN,
 		.procname	= "tcp_app_win",
 		.data		= &sysctl_tcp_app_win,
@@ -712,6 +688,38 @@  static struct ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{ .ctl_name = 0 }
+};
+
+static struct ctl_table proc_tcp_table[] = {
+	{
+		.ctl_name	= NET_TCP_MEM,
+		.procname	= "tcp_mem",
+		.data		= &sysctl_tcp_mem,
+		.maxlen		= sizeof(sysctl_tcp_mem),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+	{
+		.ctl_name	= NET_TCP_WMEM,
+		.procname	= "tcp_wmem",
+		.data		= &sysctl_tcp_wmem,
+		.maxlen		= sizeof(sysctl_tcp_wmem),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+	{
+		.ctl_name	= NET_TCP_RMEM,
+		.procname	= "tcp_rmem",
+		.data		= &sysctl_tcp_rmem,
+		.maxlen		= sizeof(sysctl_tcp_rmem),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+	{},
+};
+
+static struct ctl_table proc_udp_table[] = {
 	{
 		.ctl_name	= CTL_UNNUMBERED,
 		.procname	= "udp_mem",
@@ -742,7 +750,7 @@  static struct ctl_table ipv4_table[] = {
 		.strategy	= sysctl_intvec,
 		.extra1		= &zero
 	},
-	{ .ctl_name = 0 }
+	{},
 };
 
 static struct ctl_table ipv4_net_table[] = {
@@ -813,6 +821,20 @@  struct ctl_path net_ipv4_ctl_path[] = {
 };
 EXPORT_SYMBOL_GPL(net_ipv4_ctl_path);
 
+struct ctl_path net_tcp_ctl_path[] = {
+	{ .procname = "net", .ctl_name = CTL_NET, },
+	{ .procname = "tcp", .ctl_name = CTL_UNNUMBERED, },
+	{},
+};
+EXPORT_SYMBOL_GPL(net_tcp_ctl_path);
+
+struct ctl_path net_udp_ctl_path[] = {
+	{ .procname = "net", .ctl_name = CTL_NET, },
+	{ .procname = "udp", .ctl_name = CTL_UNNUMBERED, },
+	{},
+};
+EXPORT_SYMBOL_GPL(net_udp_ctl_path);
+
 static __net_init int ipv4_sysctl_init_net(struct net *net)
 {
 	struct ctl_table *table;
@@ -871,14 +893,29 @@  static __net_initdata struct pernet_operations ipv4_sysctl_ops = {
 
 static __init int sysctl_ipv4_init(void)
 {
-	struct ctl_table_header *hdr;
+	struct ctl_table_header *hdr_ipv4, *hdr_tcp, *hdr_udp;
+
+	hdr_ipv4 = register_sysctl_paths(net_ipv4_ctl_path, ipv4_table);
+	if (hdr_ipv4 == NULL)
+		return -ENOMEM;
 
-	hdr = register_sysctl_paths(net_ipv4_ctl_path, ipv4_table);
-	if (hdr == NULL)
+	hdr_tcp = register_sysctl_paths(net_tcp_ctl_path, proc_tcp_table);
+	if (hdr_tcp == NULL) {
+		unregister_sysctl_table(hdr_ipv4);
 		return -ENOMEM;
+	}
+
+	hdr_udp = register_sysctl_paths(net_udp_ctl_path, proc_udp_table);
+	if (hdr_udp == NULL) {
+		unregister_sysctl_table(hdr_ipv4);
+		unregister_sysctl_table(hdr_tcp);
+		return -ENOMEM;
+	}
 
 	if (register_pernet_subsys(&ipv4_sysctl_ops)) {
-		unregister_sysctl_table(hdr);
+		unregister_sysctl_table(hdr_ipv4);
+		unregister_sysctl_table(hdr_tcp);
+		unregister_sysctl_table(hdr_udp);
 		return -ENOMEM;
 	}