Sysctl shadow management

kernel/sysctl.c void unregister_sysctl_table(struct ctl_table_header * header) · line 1652

Quoted 10 lines · Pavel Emelyanov

kernel/sysctl.c +10 -0

+static void ctl_free_table(struct ctl_table *t)
+{
+       struct ctl_table *tmp;
+
+       for (tmp = t; tmp->ctl_name || tmp->procname; tmp++)
+               if (tmp->child)
+                       ctl_free_table(tmp->child);
+
+       kfree(t);
+} 

Are you worried about the recursion at all?

-- Dave

ipc/ipc_sysctl.c +12 -0

+int ipc_clone_sysctl(struct ipc_namespace *ns)
+{
+       struct ctl_table_header *h;
+       struct ctl_table *t;
+
+       h = create_sysctl_shadow(init_ipc_ns.ctl_head);
+       if (h == NULL)
+               return -ENOMEM;
+
+       t = h->ctl_table->child;
+
+       t[0].data = &ns->shm_ctlmax;

Quoted 23 lines · Pavel Emelyanov

ipc/ipc_sysctl.c +22 -0

+int ipc_clone_sysctl(struct ipc_namespace *ns)
+{
+       struct ctl_table_header *h;
+       struct ctl_table *t;
+
+       h = create_sysctl_shadow(init_ipc_ns.ctl_head);
+       if (h == NULL)
+               return -ENOMEM;
+
+       t = h->ctl_table->child;
+
+       t[0].data = &ns->shm_ctlmax;
+       t[1].data = &ns->shm_ctlall;
+       t[2].data = &ns->shm_ctlmni;
+       t[3].data = &ns->msg_ctlmax;
+       t[4].data = &ns->msg_ctlmni;
+       t[5].data = &ns->msg_ctlmnb;
+       t[6].data = &ns->sem_ctls;
+
+       ns->ctl_head = h;
+       return 0;
+} 

Quoted 1 line

From where does the order for these things come?

-- Dave

Hi guys!

You all know, that with multiple namespaces we have to take special care about sysctls. E.g. IPC sysctl handlers are equipped with kludges to alter the sysctl parameters of appropriate namespace. The same thing should be done for UTS namespace (but it is not - we have a BUG in mainstream) and (!) for network namespaces.

Unlike all the other namespaces, network will have to not just address different variables via same sysctl names, but to have different tables with different sysctl names. E.g. /proc/sys/net/conf have entries for devices, which differ across namespaces.

Eric currently have some work done in that directions, I like the approach in general very much, but it looks rather raw (Eric, take this in good part). You know, ifdefs in the middle of the code, explicit references to net namespace and so on and so forth.

So here's the RFC for a bit better sysctls shadow management.

I will provide 3 patches:
1. the sysctl shadows themselves;
2. using shadows in UTS namespace;
3. using shadows in IPC namespace;

Using them in net namespace is already checked (I created sysctl entries with different names), but I don't have any patches against any David's tree yet. If we're OK with this set I will start talking to Andrew and David about who to send these patches to and making shadows for net-related sysctl variables.

Quoted 8 lines · Pavel Emelyanov

Hi guys!

You all know, that with multiple namespaces we have to take special care about sysctls. E.g. IPC sysctl handlers are equipped with kludges to alter the sysctl parameters of appropriate namespace. The same thing should be done for UTS namespace (but it is not - we have a BUG in mainstream) and (!) for network namespaces.

The bug in mainstream was introduced by commit:
7d69a1f4a72b18876c99c697692b78339d491568

Thee code should read:
static void *get_uts(ctl_table *table, int write) {

(quoted diff) +2 -0

 	char *which = table->data;
+	struct uts_namespace *uts_ns = current->nsproxy->uts_ns;
+	which = (which - (char *)&init_uts_ns) + (char *)uts_ns;
 
 	if (!write)
 		down_read(&uts_sem);
 	else
 		down_write(&uts_sem);
 	return which;
 }

And for 2.6.24 we should just restore the two missing lines.

Quoted 7 lines

Unlike all the other namespaces, network will have to not just address different variables via same sysctl names, but to have different tables with different sysctl names. E.g. /proc/sys/net/conf have entries for devices, which differ across namespaces.

Quoted 5 lines

Eric currently have some work done in that directions, I like the approach in general very much, but it looks rather raw (Eric, take this in good part). You know, ifdefs in the middle of the code, explicit references to net namespace and so on and so forth.

Sure. I don't in principle have any problem with the set of roots we go through be dynamic at run time instead of compile time. That is probably a cleaner approach and likely to solve my problem with sched_debug registering sysctls before the sysctl subsystem is initialized.

One direction I have always intended to expand things when there was a bit of time to modify /proc/sys so that it is a symlink to /proc/self/sys. Then make /proc/<pid>/sys have cachable dentries for the sysctls.

To achieve that we need to pass our namespaces into your shadow function. Instead of always using current.

So I am thinking something like:

struct ctl_table_root {
	struct list_head ctl_entry;
	struct ctl_table_header *ctl_head;
	struct ctl_table_header *lookup_ctl_head(struct nsproxy *namespaces);
};

To actually handle the set of network devices in a namespace we need to have a list so making sysctl_head_next just loop over a list of lists should be no extra work and make implementing the users easier.

Beyond that through from my quick skim I have a preference for the way I am handling it.

We really need to add the tables with some variant of register_sysctl_table or else we will have module unload races.

Introducing register_sysctl_paths is a very useful cleanup in

it's own right and it helps quite a bit. In most cases it removes the need for your create_sysctl_shadow function, and it always reduced the amount of code for tables.

Further simply using kmemdup instead of a custom crafted function is a little more straight forward and is the idiom already established throughout the networking code.

Then we will just need a base sysctl function:
struct ctl_table_header * register_sysctl_rooted_paths(struct ctl_table_root *root,
struct nsproxy *namespaces, struct ctl_path *path, struct ctl_table *table)

For the simple namespaces we can call it once per namespace after registering our root (we can't use current because we initialize things before we update nsproxy), and we still need to run sysctl_check.

For the more complex namespaces (i.e. the network namespace). We can write a simple wrapper around register_sysctl_rooted_paths.

And of course non-namespace specific sysctls can just use a wrapper that assumes the default global list of sysctl_headers.

Quoted 2 lines

So here's the RFC for a bit better sysctls shadow management.

Quoted 4 lines

I will provide 3 patches:
1. the sysctl shadows themselves;
2. using shadows in UTS namespace;
3. using shadows in IPC namespace;

Your patches look fairly reasonable. Other then the pieces
I have mentioned already.

Quoted 6 lines

Using them in net namespace is already checked (I created sysctl entries with different names), but I don't have any patches against any David's tree yet. If we're OK with this set I will start talking to Andrew and David about who to send these patches to and making shadows for net-related sysctl variables.

I think we need another round. My hunch is that it will be easiest if David collects them up, and then Andrew updates his tree, but we will see.

Eric

To be very very very clear.

This is the way I think we should do the core sysctl infrastructure.

On top of the register_sysctl_table patch, getting all of the infrastructure in at once.

With a list of lists so we don't kill ourselves when we try to implement sysctls that are per network devices.

I'm not yet finished testing and reviewing the code yet but I think this is pretty close.

Quoted 1 line

From 7a0ab8b4d471fb1f2721150a5b1737a6e407b7b8 Mon Sep 17 00:00:00 2001

From: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
Date: Tue, 20 Nov 2007 07:51:50 -0700
Subject: [PATCH] sysctl: Infrastructure for per namespace sysctls

This patch implements the basic infrastructure for per namespace sysctls.

A list of lists of sysctl headers is added, allowing each namespace to have it's own list of sysctl headers.

Each list of sysctl headers has a lookup function to find the first sysctl header in the list, allowing the lists to have a per namespace instance.

register_sysct_root is added to tell sysctl.c about additional lists of lists. As all of the users are expected to be in kernel no unregister function is provided.

sysctl_head_next is updated to walk through the list of lists.

__reegister_sysctl_paths is added to add a new sysctl table on a non-default sysctl list.

Signed-off-by Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>

---
 include/linux/sysctl.h |   16 ++++++++-
 kernel/sysctl.c        |   92 +++++++++++++++++++++++++++++++++++++++++-------
 kernel/sysctl_check.c  |   25 +++++++------
 3 files changed, 108 insertions(+), 25 deletions(-)

include/linux/sysctl.h +15 -1

index 8b2e9e0..eeea2bb 100644
@@ -951,7 +951,9 @@ enum
 
 /* For the /proc/sys support */
 struct ctl_table;
+struct nsproxy;
 extern struct ctl_table_header *sysctl_head_next(struct ctl_table_header *prev);
+extern struct ctl_table_header *__sysctl_head_next(struct nsproxy *namespaces, struct ctl_table_header *prev);
 extern void sysctl_head_finish(struct ctl_table_header *prev);
 extern int sysctl_perm(struct ctl_table *table, int op);
 

@@ -1055,6 +1057,12 @@ struct ctl_table
 	void *extra2;
 };
 
+struct ctl_table_root {
+	struct list_head list;
+	struct ctl_table_header *ctl_header;
+	struct ctl_table_header *(*lookup)(struct nsproxy *namespaces);
+};
+
 /* struct ctl_table_header is used to maintain dynamic lists of
    struct ctl_table trees. */
 struct ctl_table_header

@@ -1064,6 +1072,7 @@ struct ctl_table_header
 	int used;
 	struct completion *unregistering;
 	struct ctl_table *ctl_table_arg;
+	struct ctl_table_root *root;
 };
 
 /* struct ctl_path describes where in the hierarchy a table is added */

@@ -1073,12 +1082,17 @@ struct ctl_path
 	int ctl_name;
 };
 
+void register_sysctl_root(struct ctl_table_root *root);
+struct ctl_table_header *__register_sysctl_paths(
+	struct ctl_table_root *root, struct nsproxy *namespaces,
+	const struct ctl_path *path, struct ctl_table *table);
 struct ctl_table_header *register_sysctl_table(struct ctl_table * table);
 struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
 						struct ctl_table *table);
 
 void unregister_sysctl_table(struct ctl_table_header * table);
-int sysctl_check_table(struct ctl_table *table);
+int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table);
+
 
 #else /* __KERNEL__ */
 

kernel/sysctl.c +79 -13

index ef023b5..d5e77ec 100644
@@ -156,8 +156,16 @@ static int proc_dointvec_taint(struct ctl_table *table, int write, struct file *
 #endif
 
 static struct ctl_table root_table[];
-static struct ctl_table_header root_table_header =
-	{ root_table, LIST_HEAD_INIT(root_table_header.ctl_entry) };
+static struct ctl_table_root sysctl_table_root;
+static struct ctl_table_header root_table_header = {
+	.ctl_table = root_table,
+	.ctl_entry = LIST_HEAD_INIT(root_table_header.ctl_entry),
+	.root = &sysctl_table_root,
+};
+static struct ctl_table_root sysctl_table_root = {
+	.list = LIST_HEAD_INIT(sysctl_table_root.list),
+	.ctl_header = &root_table_header,
+};
 
 static struct ctl_table kern_table[];
 static struct ctl_table vm_table[];

@@ -1300,10 +1308,13 @@ void sysctl_head_finish(struct ctl_table_header *head)
 	spin_unlock(&sysctl_lock);
 }
 
-struct ctl_table_header *sysctl_head_next(struct ctl_table_header *prev)
+struct ctl_table_header *__sysctl_head_next(struct nsproxy *namespaces,
+					    struct ctl_table_header *prev)
 {
-	struct ctl_table_header *head;
+	struct ctl_table_root *root;
+	struct ctl_table_header *head, *first;
 	struct list_head *tmp;
+
 	spin_lock(&sysctl_lock);
 	if (prev) {
 		tmp = &prev->ctl_entry;

@@ -1320,13 +1331,42 @@ struct ctl_table_header *sysctl_head_next(struct ctl_table_header *prev)
 		return head;
 	next:
 		tmp = tmp->next;
-		if (tmp == &root_table_header.ctl_entry)
-			break;
+		head = list_entry(tmp, struct ctl_table_header, ctl_entry);
+		root = head->root;
+		first = root->ctl_header;
+		if (root->lookup)
+			first = root->lookup(namespaces);
+
+		if (head == first) {
+		next_root:
+			root = list_entry(root->list.next,
+						struct ctl_table_root, list);
+			if (root == &sysctl_table_root)
+				break;
+			first = root->ctl_header;
+			if (root->lookup)
+				first = root->lookup(namespaces);
+			if (!first)
+				goto next_root;
+			tmp = &first->ctl_entry;
+		}
 	}
 	spin_unlock(&sysctl_lock);
 	return NULL;
 }
 
+struct ctl_table_header *sysctl_head_next(struct ctl_table_header *prev)
+{
+	return __sysctl_head_next(current->nsproxy, prev);
+}
+
+void register_sysctl_root(struct ctl_table_root *root)
+{
+	spin_lock(&sysctl_lock);
+	list_add_tail(&sysctl_table_root.list, &root->list);
+	spin_unlock(&sysctl_lock);
+}
+
 #ifdef CONFIG_SYSCTL_SYSCALL
 int do_sysctl(int __user *name, int nlen, void __user *oldval, size_t __user *oldlenp,
 	       void __user *newval, size_t newlen)

@@ -1483,14 +1523,16 @@ static __init int sysctl_init(void)
 {
 	int err;
 	sysctl_set_parent(NULL, root_table);
-	err = sysctl_check_table(root_table);
+	err = sysctl_check_table(current->nsproxy, root_table);
 	return 0;
 }
 
 core_initcall(sysctl_init);
 
 /**
- * register_sysctl_paths - register a sysctl hierarchy
+ * __register_sysctl_paths - register a sysctl hierarchy
+ * @root: List of sysctl headers to register on
+ * @namespaces: Data to compute which lists of sysctl entries are visible
  * @path: The path to the directory the sysctl table is in.
  * @table: the top-level table structure
  *

@@ -1558,10 +1600,12 @@ core_initcall(sysctl_init);
  * This routine returns %NULL on a failure to register, and a pointer
  * to the table header on success.
  */
-struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
-						struct ctl_table *table)
+struct ctl_table_header *__register_sysctl_paths(
+	struct ctl_table_root *root,
+	struct nsproxy *namespaces,
+	const struct ctl_path *path, struct ctl_table *table)
 {
-	struct ctl_table_header *header;
+	struct ctl_table_header *header, *first;
 	struct ctl_table *new, **prevp;
 	unsigned int n, npath;
 

@@ -1603,19 +1647,40 @@ struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
 	INIT_LIST_HEAD(&header->ctl_entry);
 	header->used = 0;
 	header->unregistering = NULL;
+	header->root = root;
 	sysctl_set_parent(NULL, header->ctl_table);
-	if (sysctl_check_table(header->ctl_table)) {
+	if (sysctl_check_table(namespaces, header->ctl_table)) {
 		kfree(header);
 		return NULL;
 	}
 	spin_lock(&sysctl_lock);
-	list_add_tail(&header->ctl_entry, &root_table_header.ctl_entry);
+	first = root->ctl_header;
+	if (root->lookup)
+		first = root->lookup(namespaces);
+	list_add_tail(&header->ctl_entry, &first->ctl_entry);
 	spin_unlock(&sysctl_lock);
 
 	return header;
 }
 
 /**
+ * register_sysctl_table_path - register a sysctl table hierarchy
+ * @path: The path to the directory the sysctl table is in.
+ * @table: the top-level table structure
+ *
+ * Register a sysctl table hierarchy. @table should be a filled in ctl_table
+ * array. A completely 0 filled entry terminates the table.
+ *
+ * See __register_sysctl_paths for more details.
+ */
+struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
+						struct ctl_table *table)
+{
+	return __register_sysctl_paths(&sysctl_table_root, current->nsproxy,
+					path, table);
+}
+
+/**
  * register_sysctl_table - register a sysctl table hierarchy
  * @table: the top-level table structure
  *

@@ -1648,6 +1713,7 @@ void unregister_sysctl_table(struct ctl_table_header * header)
 	kfree(header);
 }
 
+
 #else /* !CONFIG_SYSCTL */
 struct ctl_table_header *register_sysctl_table(struct ctl_table * table)
 {

kernel/sysctl_check.c +14 -11

index 5a2f2b2..9761561 100644
@@ -1383,7 +1383,8 @@ static void sysctl_repair_table(struct ctl_table *table)
 	}
 }
 
-static struct ctl_table *sysctl_check_lookup(struct ctl_table *table)
+static struct ctl_table *sysctl_check_lookup(struct nsproxy *namespaces,
+						struct ctl_table *table)
 {
 	struct ctl_table_header *head;
 	struct ctl_table *ref, *test;

@@ -1391,8 +1392,8 @@ static struct ctl_table *sysctl_check_lookup(struct ctl_table *table)
 
 	depth = sysctl_depth(table);
 
-	for (head = sysctl_head_next(NULL); head;
-	     head = sysctl_head_next(head)) {
+	for (head = __sysctl_head_next(namespaces, NULL); head;
+	     head = __sysctl_head_next(namespaces, head)) {
 		cur_depth = depth;
 		ref = head->ctl_table;
 repeat:

@@ -1437,13 +1438,14 @@ static void set_fail(const char **fail, struct ctl_table *table, const char *str
 	*fail = str;
 }
 
-static int sysctl_check_dir(struct ctl_table *table)
+static int sysctl_check_dir(struct nsproxy *namespaces,
+				struct ctl_table *table)
 {
 	struct ctl_table *ref;
 	int error;
 
 	error = 0;
-	ref = sysctl_check_lookup(table);
+	ref = sysctl_check_lookup(namespaces, table);
 	if (ref) {
 		int match = 0;
 		if ((!table->procname && !ref->procname) ||

@@ -1468,11 +1470,12 @@ static int sysctl_check_dir(struct ctl_table *table)
 	return error;
 }
 
-static void sysctl_check_leaf(struct ctl_table *table, const char **fail)
+static void sysctl_check_leaf(struct nsproxy *namespaces,
+				struct ctl_table *table, const char **fail)
 {
 	struct ctl_table *ref;
 
-	ref = sysctl_check_lookup(table);
+	ref = sysctl_check_lookup(namespaces, table);
 	if (ref && (ref != table))
 		set_fail(fail, table, "Sysctl already exists");
 }

@@ -1496,7 +1499,7 @@ static void sysctl_check_bin_path(struct ctl_table *table, const char **fail)
 	}
 }
 
-int sysctl_check_table(struct ctl_table *table)
+int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
 {
 	int error = 0;
 	for (; table->ctl_name || table->procname; table++) {

@@ -1526,7 +1529,7 @@ int sysctl_check_table(struct ctl_table *table)
 				set_fail(&fail, table, "Directory with extra1");
 			if (table->extra2)
 				set_fail(&fail, table, "Directory with extra2");
-			if (sysctl_check_dir(table))
+			if (sysctl_check_dir(namespaces, table))
 				set_fail(&fail, table, "Inconsistent directory names");
 		} else {
 			if ((table->strategy == sysctl_data) ||

@@ -1575,7 +1578,7 @@ int sysctl_check_table(struct ctl_table *table)
 			if (!table->procname && table->proc_handler)
 				set_fail(&fail, table, "proc_handler without procname");
 #endif
-			sysctl_check_leaf(table, &fail);
+			sysctl_check_leaf(namespaces, table, &fail);
 		}
 		sysctl_check_bin_path(table, &fail);
 		if (fail) {

@@ -1583,7 +1586,7 @@ int sysctl_check_table(struct ctl_table *table)
 			error = -EINVAL;
 		}
 		if (table->child)
-			error |= sysctl_check_table(table->child);
+			error |= sysctl_check_table(namespaces, table->child);
 	}
 	return error;
 }

Quoted 7 lines · Eric W. Biederman

To be very very very clear.

This is the way I think we should do the core sysctl infrastructure.

On top of the register_sysctl_table patch, getting all of the infrastructure in at once.

Quoted 2 lines

With a list of lists so we don't kill ourselves when we try to implement sysctls that are per network devices.

Hm... My patch looks to do very very same thing, but in a bit simpler manner. Except for the absence of the sysctl paths, but they are just cleanups. I think I can port them on top of my shadows :) Thanks

Quoted 8 lines · Pavel Emelyanov

Quoted 7 lines · Eric W. Biederman

To be very very very clear.

This is the way I think we should do the core sysctl infrastructure.

On top of the register_sysctl_table patch, getting all of the infrastructure in at once.

Quoted 6 lines

Quoted 2 lines

With a list of lists so we don't kill ourselves when we try to implement sysctls that are per network devices.

Hm... My patch looks to do very very same thing, but in a bit simpler manner. Except for the absence of the sysctl paths, but they are just cleanups. I think I can port them on top of my shadows :) Thanks

Then slow down and listen.

Your code simply does not address module unload races. BAD BAD BAD.

Your code skips sysctl_check allowing buggy sysctl tables to continue to exist BAD BAD BAD.

Your code does not implement a list of lists needed to implement a dynamic list of network devices, instead leaving it to the user of sysctl registration code to do. (Which is where the bulk of your simplicity seems to come from). BAD BAD BAD.

So I'm sorry. Your code is simpler because it is WORSE.

Your code is race prone, encourages buggy users, and doesn't even solve the same problem. So no I don't think it makes a bit of sense.

Eric

The user interface is: register_net_sysctl_table and unregister_net_sysctl_table. Very much like the current interface except there is a network namespace parameter.

With this any sysctl registered with register_net_sysctl_table will only show up to tasks in the same network namespace.

All other sysctls continue to be globally visible.

Signed-off-by Eric W. Biederman <[email protected]>

---
 include/net/net_namespace.h |    9 +++++++
 net/sysctl_net.c            |   57 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 0 deletions(-)

include/net/net_namespace.h +9 -0

index 4d0d634..235214c 100644
@@ -25,6 +25,8 @@ struct net {
 	struct proc_dir_entry 	*proc_net_stat;
 	struct proc_dir_entry 	*proc_net_root;
 
+	struct list_head	sysctl_table_headers;
+
 	struct net_device       *loopback_dev;          /* The loopback */
 
 	struct list_head 	dev_base_head;

@@ -144,4 +146,11 @@ extern void unregister_pernet_subsys(struct pernet_operations *);
 extern int register_pernet_device(struct pernet_operations *);
 extern void unregister_pernet_device(struct pernet_operations *);
 
+struct ctl_path;
+struct ctl_table;
+struct ctl_table_header;
+extern struct ctl_table_header *register_net_sysctl_table(struct net *net,
+	const struct ctl_path *path, struct ctl_table *table);
+extern void unregister_net_sysctl_table(struct ctl_table_header *header);
+
 #endif /* __NET_NET_NAMESPACE_H */

net/sysctl_net.c +39 -0

index cd4eafb..c50c793 100644
@@ -14,6 +14,7 @@
 
 #include <linux/mm.h>
 #include <linux/sysctl.h>
+#include <linux/nsproxy.h>
 
 #include <net/sock.h>
 

@@ -54,3 +55,59 @@ struct ctl_table net_table[] = {
 #endif
 	{ 0 },
 };
+
+static struct list_head *
+net_ctl_header_lookup(struct ctl_table_root *root, struct nsproxy *namespaces)
+{
+	return &namespaces->net_ns->sysctl_table_headers;
+}
+
+static struct ctl_table_root net_sysctl_root = {
+	.lookup = net_ctl_header_lookup,
+};
+
+static int sysctl_net_init(struct net *net)
+{
+	INIT_LIST_HEAD(&net->sysctl_table_headers);
+	return 0;
+}
+
+static void sysctl_net_exit(struct net *net)
+{
+	WARN_ON(!list_empty(&net->sysctl_table_headers));
+	return;
+}
+
+static struct pernet_operations sysctl_pernet_ops = {
+	.init = sysctl_net_init,
+	.exit = sysctl_net_exit,
+};
+
+static __init int sysctl_init(void)
+{
+	int ret;
+	ret = register_pernet_subsys(&sysctl_pernet_ops);
+	if (ret)
+		goto out;
+	register_sysctl_root(&net_sysctl_root);
+out:
+	return ret;
+}

net/sysctl_net.c +8 -0

+}
+EXPORT_SYMBOL_GPL(register_net_sysctl_table);
+
+void unregister_net_sysctl_table(struct ctl_table_header *header)
+{
+	return unregister_sysctl_table(header);
+}
+EXPORT_SYMBOL_GPL(unregister_net_sysctl_table);

Signature · 1 line

1.5.3.rc6.17.g1911

net/sysctl_net.c struct ctl_table net_table[] = { · line 105

Quoted 1 line

net/sysctl_net.c +1 -0

+					&namespaces, path, table);

Hey Eric,

the patches look nice.

The hand-forcing of the passed-in net_ns into a copy of current->nsproxy does make it seem like nsproxy may not be the best choice of what to pass in. Doesn't only net_sysctl_root->lookup() look at the argument?

But I assume you don't want to be more general than sending in a nsproxy so as to dissuade abuse of this interface for needlessly complex sysctl interfaces?

(Well I expect that'll become clear once the the patches using this come out.)

Are you planning to use this infrastructure for the uts and ipc sysctls as well?

thanks,
-serge

Hey Eric,

the patches look nice.

The hand-forcing of the passed-in net_ns into a copy of current->nsproxy does make it seem like nsproxy may not be the best choice of what to pass in. Doesn't only net_sysctl_root->lookup() look at the argument?

But I assume you don't want to be more general than sending in a nsproxy so as to dissuade abuse of this interface for needlessly complex sysctl interfaces?

(Well I expect that'll become clear once the the patches using this come out.)

Are you planning to use this infrastructure for the uts and ipc sysctls as well?

Quoted 8 lines · Serge E. Hallyn

Hey Eric,

the patches look nice.

The hand-forcing of the passed-in net_ns into a copy of current->nsproxy does make it seem like nsproxy may not be the best choice of what to pass in. Doesn't only net_sysctl_root->lookup() look at the argument?

Yes. Although I call it from __register_sysctl_paths.

Quoted 3 lines

But I assume you don't want to be more general than sending in a nsproxy so as to dissuade abuse of this interface for needlessly complex sysctl interfaces?

A bit of that. I would love to pass in a task_struct so you can use anything from a task. The trouble is I don't have any task_structs or nsproxys with the proper value at the point where I am first setting this up. Further I have to have the full sysctl lookup working or I could not call sysctl_check.

Quoted 5 lines

(Well I expect that'll become clear once the the patches using this come out.)

Are you planning to use this infrastructure for the uts and ipc sysctls as well?

Yes. Where it comes in especially useful, is I can move /proc/sys to /proc/sys/<tgid>/task/<pid>/sys. And get a particular processes view of sysctl.

We also get a little more reuse of common functions.

Otherwise Pavel does have a point that using this for uts and ipc is not a savings lines of code wise.

After having seen Pavel changes I am asking myself if there is a sane way to remove the ctl_name argument from the ctl_path.

Anyway where I am with the nsproxy question was that I don't see anything easily better. What I have works and gets the job done, and doesn't have any module unload races or holes where a sloppy programmer can mess up the sysctl tree. We needed a solution. Trying any harder to find something better would take ages. So I figured this implementation was good enough.

Herbert we need this infrastructure most in net-2.6.25 (as not having it is a current bottleneck to further development of the network namespace) so these patches are against net-2.6.25.

Quoted 4 lines · Eric W. Biederman

Herbert we need this infrastructure most in net-2.6.25 (as not having it is a current bottleneck to further development of the network namespace) so these patches are against net-2.6.25.

I've applied them all to net-2.6.25 with Andrew's fixes included. Thanks Eric.