linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/10] sysfs network namespace support
@ 2007-12-01  9:06 Eric W. Biederman
  2007-12-01  9:12 ` [PATCH 01/10] sysfs: Make sysfs_mount static again Eric W. Biederman
                   ` (2 more replies)
  0 siblings, 3 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:06 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


Now that we have network namespace support merged it is time to
revisit the sysfs support so we can remove the dependency on !SYSFS.

I'm not even trying to base this on any of Tejun's very interesting 
work on sysfs to remove the coupling between kobjects and
sysfs_dirents.  For my objective that just means I would need to
spend several more weeks staring at sysfs trying to figure out
how to get where I am going and iterating several times from yet
another new starting place.  I want to get something working before I
try for anymore perfection.

I don't expect the userspace side of this to ever change which is
close enough to perfect for me.

The bulk of the patches are the changes to allow multiple sysfs
superblocks.

Then comes the tagged directory sysfs support which uses information
captured at mount time to decide which object with which tag will
appear in a directory.

Then the support for renaming and deleting objects where the source
may be ambiguous because of tagging.

Then finally the network namespace support so it is clear how all
of this tied together.

Greg the last patch that enables tagged directory support seems
to make most sense living in your tree, as it lives half in
fs/sysfs/mount.c, and half in net/core/net-sysfs.c and all of
it's dependencies are in Linus tree except for this patchset.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH 01/10] sysfs: Make sysfs_mount static again.
  2007-12-01  9:06 [PATCH 0/10] sysfs network namespace support Eric W. Biederman
@ 2007-12-01  9:12 ` Eric W. Biederman
  2007-12-01  9:13   ` [PATCH 02/10] sysfs: Support for preventing unmounts Eric W. Biederman
  2007-12-01 13:10 ` namespace support requires network modules to say "GPL" Mark Lord
  2007-12-21  3:07 ` [PATCH 0/10] sysfs network namespace support Greg KH
  2 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


In preparation for supporting multiple mounts of sysfs I need to
remove all assumptions that we have a single mount of sysfs.  So this
patch modifies sysfs_open_file to use the vfsmount from the struct
file instead of fibbing and using the global vfsmount.

We get a little more noise this way but we should continue to get the
useful part of the debugging information.

This was the reason I made sysfs_mount static earlier.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/file.c  |    2 +-
 fs/sysfs/mount.c |    2 +-
 fs/sysfs/sysfs.h |    1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 87e4a0e..ad13151 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -330,7 +330,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
 	int error = -EACCES;
 	char *p;
 
-	p = d_path(file->f_dentry, sysfs_mount, last_sysfs_file,
+	p = d_path(file->f_dentry, file->f_vfsmnt, last_sysfs_file,
 		   sizeof(last_sysfs_file));
 	if (p)
 		memmove(last_sysfs_file, p, strlen(p) + 1);
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index a3410d6..7416826 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -22,7 +22,7 @@
 /* Random magic number */
 #define SYSFS_MAGIC 0x62656572
 
-struct vfsmount *sysfs_mount;
+static struct vfsmount *sysfs_mount;
 struct super_block * sysfs_sb = NULL;
 struct kmem_cache *sysfs_dir_cachep;
 
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 52aaa8c..ff17f8d 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -91,7 +91,6 @@ struct sysfs_addrm_cxt {
 extern struct sysfs_dirent sysfs_root;
 extern struct super_block *sysfs_sb;
 extern struct kmem_cache *sysfs_dir_cachep;
-extern struct vfsmount *sysfs_mount;
 
 /*
  * dir.c
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 02/10] sysfs: Support for preventing unmounts.
  2007-12-01  9:12 ` [PATCH 01/10] sysfs: Make sysfs_mount static again Eric W. Biederman
@ 2007-12-01  9:13   ` Eric W. Biederman
  2007-12-01  9:16     ` [PATCH 03/10] sysfs: sysfs_get_dentry add a sb parameter Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


To support mounting multiple instances of sysfs occassionally I
need to walk through all of the currently present sysfs super blocks.

To allow this iteration this patch adds sysfs_grab_supers and
sysfs_release_supers.  While a piece of code is in a section
surrounded by these no more sysfs super blocks will be either created
or destroyed.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/mount.c |   79 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 fs/sysfs/sysfs.h |   10 +++++++
 2 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index 7416826..ef5f7ae 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -41,47 +41,110 @@ struct sysfs_dirent sysfs_root = {
 
 static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
 {
-	struct inode *inode;
-	struct dentry *root;
+	struct sysfs_super_info *info = NULL;
+	struct inode *inode = NULL;
+	struct dentry *root = NULL;
+	int error;
 
 	sb->s_blocksize = PAGE_CACHE_SIZE;
 	sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
 	sb->s_magic = SYSFS_MAGIC;
 	sb->s_op = &sysfs_ops;
 	sb->s_time_gran = 1;
-	sysfs_sb = sb;
+	if (!sysfs_sb)
+		sysfs_sb = sb;
+
+	error = -ENOMEM;
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		goto out_err;
 
 	/* get root inode, initialize and unlock it */
+	error = -ENOMEM;
 	inode = sysfs_get_inode(&sysfs_root);
 	if (!inode) {
 		pr_debug("sysfs: could not get root inode\n");
-		return -ENOMEM;
+		goto out_err;
 	}
 
 	/* instantiate and link root dentry */
+	error = -ENOMEM;
 	root = d_alloc_root(inode);
 	if (!root) {
 		pr_debug("%s: could not get root dentry!\n",__FUNCTION__);
-		iput(inode);
-		return -ENOMEM;
+		goto out_err;
 	}
 	root->d_fsdata = &sysfs_root;
 	sb->s_root = root;
+	sb->s_fs_info = info;
 	return 0;
+
+out_err:
+	dput(root);
+	iput(inode);
+	kfree(info);
+	if (sysfs_sb == sb)
+		sysfs_sb = NULL;
+	return error;
 }
 
 static int sysfs_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
 {
-	return get_sb_single(fs_type, flags, data, sysfs_fill_super, mnt);
+	int rc;
+	mutex_lock(&sysfs_rename_mutex);
+	rc = get_sb_single(fs_type, flags, data, sysfs_fill_super, mnt);
+	mutex_unlock(&sysfs_rename_mutex);
+	return rc;
 }
 
-static struct file_system_type sysfs_fs_type = {
+struct file_system_type sysfs_fs_type = {
 	.name		= "sysfs",
 	.get_sb		= sysfs_get_sb,
 	.kill_sb	= kill_anon_super,
 };
 
+void sysfs_grab_supers(void)
+{
+	/* must hold sysfs_rename_mutex */
+	struct super_block *sb;
+	/* Loop until I have taken s_umount on all sysfs superblocks */
+restart:
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
+		if (sysfs_info(sb)->grabbed)
+			continue;
+		/* Wait for unmount activity to complete. */
+		if (sb->s_count < S_BIAS) {
+			sb->s_count += 1;
+			spin_unlock(&sb_lock);
+			down_read(&sb->s_umount);
+			drop_super(sb);
+			goto restart;
+		}
+		atomic_inc(&sb->s_active);
+		sysfs_info(sb)->grabbed = 1;
+	}
+	spin_unlock(&sb_lock);
+}
+
+void sysfs_release_supers(void)
+{
+	/* must hold sysfs_rename_mutex */
+	struct super_block *sb;
+restart:
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
+		if (!sysfs_info(sb)->grabbed)
+			continue;
+		sysfs_info(sb)->grabbed = 0;
+		spin_unlock(&sb_lock);
+		deactivate_super(sb);
+		goto restart;
+	}
+	spin_unlock(&sb_lock);
+}
+
 int __init sysfs_init(void)
 {
 	int err = -ENOMEM;
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index ff17f8d..3308759 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -85,12 +85,22 @@ struct sysfs_addrm_cxt {
 	int			cnt;
 };
 
+struct sysfs_super_info {
+	int	grabbed;
+};
+
+#define sysfs_info(SB) ((struct sysfs_super_info *)(SB)->s_fs_info)
+
 /*
  * mount.c
  */
 extern struct sysfs_dirent sysfs_root;
 extern struct super_block *sysfs_sb;
 extern struct kmem_cache *sysfs_dir_cachep;
+extern struct file_system_type sysfs_fs_type;
+
+void sysfs_grab_supers(void);
+void sysfs_release_supers(void);
 
 /*
  * dir.c
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 03/10] sysfs: sysfs_get_dentry add a sb parameter
  2007-12-01  9:13   ` [PATCH 02/10] sysfs: Support for preventing unmounts Eric W. Biederman
@ 2007-12-01  9:16     ` Eric W. Biederman
  2007-12-01  9:18       ` [PATCH 04/10] sysfs: Implement __sysfs_get_dentry Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


In preparation for multiple mounts of sysfs add a superblock parameter to
sysfs_get_dentry.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/dir.c   |   11 ++++++-----
 fs/sysfs/file.c  |    2 +-
 fs/sysfs/sysfs.h |    2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 3371629..cff2b12 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -84,6 +84,7 @@ static void sysfs_unlink_sibling(struct sysfs_dirent *sd)
 
 /**
  *	sysfs_get_dentry - get dentry for the given sysfs_dirent
+ *	@sb: superblock of the dentry to return
  *	@sd: sysfs_dirent of interest
  *
  *	Get dentry for @sd.  Dentry is looked up if currently not
@@ -96,9 +97,9 @@ static void sysfs_unlink_sibling(struct sysfs_dirent *sd)
  *	RETURNS:
  *	Pointer to found dentry on success, ERR_PTR() value on error.
  */
-struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd)
+struct dentry *sysfs_get_dentry(struct super_block *sb, struct sysfs_dirent *sd)
 {
-	struct dentry *dentry = dget(sysfs_sb->s_root);
+	struct dentry *dentry = dget(sb->s_root);
 
 	while (dentry->d_fsdata != sd) {
 		struct sysfs_dirent *cur;
@@ -778,7 +779,7 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 		goto out;	/* nothing to rename */
 
 	/* get the original dentry */
-	old_dentry = sysfs_get_dentry(sd);
+	old_dentry = sysfs_get_dentry(sysfs_sb, sd);
 	if (IS_ERR(old_dentry)) {
 		error = PTR_ERR(old_dentry);
 		goto out;
@@ -845,14 +846,14 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)
 		goto out;	/* nothing to move */
 
 	/* get dentries */
-	old_dentry = sysfs_get_dentry(sd);
+	old_dentry = sysfs_get_dentry(sysfs_sb, sd);
 	if (IS_ERR(old_dentry)) {
 		error = PTR_ERR(old_dentry);
 		goto out;
 	}
 	old_parent = old_dentry->d_parent;
 
-	new_parent = sysfs_get_dentry(new_parent_sd);
+	new_parent = sysfs_get_dentry(sysfs_sb, new_parent_sd);
 	if (IS_ERR(new_parent)) {
 		error = PTR_ERR(new_parent);
 		goto out;
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index ad13151..8c7bba0 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -569,7 +569,7 @@ int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode)
 		goto out;
 
 	mutex_lock(&sysfs_rename_mutex);
-	victim = sysfs_get_dentry(victim_sd);
+	victim = sysfs_get_dentry(sysfs_sb, victim_sd);
 	mutex_unlock(&sysfs_rename_mutex);
 	if (IS_ERR(victim)) {
 		rc = PTR_ERR(victim);
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 3308759..d4269ba 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -112,7 +112,7 @@ extern spinlock_t sysfs_assoc_lock;
 extern const struct file_operations sysfs_dir_operations;
 extern const struct inode_operations sysfs_dir_inode_operations;
 
-struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd);
+struct dentry *sysfs_get_dentry(struct super_block *sb, struct sysfs_dirent *sd);
 struct sysfs_dirent *sysfs_get_active_two(struct sysfs_dirent *sd);
 void sysfs_put_active_two(struct sysfs_dirent *sd);
 void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 04/10] sysfs: Implement __sysfs_get_dentry
  2007-12-01  9:16     ` [PATCH 03/10] sysfs: sysfs_get_dentry add a sb parameter Eric W. Biederman
@ 2007-12-01  9:18       ` Eric W. Biederman
  2007-12-01  9:23         ` [PATCH 05/10] sysfs: Rename Support multiple superblocks Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


This function is similar but much simpler to sysfs_get_dentry
returns a sysfs dentry if one curently exists.

This requires less locking the sysfs_get_dentry and which
makes it preferable in some contexts.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/dir.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index cff2b12..3ec9040 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -764,6 +764,44 @@ void sysfs_remove_dir(struct kobject * kobj)
 	__sysfs_remove_dir(sd);
 }
 
+/**
+ *	__sysfs_get_dentry - get dentry for the given sysfs_dirent
+ *	@sb: superblock of the dentry to return
+ *	@sd: sysfs_dirent of interest
+ *
+ *	Get dentry for @sd.  Only return a dentry if one currently
+ *	exists.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:
+ *	Pointer to found dentry on success, NULL on failure.
+ */
+static struct dentry *__sysfs_get_dentry(struct super_block *sb, struct sysfs_dirent *sd)
+{
+	struct inode *inode;
+	struct dentry *dentry = NULL;
+
+	inode = ilookup5_nowait(sysfs_sb, sd->s_ino, sysfs_ilookup_test, sd);
+	if (inode && !(inode->i_state & I_NEW)) {
+		struct dentry *alias;
+		spin_lock(&dcache_lock);
+		list_for_each_entry(alias, &inode->i_dentry, d_alias) {
+			if (!IS_ROOT(alias) && d_unhashed(alias))
+				continue;
+			if (alias->d_sb != sb)
+				continue;
+			dentry = alias;
+			dget_locked(dentry);
+			break;
+		}
+		spin_unlock(&dcache_lock);
+	}
+	iput(inode);
+	return dentry;
+}
+
 int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 {
 	struct sysfs_dirent *sd = kobj->sd;
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 05/10] sysfs: Rename Support multiple superblocks
  2007-12-01  9:18       ` [PATCH 04/10] sysfs: Implement __sysfs_get_dentry Eric W. Biederman
@ 2007-12-01  9:23         ` Eric W. Biederman
  2007-12-01  9:25           ` [PATCH 06/10] sysfs: sysfs_chmod_file handle " Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


This patch modifies the sysfs_rename_dir and sysfs_move_dir
to support multiple sysfs dentry trees rooted in different
sysfs superblocks.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/dir.c |  190 +++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 135 insertions(+), 55 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 3ec9040..0d0c87e 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -802,42 +802,112 @@ static struct dentry *__sysfs_get_dentry(struct super_block *sb, struct sysfs_di
 	return dentry;
 }
 
+struct sysfs_rename_struct {
+	struct list_head list;
+	struct dentry *old_dentry;
+	struct dentry *new_dentry;
+	struct dentry *old_parent;
+	struct dentry *new_parent;
+};
+
+static void post_rename(struct list_head *head)
+{
+	struct sysfs_rename_struct *srs;
+	while (!list_empty(head)) {
+		srs = list_entry(head->next, struct sysfs_rename_struct, list);
+		dput(srs->old_dentry);
+		dput(srs->new_dentry);
+		dput(srs->old_parent);
+		dput(srs->new_parent);
+		list_del(&srs->list);
+		kfree(srs);
+	}
+}
+
+static int prep_rename(struct list_head *head,
+	struct sysfs_dirent *sd, struct sysfs_dirent *new_parent_sd,
+	const char *name)
+{
+	struct sysfs_rename_struct *srs;
+	struct super_block *sb;
+	struct dentry *dentry;
+	int error;
+
+	list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
+		dentry = sysfs_get_dentry(sb, sd);
+		if (dentry == ERR_PTR(-EXDEV))
+			continue;
+		if (IS_ERR(dentry)) {
+			error = PTR_ERR(dentry);
+			goto err_out;
+		}
+
+		srs = kzalloc(sizeof(*srs), GFP_KERNEL);
+		if (!srs) {
+			dput(dentry);
+			goto err_out;
+		}
+
+		INIT_LIST_HEAD(&srs->list);
+		list_add(head, &srs->list);
+		srs->old_dentry = dentry;
+		srs->old_parent = dget(dentry->d_parent);
+
+		dentry = sysfs_get_dentry(sb, new_parent_sd);
+		if (IS_ERR(dentry)) {
+			error = PTR_ERR(dentry);
+			goto err_out;
+		}
+		srs->new_parent = dentry;
+
+		error = -ENOMEM;
+		dentry = d_alloc_name(srs->new_parent, name);
+		if (!dentry)
+			goto err_out;
+		srs->new_dentry = dentry;
+	}
+	return 0;
+
+err_out:
+	post_rename(head);
+	return error;
+}
+
 int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 {
 	struct sysfs_dirent *sd = kobj->sd;
-	struct dentry *parent = NULL;
-	struct dentry *old_dentry = NULL, *new_dentry = NULL;
+	struct list_head todo;
+	struct sysfs_rename_struct *srs;
+	struct inode *parent_inode = NULL;
 	const char *dup_name = NULL;
 	int error;
 
+	INIT_LIST_HEAD(&todo);
 	mutex_lock(&sysfs_rename_mutex);
 
 	error = 0;
 	if (strcmp(sd->s_name, new_name) == 0)
 		goto out;	/* nothing to rename */
 
-	/* get the original dentry */
-	old_dentry = sysfs_get_dentry(sysfs_sb, sd);
-	if (IS_ERR(old_dentry)) {
-		error = PTR_ERR(old_dentry);
-		goto out;
-	}
+	sysfs_grab_supers();
+	error = prep_rename(&todo, sd, sd->s_parent, new_name);
+	if (error)
+		goto out_release;
 
-	parent = old_dentry->d_parent;
+	error = -ENOMEM;
+	mutex_lock(&sysfs_mutex);
+	parent_inode = sysfs_get_inode(sd->s_parent);
+	mutex_unlock(&sysfs_mutex);
+	if (!parent_inode)
+		goto out_release;
 
-	/* lock parent and get dentry for new name */
-	mutex_lock(&parent->d_inode->i_mutex);
+	mutex_lock(&parent_inode->i_mutex);
 	mutex_lock(&sysfs_mutex);
 
 	error = -EEXIST;
 	if (sysfs_find_dirent(sd->s_parent, new_name))
 		goto out_unlock;
 
-	error = -ENOMEM;
-	new_dentry = d_alloc_name(parent, new_name);
-	if (!new_dentry)
-		goto out_unlock;
-
 	/* rename kobject and sysfs_dirent */
 	error = -ENOMEM;
 	new_name = dup_name = kstrdup(new_name, GFP_KERNEL);
@@ -852,17 +922,21 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 	sd->s_name = new_name;
 
 	/* rename */
-	d_add(new_dentry, NULL);
-	d_move(old_dentry, new_dentry);
+	list_for_each_entry(srs, &todo, list) {
+		d_add(srs->new_dentry, NULL);
+		d_move(srs->old_dentry, srs->new_dentry);
+	}
 
 	error = 0;
- out_unlock:
+out_unlock:
 	mutex_unlock(&sysfs_mutex);
-	mutex_unlock(&parent->d_inode->i_mutex);
+	mutex_unlock(&parent_inode->i_mutex);
 	kfree(dup_name);
-	dput(old_dentry);
-	dput(new_dentry);
- out:
+out_release:
+	iput(parent_inode);
+	post_rename(&todo);
+	sysfs_release_supers();
+out:
 	mutex_unlock(&sysfs_rename_mutex);
 	return error;
 }
@@ -871,10 +945,12 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)
 {
 	struct sysfs_dirent *sd = kobj->sd;
 	struct sysfs_dirent *new_parent_sd;
-	struct dentry *old_parent, *new_parent = NULL;
-	struct dentry *old_dentry = NULL, *new_dentry = NULL;
+	struct list_head todo;
+	struct sysfs_rename_struct *srs;
+	struct inode *old_parent_inode = NULL, *new_parent_inode = NULL;
 	int error;
 
+	INIT_LIST_HEAD(&todo);
 	mutex_lock(&sysfs_rename_mutex);
 	BUG_ON(!sd->s_parent);
 	new_parent_sd = new_parent_kobj->sd ? new_parent_kobj->sd : &sysfs_root;
@@ -883,24 +959,29 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)
 	if (sd->s_parent == new_parent_sd)
 		goto out;	/* nothing to move */
 
-	/* get dentries */
-	old_dentry = sysfs_get_dentry(sysfs_sb, sd);
-	if (IS_ERR(old_dentry)) {
-		error = PTR_ERR(old_dentry);
-		goto out;
-	}
-	old_parent = old_dentry->d_parent;
+	sysfs_grab_supers();
+	error = prep_rename(&todo, sd, new_parent_sd, sd->s_name);
+	if (error)
+		goto out_release;
 
-	new_parent = sysfs_get_dentry(sysfs_sb, new_parent_sd);
-	if (IS_ERR(new_parent)) {
-		error = PTR_ERR(new_parent);
-		goto out;
-	}
+	error = -ENOMEM;
+	mutex_lock(&sysfs_mutex);
+	old_parent_inode = sysfs_get_inode(sd->s_parent);
+	mutex_unlock(&sysfs_mutex);
+	if (!old_parent_inode)
+		goto out_release;
+
+	error = -ENOMEM;
+	mutex_lock(&sysfs_mutex);
+	new_parent_inode = sysfs_get_inode(new_parent_sd);
+	mutex_unlock(&sysfs_mutex);
+	if (!new_parent_inode)
+		goto out_release;
 
 again:
-	mutex_lock(&old_parent->d_inode->i_mutex);
-	if (!mutex_trylock(&new_parent->d_inode->i_mutex)) {
-		mutex_unlock(&old_parent->d_inode->i_mutex);
+	mutex_lock(&old_parent_inode->i_mutex);
+	if (!mutex_trylock(&new_parent_inode->i_mutex)) {
+		mutex_unlock(&old_parent_inode->i_mutex);
 		goto again;
 	}
 	mutex_lock(&sysfs_mutex);
@@ -909,15 +990,11 @@ again:
 	if (sysfs_find_dirent(new_parent_sd, sd->s_name))
 		goto out_unlock;
 
-	error = -ENOMEM;
-	new_dentry = d_alloc_name(new_parent, sd->s_name);
-	if (!new_dentry)
-		goto out_unlock;
-
 	error = 0;
-	d_add(new_dentry, NULL);
-	d_move(old_dentry, new_dentry);
-	dput(new_dentry);
+	list_for_each_entry(srs, &todo, list) {
+		d_add(srs->new_dentry, NULL);
+		d_move(srs->old_dentry, srs->new_dentry);
+	}
 
 	/* Remove from old parent's list and insert into new parent's list. */
 	sysfs_unlink_sibling(sd);
@@ -926,14 +1003,17 @@ again:
 	sd->s_parent = new_parent_sd;
 	sysfs_link_sibling(sd);
 
- out_unlock:
+out_unlock:
 	mutex_unlock(&sysfs_mutex);
-	mutex_unlock(&new_parent->d_inode->i_mutex);
-	mutex_unlock(&old_parent->d_inode->i_mutex);
- out:
-	dput(new_parent);
-	dput(old_dentry);
-	dput(new_dentry);
+	mutex_unlock(&new_parent_inode->i_mutex);
+	mutex_unlock(&old_parent_inode->i_mutex);
+
+out_release:
+	iput(new_parent_inode);
+	iput(old_parent_inode);
+	post_rename(&todo);
+	sysfs_release_supers();
+out:
 	mutex_unlock(&sysfs_rename_mutex);
 	return error;
 }
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 06/10] sysfs: sysfs_chmod_file handle multiple superblocks
  2007-12-01  9:23         ` [PATCH 05/10] sysfs: Rename Support multiple superblocks Eric W. Biederman
@ 2007-12-01  9:25           ` Eric W. Biederman
  2007-12-01  9:28             ` [PATCH 07/10] sysfs: Implement sysfs tagged directory support Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


Teach sysfs_chmod_file how to handle multiple sysfs superblocks.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/file.c |   51 ++++++++++++++++++++++++++++-----------------------
 1 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 8c7bba0..ade6140 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -558,7 +558,8 @@ EXPORT_SYMBOL_GPL(sysfs_add_file_to_group);
 int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode)
 {
 	struct sysfs_dirent *victim_sd = NULL;
-	struct dentry *victim = NULL;
+	struct super_block *sb;
+	struct dentry *victim;
 	struct inode * inode;
 	struct iattr newattrs;
 	int rc;
@@ -569,31 +570,35 @@ int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode)
 		goto out;
 
 	mutex_lock(&sysfs_rename_mutex);
-	victim = sysfs_get_dentry(sysfs_sb, victim_sd);
-	mutex_unlock(&sysfs_rename_mutex);
-	if (IS_ERR(victim)) {
-		rc = PTR_ERR(victim);
-		victim = NULL;
-		goto out;
-	}
-
-	inode = victim->d_inode;
-
-	mutex_lock(&inode->i_mutex);
+	sysfs_grab_supers();
+	list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
+		victim = sysfs_get_dentry(sb, victim_sd);
+		if (victim == ERR_PTR(-EXDEV))
+			continue;
+		if (IS_ERR(victim)) {
+			rc = PTR_ERR(victim);
+			victim = NULL;
+			goto out_unlock;
+		}
 
-	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
-	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
-	rc = notify_change(victim, &newattrs);
+		inode = victim->d_inode;
+		mutex_lock(&inode->i_mutex);
+		newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
+		newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
+		rc = notify_change(victim, &newattrs);
+		if (rc == 0) {
+			mutex_lock(&sysfs_mutex);
+			victim_sd->s_mode = newattrs.ia_mode;
+			mutex_unlock(&sysfs_mutex);
+		}
+		mutex_unlock(&inode->i_mutex);
 
-	if (rc == 0) {
-		mutex_lock(&sysfs_mutex);
-		victim_sd->s_mode = newattrs.ia_mode;
-		mutex_unlock(&sysfs_mutex);
+		dput(victim);
 	}
-
-	mutex_unlock(&inode->i_mutex);
- out:
-	dput(victim);
+out_unlock:
+	sysfs_release_supers();
+	mutex_unlock(&sysfs_rename_mutex);
+out:
 	sysfs_put(victim_sd);
 	return rc;
 }
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 07/10] sysfs: Implement sysfs tagged directory support.
  2007-12-01  9:25           ` [PATCH 06/10] sysfs: sysfs_chmod_file handle " Eric W. Biederman
@ 2007-12-01  9:28             ` Eric W. Biederman
  2007-12-01  9:30               ` [PATCH 08/10] sysfs: Implement sysfs_delete_link and sysfs_rename_link Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


The problem.  When implementing a network namespace I need to be able
to have multiple network devices with the same name.  Currently this
is a problem for /sys/class/net/*, /sys/devices/virtual/net/*, and
potentially a few other directories of the form /sys/ ... /net/*.

What this patch does is to add an additional tag field to the
sysfs dirent structure.  For directories that should show different
contents depending on the context such as /sys/class/net/, and
/sys/devices/virtual/net/ this tag field is used to specify the
context in which those directories should be visible.  Effectively
this is the same as creating multiple distinct directories with
the same name the internally to sysfs the result is nicer.

I am calling the concept of a single directory that looks like multiple
directories all at the same path in the filesystem tagged directories.

For the networking namespace the set of directories whose contents I need
to filter with tags can depend on the presence or absence of hotplug
hardware or which modules are currently loaded.  Which means I need
a simple race free way to setup those directories as tagged.

To achieve a race free design all tagged directories are created
and managed by sysfs itself.  The upper level code that knows what
tagged directories we need provides just two methods that enable
this:
  sb_tag() - that returns a "void *" tag that identifies the context of
	the process that mounted sysfs.
  kobject_tag(kobj) - that returns a "void *" tag that identifies the context
	a kobject should be in.
Everything else is left up to sysfs.

For the network namespace sb_tag and kobject_tag are essentially
one line functions, and look to remain that.

The work needed in sysfs is more extensive.  At each directory
or symlink creating I need to check if the directory it is being
created in is a tagged directory and if so generate the appropriate
tag to place on the sysfs_dirent.  Likewise at each symlink or
directory removal I need to check if the sysfs directory it is
being removed from is a tagged directory and if so figure out
which tag goes along with the name I am deleting.

Currently only directories which hold kobjects, and
symlinks are supported.  There is not enough information
in the current file attribute interfaces to give us anything
to discriminate on which makes it useless, and there are
no potential users which makes it an uninteresting problem
to solve.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/bin.c        |    2 +-
 fs/sysfs/dir.c        |  182 +++++++++++++++++++++++++++++++++++++++++++++----
 fs/sysfs/file.c       |    8 +-
 fs/sysfs/group.c      |   12 ++--
 fs/sysfs/inode.c      |    6 +-
 fs/sysfs/mount.c      |   44 +++++++++++-
 fs/sysfs/symlink.c    |    2 +-
 fs/sysfs/sysfs.h      |   16 ++++-
 include/linux/sysfs.h |   16 ++++
 9 files changed, 255 insertions(+), 33 deletions(-)

diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index 006fc64..86e1128 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -252,7 +252,7 @@ int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 
 void sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 {
-	sysfs_hash_and_remove(kobj->sd, attr->attr.name);
+	sysfs_hash_and_remove(kobj, kobj->sd, attr->attr.name);
 }
 
 EXPORT_SYMBOL_GPL(sysfs_create_bin_file);
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 0d0c87e..f4bd41a 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -99,8 +99,17 @@ static void sysfs_unlink_sibling(struct sysfs_dirent *sd)
  */
 struct dentry *sysfs_get_dentry(struct super_block *sb, struct sysfs_dirent *sd)
 {
-	struct dentry *dentry = dget(sb->s_root);
+	struct dentry *dentry;
+
+	/* Bail if this sd won't show up in this superblock */
+	if (sd->s_parent && sd->s_parent->s_flags & SYSFS_FLAG_TAGGED) {
+		const void *tag;
+		tag = sysfs_lookup_tag(sd->s_parent, sb);
+		if (sd->s_tag.tag != tag)
+			return ERR_PTR(-EXDEV);
+	}
 
+	dentry = dget(sb->s_root);
 	while (dentry->d_fsdata != sd) {
 		struct sysfs_dirent *cur;
 		struct dentry *parent;
@@ -419,7 +428,11 @@ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
  */
 int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
 {
-	if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) {
+	const void *tag = NULL;
+
+	tag = sysfs_creation_tag(acxt->parent_sd, sd);
+
+	if (sysfs_find_dirent(acxt->parent_sd, tag, sd->s_name)) {
 		printk(KERN_WARNING "sysfs: duplicate filename '%s' "
 		       "can not be created\n", sd->s_name);
 		WARN_ON(1);
@@ -428,6 +441,9 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
 
 	sd->s_parent = sysfs_get(acxt->parent_sd);
 
+	if (sd->s_parent->s_flags & SYSFS_FLAG_TAGGED)
+		sd->s_tag.tag = tag;
+
 	if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode)
 		inc_nlink(acxt->parent_inode);
 
@@ -574,13 +590,18 @@ void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt)
  *	Pointer to sysfs_dirent if found, NULL if not.
  */
 struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,
+				       const void *tag,
 				       const unsigned char *name)
 {
 	struct sysfs_dirent *sd;
 
-	for (sd = parent_sd->s_dir.children; sd; sd = sd->s_sibling)
+	for (sd = parent_sd->s_dir.children; sd; sd = sd->s_sibling) {
+		if ((parent_sd->s_flags & SYSFS_FLAG_TAGGED) &&
+		    (sd->s_tag.tag != tag))
+			continue;
 		if (!strcmp(sd->s_name, name))
 			return sd;
+	}
 	return NULL;
 }
 
@@ -604,7 +625,7 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
 	struct sysfs_dirent *sd;
 
 	mutex_lock(&sysfs_mutex);
-	sd = sysfs_find_dirent(parent_sd, name);
+	sd = sysfs_find_dirent(parent_sd, NULL, name);
 	sysfs_get(sd);
 	mutex_unlock(&sysfs_mutex);
 
@@ -670,13 +691,16 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 				struct nameidata *nd)
 {
 	struct dentry *ret = NULL;
-	struct sysfs_dirent *parent_sd = dentry->d_parent->d_fsdata;
+	struct dentry *parent = dentry->d_parent;
+	struct sysfs_dirent *parent_sd = parent->d_fsdata;
 	struct sysfs_dirent *sd;
 	struct inode *inode;
+	const void *tag;
 
 	mutex_lock(&sysfs_mutex);
 
-	sd = sysfs_find_dirent(parent_sd, dentry->d_name.name);
+	tag = sysfs_lookup_tag(parent_sd, parent->d_sb);
+	sd = sysfs_find_dirent(parent_sd, tag, dentry->d_name.name);
 
 	/* no such entry */
 	if (!sd)
@@ -880,19 +904,24 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 	struct sysfs_rename_struct *srs;
 	struct inode *parent_inode = NULL;
 	const char *dup_name = NULL;
+	const void *old_tag, *tag;
 	int error;
 
 	INIT_LIST_HEAD(&todo);
 	mutex_lock(&sysfs_rename_mutex);
+	old_tag = sysfs_dirent_tag(sd);
+	tag = sysfs_creation_tag(sd->s_parent, sd);
 
 	error = 0;
-	if (strcmp(sd->s_name, new_name) == 0)
+	if ((old_tag == tag) && (strcmp(sd->s_name, new_name) == 0))
 		goto out;	/* nothing to rename */
 
 	sysfs_grab_supers();
-	error = prep_rename(&todo, sd, sd->s_parent, new_name);
-	if (error)
-		goto out_release;
+	if (old_tag == tag) {
+		error = prep_rename(&todo, sd, sd->s_parent, new_name);
+		if (error)
+			goto out_release;
+	}
 
 	error = -ENOMEM;
 	mutex_lock(&sysfs_mutex);
@@ -905,7 +934,7 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 	mutex_lock(&sysfs_mutex);
 
 	error = -EEXIST;
-	if (sysfs_find_dirent(sd->s_parent, new_name))
+	if (sysfs_find_dirent(sd->s_parent, tag, new_name))
 		goto out_unlock;
 
 	/* rename kobject and sysfs_dirent */
@@ -920,6 +949,8 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 
 	dup_name = sd->s_name;
 	sd->s_name = new_name;
+	if (sd->s_parent->s_flags & SYSFS_FLAG_TAGGED)
+		sd->s_tag.tag = tag;
 
 	/* rename */
 	list_for_each_entry(srs, &todo, list) {
@@ -927,6 +958,20 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
 		d_move(srs->old_dentry, srs->new_dentry);
 	}
 
+	/* If we are moving across superblocks drop the dcache entries */
+	if (old_tag != tag) {
+		struct super_block *sb;
+		struct dentry *dentry;
+		list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
+			dentry = __sysfs_get_dentry(sb, sd);
+			if (!dentry)
+				continue;
+			shrink_dcache_parent(dentry);
+			d_drop(dentry);
+			dput(dentry);
+		}
+	}
+
 	error = 0;
 out_unlock:
 	mutex_unlock(&sysfs_mutex);
@@ -949,11 +994,13 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj)
 	struct sysfs_rename_struct *srs;
 	struct inode *old_parent_inode = NULL, *new_parent_inode = NULL;
 	int error;
+	const void *tag;
 
 	INIT_LIST_HEAD(&todo);
 	mutex_lock(&sysfs_rename_mutex);
 	BUG_ON(!sd->s_parent);
 	new_parent_sd = new_parent_kobj->sd ? new_parent_kobj->sd : &sysfs_root;
+	tag = sysfs_dirent_tag(sd);
 
 	error = 0;
 	if (sd->s_parent == new_parent_sd)
@@ -987,7 +1034,7 @@ again:
 	mutex_lock(&sysfs_mutex);
 
 	error = -EEXIST;
-	if (sysfs_find_dirent(new_parent_sd, sd->s_name))
+	if (sysfs_find_dirent(new_parent_sd, tag, sd->s_name))
 		goto out_unlock;
 
 	error = 0;
@@ -1026,10 +1073,11 @@ static inline unsigned char dt_type(struct sysfs_dirent *sd)
 
 static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 {
-	struct dentry *dentry = filp->f_path.dentry;
-	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+	struct dentry *parent = filp->f_path.dentry;
+	struct sysfs_dirent * parent_sd = parent->d_fsdata;
 	struct sysfs_dirent *pos;
 	ino_t ino;
+	const void *tag;
 
 	if (filp->f_pos == 0) {
 		ino = parent_sd->s_ino;
@@ -1047,6 +1095,8 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 	if ((filp->f_pos > 1) && (filp->f_pos < INT_MAX)) {
 		mutex_lock(&sysfs_mutex);
 
+		tag = sysfs_lookup_tag(parent_sd, parent->d_sb);
+
 		/* Skip the dentries we have already reported */
 		pos = parent_sd->s_dir.children;
 		while (pos && (filp->f_pos > pos->s_ino))
@@ -1056,6 +1106,10 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
 			const char * name;
 			int len;
 
+			if ((parent_sd->s_flags & SYSFS_FLAG_TAGGED) &&
+			    (pos->s_tag.tag != tag))
+				continue;
+
 			name = pos->s_name;
 			len = strlen(name);
 			filp->f_pos = ino = pos->s_ino;
@@ -1076,3 +1130,103 @@ const struct file_operations sysfs_dir_operations = {
 	.read		= generic_read_dir,
 	.readdir	= sysfs_readdir,
 };
+
+const void *sysfs_creation_tag(struct sysfs_dirent *parent_sd, struct sysfs_dirent *sd)
+{
+	const void *tag = NULL;
+
+	if (parent_sd->s_flags & SYSFS_FLAG_TAGGED) {
+		struct kobject *kobj;
+		switch (sysfs_type(sd)) {
+		case SYSFS_DIR:
+			kobj = sd->s_dir.kobj;
+			break;
+		case SYSFS_KOBJ_LINK:
+			kobj = sd->s_symlink.target_sd->s_dir.kobj;
+			break;
+		default:
+			BUG();
+		}
+		tag = parent_sd->s_tag.ops->kobject_tag(kobj);
+	}
+	return tag;
+}
+
+const void *sysfs_removal_tag(struct kobject *kobj, struct sysfs_dirent *dir_sd)
+{
+	const void *tag = NULL;
+
+	if (dir_sd->s_flags & SYSFS_FLAG_TAGGED)
+		tag = kobj->sd->s_tag.tag;
+
+	return tag;
+}
+
+const void *sysfs_lookup_tag(struct sysfs_dirent *dir_sd, struct super_block *sb)
+{
+	const void *tag = NULL;
+
+	if (dir_sd->s_flags & SYSFS_FLAG_TAGGED)
+		tag = dir_sd->s_tag.ops->sb_tag(&sysfs_info(sb)->tag);
+
+	return tag;
+}
+
+const void *sysfs_dirent_tag(struct sysfs_dirent *sd)
+{
+	const void *tag = NULL;
+
+	if (sd->s_parent && (sd->s_parent->s_flags & SYSFS_FLAG_TAGGED))
+		tag = sd->s_tag.tag;
+
+	return tag;
+}
+
+/**
+ *	sysfs_enable_tagging - Automatically tag all of the children in a directory.
+ *	@kobj:	object whose children should be filtered by tags
+ *
+ *	Once tagging has been enabled on a directory the contents
+ *	of the directory become dependent upon context captured when
+ *	sysfs was mounted.
+ *
+ *	tag_ops->sb_tag() returns the context for a given superblock.
+ *
+ *	tag_ops->kobject_tag() returns the context that a given kobj
+ *	resides in.
+ *
+ *	Using those methods the sysfs code on tagged directories
+ *	carefully stores the files so that when we lookup files
+ *	we get the proper answer for our context.
+ *
+ *	If the context of a kobject is changed it is expected that
+ *	the kobject will be renamed so the appopriate sysfs data structures
+ *	can be updated.
+ */
+int sysfs_enable_tagging(struct kobject *kobj,
+	const struct sysfs_tagged_dir_operations *tag_ops)
+{
+	struct sysfs_dirent *sd;
+	int err;
+
+	err = -ENOENT;
+	sd = kobj->sd;
+
+	mutex_lock(&sysfs_mutex);
+	err = -EINVAL;
+	/* We can only enable tagging on empty directories
+	 * where tagging is not already enabled, and
+	 * who are not subdirectories of directories where tagging is
+	 * enabled.
+	 */
+	if (!sd->s_dir.children && (sysfs_type(sd) == SYSFS_DIR) &&
+	    !(sd->s_flags & SYSFS_FLAG_TAGGED) &&
+	    sd->s_parent &&
+	    !(sd->s_parent->s_flags & SYSFS_FLAG_TAGGED)) {
+		err = 0;
+		sd->s_flags |= SYSFS_FLAG_TAGGED;
+		sd->s_tag.ops = tag_ops;
+	}
+	mutex_unlock(&sysfs_mutex);
+	return err;
+}
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index ade6140..8399c75 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -455,9 +455,9 @@ void sysfs_notify(struct kobject *k, char *dir, char *attr)
 	mutex_lock(&sysfs_mutex);
 
 	if (sd && dir)
-		sd = sysfs_find_dirent(sd, dir);
+		sd = sysfs_find_dirent(sd, NULL, dir);
 	if (sd && attr)
-		sd = sysfs_find_dirent(sd, attr);
+		sd = sysfs_find_dirent(sd, NULL, attr);
 	if (sd) {
 		struct sysfs_open_dirent *od;
 
@@ -615,7 +615,7 @@ EXPORT_SYMBOL_GPL(sysfs_chmod_file);
 
 void sysfs_remove_file(struct kobject * kobj, const struct attribute * attr)
 {
-	sysfs_hash_and_remove(kobj->sd, attr->name);
+	sysfs_hash_and_remove(kobj, kobj->sd, attr->name);
 }
 
 
@@ -632,7 +632,7 @@ void sysfs_remove_file_from_group(struct kobject *kobj,
 
 	dir_sd = sysfs_get_dirent(kobj->sd, group);
 	if (dir_sd) {
-		sysfs_hash_and_remove(dir_sd, attr->name);
+		sysfs_hash_and_remove(kobj, dir_sd, attr->name);
 		sysfs_put(dir_sd);
 	}
 }
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index d197237..57a7dae 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -16,16 +16,16 @@
 #include "sysfs.h"
 
 
-static void remove_files(struct sysfs_dirent *dir_sd,
+static void remove_files(struct kobject *kobj, struct sysfs_dirent *dir_sd,
 			 const struct attribute_group *grp)
 {
 	struct attribute *const* attr;
 
 	for (attr = grp->attrs; *attr; attr++)
-		sysfs_hash_and_remove(dir_sd, (*attr)->name);
+		sysfs_hash_and_remove(kobj, dir_sd, (*attr)->name);
 }
 
-static int create_files(struct sysfs_dirent *dir_sd,
+static int create_files(struct kobject *kobj, struct sysfs_dirent *dir_sd,
 			const struct attribute_group *grp)
 {
 	struct attribute *const* attr;
@@ -34,7 +34,7 @@ static int create_files(struct sysfs_dirent *dir_sd,
 	for (attr = grp->attrs; *attr && !error; attr++)
 		error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
 	if (error)
-		remove_files(dir_sd, grp);
+		remove_files(kobj, dir_sd, grp);
 	return error;
 }
 
@@ -54,7 +54,7 @@ int sysfs_create_group(struct kobject * kobj,
 	} else
 		sd = kobj->sd;
 	sysfs_get(sd);
-	error = create_files(sd, grp);
+	error = create_files(kobj, sd, grp);
 	if (error) {
 		if (grp->name)
 			sysfs_remove_subdir(sd);
@@ -75,7 +75,7 @@ void sysfs_remove_group(struct kobject * kobj,
 	} else
 		sd = sysfs_get(dir_sd);
 
-	remove_files(sd, grp);
+	remove_files(kobj, sd, grp);
 	if (grp->name)
 		sysfs_remove_subdir(sd);
 
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index d9262f7..24b8720 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -215,17 +215,19 @@ struct inode * sysfs_get_inode(struct sysfs_dirent *sd)
 	return inode;
 }
 
-int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name)
+int sysfs_hash_and_remove(struct kobject *kobj, struct sysfs_dirent *dir_sd, const char *name)
 {
 	struct sysfs_addrm_cxt acxt;
 	struct sysfs_dirent *sd;
+	const void *tag;
 
 	if (!dir_sd)
 		return -ENOENT;
 
 	sysfs_addrm_start(&acxt, dir_sd);
+	tag = sysfs_removal_tag(kobj, dir_sd);
 
-	sd = sysfs_find_dirent(dir_sd, name);
+	sd = sysfs_find_dirent(dir_sd, tag, name);
 	if (sd)
 		sysfs_remove_one(&acxt, sd);
 
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index ef5f7ae..f6e49d9 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -75,6 +75,7 @@ static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
 		goto out_err;
 	}
 	root->d_fsdata = &sysfs_root;
+	root->d_sb = sb;
 	sb->s_root = root;
 	sb->s_fs_info = info;
 	return 0;
@@ -88,20 +89,55 @@ out_err:
 	return error;
 }
 
+static int sysfs_test_super(struct super_block *sb, void *ptr)
+{
+	struct task_struct *task = ptr;
+	struct sysfs_super_info *info = sysfs_info(sb);
+	int found = 1;
+
+	return found;
+}
+
 static int sysfs_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
 {
-	int rc;
+	struct super_block *sb;
+	int error;
 	mutex_lock(&sysfs_rename_mutex);
-	rc = get_sb_single(fs_type, flags, data, sysfs_fill_super, mnt);
+	sb = sget(fs_type, sysfs_test_super, set_anon_super, current);
+	if (IS_ERR(sb)) {
+		error = PTR_ERR(sb);
+		goto out;
+	}
+	if (!sb->s_root) {
+		sb->s_flags = flags;
+		error = sysfs_fill_super(sb, data, flags & MS_SILENT ? 1 : 0);
+		if (error) {
+			up_write(&sb->s_umount);
+			deactivate_super(sb);
+			goto out;
+		}
+		sb->s_flags |= MS_ACTIVE;
+	}
+	do_remount_sb(sb, flags, data, 0);
+	error = simple_set_mnt(mnt, sb);
+out:
 	mutex_unlock(&sysfs_rename_mutex);
-	return rc;
+	return error;
+}
+
+static void sysfs_kill_sb(struct super_block *sb)
+{
+	struct sysfs_super_info *info = sysfs_info(sb);
+
+	kill_anon_super(sb);
+	kfree(info);
 }
 
 struct file_system_type sysfs_fs_type = {
 	.name		= "sysfs",
 	.get_sb		= sysfs_get_sb,
-	.kill_sb	= kill_anon_super,
+	.kill_sb	= sysfs_kill_sb,
 };
 
 void sysfs_grab_supers(void)
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 5f66c44..b0f8070 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -87,7 +87,7 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
 
 void sysfs_remove_link(struct kobject * kobj, const char * name)
 {
-	sysfs_hash_and_remove(kobj->sd, name);
+	sysfs_hash_and_remove(kobj, kobj->sd, name);
 }
 
 static int sysfs_get_target_path(struct sysfs_dirent *parent_sd,
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index d4269ba..1a19eca 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -46,6 +46,10 @@ struct sysfs_dirent {
 	const char		*s_name;
 
 	union {
+		const struct sysfs_tagged_dir_operations	*ops;
+		const void 					*tag;
+	} s_tag;
+	union {
 		struct sysfs_elem_dir		s_dir;
 		struct sysfs_elem_symlink	s_symlink;
 		struct sysfs_elem_attr		s_attr;
@@ -69,6 +73,7 @@ struct sysfs_dirent {
 
 #define SYSFS_FLAG_MASK			~SYSFS_TYPE_MASK
 #define SYSFS_FLAG_REMOVED		0x0200
+#define SYSFS_FLAG_TAGGED		0x0400
 
 static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
 {
@@ -87,6 +92,7 @@ struct sysfs_addrm_cxt {
 
 struct sysfs_super_info {
 	int	grabbed;
+	struct sysfs_tag_info tag;
 };
 
 #define sysfs_info(SB) ((struct sysfs_super_info *)(SB)->s_fs_info)
@@ -112,6 +118,13 @@ extern spinlock_t sysfs_assoc_lock;
 extern const struct file_operations sysfs_dir_operations;
 extern const struct inode_operations sysfs_dir_inode_operations;
 
+extern const void *sysfs_creation_tag(struct sysfs_dirent *parent_sd,
+				      struct sysfs_dirent *sd);
+extern const void *sysfs_removal_tag(struct kobject *kobj,
+				     struct sysfs_dirent *dir_sd);
+extern const void *sysfs_lookup_tag(struct sysfs_dirent *dir_sd,
+				    struct super_block *sb);
+extern const void *sysfs_dirent_tag(struct sysfs_dirent *sd);
 struct dentry *sysfs_get_dentry(struct super_block *sb, struct sysfs_dirent *sd);
 struct sysfs_dirent *sysfs_get_active_two(struct sysfs_dirent *sd);
 void sysfs_put_active_two(struct sysfs_dirent *sd);
@@ -122,6 +135,7 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd);
 void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt);
 
 struct sysfs_dirent *sysfs_find_dirent(struct sysfs_dirent *parent_sd,
+				       const void *tag,
 				       const unsigned char *name);
 struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
 				      const unsigned char *name);
@@ -153,7 +167,7 @@ static inline void sysfs_put(struct sysfs_dirent *sd)
  */
 struct inode *sysfs_get_inode(struct sysfs_dirent *sd);
 int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);
-int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name);
+int sysfs_hash_and_remove(struct kobject *kobj, struct sysfs_dirent *dir_sd, const char *name);
 int sysfs_inode_init(void);
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 483356c..c8d7a69 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -76,6 +76,14 @@ struct sysfs_ops {
 	ssize_t	(*store)(struct kobject *,struct attribute *,const char *, size_t);
 };
 
+struct sysfs_tag_info {
+};
+
+struct sysfs_tagged_dir_operations {
+	const void *(*sb_tag)(struct sysfs_tag_info *info);
+	const void *(*kobject_tag)(struct kobject *kobj);
+};
+
 #ifdef CONFIG_SYSFS
 
 int sysfs_schedule_callback(struct kobject *kobj, void (*func)(void *),
@@ -113,6 +121,8 @@ void sysfs_remove_file_from_group(struct kobject *kobj,
 void sysfs_notify(struct kobject *kobj, char *dir, char *attr);
 void sysfs_printk_last_file(void);
 
+int sysfs_enable_tagging(struct kobject *, const struct sysfs_tagged_dir_operations *);
+
 extern int __must_check sysfs_init(void);
 
 #else /* CONFIG_SYSFS */
@@ -212,6 +222,12 @@ static inline void sysfs_notify(struct kobject *kobj, char *dir, char *attr)
 {
 }
 
+static inline int sysfs_enable_tagging(struct kobject *kobj,
+				const struct sysfs_tagged_dir_operations *tag_ops)
+{
+	return 0;
+}
+
 static inline int __must_check sysfs_init(void)
 {
 	return 0;
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 08/10] sysfs: Implement sysfs_delete_link and sysfs_rename_link
  2007-12-01  9:28             ` [PATCH 07/10] sysfs: Implement sysfs tagged directory support Eric W. Biederman
@ 2007-12-01  9:30               ` Eric W. Biederman
  2007-12-01  9:33                 ` [PATCH 09/10] driver core: Implement tagged directory support for device classes Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


When removing a symlink sysfs_remove_link does not provide enough
information to figure out which tagged directory the symlink falls in.
So I need sysfs_delete_link which is passed the target of the symlink
to delete.

Further half the time when we are removing a symlink the code is
actually renaming the symlink but not doing so explicitly because we
don't have a symlink rename method.  So I have added sysfs_rename_link
as well.

Both of these functions now have enough information to find a symlink
in a tagged directory.  The only restriction is that they must be
called before the target kobject is renamed or deleted.  If they are
called later I loose track of which tag the target kobject was marked
with and can no longer find the old symlink to remove it.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/symlink.c    |   31 +++++++++++++++++++++++++++++++
 include/linux/sysfs.h |   17 +++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index b0f8070..89c98cb 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -80,6 +80,21 @@ int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char
 }
 
 /**
+ *	sysfs_delete_link - remove symlink in object's directory.
+ *	@kobj:	object we're acting for.
+ *	@targ:	object we're pointing to.
+ *	@name:	name of the symlink to remove.
+ *
+ *	Unlike sysfs_remove_link sysfs_delete_link has enough information
+ *	to successfully delete symlinks in tagged directories.
+ */
+void sysfs_delete_link(struct kobject *kobj, struct kobject *targ,
+			const char *name)
+{
+	sysfs_hash_and_remove(targ, kobj->sd, name);
+}
+
+/**
  *	sysfs_remove_link - remove symlink in object's directory.
  *	@kobj:	object we're acting for.
  *	@name:	name of the symlink to remove.
@@ -90,6 +105,22 @@ void sysfs_remove_link(struct kobject * kobj, const char * name)
 	sysfs_hash_and_remove(kobj, kobj->sd, name);
 }
 
+/**
+ *	sysfs_rename_link - rename symlink in object's directory.
+ *	@kobj:	object we're acting for.
+ *	@targ:	object we're pointing to.
+ *	@old:	previous name of the symlink.
+ *	@new:	new name of the symlink.
+ *
+ *	A helper function for the common rename symlink idiom.
+ */
+int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
+			const char *old, const char *new)
+{
+	sysfs_delete_link(kobj, targ, old);
+	return sysfs_create_link(kobj, targ, new);
+}
+
 static int sysfs_get_target_path(struct sysfs_dirent *parent_sd,
 				 struct sysfs_dirent *target_sd, char *path)
 {
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index c8d7a69..c2e8b0d 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -109,6 +109,12 @@ int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
 				   const char *name);
 void sysfs_remove_link(struct kobject *kobj, const char *name);
 
+int sysfs_rename_link(struct kobject *kobj, struct kobject *target,
+			const char *old_name, const char *new_name);
+
+void sysfs_delete_link(struct kobject *dir, struct kobject *targ,
+			const char *name);
+
 int __must_check sysfs_create_group(struct kobject *kobj,
 				    const struct attribute_group *grp);
 void sysfs_remove_group(struct kobject *kobj,
@@ -195,6 +201,17 @@ static inline void sysfs_remove_link(struct kobject *kobj, const char *name)
 	;
 }
 
+static inline int sysfs_rename_link(struct kobject * k, struct kobject *t,
+				    const char *old_name, const char * new_name)
+{
+	return 0;
+}
+
+static inline void sysfs_delete_link(struct kobject *k, struct kobject *t,
+				     const char *name)
+{
+}
+
 static inline int sysfs_create_group(struct kobject *kobj,
 				     const struct attribute_group *grp)
 {
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 09/10] driver core: Implement tagged directory support for device classes.
  2007-12-01  9:30               ` [PATCH 08/10] sysfs: Implement sysfs_delete_link and sysfs_rename_link Eric W. Biederman
@ 2007-12-01  9:33                 ` Eric W. Biederman
  2007-12-01  9:35                   ` [PATCH 10/10] net: Enable tagging for net_class directories in sysfs Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


This patch enables tagging on every class directory if struct class
has tag_ops.

In addition device_del and device_rename were modified to use
sysfs_delete_link and sysfs_rename_link respectively to ensure when
these operations happen on devices whose classes have tag_ops that they
work properly.


Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 drivers/base/class.c   |   30 ++++++++++++++++++++++++---
 drivers/base/core.c    |   51 +++++++++++++++++++++++++----------------------
 include/linux/device.h |    2 +
 3 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/drivers/base/class.c b/drivers/base/class.c
index c4f8843..ed9393d 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -135,6 +135,17 @@ static void remove_class_attrs(struct class * cls)
 	}
 }
 
+static int class_setup_tagging(struct class *cls)
+{
+	const struct sysfs_tagged_dir_operations *tag_ops;
+
+	tag_ops = cls->tag_ops;
+	if (!tag_ops)
+		return 0;
+
+	return sysfs_enable_tagging(&cls->subsys.kobj, tag_ops);
+}
+
 int class_register(struct class * cls)
 {
 	int error;
@@ -160,11 +171,22 @@ int class_register(struct class * cls)
 	cls->subsys.kobj.ktype = &class_ktype;
 
 	error = kset_register(&cls->subsys);
-	if (!error) {
-		error = add_class_attrs(class_get(cls));
-		class_put(cls);
-	}
+	if (error)
+		goto out;
+
+	error = class_setup_tagging(cls);
+	if (error)
+		goto out_unregister;
+
+	error = add_class_attrs(cls);
+	if (error)
+		goto out_unregister;
+
+out:
 	return error;
+out_unregister:
+	kset_unregister(&cls->subsys);
+	goto out;
 }
 
 void class_unregister(struct class * cls)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index a2c3d4e..f9d3fcf 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -600,16 +600,20 @@ static struct kobject *get_device_parent(struct device *dev,
 			return kobj;
 
 		/* or create a new class-directory at the parent device */
-		k = kobject_create(dev->class->name, parent_kobj);
-		if (!k)
+bser		kobj = kobject_create(dev->class->name, parent_kobj);
+		if (!kobj)
 			return NULL;
-		k->kset = &dev->class->class_dirs;
-		retval = kobject_register(k);
+		kobj->kset = &dev->class->class_dirs;
+		retval = kobject_register(kobj);
 		if (retval < 0) {
-			kfree(k);
+			kfree(kobj);
 			return NULL;
 		}
-		return k;
+		/* If we created a new class-directory setup tagging */
+		if (kobj && dev->class->tag_ops)
+			sysfs_enable_tagging(k, dev->class->tag_ops);
+
+		return kobj;
 	}
 
 	if (parent)
@@ -758,7 +762,8 @@ static void device_remove_class_symlinks(struct device *dev)
 
 	if (dev->kobj.parent != &dev->class->subsys.kobj &&
 	    dev->type != &part_type)
-		sysfs_remove_link(&dev->class->subsys.kobj, dev->bus_id);
+		sysfs_delete_link(&dev->class->subsys.kobj,
+				  &dev->kobj, dev->bus_id);
 #else
 	if (dev->parent && dev->type != &part_type)
 		sysfs_remove_link(&dev->kobj, "device");
@@ -1223,6 +1228,15 @@ int device_rename(struct device *dev, char *new_name)
 	strlcpy(old_device_name, dev->bus_id, BUS_ID_SIZE);
 	strlcpy(dev->bus_id, new_name, BUS_ID_SIZE);
 
+#ifndef CONFIG_SYSFS_DEPRECATED
+	if (dev->class && (dev->kobj.parent != &dev->class->subsys.kobj)) {
+		error = sysfs_rename_link(&dev->class->subsys.kobj,
+			&dev->kobj, old_device_name, new_name);
+		if (error)
+			goto out;
+	}
+#endif
+
 	error = kobject_rename(&dev->kobj, new_name);
 	if (error) {
 		strlcpy(dev->bus_id, old_device_name, BUS_ID_SIZE);
@@ -1231,24 +1245,13 @@ int device_rename(struct device *dev, char *new_name)
 
 #ifdef CONFIG_SYSFS_DEPRECATED
 	if (old_class_name) {
+		error = -ENOMEM;
 		new_class_name = make_class_name(dev->class->name, &dev->kobj);
-		if (new_class_name) {
-			error = sysfs_create_link(&dev->parent->kobj,
-						  &dev->kobj, new_class_name);
-			if (error)
-				goto out;
-			sysfs_remove_link(&dev->parent->kobj, old_class_name);
-		}
-	}
-#else
-	if (dev->class) {
-		sysfs_remove_link(&dev->class->subsys.kobj, old_device_name);
-		error = sysfs_create_link(&dev->class->subsys.kobj, &dev->kobj,
-					  dev->bus_id);
-		if (error) {
-			dev_err(dev, "%s: sysfs_create_symlink failed (%d)\n",
-				__FUNCTION__, error);
-		}
+		if (new_class_name)
+			error = sysfs_rename_link(&dev->parent->kobj,
+						  &dev->kobj,
+						  old_class_name,
+						  new_class_name);
 	}
 #endif
 
diff --git a/include/linux/device.h b/include/linux/device.h
index ed38712..80ba08f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -187,6 +187,8 @@ struct class {
 
 	int	(*suspend)(struct device *, pm_message_t state);
 	int	(*resume)(struct device *);
+
+	const struct sysfs_tagged_dir_operations *tag_ops;
 };
 
 extern int __must_check class_register(struct class *);
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 10/10] net: Enable tagging for net_class directories in sysfs
  2007-12-01  9:33                 ` [PATCH 09/10] driver core: Implement tagged directory support for device classes Eric W. Biederman
@ 2007-12-01  9:35                   ` Eric W. Biederman
  0 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01  9:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Greg KH
  Cc: Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller


The problem.  Network devices show up in sysfs and with the network
namespace active multiple devices with the same name can show up in
the same directory, ouch!

To avoid that problem and allow existing applications in network namespaces
to see the same interface that is currently presented in sysfs, this
patch enables the tagging directory support in sysfs.

By using the network namespace pointers as tags to separate out the
sysfs directory entries we ensure that we don't have conflicts in the
directories and applications only see a limited set of the network
devices.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 fs/sysfs/mount.c      |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/sysfs.h |    2 ++
 net/Kconfig           |    2 +-
 net/core/net-sysfs.c  |   20 ++++++++++++++++++++
 4 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index f6e49d9..ed47133 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -16,6 +16,8 @@
 #include <linux/mount.h>
 #include <linux/pagemap.h>
 #include <linux/init.h>
+#include <linux/nsproxy.h>
+#include <net/net_namespace.h>
 
 #include "sysfs.h"
 
@@ -78,6 +80,7 @@ static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
 	root->d_sb = sb;
 	sb->s_root = root;
 	sb->s_fs_info = info;
+	info->tag.net_ns = hold_net(current->nsproxy->net_ns);
 	return 0;
 
 out_err:
@@ -95,6 +98,9 @@ static int sysfs_test_super(struct super_block *sb, void *ptr)
 	struct sysfs_super_info *info = sysfs_info(sb);
 	int found = 1;
 
+	if (task->nsproxy->net_ns != info->tag.net_ns)
+		found = 0;
+
 	return found;
 }
 
@@ -131,6 +137,8 @@ static void sysfs_kill_sb(struct super_block *sb)
 	struct sysfs_super_info *info = sysfs_info(sb);
 
 	kill_anon_super(sb);
+	if (info->tag.net_ns)
+		release_net(info->tag.net_ns);
 	kfree(info);
 }
 
@@ -181,6 +189,31 @@ restart:
 	spin_unlock(&sb_lock);
 }
 
+#ifdef CONFIG_NET
+static void sysfs_net_exit(struct net *net)
+{
+	/* Allow the net namespace to go away while sysfs is still mounted. */
+	struct super_block *sb;
+	mutex_lock(&sysfs_rename_mutex);
+	sysfs_grab_supers();
+	mutex_lock(&sysfs_mutex);
+	list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
+		struct sysfs_super_info *info = sysfs_info(sb);
+		if (info->tag.net_ns != net)
+			continue;
+		release_net(info->tag.net_ns);
+		info->tag.net_ns = NULL;
+	}
+	mutex_unlock(&sysfs_mutex);
+	sysfs_release_supers();
+	mutex_unlock(&sysfs_rename_mutex);
+}
+
+static struct pernet_operations sysfs_net_ops = {
+	.exit = sysfs_net_exit,
+};
+#endif
+
 int __init sysfs_init(void)
 {
 	int err = -ENOMEM;
@@ -205,6 +238,9 @@ int __init sysfs_init(void)
 			unregister_filesystem(&sysfs_fs_type);
 			goto out_err;
 		}
+#ifdef CONFIG_NET
+		register_pernet_subsys(&sysfs_net_ops);
+#endif
 	} else
 		goto out_err;
 out:
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index c2e8b0d..2c93278 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -19,6 +19,7 @@
 
 struct kobject;
 struct module;
+struct net;
 
 /* FIXME
  * The *owner field is no longer used, but leave around
@@ -77,6 +78,7 @@ struct sysfs_ops {
 };
 
 struct sysfs_tag_info {
+	struct net *net_ns;
 };
 
 struct sysfs_tagged_dir_operations {
diff --git a/net/Kconfig b/net/Kconfig
index ab4e6da..250585e 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -30,7 +30,7 @@ menu "Networking options"
 config NET_NS
 	bool "Network namespace support"
 	default n
-	depends on EXPERIMENTAL && !SYSFS
+	depends on EXPERIMENTAL
 	help
 	  Allow user space to create what appear to be multiple instances
 	  of the network stack.
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 61ead1d..2aa64d0 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -13,7 +13,9 @@
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/if_arp.h>
+#include <linux/nsproxy.h>
 #include <net/sock.h>
+#include <net/net_namespace.h>
 #include <linux/rtnetlink.h>
 #include <linux/wireless.h>
 #include <net/iw_handler.h>
@@ -431,6 +433,23 @@ static void netdev_release(struct device *d)
 	kfree((char *)dev - dev->padded);
 }
 
+static const void *net_sb_tag(struct sysfs_tag_info *info)
+{
+	return info->net_ns;
+}
+
+static const void *net_kobject_tag(struct kobject *kobj)
+{
+	struct net_device *dev;
+	dev = container_of(kobj, struct net_device, dev.kobj);
+	return dev->nd_net;
+}
+
+static const struct sysfs_tagged_dir_operations net_tagged_dir_operations = {
+	.sb_tag = net_sb_tag,
+	.kobject_tag = net_kobject_tag,
+};
+
 static struct class net_class = {
 	.name = "net",
 	.dev_release = netdev_release,
@@ -440,6 +459,7 @@ static struct class net_class = {
 #ifdef CONFIG_HOTPLUG
 	.dev_uevent = netdev_uevent,
 #endif
+	.tag_ops = &net_tagged_dir_operations,
 };
 
 /* Delete sysfs entries but hold kobject reference until after all
-- 
1.5.3.rc6.17.g1911


^ permalink raw reply related	[flat|nested] 64+ messages in thread

* namespace support requires network modules to say "GPL"
  2007-12-01  9:06 [PATCH 0/10] sysfs network namespace support Eric W. Biederman
  2007-12-01  9:12 ` [PATCH 01/10] sysfs: Make sysfs_mount static again Eric W. Biederman
@ 2007-12-01 13:10 ` Mark Lord
  2007-12-01 13:13   ` Mark Lord
  2007-12-01 19:17   ` Stephen Hemminger
  2007-12-21  3:07 ` [PATCH 0/10] sysfs network namespace support Greg KH
  2 siblings, 2 replies; 64+ messages in thread
From: Mark Lord @ 2007-12-01 13:10 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

> Now that we have network namespace support merged it is time to
> revisit the sysfs support so we can remove the dependency on !SYSFS.
...

Now that the namespace updates are part of 2.6.24,
there is a major inconsistency in network EXPORT_SYMBOLs.

It used to be that an external network module could get away without
having to add a MODULE_LICENSE("GPL*") line to the source.

In support of that, common networking functions (still) use EXPORT_SYMBOL()
rather than the more restrictive EXPORT_SYMBOL_GPL().

Eg.  register_netdev(), sk_alloc(), __dev_get_by_name().

But now, none of those three are actually usable by default,
because they all require "init_net", which is EXPORT_SYMBOL_GPL().

So.. It appears that one of three things should really happen next:

1) Change the other exports to also be EXPORT_SYMBOL_GPL.

2) Have register_netdev, sk_alloc, and __dev_get_by_name default
to using init_net when NULL is specified in the namespace field.

or 

3) Change init_net to be EXPORT_SYMBOL_GPL.

Right now, things are just a bit inconsistent, and it's not clear
whether the namespace changes intended this consequence or not.

Cheers

(as for me, I think all kernel modules are GPL, whether they have
 the MODULE_LICENSE line or not, so flames to /dev/null on that).

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 13:10 ` namespace support requires network modules to say "GPL" Mark Lord
@ 2007-12-01 13:13   ` Mark Lord
  2007-12-01 19:17   ` Stephen Hemminger
  1 sibling, 0 replies; 64+ messages in thread
From: Mark Lord @ 2007-12-01 13:13 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

Mark Lord wrote:
>> Now that we have network namespace support merged it is time to
>> revisit the sysfs support so we can remove the dependency on !SYSFS.
> ...
> Now that the namespace updates are part of 2.6.24,
> there is a major inconsistency in network EXPORT_SYMBOLs.
> 
> It used to be that an external network module could get away without
> having to add a MODULE_LICENSE("GPL*") line to the source.
> 
> In support of that, common networking functions (still) use EXPORT_SYMBOL()
> rather than the more restrictive EXPORT_SYMBOL_GPL().
> 
> Eg.  register_netdev(), sk_alloc(), __dev_get_by_name().
> 
> But now, none of those three are actually usable by default,
> because they all require "init_net", which is EXPORT_SYMBOL_GPL().
> 
> So.. It appears that one of three things should really happen next:
> 
> 1) Change the other exports to also be EXPORT_SYMBOL_GPL.
> 
> 2) Have register_netdev, sk_alloc, and __dev_get_by_name default
> to using init_net when NULL is specified in the namespace field.
> 
> or
> 3) Change init_net to be EXPORT_SYMBOL_GPL.
..

Obviously that should instead say:

3) Change init_net to be EXPORT_SYMBOL instead of EXPORT_SYMBOL_GPL.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 13:10 ` namespace support requires network modules to say "GPL" Mark Lord
  2007-12-01 13:13   ` Mark Lord
@ 2007-12-01 19:17   ` Stephen Hemminger
  2007-12-01 19:23     ` Alan Cox
                       ` (2 more replies)
  1 sibling, 3 replies; 64+ messages in thread
From: Stephen Hemminger @ 2007-12-01 19:17 UTC (permalink / raw)
  To: Mark Lord
  Cc: Eric W. Biederman, Greg Kroah-Hartman, Greg KH, Tejun Heo,
	Linux Containers, netdev, cornelia.huck, stern, kay.sievers,
	linux-kernel, Andrew Morton, Herbert Xu, David Miller,
	Linus Torvalds

On Sat, 01 Dec 2007 08:10:17 -0500
Mark Lord <lkml@rtr.ca> wrote:

> > Now that we have network namespace support merged it is time to
> > revisit the sysfs support so we can remove the dependency on !SYSFS.
> ...
> 
> Now that the namespace updates are part of 2.6.24,
> there is a major inconsistency in network EXPORT_SYMBOLs.
> 
> It used to be that an external network module could get away without
> having to add a MODULE_LICENSE("GPL*") line to the source.
> 
> In support of that, common networking functions (still) use EXPORT_SYMBOL()
> rather than the more restrictive EXPORT_SYMBOL_GPL().
> 
> Eg.  register_netdev(), sk_alloc(), __dev_get_by_name().
> 
> But now, none of those three are actually usable by default,
> because they all require "init_net", which is EXPORT_SYMBOL_GPL().
> 

Then init_net needs to be not GPL limited. Sorry, we need to allow
non GPL network drivers.  There is a fine line between keeping the
binary seething masses from accessing random kernel functions, and allowing
reasonable (but still non GPL) things like ndiswrapper to use network
device interface.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 19:17   ` Stephen Hemminger
@ 2007-12-01 19:23     ` Alan Cox
  2007-12-01 19:38       ` Stephen Hemminger
  2007-12-03  0:02       ` David Schwartz
  2007-12-01 19:54     ` Eric W. Biederman
  2007-12-02  0:30     ` Stephen Hemminger
  2 siblings, 2 replies; 64+ messages in thread
From: Alan Cox @ 2007-12-01 19:23 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Mark Lord, Eric W. Biederman, Greg Kroah-Hartman, Greg KH,
	Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller, Linus Torvalds

> Then init_net needs to be not GPL limited. Sorry, we need to allow
> non GPL network drivers.  There is a fine line between keeping the

Why - they aren't exactly likely to be permissible by law

> binary seething masses from accessing random kernel functions, and allowing
> reasonable (but still non GPL) things like ndiswrapper to use network
> device interface.

Its up to the ndiswrapper authors how the licence their code, but they
should respect how we licence ours.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 19:23     ` Alan Cox
@ 2007-12-01 19:38       ` Stephen Hemminger
  2007-12-01 19:45         ` Alan Cox
  2007-12-01 20:13         ` Eric W. Biederman
  2007-12-03  0:02       ` David Schwartz
  1 sibling, 2 replies; 64+ messages in thread
From: Stephen Hemminger @ 2007-12-01 19:38 UTC (permalink / raw)
  To: Alan Cox
  Cc: Mark Lord, Eric W. Biederman, Greg Kroah-Hartman, Greg KH,
	Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller, Linus Torvalds

On Sat, 1 Dec 2007 19:23:41 +0000
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > Then init_net needs to be not GPL limited. Sorry, we need to allow
> > non GPL network drivers.  There is a fine line between keeping the
> 
> Why - they aren't exactly likely to be permissible by law

Matter of debate in which there are several opinions.
I don't like binary modules either, but don't feel qualified to render
a legal opinion.

> 
> > binary seething masses from accessing random kernel functions, and allowing
> > reasonable (but still non GPL) things like ndiswrapper to use network
> > device interface.
> 
> Its up to the ndiswrapper authors how the licence their code, but they
> should respect how we licence ours.

Then change the license, explicitly and get it approved, forcing license
changes by technically subversive means is bad policy. It is like Euro bureaucrats
sneaking in software patents in regulations. If you want to have the debate and
can get it resolved, then I support you.

Actually, the whole mess would go away if the api for dev_get_by_XXXX hadn't
been changed in the namespace transition. IMHO the interface to dev_get_by_name()
should not have added a namespace parameter, of the callers in the tree, only
two use a different namespace. So it would have been better to to introduce
dev_get_by_name_ns() with the extra parameter.

Can we get this resolved before 2.6.24 is released? Going back and forth
on API's is just needless frottage.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 19:38       ` Stephen Hemminger
@ 2007-12-01 19:45         ` Alan Cox
  2007-12-01 20:13         ` Eric W. Biederman
  1 sibling, 0 replies; 64+ messages in thread
From: Alan Cox @ 2007-12-01 19:45 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Mark Lord, Eric W. Biederman, Greg Kroah-Hartman, Greg KH,
	Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller, Linus Torvalds

> Then change the license, explicitly and get it approved, forcing license
> changes by technically subversive means is bad policy. It is like Euro bureaucrats

I don't need to - the licence has been the same since about 0.12

Alan

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 19:17   ` Stephen Hemminger
  2007-12-01 19:23     ` Alan Cox
@ 2007-12-01 19:54     ` Eric W. Biederman
  2007-12-02  0:30     ` Stephen Hemminger
  2 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01 19:54 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Mark Lord, Greg Kroah-Hartman, Greg KH, Tejun Heo,
	Linux Containers, netdev, cornelia.huck, stern, kay.sievers,
	linux-kernel, Andrew Morton, Herbert Xu, David Miller,
	Linus Torvalds

Stephen Hemminger <shemminger@linux-foundation.org> writes:

> On Sat, 01 Dec 2007 08:10:17 -0500
> Mark Lord <lkml@rtr.ca> wrote:
>
>> > Now that we have network namespace support merged it is time to
>> > revisit the sysfs support so we can remove the dependency on !SYSFS.
>> ...
>> 
>> Now that the namespace updates are part of 2.6.24,
>> there is a major inconsistency in network EXPORT_SYMBOLs.
>> 
>> It used to be that an external network module could get away without
>> having to add a MODULE_LICENSE("GPL*") line to the source.
>> 
>> In support of that, common networking functions (still) use EXPORT_SYMBOL()
>> rather than the more restrictive EXPORT_SYMBOL_GPL().
>> 
>> Eg.  register_netdev(), sk_alloc(), __dev_get_by_name().
>> 
>> But now, none of those three are actually usable by default,
>> because they all require "init_net", which is EXPORT_SYMBOL_GPL().

Which alternative kernel does the above comment apply to?

> Then init_net needs to be not GPL limited. Sorry, we need to allow
> non GPL network drivers.

For the record network drivers should not be affected.  As a practical
measure that just gets unmaintainable and it is unnecessary.

There are specific exceptions where network drivers mess with the userspace
interfaces where I do have some impact.  However if you are messing
with our userspace interface especially with network namespaces in place
I don't see how it is possible for you to be anything other then a derivative
work, and something we need in tree to keep maintenance a manageable thing.

It should just be the core of the network stack that struct net has some
effect on.

> There is a fine line between keeping the
> binary seething masses from accessing random kernel functions, and allowing
> reasonable (but still non GPL) things like ndiswrapper to use network
> device interface.

Does ndiswrapper break?  If so what dubious and unsupportable thing is
it doing?

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 19:38       ` Stephen Hemminger
  2007-12-01 19:45         ` Alan Cox
@ 2007-12-01 20:13         ` Eric W. Biederman
  2007-12-01 20:21           ` Mark Lord
  1 sibling, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01 20:13 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Alan Cox, Mark Lord, Greg Kroah-Hartman, Greg KH, Tejun Heo,
	Linux Containers, netdev, cornelia.huck, stern, kay.sievers,
	linux-kernel, Andrew Morton, Herbert Xu, David Miller,
	Linus Torvalds

Stephen Hemminger <shemminger@linux-foundation.org> writes:

> Actually, the whole mess would go away if the api for dev_get_by_XXXX hadn't
> been changed in the namespace transition. IMHO the interface to
> dev_get_by_name()
> should not have added a namespace parameter, of the callers in the tree, only
> two use a different namespace. So it would have been better to to introduce
> dev_get_by_name_ns() with the extra parameter.

As a general rule if you are calling dev_get_by_name and taking an &init_net
parameter that means you code has not yet been converted to actually support
network namespaces.

Not everything can be safely changed at once so we take it by steps.  When
the code fully supports network namespaces practically nothing will take
an &init_net parameter.  The network namespace parameter will come in
some form from userspace.  Either from current or from the network
socket.

Except for boot time initialization I don't know of any cases using
dev_get_by_XXXX that won't need to be modified before the network
namespace work is complete.

I believe I mentioned that this getting the fully network namespace
support was going to take a while and a bunch of patches at the
outset.

> Can we get this resolved before 2.6.24 is released? Going back and forth
> on API's is just needless frottage.

Sure.  We keep the updated dev_get_by_XXXX that takes a network
namespace parameter.

Or is their some legitimate usage of it by out of tree code that
I'm not aware of?

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 20:13         ` Eric W. Biederman
@ 2007-12-01 20:21           ` Mark Lord
  2007-12-01 20:29             ` Arjan van de Ven
                               ` (2 more replies)
  0 siblings, 3 replies; 64+ messages in thread
From: Mark Lord @ 2007-12-01 20:21 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Stephen Hemminger, Alan Cox, Greg Kroah-Hartman, Greg KH,
	Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller, Linus Torvalds

Eric W. Biederman wrote:
> Stephen Hemminger <shemminger@linux-foundation.org> writes:
> 
>> Actually, the whole mess would go away if the api for dev_get_by_XXXX hadn't
>> been changed in the namespace transition. IMHO the interface to
>> dev_get_by_name()
>> should not have added a namespace parameter, of the callers in the tree, only
>> two use a different namespace. So it would have been better to to introduce
>> dev_get_by_name_ns() with the extra parameter.
> 
> As a general rule if you are calling dev_get_by_name and taking an &init_net
> parameter that means you code has not yet been converted to actually support
> network namespaces.
> 
> Not everything can be safely changed at once so we take it by steps.  When
> the code fully supports network namespaces practically nothing will take
> an &init_net parameter.  The network namespace parameter will come in
> some form from userspace.  Either from current or from the network
> socket.
> 
> Except for boot time initialization I don't know of any cases using
> dev_get_by_XXXX that won't need to be modified before the network
> namespace work is complete.
> 
> I believe I mentioned that this getting the fully network namespace
> support was going to take a while and a bunch of patches at the
> outset.
> 
>> Can we get this resolved before 2.6.24 is released? Going back and forth
>> on API's is just needless frottage.
> 
> Sure.  We keep the updated dev_get_by_XXXX that takes a network
> namespace parameter.
..

And what should code be passing in when "# CONFIG_NET_NS is not set" ?

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 20:21           ` Mark Lord
@ 2007-12-01 20:29             ` Arjan van de Ven
  2007-12-01 22:12               ` Mark Lord
  2007-12-01 20:52             ` Eric W. Biederman
  2007-12-01 22:13             ` Mark Lord
  2 siblings, 1 reply; 64+ messages in thread
From: Arjan van de Ven @ 2007-12-01 20:29 UTC (permalink / raw)
  To: Mark Lord
  Cc: Eric W. Biederman, Stephen Hemminger, Alan Cox,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

On Sat, 01 Dec 2007 15:21:12 -0500
Mark Lord <lkml@rtr.ca> wrote:

> Eric W. Biederman wrote:
> > Stephen Hemminger <shemminger@linux-foundation.org> writes:
> > Sure.  We keep the updated dev_get_by_XXXX that takes a network
> > namespace parameter.
> ..
> 
> And what should code be passing in when "# CONFIG_NET_NS is not set" ?

network drivers probably really really don't want to call
dev_get_by_XXX...

in fact no NIC driver in drivers/net does so!
Sounds like whatever driver you're looking at has a nasty bug in that
it's using non-driver APIs...



-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 20:21           ` Mark Lord
  2007-12-01 20:29             ` Arjan van de Ven
@ 2007-12-01 20:52             ` Eric W. Biederman
  2007-12-01 22:13             ` Mark Lord
  2 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01 20:52 UTC (permalink / raw)
  To: Mark Lord
  Cc: Stephen Hemminger, Alan Cox, Greg Kroah-Hartman, Greg KH,
	Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller, Linus Torvalds

Mark Lord <lkml@rtr.ca> writes:

>>> Can we get this resolved before 2.6.24 is released? Going back and forth
>>> on API's is just needless frottage.
>>
>> Sure.  We keep the updated dev_get_by_XXXX that takes a network
>> namespace parameter.
> ..
>
> And what should code be passing in when "# CONFIG_NET_NS is not set" ?

Mostly CONFIG_NET_NS is a define to keep us from exposing the feature to
user space not to remove the code impact.  People could not stand the
look of the code that would actually allow us to compile everything out.

So all of the struct net * fields remain when !CONFIG_NET_NS.
Including the global variable init_net.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 20:29             ` Arjan van de Ven
@ 2007-12-01 22:12               ` Mark Lord
  2007-12-01 23:13                 ` Eric W. Biederman
  0 siblings, 1 reply; 64+ messages in thread
From: Mark Lord @ 2007-12-01 22:12 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Eric W. Biederman, Stephen Hemminger, Alan Cox,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

Arjan van de Ven wrote:
> On Sat, 01 Dec 2007 15:21:12 -0500
> Mark Lord <lkml@rtr.ca> wrote:
> 
>> Eric W. Biederman wrote:
>>> Stephen Hemminger <shemminger@linux-foundation.org> writes:
>>> Sure.  We keep the updated dev_get_by_XXXX that takes a network
>>> namespace parameter.
>> ..
>>
>> And what should code be passing in when "# CONFIG_NET_NS is not set" ?
> 
> network drivers probably really really don't want to call
> dev_get_by_XXX...
..

Fine.  But all of them want to call sk_alloc(),
and many want to do register_netdev().

So what should they be using there ?

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 20:21           ` Mark Lord
  2007-12-01 20:29             ` Arjan van de Ven
  2007-12-01 20:52             ` Eric W. Biederman
@ 2007-12-01 22:13             ` Mark Lord
  2 siblings, 0 replies; 64+ messages in thread
From: Mark Lord @ 2007-12-01 22:13 UTC (permalink / raw)
  To: Eric W. Biederman, arjan
  Cc: Stephen Hemminger, Alan Cox, Greg Kroah-Hartman, Greg KH,
	Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller, Linus Torvalds

Arjan van de Ven wrote:
> On Sat, 01 Dec 2007 15:21:12 -0500
> Mark Lord <lkml@rtr.ca> wrote:
>
>> Eric W. Biederman wrote:
>>> Stephen Hemminger <shemminger@linux-foundation.org> writes:
>>> Sure.  We keep the updated dev_get_by_XXXX that takes a network
>>> namespace parameter.
>> ..
>>
>> And what should code be passing in when "# CONFIG_NET_NS is not set" ?
>
> network drivers probably really really don't want to call
> dev_get_by_XXX...
..

Fine.  But all of them want to call sk_alloc(),
and many want to do register_netdev().

So what should they be using there ?

And please STOP trimming the CC list.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 22:12               ` Mark Lord
@ 2007-12-01 23:13                 ` Eric W. Biederman
  2007-12-01 23:24                   ` Jiri Slaby
  2007-12-01 23:51                   ` Mark Lord
  0 siblings, 2 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-01 23:13 UTC (permalink / raw)
  To: Mark Lord
  Cc: Arjan van de Ven, Stephen Hemminger, Alan Cox,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

Mark Lord <lkml@rtr.ca> writes:

> Arjan van de Ven wrote:
>> On Sat, 01 Dec 2007 15:21:12 -0500
>> Mark Lord <lkml@rtr.ca> wrote:
>>
>>> Eric W. Biederman wrote:
>>>> Stephen Hemminger <shemminger@linux-foundation.org> writes:
>>>> Sure.  We keep the updated dev_get_by_XXXX that takes a network
>>>> namespace parameter.
>>> ..
>>>
>>> And what should code be passing in when "# CONFIG_NET_NS is not set" ?
>>
>> network drivers probably really really don't want to call
>> dev_get_by_XXX...
> ..
>
> Fine.  But all of them want to call sk_alloc(),

network drivers should be calling sk_alloc less then they should
call dev_get_by_XXXX.  Only protocols call sk_alloc.

> and many want to do register_netdev().

I haven't even touched register_netdev.

> So what should they be using there ?

What are you having problems with?

It is hard to answer specific questions without a context.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 23:13                 ` Eric W. Biederman
@ 2007-12-01 23:24                   ` Jiri Slaby
  2007-12-02  1:14                     ` Eric W. Biederman
  2007-12-01 23:51                   ` Mark Lord
  1 sibling, 1 reply; 64+ messages in thread
From: Jiri Slaby @ 2007-12-01 23:24 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Mark Lord, Arjan van de Ven, Stephen Hemminger, Alan Cox,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

On 12/02/2007 12:13 AM, Eric W. Biederman wrote:
> Mark Lord <lkml@rtr.ca> writes:
>> Fine.  But all of them want to call sk_alloc(),
> 
> network drivers should be calling sk_alloc less then they should
> call dev_get_by_XXXX.  Only protocols call sk_alloc.
> 
>> and many want to do register_netdev().
> 
> I haven't even touched register_netdev.
> 
>> So what should they be using there ?
> 
> What are you having problems with?
> 
> It is hard to answer specific questions without a context.

VMware vmnet.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 23:13                 ` Eric W. Biederman
  2007-12-01 23:24                   ` Jiri Slaby
@ 2007-12-01 23:51                   ` Mark Lord
  2007-12-02  1:08                     ` Eric W. Biederman
  1 sibling, 1 reply; 64+ messages in thread
From: Mark Lord @ 2007-12-01 23:51 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Arjan van de Ven, Stephen Hemminger, Alan Cox,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

Eric W. Biederman wrote:
> Mark Lord <lkml@rtr.ca> writes:
> 
>> Arjan van de Ven wrote:
>>> On Sat, 01 Dec 2007 15:21:12 -0500
>>> Mark Lord <lkml@rtr.ca> wrote:
>>>
>>>> Eric W. Biederman wrote:
>>>>> Stephen Hemminger <shemminger@linux-foundation.org> writes:
>>>>> Sure.  We keep the updated dev_get_by_XXXX that takes a network
>>>>> namespace parameter.
>>>> ..
>>>>
>>>> And what should code be passing in when "# CONFIG_NET_NS is not set" ?
>>> network drivers probably really really don't want to call
>>> dev_get_by_XXX...
>> ..
>>
>> Fine.  But all of them want to call sk_alloc(),
> 
> network drivers should be calling sk_alloc less then they should
> call dev_get_by_XXXX.  Only protocols call sk_alloc.
..

I think I saw some bridge code that calls it, too.
Regardless, it's EXPORT_SYMBOL(), but now unusable
due to the GPL-only symbol "init_net".

Meanwhile, you are avoiding answering the question:

>>>> And what should code be passing in when "# CONFIG_NET_NS is not set" ?
..

>> and many want to do register_netdev().
> 
> I haven't even touched register_netdev.
..

Bull-pucky.  Somebody did:

> @@ -3361,6 +3595,8 @@ int register_netdevice(struct net_device *dev)
> 
>         /* When net_device's are persistent, this will be fatal. */
>         BUG_ON(dev->reg_state != NETREG_UNINITIALIZED);
> +       BUG_ON(!dev->nd_net);
> +       net = dev->nd_net;
> 
>         spin_lock_init(&dev->queue_lock);
>         spin_lock_init(&dev->_xmit_lock);
..

That new BUG_ON() line complains if nd_net has not been initialized,
and the only thing I see drivers putting there is GPL-only "&init_net".

>> So what should they be using there ?
> 
> What are you having problems with?
..

Avoiding the question again there, too.

I personally am not having problems with anything.
But I noticed the API change, and would like somebody to fix it.
Any of the three ways (or a fourth, if you've got one) might do.

Cheers

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 19:17   ` Stephen Hemminger
  2007-12-01 19:23     ` Alan Cox
  2007-12-01 19:54     ` Eric W. Biederman
@ 2007-12-02  0:30     ` Stephen Hemminger
  2007-12-02  2:02       ` Eric W. Biederman
                         ` (2 more replies)
  2 siblings, 3 replies; 64+ messages in thread
From: Stephen Hemminger @ 2007-12-02  0:30 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev, containers, netdev, linux-kernel

On Sat, 1 Dec 2007 11:17:36 -0800
Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> Then init_net needs to be not GPL limited. Sorry, we need to allow
> non GPL network drivers.  There is a fine line between keeping the
> binary seething masses from accessing random kernel functions, and allowing
> reasonable (but still non GPL) things like ndiswrapper to use network
> device interface.
> 
I spoke too soon earlier, ndiswrapper builds and loads against current
2.6.24-rc3. Vmware and proprietary VPN software probably do not. Once again I don't
give a damn, but the enterprise distro vendors certainly care.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

-- 
Stephen Hemminger <shemminger@linux-foundation.org>


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 23:51                   ` Mark Lord
@ 2007-12-02  1:08                     ` Eric W. Biederman
  0 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-02  1:08 UTC (permalink / raw)
  To: Mark Lord
  Cc: Arjan van de Ven, Stephen Hemminger, Alan Cox,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

Mark Lord <lkml@rtr.ca> writes:

> Eric W. Biederman wrote:
>> Mark Lord <lkml@rtr.ca> writes:
>>
>>> Arjan van de Ven wrote:
>>>> On Sat, 01 Dec 2007 15:21:12 -0500
>>>> Mark Lord <lkml@rtr.ca> wrote:
>>>>
>>>>> Eric W. Biederman wrote:
>>>>>> Stephen Hemminger <shemminger@linux-foundation.org> writes:
>>>>>> Sure.  We keep the updated dev_get_by_XXXX that takes a network
>>>>>> namespace parameter.
>>>>> ..
>>>>>
>>>>> And what should code be passing in when "# CONFIG_NET_NS is not set" ?
>>>> network drivers probably really really don't want to call
>>>> dev_get_by_XXX...
>>> ..
>>>
>>> Fine.  But all of them want to call sk_alloc(),
>>
>> network drivers should be calling sk_alloc less then they should
>> call dev_get_by_XXXX.  Only protocols call sk_alloc.
> ..
>
> I think I saw some bridge code that calls it, too.
> Regardless, it's EXPORT_SYMBOL(), but now unusable
> due to the GPL-only symbol "init_net".
>
> Meanwhile, you are avoiding answering the question:

What EXPORT_SYMBOL_GPL means is I don't have a clue how you
can use this without being a derivative work of the linux kernel.  I
think that is true for init_net and I stand by that.  It is a very
internal symbol.

Further the only case that I think might pass the derivative work
test would be a network driver, and I am not affecting those.

I also believe that since it isn't mandatory you get your struct
net pointer by taking the address of a GPL only symbol that
those interfaces can continue to be used.

However I do suspect you are right that it may be more correct
to realized that only a derivative work could possibly use those
interfaces and tell 3rd parties that loud and clear.

We also have the policy of not changing exports from EXPORT_SYMBOL
to EXPORT_SYMBOL_GPL.  So making that such a change is difficult.

>>>>> And what should code be passing in when "# CONFIG_NET_NS is not set" ?
> ..
>
>>> and many want to do register_netdev().
>>
>> I haven't even touched register_netdev.
> ..
>
> Bull-pucky.  Somebody did:
>
>> @@ -3361,6 +3595,8 @@ int register_netdevice(struct net_device *dev)
>>
>>         /* When net_device's are persistent, this will be fatal. */
>>         BUG_ON(dev->reg_state != NETREG_UNINITIALIZED);
>> +       BUG_ON(!dev->nd_net);
>> +       net = dev->nd_net;
>>
>>         spin_lock_init(&dev->queue_lock);
>>         spin_lock_init(&dev->_xmit_lock);
> ..
>
> That new BUG_ON() line complains if nd_net has not been initialized,
> and the only thing I see drivers putting there is GPL-only "&init_net".

Yes and alloc_netdev initializes it.  All network drivers are required
to call alloc_netdev.

So register_netdev from a device driver perspective has not changed.

Anything the above change breaks happens to be a broken network device
driver.

>>> So what should they be using there ?
>>
>> What are you having problems with?
> ..
>
> Avoiding the question again there, too.

No.  I am saying that I don't see a problem.  All of the real world
problems that I actually know of are code bugs.  I asked for
a real world problem to see if there was something I was missing
in my analysis.  That isn't avoiding the problem that is trying
to see if I was wrong.

> I personally am not having problems with anything.
> But I noticed the API change, and would like somebody to fix it.
> Any of the three ways (or a fourth, if you've got one) might do.

Honestly I think I have done the best I can with the knowledge and
information that I have.    I do not think dropping the GPL from
the export of init_net makes sense.  I think turning a NULL into
init_net is a technical joke.  That parameter is there because
that is someplace where we need to pay attention to our network
namespace.

Which only leaves your first option of making more symbols
EXPORT_SYMBOL_GPL as even interesting.

That would seem to make some sense, given the attitudes and
perceptions of the current network developers.  

However I'm not volunteering for that one as finishing up the network
and pid namespaces already has my plate full.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-01 23:24                   ` Jiri Slaby
@ 2007-12-02  1:14                     ` Eric W. Biederman
  0 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-02  1:14 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Mark Lord, Arjan van de Ven, Stephen Hemminger, Alan Cox,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

Jiri Slaby <jirislaby@gmail.com> writes:

> On 12/02/2007 12:13 AM, Eric W. Biederman wrote:
>> Mark Lord <lkml@rtr.ca> writes:
>>> Fine.  But all of them want to call sk_alloc(),
>> 
>> network drivers should be calling sk_alloc less then they should
>> call dev_get_by_XXXX.  Only protocols call sk_alloc.
>> 
>>> and many want to do register_netdev().
>> 
>> I haven't even touched register_netdev.
>> 
>>> So what should they be using there ?
>> 
>> What are you having problems with?
>> 
>> It is hard to answer specific questions without a context.
>
> VMware vmnet.

With a quick glance in that direction it appears to the result
of a design bug in vmnet that they call sk_alloc at all, and someone
seems to have found a work around in vmnet for this situation.

My gut feel is that vmware should just use tun or tap (whichever
is the appropriate one), and be done with it.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02  0:30     ` Stephen Hemminger
@ 2007-12-02  2:02       ` Eric W. Biederman
  2007-12-02  3:34       ` Mark Lord
  2007-12-02 13:51       ` Alan Cox
  2 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-02  2:02 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, containers, linux-kernel

Stephen Hemminger <shemminger@linux-foundation.org> writes:

> On Sat, 1 Dec 2007 11:17:36 -0800
> Stephen Hemminger <shemminger@linux-foundation.org> wrote:
>
>> Then init_net needs to be not GPL limited. Sorry, we need to allow
>> non GPL network drivers.  There is a fine line between keeping the
>> binary seething masses from accessing random kernel functions, and allowing
>> reasonable (but still non GPL) things like ndiswrapper to use network
>> device interface.
>> 
> I spoke too soon earlier, ndiswrapper builds and loads against current
> 2.6.24-rc3. Vmware and proprietary VPN software probably do not. Once again I
> don't
> give a damn, but the enterprise distro vendors certainly care.

It looks like someone found a work around for vmware.

As for proprietary VPN software, the one case that came of David Miller
said we had sufficient alternatives in the kernel that he didn't care.

We have to make a lot of changes to get the network namespaces
complete.  I don't have pity on any external code that breaks because
they won't let me at their code.

If it turns out the code that was broken was an unacknowledged
derivative work and it can't be fixed I have even less pity on
them.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02  0:30     ` Stephen Hemminger
  2007-12-02  2:02       ` Eric W. Biederman
@ 2007-12-02  3:34       ` Mark Lord
  2007-12-02  4:23         ` Stephen Hemminger
  2007-12-03  8:24         ` Romano Giannetti
  2007-12-02 13:51       ` Alan Cox
  2 siblings, 2 replies; 64+ messages in thread
From: Mark Lord @ 2007-12-02  3:34 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: linux-kernel, netdev, containers

Stephen Hemminger wrote:
> On Sat, 1 Dec 2007 11:17:36 -0800
> Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> 
>> Then init_net needs to be not GPL limited. Sorry, we need to allow
>> non GPL network drivers.  There is a fine line between keeping the
>> binary seething masses from accessing random kernel functions, and allowing
>> reasonable (but still non GPL) things like ndiswrapper to use network
>> device interface.
>>
> I spoke too soon earlier, ndiswrapper builds and loads against current
> 2.6.24-rc3. Vmware and proprietary VPN software probably do not. Once again I don't
> give a damn, but the enterprise distro vendors certainly care.
...

Naw, enterprise (or any other) distro vendors shouldn't have any issues here,
since they can just patch their kernels around any issues.

But it looks like Eric has this one thought out well enough.

Cheers


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02  3:34       ` Mark Lord
@ 2007-12-02  4:23         ` Stephen Hemminger
  2007-12-02 19:28           ` Ben Greear
  2007-12-03  8:24         ` Romano Giannetti
  1 sibling, 1 reply; 64+ messages in thread
From: Stephen Hemminger @ 2007-12-02  4:23 UTC (permalink / raw)
  To: Mark Lord; +Cc: linux-kernel, netdev, containers

On Sat, 01 Dec 2007 22:34:09 -0500
Mark Lord <lkml@rtr.ca> wrote:

> Stephen Hemminger wrote:
> > On Sat, 1 Dec 2007 11:17:36 -0800
> > Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > 
> >> Then init_net needs to be not GPL limited. Sorry, we need to allow
> >> non GPL network drivers.  There is a fine line between keeping the
> >> binary seething masses from accessing random kernel functions, and allowing
> >> reasonable (but still non GPL) things like ndiswrapper to use network
> >> device interface.
> >>
> > I spoke too soon earlier, ndiswrapper builds and loads against current
> > 2.6.24-rc3. Vmware and proprietary VPN software probably do not. Once again I don't
> > give a damn, but the enterprise distro vendors certainly care.
> ...
> 
> Naw, enterprise (or any other) distro vendors shouldn't have any issues here,
> since they can just patch their kernels around any issues.
> 
> But it looks like Eric has this one thought out well enough.

So you are saying all this is not a problem, fine.
Any affected parties can certainly lobby for themselves. But I suspect
they all think the kernel community is a bunch of ... and will just ignore
the problem. 

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02  0:30     ` Stephen Hemminger
  2007-12-02  2:02       ` Eric W. Biederman
  2007-12-02  3:34       ` Mark Lord
@ 2007-12-02 13:51       ` Alan Cox
  2007-12-02 19:56         ` Valdis.Kletnieks
  2 siblings, 1 reply; 64+ messages in thread
From: Alan Cox @ 2007-12-02 13:51 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: linux-kernel, netdev, containers

On Sat, 1 Dec 2007 16:30:35 -0800
Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> On Sat, 1 Dec 2007 11:17:36 -0800
> Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> 
> > Then init_net needs to be not GPL limited. Sorry, we need to allow
> > non GPL network drivers.  There is a fine line between keeping the
> > binary seething masses from accessing random kernel functions, and allowing
> > reasonable (but still non GPL) things like ndiswrapper to use network
> > device interface.
> > 
> I spoke too soon earlier, ndiswrapper builds and loads against current
> 2.6.24-rc3. Vmware and proprietary VPN software probably do not. Once again I don't
> give a damn, but the enterprise distro vendors certainly care.

Enterprise distro vendors ship kernels from the 2.6.19 era, so I don't
see why they care.

Alan

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02  4:23         ` Stephen Hemminger
@ 2007-12-02 19:28           ` Ben Greear
  2007-12-02 20:03             ` Patrick McHardy
  0 siblings, 1 reply; 64+ messages in thread
From: Ben Greear @ 2007-12-02 19:28 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Mark Lord, linux-kernel, netdev, containers

Stephen Hemminger wrote:
>>
>> Naw, enterprise (or any other) distro vendors shouldn't have any issues here,
>> since they can just patch their kernels around any issues.
>>
>> But it looks like Eric has this one thought out well enough.
>>     
>
> So you are saying all this is not a problem, fine.
> Any affected parties can certainly lobby for themselves. But I suspect
> they all think the kernel community is a bunch of ... and will just ignore
> the problem. 
>   
I have a binary module that uses dev_get_by_name...it's sort of a 
bridge-like thing and
needs user-space to tell it which device to listen for packets on...

This code doesn't need or care about name-spaces, so I don't see how it 
could really
be infringing on the author's code (any worse than loading a binary 
driver into the kernel
ever does).

I would certainly prefer to not have to patch around any problems with 
calling dev_get_by_name
from a non-gpl module, but if required, I can probably figure something 
out...

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02 13:51       ` Alan Cox
@ 2007-12-02 19:56         ` Valdis.Kletnieks
  0 siblings, 0 replies; 64+ messages in thread
From: Valdis.Kletnieks @ 2007-12-02 19:56 UTC (permalink / raw)
  To: Alan Cox; +Cc: Stephen Hemminger, linux-kernel, netdev, containers

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Sun, 02 Dec 2007 13:51:04 GMT, Alan Cox said:
> On Sat, 1 Dec 2007 16:30:35 -0800
> > I spoke too soon earlier, ndiswrapper builds and loads against current
> > 2.6.24-rc3. Vmware and proprietary VPN software probably do not. Once again I don't
> > give a damn, but the enterprise distro vendors certainly care.
> 
> Enterprise distro vendors ship kernels from the 2.6.19 era, so I don't
> see why they care.

They don't care *now*.  They will care when they try to rev forward from .19.

Not that they'll care a *lot* - it took *me* all of about an hour to get VMware
Server 1.0.4 working under -rc3-mm2.  Probably will take an enterprise distro
4-5 hours, 30 mins for the port and 4 1/2 hours for the paperwork. :)


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02 19:28           ` Ben Greear
@ 2007-12-02 20:03             ` Patrick McHardy
  2007-12-02 20:43               ` Adrian Bunk
  2007-12-03 17:35               ` Eric W. Biederman
  0 siblings, 2 replies; 64+ messages in thread
From: Patrick McHardy @ 2007-12-02 20:03 UTC (permalink / raw)
  To: Ben Greear; +Cc: Stephen Hemminger, Mark Lord, linux-kernel, netdev, containers

Ben Greear wrote:
> Stephen Hemminger wrote:
>>>
>>> Naw, enterprise (or any other) distro vendors shouldn't have any 
>>> issues here,
>>> since they can just patch their kernels around any issues.
>>>
>>> But it looks like Eric has this one thought out well enough.
>>>     
>>
>> So you are saying all this is not a problem, fine.
>> Any affected parties can certainly lobby for themselves. But I suspect
>> they all think the kernel community is a bunch of ... and will just 
>> ignore
>> the problem.   
 >
> I have a binary module that uses dev_get_by_name...it's sort of a 
> bridge-like thing and
> needs user-space to tell it which device to listen for packets on...
> 
> This code doesn't need or care about name-spaces, so I don't see how it 
> could really
> be infringing on the author's code (any worse than loading a binary 
> driver into the kernel
> ever does).
> 
> I would certainly prefer to not have to patch around any problems with 
> calling dev_get_by_name
> from a non-gpl module, but if required, I can probably figure something 
> out...


For all I care binary modules can break, but frankly I don't see
how encapsulating a couple of structures and pointers in a new
structure and adding a new argument to existing functions shifts
the decision about how a function should be usable to the namespace
guys. IMO all functions should continue to be usable as before,
as decided by whoever actually wrote them. The only exception
might be stuff where an existing EXPORT_SYMBOL is clearly wrong,
but that would be a seperate discussion.


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02 20:03             ` Patrick McHardy
@ 2007-12-02 20:43               ` Adrian Bunk
  2007-12-02 21:59                 ` Patrick McHardy
  2007-12-03 17:35               ` Eric W. Biederman
  1 sibling, 1 reply; 64+ messages in thread
From: Adrian Bunk @ 2007-12-02 20:43 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Ben Greear, Stephen Hemminger, Mark Lord, linux-kernel, netdev,
	containers

On Sun, Dec 02, 2007 at 09:03:56PM +0100, Patrick McHardy wrote:
> Ben Greear wrote:
>> Stephen Hemminger wrote:
>>>>
>>>> Naw, enterprise (or any other) distro vendors shouldn't have any issues 
>>>> here,
>>>> since they can just patch their kernels around any issues.
>>>>
>>>> But it looks like Eric has this one thought out well enough.
>>>>     
>>>
>>> So you are saying all this is not a problem, fine.
>>> Any affected parties can certainly lobby for themselves. But I suspect
>>> they all think the kernel community is a bunch of ... and will just 
>>> ignore
>>> the problem.   
> >
>> I have a binary module that uses dev_get_by_name...it's sort of a 
>> bridge-like thing and
>> needs user-space to tell it which device to listen for packets on...
>>
>> This code doesn't need or care about name-spaces, so I don't see how it 
>> could really
>> be infringing on the author's code (any worse than loading a binary driver 
>> into the kernel
>> ever does).
>>
>> I would certainly prefer to not have to patch around any problems with 
>> calling dev_get_by_name
>> from a non-gpl module, but if required, I can probably figure something 
>> out...
>
>
> For all I care binary modules can break, but frankly I don't see
> how encapsulating a couple of structures and pointers in a new
> structure and adding a new argument to existing functions shifts
> the decision about how a function should be usable to the namespace
> guys. IMO all functions should continue to be usable as before,
> as decided by whoever actually wrote them.
>...

Even ignoring the fact that it's unclear whether distributing modules 
with not GPLv2 compatible licences is legal at all or might bring you in 
jail, your statement has an interesting implication:

Stuff like e.g. the EXPORT_SYMBOL(sk_alloc) predates the 
EXPORT_SYMBOL_GPL stuff.

Who is considered the author of this code?

And when should he state whether he prefers to use EXPORT_SYMBOL_GPL 
but wasn't able to use it at that when he wrote it since his code 
predates it and is glad to be able to decide this now?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02 20:43               ` Adrian Bunk
@ 2007-12-02 21:59                 ` Patrick McHardy
  2007-12-03  1:14                   ` Adrian Bunk
  2007-12-03  8:33                   ` Denis V. Lunev
  0 siblings, 2 replies; 64+ messages in thread
From: Patrick McHardy @ 2007-12-02 21:59 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Ben Greear, Stephen Hemminger, Mark Lord, linux-kernel, netdev,
	containers

Adrian Bunk wrote:
> On Sun, Dec 02, 2007 at 09:03:56PM +0100, Patrick McHardy wrote:
>
>> For all I care binary modules can break, but frankly I don't see
>> how encapsulating a couple of structures and pointers in a new
>> structure and adding a new argument to existing functions shifts
>> the decision about how a function should be usable to the namespace
>> guys. IMO all functions should continue to be usable as before,
>> as decided by whoever actually wrote them.
>> ...
> 
> Even ignoring the fact that it's unclear whether distributing modules 
> with not GPLv2 compatible licences is legal at all or might bring you in 
> jail,

Agreed, lets ignore that :)

> your statement has an interesting implication:
> 
> Stuff like e.g. the EXPORT_SYMBOL(sk_alloc) predates the 
> EXPORT_SYMBOL_GPL stuff.
> 
> Who is considered the author of this code?
> 
> And when should he state whether he prefers to use EXPORT_SYMBOL_GPL 
> but wasn't able to use it at that when he wrote it since his code 
> predates it and is glad to be able to decide this now?


He can state it when he feels like it, I don't see the point.
Authors generally get to decide whether they use EXPORT_SYMBOL
or EXPORT_SYMBOL_GPL unless in cases where its really clear-cut
that EXPORT_SYMBOL is inapproriate. But thats a different matter.

If a symbol was OK to be used previously and something using it
would not automatically be considered a derived work, how does
passing &init_net to the function just to make the compiler
happy, avoid BUG_ONs and generally keep things working as before
make it more of a derived work?

^ permalink raw reply	[flat|nested] 64+ messages in thread

* RE: namespace support requires network modules to say "GPL"
  2007-12-01 19:23     ` Alan Cox
  2007-12-01 19:38       ` Stephen Hemminger
@ 2007-12-03  0:02       ` David Schwartz
  2007-12-03  0:14         ` Alan Cox
  1 sibling, 1 reply; 64+ messages in thread
From: David Schwartz @ 2007-12-03  0:02 UTC (permalink / raw)
  To: Stephen Hemminger, Alan Cox
  Cc: Mark Lord, Eric W. Biederman, Greg Kroah-Hartman, Greg KH,
	Tejun Heo, Linux Containers, netdev, cornelia.huck, stern,
	kay.sievers, linux-kernel, Andrew Morton, Herbert Xu,
	David Miller, Linus Torvalds


> > Then init_net needs to be not GPL limited. Sorry, we need to allow
> > non GPL network drivers.  There is a fine line between keeping the

> Why - they aren't exactly likely to be permissible by law

Really? What law and/or what clause in the GPL says that derivative works
have to be licensed under the GPL? Or does the kernel have some new
technique to determine whether or not code has been distributed?

As I read the GPL, it only requires you to release something under the GPL
if you distribute it. The kernel has no idea whether or not code has been
distributed. So if it's enforcing the GPL, it cannot prohibit anything
non-distributed code can lawfully do. (Ergo, it's *NOT* *ENFORCING* the
GPL.)

> > binary seething masses from accessing random kernel functions,
> and allowing
> > reasonable (but still non GPL) things like ndiswrapper to use network
> > device interface.
>
> Its up to the ndiswrapper authors how the licence their code, but they
> should respect how we licence ours.

You license yours under the GPL, so they should respect the GPL.

It sounds like we're back to where we were years ago. Didn't we already
agree that EXPORT_SYMBOL_GPL was *NOT* a GPL-enforcement mechanism and had
nothing to do with respecting the GPL? After all, if it s a GPL-enforcement
mechanism, why is it not a "further restriction" which is prohibited by the
GPL? (The GPL contains no restrictions on what code can use what symbols if
that code is not distributed, but EXPORT_SYMBOL_GPL does.)

Are you now claiming that EXPORT_SYMBOL_GPL is intended to enforce the GPL?

DS



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03  0:02       ` David Schwartz
@ 2007-12-03  0:14         ` Alan Cox
  0 siblings, 0 replies; 64+ messages in thread
From: Alan Cox @ 2007-12-03  0:14 UTC (permalink / raw)
  To: davids
  Cc: Stephen Hemminger, Mark Lord, Eric W. Biederman,
	Greg Kroah-Hartman, Greg KH, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller, Linus Torvalds

> You license yours under the GPL, so they should respect the GPL.
> 
> It sounds like we're back to where we were years ago. Didn't we already
> agree that EXPORT_SYMBOL_GPL was *NOT* a GPL-enforcement mechanism and had
> nothing to do with respecting the GPL? After all, if it s a GPL-enforcement

No we seem to be back recycling the fact that certain people were making
statements that might be construed, unanswered, as giving permission to
violate the GPL.

I'm merely reminding people that I've not waived my GPL rights, I've not
said modules are somehow magically OK, and I don't agree with Linus. 

The GPL very clearly says that you can make your own unredistributed
modifications and keep them that way.

Alan

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02 21:59                 ` Patrick McHardy
@ 2007-12-03  1:14                   ` Adrian Bunk
  2007-12-03  8:33                   ` Denis V. Lunev
  1 sibling, 0 replies; 64+ messages in thread
From: Adrian Bunk @ 2007-12-03  1:14 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Ben Greear, Stephen Hemminger, Mark Lord, linux-kernel, netdev,
	containers

On Sun, Dec 02, 2007 at 10:59:46PM +0100, Patrick McHardy wrote:
> Adrian Bunk wrote:
>> On Sun, Dec 02, 2007 at 09:03:56PM +0100, Patrick McHardy wrote:
>...
>> your statement has an interesting implication:
>>
>> Stuff like e.g. the EXPORT_SYMBOL(sk_alloc) predates the EXPORT_SYMBOL_GPL 
>> stuff.
>>
>> Who is considered the author of this code?
>>
>> And when should he state whether he prefers to use EXPORT_SYMBOL_GPL but 
>> wasn't able to use it at that when he wrote it since his code predates it 
>> and is glad to be able to decide this now?
>
> He can state it when he feels like it, I don't see the point.
> Authors generally get to decide whether they use EXPORT_SYMBOL
> or EXPORT_SYMBOL_GPL unless in cases where its really clear-cut
> that EXPORT_SYMBOL is inapproriate. But thats a different matter.
>...

You miss my point.

Stuff like sk_alloc was exported to modules before EXPORT_SYMBOL_GPL 
existed (it was even exported to modules before EXPORT_SYMBOL existed).

We are talking about code and exports that are at about 12 years old, 
which is at about twice as old as EXPORT_SYMBOL_GPL.

So what should happen in your opinion if e.g. Alan checks which of the 
network code he had written when it was exported a dozen years ago, 
stating that he never wanted it to be available to non-GPL modules?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02  3:34       ` Mark Lord
  2007-12-02  4:23         ` Stephen Hemminger
@ 2007-12-03  8:24         ` Romano Giannetti
  2007-12-03 15:34           ` Arjan van de Ven
  2007-12-03 18:03           ` Eric W. Biederman
  1 sibling, 2 replies; 64+ messages in thread
From: Romano Giannetti @ 2007-12-03  8:24 UTC (permalink / raw)
  To: Mark Lord; +Cc: Stephen Hemminger, linux-kernel, netdev, containers



On Sat, 2007-12-01 at 22:34 -0500, Mark Lord wrote:
> Stephen Hemminger wrote:.
> >>
> > I spoke too soon earlier, ndiswrapper builds and loads against current
> > 2.6.24-rc3. Vmware and proprietary VPN software probably do not. Once again I don't
> > give a damn, but the enterprise distro vendors certainly care.
> ...
> 
> Naw, enterprise (or any other) distro vendors shouldn't have any issues here,
> since they can just patch their kernels around any issues.

Please pardon me for jumping in; I am not a kernel developer, but I try
to help with debugging whenever I can (and it's not just hand-waving, I
helped to track down a couple of nasty bugs on MMC or ACPI EC,
recently). And I am an engineer and IANAL, so I wouldn't speak about
laws here. But I think it's not just a distribution's problem.

Unfortunately, I need VMware and ndiswrapper to get work done with my
laptop. It's not the perfect world, but the only alternative is to boot
in XP. So I normally stick with vendors kernels and, when I have time to
"play" with new kernel, I go for it. If ndiswrapper and VMware work,
perfect, I can test extensively the new kernel; if I find problems, I
*know* I have to restart without proprietary modules, try to reproduce,
report back. I did it a lot of times.

What I think is that every time VMware or (worst) ndiswrapper breaks,
the kernel loose an awful lot of testers. In the span of time before
Giri and the VMware team post a patch (-rc1 and -rc4, tipically), my
testing activity is just occasional. And I guess a lot of people is in
the same situation. 

These are just my 2cents. I will continue to test new kernels every time
I can, and to use native solutions as often as I can (go, ath5k, go!;
and LabWindows/CVI for Linux, anyone?). But maybe a bit of tolerance can
help everyone...

Back in my hole,

		Romano 


-- 
Sorry for the disclaimer --- ¡I cannot stop it!



--
La presente comunicación tiene carácter confidencial y es para el exclusivo uso del destinatario indicado en la misma. Si Ud. no es el destinatario indicado, le informamos que cualquier forma de distribución, reproducción o uso de esta comunicación y/o de la información contenida en la misma están estrictamente prohibidos por la ley. Si Ud. ha recibido esta comunicación por error, por favor, notifíquelo inmediatamente al remitente contestando a este mensaje y proceda a continuación a destruirlo. Gracias por su colaboración.

This communication contains confidential information. It is for the exclusive use of the intended addressee. If you are not the intended addressee, please note that any form of distribution, copying or use of this communication or the information in it is strictly prohibited by law. If you have received this communication in error, please immediately notify the sender by reply e-mail and destroy this message. Thank you for your cooperation. 

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02 21:59                 ` Patrick McHardy
  2007-12-03  1:14                   ` Adrian Bunk
@ 2007-12-03  8:33                   ` Denis V. Lunev
  1 sibling, 0 replies; 64+ messages in thread
From: Denis V. Lunev @ 2007-12-03  8:33 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Adrian Bunk, Ben Greear, Stephen Hemminger, Mark Lord,
	linux-kernel, netdev, containers

Patrick McHardy wrote:
> Adrian Bunk wrote:
>> On Sun, Dec 02, 2007 at 09:03:56PM +0100, Patrick McHardy wrote:
>>
>>> For all I care binary modules can break, but frankly I don't see
>>> how encapsulating a couple of structures and pointers in a new
>>> structure and adding a new argument to existing functions shifts
>>> the decision about how a function should be usable to the namespace
>>> guys. IMO all functions should continue to be usable as before,
>>> as decided by whoever actually wrote them.
>>> ...
>>
>> Even ignoring the fact that it's unclear whether distributing modules
>> with not GPLv2 compatible licences is legal at all or might bring you
>> in jail,
> 
> Agreed, lets ignore that :)
> 
>> your statement has an interesting implication:
>>
>> Stuff like e.g. the EXPORT_SYMBOL(sk_alloc) predates the
>> EXPORT_SYMBOL_GPL stuff.
>>
>> Who is considered the author of this code?
>>
>> And when should he state whether he prefers to use EXPORT_SYMBOL_GPL
>> but wasn't able to use it at that when he wrote it since his code
>> predates it and is glad to be able to decide this now?
> 
> 
> He can state it when he feels like it, I don't see the point.
> Authors generally get to decide whether they use EXPORT_SYMBOL
> or EXPORT_SYMBOL_GPL unless in cases where its really clear-cut
> that EXPORT_SYMBOL is inapproriate. But thats a different matter.
> 
> If a symbol was OK to be used previously and something using it
> would not automatically be considered a derived work, how does
> passing &init_net to the function just to make the compiler
> happy, avoid BUG_ONs and generally keep things working as before
> make it more of a derived work?

We, namely, Pavel Emelyanov and me, if we have some rights as a
committers to this staff :), do not mind against change
EXPORT_SYMBOL_GPL to EXPORT_SYMBOL.

Regards,
	Den

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03  8:24         ` Romano Giannetti
@ 2007-12-03 15:34           ` Arjan van de Ven
  2007-12-03 18:03           ` Eric W. Biederman
  1 sibling, 0 replies; 64+ messages in thread
From: Arjan van de Ven @ 2007-12-03 15:34 UTC (permalink / raw)
  To: Romano Giannetti
  Cc: Mark Lord, Stephen Hemminger, linux-kernel, netdev, containers

On Mon, 03 Dec 2007 09:24:15 +0100
Romano Giannetti <romanol@upcomillas.es> wrote:

> 
> 
> On Sat, 2007-12-01 at 22:34 -0500, Mark Lord wrote:
> > Stephen Hemminger wrote:.
> > >>
> > > I spoke too soon earlier, ndiswrapper builds and loads against
> > > current 2.6.24-rc3. Vmware and proprietary VPN software probably
> > > do not. Once again I don't give a damn, but the enterprise distro
> > > vendors certainly care.
> > ...
> > 
> > Naw, enterprise (or any other) distro vendors shouldn't have any
> > issues here, since they can just patch their kernels around any
> > issues.
> 
> Please pardon me for jumping in; 

> 
> What I think is that every time VMware or (worst) ndiswrapper breaks,

if you had read the thread... ndiswrapper doesn't break, and vmware
driver had some bugs that, once fixed, no no longer break either....


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-02 20:03             ` Patrick McHardy
  2007-12-02 20:43               ` Adrian Bunk
@ 2007-12-03 17:35               ` Eric W. Biederman
  2007-12-03 18:19                 ` Ben Greear
  1 sibling, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-03 17:35 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Ben Greear, Stephen Hemminger, Mark Lord, linux-kernel, netdev,
	containers

Patrick McHardy <kaber@trash.net> writes:

> Ben Greear wrote:
>> I have a binary module that uses dev_get_by_name...it's sort of a bridge-like
>> thing and
>> needs user-space to tell it which device to listen for packets on...
>>
>> This code doesn't need or care about name-spaces, so I don't see how it could
>> really
>> be infringing on the author's code (any worse than loading a binary driver
>> into the kernel
>> ever does).

Regardless of infringement it is incompatible with a complete network
namespace implementation.  Further it sounds like the module you are
describing defines a kernel ABI without being merged and hopes that
ABI will still be supportable in the future.  Honestly I think doing so
is horrible code maintenance policy.

>> I would certainly prefer to not have to patch around any problems with calling
>> dev_get_by_name
>> from a non-gpl module, but if required, I can probably figure something out...
>
>
> For all I care binary modules can break, but frankly I don't see
> how encapsulating a couple of structures and pointers in a new
> structure and adding a new argument to existing functions shifts
> the decision about how a function should be usable to the namespace
> guys. IMO all functions should continue to be usable as before,
> as decided by whoever actually wrote them. The only exception
> might be stuff where an existing EXPORT_SYMBOL is clearly wrong,
> but that would be a seperate discussion.

I don't think we have actually shifted the decision.

Further from a namespace perspective if I had to support out of tree
modules and the current in kernel API the implementation would be
impossible short of loading kernel modules multiple times once
for each namespace.  I totally refuse to give out of tree modules
that power whatever their license.

Right now the network namespace code that has been merged isn't that
interesting as it does not include ipv4 and ipv6 support which everyone
uses.

One of the tests for completion of the network namespace work is
grepping for &init_net and making certain we have cleanly removed
all references to except in a handful of cases like the boot code.

Once things are largely complete it makes sense to argue with out of
tree module authors that because they don't have network namespace
support in their modules, their modules are broken.   

Right now I suspect to many developers even of in-tree modules
I have just shifted code around in an annoying looking way.  I can
completely see other developers not getting the point.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03  8:24         ` Romano Giannetti
  2007-12-03 15:34           ` Arjan van de Ven
@ 2007-12-03 18:03           ` Eric W. Biederman
  2007-12-03 18:13             ` David Miller
  1 sibling, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-03 18:03 UTC (permalink / raw)
  To: Romano Giannetti
  Cc: Mark Lord, Stephen Hemminger, linux-kernel, netdev, containers

Romano Giannetti <romanol@upcomillas.es> writes:

> Please pardon me for jumping in; I am not a kernel developer, but I try
> to help with debugging whenever I can (and it's not just hand-waving, I
> helped to track down a couple of nasty bugs on MMC or ACPI EC,
> recently). And I am an engineer and IANAL, so I wouldn't speak about
> laws here. But I think it's not just a distribution's problem.
>
> Unfortunately, I need VMware and ndiswrapper to get work done with my
> laptop. It's not the perfect world, but the only alternative is to boot
> in XP. So I normally stick with vendors kernels and, when I have time to
> "play" with new kernel, I go for it. If ndiswrapper and VMware work,
> perfect, I can test extensively the new kernel; if I find problems, I
> *know* I have to restart without proprietary modules, try to reproduce,
> report back. I did it a lot of times.
>
> What I think is that every time VMware or (worst) ndiswrapper breaks,
> the kernel loose an awful lot of testers. In the span of time before
> Giri and the VMware team post a patch (-rc1 and -rc4, tipically), my
> testing activity is just occasional. And I guess a lot of people is in
> the same situation. 
>
> These are just my 2cents. I will continue to test new kernels every time
> I can, and to use native solutions as often as I can (go, ath5k, go!;
> and LabWindows/CVI for Linux, anyone?). But maybe a bit of tolerance can
> help everyone...

As a kernel developer let me say thank you for doing what testing you can.

I think a bit of tolerance for others can help the conversation.  At the
same time since out of tree modules (even GPL'd ones) have not chosen
to play with us we have to move forward as best we can without their
input.  It isn't possible to do anything else.

Right now I have made some changes for good technical reasons, and
some out of tree modules have broken.  Regardless of the flavor of
EXPORT_SYMBOL they would have broken.

Based on my experience with in-tree code and the few glimpses I
have gotten of out of tree code the reason the out of tree code broke
is because it is doing very questionable things.

So the best I can say at this point, is my apologies that we have not
served you better and made it possible to do what you need to do
without relying on code of questionable character.  Hopefully this
situation will be better in the future.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03 18:03           ` Eric W. Biederman
@ 2007-12-03 18:13             ` David Miller
  0 siblings, 0 replies; 64+ messages in thread
From: David Miller @ 2007-12-03 18:13 UTC (permalink / raw)
  To: ebiederm; +Cc: romanol, lkml, shemminger, linux-kernel, netdev, containers

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 03 Dec 2007 11:03:38 -0700

> Based on my experience with in-tree code and the few glimpses I
> have gotten of out of tree code the reason the out of tree code broke
> is because it is doing very questionable things.

Calling dev_get_by_foo() was never ever a very questionable thing.
Stop saying bullshit, because that's all that is coming out of your
mouth in this thread.

The fact is, these modules called perfectly fine interfaces and by
adding namespaces YOU BROKE THEM.

That by itself is OK, they can make the code changes to adapt and use
the init namespace.

Enforcing new licensing restrictions on them for existing interfaces
just because you add a new freaking argument that is practically
speaking a constant and always the same right now, on the other hand,
IS NOT FINE and you must fix this now.

I don't care how you do it.

If you don't want them to get at the init namespace symbol, fine,
revert all the dev_get_by_*() interfaces to not take the namespace
symbol and make them internally use the init namespace albeit
invisibly to the caller.

Then you make all the existing call sites invoke new dev_get_by_*_ns()
interfaces that take an explicit argument.  But only do this where it
is truly necessary, everything uses the init namespace practically
speaking and it's clearer if you conver to the *_ns() variant when the
code itself is converted.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03 17:35               ` Eric W. Biederman
@ 2007-12-03 18:19                 ` Ben Greear
  2007-12-03 18:57                   ` Daniel Lezcano
  2007-12-04 17:59                   ` Eric W. Biederman
  0 siblings, 2 replies; 64+ messages in thread
From: Ben Greear @ 2007-12-03 18:19 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Patrick McHardy, Stephen Hemminger, Mark Lord, linux-kernel,
	netdev, containers

Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
>
>   
>> Ben Greear wrote:
>>     
>>> I have a binary module that uses dev_get_by_name...it's sort of a bridge-like
>>> thing and
>>> needs user-space to tell it which device to listen for packets on...
>>>
>>> This code doesn't need or care about name-spaces, so I don't see how it could
>>> really
>>> be infringing on the author's code (any worse than loading a binary driver
>>> into the kernel
>>> ever does).
>>>       
>
> Regardless of infringement it is incompatible with a complete network
> namespace implementation.  Further it sounds like the module you are
> describing defines a kernel ABI without being merged and hopes that
> ABI will still be supportable in the future.  Honestly I think doing so
> is horrible code maintenance policy.
>   
I don't mind if the ABI changes, so long as I can still use something 
similar.

The namespace logic is interesting to me in general, but at this point I 
can't think of a way that
it actually helps this particular module.  All I really need is a way to 
grab every frame
from eth0 and then transmit it to eth1.  I'm currently doing this by 
finding the netdevice
and registering a raw-packet protocol (ie, like tcpdump would do).  At 
least up to 2.6.23,
this does not require any hacks to the kernel and uses only non GPL 
exported symbols.

Based on my understanding of the namespace logic, if I never add any 
namespaces,
the general network layout should look similar to how it does today, so 
I should have
no logical problem with my module.

> Once things are largely complete it makes sense to argue with out of
> tree module authors that because they don't have network namespace
> support in their modules, their modules are broken.   
>   
Does this imply that every module that accesses the network code *must* 
become
GPL simply because it must interact with namespace logic that is 
exported as GPL only symbols?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03 18:19                 ` Ben Greear
@ 2007-12-03 18:57                   ` Daniel Lezcano
  2007-12-04 15:19                     ` Daniel Lezcano
  2007-12-04 18:03                     ` Eric W. Biederman
  2007-12-04 17:59                   ` Eric W. Biederman
  1 sibling, 2 replies; 64+ messages in thread
From: Daniel Lezcano @ 2007-12-03 18:57 UTC (permalink / raw)
  To: Ben Greear
  Cc: Eric W. Biederman, netdev, linux-kernel, containers, Mark Lord,
	Stephen Hemminger

Ben Greear wrote:
> Eric W. Biederman wrote:
>> Patrick McHardy <kaber@trash.net> writes:
>>
>>  
>>> Ben Greear wrote:
>>>    
>>>> I have a binary module that uses dev_get_by_name...it's sort of a 
>>>> bridge-like
>>>> thing and
>>>> needs user-space to tell it which device to listen for packets on...
>>>>
>>>> This code doesn't need or care about name-spaces, so I don't see how 
>>>> it could
>>>> really
>>>> be infringing on the author's code (any worse than loading a binary 
>>>> driver
>>>> into the kernel
>>>> ever does).
>>>>       
>>
>> Regardless of infringement it is incompatible with a complete network
>> namespace implementation.  Further it sounds like the module you are
>> describing defines a kernel ABI without being merged and hopes that
>> ABI will still be supportable in the future.  Honestly I think doing so
>> is horrible code maintenance policy.
>>   
> I don't mind if the ABI changes, so long as I can still use something 
> similar.
> 
> The namespace logic is interesting to me in general, but at this point I 
> can't think of a way that
> it actually helps this particular module.  All I really need is a way to 
> grab every frame
> from eth0 and then transmit it to eth1.  I'm currently doing this by 
> finding the netdevice
> and registering a raw-packet protocol (ie, like tcpdump would do).  At 
> least up to 2.6.23,
> this does not require any hacks to the kernel and uses only non GPL 
> exported symbols.
> 
> Based on my understanding of the namespace logic, if I never add any 
> namespaces,
> the general network layout should look similar to how it does today, so 
> I should have
> no logical problem with my module.
> 
>> Once things are largely complete it makes sense to argue with out of
>> tree module authors that because they don't have network namespace
>> support in their modules, their modules are broken.     
> Does this imply that every module that accesses the network code *must* 
> become
> GPL simply because it must interact with namespace logic that is 
> exported as GPL only symbols?

That's right, with init_net's EXPORT_SYMBOL_GPL and dev_get_xx, we 
enforce people to be GPL whatever they didn't asked to have the 
namespaces in their code.

Eric, why can we simply change EXPORT_SYMBOL_GPL to EXPORT_SYMBOL for 
init_net ?



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03 18:57                   ` Daniel Lezcano
@ 2007-12-04 15:19                     ` Daniel Lezcano
  2007-12-04 18:03                     ` Eric W. Biederman
  1 sibling, 0 replies; 64+ messages in thread
From: Daniel Lezcano @ 2007-12-04 15:19 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Ben Greear, netdev, linux-kernel, Eric W. Biederman, containers,
	Mark Lord, Stephen Hemminger

Daniel Lezcano wrote:
> Ben Greear wrote:
>> Eric W. Biederman wrote:
>>> Patrick McHardy <kaber@trash.net> writes:
>>>
>>>  
>>>> Ben Greear wrote:
>>>>   
>>>>> I have a binary module that uses dev_get_by_name...it's sort of a 
>>>>> bridge-like
>>>>> thing and
>>>>> needs user-space to tell it which device to listen for packets on...
>>>>>
>>>>> This code doesn't need or care about name-spaces, so I don't see 
>>>>> how it could
>>>>> really
>>>>> be infringing on the author's code (any worse than loading a binary 
>>>>> driver
>>>>> into the kernel
>>>>> ever does).
>>>>>       
>>>
>>> Regardless of infringement it is incompatible with a complete network
>>> namespace implementation.  Further it sounds like the module you are
>>> describing defines a kernel ABI without being merged and hopes that
>>> ABI will still be supportable in the future.  Honestly I think doing so
>>> is horrible code maintenance policy.
>>>   
>> I don't mind if the ABI changes, so long as I can still use something 
>> similar.
>>
>> The namespace logic is interesting to me in general, but at this point 
>> I can't think of a way that
>> it actually helps this particular module.  All I really need is a way 
>> to grab every frame
>> from eth0 and then transmit it to eth1.  I'm currently doing this by 
>> finding the netdevice
>> and registering a raw-packet protocol (ie, like tcpdump would do).  At 
>> least up to 2.6.23,
>> this does not require any hacks to the kernel and uses only non GPL 
>> exported symbols.
>>
>> Based on my understanding of the namespace logic, if I never add any 
>> namespaces,
>> the general network layout should look similar to how it does today, 
>> so I should have
>> no logical problem with my module.
>>
>>> Once things are largely complete it makes sense to argue with out of
>>> tree module authors that because they don't have network namespace
>>> support in their modules, their modules are broken.     
>> Does this imply that every module that accesses the network code 
>> *must* become
>> GPL simply because it must interact with namespace logic that is 
>> exported as GPL only symbols?
> 
> That's right, with init_net's EXPORT_SYMBOL_GPL and dev_get_xx, we 
> enforce people to be GPL whatever they didn't asked to have the 
> namespaces in their code.
> 
> Eric, why can we simply change EXPORT_SYMBOL_GPL to EXPORT_SYMBOL for 
> init_net ?

Another suggestion/question, is it acceptable to say non-gpl driver 
should use init_task.nsproxy->net_ns instead of &init_net ?

Or does it make sense to have init_net gpl-exported, since we can access 
it through init_task which is exported without gpl mention ?


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03 18:19                 ` Ben Greear
  2007-12-03 18:57                   ` Daniel Lezcano
@ 2007-12-04 17:59                   ` Eric W. Biederman
  2007-12-04 18:57                     ` Ben Greear
  1 sibling, 1 reply; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-04 17:59 UTC (permalink / raw)
  To: Ben Greear
  Cc: Patrick McHardy, Stephen Hemminger, Mark Lord, linux-kernel,
	netdev, containers, David Miller

Ben Greear <greearb@candelatech.com> writes:

>> Regardless of infringement it is incompatible with a complete network
>> namespace implementation.  Further it sounds like the module you are
>> describing defines a kernel ABI without being merged and hopes that
>> ABI will still be supportable in the future.  Honestly I think doing so
>> is horrible code maintenance policy.
>>
> I don't mind if the ABI changes, so long as I can still use something similar.

It has occurred to me that I am seeing an implication here that may in fact not
exist.

My impression of dev_get_by_xxxx is that the function is only able to be used
sanely when being part of the definition of a kernel/userspace interface.  With
the further assumption on my part that you need to define a new instance of
dev_get_by_xxxx 

It has just occurred to me that it is possible to reuse the SIOCBRADDIF
and SIOCBRDELIF for that same purpose without defining a new kernel/userspace
interface.

What and how are you using dev_get_by_xxx?

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-03 18:57                   ` Daniel Lezcano
  2007-12-04 15:19                     ` Daniel Lezcano
@ 2007-12-04 18:03                     ` Eric W. Biederman
  2007-12-04 18:44                       ` Ben Greear
  2007-12-05  6:01                       ` David Miller
  1 sibling, 2 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-04 18:03 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Ben Greear, netdev, linux-kernel, containers, Mark Lord,
	Stephen Hemminger, David Miller

Daniel Lezcano <daniel.lezcano@free.fr> writes:

> Ben Greear wrote:

>>> Once things are largely complete it makes sense to argue with out of
>>> tree module authors that because they don't have network namespace
>>> support in their modules, their modules are broken.
>> Does this imply that every module that accesses the network code *must* become
>> GPL simply because it must interact with namespace logic that is exported as
>> GPL only symbols?
>
> That's right, with init_net's EXPORT_SYMBOL_GPL and dev_get_xx, we enforce
> people to be GPL whatever they didn't asked to have the namespaces in their
> code.
>
> Eric, why can we simply change EXPORT_SYMBOL_GPL to EXPORT_SYMBOL for init_net ?

Hmm.  I need to think this one through.

EXPORT_SYMBOL_GPL acts as a strong hint, and a hindrance to using
symbols in a non-GPL'd module.  Not exactly an enforcement mechanism.

...

The current pattern is to first change the code to only work in the
initial network namespace.  Which can usually be done with a few
trivial lines of code that utilize init_net.

Then the pattern is to move the globals (or at least a pointer to
them) into struct net, and utilize register_pernet_subsys to ensure
those variables are properly initialized and cleaned up after.

However there also seem to be simpler cases like Ben's bridge module,
that don't appear to have any global state.

Ben I don't have a clue how your user space interface works.  My gut
feel is that you can likely use sk->sk_net (if your configuration is
through a socket), or failing that current->nsproxy->net_ns.  To get
the network namespace to look up "eth0" and "eth1".

This however still begs the question how do we want to handle this
so there is a minimum of pain.

Since using register_pernet_subsys implies you need your own member
in struct net.  I am inclined to leave that with the GPL hint on
the EXPORT as you need to be really tight with the system to use that.

...

Currently I don't know if the _GPL hint on the export of init_net buys
us anything except trouble so I am almost inclined to do something
there.

....

What really disturbs me is that as I look at this I see that we have
historically at least done a very haphazard job of maintaining our
kernel/userspace ABIs while making a commitment to maintain them
forever.  Especially if as it seems that some would see that
commitment extending beyond the code that is ever potentially
mergable with the kernel.

....

Currently the only angle that I can see that makes sense to me in the
argument for change of how we are currently doing things is that by
adding a parameter to new existing functions I make it very difficult
for code with network namespace support to have one version that works
on both old and new kernels as we can not define the new API on the
old hardware.

I can see some technical merit in making that case better.

.....

My thinking on the namespaces have been that their interfaces are new
core kernel interfaces that have not existed on any other kernel.  And
as such any code that needed to use those interfaces was:
a) definitely a derived work of the kernel.
b) was a core part of the kernel, and we don't even want normal
   day to day drivers using those interfaces much less weird
   random code outside of the kernel.

The above is why I habitually place a _GPL hint on my exports
of namespace related functions and data.  To strongly suggest
to module authors that they are getting into hot water if
they use these interfaces and don't merge their code.

So far I really don't see anything to challenge my understanding above
but I am human and as such my heuristics for analysis and understanding
are not guaranteed to give me the right answer.

....

I don't want this to be a stupid political fight about GPL stuff.
Generally I am with Alan in not seeing any basis for distributing
non-GPL code that works in the kernel.  Although I see Linus' point
that a legal case may be made that certain modules are not a
derivative work of the kernel.

...

I am confused.  I don't see a path forward that feels right.
So I am going to sit and think about this some more, before I do anything.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 18:03                     ` Eric W. Biederman
@ 2007-12-04 18:44                       ` Ben Greear
  2007-12-04 19:17                         ` Eric W. Biederman
  2007-12-05  6:01                       ` David Miller
  1 sibling, 1 reply; 64+ messages in thread
From: Ben Greear @ 2007-12-04 18:44 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Daniel Lezcano, netdev, linux-kernel, containers, Mark Lord,
	Stephen Hemminger, David Miller

Eric W. Biederman wrote:
> However there also seem to be simpler cases like Ben's bridge module,
> that don't appear to have any global state.
>   
Well, my module has some global state, but I don't think it needs to 
care about
namespaces.  My first impression is that my module should be able to bridge
namespaces...not be contained within one.   I can have user-space make 
sure that I don't bridge between
devices in different name-spaces, or perhaps bridging between namespaces
wouldn't be a problem anyway.  If I *do* need to add some sort of namespace
awareness to just achieve today's functionality, I don't mind making the 
changes,
so long as I don't need to change to GPL licensing.  Perhaps at the 
least you
can export enough symbols w/out GPL tag to achieve backwards compat with .23
and previous kernels, or rework dev_get_by_* etc to not need GPL'd namespace
symbols and just return the device in the default namespace?
> Ben I don't have a clue how your user space interface works.  My gut
> feel is that you can likely use sk->sk_net (if your configuration is
> through a socket), or failing that current->nsproxy->net_ns.  To get
> the network namespace to look up "eth0" and "eth1".
>   
Currently I use procfs and ioctls bound to a procfs file descriptor.

For namespaces in general, will there be a way to just do a dev_get_by_* 
and find the
device in *any* namespace and query the device to see what namespace it 
is in?
Then my module or some other more clever piece of code can determine the 
namespaces
(by comparing pointers if nothing else) and make proper decision.  For 
instance, maybe
we want to bridge two namespaces, or maybe we want to forbid that ever 
happening...
> This however still begs the question how do we want to handle this
> so there is a minimum of pain.
>
> Since using register_pernet_subsys implies you need your own member
> in struct net.  I am inclined to leave that with the GPL hint on
> the EXPORT as you need to be really tight with the system to use that.
>   
I certainly don't want to have to muck with struct net unless you have 
some way to
register anonymous (and non GPL) subsystems.  I'm guessing you do not 
want to
support that....

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 17:59                   ` Eric W. Biederman
@ 2007-12-04 18:57                     ` Ben Greear
  2007-12-04 20:01                       ` Eric W. Biederman
  2007-12-05  6:07                       ` David Miller
  0 siblings, 2 replies; 64+ messages in thread
From: Ben Greear @ 2007-12-04 18:57 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Patrick McHardy, Stephen Hemminger, Mark Lord, linux-kernel,
	netdev, containers, David Miller

Eric W. Biederman wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>   
>>> Regardless of infringement it is incompatible with a complete network
>>> namespace implementation.  Further it sounds like the module you are
>>> describing defines a kernel ABI without being merged and hopes that
>>> ABI will still be supportable in the future.  Honestly I think doing so
>>> is horrible code maintenance policy.
>>>
>>>       
>> I don't mind if the ABI changes, so long as I can still use something similar.
>>     
>
> It has occurred to me that I am seeing an implication here that may in fact not
> exist.
>
> My impression of dev_get_by_xxxx is that the function is only able to be used
> sanely when being part of the definition of a kernel/userspace interface.  With
> the further assumption on my part that you need to define a new instance of
> dev_get_by_xxxx 
>
> It has just occurred to me that it is possible to reuse the SIOCBRADDIF
> and SIOCBRDELIF for that same purpose without defining a new kernel/userspace
> interface.
>
> What and how are you using dev_get_by_xxx?
>   
I have a module that has a collection of 2-port bridges.  These bridges 
are used for emulation
purposes (somewhat similar to netem's feature set).  Each bridge is 
logically independent
of the others.   To set up a bridge, I do something like:

echo add_my_bridge my_br1 eth0 eth1 > /proc/net/foo/config

Inside the module, it reads "eth0" and "eth1" and needs to find those
devices (ie, dev_get_by_name).  It then registers to receive all pkts from
eth1 and transmit them on eth0, and vice versa.

If it would not require GPL symbols, I have no problem changing my API 
to be something
like:

echo add_my_bridge my_br1 eth0 namespaceX eth1 namespaceY > 
/proc/net/foo/config

I am using procfs so that I don't have to define any new 'official'
kernel ABI, as that would more likely be a derivative work, and is a pain
to keep up to date with changing kernels anyway...

Personally, it seems useful for my module to be able to have eth0 in one 
namespace
and eth1 in another, but I won't complain if they both have to be in the 
same namespace
or even just in the default namespace due to GPL symbol issues.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 18:44                       ` Ben Greear
@ 2007-12-04 19:17                         ` Eric W. Biederman
  2007-12-04 19:35                           ` Ben Greear
  2007-12-05  6:14                           ` David Miller
  0 siblings, 2 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-04 19:17 UTC (permalink / raw)
  To: Ben Greear
  Cc: Daniel Lezcano, netdev, linux-kernel, containers, Mark Lord,
	Stephen Hemminger, David Miller

Ben Greear <greearb@candelatech.com> writes:

> Eric W. Biederman wrote:
>> However there also seem to be simpler cases like Ben's bridge module,
>> that don't appear to have any global state.
>>
> Well, my module has some global state, but I don't think it needs to care about
> namespaces.  My first impression is that my module should be able to bridge
> namespaces...not be contained within one.  I can have user-space make sure that
> I don't bridge between
> devices in different name-spaces, or perhaps bridging between namespaces
> wouldn't be a problem anyway. 

Bridging between namespaces should not be a problem, but it could be
a bit of a challenge to setup (in finding the network devices).
Probably the easy way is to setup the bridging and then move one of the
network devices to the other network namespace.

Essentially bridging between two network devices in two network
namespaces looks like bridging between two network devices on two
separate network stacks.   Although internally things look a little
better.

> If I *do* need to add some sort of namespace
> awareness to just achieve today's functionality, I don't mind making the
> changes,
> so long as I don't need to change to GPL licensing.  Perhaps at the least you
> can export enough symbols w/out GPL tag to achieve backwards compat with .23
> and previous kernels, or rework dev_get_by_* etc to not need GPL'd namespace
> symbols and just return the device in the default namespace?

IANAL but to me your code sounds like a derivative work of the linux
kernel.  Which implies that if you are distributing your module you
need to change to GPL licensing.  The _GPL tag on EXPORT_SYMBOL does
not change those rules.

>> Ben I don't have a clue how your user space interface works.  My gut
>> feel is that you can likely use sk->sk_net (if your configuration is
>> through a socket), or failing that current->nsproxy->net_ns.  To get
>> the network namespace to look up "eth0" and "eth1".
>>
> Currently I use procfs and ioctls bound to a procfs file descriptor.

Which is where it gets tricky   You are defining new userspace ABIs.
I can see where they occasionally make sense during development
and prototyping but long term out of tree userspace interfaces appear
to me to be a real maintenance problem.

> For namespaces in general, will there be a way to just do a dev_get_by_* and
> find the
> device in *any* namespace and query the device to see what namespace it is in?
> Then my module or some other more clever piece of code can determine the
> namespaces
> (by comparing pointers if nothing else) and make proper decision.  For instance,
> maybe
> we want to bridge two namespaces, or maybe we want to forbid that ever
> happening...

The issue is that fundamentally all userspace device identifiers can
be duped between namespaces.  So since there is no unique identifier
we can not implement a function to do that.

>> This however still begs the question how do we want to handle this
>> so there is a minimum of pain.
>>
>> Since using register_pernet_subsys implies you need your own member
>> in struct net.  I am inclined to leave that with the GPL hint on
>> the EXPORT as you need to be really tight with the system to use that.
>>
> I certainly don't want to have to muck with struct net unless you have some way
> to
> register anonymous (and non GPL) subsystems.  I'm guessing you do not want to
> support that....

Well I don't see a license compatible way to have any GPL incompatible
licensed linux kernel code.  Off hand that means code needs to be
licensed under the GPL or BSD without advertising clause.

Does EXPORT_SYMBOL_GPL complain if you have a BSD licensed module?

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 19:17                         ` Eric W. Biederman
@ 2007-12-04 19:35                           ` Ben Greear
  2007-12-04 20:09                             ` Eric W. Biederman
  2007-12-05  6:14                           ` David Miller
  1 sibling, 1 reply; 64+ messages in thread
From: Ben Greear @ 2007-12-04 19:35 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Daniel Lezcano, netdev, linux-kernel, containers, Mark Lord,
	Stephen Hemminger, David Miller

Eric W. Biederman wrote:
> Ben Greear <greearb@candelatech.com> writes:
> 
>> Eric W. Biederman wrote:
>>> However there also seem to be simpler cases like Ben's bridge module,
>>> that don't appear to have any global state.
>>>
>> Well, my module has some global state, but I don't think it needs to care about
>> namespaces.  My first impression is that my module should be able to bridge
>> namespaces...not be contained within one.  I can have user-space make sure that
>> I don't bridge between
>> devices in different name-spaces, or perhaps bridging between namespaces
>> wouldn't be a problem anyway. 
> 
> Bridging between namespaces should not be a problem, but it could be
> a bit of a challenge to setup (in finding the network devices).
> Probably the easy way is to setup the bridging and then move one of the
> network devices to the other network namespace.
> 
> Essentially bridging between two network devices in two network
> namespaces looks like bridging between two network devices on two
> separate network stacks.   Although internally things look a little
> better.

Ok, that sounds fine.

>> Currently I use procfs and ioctls bound to a procfs file descriptor.
> 
> Which is where it gets tricky   You are defining new userspace ABIs.
> I can see where they occasionally make sense during development
> and prototyping but long term out of tree userspace interfaces appear
> to me to be a real maintenance problem.

They are completely contained within my module, and no one is going
to change my module w/out me knowing, so actually I have very little
problem here :)

>> For namespaces in general, will there be a way to just do a dev_get_by_* and
>> find the
>> device in *any* namespace and query the device to see what namespace it is in?
>> Then my module or some other more clever piece of code can determine the
>> namespaces
>> (by comparing pointers if nothing else) and make proper decision.  For instance,
>> maybe
>> we want to bridge two namespaces, or maybe we want to forbid that ever
>> happening...
> 
> The issue is that fundamentally all userspace device identifiers can
> be duped between namespaces.  So since there is no unique identifier
> we can not implement a function to do that.

Ok, but can a netdev at least know what namespace it is in?  I don't
need this for my module, but it seems very useful knowledge...

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 18:57                     ` Ben Greear
@ 2007-12-04 20:01                       ` Eric W. Biederman
  2007-12-05  6:07                       ` David Miller
  1 sibling, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-04 20:01 UTC (permalink / raw)
  To: Ben Greear
  Cc: Patrick McHardy, Stephen Hemminger, Mark Lord, linux-kernel,
	netdev, containers, David Miller

Ben Greear <greearb@candelatech.com> writes:

> I have a module that has a collection of 2-port bridges.  These bridges are used
> for emulation
> purposes (somewhat similar to netem's feature set).  Each bridge is logically
> independent
> of the others.   To set up a bridge, I do something like:
>
> echo add_my_bridge my_br1 eth0 eth1 > /proc/net/foo/config

Interesting.  Currently /proc/net is also per network namespace.
So normally I would say just call get_proc_net from inside your
proc handler and all would be well.

At another location in /proc (not under /proc/net) I would just do:
dev_get_by_name(current->nsproxy->net_ns, "ethX");

I would probably be paranoid and grab current->nsproxy->net_ns
when the file was opened and put it when the file was closed
just to ensure that if someone opened it and then passed
the file descriptor to someone else there were not any
weird little races.  But I don't expect that is a problem
in your case.

> Personally, it seems useful for my module to be able to have eth0 in one
> namespace
> and eth1 in another, but I won't complain if they both have to be in the same
> namespace
> or even just in the default namespace due to GPL symbol issues.

It probably is easiest to move the devices after your module has
bridged them.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 19:35                           ` Ben Greear
@ 2007-12-04 20:09                             ` Eric W. Biederman
  0 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-04 20:09 UTC (permalink / raw)
  To: Ben Greear
  Cc: Daniel Lezcano, netdev, linux-kernel, containers, Mark Lord,
	Stephen Hemminger, David Miller

Ben Greear <greearb@candelatech.com> writes:

> Ok, but can a netdev at least know what namespace it is in?  I don't
> need this for my module, but it seems very useful knowledge...

Sure.   dev->nd_net
It is a don't care not a don't know, and there should be device
events when it goes in and out of a network namespace.

I don't know if the device gets those or not.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 18:03                     ` Eric W. Biederman
  2007-12-04 18:44                       ` Ben Greear
@ 2007-12-05  6:01                       ` David Miller
  1 sibling, 0 replies; 64+ messages in thread
From: David Miller @ 2007-12-05  6:01 UTC (permalink / raw)
  To: ebiederm
  Cc: daniel.lezcano, greearb, netdev, linux-kernel, containers, lkml,
	shemminger

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Tue, 04 Dec 2007 11:03:01 -0700

> I am confused.  I don't see a path forward that feels right.

Eric, instead of writing a book about how you feel, look
at the simple facts and resolve this quickly.

You added a new key, the namespace, to the looking up of
network objects.

Big deal.

That does not imply a change in licensing for the interfaces
where you added that new aspect of the key to the argument
list.

This symbol licensing decision was not your's to make, so you must
revert the new licensing change you are enforcing upon everyone.

I want a de-GPL patch in my mailbox from you within the next 24 hours
or I will code one up myself.  This is getting beyond rediculious.

My patience is completely gone on this matter, resolve this now.

Thanks.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 18:57                     ` Ben Greear
  2007-12-04 20:01                       ` Eric W. Biederman
@ 2007-12-05  6:07                       ` David Miller
  1 sibling, 0 replies; 64+ messages in thread
From: David Miller @ 2007-12-05  6:07 UTC (permalink / raw)
  To: greearb
  Cc: ebiederm, kaber, shemminger, lkml, linux-kernel, netdev, containers

From: Ben Greear <greearb@candelatech.com>
Date: Tue, 04 Dec 2007 10:57:01 -0800

> echo add_my_bridge my_br1 eth0 namespaceX eth1 namespaceY > 
> /proc/net/foo/config

Each process executes in a namespace, so specifying the namespace
is redundant, just fetch the current process's namespace to pass
into the dev_get_by_*() routines.

Anyone interested in using a different namespace's devices
can change the namespace the process is executing in before
the procfs echo.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: namespace support requires network modules to say "GPL"
  2007-12-04 19:17                         ` Eric W. Biederman
  2007-12-04 19:35                           ` Ben Greear
@ 2007-12-05  6:14                           ` David Miller
  1 sibling, 0 replies; 64+ messages in thread
From: David Miller @ 2007-12-05  6:14 UTC (permalink / raw)
  To: ebiederm
  Cc: greearb, daniel.lezcano, netdev, linux-kernel, containers, lkml,
	shemminger

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Tue, 04 Dec 2007 12:17:57 -0700

> Ben Greear <greearb@candelatech.com> writes:
> 
> > If I *do* need to add some sort of namespace
> > awareness to just achieve today's functionality, I don't mind making the
> > changes,
> > so long as I don't need to change to GPL licensing.  Perhaps at the least you
> > can export enough symbols w/out GPL tag to achieve backwards compat with .23
> > and previous kernels, or rework dev_get_by_* etc to not need GPL'd namespace
> > symbols and just return the device in the default namespace?
> 
> IANAL but to me your code sounds like a derivative work of the linux
> kernel.  Which implies that if you are distributing your module you
> need to change to GPL licensing.  The _GPL tag on EXPORT_SYMBOL does
> not change those rules.

Eric, YANAL and you are also full of hot air.  You are really
testing my patience on this issue.

You fail to ever describe on what factual basis you are making
these claims.  And the reason is that you have ZERO factual basis
for your claims.

Here are the facts:

1) Never, ever, have the function for looking up network devices been
   classified as GPL-only symbols.

   They provide a device based upon a lookup key.

2) You in no way have changed what those functions do in any way
   whatsoever.  They still provide a reference to a network device
   based upon a given lookup key.

   The functions are still doing the same thing they always have.

Therefore, you have decided to uniliaterally change the licensing of
these functions based solely upon your opinion, and not because of
some real change you've made to the code in question.

You have no right to do this.

This is unreasonable, and you must fix this immediately.

And I do mean now, not after you've written several more excessively
long diatribes about how you feel in this matter.

Thank you.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH 0/10] sysfs network namespace support
  2007-12-01  9:06 [PATCH 0/10] sysfs network namespace support Eric W. Biederman
  2007-12-01  9:12 ` [PATCH 01/10] sysfs: Make sysfs_mount static again Eric W. Biederman
  2007-12-01 13:10 ` namespace support requires network modules to say "GPL" Mark Lord
@ 2007-12-21  3:07 ` Greg KH
  2007-12-21 13:04   ` Eric W. Biederman
  2 siblings, 1 reply; 64+ messages in thread
From: Greg KH @ 2007-12-21  3:07 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg Kroah-Hartman, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller

On Sat, Dec 01, 2007 at 02:06:58AM -0700, Eric W. Biederman wrote:
> 
> Now that we have network namespace support merged it is time to
> revisit the sysfs support so we can remove the dependency on !SYSFS.

<snip>

Oops, I forgot to apply this to my tree.  Eric, you still want this
submitted, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH 0/10] sysfs network namespace support
  2007-12-21  3:07 ` [PATCH 0/10] sysfs network namespace support Greg KH
@ 2007-12-21 13:04   ` Eric W. Biederman
  0 siblings, 0 replies; 64+ messages in thread
From: Eric W. Biederman @ 2007-12-21 13:04 UTC (permalink / raw)
  To: Greg KH
  Cc: Greg Kroah-Hartman, Tejun Heo, Linux Containers, netdev,
	cornelia.huck, stern, kay.sievers, linux-kernel, Andrew Morton,
	Herbert Xu, David Miller

Greg KH <greg@kroah.com> writes:

> On Sat, Dec 01, 2007 at 02:06:58AM -0700, Eric W. Biederman wrote:
>> 
>> Now that we have network namespace support merged it is time to
>> revisit the sysfs support so we can remove the dependency on !SYSFS.
>
> <snip>
>
> Oops, I forgot to apply this to my tree.  Eric, you still want this
> submitted, right?

Yes.

I'm am just about to head out of town to visit my parents over Christmas.
So I'm not going to be very responsive until I after the New Year.

Eric

^ permalink raw reply	[flat|nested] 64+ messages in thread

end of thread, other threads:[~2007-12-21 13:07 UTC | newest]

Thread overview: 64+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-01  9:06 [PATCH 0/10] sysfs network namespace support Eric W. Biederman
2007-12-01  9:12 ` [PATCH 01/10] sysfs: Make sysfs_mount static again Eric W. Biederman
2007-12-01  9:13   ` [PATCH 02/10] sysfs: Support for preventing unmounts Eric W. Biederman
2007-12-01  9:16     ` [PATCH 03/10] sysfs: sysfs_get_dentry add a sb parameter Eric W. Biederman
2007-12-01  9:18       ` [PATCH 04/10] sysfs: Implement __sysfs_get_dentry Eric W. Biederman
2007-12-01  9:23         ` [PATCH 05/10] sysfs: Rename Support multiple superblocks Eric W. Biederman
2007-12-01  9:25           ` [PATCH 06/10] sysfs: sysfs_chmod_file handle " Eric W. Biederman
2007-12-01  9:28             ` [PATCH 07/10] sysfs: Implement sysfs tagged directory support Eric W. Biederman
2007-12-01  9:30               ` [PATCH 08/10] sysfs: Implement sysfs_delete_link and sysfs_rename_link Eric W. Biederman
2007-12-01  9:33                 ` [PATCH 09/10] driver core: Implement tagged directory support for device classes Eric W. Biederman
2007-12-01  9:35                   ` [PATCH 10/10] net: Enable tagging for net_class directories in sysfs Eric W. Biederman
2007-12-01 13:10 ` namespace support requires network modules to say "GPL" Mark Lord
2007-12-01 13:13   ` Mark Lord
2007-12-01 19:17   ` Stephen Hemminger
2007-12-01 19:23     ` Alan Cox
2007-12-01 19:38       ` Stephen Hemminger
2007-12-01 19:45         ` Alan Cox
2007-12-01 20:13         ` Eric W. Biederman
2007-12-01 20:21           ` Mark Lord
2007-12-01 20:29             ` Arjan van de Ven
2007-12-01 22:12               ` Mark Lord
2007-12-01 23:13                 ` Eric W. Biederman
2007-12-01 23:24                   ` Jiri Slaby
2007-12-02  1:14                     ` Eric W. Biederman
2007-12-01 23:51                   ` Mark Lord
2007-12-02  1:08                     ` Eric W. Biederman
2007-12-01 20:52             ` Eric W. Biederman
2007-12-01 22:13             ` Mark Lord
2007-12-03  0:02       ` David Schwartz
2007-12-03  0:14         ` Alan Cox
2007-12-01 19:54     ` Eric W. Biederman
2007-12-02  0:30     ` Stephen Hemminger
2007-12-02  2:02       ` Eric W. Biederman
2007-12-02  3:34       ` Mark Lord
2007-12-02  4:23         ` Stephen Hemminger
2007-12-02 19:28           ` Ben Greear
2007-12-02 20:03             ` Patrick McHardy
2007-12-02 20:43               ` Adrian Bunk
2007-12-02 21:59                 ` Patrick McHardy
2007-12-03  1:14                   ` Adrian Bunk
2007-12-03  8:33                   ` Denis V. Lunev
2007-12-03 17:35               ` Eric W. Biederman
2007-12-03 18:19                 ` Ben Greear
2007-12-03 18:57                   ` Daniel Lezcano
2007-12-04 15:19                     ` Daniel Lezcano
2007-12-04 18:03                     ` Eric W. Biederman
2007-12-04 18:44                       ` Ben Greear
2007-12-04 19:17                         ` Eric W. Biederman
2007-12-04 19:35                           ` Ben Greear
2007-12-04 20:09                             ` Eric W. Biederman
2007-12-05  6:14                           ` David Miller
2007-12-05  6:01                       ` David Miller
2007-12-04 17:59                   ` Eric W. Biederman
2007-12-04 18:57                     ` Ben Greear
2007-12-04 20:01                       ` Eric W. Biederman
2007-12-05  6:07                       ` David Miller
2007-12-03  8:24         ` Romano Giannetti
2007-12-03 15:34           ` Arjan van de Ven
2007-12-03 18:03           ` Eric W. Biederman
2007-12-03 18:13             ` David Miller
2007-12-02 13:51       ` Alan Cox
2007-12-02 19:56         ` Valdis.Kletnieks
2007-12-21  3:07 ` [PATCH 0/10] sysfs network namespace support Greg KH
2007-12-21 13:04   ` Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).