[net-next-2.6 PATCH 1/2] net ipv4: Decouple ipv4 interface parameters from binary sysctl numbers

[net-next-2.6 PATCH 1/2] net ipv4: Decouple ipv4 interface parameters from binary sysctl numbers

[Eric W. Biederman]

Stop using the binary sysctl enumeartion in sysctl.h as an index into
a per interface array.  This leads to unnecessary binary sysctl number
allocation, and a fragility in data structure and implementation
because of unnecessary coupling.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 include/linux/inetdevice.h |   42 ++++++++++++++++++++++++++++++++++++------
 include/linux/sysctl.h     |    4 ----
 net/ipv4/devinet.c         |   26 +++++++++++++-------------
 3 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index cf25780..2be1a1a 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -10,10 +10,40 @@
 #include <linux/timer.h>
 #include <linux/sysctl.h>
 
+enum
+{
+	IPV4_DEVCONF_FORWARDING=1,
+	IPV4_DEVCONF_MC_FORWARDING,
+	IPV4_DEVCONF_PROXY_ARP,
+	IPV4_DEVCONF_ACCEPT_REDIRECTS,
+	IPV4_DEVCONF_SECURE_REDIRECTS,
+	IPV4_DEVCONF_SEND_REDIRECTS,
+	IPV4_DEVCONF_SHARED_MEDIA,
+	IPV4_DEVCONF_RP_FILTER,
+	IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE,
+	IPV4_DEVCONF_BOOTP_RELAY,
+	IPV4_DEVCONF_LOG_MARTIANS,
+	IPV4_DEVCONF_TAG,
+	IPV4_DEVCONF_ARPFILTER,
+	IPV4_DEVCONF_MEDIUM_ID,
+	IPV4_DEVCONF_NOXFRM,
+	IPV4_DEVCONF_NOPOLICY,
+	IPV4_DEVCONF_FORCE_IGMP_VERSION,
+	IPV4_DEVCONF_ARP_ANNOUNCE,
+	IPV4_DEVCONF_ARP_IGNORE,
+	IPV4_DEVCONF_PROMOTE_SECONDARIES,
+	IPV4_DEVCONF_ARP_ACCEPT,
+	IPV4_DEVCONF_ARP_NOTIFY,
+	IPV4_DEVCONF_ACCEPT_LOCAL,
+	IPV4_DEVCONF_SRC_VMARK,
+	IPV4_DEVCONF_PROXY_ARP_PVLAN,
+	__IPV4_DEVCONF_MAX
+};
+
 struct ipv4_devconf {
 	void	*sysctl;
-	int	data[__NET_IPV4_CONF_MAX - 1];
-	DECLARE_BITMAP(state, __NET_IPV4_CONF_MAX - 1);
+	int	data[__IPV4_DEVCONF_MAX - 1];
+	DECLARE_BITMAP(state, __IPV4_DEVCONF_MAX - 1);
 };
 
 struct in_device {
@@ -40,7 +70,7 @@ struct in_device {
 	struct rcu_head		rcu_head;
 };
 
-#define IPV4_DEVCONF(cnf, attr) ((cnf).data[NET_IPV4_CONF_ ## attr - 1])
+#define IPV4_DEVCONF(cnf, attr) ((cnf).data[IPV4_DEVCONF_ ## attr - 1])
 #define IPV4_DEVCONF_ALL(net, attr) \
 	IPV4_DEVCONF((*(net)->ipv4.devconf_all), attr)
 
@@ -60,13 +90,13 @@ static inline void ipv4_devconf_set(struct in_device *in_dev, int index,
 
 static inline void ipv4_devconf_setall(struct in_device *in_dev)
 {
-	bitmap_fill(in_dev->cnf.state, __NET_IPV4_CONF_MAX - 1);
+	bitmap_fill(in_dev->cnf.state, __IPV4_DEVCONF_MAX - 1);
 }
 
 #define IN_DEV_CONF_GET(in_dev, attr) \
-	ipv4_devconf_get((in_dev), NET_IPV4_CONF_ ## attr)
+	ipv4_devconf_get((in_dev), IPV4_DEVCONF_ ## attr)
 #define IN_DEV_CONF_SET(in_dev, attr, val) \
-	ipv4_devconf_set((in_dev), NET_IPV4_CONF_ ## attr, (val))
+	ipv4_devconf_set((in_dev), IPV4_DEVCONF_ ## attr, (val))
 
 #define IN_DEV_ANDCONF(in_dev, attr) \
 	(IPV4_DEVCONF_ALL(dev_net(in_dev->dev), attr) && \
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 9f236cd..7c74e91 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -481,10 +481,6 @@ enum
 	NET_IPV4_CONF_PROMOTE_SECONDARIES=20,
 	NET_IPV4_CONF_ARP_ACCEPT=21,
 	NET_IPV4_CONF_ARP_NOTIFY=22,
-	NET_IPV4_CONF_ACCEPT_LOCAL=23,
-	NET_IPV4_CONF_SRC_VMARK=24,
-	NET_IPV4_CONF_PROXY_ARP_PVLAN=25,
-	__NET_IPV4_CONF_MAX
 };
 
 /* /proc/sys/net/ipv4/netfilter */
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index cd71a39..b1eddee 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -64,20 +64,20 @@
 
 static struct ipv4_devconf ipv4_devconf = {
 	.data = {
-		[NET_IPV4_CONF_ACCEPT_REDIRECTS - 1] = 1,
-		[NET_IPV4_CONF_SEND_REDIRECTS - 1] = 1,
-		[NET_IPV4_CONF_SECURE_REDIRECTS - 1] = 1,
-		[NET_IPV4_CONF_SHARED_MEDIA - 1] = 1,
+		[IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
+		[IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
+		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
+		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
 	},
 };
 
 static struct ipv4_devconf ipv4_devconf_dflt = {
 	.data = {
-		[NET_IPV4_CONF_ACCEPT_REDIRECTS - 1] = 1,
-		[NET_IPV4_CONF_SEND_REDIRECTS - 1] = 1,
-		[NET_IPV4_CONF_SECURE_REDIRECTS - 1] = 1,
-		[NET_IPV4_CONF_SHARED_MEDIA - 1] = 1,
-		[NET_IPV4_CONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
+		[IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
+		[IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
+		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
+		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
+		[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
 	},
 };
 
@@ -1360,7 +1360,7 @@ int ipv4_doint_and_flush(ctl_table *ctl, int write,
 	{ \
 		.procname	= name, \
 		.data		= ipv4_devconf.data + \
-				  NET_IPV4_CONF_ ## attr - 1, \
+				  IPV4_DEVCONF_ ## attr - 1, \
 		.maxlen		= sizeof(int), \
 		.mode		= mval, \
 		.proc_handler	= proc, \
@@ -1381,7 +1381,7 @@ int ipv4_doint_and_flush(ctl_table *ctl, int write,
 
 static struct devinet_sysctl_table {
 	struct ctl_table_header *sysctl_header;
-	struct ctl_table devinet_vars[__NET_IPV4_CONF_MAX];
+	struct ctl_table devinet_vars[__IPV4_DEVCONF_MAX];
 	char *dev_name;
 } devinet_sysctl = {
 	.devinet_vars = {
@@ -1503,7 +1503,7 @@ static struct ctl_table ctl_forward_entry[] = {
 	{
 		.procname	= "ip_forward",
 		.data		= &ipv4_devconf.data[
-					NET_IPV4_CONF_FORWARDING - 1],
+					IPV4_DEVCONF_FORWARDING - 1],
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= devinet_sysctl_forward,
@@ -1547,7 +1547,7 @@ static __net_init int devinet_init_net(struct net *net)
 		if (tbl == NULL)
 			goto err_alloc_ctl;
 
-		tbl[0].data = &all->data[NET_IPV4_CONF_FORWARDING - 1];
+		tbl[0].data = &all->data[IPV4_DEVCONF_FORWARDING - 1];
 		tbl[0].extra1 = all;
 		tbl[0].extra2 = net;
 #endif
-- 
1.6.5.2.143.g8cc62

[net-next-2.6 PATCH 2/2] net neigh: Decouple per interface neighbour table controls from binary sysctls

[Eric W. Biederman]

Stop computing the number of neighbour table settings we have by
counting the number of binary sysctls.  This behaviour was silly
and meant that we could not add another neighbour table setting
without also adding another binary sysctl.

Don't pass the binary sysctl path for neighour table entries
into neigh_sysctl_register.  These parameters are no longer
used and so are just dead code.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 include/linux/sysctl.h  |    1 -
 include/net/neighbour.h |    1 -
 net/core/neighbour.c    |    7 ++++---
 net/ipv4/arp.c          |    3 +--
 net/ipv4/devinet.c      |    3 +--
 net/ipv6/addrconf.c     |    3 +--
 net/ipv6/ndisc.c        |    3 +--
 7 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 7c74e91..f66014c 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -596,7 +596,6 @@ enum {
 	NET_NEIGH_GC_THRESH3=16,
 	NET_NEIGH_RETRANS_TIME_MS=17,
 	NET_NEIGH_REACHABLE_TIME_MS=18,
-	__NET_NEIGH_MAX
 };
 
 /* /proc/sys/net/dccp */
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index b017320..7834f47 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -251,7 +251,6 @@ extern void neigh_seq_stop(struct seq_file *, void *);
 
 extern int			neigh_sysctl_register(struct net_device *dev, 
 						      struct neigh_parms *p,
-						      int p_id, int pdev_id,
 						      char *p_name,
 						      proc_handler *proc_handler);
 extern void			neigh_sysctl_unregister(struct neigh_parms *p);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index f2efd72..d102f6d 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2556,9 +2556,11 @@ EXPORT_SYMBOL(neigh_app_ns);
 
 #ifdef CONFIG_SYSCTL
 
+#define NEIGH_VARS_MAX 19
+
 static struct neigh_sysctl_table {
 	struct ctl_table_header *sysctl_header;
-	struct ctl_table neigh_vars[__NET_NEIGH_MAX];
+	struct ctl_table neigh_vars[NEIGH_VARS_MAX];
 	char *dev_name;
 } neigh_sysctl_template __read_mostly = {
 	.neigh_vars = {
@@ -2675,8 +2677,7 @@ static struct neigh_sysctl_table {
 };
 
 int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
-			  int p_id, int pdev_id, char *p_name,
-			  proc_handler *handler)
+			  char *p_name, proc_handler *handler)
 {
 	struct neigh_sysctl_table *t;
 	const char *dev_name_source = NULL;
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 1940b4d..c4dd135 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1284,8 +1284,7 @@ void __init arp_init(void)
 	dev_add_pack(&arp_packet_type);
 	arp_proc_init();
 #ifdef CONFIG_SYSCTL
-	neigh_sysctl_register(NULL, &arp_tbl.parms, NET_IPV4,
-			      NET_IPV4_NEIGH, "ipv4", NULL);
+	neigh_sysctl_register(NULL, &arp_tbl.parms, "ipv4", NULL);
 #endif
 	register_netdevice_notifier(&arp_netdev_notifier);
 }
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index b1eddee..014982b 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1487,8 +1487,7 @@ static void __devinet_sysctl_unregister(struct ipv4_devconf *cnf)
 
 static void devinet_sysctl_register(struct in_device *idev)
 {
-	neigh_sysctl_register(idev->dev, idev->arp_parms, NET_IPV4,
-			NET_IPV4_NEIGH, "ipv4", NULL);
+	neigh_sysctl_register(idev->dev, idev->arp_parms, "ipv4", NULL);
 	__devinet_sysctl_register(dev_net(idev->dev), idev->dev->name,
 					&idev->cnf);
 }
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 764ad37..c79cbff 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4416,8 +4416,7 @@ static void __addrconf_sysctl_unregister(struct ipv6_devconf *p)
 
 static void addrconf_sysctl_register(struct inet6_dev *idev)
 {
-	neigh_sysctl_register(idev->dev, idev->nd_parms, NET_IPV6,
-			      NET_IPV6_NEIGH, "ipv6",
+	neigh_sysctl_register(idev->dev, idev->nd_parms, "ipv6",
 			      &ndisc_ifinfo_sysctl_change);
 	__addrconf_sysctl_register(dev_net(idev->dev), idev->dev->name,
 					idev, &idev->cnf);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 2dfec6b..8bcc4b7 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1820,8 +1820,7 @@ int __init ndisc_init(void)
 	neigh_table_init(&nd_tbl);
 
 #ifdef CONFIG_SYSCTL
-	err = neigh_sysctl_register(NULL, &nd_tbl.parms, NET_IPV6,
-				    NET_IPV6_NEIGH, "ipv6",
+	err = neigh_sysctl_register(NULL, &nd_tbl.parms, "ipv6",
 				    &ndisc_ifinfo_sysctl_change);
 	if (err)
 		goto out_unregister_pernet;
-- 
1.6.5.2.143.g8cc62

Re: [net-next-2.6 PATCH 1/2] net ipv4: Decouple ipv4 interface parameters from binary sysctl numbers

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sun, 14 Feb 2010 05:25:51 -0800

> 
> Stop using the binary sysctl enumeartion in sysctl.h as an index into
> a per interface array.  This leads to unnecessary binary sysctl number
> allocation, and a fragility in data structure and implementation
> because of unnecessary coupling.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>

Applied.

Re: [net-next-2.6 PATCH 2/2] net neigh: Decouple per interface neighbour table controls from binary sysctls

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sun, 14 Feb 2010 05:27:03 -0800

> 
> Stop computing the number of neighbour table settings we have by
> counting the number of binary sysctls.  This behaviour was silly
> and meant that we could not add another neighbour table setting
> without also adding another binary sysctl.
> 
> Don't pass the binary sysctl path for neighour table entries
> into neigh_sysctl_register.  These parameters are no longer
> used and so are just dead code.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>

Applied.

I bet we can do even better and make it such that even
this NEIGH_VARS_MAX thing isn't needed.

Anyways, thanks for doing this work Eric.