net: sysctl: share ipv4/ipv6 sysctl tables

Determining the parent of a node at depth d
- previous implementation: O(d)
- current implementation: O(1)

Printing the path to a node at depth d
- previous implementation: O(d^2)
- current implementation: O(d)

This comes to a cost: we use an array ('parents') holding as many pointers as there can be sysctl levels (currently CTL_MAXNAME=10).

The 'parents' array of pointers holds the same values as the ctl_table->parents field because the function that updates ->parents (sysctl_set_parent) is called with either NULL (for root nodes) or with sysctl_set_parent(table, table->child).

Signed-off-by Lucian Adrian Grijincu <[email protected]>

---
 kernel/sysctl_check.c |  121 ++++++++++++++++++++++++-------------------------
 1 files changed, 60 insertions(+), 61 deletions(-)

kernel/sysctl_check.c +60 -61

index 10b90d8..9b4fecd 100644
@@ -6,58 +6,34 @@
 #include <net/ip_vs.h>
 
 
-static int sysctl_depth(struct ctl_table *table)
-{
-	struct ctl_table *tmp;
-	int depth;
-
-	depth = 0;
-	for (tmp = table; tmp->parent; tmp = tmp->parent)
-		depth++;
-
-	return depth;
-}
-
-static struct ctl_table *sysctl_parent(struct ctl_table *table, int n)
+static void sysctl_print_path(struct ctl_table *table,
+			      struct ctl_table **parents, int depth)
 {
+	struct ctl_table *p;
 	int i;
-
-	for (i = 0; table && i < n; i++)
-		table = table->parent;
-
-	return table;
-}
-
-
-static void sysctl_print_path(struct ctl_table *table)
-{
-	struct ctl_table *tmp;
-	int depth, i;
-	depth = sysctl_depth(table);
 	if (table->procname) {
-		for (i = depth; i >= 0; i--) {
-			tmp = sysctl_parent(table, i);
-			printk("/%s", tmp->procname?tmp->procname:"");
+		for (i = 0; i < depth; i++) {
+			p = parents[i];
+			printk("/%s", p->procname ? p->procname : "");
 		}
+		printk("/%s", table->procname);
 	}
 	printk(" ");
 }
 
 static struct ctl_table *sysctl_check_lookup(struct nsproxy *namespaces,
-						struct ctl_table *table)
+	     struct ctl_table *table, struct ctl_table **parents, int depth)
 {
 	struct ctl_table_header *head;
 	struct ctl_table *ref, *test;
-	int depth, cur_depth;
-
-	depth = sysctl_depth(table);
+	int cur_depth;
 
 	for (head = __sysctl_head_next(namespaces, NULL); head;
 	     head = __sysctl_head_next(namespaces, head)) {
 		cur_depth = depth;
 		ref = head->ctl_table;
 repeat:
-		test = sysctl_parent(table, cur_depth);
+		test = parents[depth - cur_depth];
 		for (; ref->procname; ref++) {
 			int match = 0;
 			if (cur_depth && !ref->child)

@@ -83,11 +59,12 @@ out:
 	return ref;
 }
 
-static void set_fail(const char **fail, struct ctl_table *table, const char *str)
+static void set_fail(const char **fail, struct ctl_table *table,
+	     const char *str, struct ctl_table **parents, int depth)
 {
 	if (*fail) {
 		printk(KERN_ERR "sysctl table check failed: ");
-		sysctl_print_path(table);
+		sysctl_print_path(table, parents, depth);
 		printk(" %s\n", *fail);
 		dump_stack();
 	}

@@ -95,16 +72,24 @@ static void set_fail(const char **fail, struct ctl_table *table, const char *str
 }
 
 static void sysctl_check_leaf(struct nsproxy *namespaces,
-				struct ctl_table *table, const char **fail)
+			      struct ctl_table *table, const char **fail,
+			      struct ctl_table **parents, int depth)
 {
 	struct ctl_table *ref;
 
-	ref = sysctl_check_lookup(namespaces, table);
-	if (ref && (ref != table))
-		set_fail(fail, table, "Sysctl already exists");
+	ref = sysctl_check_lookup(namespaces, table, parents, depth);
+	if (ref && (ref != table)) {
+		printk(KERN_ALERT "sysctl_check_leaf ref[%s], table[%s]\n", ref->procname, table->procname);
+		set_fail(fail, table, "Sysctl already exists", parents, depth);
+	}
 }
 
-int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
+
+
+#define SET_FAIL(str) set_fail(&fail, table, str, parents, depth)
+
+static int __sysctl_check_table(struct nsproxy *namespaces,
+	struct ctl_table *table, struct ctl_table **parents, int depth)
 {
 	int error = 0;
 	for (; table->procname; table++) {

@@ -112,23 +97,23 @@ int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
 
 		if (table->parent) {
 			if (table->procname && !table->parent->procname)
-				set_fail(&fail, table, "Parent without procname");
+				SET_FAIL("Parent without procname");
 		}
 		if (!table->procname)
-			set_fail(&fail, table, "No procname");
+			SET_FAIL("No procname");
 		if (table->child) {
 			if (table->data)
-				set_fail(&fail, table, "Directory with data?");
+				SET_FAIL("Directory with data?");
 			if (table->maxlen)
-				set_fail(&fail, table, "Directory with maxlen?");
+				SET_FAIL("Directory with maxlen?");
 			if ((table->mode & (S_IRUGO|S_IXUGO)) != table->mode)
-				set_fail(&fail, table, "Writable sysctl directory");
+				SET_FAIL("Writable sysctl directory");
 			if (table->proc_handler)
-				set_fail(&fail, table, "Directory with proc_handler");
+				SET_FAIL("Directory with proc_handler");
 			if (table->extra1)
-				set_fail(&fail, table, "Directory with extra1");
+				SET_FAIL("Directory with extra1");
 			if (table->extra2)
-				set_fail(&fail, table, "Directory with extra2");
+				SET_FAIL("Directory with extra2");
 		} else {
 			if ((table->proc_handler == proc_dostring) ||
 			    (table->proc_handler == proc_dointvec) ||

@@ -139,28 +124,42 @@ int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
 			    (table->proc_handler == proc_doulongvec_minmax) ||
 			    (table->proc_handler == proc_doulongvec_ms_jiffies_minmax)) {
 				if (!table->data)
-					set_fail(&fail, table, "No data");
+					SET_FAIL("No data");
 				if (!table->maxlen)
-					set_fail(&fail, table, "No maxlen");
+					SET_FAIL("No maxlen");
 			}
 #ifdef CONFIG_PROC_SYSCTL
 			if (table->procname && !table->proc_handler)
-				set_fail(&fail, table, "No proc_handler");
-#endif
-#if 0
-			if (!table->procname && table->proc_handler)
-				set_fail(&fail, table, "proc_handler without procname");
+				SET_FAIL("No proc_handler");
 #endif
-			sysctl_check_leaf(namespaces, table, &fail);
+			parents[depth] = table;
+			sysctl_check_leaf(namespaces, table, &fail,
+					  parents, depth);
 		}
 		if (table->mode > 0777)
-			set_fail(&fail, table, "bogus .mode");
+			SET_FAIL("bogus .mode");
 		if (fail) {
-			set_fail(&fail, table, NULL);
+			SET_FAIL(NULL);
 			error = -EINVAL;
 		}
-		if (table->child)
-			error |= sysctl_check_table(namespaces, table->child);
+		if (table->child) {
+			parents[depth] = table;
+			error |= __sysctl_check_table(namespaces, table->child,
+						      parents, depth + 1);
+		}
 	}
 	return error;
 }
+
+
+int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
+{
+	struct ctl_table *parents[CTL_MAXNAME];
+	/* Keep track of parents as we go down into the tree.
+	 *
+	 * parents[i-1] will be the parent for parents[i].
+	 * The node at depth 'd' will have the parent at parents[d-1].
+	 * The root node (depth=0) has no parent in this array.
+	 */
+	return __sysctl_check_table(namespaces, table, parents, 0);
+}

Determining the parent of a node at depth d
- previous implementation: O(d)
- current implementation: O(1)

Printing the path to a node at depth d
- previous implementation: O(d^2)
- current implementation: O(d)

This comes to a cost: we use an array ('parents') holding as many pointers as there can be sysctl levels (currently CTL_MAXNAME=10).

The 'parents' array of pointers holds the same values as the ctl_table->parents field because the function that updates ->parents (sysctl_set_parent) is called with either NULL (for root nodes) or with sysctl_set_parent(table, table->child).

Signed-off-by Lucian Adrian Grijincu <[email protected]>

---
 kernel/sysctl_check.c |  130 +++++++++++++++++++++++++-----------------------
 1 files changed, 68 insertions(+), 62 deletions(-)

kernel/sysctl_check.c +21 -41

index 10b90d8..d23b085 100644
@@ -6,58 +6,34 @@
 #include <net/ip_vs.h>
 
 
-static int sysctl_depth(struct ctl_table *table)
-{
-	struct ctl_table *tmp;
-	int depth;
-
-	depth = 0;
-	for (tmp = table; tmp->parent; tmp = tmp->parent)
-		depth++;
-
-	return depth;
-}
-
-static struct ctl_table *sysctl_parent(struct ctl_table *table, int n)
+static void sysctl_print_path(struct ctl_table *table,
+			      struct ctl_table **parents, int depth)
 {
+	struct ctl_table *p;
 	int i;
-
-	for (i = 0; table && i < n; i++)
-		table = table->parent;
-
-	return table;
-}
-
-
-static void sysctl_print_path(struct ctl_table *table)
-{
-	struct ctl_table *tmp;
-	int depth, i;
-	depth = sysctl_depth(table);
 	if (table->procname) {
-		for (i = depth; i >= 0; i--) {
-			tmp = sysctl_parent(table, i);
-			printk("/%s", tmp->procname?tmp->procname:"");
+		for (i = 0; i < depth; i++) {
+			p = parents[i];
+			printk("/%s", p->procname ? p->procname : "");
 		}
+		printk("/%s", table->procname);
 	}
 	printk(" ");
 }
 
 static struct ctl_table *sysctl_check_lookup(struct nsproxy *namespaces,
-						struct ctl_table *table)
+	     struct ctl_table *table, struct ctl_table **parents, int depth)
 {
 	struct ctl_table_header *head;
 	struct ctl_table *ref, *test;
-	int depth, cur_depth;
-
-	depth = sysctl_depth(table);
+	int cur_depth;
 
 	for (head = __sysctl_head_next(namespaces, NULL); head;
 	     head = __sysctl_head_next(namespaces, head)) {
 		cur_depth = depth;
 		ref = head->ctl_table;
 repeat:
-		test = sysctl_parent(table, cur_depth);
+		test = parents[depth - cur_depth];
 		for (; ref->procname; ref++) {
 			int match = 0;
 			if (cur_depth && !ref->child)

@@ -83,11 +59,12 @@ out:
 	return ref;
 }
 
-static void set_fail(const char **fail, struct ctl_table *table, const char *str)
+static void set_fail(const char **fail, struct ctl_table *table,
+	     const char *str, struct ctl_table **parents, int depth)
 {
 	if (*fail) {
 		printk(KERN_ERR "sysctl table check failed: ");
-		sysctl_print_path(table);
+		sysctl_print_path(table, parents, depth);
 		printk(" %s\n", *fail);
 		dump_stack();
 	}

@@ -95,40 +72,55 @@ static void set_fail(const char **fail, struct ctl_table *table, const char *str
 }
 
 static void sysctl_check_leaf(struct nsproxy *namespaces,
-				struct ctl_table *table, const char **fail)
+			      struct ctl_table *table, const char **fail,
+			      struct ctl_table **parents, int depth)
 {
 	struct ctl_table *ref;
 
-	ref = sysctl_check_lookup(namespaces, table);
-	if (ref && (ref != table))
-		set_fail(fail, table, "Sysctl already exists");
+	ref = sysctl_check_lookup(namespaces, table, parents, depth);
+	if (ref && (ref != table)) {
+		printk(KERN_ALERT "sysctl_check_leaf ref[%s], table[%s]\n", ref->procname, table->procname);
+		set_fail(fail, table, "Sysctl already exists", parents, depth);
+	}
 }
 
-int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
+

kernel/sysctl_check.c +39 -21

+		SET_FAIL("Sysctl tree too deep");
+		return -EINVAL;
+	}
+
 	for (; table->procname; table++) {
-		const char *fail = NULL;
+		fail = NULL;
 
 		if (table->parent) {
 			if (table->procname && !table->parent->procname)
-				set_fail(&fail, table, "Parent without procname");
+				SET_FAIL("Parent without procname");
 		}
 		if (!table->procname)
-			set_fail(&fail, table, "No procname");
+			SET_FAIL("No procname");
 		if (table->child) {
 			if (table->data)
-				set_fail(&fail, table, "Directory with data?");
+				SET_FAIL("Directory with data?");
 			if (table->maxlen)
-				set_fail(&fail, table, "Directory with maxlen?");
+				SET_FAIL("Directory with maxlen?");
 			if ((table->mode & (S_IRUGO|S_IXUGO)) != table->mode)
-				set_fail(&fail, table, "Writable sysctl directory");
+				SET_FAIL("Writable sysctl directory");
 			if (table->proc_handler)
-				set_fail(&fail, table, "Directory with proc_handler");
+				SET_FAIL("Directory with proc_handler");
 			if (table->extra1)
-				set_fail(&fail, table, "Directory with extra1");
+				SET_FAIL("Directory with extra1");
 			if (table->extra2)
-				set_fail(&fail, table, "Directory with extra2");
+				SET_FAIL("Directory with extra2");
 		} else {
 			if ((table->proc_handler == proc_dostring) ||
 			    (table->proc_handler == proc_dointvec) ||

@@ -139,28 +131,42 @@ int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
 			    (table->proc_handler == proc_doulongvec_minmax) ||
 			    (table->proc_handler == proc_doulongvec_ms_jiffies_minmax)) {
 				if (!table->data)
-					set_fail(&fail, table, "No data");
+					SET_FAIL("No data");
 				if (!table->maxlen)
-					set_fail(&fail, table, "No maxlen");
+					SET_FAIL("No maxlen");
 			}
 #ifdef CONFIG_PROC_SYSCTL
 			if (table->procname && !table->proc_handler)
-				set_fail(&fail, table, "No proc_handler");
-#endif
-#if 0
-			if (!table->procname && table->proc_handler)
-				set_fail(&fail, table, "proc_handler without procname");
+				SET_FAIL("No proc_handler");
 #endif
-			sysctl_check_leaf(namespaces, table, &fail);
+			parents[depth] = table;
+			sysctl_check_leaf(namespaces, table, &fail,
+					  parents, depth);
 		}
 		if (table->mode > 0777)
-			set_fail(&fail, table, "bogus .mode");
+			SET_FAIL("bogus .mode");
 		if (fail) {
-			set_fail(&fail, table, NULL);
+			SET_FAIL(NULL);
 			error = -EINVAL;
 		}
-		if (table->child)
-			error |= sysctl_check_table(namespaces, table->child);
+		if (table->child) {
+			parents[depth] = table;
+			error |= __sysctl_check_table(namespaces, table->child,
+						      parents, depth + 1);
+		}
 	}
 	return error;
 }
+
+
+int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
+{
+	struct ctl_table *parents[CTL_MAXNAME];
+	/* Keep track of parents as we go down into the tree.
+	 *
+	 * parents[i-1] will be the parent for parents[i].
+	 * The node at depth 'd' will have the parent at parents[d-1].
+	 * The root node (depth=0) has no parent in this array.
+	 */
+	return __sysctl_check_table(namespaces, table, parents, 0);
+}

Quoted 7 lines

+static int __sysctl_check_table(struct nsproxy *namespaces,
+     struct ctl_table *table, struct ctl_table **parents, int depth) {
+     const char *fail = NULL;
      int error = 0;
+
+     if (depth >= CTL_MAXNAME) {

This should be depth > CTL_MAXNAME.  Because there are only CTL_MAXNAME entries in the array.

Quoted 11 lines

-                     sysctl_check_leaf(namespaces, table, &fail);
+                     parents[depth] = table;
+                     sysctl_check_leaf(namespaces, table, &fail,
+                                       parents, depth); }

+             if (table->child) {
+                     parents[depth] = table;
+                     error |= __sysctl_check_table(namespaces, table->child,
+                                                   parents, depth + 1);
+             }

The 'parent' field was added for selinux in:
commit d912b0cc1a617d7c590d57b7ea971d50c7f02503
[PATCH] sysctl: add a parent entry to ctl_table and set the parent entry

and then was used for sysctl_check_table.

Both of the users have found other implementations.

CC Eric W. Biederman <[email protected]>

Signed-off-by Lucian Adrian Grijincu <[email protected]>

---
 include/linux/sysctl.h |    1 -
 kernel/sysctl.c        |   11 -----------
 kernel/sysctl_check.c  |    4 ++--
 3 files changed, 2 insertions(+), 14 deletions(-)

include/linux/sysctl.h +0 -1

index 7bb5cb6..1f1da4b 100644
@@ -1018,7 +1018,6 @@ struct ctl_table
 	int maxlen;
 	mode_t mode;
 	struct ctl_table *child;
-	struct ctl_table *parent;	/* Automatically set */
 	proc_handler *proc_handler;	/* Callback for text formatting */
 	void *extra1;
 	void *extra2;

kernel/sysctl.c +0 -11

index 56f6fc1..42025ec 100644
@@ -1695,18 +1695,8 @@ int sysctl_perm(struct ctl_table_root *root, struct ctl_table *table, int op)
 	return test_perm(mode, op);
 }
 
-static void sysctl_set_parent(struct ctl_table *parent, struct ctl_table *table)
-{
-	for (; table->procname; table++) {
-		table->parent = parent;
-		if (table->child)
-			sysctl_set_parent(table, table->child);
-	}
-}
-
 static __init int sysctl_init(void)
 {
-	sysctl_set_parent(NULL, root_table);
 #ifdef CONFIG_SYSCTL_SYSCALL_CHECK
 	sysctl_check_table(current->nsproxy, root_table);
 #endif

@@ -1864,7 +1854,6 @@ struct ctl_table_header *__register_sysctl_paths(
 	header->used = 0;
 	header->unregistering = NULL;
 	header->root = root;
-	sysctl_set_parent(NULL, header->ctl_table);
 	header->count = 1;
 #ifdef CONFIG_SYSCTL_SYSCALL_CHECK
 	if (sysctl_check_table(namespaces, header->ctl_table)) {

kernel/sysctl_check.c +2 -2

index 9b4fecd..b7d9c66 100644
@@ -95,8 +95,8 @@ static int __sysctl_check_table(struct nsproxy *namespaces,
 	for (; table->procname; table++) {
 		const char *fail = NULL;
 
-		if (table->parent) {
-			if (table->procname && !table->parent->procname)
+		if (depth != 0) { /* has parent */
+			if (table->procname && !parents[depth - 1]->procname)
 				SET_FAIL("Parent without procname");
 		}
 		if (!table->procname)

Before this, for each network device DEVNAME that supports ipv4 a new sysctl table was registered in $PROC/sys/net/ipv4/conf/DEVNAME/.

The sysctl table was identical for all network devices, except for:
* data: pointer to the data to be accessed in the sysctl
* extra1: the 'struct ipv4_devconf*' of the network device
* extra2: the 'struct net*' of the network namespace

Assuming we have a device name and a 'struct net*', we can get the 'struct net_device*'. From there we can compute:
* data: each entry corresponds to a position in 'struct ipv4_devconf*'
* extra1: 'struct ipv4_devconf*' can be reached from 'struct net_device*'
* extra2: the 'struct net*' that we assumed to have

The device name is determined from the path to the file (the name of the parent dentry).

The 'struct net*' is stored in the parent 'struct ctl_table*' path by register_net_sysctl_table_pathdata().

Signed-off-by Lucian Adrian Grijincu <[email protected]>

---
 fs/proc/proc_sysctl.c      |   16 +++-
 include/linux/inetdevice.h |   12 +++-
 net/ipv4/devinet.c         |  203 +++++++++++++++++++++++++++++---------------
 3 files changed, 161 insertions(+), 70 deletions(-)

fs/proc/proc_sysctl.c +15 -1

index fb707e0..fe392f1 100644
@@ -128,6 +128,11 @@ out:
 	return err;
 }
 
+
+typedef int proc_handler_extended(struct ctl_table *ctl, int write,
+				  void __user *buffer, size_t *lenp, loff_t *ppos,
+				  struct file *filp);
+
 static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
 		size_t count, loff_t *ppos, int write)
 {

@@ -136,6 +141,7 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
 	struct ctl_table *table = PROC_I(inode)->sysctl_entry;
 	ssize_t error;
 	size_t res;
+	proc_handler_extended *phx = (proc_handler_extended *) table->proc_handler;
 
 	if (IS_ERR(head))
 		return PTR_ERR(head);

@@ -155,7 +161,15 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
 
 	/* careful: calling conventions are nasty here */
 	res = count;
-	error = table->proc_handler(table, write, buf, &res, ppos);
+	/* Most handlers only use the first 5 arguments (without @filp).
+	 * Changing all is too much of work, as, at the time of writting only
+	 * the devinet.c proc_handlers know about and use the @filp.
+	 *
+	 * This is just a HACK for now, I did this this way to not
+	 * waste time changing all the handlers, in the final version
+	 * I'll change all the handlers if there's not other solution.
+	 */
+	error = phx(table, write, buf, &res, ppos, filp);
 	if (!error)
 		error = res;
 out:

include/linux/inetdevice.h +11 -1

index ae8fdc5..caf06b3 100644
@@ -43,8 +43,18 @@ enum
 
 #define IPV4_DEVCONF_MAX (__IPV4_DEVCONF_MAX - 1)
 
+
+struct devinet_sysctl {
+	/* dev_name holds a copy of dev_name, because '.procname' is
+	 * regarded as const by sysctl and we wouldn't want anyone to
+	 * change it under our feet (see SIOCSIFNAME). */
+	char *dev_name;
+	struct ctl_table_header *sysctl_header;
+};
+
+
 struct ipv4_devconf {
-	void	*sysctl;
+	struct devinet_sysctl devinet_sysctl;
 	int	data[IPV4_DEVCONF_MAX];
 	DECLARE_BITMAP(state, IPV4_DEVCONF_MAX);
 };

net/ipv4/devinet.c +135 -68

index 748cb5b..774d347 100644
@@ -147,7 +147,7 @@ void in_dev_finish_destroy(struct in_device *idev)
 }
 EXPORT_SYMBOL(in_dev_finish_destroy);
 
-static struct in_device *inetdev_init(struct net_device *dev)
+struct in_device *inetdev_init(struct net_device *dev)
 {
 	struct in_device *in_dev;
 

@@ -158,7 +158,8 @@ static struct in_device *inetdev_init(struct net_device *dev)
 		goto out;
 	memcpy(&in_dev->cnf, dev_net(dev)->ipv4.devconf_dflt,
 			sizeof(in_dev->cnf));
-	in_dev->cnf.sysctl = NULL;
+	in_dev->cnf.devinet_sysctl.dev_name = NULL;
+	in_dev->cnf.devinet_sysctl.sysctl_header = NULL;
 	in_dev->dev = dev;
 	in_dev->arp_parms = neigh_parms_alloc(dev, &arp_tbl);
 	if (!in_dev->arp_parms)

@@ -1375,6 +1376,67 @@ static void inet_forward_change(struct net *net)
 	}
 }
 
+
+
+static int devinet_conf_handler(ctl_table *ctl, int write,
+				void __user *buffer,
+				size_t *lenp, loff_t *ppos,
+				struct file *filp,
+				proc_handler *proc_handler)
+{
+	/* The path to this file is of the form:
+	 *  $PROC_MOUNT/sys/net/ipv4/conf/$DEVNAME/$CTL
+	 *
+	 * The array of 'struct ctl_table' of devinet entries is
+	 * shared between all ipv4 network devices and the 'data'
+	 * field of each structure only hold the offset into the
+	 * 'data' field of 'struct ipv4_devconf'.
+	 *
+	 * To find the propper location of the data that must be
+	 * accessed by this handler we need the device name and the
+	 * network namespace in which it belongs.
+	 */
+
+	/* We store the network namespace in the parent table's ->extra2 */
+	struct inode *parent_inode = filp->f_path.dentry->d_parent->d_inode;
+	struct ctl_table *parent_table = PROC_I(parent_inode)->sysctl_entry;
+	struct net *net = parent_table->extra2;
+
+	const char *dev_name = filp->f_path.dentry->d_parent->d_name.name;
+	struct ctl_table tmp_ctl;
+	struct net_device *dev = NULL;
+	struct in_device *in_dev = NULL;
+	struct ipv4_devconf *cnf;
+	int ret;
+
+	if (strcmp(dev_name, "all") == 0) {
+		cnf = net->ipv4.devconf_all;
+	} else if (strcmp(dev_name, "default") == 0) {
+		cnf = net->ipv4.devconf_dflt;
+	} else {
+		/* the device could have been renamed (SIOCSIFADDR) or
+		 * deleted since we started accessing it's proc sysctl */
+		dev = dev_get_by_name(net, dev_name);
+		if (dev == NULL)
+			return -ENOENT;
+		in_dev = in_dev_get(dev);
+		cnf = &in_dev->cnf;
+	}
+
+	tmp_ctl = *ctl;
+	tmp_ctl.data += (char *)cnf - (char *)&ipv4_devconf;
+	tmp_ctl.extra1 = cnf;
+	tmp_ctl.extra2 = net;
+
+	ret = proc_handler(&tmp_ctl, write, buffer, lenp, ppos);
+
+	if (in_dev)
+		in_dev_put(in_dev);
+	if (dev)
+		dev_put(dev);
+	return ret;
+}
+
 static int devinet_conf_proc(ctl_table *ctl, int write,
 			     void __user *buffer,
 			     size_t *lenp, loff_t *ppos)

@@ -1445,6 +1507,33 @@ static int ipv4_doint_and_flush(ctl_table *ctl, int write,
 	return ret;
 }
 
+static int devinet_conf_proc__(ctl_table *ctl, int write,
+			       void __user *buffer,
+			       size_t *lenp, loff_t *ppos,
+			       struct file *filp)
+{
+	return devinet_conf_handler(ctl, write, buffer, lenp, ppos, filp,
+				    devinet_conf_proc);
+}
+
+static int devinet_sysctl_forward__(ctl_table *ctl, int write,
+				    void __user *buffer,
+				    size_t *lenp, loff_t *ppos,
+				    struct file *filp)
+{
+	return devinet_conf_handler(ctl, write, buffer, lenp, ppos, filp,
+				    devinet_sysctl_forward);
+}
+
+static int ipv4_doint_and_flush__(ctl_table *ctl, int write,
+				  void __user *buffer,
+				  size_t *lenp, loff_t *ppos,
+				  struct file *filp)
+{
+	return devinet_conf_handler(ctl, write, buffer, lenp, ppos, filp,
+				    ipv4_doint_and_flush);
+}
+
 #define DEVINET_SYSCTL_ENTRY(attr, name, mval, proc) \
 	{ \
 		.procname	= name, \

@@ -1452,67 +1541,60 @@ static int ipv4_doint_and_flush(ctl_table *ctl, int write,
 				  IPV4_DEVCONF_ ## attr - 1, \
 		.maxlen		= sizeof(int), \
 		.mode		= mval, \
-		.proc_handler	= proc, \
-		.extra1		= &ipv4_devconf, \
+		.proc_handler	= (proc_handler *) proc, \
 	}
 
 #define DEVINET_SYSCTL_RW_ENTRY(attr, name) \
-	DEVINET_SYSCTL_ENTRY(attr, name, 0644, devinet_conf_proc)
+	DEVINET_SYSCTL_ENTRY(attr, name, 0644, devinet_conf_proc__)
 
 #define DEVINET_SYSCTL_RO_ENTRY(attr, name) \
-	DEVINET_SYSCTL_ENTRY(attr, name, 0444, devinet_conf_proc)
+	DEVINET_SYSCTL_ENTRY(attr, name, 0444, devinet_conf_proc__)
 
 #define DEVINET_SYSCTL_COMPLEX_ENTRY(attr, name, proc) \
 	DEVINET_SYSCTL_ENTRY(attr, name, 0644, proc)
 
 #define DEVINET_SYSCTL_FLUSHING_ENTRY(attr, name) \
-	DEVINET_SYSCTL_COMPLEX_ENTRY(attr, name, ipv4_doint_and_flush)
-
-static struct devinet_sysctl_table {
-	struct ctl_table_header *sysctl_header;
-	struct ctl_table devinet_vars[__IPV4_DEVCONF_MAX];
-	char *dev_name;
-} devinet_sysctl = {
-	.devinet_vars = {
-		DEVINET_SYSCTL_COMPLEX_ENTRY(FORWARDING, "forwarding",
-					     devinet_sysctl_forward),
-		DEVINET_SYSCTL_RO_ENTRY(MC_FORWARDING, "mc_forwarding"),
-
-		DEVINET_SYSCTL_RW_ENTRY(ACCEPT_REDIRECTS, "accept_redirects"),
-		DEVINET_SYSCTL_RW_ENTRY(SECURE_REDIRECTS, "secure_redirects"),
-		DEVINET_SYSCTL_RW_ENTRY(SHARED_MEDIA, "shared_media"),
-		DEVINET_SYSCTL_RW_ENTRY(RP_FILTER, "rp_filter"),
-		DEVINET_SYSCTL_RW_ENTRY(SEND_REDIRECTS, "send_redirects"),
-		DEVINET_SYSCTL_RW_ENTRY(ACCEPT_SOURCE_ROUTE,
-					"accept_source_route"),
-		DEVINET_SYSCTL_RW_ENTRY(ACCEPT_LOCAL, "accept_local"),
-		DEVINET_SYSCTL_RW_ENTRY(SRC_VMARK, "src_valid_mark"),
-		DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP, "proxy_arp"),
-		DEVINET_SYSCTL_RW_ENTRY(MEDIUM_ID, "medium_id"),
-		DEVINET_SYSCTL_RW_ENTRY(BOOTP_RELAY, "bootp_relay"),
-		DEVINET_SYSCTL_RW_ENTRY(LOG_MARTIANS, "log_martians"),
-		DEVINET_SYSCTL_RW_ENTRY(TAG, "tag"),
-		DEVINET_SYSCTL_RW_ENTRY(ARPFILTER, "arp_filter"),
-		DEVINET_SYSCTL_RW_ENTRY(ARP_ANNOUNCE, "arp_announce"),
-		DEVINET_SYSCTL_RW_ENTRY(ARP_IGNORE, "arp_ignore"),
-		DEVINET_SYSCTL_RW_ENTRY(ARP_ACCEPT, "arp_accept"),
-		DEVINET_SYSCTL_RW_ENTRY(ARP_NOTIFY, "arp_notify"),
-		DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
-
-		DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
-					      "force_igmp_version"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
-					      "promote_secondaries"),
-	},
+	DEVINET_SYSCTL_COMPLEX_ENTRY(attr, name, ipv4_doint_and_flush__)
+
+const struct ctl_table ipv4_devinet_sysctl_table[__IPV4_DEVCONF_MAX] = {
+	DEVINET_SYSCTL_COMPLEX_ENTRY(FORWARDING, "forwarding",
+				     devinet_sysctl_forward__),
+	DEVINET_SYSCTL_RO_ENTRY(MC_FORWARDING, "mc_forwarding"),
+
+	DEVINET_SYSCTL_RW_ENTRY(ACCEPT_REDIRECTS, "accept_redirects"),
+	DEVINET_SYSCTL_RW_ENTRY(SECURE_REDIRECTS, "secure_redirects"),
+	DEVINET_SYSCTL_RW_ENTRY(SHARED_MEDIA, "shared_media"),
+	DEVINET_SYSCTL_RW_ENTRY(RP_FILTER, "rp_filter"),
+	DEVINET_SYSCTL_RW_ENTRY(SEND_REDIRECTS, "send_redirects"),
+	DEVINET_SYSCTL_RW_ENTRY(ACCEPT_SOURCE_ROUTE,
+				"accept_source_route"),
+	DEVINET_SYSCTL_RW_ENTRY(ACCEPT_LOCAL, "accept_local"),
+	DEVINET_SYSCTL_RW_ENTRY(SRC_VMARK, "src_valid_mark"),
+	DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP, "proxy_arp"),
+	DEVINET_SYSCTL_RW_ENTRY(MEDIUM_ID, "medium_id"),
+	DEVINET_SYSCTL_RW_ENTRY(BOOTP_RELAY, "bootp_relay"),
+	DEVINET_SYSCTL_RW_ENTRY(LOG_MARTIANS, "log_martians"),
+	DEVINET_SYSCTL_RW_ENTRY(TAG, "tag"),
+	DEVINET_SYSCTL_RW_ENTRY(ARPFILTER, "arp_filter"),
+	DEVINET_SYSCTL_RW_ENTRY(ARP_ANNOUNCE, "arp_announce"),
+	DEVINET_SYSCTL_RW_ENTRY(ARP_IGNORE, "arp_ignore"),
+	DEVINET_SYSCTL_RW_ENTRY(ARP_ACCEPT, "arp_accept"),
+	DEVINET_SYSCTL_RW_ENTRY(ARP_NOTIFY, "arp_notify"),
+	DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
+
+	DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
+	DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
+	DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
+				      "force_igmp_version"),
+	DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
+				      "promote_secondaries"),
+	{ }
 };
 
 static int __devinet_sysctl_register(struct net *net, char *dev_name,
-					struct ipv4_devconf *p)
+				     struct ipv4_devconf *cnf)
 {
-	int i;
-	struct devinet_sysctl_table *t;
+	struct devinet_sysctl *t = &cnf->devinet_sysctl;
 
 #define DEVINET_CTL_PATH_DEV	3
 

@@ -1524,16 +1606,6 @@ static int __devinet_sysctl_register(struct net *net, char *dev_name,
 		{ },
 	};
 
-	t = kmemdup(&devinet_sysctl, sizeof(*t), GFP_KERNEL);
-	if (!t)
-		goto out;
-
-	for (i = 0; i < ARRAY_SIZE(t->devinet_vars) - 1; i++) {
-		t->devinet_vars[i].data += (char *)p - (char *)&ipv4_devconf;
-		t->devinet_vars[i].extra1 = p;
-		t->devinet_vars[i].extra2 = net;
-	}
-
 	/*
 	 * Make a copy of dev_name, because '.procname' is regarded as const
 	 * by sysctl and we wouldn't want anyone to change it under our feet

@@ -1541,37 +1613,32 @@ static int __devinet_sysctl_register(struct net *net, char *dev_name,
 	 */
 	t->dev_name = kstrdup(dev_name, GFP_KERNEL);
 	if (!t->dev_name)
-		goto free;
+		goto out;
 
 	devinet_ctl_path[DEVINET_CTL_PATH_DEV].procname = t->dev_name;
 
-	t->sysctl_header = register_net_sysctl_table(net, devinet_ctl_path,
-			t->devinet_vars);
+	t->sysctl_header = register_net_sysctl_table_pathdata(net,
+			 devinet_ctl_path, ipv4_devinet_sysctl_table, net);
 	if (!t->sysctl_header)
 		goto free_procname;
 
-	p->sysctl = t;
 	return 0;
 
 free_procname:
 	kfree(t->dev_name);
-free:
-	kfree(t);
 out:
 	return -ENOBUFS;
 }
 
 static void __devinet_sysctl_unregister(struct ipv4_devconf *cnf)
 {
-	struct devinet_sysctl_table *t = cnf->sysctl;
+	struct devinet_sysctl *t = &cnf->devinet_sysctl;
 
 	if (t == NULL)
 		return;
 
-	cnf->sysctl = NULL;
 	unregister_sysctl_table(t->sysctl_header);
 	kfree(t->dev_name);
-	kfree(t);
 }
 
 static void devinet_sysctl_register(struct in_device *idev)

Each network device gets the same 25/24 sysctl entries for ipv4/ipv6 in /proc/sys/net/ipv4/conf/DEVNAME and /proc/sys/net/ipv6/conf/DEVNAME

Unfortunately, space is wasted holding very much similar data. Fortunately, with some tricks these entries can be shared between all network devices.

The single entry in 'struct ctl_table' that was modified at runtime for leaf ctl_table nodes and prevented sharing was 'parent'. This field was first introduces for selinux and then was used to implement sysctl_check_table. Selinux recently removed the need for this field:
* http://thread.gmane.org/gmane.linux.kernel.lsm/12623
* LKML-Reference: [email protected]

Remove the need for 'parent' in sysctl_check_table and remove the 'parent' field:

 [PATCH 1/5] sysctl: faster reimplementation of sysctl_check_table [PATCH 2/5] sysctl: remove useless ctl_table->parent field

Pave the way for sharing of ipv4/6 tables: allow data to be stored in the nodes above the leafs that will be shared:

 [PATCH 3/5] sysctl: write ctl_table->extra2 to entries created from ctl_path

Finally share the leaf sysctl tables for ipv4/ipv6:

 [PATCH 4/5] ipv4: share sysctl net/ipv4/conf/DEVNAME/ tables [PATCH 5/5] ipv6: share sysctl net/ipv6/conf/DEVNAME/ tables

Quoted 4 lines

Finally share the leaf sysctl tables for ipv4/ipv6:

 [PATCH 4/5] ipv4: share sysctl net/ipv4/conf/DEVNAME/ tables [PATCH 5/5] ipv6: share sysctl net/ipv6/conf/DEVNAME/ tables

Meh.

First you remove ->parent, then heroically pass "struct file *" to sysctl handlers which duplicates all information already passed and brings dcache into picture.

Binary sysctl rewrite confused you into thinking that d_name.name is the way, but it isn't.
For binary sysctl(2) you wouldn't get d_name.name.

Quoted 7 lines · Alexey Dobriyan

Quoted 4 lines

Finally share the leaf sysctl tables for ipv4/ipv6:

 [PATCH 4/5] ipv4: share sysctl net/ipv4/conf/DEVNAME/ tables [PATCH 5/5] ipv6: share sysctl net/ipv6/conf/DEVNAME/ tables

Meh.

Quoted 7 lines

First you remove ->parent, then heroically pass "struct file *" to sysctl handlers which duplicates all information already passed and brings dcache into picture.

Binary sysctl rewrite confused you into thinking that d_name.name is the way, but it isn't.
For binary sysctl(2) you wouldn't get d_name.name.

Are you really sure?

I ran this code on a machine with and without these patches. It seems to work fine.

It reads the value from /proc/sys/net/ipv4/conf/default/tag and writes 42 back.

I'm not sure what I have to do to pass the name of a device (e.g. "eth0") instead of "default" but at least "default" and "all" work and have valid dentries.

Quoted 3 lines

I'm not sure what I have to do to pass the name of a device (e.g. "eth0") instead of "default" but at least "default" and "all" work and have valid dentries.

You do ->parent->procname.

But, but you removed parent.